945754 - Assertion failure: !cx->isExceptionPending(), at ../vm/Interpreter.cpp:3468 due to OOM

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Christian Holler (:decoder)]

Attachment: js-ti-oom.patch

I'm hitting the mentioned assertion during fuzzing and tracked it down to the function EnsureTrackPropertyTypes together with jandem. He suggested that this function should propagate the OOM condition, so I made a small patch to do that. It passes all jit-tests but I'm not exactly sure about the first return. Is it right to return true in this function if the inference is not running at all?

Comment 1

[Brian Hackett [Laid off!]]

Comment on attachment 8341744 [details] [diff] [review]
js-ti-oom.patch

Review of attachment 8341744 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/src/jsinferinlines.h
@@ +429,5 @@
>  
>      if (obj->hasSingletonType()) {
>          AutoEnterAnalysis enter(cx);
>          if (obj->hasLazyType() && !obj->getType(cx)) {
>              cx->compartment()->types.setPendingNukeTypes(cx);

Generally, TI stuff isn't meant to be fallible because it can just setPendingNukeTypes and let execution proceed as normal.  This is no longer strictly necessary but is still nice for clean APIs and so forth, e.g. this patch is missing edits to about 10 calls to EnsureTrackPropertyTypes that now do not check their return value.

Instead of changing the interface to EnsureTrackPropertyTypes, can you just add a cx->clearPendingException() call here?

Comment 2

[Christian Holler (:decoder)]

Attachment: js-ti-oom.patch

Like this? Also fixes the assert for me.

Comment 3

[Christian Holler (:decoder)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/8a7ed8362caa

Comment 4

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/8a7ed8362caa