1264561 - Differential Testing: Different output message involving RegExp

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, defect)

Description

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

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?

Comment 1

[Tooru Fujisawa [:arai]]

>     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.

Comment 2

[Tooru Fujisawa [:arai]]

Attachment: Fix ClassOps::call and ClassOps::construct address calculation in visitIsCallable and IsConstructor.

Just added loadPtr.

Comment 3

[Nicholas Nethercote [inactive]]

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 :)

Comment 4

[Eric Faust [:efaust]]

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.

Comment 5

[Tooru Fujisawa [:arai]]

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

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/1a6be8c5e45b

Comment 7

[Nicholas Nethercote [inactive]]

Thank you for the fix.