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

RESOLVED FIXED in mozilla28

Status

()

Core
JavaScript Engine
--
critical
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: decoder, Assigned: decoder)

Tracking

(Blocks: 2 bugs, {assertion, testcase})

Trunk
mozilla28
x86_64
Linux
assertion, testcase
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

4 years ago
Created attachment 8341744 [details] [diff] [review]
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?
Attachment #8341744 - Flags: review?(bhackett1024)
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?
Attachment #8341744 - Flags: review?(bhackett1024)
(Assignee)

Comment 2

4 years ago
Created attachment 8341773 [details] [diff] [review]
js-ti-oom.patch

Like this? Also fixes the assert for me.
Assignee: general → choller
Attachment #8341744 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #8341773 - Flags: review?(bhackett1024)
Attachment #8341773 - Flags: review?(bhackett1024) → review+
(Assignee)

Comment 3

4 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/8a7ed8362caa
https://hg.mozilla.org/mozilla-central/rev/8a7ed8362caa
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla28
You need to log in before you can comment on or make changes to this bug.