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

Issue 796041 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Closed: Jan 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 1
Type: Bug



Sign in to add a comment

Null-dereference READ in v8::internal::Invoke

Project Member Reported by ClusterFuzz, Dec 19 2017

Issue description

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

Job Type: linux_asan_chrome_mp
Crash Type: Null-dereference READ
Crash Address: 0x000000000000
Crash State:
  v8::internal::Invoke
  v8::internal::Execution::Call
  v8::Script::Run
  
Sanitizer: address (ASAN)

Regressed: https://clusterfuzz.com/revisions?job=linux_asan_chrome_mp&range=514498:517698

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

Issue filed automatically.

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

Comment 1 by ClusterFuzz, Dec 19 2017

Cc: alph@chromium.org kouhei@chromium.org leszeks@chromium.org
Labels: Test-Predator-Auto-CC
Automatically adding ccs based on suspected regression changelists:

[v8] Log the source origin of scripts passed to V8 by leszeks@chromium.org - https://chromium.googlesource.com/chromium/src/+/53bcd586175cdade5b0f7693335419e8789d09b7

DevTools: Instrument Compile & Evaluate Module events for timeline. by alph@chromium.org - https://chromium.googlesource.com/chromium/src/+/5efc3d9ec2759c5efdea7e97369f0f7d881e63f4

javascript url: Add spec ref for ScriptFetchOptions to use by kouhei@chromium.org - https://chromium.googlesource.com/chromium/src/+/2f5801db1a731e5ec631ed70fdaf8ece125ed744

If this is incorrect, please apply the Test-Predator-Wrong-CLs label.
Project Member

Comment 2 by ClusterFuzz, Dec 19 2017

Components: Blink>Bindings Blink>JavaScript
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.
FWIW, my CL shouldn't have an effect on function invocation, so I don't think it's the culprit.
Reproduces in d8, bisecting...
Components: -Blink>Bindings
Owner: tebbi@chromium.org
Status: Assigned (was: Untriaged)
Hooray, bisects to ac0661b358bce7f9af6f23c3e640121f6ca20170 (Reland^5 "[turbofan] eagerly prune None types and deadness from the graph").
Tobi, can you take a look?
Project Member

Comment 6 by ClusterFuzz, Dec 21 2017

ClusterFuzz has detected this issue as fixed in range 525420:525422.

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

Job Type: linux_asan_chrome_mp
Crash Type: Null-dereference READ
Crash Address: 0x000000000000
Crash State:
  v8::internal::Invoke
  v8::internal::Execution::Call
  v8::Script::Run
  
Sanitizer: address (ASAN)

Regressed: https://clusterfuzz.com/revisions?job=linux_asan_chrome_mp&range=514498:517698
Fixed: https://clusterfuzz.com/revisions?job=linux_asan_chrome_mp&range=525420:525422

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

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.

Comment 7 by tebbi@chromium.org, Dec 21 2017

Yes, this is a bug in dead code elimination. And the root cause is certainly not fixed yet, even though ClusterFuzz is confused.
Project Member

Comment 8 by ClusterFuzz, Dec 21 2017

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

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

Comment 9 by tebbi@chromium.org, Dec 21 2017

Labels: -ClusterFuzz-Verified ClusterFuzz-Wrong
Status: Assigned (was: Verified)
Labels: M-64 M-65
Project Member

Comment 11 by bugdroid1@chromium.org, Jan 3 2018

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

commit 8de3a3bcf9d3e59afdcbf7309561c5f09e27ceae
Author: Tobias Tebbi <tebbi@chromium.org>
Date: Wed Jan 03 12:28:14 2018

[turbofan] add value input to DeadValue

DeadValue was a constant node of type None. This is unsound in the
presence of re-scheduling. This CL adds a value input to DeadValue,
which preserves the dependency on the original node of type None.

