New issue
Advanced search Search tips

Issue 688734 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Feb 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_opt

Project Member Reported by ClusterFuzz, Feb 4 2017

Issue description

Cc: bmeu...@chromium.org jarin@chromium.org mstarzinger@chromium.org
Components: -Blink>JavaScript Blink>JavaScript>Compiler
Status: Available (was: Untriaged)
// PTAL. Requires always opt for repro:

function foo(a) { a[0] = 3; }
var v = {};
foo(v);
v.__p_1469921916 = 0;
foo([3]);
foo(v);
v = [,5];
gc();
v.__proto__ = [];
foo(v);
delete v[0];
var count = 0;
v.__proto__.__defineSetter__(0, function() { count++; });
foo([,5]);
foo(v);
print(count);

// Output:
# Compared x64,ignition with x64,ignition_turbo_opt
#
# Flags of x64,ignition:
--abort_on_stack_overflow --expose-gc --allow-natives-syntax --invoke-weak-callbacks --omit-quit --es-staging --random-seed 1234 --ignition --turbo-filter=~ --hydrogen-filter=~ --validate-asm --nocrankshaft
# Flags of x64,ignition_turbo_opt:
--abort_on_stack_overflow --expose-gc --allow-natives-syntax --invoke-weak-callbacks --omit-quit --es-staging --random-seed 1234 --ignition-staging --turbo --always-opt --validate-asm
#
# Difference:
- 1
+ 0
#
# Source file:
none
#
### Start of configuration x64,ignition:
1

### End of configuration x64,ignition
#
### Start of configuration x64,ignition_turbo_opt:
0

### End of configuration x64,ignition_turbo_opt

Components: -Blink>JavaScript>Compiler Blink>JavaScript
This actually seems to be unrelated to which configuration is being used and solely relies on GC timing. I can make both configuration either succeed or fail by slightly changing the GC timing. Needs further investigation.
Cc: ishell@chromium.org cbruni@chromium.org verwa...@chromium.org
This is caused by the {StoreFastElementStub} used inside a polymorphic keyed store but ignoring the prototype chain. The following is a stable repro across configurations:

function foo(a) { a[0] = 3; }
var v = [,6];
v.__proto__ = [];
foo(v);
delete v[0];
var count = 0;
v.__proto__.__defineSetter__(0, function() { print("YUP!"); count++; });
foo([1]);
foo(v);
print(count);

Comment 4 by ishell@chromium.org, Feb 21 2017

Owner: ishell@chromium.org
Status: Assigned (was: Available)
Thanks for the repro!
Project Member

Comment 5 by bugdroid1@chromium.org, Feb 24 2017

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

commit 3233afb6261e6b1b7a62dbcbf40e4e20f8170309
Author: Igor Sheludko <ishell@chromium.org>
Date: Fri Feb 24 10:32:48 2017

[ic] Move code from ic-compiler.h/.cc to KeyedStoreIC.

This is a preliminary cleanup necessary for the actual fix of the associated issue.

BUG= chromium:688734 

