New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.
Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Closed: Aug 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Mac
Pri: 1
Type: Bug-Security

Blocking:
issue 758821



Sign in to add a comment

DCHECK failure in result->owns_descriptors() in objects.cc

Project Member Reported by ClusterFuzz, Aug 19 2017

Issue description

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

Fuzzer: mbarbella_js_mutation
Job Type: mac_asan_d8_dbg
Platform Id: mac

Crash Type: DCHECK failure
Crash Address: 
Crash State:
  result->owns_descriptors() in objects.cc
  
Sanitizer: address (ASAN)

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

Issue filed automatically.

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

Comment 1 by sheriffbot@chromium.org, Aug 20 2017

Labels: M-60
Project Member

Comment 2 by sheriffbot@chromium.org, Aug 20 2017

Labels: Pri-1

Comment 3 by rsesek@chromium.org, Aug 21 2017

Owner: rossberg@chromium.org
Status: Assigned (was: Untriaged)
rossberg: Assigning to you (v8 sheriff) to help triage, since CF couldn't find a regression range.
Cc: rossberg@chromium.org
Owner: ----
Status: Untriaged (was: Assigned)
rossberg@ is on vacation.
Owner: mstarzinger@chromium.org
Status: Assigned (was: Untriaged)
Assigning to new on-duty sheriff.
Cc: jarin@chromium.org
Components: -Blink>JavaScript Blink>JavaScript>Compiler
Somewhat reduced repro ...

var obj1 = {};
var obj2 = {};
function h() {}
h.prototype = obj2;
function g(v) {
  v.constructor;
}
function f() {
  g(obj1);
}
obj1.x = 0;
f();
obj1.__defineGetter__("x", function() {});
%OptimizeFunctionOnNextCall(f);
g(obj2);
obj2.y = 0;
f();

Cc: cbruni@chromium.org
Components: Blink>JavaScript>Runtime
The Map we are trying to find the root map for looks "weird", descriptor array being the EmptyFixedArray but the map not owning it ... I will need help from the runtime team with this ...
Actually pasting the map ...

0x1f8bcaa8d519: [Map]
 - type: JS_OBJECT_TYPE
 - instance size: 56
 - inobject properties: 4
 - elements kind: HOLEY_ELEMENTS
 - unused property fields: 4
 - enum length: invalid
 - prototype_map
 - prototype info: 0
 - instance descriptors #0: 0x8839ea02231 <FixedArray[0]>
 - layout descriptor: 0
 - prototype: 0x338152f845f9 <Object map = 0x1f8bcaa822b1>
 - constructor: 0x338152f84631 <JSFunction Object (sfi = 0x8839ea0c3d9)>
 - code cache: 0x8839ea02241 <FixedArray[0]>
 - dependent code: 0x8839ea02241 <FixedArray[0]>
 - construction counter: 0

Comment 9 by cbruni@chromium.org, Aug 24 2017

Cc: mstarzinger@chromium.org
Owner: ishell@chromium.org
We should actually not look at that prototype map anymore.
Project Member

Comment 10 by bugdroid1@chromium.org, Aug 24 2017

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

commit 8974b75bce037555cb93c4d501176986dd0dc935
Author: Camillo Bruni <cbruni@chromium.org>
Date: Thu Aug 24 16:55:13 2017

[runtime] Deprecate old prototype maps

