Closed Bug 1264561 Opened 8 years ago Closed 8 years ago

Differential Testing: Different output message involving RegExp

Categories

(Core :: JavaScript Engine: JIT, defect)

x86_64
All
defect
Not set
major

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)
>     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.
Just added loadPtr.
Assignee: nobody → arai.unmht
Attachment #8741303 - Flags: review?(efaustbmo)
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 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+
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
https://hg.mozilla.org/mozilla-central/rev/1a6be8c5e45b
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
Thank you for the fix.
Group: javascript-core-security → core-security-release
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.