New issue
Advanced search Search tips

Issue 760790 link

Starred by 1 user

Issue metadata

Status: Verified
Owner:
Closed: Sep 2017
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 1
Type: Bug



Sign in to add a comment

Ill in v8::internal::__RT_impl_Runtime_AbortJS

Project Member Reported by ClusterFuzz, Aug 30 2017

Issue description

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

Fuzzer: inferno_js_fuzzer
Job Type: linux_asan_d8_dbg
Platform Id: linux

Crash Type: Ill
Crash Address: 0x7f269288fac8
Crash State:
  v8::internal::__RT_impl_Runtime_AbortJS
  v8::internal::Runtime_AbortJS
  v8::internal::Invoke
  
Sanitizer: address (ASAN)

Regressed: https://clusterfuzz.com/revisions?job=linux_asan_d8_dbg&range=47567:47568

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

Issue filed automatically.

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

abort: CSA_ASSERT failed: IntPtrOrSmiGreaterThan(capacity_node, IntPtrOrSmiConstant(0, mode), mode) [../src/code-stub-assembler.cc:2486]

Simplified repro ...

function g() {
  var a = Array(0); 
  a[0]++;
}
g();
g();
g();
Owner: jgruber@chromium.org
Status: Assigned (was: Untriaged)
Jakob, would you be willing to take a look at this? We seem to forget to canonicalize the empty FixedArray and hence the {ElementsTransitionAndStoreStub} gets confused and tries to allocate a zero-length FixedArray itself. Adding a %DebugPrint into the {g} method above shows different empty FixedArray objects as the elements ...

DebugPrint: 0x134352c8ea01: [JSArray]
 - map = 0x24784d302939 [FastProperties]
 - prototype = 0x1821b3e07f81
 - elements = 0x134352c8ea21 <FixedArray[0]> [HOLEY_SMI_ELEMENTS]
 - length = 0
 - properties = 0x241f96d02251 <FixedArray[0]> {
    #length: 0x241f96d7ec71 <AccessorInfo> (const accessor descriptor)
 }

DebugPrint: 0x134352c8ead9: [JSArray]
 - map = 0x24784d302939 [FastProperties]
 - prototype = 0x1821b3e07f81
 - elements = 0x134352c8eaf9 <FixedArray[0]> [HOLEY_SMI_ELEMENTS]
 - length = 0
 - properties = 0x241f96d02251 <FixedArray[0]> {
    #length: 0x241f96d7ec71 <AccessorInfo> (const accessor descriptor)
 }

Sure, I'll make some time tomorrow.
CSA::AllocateJSArray [0] doesn't canonicalize empty arrays if the given capacity is not a builtin-compile-time constant. We're also not running into any asserts that guard against this because we're extra-clever and fold JSArray + elements allocation (and there's no such asserts on this path). 

Got a fix in-flight.

[0] https://cs.chromium.org/chromium/src/v8/src/ic/ic.cc?l=2476&rcl=629b23ae58c01b3eba0f22e08f265942d2bca301
Project Member

Comment 5 by bugdroid1@chromium.org, Sep 1 2017

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

commit 2859dba71373f2bc6b58f6c3853623763bae8d4b
Author: Jakob Gruber <jgruber@chromium.org>
Date: Fri Sep 01 16:56:53 2017

[csa] Canonicalize empty elements in AllocateJSArray

Prior to this, AllocateJSArray would go ahead and allocate an empty
FixedArray as elements if passed any capacity that is not a compile-time
constant 0.

Things break later on since we rely on the fact that empty fixed arrays
are always canonicalize, and we use

  obj.elements == empty_fixed_array_constant

interchangeably with

  obj.elements.length == 0.

This CL introduces two new branches in AllocateJSArray: one if the
capacity is known to be non-zero; and another that explicitly
distinguishes between 0 and non-zero capacities.

Bug:  chromium:760790 
Change-Id: I7c22b19ce9ce15a46f91b0f75e6b4a1ff3a29a0f
Reviewed-on: https://chromium-review.googlesource.com/645959
Commit-Queue: Jakob Gruber <jgruber@chromium.org>
Reviewed-by: Camillo Bruni <cbruni@chromium.org>
Cr-Commit-Position: refs/heads/master@{#47776}
[modify] https://crrev.com/2859dba71373f2bc6b58f6c3853623763bae8d4b/src/builtins/builtins-proxy-gen.cc
[modify] https://crrev.com/2859dba71373f2bc6b58f6c3853623763bae8d4b/src/code-stub-assembler.cc
[modify] https://crrev.com/2859dba71373f2bc6b58f6c3853623763bae8d4b/src/code-stub-assembler.h
[add] https://crrev.com/2859dba71373f2bc6b58f6c3853623763bae8d4b/test/mjsunit/regress/regress-760790.js

Project Member

Comment 6 by ClusterFuzz, Sep 2 2017

ClusterFuzz has detected this issue as fixed in range 47775:47776.

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

Fuzzer: inferno_js_fuzzer
Job Type: linux_asan_d8_dbg
Platform Id: linux

Crash Type: Ill
Crash Address: 0x7f269288fac8
Crash State:
  v8::internal::__RT_impl_Runtime_AbortJS
  v8::internal::Runtime_AbortJS
  v8::internal::Invoke
  
Sanitizer: address (ASAN)

Regressed: https://clusterfuzz.com/revisions?job=linux_asan_d8_dbg&range=47567:47568
Fixed: https://clusterfuzz.com/revisions?job=linux_asan_d8_dbg&range=47775:47776

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

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 7 by ClusterFuzz, Sep 2 2017

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

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

Sign in to add a comment