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

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect)

Description

[Christian Holler (:decoder)]

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 :)

Comment 1

[Luke Wagner [:luke]]

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.

Comment 2

[Luke Wagner [:luke]]

Attachment: fix and test

Comment 3

[Luke Wagner [:luke]]

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

Erm, nevermind, that's totally wrong.

Comment 4

[Luke Wagner [:luke]]

Attachment: 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.

Comment 5

[Luke Wagner [:luke]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/a2128894e47f

Comment 6

[Luke Wagner [:luke]]

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.

Comment 7

[Alex Keybl [:akeybl]]

(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!

Comment 8

[Luke Wagner [:luke]]

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

Comment 9

[Alex Keybl [:akeybl]]

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.

Comment 10

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

https://hg.mozilla.org/mozilla-central/rev/a2128894e47f

Comment 11

[Luke Wagner [:luke]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/057de3c5ea22

Comment 12

[Al Billings [:abillings - ex-MoCo]]

Was Firefox 12 unaffected by this?

Comment 13

[Christian Holler (:decoder)]

A testcase for this bug was automatically identified at js/src/jit-test/tests/basic/testBug736012.js.