Closed
Bug 1264561
Opened 8 years ago
Closed 8 years ago
Differential Testing: Different output message involving RegExp
Categories
(Core :: JavaScript Engine: JIT, defect)
Tracking
()
RESOLVED
FIXED
mozilla48
Tracking | Status | |
---|---|---|
firefox47 | --- | unaffected |
firefox48 | --- | fixed |
People
(Reporter: gkw, Assigned: arai)
References
Details
(Keywords: testcase, Whiteboard: [fuzzblocker])
Attachments
(1 file)
r = RegExp("") "".replace(r, Function("x")) try { for (var x = 0; x < 5; x++) { print("".replace(r, this)) } } catch (e) {} $ ./js-dbg-64-dm-clang-darwin-8630367f5e3f --fuzzing-safe --no-threads --baseline-eager testcase.js [object global] [object global] [object global] [object global] [object global] $ ./js-dbg-64-dm-clang-darwin-8630367f5e3f --fuzzing-safe --no-threads --ion-eager testcase.js [object global] [object global] [object global] [object global] Tested this on m-c rev 8630367f5e3f. My configure flags are: 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-darwin14.5.0 --disable-jemalloc --enable-debug --enable-more-deterministic --with-ccache --enable-gczeal --enable-debug-symbols --disable-tests python -u ~/funfuzz/js/compileShell.py -b "--enable-debug --enable-more-deterministic" -r 8630367f5e3f autoBisect shows this is probably related to the following changeset: The first bad revision is: changeset: https://hg.mozilla.org/mozilla-central/rev/aa88b0d0cd4a user: Nicholas Nethercote date: Fri Apr 01 11:00:01 2016 +1100 summary: Bug 1261723 (part 2) - Separate class ops from js::Class. code=njn,h4writer. r=efaust,bz. Locking in case this might also be related to recent s-s RegExp bugs even though autoBisect points to bug 1261723. Also setting [fuzzblocker] as this seems to be happening often. njn, is bug 1261723 a likely regressor?
Flags: needinfo?(n.nethercote)
Assignee | ||
Comment 1•8 years ago
|
||
> masm.branchPtr(Assembler::NonZero, Address(output, offsetof(js::Class, cOps)),
> ImmPtr(nullptr), &hasCOps);
> masm.move32(Imm32(0), output);
> masm.jump(&done);
>
> masm.bind(&hasCOps);
> masm.cmpPtrSet(Assembler::NonZero, Address(output, offsetof(js::ClassOps, call)),
> ImmPtr(nullptr), output);
at the point of cmpPtrSet, the value in output is the object's class, not cOps.
it should load |Address(output, offsetof(js::Class, cOps))| to output before cmpPtrSet.
same for CodeGenerator::visitIsConstructor.
Assignee | ||
Comment 2•8 years ago
|
||
Just added loadPtr.
Assignee: nobody → arai.unmht
Attachment #8741303 -
Flags: review?(efaustbmo)
Comment 3•8 years ago
|
||
arai's patch looks plausible to me, though I'm no expert on JIT code generation -- that's why I asked h4writer to do that part for me :)
Flags: needinfo?(n.nethercote)
Comment 4•8 years ago
|
||
Comment on attachment 8741303 [details] [diff] [review] Fix ClassOps::call and ClassOps::construct address calculation in visitIsCallable and IsConstructor. Review of attachment 8741303 [details] [diff] [review]: ----------------------------------------------------------------- Yeah, I should have caught this the first time. Thanks. ::: js/src/jit/CodeGenerator.cpp @@ +10727,1 @@ > masm.cmpPtrSet(Assembler::NonZero, Address(output, offsetof(js::ClassOps, call)), OK, this is lame, but we prefer static js::Class::offsetOfClassOps() and static js::ClassOps::offsetOfCall(). This is pre-existing, but we should fix it anyway.
Attachment #8741303 -
Flags: review?(efaustbmo) → review+
Assignee | ||
Comment 5•8 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/1a6be8c5e45b8405c652111643d7bc808e68c849 Bug 1264561 - Fix ClassOps::call and ClassOps::construct address calculation in visitIsCallable and visitIsConstructor. r=efaust
Comment 6•8 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/1a6be8c5e45b
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
Comment 7•8 years ago
|
||
Thank you for the fix.
Updated•8 years ago
|
Group: javascript-core-security → core-security-release
Updated•8 years ago
|
status-firefox47:
--- → unaffected
Updated•7 years ago
|
Group: core-security-release
You need to log in
before you can comment on or make changes to this bug.
Description
•