New issue
Advanced search Search tips

Issue 695730 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Feb 2017
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 1
Type: Bug

Blocking:
issue 696775



Sign in to add a comment

m_element in PendingScript.cpp

Project Member Reported by ClusterFuzz, Feb 24 2017

Issue description

Components: Blink
Labels: Test-Predator-Wrong M-58
Owner: hirosh...@chromium.org
Status: Assigned (was: Untriaged)
Through code search on file PendingScript.cpp, suspected CL is
https://chromium.googlesource.com/chromium/src/+/85a1f107f95bc55f75992f52d61343cd51f1aa53%5E%21/third_party/WebKit/Source/core/dom/PendingScript.cpp
hiroshige@, could you please take a look?.
Thank you


Components: -Blink Blink>Loader
Status: Started (was: Assigned)
Reproduced locally, nearly 100% on Linux Debug ASAN build.

PendingScript is disposed at:
#3 0x7f1a0304de20 blink::PendingScript::dispose()
#4 0x7f1a040de2c7 blink::HTMLParserScriptRunner::pendingScriptFinished()
#5 0x7f1a0304f98a blink::PendingScript::notifyFinished()
#6 0x7f1a0f26648d blink::Resource::checkNotify()
#7 0x7f1a0f268489 blink::Resource::error()
#8 0x7f1a0f2aa609 blink::ResourceFetcher::handleLoaderError()
#9 0x7f1a0f2f0f1d blink::ResourceLoader::handleError()
#10 0x7f1a0f2eee33 blink::ResourceLoader::cancel()
#11 0x7f1a0f2aaac6 blink::ResourceFetcher::stopFetching()
#12 0x7f1a0537ee17 blink::DocumentLoader::detachFromFrame()
#13 0x7f1a0cb14dc2 blink::WebDataSourceImpl::detachFromFrame()
#14 0x7f1a053e9754 blink::FrameLoader::detachDocumentLoader()
#15 0x7f1a053f0114 blink::FrameLoader::detach()
#16 0x7f1a039c2ad6 blink::LocalFrame::detach()
#17 0x7f1a03c7f4f8 blink::HTMLFrameOwnerElement::disconnectContentFrame()
#18 0x7f1a02b724be blink::ChildFrameDisconnector::disconnectCollectedFrameOwners()
#19 0x7f1a02b7170c blink::ChildFrameDisconnector::disconnect()
#20 0x7f1a02bb76a7 blink::ContainerNode::willRemoveChild()
#21 0x7f1a02bb6f8e blink::ContainerNode::removeChild()
#22 0x7f1a02fc5710 blink::Node::remove()
#23 0x7f1a06459e4d blink::ChildNode::remove()
#24 0x7f1a0665b4e8 blink::ElementV8Internal::removeMethod()
#25 0x7f1a0665b170 blink::V8Element::removeMethodCallback()
#26 0x7f1a10f3527d v8::internal::FunctionCallbackArguments::Call()
#27 0x7f1a1118c785 v8::internal::(anonymous namespace)::HandleApiCallHelper<>()
#28 0x7f1a1118874c v8::internal::Builtin_Impl_HandleApiCall()
#29 0x7f1a11187992 v8::internal::Builtin_HandleApiCall()

