Closed
Bug 1456392
Opened 4 years ago
Closed 4 years ago
Differential Testing: Different output message involving baselineCompile
Categories
(Core :: JavaScript Engine: JIT, enhancement, P2)
Core
JavaScript Engine: JIT
Tracking
()
RESOLVED
FIXED
mozilla63
People
(Reporter: gkw, Assigned: nbp)
References
Details
(Keywords: testcase)
Attachments
(1 file)
3.66 KB,
patch
|
jorendorff
:
review+
|
Details | Diff | Splinter Review |
// 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)
Assignee | ||
Comment 1•4 years ago
|
||
(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)
Assignee | ||
Updated•4 years ago
|
Priority: -- → P2
![]() |
Reporter | |
Comment 2•4 years ago
|
||
(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 | ||
Updated•4 years ago
|
Assignee: nobody → nicolas.b.pierron
Status: NEW → ASSIGNED
Flags: needinfo?(nicolas.b.pierron)
Assignee | ||
Comment 3•4 years ago
|
||
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)
Comment 4•4 years ago
|
||
Can we use #ifdef JS_MORE_DETERMINISTIC? It should be fine to fuzz baselineCompile() when we're not doing differential testing.
Assignee | ||
Comment 5•4 years ago
|
||
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•4 years ago
|
||
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
Comment 8•4 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/a8d3519a5995
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
status-firefox63:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
Updated•4 years ago
|
You need to log in
before you can comment on or make changes to this bug.
Description
•