Bug:  chromium:796041 
Change-Id: I3ac459bf661fb78c56552e8201aa18a7dbc4d182
Reviewed-on: https://chromium-review.googlesource.com/847011
Commit-Queue: Tobias Tebbi <tebbi@chromium.org>
Reviewed-by: Jaroslav Sevcik <jarin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#50340}
[modify] https://crrev.com/8de3a3bcf9d3e59afdcbf7309561c5f09e27ceae/src/compiler/common-operator.cc
[modify] https://crrev.com/8de3a3bcf9d3e59afdcbf7309561c5f09e27ceae/src/compiler/common-operator.h
[modify] https://crrev.com/8de3a3bcf9d3e59afdcbf7309561c5f09e27ceae/src/compiler/dead-code-elimination.cc
[modify] https://crrev.com/8de3a3bcf9d3e59afdcbf7309561c5f09e27ceae/src/compiler/dead-code-elimination.h
[modify] https://crrev.com/8de3a3bcf9d3e59afdcbf7309561c5f09e27ceae/src/compiler/effect-control-linearizer.cc
[modify] https://crrev.com/8de3a3bcf9d3e59afdcbf7309561c5f09e27ceae/src/compiler/instruction-selector-impl.h
[modify] https://crrev.com/8de3a3bcf9d3e59afdcbf7309561c5f09e27ceae/src/compiler/instruction-selector.cc
[modify] https://crrev.com/8de3a3bcf9d3e59afdcbf7309561c5f09e27ceae/src/compiler/instruction-selector.h
[modify] https://crrev.com/8de3a3bcf9d3e59afdcbf7309561c5f09e27ceae/src/compiler/js-graph.cc
[modify] https://crrev.com/8de3a3bcf9d3e59afdcbf7309561c5f09e27ceae/src/compiler/js-graph.h
[modify] https://crrev.com/8de3a3bcf9d3e59afdcbf7309561c5f09e27ceae/src/compiler/representation-change.cc
[modify] https://crrev.com/8de3a3bcf9d3e59afdcbf7309561c5f09e27ceae/src/compiler/typer.cc
[modify] https://crrev.com/8de3a3bcf9d3e59afdcbf7309561c5f09e27ceae/src/compiler/verifier.cc

