Null-dereference READ in v8::internal::MarkCompactCollector::RootMarkingVisitor::VisitRootPointers |
|||||||
Issue descriptionDetailed report: https://clusterfuzz.com/testcase?key=6576519795965952 Fuzzer: mbarbella_js_mutation Job Type: linux_msan_d8 Platform Id: linux Crash Type: Null-dereference READ Crash Address: 0x000000000000 Crash State: v8::internal::MarkCompactCollector::RootMarkingVisitor::VisitRootPointers v8::internal::WasmCompileLazyFrame::Iterate v8::internal::Isolate::Iterate Sanitizer: memory (MSAN) Regressed: https://clusterfuzz.com/revisions?job=linux_msan_d8&range=53697:53698 Reproducer Testcase: https://clusterfuzz.com/download?testcase_id=6576519795965952 Issue filed automatically. See https://github.com/google/clusterfuzz-tools for more information.
,
Jun 16 2018
Automatically assigning owner based on suspected regression changelist https://chromium.googlesource.com/v8/v8/+/b9b4b8798fba40237f53455ef37f6a94f41e16c6 ([wasm] Merge {WasmSharedModuleData} with {WasmModuleObject}). If this is incorrect, please let us know why and apply the Test-Predator-Wrong-CLs label. If you aren't the correct owner for this issue, please unassign yourself as soon as possible so it can be re-triaged.
,
Jun 18 2018
Just debugged a bit with mstarzinger, seems that frame constants on arm64 are off. My CL is not really the culprit, it just changes the GC timing such that the bug is observable. I will try to add explicit GC calls and create a minimized repo tomorrow.
,
Jun 19 2018
I recently changed something there, see:
[masm] Don't push CodeObject when entering INTERNAL frames
Reviewed-on: https://chromium-review.googlesource.com/1075047
and the follow-up fix:
[mips,mips64] Fix sp and fp offsets for INTERNAL frames
Reviewed-on: https://chromium-review.googlesource.com/1078109
,
Jun 19 2018
I had another look over the changes in #4, they still look correct to me and shouldn't affect WasmCompileLazyFrame constants.
,
Jun 19 2018
After adding an explicit GC in the WasmCompileLazy runtime function, it reproduces totally reliable (on arm64 simulator).
Patch:
=================================================================
diff --git a/src/runtime/runtime-wasm.cc b/src/runtime/runtime-wasm.cc
index 63ecaa53ef..d0955eb681 100644
--- a/src/runtime/runtime-wasm.cc
+++ b/src/runtime/runtime-wasm.cc
@@ -297,6 +297,9 @@ RUNTIME_FUNCTION(Runtime_WasmCompileLazy) {
ClearThreadInWasmScope wasm_flag(true);
+ isolate->heap()->CollectAllAvailableGarbage(
+ i::GarbageCollectionReason::kTesting);
+
#ifdef DEBUG
StackFrameIterator it(isolate, isolate->thread_local_top());
// On top: C entry stub.
=================================================================
Repro:
=================================================================
function asm() {
'use asm';
function f() { }
return f;
}
asm();
=================================================================
Bisecting now.
,
Jun 19 2018
Still bisecting. Just figured out that this is the same bug that lead to the revert of the jump table for wasm: https://ci.chromium.org/buildbot/client.v8.ports/V8%20Linux%20-%20arm64%20-%20sim%20-%20gc%20stress/11659
,
Jun 19 2018
Ok, bisects all the way back to b2abe2cf971a382e973a9e99124452a9bfe2005a ([wasm] Introduce specialized WasmCompileLazy frame type.). A fix is to replace (in WasmCompileLazyFrameConstants) TypedFrameConstants::kFixedFrameSizeFromFp by either RoundUp<16>(TypedFrameConstants::kFixedFrameSizeFromFp) or TypedFrameConstants::kFixedFrameSizeFromFp + kPointerSize I prefer the RoundUp version, will upload a fix.
,
Jun 19 2018
The following revision refers to this bug: https://chromium.googlesource.com/v8/v8.git/+/49946f02bbfbd294f53aaa2641184cbf049f9f3d commit 49946f02bbfbd294f53aaa2641184cbf049f9f3d Author: Clemens Hammacher <clemensh@chromium.org> Date: Tue Jun 19 08:41:58 2018 [arm64] Fix WasmCompileLazyFrameConstants {WasmCompileLazyFrameConstants::kFixedFrameSizeFromFp} did not incorporate padding. This CL fixes that. R=mstarzinger@chromium.org No-Try: true Bug: chromium:853468 Change-Id: I042e68623bdfd81c96180a39c29ecd70271ba1be Reviewed-on: https://chromium-review.googlesource.com/1105051 Commit-Queue: Clemens Hammacher <clemensh@chromium.org> Reviewed-by: Michael Starzinger <mstarzinger@chromium.org> Cr-Commit-Position: refs/heads/master@{#53817} [modify] https://crrev.com/49946f02bbfbd294f53aaa2641184cbf049f9f3d/src/arm64/frame-constants-arm64.h
,
Jun 19 2018
Fixed. No backmerge required, as b2abe2cf971a382e973a9e99124452a9bfe2005a landed after the 68 branch.
,
Jun 19 2018
Let's ensure that it's well tested in canary first, before merging to m68.
,
Jun 20 2018
ClusterFuzz has detected this issue as fixed in range 53816:53817. Detailed report: https://clusterfuzz.com/testcase?key=6576519795965952 Fuzzer: mbarbella_js_mutation Job Type: linux_msan_d8 Platform Id: linux Crash Type: Null-dereference READ Crash Address: 0x000000000000 Crash State: v8::internal::MarkCompactCollector::RootMarkingVisitor::VisitRootPointers v8::internal::WasmCompileLazyFrame::Iterate v8::internal::Isolate::Iterate Sanitizer: memory (MSAN) Regressed: https://clusterfuzz.com/revisions?job=linux_msan_d8&range=53697:53698 Fixed: https://clusterfuzz.com/revisions?job=linux_msan_d8&range=53816:53817 Reproducer Testcase: https://clusterfuzz.com/download?testcase_id=6576519795965952 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.
,
Jun 20 2018
Sorry, wrong label. I wanted to add the reject label directly, since this does not require backmerge.
,
Jun 20 2018
ClusterFuzz testcase 6576519795965952 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 |
|||||||
Comment 1 by ClusterFuzz
, Jun 16 2018Labels: Test-Predator-Auto-Components