And just after that, PendingScript::element() is referenced at:
#4 0x7f1a0304eeb0 blink::PendingScript::element()
#5 0x7f1a040e02d3 blink::(anonymous namespace)::traceParserBlockingScript()
#6 0x7f1a040decaf blink::HTMLParserScriptRunner::processScriptElement()
#7 0x7f1a0405f6b6 blink::HTMLDocumentParser::runScriptsForPausedTreeBuilder()
#8 0x7f1a040698c5 blink::HTMLDocumentParser::processTokenizedChunkFromBackgroundParser()
#9 0x7f1a0405e6a0 blink::HTMLDocumentParser::pumpPendingSpeculations()
#10 0x7f1a0407519b blink::HTMLDocumentParser::resumeParsingAfterPause()
#11 0x7f1a04075c4b blink::HTMLDocumentParser::notifyScriptLoaded()
#12 0x7f1a040de34e blink::HTMLParserScriptRunner::pendingScriptFinished()
#13 0x7f1a0304f98a blink::PendingScript::notifyFinished()
#14 0x7f1a0f26648d blink::Resource::checkNotify()
#15 0x7f1a0f268489 blink::Resource::error()
#16 0x7f1a0f2aa609 blink::ResourceFetcher::handleLoaderError()
#17 0x7f1a0f2f0f1d blink::ResourceLoader::handleError()
#18 0x7f1a0f2eee33 blink::ResourceLoader::cancel()
#19 0x7f1a0f2aaac6 blink::ResourceFetcher::stopFetching()
#20 0x7f1a053e37c4 blink::FrameLoader::stopAllLoaders()
#21 0x7f1a039c2a3e blink::LocalFrame::detach()
#22 0x7f1a03c7f4f8 blink::HTMLFrameOwnerElement::disconnectContentFrame()
#23 0x7f1a02b724be blink::ChildFrameDisconnector::disconnectCollectedFrameOwners()
#24 0x7f1a02b7170c blink::ChildFrameDisconnector::disconnect()
#25 0x7f1a02bb76a7 blink::ContainerNode::willRemoveChild()
#26 0x7f1a02bb6f8e blink::ContainerNode::removeChild()
#27 0x7f1a02bb2de2 blink::collectChildrenAndRemoveFromOldParent()
#28 0x7f1a02bb2911 blink::ContainerNode::collectChildrenAndRemoveFromOldParentWithCheck()
#29 0x7f1a02bb3726 blink::ContainerNode::appendChild()
#30 0x7f1a02fc4ef9 blink::Node::appendChild()
#31 0x7f1a066c59ff blink::NodeV8Internal::appendChildMethodForMainWorld()
#32 0x7f1a066c4f30 blink::V8Node::appendChildMethodCallbackForMainWorld()
#33 0x7f1a10f3527d v8::internal::FunctionCallbackArguments::Call()
#34 0x7f1a1118c785 v8::internal::(anonymous namespace)::HandleApiCallHelper<>()
#35 0x7f1a1118874c v8::internal::Builtin_Impl_HandleApiCall()
#36 0x7f1a11187992 v8::internal::Builtin_HandleApiCall()

Very strange.
After disposing PendingScript, we return from
HTMLParserScriptRunner::pendingScriptFinished()
to PendingScript::notifyFinished() as expected (and I confirmed it by printf
debugging), but then
HTMLDocumentParser::notifyScriptLoaded() is called from
HTMLParserScriptRunner::pendingScriptFinished().

I inserted printf()s into HTMLParserScriptRunner::pendingScriptFinished(),
including just before and after notifyScriptLoaded() call,
but none of them are executed when HTMLDocumentParser::notifyScriptLoaded()
is executed.

Suspecting a compiler bug.
Didn't reproduced on a Linux Debug non-ASAN build (tried 4 times, no crash at this stack trace).
ASAN-only issue?

Testing on r452655, and
$ ./third_party/llvm-build/Release+Asserts/bin/clang --version
clang version 4.0.0 (trunk 289944)

Er, I understood the stack trace. It was not a compiler bug.

1. JavaScript's appendChild() is called, which causes
   LocalFrame::detach(), which eventually calls
   HTMLDocumentParser::notifyScriptLoaded().

2. Another JavaScript is called inside notifyScriptLoaded(), probably from
   HTMLParserScriptRunner::executePendingScriptAndDispatchEvent().

