1456392 - Differential Testing: Different output message involving baselineCompile

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, enhancement, P2)

Description

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

// Adapted from randomly chosen test: js/src/jit-test/tests/self-test/baselineCompile-Bug1444894.js
print(eval("newGlobal().baselineCompile();"));

$ ./js-dbg-64-dm-linux-28db2c96ac69 --fuzzing-safe --no-threads --baseline-eager --no-ion testcase.js 
undefined

$ ./js-dbg-64-dm-linux-28db2c96ac69 --fuzzing-safe --no-threads --no-baseline --no-ion testcase.js 
baseline disabled

Tested this on m-c rev 28db2c96ac69.

My configure flags are:

AR=ar sh ./configure --enable-debug --enable-more-deterministic --with-ccache --enable-gczeal --enable-debug-symbols --disable-tests

python -u -m funfuzz.js.compile_shell -b "--enable-debug --enable-more-deterministic" -r 28db2c96ac69

autobisectjs shows this is probably related to the following changeset:

The first bad revision is:
changeset:   https://hg.mozilla.org/mozilla-central/rev/51d604ea785b
user:        Jason Orendorff
date:        Thu Feb 22 13:04:46 2018 -0600
summary:     Bug 1440431 - Part 3: Add baselineCompile() testing function. r=nbp.

Jason, bug 1440431 does seem to have this direct consequence, but for deterministic builds should we:

(1) Ignore baselineCompile() for differential testing
(2) Make baselineCompile() output the same thing for the different runtime flags above

Comment 1

[Nicolas B. Pierron [:nbp] {pto until 30/05}]

(In reply to Gary Kwong [:gkw] [:nth10sd] from comment #0)
> (1) Ignore baselineCompile() for differential testing
> (2) Make baselineCompile() output the same thing for the different runtime
> flags above

Eager baseline compilation can already be fuzzily triggered with the compile option.
I guess that ignoring it for differential testing is a safe option which will not reduce the fuzzing space.

Comment 2

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

(In reply to Nicolas B. Pierron [:nbp] {backlog: 39} from comment #1)
> I guess that ignoring it for differential testing is a safe option which
> will not reduce the fuzzing space.

Can we make it have the same output for all --enable-more-deterministic builds?

Comment 3

[Nicolas B. Pierron [:nbp] {pto until 30/05}]

Attachment: Disable baselineCompile when --fuzzing-safe is given.

Update baselineCompile function. Add error cases in the help text and check that
a non-string result implies a baseline-compiled script.

Comment 4

[Jan de Mooij [:jandem]]

Can we use #ifdef JS_MORE_DETERMINISTIC? It should be fine to fuzz baselineCompile() when we're not doing differential testing.

Comment 5

[Nicolas B. Pierron [:nbp] {pto until 30/05}]

That sounds good to me, we can wrap the added condition with #ifdef JS_MORE_DETERMINISTIC.
The reason I removed it was because I was not sure what was non-deterministic here, except for the JS shell command line?

Comment 6

[Jason Orendorff [:jorendorff]]

Comment on attachment 8994816 [details] [diff] [review]
Disable baselineCompile when --fuzzing-safe is given.

Review of attachment 8994816 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks.

::: js/src/builtin/TestingFunctions.cpp
@@ +5255,5 @@
>      const char* returnedStr = nullptr;
>      do {
> +        // In order to check for differential behaviour, baselineCompile should have
> +        // the same output whether --no-baseline is used or not.
> +        if (fuzzingSafe) {

OK. Please do make the change you described in comment 5 in response to Jan's comment.

@@ +5980,5 @@
>  "  that extra boilerplate is needed afterwards to cause the VM to start\n"
>  "  running the jitcode rather than staying in the interpreter:\n"
>  "    baselineCompile();  for (var i=0; i<1; i++) {} ...\n"
> +"  The interpreter will enter the new jitcode at the loop header unless\n"
> +"  baselineCompile returned a string or thrown an error.\n"),

Grammar nit: "or threw an error"

::: js/src/jit-test/tests/self-test/baselineCompile.js
@@ +1,1 @@
> +// |jit-test| test-also=--fuzzing-safe

Please bring js/src/jit-test/README up to date. Several of these directives are not documented:

https://searchfox.org/mozilla-central/rev/5dbfd833bbb114afe758db4d4bdbc5b13bcc33ef/js/src/tests/lib/jittests.py#272-291

Just a couple words each is enough, and this can be a follow-up bug.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/a8d3519a5995
In deterministic builds, disable baselineCompile when --fuzzing-safe is provided. r=jorendorff

Comment 8

[Dorel Luca [:dluca]]

https://hg.mozilla.org/mozilla-central/rev/a8d3519a5995