New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 748137 link

Starred by 3 users

Issue metadata

Status: Verified
Owner:
OOO until 2019-02-10
Closed: Aug 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 1
Type: Bug



Sign in to add a comment

V8 correctness failure in configs: x64,ignition:ia32,ignition

Project Member Reported by ClusterFuzz, Jul 24 2017

Issue description

Detailed report: https://clusterfuzz.com/testcase?key=6495526450364416

Fuzzer: foozzie_js_mutation
Job Type: v8_foozzie
Platform Id: linux

Crash Type: V8 correctness failure
Crash Address: 
Crash State:
  configs: x64,ignition:ia32,ignition
  sources: ed5
  
Sanitizer: address (ASAN)

Regressed: V8: 46837:46838

Reproducer Testcase: https://clusterfuzz.com/download?testcase_id=6495526450364416


Issue filed automatically.

See https://github.com/google/clusterfuzz-tools for more information.
 
Cc: yangguo@chromium.org
Labels: -Pri-1 Pri-2
Owner: petermarshall@chromium.org
Status: Assigned (was: Untriaged)
This seems to run into a range error underneath. At different string sizes on 32/64 bits. It bisects to 
https://chromium-review.googlesource.com/c/570047/

[builtins] Increase the maximum string length on 64-bit platforms.

@Peter: Is it by design that we made 32/64 bits unequal for this case? Is there a way to suppress this? E.g. could we make it crash behind the --abort_on_stack_overflow flag if this difference is by design? Repro:

s1 = 'xxxxxxxxxxxxxxxxxxxxxxxxxxxxx';
s1 += s1;
s1 += s1;
s0 = '';
function foo() {  }
try {
  for (var i = 0; i < 4000000; i++) { s0 += s1; }
} catch (e) {
  print("caught")
}
 Issue 748302  has been merged into this issue.
 Issue 748365  has been merged into this issue.
 Issue 748422  has been merged into this issue.
 Issue 748423  has been merged into this issue.
 Issue 748468  has been merged into this issue.
Yes it is intentional that they are different, and in the future we might increase the 64 bit limit even more so it would be good to be able to handle different max string lengths with the fuzzer. I suspect this issue will come up in a few different codepaths which deal with strings.
Could we possibly just pass in the max string length as a flag to d8, in order to just limit it again for the fuzzer and make sure they are equal?
 Issue 748498  has been merged into this issue.
 Issue 748892  has been merged into this issue.
 Issue 748659  has been merged into this issue.
Labels: -Pri-2 Pri-1
Any update here how we can get rid of the noise? If we make intentional differences between ia32 and x64, then we can't compare for observable equality, unless we have a suppression mechanism. The noise this produces makes the system useless.
Cc: petermarshall@chromium.org
 Issue 748835  has been merged into this issue.
Status: Started (was: Assigned)
I'll work on crashing behind the --abort_on_stack_overflow flag in these cases, it seems to be the most reasonable thing. We could configure the length with a flag as suggested but I think this will be easier overall.
Project Member

Comment 15 by bugdroid1@chromium.org, Jul 31 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/1a087f027f93862288cf75d35ab8755ea47c172f

commit 1a087f027f93862288cf75d35ab8755ea47c172f
Author: Peter Marshall <petermarshall@chromium.org>
Date: Mon Jul 31 12:59:50 2017

[test] Crash on InvalidStringLength for correctness fuzzer.

Now that the maximum string length varies between platforms, the
correctness fuzzer is unhappy. It will ignore crashes, so when we know
we have reached platform-dependant behavior just crash if
--abort_on_stack_overflow is enabled.

Also rename abort_on_stack_overflow to
abort_on_stack_or_string_length_overflow.

Bug:  chromium:748137 
Change-Id: Ie4e96709b90029b5ce3c8408064d928f841b3b9f
Reviewed-on: https://chromium-review.googlesource.com/589269
Commit-Queue: Peter Marshall <petermarshall@chromium.org>
Reviewed-by: Michael Achenbach <machenbach@chromium.org>
Reviewed-by: Jakob Gruber <jgruber@chromium.org>
Reviewed-by: Michael Starzinger <mstarzinger@chromium.org>
Cr-Commit-Position: refs/heads/master@{#47007}
[modify] https://crrev.com/1a087f027f93862288cf75d35ab8755ea47c172f/src/compiler/js-intrinsic-lowering.cc
[modify] https://crrev.com/1a087f027f93862288cf75d35ab8755ea47c172f/src/compiler/js-intrinsic-lowering.h
[modify] https://crrev.com/1a087f027f93862288cf75d35ab8755ea47c172f/src/debug/debug-evaluate.cc
[modify] https://crrev.com/1a087f027f93862288cf75d35ab8755ea47c172f/src/factory.cc
[modify] https://crrev.com/1a087f027f93862288cf75d35ab8755ea47c172f/src/flag-definitions.h
[modify] https://crrev.com/1a087f027f93862288cf75d35ab8755ea47c172f/src/isolate.cc
[modify] https://crrev.com/1a087f027f93862288cf75d35ab8755ea47c172f/src/js/string.js
[modify] https://crrev.com/1a087f027f93862288cf75d35ab8755ea47c172f/src/regexp/regexp-parser.cc
[modify] https://crrev.com/1a087f027f93862288cf75d35ab8755ea47c172f/tools/foozzie/testdata/failure_output.txt
[modify] https://crrev.com/1a087f027f93862288cf75d35ab8755ea47c172f/tools/foozzie/v8_foozzie.py

 Issue 750437  has been merged into this issue.
Cc: machenb...@chromium.org
machenbach@ this should be fixed now with the CL above, is there any way to check? Or just wait a few days and see if any more issues come up?
Project Member

Comment 18 by ClusterFuzz, Aug 1 2017

ClusterFuzz has detected this issue as fixed in range 47006:47007.

Detailed report: https://clusterfuzz.com/testcase?key=6495526450364416

Fuzzer: foozzie_js_mutation
Job Type: v8_foozzie
Platform Id: linux

Crash Type: V8 correctness failure
Crash Address: 
Crash State:
  configs: x64,ignition:ia32,ignition
  sources: ed5
  
Sanitizer: address (ASAN)

Regressed: V8: 46837:46838
Fixed: V8: 47006:47007

Reproducer Testcase: https://clusterfuzz.com/download?testcase_id=6495526450364416


See https://github.com/google/clusterfuzz-tools for more information.

If you suspect that the result above is incorrect, try re-doing that job on the test case report page.
Status: Fixed (was: Started)
Clustefuzz verifications mean it's now fixed
Status: Verified (was: Fixed)
Yes, thanks for fixing. CF also closed all the duplicates...

Sign in to add a comment