Labels: Merge-Request-64
I suspect that this bug is responsible for the majority of Invoke crashes on M64. The spike in Invoke crashes on 64.0.3278.0 on the dev channel coincides with the regressing CL of this bug (https://chromium-review.googlesource.com/c/v8/v8/+/772150). Once we have canary coverage, we should consider merging the fix to M64 before it gets stable.
Please add affected OSs.
Project Member

Comment 15 by bugdroid1@chromium.org, Jan 4 2018

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

commit dbc377ed8eef762381587fce01dcbe43e98305d0
Author: Tobias Tebbi <tebbi@chromium.org>
Date: Thu Jan 04 00:36:09 2018

[turbofan] add regression test for chromium:796041

The missing regression test for
https://chromium-review.googlesource.com/c/v8/v8/+/847011

Bug:  chromium:796041 
Change-Id: I3d791d6485221d2fa68def2c7be96c48822aa651
Reviewed-on: https://chromium-review.googlesource.com/848995
Reviewed-by: Jaroslav Sevcik <jarin@chromium.org>
Commit-Queue: Tobias Tebbi <tebbi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#50349}
[add] https://crrev.com/dbc377ed8eef762381587fce01dcbe43e98305d0/test/mjsunit/compiler/regress-796041.js

Project Member

Comment 16 by bugdroid1@chromium.org, Jan 4 2018

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

commit 6328c56570baea725a342ce460aa16ddf04df247
Author: Tobias Tebbi <tebbi@chromium.org>
Date: Thu Jan 04 13:15:06 2018

Reland "[turbofan] add value input to DeadValue"

DeadValue was a constant node of type None. This is unsound in the
presence of re-scheduling. This CL adds a value input to DeadValue,
which preserves the dependency on the original node of type None.

This reland addresses the bug that the EffectControlLinearizer could destroy dependencies of DeadValue by attaching DeadValue nodes to the effect chain in the EffectControlLinearizer.

Bug:  chromium:796041   chromium:798938 
Change-Id: If47b54a7986d257eb63b437f855769b503679ff5
Reviewed-on: https://chromium-review.googlesource.com/850392
Reviewed-by: Jaroslav Sevcik <jarin@chromium.org>
Commit-Queue: Tobias Tebbi <tebbi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#50360}
[modify] https://crrev.com/6328c56570baea725a342ce460aa16ddf04df247/src/compiler/common-operator.cc
[modify] https://crrev.com/6328c56570baea725a342ce460aa16ddf04df247/src/compiler/common-operator.h
[modify] https://crrev.com/6328c56570baea725a342ce460aa16ddf04df247/src/compiler/dead-code-elimination.cc
[modify] https://crrev.com/6328c56570baea725a342ce460aa16ddf04df247/src/compiler/dead-code-elimination.h
[modify] https://crrev.com/6328c56570baea725a342ce460aa16ddf04df247/src/compiler/effect-control-linearizer.cc
[modify] https://crrev.com/6328c56570baea725a342ce460aa16ddf04df247/src/compiler/effect-control-linearizer.h
[modify] https://crrev.com/6328c56570baea725a342ce460aa16ddf04df247/src/compiler/graph-assembler.cc
[modify] https://crrev.com/6328c56570baea725a342ce460aa16ddf04df247/src/compiler/graph-assembler.h
[modify] https://crrev.com/6328c56570baea725a342ce460aa16ddf04df247/src/compiler/instruction-selector-impl.h
[modify] https://crrev.com/6328c56570baea725a342ce460aa16ddf04df247/src/compiler/instruction-selector.cc
[modify] https://crrev.com/6328c56570baea725a342ce460aa16ddf04df247/src/compiler/instruction-selector.h
[modify] https://crrev.com/6328c56570baea725a342ce460aa16ddf04df247/src/compiler/js-graph.cc
[modify] https://crrev.com/6328c56570baea725a342ce460aa16ddf04df247/src/compiler/js-graph.h
[modify] https://crrev.com/6328c56570baea725a342ce460aa16ddf04df247/src/compiler/representation-change.cc
[modify] https://crrev.com/6328c56570baea725a342ce460aa16ddf04df247/src/compiler/typer.cc
[modify] https://crrev.com/6328c56570baea725a342ce460aa16ddf04df247/src/compiler/verifier.cc
[add] https://crrev.com/6328c56570baea725a342ce460aa16ddf04df247/test/mjsunit/compiler/regress-796041.js

Labels: OS-Windows
It seems to affect at least Windows, but I don't see anything about this bug that should be platform or architecture specific.
Cc: abdulsyed@chromium.org
+Abdul for M64 merge review.
Project Member

Comment 19 by sheriffbot@chromium.org, Jan 5 2018

Labels: -Merge-Request-64 Hotlist-Merge-Review Merge-Review-64
This bug requires manual review: M64 has already been promoted to the beta branch, so this requires manual review
Please contact the milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), kbleicher@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
I am not sure how many Invoke crashes are caused by this bug. I checked a few minidumps, and they didn't look like it, but I'm not sure.
Cc: hablich@chromium.org
This seems like a huge change. Can you please comment on how safe it is overall, and why this is needed for M64? My recommendation is to wait until M65.
I checked Chromecrash in more detail. It seems that the vast majority of Invoke crashes are not related to this issue. (I checked more than 10 minidumps, and none of them seemed to be this issue.) So as long as the crash rate doesn't increase, I think we don't have to merge the fix. I don't think it is a security bug.
Labels: -Merge-Review-64 Merge-Rejected-64
let's punt this to M65, since the fix is quite complex and per #22, doesn't seem to be that prevalent. 

@mano - can you please closely monitor this crash to see if it spikes either in next Beta and as we ramp-up to stable?
Sounds good Abdul!
Project Member

Comment 25 by sheriffbot@chromium.org, Jan 18 2018

Labels: FoundIn-M-64 Fracas
Users experienced this crash on the following builds:

Win Beta 64.0.3282.85 -  1.03 CPM, 1018 reports, 802 clients (signature v8::internal::`anonymous namespace'::Invoke)

If this update was incorrect, please add "Fracas-Wrong" label to prevent future updates.

- Go/Fracas
Project Member

Comment 26 by sheriffbot@chromium.org, Jan 18 2018

Labels: FoundIn-M-65
Users experienced this crash on the following builds:

Win Canary 65.0.3323.0 -  0.83 CPM, 17 reports, 13 clients (signature v8::internal::`anonymous namespace'::Invoke)

If this update was incorrect, please add "Fracas-Wrong" label to prevent future updates.

- Go/Fracas

Comment 27 by tebbi@chromium.org, Jan 23 2018

Status: Fixed (was: Assigned)
Fixed by commit from comment 16. There are still crashes of the same signature, but they seem unrelated.

Sign in to add a comment