Bug:  chromium:757199 
Change-Id: I5936fab1784ebf8de6eddd3b2bec0e2cf1b73f82
Reviewed-on: https://chromium-review.googlesource.com/632317
Reviewed-by: Igor Sheludko <ishell@chromium.org>
Commit-Queue: Camillo Bruni <cbruni@chromium.org>
Cr-Commit-Position: refs/heads/master@{#47581}
[modify] https://crrev.com/8974b75bce037555cb93c4d501176986dd0dc935/src/feedback-vector.cc
[modify] https://crrev.com/8974b75bce037555cb93c4d501176986dd0dc935/src/objects.cc
[add] https://crrev.com/8974b75bce037555cb93c4d501176986dd0dc935/test/mjsunit/regress/regress-crbug-757199.js

Project Member

Comment 11 by ClusterFuzz, Aug 25 2017

ClusterFuzz has detected this issue as fixed in range 47580:47581.

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

Fuzzer: mbarbella_js_mutation
Job Type: mac_asan_d8_dbg
Platform Id: mac

Crash Type: DCHECK failure
Crash Address: 
Crash State:
  result->owns_descriptors() in objects.cc
  v8::internal::Map::FindRootMap
  v8::internal::compiler::JSNativeContextSpecialization::ExtractReceiverMaps
  
Sanitizer: address (ASAN)

Fixed: https://clusterfuzz.com/revisions?job=mac_asan_d8_dbg&range=47580:47581

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

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.
Status: Fixed (was: Assigned)
Project Member

Comment 13 by ClusterFuzz, Aug 25 2017

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

If this is incorrect, please add ClusterFuzz-Wrong label and re-open the issue.
Status: Started (was: Verified)
revert about to land, initial fixed caused serious performance regressions.
Project Member

Comment 15 by bugdroid1@chromium.org, Aug 25 2017

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

commit 0582f029df3ef9eaea7a9260dbffb07614c6cf75
Author: Camillo Bruni <cbruni@chromium.org>
Date: Fri Aug 25 08:59:20 2017

Revert "[runtime] Deprecate old prototype maps"

This reverts commit 8974b75bce037555cb93c4d501176986dd0dc935.

Reason for revert: In hindsight, the CL made only partially sense and causes unnecessary IC-misses.

Original change's description:
> [runtime] Deprecate old prototype maps
> 
> Bug:  chromium:757199 
> Change-Id: I5936fab1784ebf8de6eddd3b2bec0e2cf1b73f82
> Reviewed-on: https://chromium-review.googlesource.com/632317
> Reviewed-by: Igor Sheludko <ishell@chromium.org>
> Commit-Queue: Camillo Bruni <cbruni@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#47581}

TBR=cbruni@chromium.org,ishell@chromium.org

Change-Id: I9f43a5f8c5242f575346f47c24377dd832eeccd1
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  chromium:757199 
Reviewed-on: https://chromium-review.googlesource.com/634906
Reviewed-by: Camillo Bruni <cbruni@chromium.org>
Commit-Queue: Camillo Bruni <cbruni@chromium.org>
Cr-Commit-Position: refs/heads/master@{#47594}
[modify] https://crrev.com/0582f029df3ef9eaea7a9260dbffb07614c6cf75/src/feedback-vector.cc
[modify] https://crrev.com/0582f029df3ef9eaea7a9260dbffb07614c6cf75/src/objects.cc
[delete] https://crrev.com/2ee967d253f0cc6a7dd61633f1176489711ef8a3/test/mjsunit/regress/regress-crbug-757199.js

Additional secondary test-case: 

(0).__defineGetter__(0, function() { });
Number.prototype[0] = "string";

Blocking: 758821
Project Member

Comment 18 by ClusterFuzz, Aug 25 2017

Labels: OS-Linux
Project Member

Comment 19 by bugdroid1@chromium.org, Aug 25 2017

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

commit 8a7ce927a685d4007e02298881d85c7d5f273777
Author: Camillo Bruni <cbruni@chromium.org>
Date: Fri Aug 25 10:44:29 2017

Don't look at abandoned prototype maps when looking for root maps

Bug:  chromium:757199 ,  chromium:758773 , chromium:758821
Change-Id: I70644853770501b13992bd7bf78d168ca2308d64
Reviewed-on: https://chromium-review.googlesource.com/635223
Reviewed-by: Igor Sheludko <ishell@chromium.org>
Reviewed-by: Michael Starzinger <mstarzinger@chromium.org>
Commit-Queue: Igor Sheludko <ishell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#47603}
[modify] https://crrev.com/8a7ce927a685d4007e02298881d85c7d5f273777/src/compiler/js-native-context-specialization.cc
[add] https://crrev.com/8a7ce927a685d4007e02298881d85c7d5f273777/test/mjsunit/regress/regress-crbug-757199.js
[add] https://crrev.com/8a7ce927a685d4007e02298881d85c7d5f273777/test/mjsunit/regress/regress-crbug-758773.js

Cc: -cbruni@chromium.org ishell@chromium.org
Owner: cbruni@chromium.org
Status: Fixed (was: Started)
This seems to have sticked and not cause any regressions on the previously affected benchmarks.
Awesome. Thanks Camillo!
Project Member

Comment 22 by sheriffbot@chromium.org, Aug 28 2017

Labels: -Restrict-View-SecurityTeam Restrict-View-SecurityNotify
Labels: -M-60 M-62
Project Member

Comment 24 by sheriffbot@chromium.org, Sep 15 2017

Labels: Merge-Request-62
Project Member

Comment 25 by sheriffbot@chromium.org, Sep 15 2017

Labels: -Merge-Request-62 Merge-Review-62 Hotlist-Merge-Review
This bug requires manual review: Reverts referenced in bugdroid comments after merge request.
Please contact the milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), bhthompson@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Cc: awhalley@chromium.org
+awhalley@ - can you please confirm if this needs to be taken for M62?
cbruni@ in #20 confirmed this has landed OK in M62 - no merge needed.
Labels: -Hotlist-Merge-Review -Merge-Review-62
Labels: Release-0-M62
Project Member

Comment 30 by sheriffbot@chromium.org, Dec 4 2017

Labels: -Restrict-View-SecurityNotify allpublic
This bug has been closed for more than 14 weeks. Removing security view restrictions.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Sign in to add a comment