New issue
Advanced search Search tips

Issue 805768 link

Starred by 2 users

Issue metadata

Status: Verified
Owner:
Closed: Jan 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 3
Type: Bug



Sign in to add a comment

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

Project Member Reported by ClusterFuzz, Jan 25 2018

Issue description

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

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: 48c
  
Sanitizer: address (ASAN)

Regressed: https://clusterfuzz.com/revisions?job=v8_foozzie&range=50839:50840

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

Issue filed automatically.

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

Comment 1 by ClusterFuzz, Jan 25 2018

Labels: Test-Predator-Auto-Owner
Owner: neis@chromium.org
Status: Assigned (was: Untriaged)
Automatically assigning owner based on suspected regression changelist https://chromium.googlesource.com/v8/v8/+/181ac2b0dcb18bc22fd119b5e228ed86b8337dc6 ([ic] Improve performance of KeyedStoreIC on literal-based arrays.).

If this is incorrect, please let us know why and apply the Test-Predator-Wrong-CLs label. If you aren't the correct owner for this issue, please unassign yourself as soon as possible so it can be re-triaged.
Cc: neis@chromium.org
 Issue 805812  has been merged into this issue.

Comment 3 by neis@chromium.org, Jan 25 2018

Cc: -neis@chromium.org bmeu...@chromium.org
Well done, Foozzie!

Here's simplified repro:

function foo() {
  var a = [''];
  print(a[0]);
  return a;
}
function bar(a) { a[0] = "bazinga!"; }
for (var i = 0; i < 5; i++) bar([]);
%OptimizeFunctionOnNextCall(bar);
bar(foo());
foo();

This incorrectly gives "bazinga!". Running with --no-turbo-load-elimination produces the correct result.

Comment 4 by neis@chromium.org, Jan 25 2018

Labels: -Pri-1 Pri-3
CL is reverted for now.
Could you check in the regression test so that we don't need to rely on clusterfuzz for this case?

Comment 6 by neis@chromium.org, Jan 25 2018

I was planning to do so in the CL that relands the change, once I have a fix.
Project Member

Comment 7 by ClusterFuzz, Jan 26 2018

ClusterFuzz has detected this issue as fixed in range 50859:50860.

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

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: 48c
  
Sanitizer: address (ASAN)

Regressed: https://clusterfuzz.com/revisions?job=v8_foozzie&range=50839:50840
Fixed: https://clusterfuzz.com/revisions?job=v8_foozzie&range=50859:50860

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

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.
Project Member

Comment 8 by ClusterFuzz, Jan 26 2018

Labels: ClusterFuzz-Verified
Status: Verified (was: Assigned)
ClusterFuzz testcase 5793358709784576 is verified as fixed, so closing issue as verified.

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

Comment 9 by bugdroid1@chromium.org, Jan 26 2018

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

commit 024d3499c28bf96c7eb2e74c1fc6df279d343857
Author: Georg Neis <neis@chromium.org>
Date: Fri Jan 26 11:11:03 2018

Reland "[ic] Improve performance of KeyedStoreIC on literal-based arrays."

This is a reland of 181ac2b0dcb18bc22fd119b5e228ed86b8337dc6 that fixes
the issue with load elimination.

Original change's description:
> [ic] Improve performance of KeyedStoreIC on literal-based arrays.
>
> In mode STORE_AND_GROW_NO_TRANSITION, the handler for elements stores
> used to bail out when seeing a COW array, even if the store that
> installed the handler had been operating on the very same array.
>
> This CL adds support for COW arrays to the mode (and renames it to
> STORE_AND_GROW_NO_TRANSITION_HANDLE_COW).
>
> Bug:  v8:7334 
> Change-Id: I6a15e8c1ff8d4ad4d5b8fc447745dce5d146c67c
> Reviewed-on: https://chromium-review.googlesource.com/876014
> Commit-Queue: Georg Neis <neis@chromium.org>
> Reviewed-by: Igor Sheludko <ishell@chromium.org>
> Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#50840}

TBR=bmeurer@chromium.org

Bug:  v8:7334 ,  chromium:805768 
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_chromium_rel_ng
Change-Id: I3d9c1b08583e08d68a1d30242a25e4a2190c8c55
Reviewed-on: https://chromium-review.googlesource.com/886261
Commit-Queue: Georg Neis <neis@chromium.org>
Reviewed-by: Igor Sheludko <ishell@chromium.org>
Reviewed-by: Jaroslav Sevcik <jarin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#50885}
[modify] https://crrev.com/024d3499c28bf96c7eb2e74c1fc6df279d343857/src/code-stub-assembler.cc
[modify] https://crrev.com/024d3499c28bf96c7eb2e74c1fc6df279d343857/src/code-stub-assembler.h
[modify] https://crrev.com/024d3499c28bf96c7eb2e74c1fc6df279d343857/src/code-stubs.cc
[modify] https://crrev.com/024d3499c28bf96c7eb2e74c1fc6df279d343857/src/compiler/js-native-context-specialization.cc
[modify] https://crrev.com/024d3499c28bf96c7eb2e74c1fc6df279d343857/src/compiler/load-elimination.cc
[modify] https://crrev.com/024d3499c28bf96c7eb2e74c1fc6df279d343857/src/ic/ic.cc
[modify] https://crrev.com/024d3499c28bf96c7eb2e74c1fc6df279d343857/src/objects.h
[add] https://crrev.com/024d3499c28bf96c7eb2e74c1fc6df279d343857/test/mjsunit/keyed-store-array-literal.js
[add] https://crrev.com/024d3499c28bf96c7eb2e74c1fc6df279d343857/test/mjsunit/regress/regress-805768.js

Sign in to add a comment