Assertion failure: enumerators == cx->enumerators, at jsinterp.cpp:453 or Crash [@ js_SuppressDeletedProperty]

VERIFIED FIXED in Firefox 13

Status

()

Core
JavaScript Engine
--
critical
VERIFIED FIXED
5 years ago
5 years ago

People

(Reporter: decoder, Assigned: luke)

Tracking

(Blocks: 1 bug, {assertion, crash, testcase})

Trunk
mozilla14
x86_64
Linux
assertion, crash, testcase
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox13+ fixed, firefox14+ fixed, firefox-esr10 unaffected)

Details

(Whiteboard: [sg:critical][advisory-tracking+], crash signature)

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

5 years ago
The following test asserts/crashes on mozilla-central revision dfcb11712ec2 (options -m -n):


var lfcode = new Array();
lfcode.push("\
function fatty() {\
    try { fatty(); } catch (e) {\
        for each (foo in [1]) {}\
    }\
}\
fatty();\
");
lfcode.push("gc()");
lfcode.push("");
while (true) {
        var file = lfcode.shift(); if (file == undefined) { break; }
        evaluate(file);
}


Crash trace:

==63414== Invalid read of size 1
==63414==    at 0x4848F0: js_SuppressDeletedProperty(JSContext*, JSObject*, long) (jsiter.cpp:1002)
==63414==    by 0x424B31: js::array_shift(JSContext*, unsigned int, JS::Value*) (jsarray.cpp:2559)
==63414==    by 0x483018: js::InvokeKernel(JSContext*, js::CallArgs, js::MaybeConstruct) (jscntxtinlines.h:314)
==63414==    by 0x47EB8D: js::Interpret(JSContext*, js::StackFrame*, js::InterpMode) (jsinterp.cpp:2710)
==63414==    by 0x483AFD: js::Execute(JSContext*, JSScript*, JSObject&, JS::Value*) (jsinterp.cpp:668)
==63414==    by 0x4187F6: JS_ExecuteScript (jsapi.cpp:5275)
==63414==    by 0x408294: Process(JSContext*, JSObject*, char const*, bool) (js.cpp:478)
==63414==    by 0x40AF03: Shell(JSContext*, js::cli::OptionParser*, char**) (js.cpp:4751)
==63414==    by 0x40B5C5: main (js.cpp:5044)
==63414==  Address 0x30 is not stack'd, malloc'd or (recently) free'd


Possibly related to bug 734987?

I'm marking this S-s because the bug requires "gc()" although the crash looks like a null-pointer crash. Please triage and remove S-s if appropriate :)
(Assignee)

Comment 1

5 years ago
This is another pre-existing issue flushed out by bug 732744.  In this case, an error while jaeger-shotting at a loop header pops the frame without jumping to 'error' first.

IIUC, this is on aurora.  I'd like to let the fix bake a bit on m-c, though.
Whiteboard: js-triage-needed → js-triage-done
(Assignee)

Comment 2

5 years ago
Created attachment 606310 [details] [diff] [review]
fix and test
Assignee: general → luke
Status: NEW → ASSIGNED
Attachment #606310 - Flags: review?(bhackett1024)
(Assignee)

Comment 3

5 years ago
Comment on attachment 606310 [details] [diff] [review]
fix and test

Erm, nevermind, that's totally wrong.
Attachment #606310 - Attachment is obsolete: true
Attachment #606310 - Flags: review?(bhackett1024)
(Assignee)

Comment 4

5 years ago
Created attachment 606332 [details] [diff] [review]
fix

So, actually reading the comment this time, Jaeger_Throwing indicates that the current frame has finished.  Thus it is wrong to 'goto error' in general for Jaeger_Throwing.  The general case of exception-thrown-in-jit-code is handled corectly by FindExceptionHandler.  It is only the special case of "failure to reserve stack space" in CheckStackAndEnterMethodJit that leaves us in this weird state.  Unfortunately, I don't see a better fix than introducing yet another enum return.  Let me know if you have any bright ideas Brian.
Attachment #606332 - Flags: review?(bhackett1024)
Attachment #606332 - Flags: review?(bhackett1024) → review+
(Assignee)

Comment 5

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/a2128894e47f
status-firefox13: --- → affected
tracking-firefox13: --- → ?
Target Milestone: --- → mozilla14
(Assignee)

Comment 6

5 years ago
Comment on attachment 606332 [details] [diff] [review]
fix

[Approval Request Comment]
Regression caused by (bug #): bug 732744
User impact if declined: crash
Testing completed (on m-c, etc.): m-c
Risk to taking this patch (and alternatives if risky): small patch, has some risk, but we still have a lot of time for this to bake.
Attachment #606332 - Flags: approval-mozilla-aurora?

Comment 7

5 years ago
(In reply to Luke Wagner [:luke] from comment #6)
> Risk to taking this patch (and alternatives if risky): small patch, has some
> risk, but we still have a lot of time for this to bake.

1) Do we expect this crash to be more pronounced with our GA? Right now there's only a handful of crashes on 13
2) Do you expect that any possible regressions will be apparent over the 11 weeks before release?

Thanks!
(Assignee)

Comment 8

5 years ago
(In reply to Alex Keybl [:akeybl] from comment #7)
1) no
2) yes.  11 weeks of fuzzing should be plenty :)

Comment 9

5 years ago
Comment on attachment 606332 [details] [diff] [review]
fix

[Triage Comment]
Approved for Aurora 13 since we believe we'll find any regressions prior to release.
Attachment #606332 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
https://hg.mozilla.org/mozilla-central/rev/a2128894e47f
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
tracking-firefox13: ? → +
status-firefox14: --- → fixed
tracking-firefox14: --- → +
Whiteboard: js-triage-done → [sg:critical]
(Assignee)

Comment 11

5 years ago
https://hg.mozilla.org/releases/mozilla-aurora/rev/057de3c5ea22
status-firefox13: affected → fixed
(Reporter)

Updated

5 years ago
Status: RESOLVED → VERIFIED

Updated

5 years ago
status-firefox-esr10: --- → unaffected
Was Firefox 12 unaffected by this?
Whiteboard: [sg:critical] → [sg:critical][advisory-tracking+]
Group: core-security
(Reporter)

Comment 13

5 years ago
A testcase for this bug was automatically identified at js/src/jit-test/tests/basic/testBug736012.js.
Flags: in-testsuite+
You need to log in before you can comment on or make changes to this bug.