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

Issue 808845 link

Starred by 2 users

Issue metadata

Status: Verified
Owner:
Closed: Mar 2018
Cc:
Components:
EstimatedDays: ----
NextAction: 2018-03-03
OS: Linux
Pri: 1
Type: Bug



Sign in to add a comment

Null-dereference READ in v8::internal::LoadHandler::LoadFromPrototype

Project Member Reported by ClusterFuzz, Feb 4 2018

Issue description

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

Fuzzer: inferno_twister
Job Type: linux_cfi_chrome
Platform Id: linux

Crash Type: Null-dereference READ
Crash Address: 0x000000000000
Crash State:
  v8::internal::LoadHandler::LoadFromPrototype
  v8::internal::LoadIC::ComputeHandler
  v8::internal::LoadIC::UpdateCaches
  
Sanitizer: cfi (CFI)

Regressed: https://clusterfuzz.com/revisions?job=linux_cfi_chrome&range=523197:523221

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

Issue filed automatically.

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

Comment 1 by ClusterFuzz, Feb 4 2018

Components: Blink>JavaScript>Runtime
Labels: Test-Predator-Auto-Components
Automatically applying components based on crash stacktrace and information from OWNERS files.

If this is incorrect, please apply the Test-Predator-Wrong-Components label.
Cc: clemensh@chromium.org ishell@chromium.org
Owner: clemensh@chromium.org
Status: Assigned (was: Untriaged)
assigning for retriage by sheriff.
Cc: -clemensh@chromium.org mstarzinger@chromium.org yangguo@chromium.org
Owner: verwa...@chromium.org
Bisects to 460652c9786c686094b71f9d64e3e6025252c11a ([ic] Migrate API getters to data handlers), which matches the stack trace of the failure.
Toon, can you take a look?
CCing reviewers.

Comment 4 by ishell@chromium.org, Feb 22 2018

Cc: -ishell@chromium.org verwa...@chromium.org
Owner: ishell@chromium.org
Status: Started (was: Assigned)
Labels: M-64 M-65 M-66
Project Member

Comment 7 by bugdroid1@chromium.org, Mar 1 2018

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

commit 16a3a4e94643a77ab00df63116191839e18ffed9
Author: Igor Sheludko <ishell@chromium.org>
Date: Thu Mar 01 15:18:44 2018

[ic] Properly handle kApiGetter case with null prototype.

Bug:  chromium:808845 
Change-Id: I406ca472e74b8fce5f79bc389bd40aec7dcebb84
Reviewed-on: https://chromium-review.googlesource.com/943261
Reviewed-by: Camillo Bruni <cbruni@chromium.org>
Commit-Queue: Igor Sheludko <ishell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#51661}
[modify] https://crrev.com/16a3a4e94643a77ab00df63116191839e18ffed9/src/ic/handler-configuration.cc
[modify] https://crrev.com/16a3a4e94643a77ab00df63116191839e18ffed9/test/cctest/test-accessors.cc

Labels: Merge-Request-64 Merge-Request-66 Merge-Request-65
Status: Fixed (was: Started)
Project Member

Comment 9 by sheriffbot@chromium.org, Mar 1 2018

Labels: -Merge-Request-66 Merge-Review-66 Hotlist-Merge-Review
This bug requires manual review: We don't branch M66 until 2018-03-01.
Please contact the milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), josafat@(ChromeOS), abdulsyed@(Desktop)

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

Comment 10 by sheriffbot@chromium.org, Mar 1 2018

Labels: -Merge-Request-65 Merge-Review-65
This bug requires manual review: We are only 4 days from stable.
Please contact the milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), bhthompson@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Before we approve merge to M65, could you pls confirm followings?

Is this M65 regression and critical to merge? I see this exists on M64 so not M65 regression. Can't this wait M66 as M65 is going to stable next week?
Is the change well baked/verified in Canary, having enough automation tests coverage and safe to merge?
Any other imp details to justify the merge.

Please note M65 is VERY close to Stable promotion so merge bar is very high. Thank you.

Cl listed at #7 is not yet baked/verified in canary as it landed 66 mins back.
We don't have a Canary coverage yet (I landed it 1 hour ago).
It's easily reproducible and the fix is also simple (basically one-liner) and safe-mergeable. 
I'd suggest to get a Canary coverage and then merge it everywhere.
NextAction: 2018-03-01
Pls update bug with canary result tomorrow. Thank you.
NextAction: 2018-03-02
Cc: hablich@chromium.org
+hablich@ (V8 TPM)
Cc: abdulsyed@chromium.org
Labels: -Merge-Request-64 Merge-Rejected-64
We're not planning further M64 releases so rejecting merge to M64.
The NextAction date has arrived: 2018-03-02
NextAction: 2018-03-03
No Canary coverage yet.
Without canary coverage, we can't take this change in for M65. 
Indeed, let's not block in this change and simply merge it to 6.5 when there is sufficient coverage.
Project Member