#4 0x7f4f583af936 blink::HTMLParserScriptRunner::executePendingScriptAndDispatchEvent()
#5 0x7f4f583b67a8 blink::HTMLParserScriptRunner::executeParsingBlockingScripts()
#6 0x7f4f583b73b4 blink::HTMLParserScriptRunner::executeScriptsWaitingForLoad()
#7 0x7f4f5834a601 blink::HTMLDocumentParser::notifyScriptLoaded()
#8 0x7f4f583b2e87 blink::HTMLParserScriptRunner::pendingScriptFinished()
#9 0x7f4f57323aea blink::PendingScript::notifyFinished()
#10 0x7f4f6353b786 blink::Resource::checkNotify()
#11 0x7f4f6353d899 blink::Resource::error()
#12 0x7f4f6357fa59 blink::ResourceFetcher::handleLoaderError()
#13 0x7f4f635c636d blink::ResourceLoader::handleError()
#14 0x7f4f635c4283 blink::ResourceLoader::cancel()
#15 0x7f4f6357ff16 blink::ResourceFetcher::stopFetching()
#16 0x7f4f596b8344 blink::FrameLoader::stopAllLoaders()
#17 0x7f4f57c96c9f blink::LocalFrame::detach()
#18 0x7f4f57f53ec8 blink::HTMLFrameOwnerElement::disconnectContentFrame()
#19 0x7f4f56e4661e blink::ChildFrameDisconnector::disconnectCollectedFrameOwners()
#20 0x7f4f56e4586c blink::ChildFrameDisconnector::disconnect()
#21 0x7f4f56e8b807 blink::ContainerNode::willRemoveChild()
#22 0x7f4f56e8b0ee blink::ContainerNode::removeChild()
#23 0x7f4f56e86f42 blink::collectChildrenAndRemoveFromOldParent()
#24 0x7f4f56e86a71 blink::ContainerNode::collectChildrenAndRemoveFromOldParentWithCheck()
#25 0x7f4f56e87886 blink::ContainerNode::appendChild()
#26 0x7f4f57299059 blink::Node::appendChild()
#27 0x7f4f5a99a57f blink::NodeV8Internal::appendChildMethodForMainWorld()
#28 0x7f4f5a999ab0 blink::V8Node::appendChildMethodCallbackForMainWorld()
#29 0x7f4f6520b27d v8::internal::FunctionCallbackArguments::Call()
#30 0x7f4f65462785 v8::internal::(anonymous namespace)::HandleApiCallHelper<>()
#31 0x7f4f6545e74c v8::internal::Builtin_Impl_HandleApiCall()
#32 0x7f4f6545d992 v8::internal::Builtin_HandleApiCall()
#33 0x7f4f12984204 <unknown>

3. From inside of 2., JavaScript's remove() is called, which causes
   LocalFrame::detach() again (reentered).
   Inside the reentered LocalFrame::detach(),
   PendingScript::dispose() is called:

#3 0x7f4f57321f80 blink::PendingScript::dispose()
#4 0x7f4f583b2d28 blink::HTMLParserScriptRunner::pendingScriptFinished()
#5 0x7f4f57323aea blink::PendingScript::notifyFinished()
#6 0x7f4f6353b786 blink::Resource::checkNotify()
#7 0x7f4f6353d899 blink::Resource::error()
#8 0x7f4f6357fa59 blink::ResourceFetcher::handleLoaderError()
#9 0x7f4f635c636d blink::ResourceLoader::handleError()
#10 0x7f4f635c4283 blink::ResourceLoader::cancel()
#11 0x7f4f6357ff16 blink::ResourceFetcher::stopFetching()
#12 0x7f4f59653997 blink::DocumentLoader::detachFromFrame()
#13 0x7f4f60de9dc2 blink::WebDataSourceImpl::detachFromFrame()
#14 0x7f4f596be2d4 blink::FrameLoader::detachDocumentLoader()
#15 0x7f4f596c4c94 blink::FrameLoader::detach()
#16 0x7f4f57c96f5c blink::LocalFrame::detach()
#17 0x7f4f57f53ec8 blink::HTMLFrameOwnerElement::disconnectContentFrame()
#18 0x7f4f56e4661e blink::ChildFrameDisconnector::disconnectCollectedFrameOwners()
#19 0x7f4f56e4586c blink::ChildFrameDisconnector::disconnect()
#20 0x7f4f56e8b807 blink::ContainerNode::willRemoveChild()
#21 0x7f4f56e8b0ee blink::ContainerNode::removeChild()
#22 0x7f4f57299870 blink::Node::remove()
#23 0x7f4f5a72e9cd blink::ChildNode::remove()
#24 0x7f4f5a930068 blink::ElementV8Internal::removeMethod()
#25 0x7f4f5a92fcf0 blink::V8Element::removeMethodCallback()
#26 0x7f4f6520b27d v8::internal::FunctionCallbackArguments::Call()
#27 0x7f4f65462785 v8::internal::(anonymous namespace)::HandleApiCallHelper<>()
#28 0x7f4f6545e74c v8::internal::Builtin_Impl_HandleApiCall()
#29 0x7f4f6545d992 v8::internal::Builtin_HandleApiCall()
#30 0x7f4f12984204 <unknown>
...
(Continues to the stack trace in Step 2)

