New issue
Advanced search Search tips

Issue 865279 link

Starred by 2 users

Issue metadata

Status: Verified
Owner:
Closed: Jul 30
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 2
Type: Bug



Sign in to add a comment

CHECK failure: !result || DOMDataStore::GetWrapper(result, info.GetIsolate()).IsEmpty() in v8_r

Project Member Reported by ClusterFuzz, Jul 19

Issue description

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

Fuzzer: puzzor
Job Type: linux_debug_chrome
Platform Id: linux

Crash Type: CHECK failure
Crash Address: 
Crash State:
  !result || DOMDataStore::GetWrapper(result, info.GetIsolate()).IsEmpty() in v8_r
  blink::RangeV8Internal::extractContentsMethod
  blink::V8Range::extractContentsMethodCallback
  
Sanitizer: address (ASAN)

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

Issue filed automatically.

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

Comment 1 by ClusterFuzz, Jul 19

Components: 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.
Cc: marja@chromium.org
Components: -Blink>JavaScript Blink>Bindings
Owner: mlippautz@chromium.org
Status: Assigned (was: Untriaged)
Seems to crash in the blink bindings.
Michael and Marja, you are listed as owners. Can you triage this?
Cc: haraken@chromium.org yukishiino@chromium.org
haraken,yukishiino: Any known bugs?
It seems like that we're checking whether [NewObject] is really creating a new object or not.  Looking at the implementation, it seems really creating a new object, so it should be fine.

Probably, what's happening is like below:

    1. Range::extractContents is invoked.
    2. In Range::extractContents, a new DocumentFragment is created (no wrapper object at this moment).
    3. Range::extractContents runs JS code internally (maybe), where a wrapper object for the DocumentFragment is created.
    4. At the end of generated callback, we're expecting that no wrapper object exists because the returned object must be a new DocumentFragment.

3. and 4. would be conflicting each other.  The behavior itself should be okay.

(Note that everything above is my imagination off the top of my head.  No evidence I have.)

If we just need to kill the check at 4, [DoNotTestNewObject] does it.  But we first need an evidence, I think.

Cc: -yukishiino@chromium.org mlippautz@chromium.org
Owner: yukishiino@chromium.org
Re-assigning. I hope you can better triage this.

I debug this now using the reproduce command on the CF issue. 

You are right and this is an issue of nested invocations of extractContentsMethod. 

Example stack trace using prints:
extractContentsMethod
  before extractContents
    Range::extractContents
    DocumentFragment::Create 0x2de4dd708a98
    1 DocumentFragment::ContainsWrapper 0x2de4dd708a98 0
    4 DocumentFragment::ContainsWrapper 0x2de4dd708a98 0
    extractContentsMethod
      before extractContents
        Range::extractContents
        DocumentFragment::Create 0x2de4dd708f00
        1 DocumentFragment::ContainsWrapper 0x2de4dd708f00 0
        2 DocumentFragment::ContainsWrapper 0x2de4dd708f00 0
      after extractContents
      bindings1: DocumentFragment: 0x2de4dd708f00 0
      bindings2: DocumentFragment: 0x2de4dd708f00 0
  after extractContents
  bindings1: DocumentFragment: 0x2de4dd708a98 1
  bindings2: DocumentFragment: 0x2de4dd708a98 1
[1:1:0724/140142.420385:FATAL:v8_range.cc(399)] Check failed: !result || DOMDataStore::GetWrapper(result, info.GetIsolate()).IsEmpty(). 

What we see here is extractContentsMethod being called again before returning to the bindings layer we DCHECK the invariant (new object, no wrapper). I guess this is due to EventQueue scope here [1].

I added stack traces to ScriptWrappable::Wrap and the relevant one (for the "new" DocumentFragment 0x2de4dd708a98) looks like

