g = newGlobal(); gcparam('maxBytes', gcparam('gcBytes')); evaluate("return 0", ({ global: g, newContext: true })); crashes js debug shell on m-c changeset 2a193b7f395c with --fuzzing-safe --no-threads --no-ion at: $ 2015-01-06/js --fuzzing-safe --no-threads --no-ion 40.js out of memory *** Compartment mismatch 0x101c77000 vs. 0x101c81800 Hit MOZ_CRASH() at /builds/slave/m-cen-osx64-d-0000000000000000/build/src/js/src/jscntxtinlines.h:52 Segmentation fault: 11 The shell was obtained from: https://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2015-01-06-mozilla-central-debug/jsshell-mac64.zip I cannot get a stack from the FTP builds as the builds from FTP do not have symbols. Not sure exactly what issue this is, so setting needinfo? from Terrence and Jon as a start, and setting s-s because compartment mismatches seem bad.

I think AutoNewContext just needs to wrap the exception into the old compartment. This is shell-only code so I don't think it's a security issue.

Comment on attachment 8545896 [details] [diff] [review] bug1118996-AutoNewContext Review of attachment 8545896 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/src/shell/js.cpp @@ +990,5 @@ > if (throwing) > JS_GetPendingException(newcx, &exc); > newCompartment.reset(); > newRequest.reset(); > + if (throwing && JS_WrapValue(oldcx, &exc)) So if JS_WrapValue fails, this will throw its failure exception if it sets one, otherwise it seems like it will leave oldcx with an uncatchable exception. Uh, I guess that's about as good as it can do.

Backed out because the test case doesn't always fail with OOM - sometimes it fails with SyntaxError. https://hg.mozilla.org/integration/mozilla-inbound/rev/936382a9e919

Requesting re-review for the following changes: - catch the syntax error in case zeal or something else makes the test case not trigger the OOM - strengthen the jit-tests harness' definition of OOM to make this testcase fail without the fix applied - also add the correct options to make the testcase fail without the fix

Comment on attachment 8546731 [details] [diff] [review] bug1118996-AutoNewContext-v2 Review of attachment 8546731 [details] [diff] [review]: ----------------------------------------------------------------- I'm going to be a sleazeball and mark this r+ even though I don't really understand what's going on. If this fixes the problem, then it seems like it's a nice minimal fix and I'd rather it just get in the tree. But: ::: js/src/jit-test/tests/basic/bug1118996.js @@ +4,5 @@ > +try { > + evaluate("return 0", ({ > + global: g, > + newContext: true > + })); Can you comment here or in the catch() that this is expected to OOM, but if it doesn't due to zeal settings, then it will throw a "SyntaxError: return not in function"? (It took me a while to figure out why this would throw a SyntaxError.)

https://hg.mozilla.org/projects/cypress/rev/55b18ee8ffb7 https://hg.mozilla.org/projects/cypress/rev/1f8566481a95