Closed
Bug 1455593
Opened 6 years ago
Closed 6 years ago
Changes to BinAST fuzzer for Multipart Reader
Categories
(Core :: JavaScript Engine, enhancement, P2)
Tracking
()
RESOLVED
FIXED
mozilla64
People
(Reporter: decoder, Assigned: decoder)
Details
(Keywords: sec-want, Whiteboard: [adv-main64-])
Attachments
(1 file, 1 obsolete file)
Now that the BinAST multipart reader code landed, we need to make minor changes to the fuzzing code so the new files are instrumented and used for fuzzing instead of the previous BinTokenReaderTester class. Also, we found that disabling the constant matching in BinTokenReaderBase is a very easy change for fuzzing builds that would allow the fuzzer to spend less time trying to get these constants right.
Comment hidden (mozreview-request) |
Comment 2•6 years ago
|
||
mozreview-review |
Comment on attachment 8969626 [details] Bug 1455593 - BinAST multipart fuzzing changes. https://reviewboard.mozilla.org/r/238416/#review244172 ::: js/src/frontend/BinTokenReaderBase.h:112 (Diff revision 1) > MOZ_ASSERT(!cx_->isExceptionPending()); > > if (current_ + N - 1 > stop_) > return false; > > +#ifndef FUZZING Nit: A comment would be nice.
Attachment #8969626 -
Flags: review?(dteller) → review+
Comment hidden (mozreview-request) |
Assignee | ||
Comment 4•6 years ago
|
||
mozreview-review |
Comment on attachment 8969626 [details] Bug 1455593 - BinAST multipart fuzzing changes. https://reviewboard.mozilla.org/r/238416/#review244174
Attachment #8969626 -
Flags: review+
Pushed by choller@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/d1bcd80c9a73 BinAST multipart fuzzing changes. r=Yoric
Comment 6•6 years ago
|
||
Backed out for spidermonkey failures on testBinASTReader.cpp:235 backout: https://hg.mozilla.org/integration/autoland/rev/f39f926f02fb push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=d1bcd80c9a73a647a583958bef60816ae90d3d6b&selectedJob=174767738 failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=174767738&repo=autoland&lineNumber=21905 [task 2018-04-20T13:46:05.322Z] AddressSanitizer:DEADLYSIGNAL [task 2018-04-20T13:46:05.322Z] ================================================================= [task 2018-04-20T13:46:05.322Z] ==66444==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000 (pc 0x00000071f274 bp 0x7fffffffa950 sp 0x7fffffff96e0 T0) [task 2018-04-20T13:46:05.322Z] ==66444==The signal is caused by a WRITE memory access. [task 2018-04-20T13:46:05.322Z] ==66444==Hint: address points to the zero page. [task 2018-04-20T13:46:05.779Z] #0 0x71f273 in void runTestFromPath<js::frontend::BinTokenReaderTester>(JSContext*, char const*) /builds/worker/workspace/build/src/js/src/jsapi-tests/testBinASTReader.cpp:235:13 [task 2018-04-20T13:46:05.780Z] #1 0x718a5b in void runTestFromPath<js::frontend::BinTokenReaderTester>(JSContext*, char const*) /builds/worker/workspace/build/src/js/src/jsapi-tests/testBinASTReader.cpp:125:13 [task 2018-04-20T13:46:05.780Z] #2 0x718a5b in void runTestFromPath<js::frontend::BinTokenReaderTester>(JSContext*, char const*) /builds/worker/workspace/build/src/js/src/jsapi-tests/testBinASTReader.cpp:125:13 [task 2018-04-20T13:46:05.780Z] #3 0x718a5b in void runTestFromPath<js::frontend::BinTokenReaderTester>(JSContext*, char const*) /builds/worker/workspace/build/src/js/src/jsapi-tests/testBinASTReader.cpp:125:13 [task 2018-04-20T13:46:05.780Z] #4 0x7169d6 in cls_testBinASTReaderSimpleECMAScript2::run(JS::Handle<JSObject*>) /builds/worker/workspace/build/src/js/src/jsapi-tests/testBinASTReader.cpp:304:5 [task 2018-04-20T13:46:05.853Z] #5 0xc6fedc in main /builds/worker/workspace/build/src/js/src/jsapi-tests/tests.cpp:132:19 [task 2018-04-20T13:46:05.919Z] #6 0x7ffff63ddeac in __libc_start_main /build/eglibc-ZYONVs/eglibc-2.13/csu/libc-start.c:244 [task 2018-04-20T13:46:05.919Z] #7 0x47c294 in _start (/builds/worker/workspace/build/src/obj-spider/dist/bin/jsapi-tests+0x47c294) [task 2018-04-20T13:46:05.919Z] [task 2018-04-20T13:46:05.919Z] AddressSanitizer can not provide additional info. [task 2018-04-20T13:46:05.919Z] SUMMARY: AddressSanitizer: SEGV /builds/worker/workspace/build/src/js/src/jsapi-tests/testBinASTReader.cpp:235:13 in void runTestFromPath<js::frontend::BinTokenReaderTester>(JSContext*, char const*) [task 2018-04-20T13:46:05.919Z] ==66444==ABORTING [task 2018-04-20T13:46:05.965Z] make -C js/src check-jstests [task 2018-04-20T13:46:06.236Z] make[1]: Entering directory '/builds/worker/workspace/build/src/obj-spider/js/src' [task 2018-04-20T13:46:06.237Z] ../../dist/bin/run-mozilla.sh /builds/worker/workspace/build/src/obj-spider/_virtualenv/bin/python -u /builds/worker/workspace/build/src/js/src/tests/jstests.py \ [task 2018-04-20T13:46:06.237Z] --no-progress --format=automation --timeout 300 \ [task 2018-04-20T13:46:06.237Z] --jitflags=none \ [task 2018-04-20T13:46:06.237Z] ../../dist/bin/js
Flags: needinfo?(choller)
Assignee | ||
Comment 7•6 years ago
|
||
The problem here is that the old BinTokenReaderTester also uses the matchConst function that I tried to patch here. In the new Multipart reader, it is only used for the header. Maybe we should additionally guard this by some kind of runtime check? One option would be to add a (fuzzing-only) API to the parser that allows switching off the matchConst strict checking at runtime. Does that sound ok to you Yoric, or would you prefer a simpler solution (e.g. using getenv/setenv)?
Flags: needinfo?(choller) → needinfo?(dteller)
Would it be ok if we just skipped the calls `matchConst` in the BinTokenReaderMultipart if we're in #ifdef FUZZING?
Flags: needinfo?(dteller) → needinfo?(choller)
Assignee | ||
Comment 9•6 years ago
|
||
Do you mean commenting out the checks directly in BinTokenReaderMultipart, e.g. disabling this? https://searchfox.org/mozilla-central/source/js/src/frontend/BinTokenReaderMultipart.cpp#69 If so, I guess that would work too.
Flags: needinfo?(choller) → needinfo?(dteller)
Yeah, this seems to be the simplest way to do it, right?
Flags: needinfo?(dteller)
Updated•6 years ago
|
Priority: -- → P2
Assignee | ||
Comment 11•6 years ago
|
||
Assignee | ||
Updated•6 years ago
|
Attachment #8969626 -
Attachment is obsolete: true
Comment 12•6 years ago
|
||
Comment on attachment 9009567 [details] Bug 1455593 - BinAST multipart fuzzing changes. r?arai Tooru Fujisawa [:arai] has approved the revision.
Attachment #9009567 -
Flags: review+
Comment 13•6 years ago
|
||
Pushed by choller@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/b26a70a0fe8f BinAST multipart fuzzing changes. r=arai
Comment 14•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/b26a70a0fe8f
Status: NEW → RESOLVED
Closed: 6 years ago
status-firefox64:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla64
Updated•6 years ago
|
status-firefox62:
--- → wontfix
status-firefox63:
--- → wontfix
Updated•5 years ago
|
Whiteboard: [adv-main64-]
You need to log in
before you can comment on or make changes to this bug.
Description
•