A crash occurs when compiling asm.js code with the IOFLAGS set to 'range'. A SIGSEGV occurs in the Sprinter initialization code because their is no JSContext: 'context->malloc_(DefaultSize)'. The asm.js compiler does not defined a JSContext when initializing the IonContext, rather it just defines a compartment. There are a good number of uses of 'Sprinter sp(GetIonContext()->cx)' that would be affected. Is there a good reason to not initialize the JSContext for the IonContext when compiling asm.js code? There is some asm.js support for parallel compilation and perhaps it is not thread safe for multiple threads to share a JSContext? If so then an alternative allocator might need to be used for Sprinter?

This patch has the Sprinter use js_malloc if there is no JSContext. It's not yet clear if this is the appropriate patch, but it at least avoids some crashes encountered using the debug code.

Hmm, I expect there will be many crashes if Sprinter::context is NULL (I see a lot of others in jsopcode.cpp). Where is this Sprinter created (we should probably be able to fish out a JSContext for it).

(In reply to Luke Wagner [:luke] from comment #3) > Hmm, I expect there will be many crashes if Sprinter::context is NULL (I see > a lot of others in jsopcode.cpp). Where is this Sprinter created (we should > probably be able to fish out a JSContext for it). Yes, there are lots of Sprinter objects created using the pattern: 'Sprinter sp(GetIonContext()->cx)'. The problems start in the asm.js compiler when the IonContext is initialized without a JSContent. CheckFunctionBody(...) ... IonContext icx(m.cx()->compartment, tempAlloc); CheckFunctionBodiesSequential() ... IonContext icx(m.cx()->compartment, &mirGen->temp());

Ah hah, I see. Is the problem fixed if you s/m.cx()->compartment/m.cx()/ in those two sites?

Yes, this is what I do locally to avoid this crash. Can it have side-effects to use a JSContext instead of the compartment, in the IonContext constructor? Otherwise, the patch is trivial and is already made on my machine.

There would be an issue for parallel compilation, but the check (and associated MIRGraph generation) phase is sequential atm (and will continue to be, necessarily, with the asm.js parser).

Perhaps a fix for this should be landed, one way of the other. If you would prefer to fix this by passing along the JSContext rather than the compartment then just let me know and I'll submit a patch for that?

Actually, looking at this again, JSContext::malloc_/realloc_ are only supposed to be used for long-lived allocations (JSContext does gcMallocBytes accounting to trigger GC; stack-bound allocations don't contribute to GC and so it doesn't make sense to account for them) so we can unconditionally use js_malloc/js_realloc. However, if allocation fails, we should report an error if (cx_ != NULL) (via js_ReportOutOfmemory(cx)).

Thank you for the review. This patch attempts to implement the suggestions. There was an existing 'reportOutOfMemory' method which flags the event and calls 'js_ReportOutOfMemory', and perhaps it would be appropriate to use this path if js_malloc or js_realloc fails?

Comment on attachment 791175 [details] [diff] [review] Use js_malloc and js_realloc for the Sprinter to avoid the need for a JSContext Great, thanks for sticking with it!