New issue
Advanced search Search tips

Issue 707065 link

Starred by 2 users

Issue metadata

Status: Duplicate
Merged: issue 707992
Owner:
Closed: Apr 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 1
Type: Bug



Sign in to add a comment

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

Project Member Reported by ClusterFuzz, Mar 30 2017

Issue description

Cc: jarin@chromium.org mstarzinger@chromium.org danno@chromium.org
Components: -Blink>JavaScript Blink>JavaScript>Compiler
Owner: bmeu...@chromium.org
Status: Assigned (was: Untriaged)
// PTAL. Bisect points to:
https://chromium.googlesource.com/v8/v8/+/9df5674bd53b4a262e72f45263df9e886842c269

// Repro:

function __f_5() {
  return function() {boom;};
}
function __f_7(array) {
  array.reduce(__f_5());
}
function __f_18() {
  __f_7([1]);
}
__f_18();
%OptimizeFunctionOnNextCall(__f_18);
__f_18();

// Output:

# Compared x64,ignition with x64,ignition_turbo
#
# Flags of x64,ignition:
--abort_on_stack_overflow --expose-gc --allow-natives-syntax --invoke-weak-callbacks --omit-quit --es-staging --random-seed 732681078 --ignition --turbo-filter=~ --hydrogen-filter=~ --nocrankshaft
# Flags of x64,ignition_turbo:
--abort_on_stack_overflow --expose-gc --allow-natives-syntax --invoke-weak-callbacks --omit-quit --es-staging --random-seed 732681078 --ignition --turbo
#
# Difference:
+ ./repro.js:2: ReferenceError: boom is not defined
#
### Start of configuration x64,ignition:

### End of configuration x64,ignition
#
### Start of configuration x64,ignition_turbo:
./repro.js:2: ReferenceError: boom is not defined
  return function() {boom;};
                     ^



### End of configuration x64,ignition_turbo

Cc: bmeu...@chromium.org
 Issue 707148  has been merged into this issue.
Note, the duplicate above might have a nicer (different?) repro.
Mergedinto: 707992
Status: Duplicate (was: Assigned)
Project Member

Comment 5 by bugdroid1@chromium.org, Apr 12 2017

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

commit d98dfd8b9b68635c3b974e1d91be414304dec35c
Author: bmeurer <bmeurer@chromium.org>
Date: Wed Apr 12 04:32:05 2017

Revert "[turbofan] Avoid going through ArgumentsAdaptorTrampoline for CSA/C++ builtins."

This reverts commit 9df5674bd53b4a262e72f45263df9e886842c269 because it
is not compatible with the way that Array.prototype.reduceRight and
Array.prototype.reduce deal with optional parameters at this point (i.e.
parameters where the behavior is different depending on whether the
parameter was skipped or undefined was passed).

In general, it might be better to not adapt arguments for builtins with
optional paramters, that are likely skipped, for example as in
Object.create or Array.prototype.reduce. Since that will require
arguments adaptor frames for normal calls, especially from baseline
code. Instead it might make sense to use the variadic arguments support
in the CodeStubAssembler instead to avoid the arguments adaptor in all
cases (not only when called from TurboFan optimized code).

BUG=v8:5267, chromium:709782 , chromium:707992 , chromium:708282 , chromium:708599 , chromium:709173 , chromium:709747 , chromium:707065 , chromium:710417 
TBR=danno@chromium.org

Review-Url: https://codereview.chromium.org/2817653002
Cr-Commit-Position: refs/heads/master@{#44593}

[modify] https://crrev.com/d98dfd8b9b68635c3b974e1d91be414304dec35c/src/compiler/js-call-reducer.cc

Project Member

Comment 6 by ClusterFuzz, Apr 12 2017

ClusterFuzz has detected this issue as fixed in range 44592:44593.

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

Fuzzer: foozzie_js_mutation
Job Type: v8_foozzie
Platform Id: linux

Crash Type: V8 correctness failure
Crash Address: 
Crash State:
  configs: x64,ignition:x64,ignition_turbo
  sources: 5fa
  
Sanitizer: address (ASAN)

Regressed: V8: 44217:44218
Fixed: V8: 44592:44593

Reproducer Testcase: https://clusterfuzz.com/download/AMIfv97gVskNxWqlUoG7ZycSTke3fhz0wiVjQkqXw7WsBJipTqfQjq6MxOH8Cn50hlZbR1IPaFYUaiamRKNA_awbaL3rtBVQ69FFQU6r7CHkU5L7AHOC3N3X8KeNk2aaH0BMCwVM77fAGKaaz8fbOo9c4VC2OaekaxI6JsM1imikTQZgVyU-r0IemXr9BgwyCSa0wjQWu4PzZ78R95gs3iJoKU5fhfV-Q9uv4N9VhgVy0b7bjv0Rh7DkfjhTof5TbC17ePd4M0ThpaZNTlN1x0MMSiYTAmPM2yBh0sYgk2L7Dz8Q-6F8cJFyoKGNJU60LhrTDtG_ND-2143SezgLAupVB92jC65nLtPnl97Ly5Zg1bzBiLl-D2QPW7OVnwwl8BSXk2jFfETtPIpumcYnM0Jg6zE6F2Yr_Q?testcase_id=5411865602818048


See https://dev.chromium.org/Home/chromium-security/bugs/reproducing-clusterfuzz-bugs for more information.

If you suspect that the result above is incorrect, try re-doing that job on the test case report page.
Project Member

Comment 7 by bugdroid1@chromium.org, Apr 13 2017

Labels: merge-merged-5.9
The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/ddf03166c01372d8da269cfaa00188e3113465ce

commit ddf03166c01372d8da269cfaa00188e3113465ce
Author: Michael Hablich <hablich@chromium.org>
Date: Thu Apr 13 12:23:05 2017

Merged: Revert "[turbofan] Avoid going through ArgumentsAdaptorTrampoline for CSA/C++ builtins."

Revision: d98dfd8b9b68635c3b974e1d91be414304dec35c

BUG= chromium:707065 , chromium:707992 , chromium:708282 , chromium:708599 , chromium:709173 , chromium:709747 , chromium:709782 , chromium:710417 ,v8:5267
LOG=N
NOTRY=true
NOPRESUBMIT=true
NOTREECHECKS=true
TBR=bmeurer@chromium.org

Change-Id: I2363c9012d7107e5e246d46bf6938bead642b486
Reviewed-on: https://chromium-review.googlesource.com/476351
Reviewed-by: Michael Hablich <hablich@chromium.org>
Cr-Commit-Position: refs/branch-heads/5.9@{#4}
Cr-Branched-From: fe9bb7e6e251159852770160cfb21dad3cf03523-refs/heads/5.9.211@{#1}
Cr-Branched-From: 70ad23791a21c0dd7ecef8d4d8dd30ff6fc291f6-refs/heads/master@{#44591}
[modify] https://crrev.com/ddf03166c01372d8da269cfaa00188e3113465ce/src/compiler/js-call-reducer.cc

Sign in to add a comment