The following testcase asserts on mozilla-central revision df82be9d89a5 (run with --fuzzing-safe): oomAfterAllocations(26); try { eval('syntax error'); } catch(exn) {}

Jason is working on this one already \o/

D'oh, qref fail.

Rename js_ReportErrorAgain -> js::CallErrorReporter. Replace its check that message is not null with an assertion (and move the check to the one call site that needs it). Make frontend::CompileError::throwError call CallErrorReporter instead of including a copy of its body. Also, delete unused API function JS_ThrowReportedError.

Comment on attachment 8345578 [details] [diff] [review] Part 1 - Make exn_finalize safe when obj's reserved slot was never initialized (due to OOM right after it was allocated). v2 Review of attachment 8345578 [details] [diff] [review]: ----------------------------------------------------------------- Oops, I thought I handled this sufficiently with the init at the top of ErrorObject::init. Maybe not quite. ::: js/src/vm/ErrorObject.h @@ +76,5 @@ > return JSExnType(getReservedSlot(EXNTYPE_SLOT).toInt32()); > } > > JSErrorReport * getErrorReport() const { > + const Value &slot = getReservedSlot(ERROR_REPORT_SLOT); HeapSlot& maybe? @@ +78,5 @@ > > JSErrorReport * getErrorReport() const { > + const Value &slot = getReservedSlot(ERROR_REPORT_SLOT); > + if (slot.isUndefined()) > + return NULL; nullptr @@ +84,1 @@ > return static_cast<JSErrorReport*>(priv); I'd just one-liner this.

Comment on attachment 8345580 [details] [diff] [review] Part 2 - Tidying. v2 Review of attachment 8345580 [details] [diff] [review]: ----------------------------------------------------------------- 30 lines down, eleventy billion exception-handling lines to go.

Comment on attachment 8345581 [details] [diff] [review] Part 3 - Change js_ErrorToException to return true iff cx->throwing was set, and document the convention. Review of attachment 8345581 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/src/jsexn.cpp @@ +671,4 @@ > RootedString messageStr(cx, reportp->ucmessage ? JS_NewUCStringCopyZ(cx, reportp->ucmessage) > : JS_NewStringCopyZ(cx, message)); > if (!messageStr) > + return true; Shouldn't this, and all the |return true;| in here, be |return cx->isExceptionPending();|? Why can't this fail via too-much-recursion, which could return false without setting an exception? ::: js/src/jsexn.h @@ +28,5 @@ > + * Return true if cx->throwing and cx->exception were set. > + * > + * This means that: > + * > + * - If the error is successfuly converted to an exception and stored in successfully

Sigh. Yeah, back to the drawing board.

On second thought, I think taking Jeff's suggestion is best.

Shall we land this now? I can do it if nobody else has time :)

Sorry, it got delayed behind other stuff that kept flunking the try server :-P https://hg.mozilla.org/integration/mozilla-inbound/rev/9d152bc73ece https://hg.mozilla.org/integration/mozilla-inbound/rev/e8e93efa64af https://hg.mozilla.org/integration/mozilla-inbound/rev/327a6af06942

https://hg.mozilla.org/mozilla-central/rev/9d152bc73ece https://hg.mozilla.org/mozilla-central/rev/e8e93efa64af https://hg.mozilla.org/mozilla-central/rev/327a6af06942

Reproduced with the 12/10 mozilla-central ASAN JS shell. Verified as fixed using the 03/25 mozilla-beta ASAN JS shell - I only get "out of memory" messages now.