#0 0x7f96a664641d base::debug::StackTrace::StackTrace()
#1 0x7f968b8896d1 blink::ScriptWrappable::Wrap()
#2 0x7f968f724027 blink::V8SetReturnValueForMainWorld<>()
#3 0x7f968f723e1d blink::V8SetReturnValueForMainWorld<>()
#4 0x7f968f75f497 blink::NodeV8Internal::parentNodeAttributeGetterForMainWorld()
#5 0x7f968f75f41a blink::V8Node::parentNodeAttributeGetterCallbackForMainWorld()
#6 0x7f96909431f4 v8::internal::FunctionCallbackArguments::Call()
#7 0x7f9690941620 v8::internal::(anonymous namespace)::HandleApiCallHelper<>()
#8 0x7f96909402e5 v8::internal::Builtins::InvokeApiFunction()
#9 0x7f9690f1ccf2 v8::internal::Object::GetPropertyWithAccessor()
#10 0x7f9690e07417 v8::internal::LoadIC::Load()
#11 0x7f9690e16177 v8::internal::__RT_impl_Runtime_LoadIC_Miss()


Light-weight args.gn
  goma_dir = "/..."
  use_goma = true
  is_component_build = true
  is_debug = true
  v8_optimized_debug = true
  strip_absolute_paths_from_debug_symbols = false
  symbol_level = 2

Command:
out/clusterfuzz_6300365629423616/chrome --ignore-gpu-blacklist --disable-field-trial-config --disable-hang-monitor --dns-prefetch-disable --disable-default-apps --disable-component-update --safebrowsing-disable-auto-update --metrics-recording-only --disable-gpu-watchdog --disable-metrics --disable-popup-blocking --disable-prompt-on-repost --enable-experimental-extension-apis --enable-extension-apps --js-flags="--expose-gc --verify-heap" --new-window --no-default-browser-check --no-first-run --no-process-singleton-dialog --enable-shadow-dom --enable-media-stream --use-gl=swiftshader --use-fake-device-for-media-stream --use-fake-ui-for-media-stream  --user-data-dir=/tmp/clusterfuzz-user-data-dir /usr/local/google/home/mlippautz/.clusterfuzz/cache/testcases/6300365629423616_testcase/fuzz-17.html

[1] https://cs.chromium.org/chromium/src/third_party/blink/renderer/core/dom/range.cc?sq=package:chromium&g=0&l=825
Cc: peria@chromium.org
Labels: -Pri-1 Pri-2
Thanks for the investigation.  I will handle this issue.
Project Member

Comment 7 by ClusterFuzz, Jul 30

ClusterFuzz has detected this issue as fixed in range 578978:578979.

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

Fuzzer: puzzor
Job Type: linux_debug_chrome
Platform Id: linux

Crash Type: CHECK failure
Crash Address: 
Crash State:
  !result || DOMDataStore::GetWrapper(result, info.GetIsolate()).IsEmpty() in v8_r
  blink::RangeV8Internal::extractContentsMethod
  blink::V8Range::extractContentsMethodCallback
  
Sanitizer: address (ASAN)

Fixed: https://clusterfuzz.com/revisions?job=linux_debug_chrome&range=578978:578979

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

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, Jul 30

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

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

Comment 9 by bugdroid1@chromium.org, Jul 30

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

commit a61a8eaac28349ba997b32541a71b822959b95fa
Author: Yuki Shiino <yukishiino@chromium.org>
Date: Mon Jul 30 13:18:25 2018

v8binding: Removes the test for [NewObject] at Range.extractContents.

As blink::Range::extractContents uses EventQueueScope that runs
arbitrary author code internally, we cannot apply a auto-generated
test for [NewObject], so this patch stops generating it by specifying
[DoNotTestNewObject].

Ideally, it's better to have an alternative test, but we need a
v8::Isolate to do it, and using [CallWith=ScriptState] looks a bit
overkilling.  So, this patch doesn't add an alternative so far.

Bug:  865279 
Change-Id: Ief434a2c7a4df9c75f5c0d3f4cdc198fa880c702
Reviewed-on: https://chromium-review.googlesource.com/1154746
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Commit-Queue: Yuki Shiino <yukishiino@chromium.org>
Cr-Commit-Position: refs/heads/master@{#579025}
[modify] https://crrev.com/a61a8eaac28349ba997b32541a71b822959b95fa/third_party/blink/renderer/core/dom/range.cc
[modify] https://crrev.com/a61a8eaac28349ba997b32541a71b822959b95fa/third_party/blink/renderer/core/dom/range.idl

Sign in to add a comment