Null-dereference READ in v8::internal::Invoke |
|||||||||||||||
Issue descriptionDetailed 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.
,
Dec 19 2017
Automatically applying components based on crash stacktrace and information from OWNERS files. If this is incorrect, please apply the Test-Predator-Wrong-Components label.
,
Dec 19 2017
FWIW, my CL shouldn't have an effect on function invocation, so I don't think it's the culprit.
,
Dec 19 2017
Reproduces in d8, bisecting...
,
Dec 19 2017
Hooray, bisects to ac0661b358bce7f9af6f23c3e640121f6ca20170 (Reland^5 "[turbofan] eagerly prune None types and deadness from the graph"). Tobi, can you take a look?
,
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.
,
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.
,
Dec 21 2017
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.
,
Dec 21 2017
,
Jan 3 2018
,
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
,
Jan 3 2018
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.
,
Jan 3 2018
Please add affected OSs.
,
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
,
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
,
Jan 4 2018
It seems to affect at least Windows, but I don't see anything about this bug that should be platform or architecture specific.
,
Jan 4 2018
+Abdul for M64 merge review.
,
Jan 5 2018
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
,
Jan 5 2018
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.
,
Jan 5 2018
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.
,
Jan 8 2018
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.
,
Jan 9 2018
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?
,
Jan 9 2018
Sounds good Abdul!
,
Jan 18 2018
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
,
Jan 18 2018
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
,
Jan 23 2018
Fixed by commit from comment 16. There are still crashes of the same signature, but they seem unrelated. |
|||||||||||||||
►
Sign in to add a comment |
|||||||||||||||
Comment 1 by ClusterFuzz
, Dec 19 2017Labels: Test-Predator-Auto-CC