Differential Testing: Different output message involving baselineCompile

RESOLVED FIXED in Firefox 63

Status

()

enhancement
P2
major
RESOLVED FIXED
Last year
Last year

People

(Reporter: gkw, Assigned: nbp)

Tracking

(Blocks 3 bugs, {testcase})

Trunk
mozilla63
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox61 wontfix, firefox62 wontfix, firefox63 fixed)

Details

Attachments

(1 attachment)

// 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
Flags: needinfo?(jorendorff)
(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.
Flags: needinfo?(jorendorff)
Priority: -- → P2
(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?
Flags: needinfo?(nicolas.b.pierron)
Assignee: nobody → nicolas.b.pierron
Status: NEW → ASSIGNED
Flags: needinfo?(nicolas.b.pierron)
Update baselineCompile function. Add error cases in the help text and check that
a non-string result implies a baseline-compiled script.
Attachment #8994816 - Flags: review?(jorendorff)
Can we use #ifdef JS_MORE_DETERMINISTIC? It should be fine to fuzz baselineCompile() when we're not doing differential testing.
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 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.
Attachment #8994816 - Flags: review?(jorendorff) → review+
Pushed by npierron@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/a8d3519a5995
In deterministic builds, disable baselineCompile when --fuzzing-safe is provided. r=jorendorff
https://hg.mozilla.org/mozilla-central/rev/a8d3519a5995
Status: ASSIGNED → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
You need to log in before you can comment on or make changes to this bug.