4. Returned from the reentered LocalFrame::detach(), then remove(),
   Going back to the original HTMLDocumentParser::notifyScriptLoaded().
   Then HTMLDocumentParser::resumeParsingAfterPause() is called, which
   eventually calls PendingScript::element() for the PendingElement
   already disposed in Step 3, causing the check failure:

[1:1:0224/124459.935840:6132602423904:FATAL:PendingScript.cpp(124)] Check failed: m_element. 
#0 0x0000004dbfc1 __interceptor_backtrace
#1 0x7f4f76d7ef0a base::debug::StackTrace::StackTrace()
#2 0x7f4f76d7a35c base::debug::StackTrace::StackTrace()
#3 0x7f4f76edf409 logging::LogMessage::~LogMessage()
#4 0x7f4f57323010 blink::PendingScript::element()
#5 0x7f4f583b4e53 blink::(anonymous namespace)::traceParserBlockingScript()
#6 0x7f4f583b382f blink::HTMLParserScriptRunner::processScriptElement()
#7 0x7f4f58334086 blink::HTMLDocumentParser::runScriptsForPausedTreeBuilder()
#8 0x7f4f5833e295 blink::HTMLDocumentParser::processTokenizedChunkFromBackgroundParser()
#9 0x7f4f58333070 blink::HTMLDocumentParser::pumpPendingSpeculations()
#10 0x7f4f58349b6b blink::HTMLDocumentParser::resumeParsingAfterPause()
#11 0x7f4f5834a61b blink::HTMLDocumentParser::notifyScriptLoaded()
#12 0x7f4f583b2e87 blink::HTMLParserScriptRunner::pendingScriptFinished()
#13 0x7f4f57323aea blink::PendingScript::notifyFinished()
#14 0x7f4f6353b786 blink::Resource::checkNotify()
#15 0x7f4f6353d899 blink::Resource::error()
#16 0x7f4f6357fa59 blink::ResourceFetcher::handleLoaderError()
#17 0x7f4f635c636d blink::ResourceLoader::handleError()
#18 0x7f4f635c4283 blink::ResourceLoader::cancel()
#19 0x7f4f6357ff16 blink::ResourceFetcher::stopFetching()
#20 0x7f4f596b8344 blink::FrameLoader::stopAllLoaders()
#21 0x7f4f57c96c9f blink::LocalFrame::detach()
#22 0x7f4f57f53ec8 blink::HTMLFrameOwnerElement::disconnectContentFrame()
#23 0x7f4f56e4661e blink::ChildFrameDisconnector::disconnectCollectedFrameOwners()
#24 0x7f4f56e4586c blink::ChildFrameDisconnector::disconnect()
#25 0x7f4f56e8b807 blink::ContainerNode::willRemoveChild()
#26 0x7f4f56e8b0ee blink::ContainerNode::removeChild()
#27 0x7f4f56e86f42 blink::collectChildrenAndRemoveFromOldParent()
#28 0x7f4f56e86a71 blink::ContainerNode::collectChildrenAndRemoveFromOldParentWithCheck()
#29 0x7f4f56e87886 blink::ContainerNode::appendChild()
#30 0x7f4f57299059 blink::Node::appendChild()
#31 0x7f4f5a99a57f blink::NodeV8Internal::appendChildMethodForMainWorld()
#32 0x7f4f5a999ab0 blink::V8Node::appendChildMethodCallbackForMainWorld()
#33 0x7f4f6520b27d v8::internal::FunctionCallbackArguments::Call()
#34 0x7f4f65462785 v8::internal::(anonymous namespace)::HandleApiCallHelper<>()
#35 0x7f4f6545e74c v8::internal::Builtin_Impl_HandleApiCall()
#36 0x7f4f6545d992 v8::internal::Builtin_HandleApiCall()
#37 0x7f4f12984204 <unknown>

Blocking: 696775
Project Member

Comment 7 by bugdroid1@chromium.org, Feb 28 2017

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

commit a08766e7f8ec19cc09341d18b91107377ccfbcd6
Author: hiroshige <hiroshige@chromium.org>
Date: Tue Feb 28 10:28:05 2017

