Automatically adding ccs based on suspected regression changelists:

[builtins] Port Array.p.join to Torque. by peter.wm.wong@gmail.com - https://chromium.googlesource.com/v8/v8/+/952c097679c5e16ae214595ad3b01381483eab7b

If this is incorrect, please let us know why and apply the Test-Predator-Wrong-CLs label.

Could some one post the repro?
Thanks!

The test case is attached, and here is some extra info from CF. Note that this appears to be a correctness, not a security issue. 

At first blush, it appears to be a difference between ia32 and x64 in the Ignition-only configuration:

#
# V8 correctness failure
# V8 correctness configs: x64,ignition:ia32,ignition
# V8 correctness sources: 6b4
# V8 correctness suppression:
#
# CHECK
#
# Compared x64,ignition with ia32,ignition
#
# Flags of x64,ignition:
--abort_on_stack_or_string_length_overflow --expose-gc --allow-natives-syntax --invoke-weak-callbacks --omit-quit --es-staging --wasm-num-compilation-tasks=0 --suppress-asm-messages --random-seed 1243996048 --turbo-filter=~ --noopt --liftoff --no-wasm-tier-up
# Flags of ia32,ignition:
--abort_on_stack_or_string_length_overflow --expose-gc --allow-natives-syntax --invoke-weak-callbacks --omit-quit --es-staging --wasm-num-compilation-tasks=0 --suppress-asm-messages --random-seed 1243996048 --turbo-filter=~ --noopt --liftoff --no-wasm-tier-up
#

Added machenbach on cc, since I don't know enough about Foozie to know if this is a real problem or not or exactly how to find the discrepancy (I assume you just compare the outputs).

Deleted: clusterfuzz-testcase-6049607967309824.js 27.3 KB

clusterfuzz-testcase-6049607967309824.js 27.3 KB View Download

Reduced repro:
(function () {  
  assertThrows = function assertThrows(code) {
    try {
        code();
    } catch (e) {
    }
  }
  })();
assertThrows((function() {
  s = "Hello World!\n";
  while (true) {
    x = new Array();
    x[0] = s;
    x[1000] = s;
    print("Still there");
    s = x.join();
  }}));

The problem seems to be a RangeError. On x64 and ia32 we iterate this loop different times due to the RangeError happening at different times.

Maybe your CL introduces a new possibility to raise a range error which isn't suppressed by the --abort-on-stack-overflow flag.

See also  issue 664068 . Maybe we need a similar patch as https://codereview.chromium.org/2509843005 ?

I see two occurrences of ThrowRangeError in the code. Could you modify this to instead crash if the --abort-on-stack-overflow flag is set?

This isn't a stack overflow though, is it? It's just that the string 's' grows unboundedly, and that the max string length differs on 32/64bit platforms. 

https://cs.chromium.org/chromium/src/v8/src/objects/string.h?l=362&rcl=c1d2886c58a697c34233fb48f5ac38188e093f3c

Not sure how to best solve this for correctness fuzzing (the code itself is behaving correctly).

It'd treat it the same way with correctness fuzzing. The moment we reach this state we can't do any reasonable comparison anymore. So it's best to just bail out. Not sure if the name --abort-on-stack-overflow is really still fitting, but for simplicity I'd add a bail-out under this flag when breaching max-string-length. We can think about renaming the flag at some point.

Ah sorry, I didn't realize the flag had already been renamed to

  FLAG_abort_on_stack_or_string_length_overflow

Suggestions in #4 and #5 make sense.. Peter could we call Runtime_ThrowInvalidStringLength instead?

Right, forgot about that. Indeed that's what we pass.

Yup yup, CL in-flight: https://chromium-review.googlesource.com/c/v8/v8/+/1322312

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

commit c80d049a75830b9c256a9a16785629b02bba1d47
Author: peterwmwong <peter.wm.wong@gmail.com>
Date: Wed Nov 07 08:29:31 2018

[builtins] Change A.p.join invalid string length errors to use ThrowInvalidStringLength runtime.

This is to enable switching from throwing a JS exception (RangeError)
to an abort when the --abort_on_stack_or_string_length_overflow flag
is set.

Bug:  chromium:901652 
Change-Id: Ia3ff2ec55e77a4f60d715f0bc767e6180a5e001a
Reviewed-on: https://chromium-review.googlesource.com/c/1322312
Commit-Queue: Peter Wong <peter.wm.wong@gmail.com>
Reviewed-by: Jakob Gruber <jgruber@chromium.org>
Reviewed-by: Michael Achenbach <machenbach@chromium.org>
Cr-Commit-Position: refs/heads/master@{#57307}
[modify] https://crrev.com/c80d049a75830b9c256a9a16785629b02bba1d47/src/builtins/array-join.tq
[modify] https://crrev.com/c80d049a75830b9c256a9a16785629b02bba1d47/src/builtins/base.tq

ClusterFuzz has detected this issue as fixed in range 57306:57307.

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

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: 6b4
  
Sanitizer: address (ASAN)

Regressed: https://clusterfuzz.com/revisions?job=v8_foozzie&range=56698:56699
Fixed: https://clusterfuzz.com/revisions?job=v8_foozzie&range=57306:57307

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

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.

ClusterFuzz testcase 6049607967309824 is verified as fixed, so closing issue as verified.

If this is incorrect, please add ClusterFuzz-Wrong label and re-open the issue.