Change-Id: Iecd39ed4cef34c6cc5d9652c5569e048f0db46af
Reviewed-on: https://chromium-review.googlesource.com/446341
Commit-Queue: Igor Sheludko <ishell@chromium.org>
Reviewed-by: Igor Sheludko <ishell@chromium.org>
Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
Cr-Commit-Position: refs/heads/master@{#43410}
[modify] https://crrev.com/3233afb6261e6b1b7a62dbcbf40e4e20f8170309/BUILD.gn
[modify] https://crrev.com/3233afb6261e6b1b7a62dbcbf40e4e20f8170309/src/ic/arm/ic-arm.cc
[modify] https://crrev.com/3233afb6261e6b1b7a62dbcbf40e4e20f8170309/src/ic/arm64/ic-arm64.cc
[modify] https://crrev.com/3233afb6261e6b1b7a62dbcbf40e4e20f8170309/src/ic/ia32/ic-ia32.cc
[delete] https://crrev.com/e0e8b2a2f2370363aa6a502e28acca193bd037cf/src/ic/ic-compiler.cc
[delete] https://crrev.com/e0e8b2a2f2370363aa6a502e28acca193bd037cf/src/ic/ic-compiler.h
[modify] https://crrev.com/3233afb6261e6b1b7a62dbcbf40e4e20f8170309/src/ic/ic.cc
[modify] https://crrev.com/3233afb6261e6b1b7a62dbcbf40e4e20f8170309/src/ic/ic.h
[modify] https://crrev.com/3233afb6261e6b1b7a62dbcbf40e4e20f8170309/src/ic/mips/ic-mips.cc
[modify] https://crrev.com/3233afb6261e6b1b7a62dbcbf40e4e20f8170309/src/ic/mips64/ic-mips64.cc
[modify] https://crrev.com/3233afb6261e6b1b7a62dbcbf40e4e20f8170309/src/ic/ppc/ic-ppc.cc
[modify] https://crrev.com/3233afb6261e6b1b7a62dbcbf40e4e20f8170309/src/ic/s390/ic-s390.cc
[modify] https://crrev.com/3233afb6261e6b1b7a62dbcbf40e4e20f8170309/src/ic/x64/ic-x64.cc
[modify] https://crrev.com/3233afb6261e6b1b7a62dbcbf40e4e20f8170309/src/ic/x87/ic-x87.cc
[modify] https://crrev.com/3233afb6261e6b1b7a62dbcbf40e4e20f8170309/src/v8.gyp

Project Member

Comment 6 by bugdroid1@chromium.org, Feb 27 2017

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

commit 9760851789c11f2ecd41734ab1c4b86b2f5269b8
Author: Igor Sheludko <ishell@chromium.org>
Date: Mon Feb 27 13:41:11 2017

[ic] KeyedStoreIC should use a slow stub when a prototype chain contains dictionary elements.

BUG= chromium:688734 

Change-Id: If5dd370971cb02c4463fa20a489d0fa60b0423c4
Reviewed-on: https://chromium-review.googlesource.com/446845
Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
Commit-Queue: Igor Sheludko <ishell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#43451}
[modify] https://crrev.com/9760851789c11f2ecd41734ab1c4b86b2f5269b8/src/ic/ic.cc
[add] https://crrev.com/9760851789c11f2ecd41734ab1c4b86b2f5269b8/test/mjsunit/regress/regress-crbug-688734.js

Comment 7 by ishell@chromium.org, Feb 27 2017

Status: Fixed (was: Assigned)
Project Member

Comment 8 by ClusterFuzz, Feb 28 2017

ClusterFuzz has detected this issue as fixed in range 43450:43451.

Detailed report: https://cluster-fuzz.appspot.com/testcase?key=6599578469269504

Fuzzer: foozzie_js_mutation
Job Type: foozzie_ignition_turbo_opt
Platform Id: linux

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

Regressed: V8: 42658:42659
Fixed: V8: 43450:43451

Reproducer Testcase: https://cluster-fuzz.appspot.com/download/AMIfv94ob5_fluttBfUnMvsIUO2AWsBHEPKHJlxmk-HOp_XfNMJGg_S0YYZ-9i4VjB28aZCOl5elCtZlIAOc55go58R2kqihDXxcG_Da9mgXgfO5Ksl4b3a1zHY-YTxt_hGZvw4BiIUBBmvcFsGb0vk61e-AsptguUp73uEP6Ks9PdZYddX2DytgTpstX1j3AmN6BtvXMy9OCUsHKJD6x2W8lfCq2JcjsCUcvv5BQr8WBw7WGc3YWcqU8xg8tJhMldWMgWKPvcTPnYWRrdeEOMLx-IPSHSVmEbuqVvRY5MBmgBnZuZL8FVTB_aNqrFlUknWJ9xaidL8fVKy1mKLmvhMqi1EK65LOv_lcOpJOYvLrqSy4ARUp4Rh5UriN47XTV0zT56qvjS1zhmwvfR7GC9sUyFZb1i3dWw?testcase_id=6599578469269504


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.

Sign in to add a comment