Closed Bug 732863 Opened 13 years ago Closed 13 years ago

IonMonkey: Crash [@ JSString::isAtom]

Categories

(Core :: JavaScript Engine, defect)

Other Branch
x86_64
Linux
defect
Not set
major

Tracking

()

RESOLVED FIXED

People

(Reporter: decoder, Assigned: nbp)

References

Details

(Keywords: crash, testcase)

Crash Data

Attachments

(2 files)

The following testcase crashes on ionmonkey revision 1fd6c40d3852 (run with --ion -n -m --ion-eager): function TestCase(n, d, e, a) {} function reportCompare (expected, actual, description) { var testcase = new TestCase("unknown-test-name", description, expected, actual); try { } catch(ex) { } } var UBound = 0; var statusitems = [ ]; var actual = ''; var actualvalues = [ ]; var expectedvalues = [ ]; addThis(); actual %= match(1, 2, Math.exp, Math.log); addThis(); function match(x, y, F, G) {} function addThis() { actualvalues[UBound] += actual; UBound++; for (var i = 0; i < UBound; i++) { reportCompare(expectedvalues[i], actualvalues[i], statusitems[i]); } }
Crash trace: Program received signal SIGSEGV, Segmentation fault. 0x0000000000444536 in JSString::isAtom (this=0x0) at /home/ownhero/homes/mozilla/repos/ionmonkey/js/src/vm/String.h:385 385 bool atomized = (d.lengthAndFlags & ATOM_MASK) == ATOM_FLAGS; (gdb) bt #0 0x0000000000444536 in JSString::isAtom (this=0x0) at /home/ownhero/homes/mozilla/repos/ionmonkey/js/src/vm/String.h:385 #1 0x000000000044674e in js::CompartmentChecker::check (this=0x7fffffffb140, str=0x0) at ../jscntxtinlines.h:184 #2 0x00000000004467e4 in js::CompartmentChecker::check (this=0x7fffffffb140, v=...) at ../jscntxtinlines.h:192 #3 0x0000000000449be3 in js::assertSameCompartment<JS::Value> (cx=0xcd5d00, t1=...) at ../jscntxtinlines.h:254 #4 0x000000000050f374 in js::Interpret (cx=0xcd5d00, entryFrame=0x7ffff0beb1d0, interpMode=js::JSINTERP_NORMAL) at /home/ownhero/homes/mozilla/repos/ionmonkey/js/src/jsinterp.cpp:3090 #5 0x0000000000500170 in js::RunScript (cx=0xcd5d00, script=0x7ffff09072e0, fp=0x7ffff0beb1d0) at /home/ownhero/homes/mozilla/repos/ionmonkey/js/src/jsinterp.cpp:463 #6 0x00000000005004b7 in js::InvokeKernel (cx=0xcd5d00, args=..., construct=js::NO_CONSTRUCT) at /home/ownhero/homes/mozilla/repos/ionmonkey/js/src/jsinterp.cpp:526 #7 0x0000000000460c89 in js::Invoke (cx=0xcd5d00, args=..., construct=js::NO_CONSTRUCT) at /home/ownhero/homes/mozilla/repos/ionmonkey/js/src/jsinterp.h:157 #8 0x00000000005006b0 in js::Invoke (cx=0xcd5d00, thisv=..., fval=..., argc=3, argv=0x7fffffffc260, rval=0x7fffffffc228) at /home/ownhero/homes/mozilla/repos/ionmonkey/js/src/jsinterp.cpp:558 #9 0x0000000000868224 in js::ion::InvokeFunction (cx=0xcd5d00, fun=0x7ffff090f780, argc=3, argv=0x7fffffffc258, rval=0x7fffffffc228) at /home/ownhero/homes/mozilla/repos/ionmonkey/js/src/ion/VMFunctions.cpp:65
I am able to reproduce it with the following (--ion -n --ion-eager): // Compiled function TestCase(a) { } // Not compiled (try) function reportCompare (actual) { TestCase(actual); try { } catch(ex) { } } // Compiled function addThis(bound) { actualvalues[bound] = undefined + actual; reportCompare(actualvalues[bound]); } var actual = ''; var actualvalues = []; addThis(0); actual = NaN; addThis(1); Reducing the test case more cause the SEGV to disappear. The first call of addThis cause the specialization of the assembly, which trigger another compilation at the second call of addThis which then cause the SEGV when checking the arguments of TestCase function.
Assignee: general → nicolas.b.pierron
Status: NEW → ASSIGNED
function addThis(bound) { actualvalues[bound] = undefined + actual; reportCompare(actualvalues[bound]); } loc line op ----- ---- -- main: 00000: 12 getgname "actualvalues" 00005: 12 getarg 0 00008: 12 getgname "undefined" 00013: 12 getgname "actual" 00018: 12 add 00019: 12 setelem (1) 00020: 12 pop 00021: 13 callgname "reportCompare" 00026: 13 undefined 00027: 13 notearg 00028: 13 getgname "actualvalues" 00033: 13 getarg 0 00036: 13 getelem (2) 00037: 13 notearg 00038: 13 call 1 00041: 13 pop 00042: 13 stop js::analyze::ScriptAnalysis::typeBarriers (this=0x7ffff7f5c850, cx=0xce5d00, offset=0) Value returned = (js::types::TypeBarrier *) 0x0 js::analyze::ScriptAnalysis::typeBarriers (this=0x7ffff7f5c850, cx=0xce5d00, offset=13) Value returned = (js::types::TypeBarrier *) 0x7ffff7f5d3e0 js::analyze::ScriptAnalysis::typeBarriers (this=0x7ffff7f5c850, cx=0xce5d00, offset=21) Value returned = (js::types::TypeBarrier *) 0x0 js::analyze::ScriptAnalysis::typeBarriers (this=0x7ffff7f5c850, cx=0xce5d00, offset=28) Value returned = (js::types::TypeBarrier *) 0x0 js::analyze::ScriptAnalysis::typeBarriers (this=0x7ffff7f5c850, cx=0xce5d00, offset=36) Value returned = (js::types::TypeBarrier *) 0x0 # wrong ! The code store a value at location (1) to «actualvalues» but the type inference does not infer that the getelem (2) output of «actualvalues» can be a value, but only returns the previous observed type, which has been observed at the first call of addThis (i-e a String).
Can you paste the compartment's type printout? (cx->compartment->types.print)
Thanks, this output is extremely helpful for debugging. So from this output, I deduce that the type barrier which is produced on «actual» should cause a bailout.
(In reply to Nicolas B. Pierron [:pierron] from comment #5) > Created attachment 604694 [details] > Type status during the second compilation of addThis. > > Thanks, this output is extremely helpful for debugging. So from this > output, I deduce that the type barrier which is produced on «actual» should > cause a bailout. Yeah, that barrier on actual should trip the second time the function is executed and invalidate the jitcode.
Attached patch Do not ignore type barrier. (deleted) — Splinter Review
Make sure type barrier are always inserted even for non-effectful instruction, because type barrier are the assumptions of the type-inference.
Attachment #604729 - Flags: review?(dvander)
Comment on attachment 604729 [details] [diff] [review] Do not ignore type barrier. Review of attachment 604729 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/src/ion/IonBuilder.cpp @@ -3137,5 @@ > return pushConstant(NullValue()); > break; > default: > - MUnbox::Mode mode = ins->isEffectful() ? MUnbox::TypeBarrier : MUnbox::Fallible; > - barrier = MUnbox::New(ins, MIRTypeFromValueType(type), mode); We can't change this though. A type barrier failure will look at regs.pc for reflowing type information, but regs.pc is undefined for idempotent instructions. As-is, this code should work, because the fallible version will trigger a bailout and re-run the whole instruction. ::: js/src/ion/MIR.h @@ +1323,5 @@ > { > JS_ASSERT(ins->type() == MIRType_Value); > setResultType(type); > setMovable(); > + if (mode_ == TypeBarrier) Nice catch - yeah, we definitely always need the guard here.
Attachment #604729 - Flags: review?(dvander) → review+
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Version: Other Branch → 14 Branch
Version: 14 Branch → Other Branch
A testcase for this bug was automatically identified at js/src/jit-test/tests/ion/bug732863.js.
Flags: in-testsuite+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: