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

Issue 893944 link

Starred by 1 user

Issue metadata

Status: Verified
Owner:
Closed: Oct 10
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 1
Type: Bug



Sign in to add a comment

Null-dereference READ in blink::V8GCController::GcEpilogue

Project Member Reported by ClusterFuzz, Oct 10

Issue description

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

Fuzzer: inferno_layout_test_unmodified
Job Type: linux_ubsan_vptr_chrome
Platform Id: linux

Crash Type: Null-dereference READ
Crash Address: 0x000000000000
Crash State:
  blink::V8GCController::GcEpilogue
  v8::internal::Heap::CallGCEpilogueCallbacks
  v8::internal::GlobalHandles::DispatchPendingPhantomCallbacks
  
Sanitizer: undefined (UBSAN)

Regressed: https://clusterfuzz.com/revisions?job=linux_ubsan_vptr_chrome&range=597524:597525

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

Issue filed automatically.

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

Comment 1 by ClusterFuzz, Oct 10

Components: Blink>Bindings Blink>JavaScript>GC
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.
Project Member

Comment 2 by ClusterFuzz, Oct 10

Labels: Test-Predator-Auto-Owner
Owner: mlippautz@chromium.org
Status: Assigned (was: Untriaged)
Automatically assigning owner based on suspected regression changelist https://chromium.googlesource.com/chromium/src/+/2018a9efe755a657ed735dcea9fe43c94b214aa4 (bindings: Fix second pass callbacks fired during teardown sequence).

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.
Project Member

Comment 3 by bugdroid1@chromium.org, Oct 10

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

commit dfa56840fe923766edb95045a764f4b56921c4b6
Author: Michael Lippautz <mlippautz@chromium.org>
Date: Wed Oct 10 18:38:14 2018

[heap] Use non-nestable tasks for finalizing garbage collection

Pass on information about the embedder state using the fact that tasks
are run from top level

Bug:  chromium:893944 
Change-Id: I01441778770c5acc784540e496eec5c3fdb87796
Reviewed-on: https://chromium-review.googlesource.com/c/1273048
Commit-Queue: Michael Lippautz <mlippautz@chromium.org>
Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#56540}
[modify] https://crrev.com/dfa56840fe923766edb95045a764f4b56921c4b6/src/heap/embedder-tracing.h
[modify] https://crrev.com/dfa56840fe923766edb95045a764f4b56921c4b6/src/heap/heap.cc
[modify] https://crrev.com/dfa56840fe923766edb95045a764f4b56921c4b6/src/heap/incremental-marking-job.cc
[modify] https://crrev.com/dfa56840fe923766edb95045a764f4b56921c4b6/src/heap/incremental-marking-job.h
[modify] https://crrev.com/dfa56840fe923766edb95045a764f4b56921c4b6/test/unittests/heap/embedder-tracing-unittest.cc

Whoops, the commit in #3 actually does not belong to this issue. The right one is coming up shortly.
Project Member

Comment 5 by bugdroid1@chromium.org, Oct 10

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

commit d8d2533d5ba395f80606b58118b6255545bbad00
Author: Michael Lippautz <mlippautz@chromium.org>
Date: Wed Oct 10 18:50:54 2018

Revert "[heap] Run phantom handle callbacks on tear down"

This reverts commit fa65063a98aca0b64bb774a3f7ed7385d9dc9509.

Reason for revert:
This changes API contract with Blink as some state is destroyed before
actually tearing down the Isolate. Flushing the second round tasks
then tries to access various state that is already gone on the Blink
side. See bugs.

Bug:  chromium:893944 , chromium:893549, chromium:890631

Original change's description:
> [heap] Run phantom handle callbacks on tear down
>
> Pending phantom handle callbacks are not reliably executed if the heap
> shuts down. This can cause to memory leaks or other unwanted behaviour,
> like in wasm where the NativeModules (held in Managed objects
> implemented via phantom handles) unregister from the WasmEngine in the
> second-pass callback. This must be executed before tearing down the
> WasmEngine.
>
> This CL fixes this by running pending callback synchronously on heap
> tear down.
>
> R=ulan@chromium.org, mlippautz@chromium.org
>
> Bug:  v8:8208 
> Change-Id: I27b630c4d8f1fb12309040ea2179b64eed38710a
> Reviewed-on: https://chromium-review.googlesource.com/1249101
> Commit-Queue: Clemens Hammacher <clemensh@chromium.org>
> Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#56286}

TBR=ulan@chromium.org,mlippautz@chromium.org,clemensh@chromium.org

Bug:  v8:8208 
Change-Id: I4b403fd84473edb8895c3725ff3348574c54247b
Reviewed-on: https://chromium-review.googlesource.com/c/1274085
Commit-Queue: Michael Lippautz <mlippautz@chromium.org>
Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#56542}
[modify] https://crrev.com/d8d2533d5ba395f80606b58118b6255545bbad00/src/global-handles.cc
[modify] https://crrev.com/d8d2533d5ba395f80606b58118b6255545bbad00/src/heap/heap.cc
[modify] https://crrev.com/d8d2533d5ba395f80606b58118b6255545bbad00/src/isolate.cc

Status: Fixed (was: Assigned)
Project Member

Comment 7 by ClusterFuzz, Oct 11

ClusterFuzz has detected this issue as fixed in range 598126:598690.

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

Fuzzer: inferno_layout_test_unmodified
Job Type: linux_ubsan_vptr_chrome
Platform Id: linux

Crash Type: Null-dereference READ
Crash Address: 0x000000000000
Crash State:
  blink::V8GCController::GcEpilogue
  v8::internal::Heap::CallGCEpilogueCallbacks
  v8::internal::GlobalHandles::DispatchPendingPhantomCallbacks
  
Sanitizer: undefined (UBSAN)

Regressed: https://clusterfuzz.com/revisions?job=linux_ubsan_vptr_chrome&range=597524:597525
Fixed: https://clusterfuzz.com/revisions?job=linux_ubsan_vptr_chrome&range=598126:598690

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

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 8 by ClusterFuzz, Oct 11

Labels: ClusterFuzz-Verified
Status: Verified (was: Fixed)
ClusterFuzz testcase 5613903630041088 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