The tracer seems perfectly happy to try to create an array with a negative length. Later on, when we try to do something with the array, it hangs (trying to fill it in?).

This just adds a check to ensure the length is non-negative.

I think this is fallout from bug 620029. Is there a test for this behaviour? I got no failures when I tested before commit? I got this review: <quote> > RecordingStatus > TraceRecorder::newArray(JSObject* ctor, uint32 argc, Value* argv, Value* rval) > { ... >+ } else if (argc == 1 && argv[0].isNumber()) { >+ /* Abort on RangeError if the double doesn't fit in a uint. */ >+ LIns* num_ins; >+ CHECK_STATUS(makeNumberInt32(get(argv), &num_ins)); >+ >+ LIns *args[] = { proto_ins, num_ins, cx_ins }; >+ arr_ins = w.call(&js::NewDenseUnallocatedArray_ci, args); >+ guard(false, w.eqp0(arr_ins), OOM_EXIT); >+ >+ } else { You changed from using d2i to makeNumberInt32, and these don't seem equivalent in this case, particularly in the cases they optimize. If this was unintentional, you can just put it back to: LIns *args[] = { proto_ins, d2i(get(argv)), cx_ins }; </quote> I think the range test was covered in the makeNumberInt32 call that I erroneously removed.

I think the correct fix is to use makeNumberInt32, as that has an optimizing case which won't call the guard. Is that right? billm, this is fallout from my bug, so feel free to assign it to me.

Comment on attachment 498442 [details] [diff] [review] fix d2i() converts a double to an int32, removing any fractional part in the process. makeNumberInt32() converts a double to an int32, but side-exits if the double was not integral (and fits in 32-bits) in the first place. So I think the patch needs to (a) change d2i() to makeNumberInt32() and (b) side-exit if the value is negative. Actually, is the size is interpreted as a uint32? The comment "Abort on RangeError if the double doesn't fit in a uint" implies it is. In which case I think we need a new function makeNumberUint32(). It would call the already-existing d2u(), which is less optimized that d2u() but that shouldn't matter for this case. No negative check would then be needed.

I think this is what you wanted. NewDenseUnallocatedArray does take an unsigned, so I went with the makeNumberUint32 path.

Comment on attachment 499043 [details] [diff] [review] updated patch >+ // This means "convert double to int if it's integral, otherwise >+ // exit". We first convert the double to an unsigned int, then Nit: Can you say "convert double to uint"? You should also add a jit-test. r=me with that. Thanks!