Difference for --validate-asm between default and ignition. |
||||||||
Issue description
The correctness fuzzer prototype reports a bunch of differences. Until it is fixed, I will blacklist --validate-asm. Here is one example, run with x64 release d8:
Minimized test case:
function __f_43() {
"use asm";
}
function __f_100( __v_3) {
var __v_11 = eval('(' + __f_43.toString() + ')');
gc();
};
__f_100();
__f_100();
__f_100();
( {
})();
Output of fuzzer:
# Result: Differed!
# CHECK
# Compared default with ignition
# Flags of default:
--abort_on_stack_overflow --expose-gc --allow-natives-syntax --invoke-weak-callbacks --omit-quit --gc-interval=398 --validate-asm --random-seed 1234
# Flags of ignition:
--abort_on_stack_overflow --expose-gc --allow-natives-syntax --invoke-weak-callbacks --omit-quit --gc-interval=398 --validate-asm --random-seed 1234 --ignition
Difference:
Different total output lines: 8 vs. 9
### Start of configuration default:
Validation of asm.js module failed: asm: line 1: Missing asm.js module export.
Validation of asm.js module failed: asm: line 1: Missing asm.js module export.
/usr/local/google/home/machenbach/v8/clusterfuzz-data/workbench/out1/fuzz-00802.minimized.js:12: TypeError: (intermediate value) is not a function
})();
^
TypeError: (intermediate value) is not a function
at /usr/local/google/home/machenbach/v8/clusterfuzz-data/workbench/out1/fuzz-00802.minimized.js:12:3
### End of configuration default
### Start of configuration ignition:
Validation of asm.js module failed: asm: line 1: Missing asm.js module export.
Validation of asm.js module failed: asm: line 1: Missing asm.js module export.
Validation of asm.js module failed: asm: line 1: Missing asm.js module export.
/usr/local/google/home/machenbach/v8/clusterfuzz-data/workbench/out1/fuzz-00802.minimized.js:12: TypeError: (intermediate value) is not a function
})();
^
TypeError: (intermediate value) is not a function
at /usr/local/google/home/machenbach/v8/clusterfuzz-data/workbench/out1/fuzz-00802.minimized.js:12:3
### End of configuration ignition
,
Nov 2 2016
Looks like the difference is just the number of verification errors reported here?
,
Nov 7 2016
,
Nov 9 2016
I'm currently trying to suppress the validator output. It would be quite easy if validator output had newlines after each error. But it often hasn't and produces output like: Validation of asm.js module failed: asm: line 100: Invalid variable initialization - it should be a literal, const, or fround(literal).output13_validate_asm/fuzz-00605.js.minimized:104: ReferenceError: __v_3 is not defined Would this be easy to add?
,
Nov 17 2016
The following revision refers to this bug: https://chromium.googlesource.com/v8/v8.git/+/ae3ca62b6b6caa212887014cbe068a9dcc878e7d commit ae3ca62b6b6caa212887014cbe068a9dcc878e7d Author: clemensh <clemensh@chromium.org> Date: Thu Nov 17 11:31:51 2016 [asmjs] Flush after printing error message This hopefully avoids multiple error messages appearing on the same line. R=titzer@chromium.org, machenbach@chromium.org BUG= chromium:660016 Review-Url: https://codereview.chromium.org/2503423006 Cr-Commit-Position: refs/heads/master@{#41070} [modify] https://crrev.com/ae3ca62b6b6caa212887014cbe068a9dcc878e7d/src/asmjs/asm-js.cc
,
Nov 22 2016
Michael, I'm close to having a CL to plumb these messages through to the dev console instead. Are the line breaks Clemens added enough, or would a flag to disable these messages be helpful? (I'd hate to loose the ability to get them on the d8 command line completely).
,
Nov 22 2016
There are still cases with missing linebreaks. E.g. just had: Validation of asm.js module failed: asm: line 100: Invalid variable initialization - it should be a literal, const, or fround(literal)./usr/local/google/home/machenbach/v8/clusterfuzz-data/fuzzers/workdir/output32_validate_asm/fuzz-00156.js.minimized:104: TypeError: 49 is not a function A flag to suppress those would be great. If the linebreaks were guaranteed to work, that'd be enough though.
,
Nov 22 2016
Hah, I know what this is. The string "asm: line 100: Invalid variable initialization - it should be a literal, const, or fround(literal)." has precisely 99 characters, and the limit for the error message (including null termiantor) is 100 characters. This means that there is no space for the terminating newline character in the buffer :) I propose to 1) Increase the size of the buffer to 128. 2) Not store the newline in the error message, for cases where it overflows again. CL to fix this: http://crrev.com/2523703002
,
Nov 22 2016
,
Nov 22 2016
The following revision refers to this bug: https://chromium.googlesource.com/v8/v8.git/+/332b9b584283d583f6f283984115426540180128 commit 332b9b584283d583f6f283984115426540180128 Author: clemensh <clemensh@chromium.org> Date: Tue Nov 22 17:17:34 2016 [asmjs] Avoid truncation of error messages We had error messages that exceeded the current limit of 100 characters, resulting in the newline being cut off. This CL also reverts http://crrev.com/2503423006 since it did not fix this issue. BUG= chromium:660016 R=machenbach@chromium.org, titzer@chromium.org Review-Url: https://codereview.chromium.org/2523703002 Cr-Commit-Position: refs/heads/master@{#41192} [modify] https://crrev.com/332b9b584283d583f6f283984115426540180128/src/asmjs/asm-js.cc [modify] https://crrev.com/332b9b584283d583f6f283984115426540180128/src/asmjs/asm-typer.cc [modify] https://crrev.com/332b9b584283d583f6f283984115426540180128/src/asmjs/asm-typer.h
,
Nov 23 2016
The last one did the trick. Lower prio as fuzzer now omits validator warnings properly. Thanks for the fix!
,
Dec 13 2016
,
Dec 14 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/07d5887d7471da0cf7bacb87d9742927f83f0c6d commit 07d5887d7471da0cf7bacb87d9742927f83f0c6d Author: bradnelson <bradnelson@chromium.org> Date: Wed Dec 14 14:21:27 2016 [wasm][asm.js] Routing asm.js warnings to the dev console. Route asm.js warnings from V8 to the developer console. Depends on: https://codereview.chromium.org/2526703002/ BUG= v8:4203 , chromium:660016 R=dgozman@chromium.org,jochen@chromium.org CC=danno@chromium.org,bmeurer@chromium.org Review-Url: https://codereview.chromium.org/2530503002 Cr-Commit-Position: refs/heads/master@{#438511} [modify] https://crrev.com/07d5887d7471da0cf7bacb87d9742927f83f0c6d/third_party/WebKit/LayoutTests/VirtualTestSuites [add] https://crrev.com/07d5887d7471da0cf7bacb87d9742927f83f0c6d/third_party/WebKit/LayoutTests/virtual/enable_asmjs/http/tests/asmjs/README.txt [add] https://crrev.com/07d5887d7471da0cf7bacb87d9742927f83f0c6d/third_party/WebKit/LayoutTests/virtual/enable_asmjs/http/tests/asmjs/asm-warnings-expected.txt [add] https://crrev.com/07d5887d7471da0cf7bacb87d9742927f83f0c6d/third_party/WebKit/LayoutTests/virtual/enable_asmjs/http/tests/asmjs/asm-warnings.html [add] https://crrev.com/07d5887d7471da0cf7bacb87d9742927f83f0c6d/third_party/WebKit/LayoutTests/virtual/enable_asmjs/http/tests/asmjs/worker.js [modify] https://crrev.com/07d5887d7471da0cf7bacb87d9742927f83f0c6d/third_party/WebKit/Source/bindings/core/v8/V8Initializer.cpp
,
Apr 27 2017
This doesn't repro anymore with current comparison setup and ToT. Is this fixed/wontfix?
,
Oct 2 2017
This has been addressed by now. |
||||||||
►
Sign in to add a comment |
||||||||
Comment 1 by machenb...@chromium.org
, Nov 2 2016