Closed Bug 907085 Opened 6 years ago Closed 6 years ago

Crash [@ JSContext::hasOption] or [@ js::ion::IsBaselineEnabled]

Categories

(Core :: JavaScript Engine, defect, critical)

x86_64
macOS
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla26

People

(Reporter: gkw, Assigned: luke)

References

(Blocks 1 open bug)

Details

(Keywords: crash, regression, testcase, Whiteboard: [jsbugmon:update])

Crash Data

Attachments

(2 files)

Attached file stack
try {
    s.e
} catch (e) {}
o = o = s2 = /x/
for (let e in []);
x = s2
schedulegc(21)
eval("x.e=x.t")
try {
    (function() {
        this.eval("\
            (function(stdlib,fgn,heap) {\
                \"use asm\";\
                var Vie = new stdlib.Float64Array(heap);\
                var Iew = new stdlib.Int8Array(heap);\
                function f(){\
                    ent\
                }\
            })()\
        ")
    })()
} catch (e) {}

crashes js debug (32-bit threadsafe deterministic) shell on m-c changeset b7f636fada9f with --baseline-eager at JSContext::hasOption with js::ion::IsBaselineEnabled on the stack.

autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   http://hg.mozilla.org/mozilla-central/rev/a63f47fcbe98
user:        Luke Wagner
date:        Thu Aug 08 22:51:29 2013 -0500
summary:     Bug 902506 - OdinMonkey: remove main-thread restriction from js::CompileAsmJS (r=bhackett)

Is bug 902506 a likely regressor?
Flags: needinfo?(luke)
Ah yeah, I made GetIonContext()->cx be NULL during asm.js compilation.
Assignee: general → luke
Flags: needinfo?(luke)
Attached patch fix and testSplinter Review
With bug 905827, parsing, and therefore Odin compilation, will be able to happen off the main thread.  That means js::CompileAsmJS takes an ExclusiveContext and the IonContext pushed during compilation has a NULL cx (as well as JSRuntime/IonCompartment/IonRuntime).  GetIonContext()->cx is being used in GC for the purpose of rooting, but instead we can just use the JSRuntime we get from being on the main thread.
Attachment #792842 - Flags: review?(jdemooij)
Comment on attachment 792842 [details] [diff] [review]
fix and test

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

::: js/src/jit/BaselineJIT.cpp
@@ +925,1 @@
>      for (; !iter.done(); ++iter) {

Nice, we can now write this loop as

for (JitActivationIterator iter(rt); !iter.done(); ++iter) {
    ...
}

and remove the iter.done() call + the comment before the loop.
Attachment #792842 - Flags: review?(jdemooij) → review+
(In reply to Jan de Mooij [:jandem] from comment #3)
Beautiful!

https://hg.mozilla.org/integration/mozilla-inbound/rev/27cf1ae86abf
https://hg.mozilla.org/mozilla-central/rev/27cf1ae86abf
Status: NEW → RESOLVED
Closed: 6 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla26
You need to log in before you can comment on or make changes to this bug.