Comment 22 by ClusterFuzz, Mar 3 2018

ClusterFuzz has detected this issue as fixed in range 540675:540696.

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

Fuzzer: inferno_twister
Job Type: linux_cfi_chrome
Platform Id: linux

Crash Type: Null-dereference READ
Crash Address: 0x000000000000
Crash State:
  v8::internal::LoadHandler::LoadFromPrototype
  v8::internal::LoadIC::ComputeHandler
  v8::internal::LoadIC::UpdateCaches
  
Sanitizer: cfi (CFI)

Regressed: https://clusterfuzz.com/revisions?job=linux_cfi_chrome&range=523197:523221
Fixed: https://clusterfuzz.com/revisions?job=linux_cfi_chrome&range=540675:540696

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

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 23 by ClusterFuzz, Mar 3 2018

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

If this is incorrect, please add ClusterFuzz-Wrong label and re-open the issue.
The NextAction date has arrived: 2018-03-03
The fix is in the Canary 3360.
Labels: -Merge-Review-65 -Merge-Review-66 Merge-Approved-66 Merge-Approved-65
Let's merge it to 65 and 66.
Project Member

Comment 27 by bugdroid1@chromium.org, Mar 5 2018

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

commit 51c6ee09c589c6505885a20250af7df04e0b145a
Author: ishell@chromium.org <ishell@chromium.org>
Date: Mon Mar 05 15:59:20 2018

Merged: [ic] Properly handle kApiGetter case with null prototype.

Revision: 16a3a4e94643a77ab00df63116191839e18ffed9

BUG= chromium:808845 
LOG=N
NOTRY=true
NOPRESUBMIT=true
NOTREECHECKS=true
R=cbruni@chromium.org

Change-Id: I5c410a427e1dda097bfa24ac26e7d94eea319891
Reviewed-on: https://chromium-review.googlesource.com/948910
Reviewed-by: Camillo Bruni <cbruni@chromium.org>
Cr-Commit-Position: refs/branch-heads/6.5@{#61}
Cr-Branched-From: 73c55f57fe8506011ff854b15026ca765b669700-refs/heads/6.5.254@{#1}
Cr-Branched-From: 594a1a0b6e551397cfdf50870f6230da34db2dc8-refs/heads/master@{#50664}
[modify] https://crrev.com/51c6ee09c589c6505885a20250af7df04e0b145a/src/ic/handler-configuration.cc
[modify] https://crrev.com/51c6ee09c589c6505885a20250af7df04e0b145a/test/cctest/test-accessors.cc

Labels: -Merge-Approved-65
Project Member

Comment 29 by bugdroid1@chromium.org, Mar 5 2018

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

commit f3996358266dabceb38f0f2aeb4f996ea7252349
Author: ishell@chromium.org <ishell@chromium.org>
Date: Mon Mar 05 16:07:16 2018

Merged: [ic] Properly handle kApiGetter case with null prototype.

Revision: 16a3a4e94643a77ab00df63116191839e18ffed9

BUG= chromium:808845 
LOG=N
NOTRY=true
NOPRESUBMIT=true
NOTREECHECKS=true
R=cbruni@chromium.org

Change-Id: I33c8da58c2592ad7159c5960cc66cdd59e491d40
Reviewed-on: https://chromium-review.googlesource.com/948911
Reviewed-by: Camillo Bruni <cbruni@chromium.org>
Cr-Commit-Position: refs/branch-heads/6.6@{#7}
Cr-Branched-From: d500271571b92cb18dcd7b15885b51e8f437d640-refs/heads/6.6.346@{#1}
Cr-Branched-From: 265ef0b635f8761df7c89eb4e8ec9c1a6ebee184-refs/heads/master@{#51624}
[modify] https://crrev.com/f3996358266dabceb38f0f2aeb4f996ea7252349/src/ic/handler-configuration.cc
[modify] https://crrev.com/f3996358266dabceb38f0f2aeb4f996ea7252349/test/cctest/test-accessors.cc

Labels: -Merge-Approved-66
Labels: NodeJS-Backport-Done

Sign in to add a comment