Closed
Bug 1132584
Opened 10 years ago
Closed 10 years ago
Assertion failure: Modified registers between VM call and OsiPoint, at jit/MacroAssembler.cpp
Categories
(Core :: JavaScript Engine: JIT, defect)
Tracking
()
RESOLVED
FIXED
mozilla38
Tracking | Status | |
---|---|---|
firefox38 | --- | fixed |
People
(Reporter: gkw, Assigned: jandem)
References
Details
(Keywords: assertion, regression, testcase, Whiteboard: [jsbugmon:update])
Attachments
(2 files)
(deleted),
text/plain
|
Details | |
(deleted),
patch
|
nbp
:
review+
|
Details | Diff | Splinter Review |
enableOsiPointRegisterChecks()
function f() {}
f.__defineGetter__("x", (function() {
this._
}))
for (var i = 0; i < 3; i++) {
(function() {
for (var j = 0; j < 1; j++) {
f.x + 1
}
})()
}
asserts js debug shell on m-c changeset 81f979b17fbd with --fuzzing-safe --no-threads --ion-eager at Assertion failure: Modified registers between VM call and OsiPoint, at jit/MacroAssembler.cpp.
Debug configure options:
CC="clang -Qunused-arguments" CXX="clang++ -Qunused-arguments" AR=ar AUTOCONF=/usr/local/Cellar/autoconf213/2.13/bin/autoconf213 sh /Users/skywalker/trees/mozilla-central/js/src/configure --target=x86_64-apple-darwin12.5.0 --enable-debug --enable-optimize --enable-nspr-build --enable-more-deterministic --with-ccache --enable-gczeal --enable-debug-symbols --disable-tests
python -u ~/fuzzing/js/compileShell.py -b "--enable-debug --enable-optimize --enable-more-deterministic --enable-nspr-build -R ~/trees/mozilla-central" -r 81f979b17fbd
autoBisect shows this is probably related to the following changeset:
The first bad revision is:
changeset: https://hg.mozilla.org/mozilla-central/rev/d96d552ff899
user: Jan de Mooij
date: Wed Feb 11 14:42:01 2015 +0100
summary: Bug 1129382 - Add Ion ICs for scripted getters/setters. r=efaust,nbp,djvj
Jan, is bug 1129382 a likely regressor?
Flags: needinfo?(jdemooij)
Reporter | ||
Comment 2•10 years ago
|
||
(lldb) bt
* thread #1: tid = 0x47816, 0x000000010403626a, queue = 'com.apple.main-thread', stop reason = EXC_BREAKPOINT (code=EXC_I386_BPT, subcode=0x0)
* frame #0: 0x000000010403626a
(lldb) dis --start $pc-100 --end $pc
0x104036206: andb $0x68, %al
0x104036208: movsd 0x60(%rsp), %xmm12
0x10403620f: movsd 0x58(%rsp), %xmm11
0x104036216: movsd 0x50(%rsp), %xmm10
0x10403621d: movsd 0x48(%rsp), %xmm9
0x104036224: movsd 0x40(%rsp), %xmm8
0x10403622b: movsd 0x38(%rsp), %xmm7
0x104036231: movsd 0x30(%rsp), %xmm6
0x104036237: movsd 0x28(%rsp), %xmm5
0x10403623d: movsd 0x20(%rsp), %xmm4
0x104036243: movsd 0x18(%rsp), %xmm3
0x104036249: movsd 0x10(%rsp), %xmm2
0x10403624f: movsd 0x8(%rsp), %xmm1
0x104036255: movsd (%rsp), %xmm0
0x10403625a: addq $0x78, %rsp
0x10403625e: popq %rax
0x10403625f: popq %rcx
0x104036260: popq %rdx
0x104036261: popq %rsi
0x104036262: popq %rdi
0x104036263: popq %r8
0x104036265: popq %r9
0x104036267: popq %r10
0x104036269: int3
(lldb)
Assignee | ||
Comment 3•10 years ago
|
||
Not security sensitive; it's a problem with the OsiPoint register checks.
Group: core-security
Flags: needinfo?(jdemooij)
Assignee | ||
Comment 4•10 years ago
|
||
The problem was:
(1) We made a call from an Ion IC getter stub.
(2) The getter script then did a callVM. We stored the live registers and did the OsiPoint register checks but left the JitActivation::checkRegs flags set to 1.
(3) Then we returned to the getter IC stub, and after the IC we verified the live registers, but of course this doesn't match the registers the callee dumped.
This wasn't a problem before because LCallGeneric/LCallKnown have no live registers. The ICs can make scripted calls from instructions with live registers (but these are saved of course).
I fixed it by making sure we set checkRegs = 0 when we verify the registers.
Assignee: nobody → jdemooij
Status: NEW → ASSIGNED
Attachment #8564116 -
Flags: review?(nicolas.b.pierron)
Comment 5•10 years ago
|
||
Comment on attachment 8564116 [details] [diff] [review]
Patch
Review of attachment 8564116 [details] [diff] [review]:
-----------------------------------------------------------------
::: js/src/jit/CodeGenerator.cpp
@@ +1778,5 @@
> + // Do this first, to ensure we always reset the checkRegs flag on the
> + // JitActivation.
> + if (shouldVerifyOsiPointRegs(safepoint))
> + verifyOsiPointRegs(safepoint);
> +#endif
To be honest, I would prefer if we could keep this code below the `markOsiPoint` function call.
Instead, add the following line in the bailout / exception path instead:
activation->setCheckRegs(false);
Attachment #8564116 -
Flags: review?(nicolas.b.pierron) → review+
Assignee | ||
Comment 6•10 years ago
|
||
Comment 7•10 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla38
You need to log in
before you can comment on or make changes to this bug.
Description
•