Closed
Bug 732863
Opened 13 years ago
Closed 13 years ago
IonMonkey: Crash [@ JSString::isAtom]
Categories
(Core :: JavaScript Engine, defect)
Tracking
()
RESOLVED
FIXED
People
(Reporter: decoder, Assigned: nbp)
References
Details
(Keywords: crash, testcase)
Crash Data
Attachments
(2 files)
(deleted),
text/plain
|
Details | |
(deleted),
patch
|
dvander
:
review+
|
Details | Diff | Splinter Review |
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]);
}
}
Reporter | ||
Comment 1•13 years ago
|
||
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
Assignee | ||
Comment 2•13 years ago
|
||
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
Assignee | ||
Comment 3•13 years ago
|
||
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).
Comment 4•13 years ago
|
||
Can you paste the compartment's type printout? (cx->compartment->types.print)
Assignee | ||
Comment 5•13 years ago
|
||
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.
Comment 6•13 years ago
|
||
(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.
Assignee | ||
Comment 7•13 years ago
|
||
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+
Assignee | ||
Comment 9•13 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Version: Other Branch → 14 Branch
Assignee | ||
Updated•13 years ago
|
Version: 14 Branch → Other Branch
Reporter | ||
Comment 10•12 years ago
|
||
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.
Description
•