Clear pointers to PendingScript when dispose()d in pendingScriptFinished()

In HTMLParserScriptRunner, |pendingScript| is dispose()d but the
corresponding pointer to it is not cleared and thus causing the use of
PendingScript that is already dispose()d,
causing CHECK() failure in PendingScript::element().

There can be two cases, where
(1) |pendingScript| is |m_parserBlockingScript|
    (Clusterfuzz found a test case at  Issue 695730 ), or
(2) |pendingScript| is |m_scriptsToExecuteAfterParsing.first()|
    (no test case found so far).

This CL fixes these cases by clearing |m_parserBlockingScript| or
removing |m_scriptsToExecuteAfterParsing.first()|.

This CL also adds CHECK(false) for (2), which should be removed shortly,
hoping clusterfuzz find a test case for (2).

This is a regression since https://codereview.chromium.org/2693423002.
Before that, when |m_parserBlockingScript| was already disposed, then
it was considered as not having a parser blocking script, and thus
calling PendingScript::dispose() was sufficient.

BUG= 695730 , 696775, 686281

Review-Url: https://codereview.chromium.org/2720683003
Cr-Commit-Position: refs/heads/master@{#453564}

[modify] https://crrev.com/a08766e7f8ec19cc09341d18b91107377ccfbcd6/third_party/WebKit/Source/core/html/parser/HTMLParserScriptRunner.cpp

Project Member

Comment 8 by ClusterFuzz, Feb 28 2017

ClusterFuzz has detected this issue as fixed in range 453545:453565.

Detailed report: https://cluster-fuzz.appspot.com/testcase?key=6273407311413248

Fuzzer: inferno_twister
Job Type: linux_asan_chrome_chromeos
Platform Id: linux

Crash Type: CHECK failure
Crash Address: 
Crash State:
  m_element in PendingScript.cpp
  blink::PendingScript::element
  blink::traceParserBlockingScript
  
Sanitizer: address (ASAN)

Fixed: https://cluster-fuzz.appspot.com/revisions?job=linux_asan_chrome_chromeos&range=453545:453565

Reproducer Testcase: https://cluster-fuzz.appspot.com/download/AMIfv974WnqrTWewYSZpszyaSsnvSRNZbkAmwMHQrlv_I1w4qSzzhw59ZL8vnQ_xaFzUx7XTteRQRBht-7oEJivkESZydEYJ6Drihd9Z_fDmVJf23wzg-Nh3rrNEO8rUHuoT9ARXaGmi76YqJeyuXrzuvygh3dWQSI01ts1-bFRraj9IvzRXFnE3kUEu7-NZOLaFSBwOY_VYuwL9hK8duYEhOFrs7ds4I2cNLduWWuv8hZUu68RBWnMPU1ite6QGl4QH8PiF5S93ezglnMG2wTdi7GmpKTpWkoZDkxPUjCBw7VF8-ZZkcCs7QfS2Jj0v1RucoB0alVc5kduIXzjP2uB1ob8ybdKmm5zLVDiKOhwQzbcCqPJ839I?testcase_id=6273407311413248


See https://dev.chromium.org/Home/chromium-security/bugs/reproducing-clusterfuzz-bugs for more information.

If you suspect that the result above is incorrect, try re-doing that job on the test case report page.
Status: Fixed (was: Started)
Project Member

Comment 10 by bugdroid1@chromium.org, May 21 2018

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

commit c365bd25d9c0c2d847bc7cc5aab378ed0d29e1c9
Author: Kouhei Ueno <kouhei@chromium.org>
Date: Mon May 21 12:26:03 2018

retire CHECK(false)

Bug:  695730 , 696775, 686281
Change-Id: Id11515ceb7ce61fa15411a57ac4a105d03f3e6e6
Reviewed-on: https://chromium-review.googlesource.com/1065559
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Commit-Queue: Kouhei Ueno <kouhei@chromium.org>
Cr-Commit-Position: refs/heads/master@{#560258}
[modify] https://crrev.com/c365bd25d9c0c2d847bc7cc5aab378ed0d29e1c9/third_party/blink/renderer/core/script/html_parser_script_runner.cc

Sign in to add a comment