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

Issue 662265 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Nov 2016
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocking:
issue 607996



Sign in to add a comment

Call on a nil pointer in CompositedLayerMapping::containingSquashedLayer

Project Member Reported by krasin@chromium.org, Nov 4 2016

Issue description

Version: tip
OS: Linux x86-64

What steps will reproduce the problem?
(1) Build browser_tests with UBSan Vptr:
$ gn gen out/ubsan '--args=is_debug=false is_ubsan_no_recover=true is_ubsan_vptr=true symbol_level=2 use_goma=false' --check
$ ninja -C out/ubsan browser_tests

(2) Run MediaRouterElementsBrowserTest.MediaRouterContainerSearch of browser_tests:

$ ./out/ubsan/browser_tests --gtest_filter=MediaRouterElementsBrowserTest.MediaRouterContainerSearch 
...
Received signal 11 SEGV_MAPERR 000000000000
#0 0x0000049dbf96 base::debug::(anonymous namespace)::StackDumpSignalHandler()
#1 0x7f597f6d6330 <unknown>
#2 0x00000bb991d0 blink::CompositedLayerMapping::containingSquashedLayer()
#3 0x00000bbf4236 blink::CompositingLayerAssigner::getReasonsPreventingSquashing()
#4 0x00000bbf1e4d blink::CompositingLayerAssigner::assignLayersToBackingsInternal()
#5 0x00000bbf2c41 blink::CompositingLayerAssigner::assignLayersToBackingsInternal()
#6 0x00000bbf2c41 blink::CompositingLayerAssigner::assignLayersToBackingsInternal()
#7 0x00000bbf1b7c blink::CompositingLayerAssigner::assign()
#8 0x00000bbab4af blink::PaintLayerCompositor::updateIfNeeded()
#9 0x00000bbaa6c7 blink::PaintLayerCompositor::updateIfNeededRecursiveInternal()
#10 0x00000bba9ca7 blink::PaintLayerCompositor::updateIfNeededRecursive()
#11 0x00000b2c1dad blink::FrameView::updateLifecyclePhasesInternal()
#12 0x00000bd6fe36 blink::PageAnimator::updateAllLifecyclePhases()
#13 0x00000a63efc5 blink::WebViewImpl::updateAllLifecyclePhases()
#14 0x00000de2007a cc::ProxyMain::BeginMainFrame()
#15 0x00000de3c94f _ZN4base8internal13FunctorTraitsIMN2cc9ProxyMainEFvSt10unique_ptrINS2_28BeginMainFrameAndCommitStateESt14default_deleteIS5_EEEvE6InvokeIRKNS_7WeakPtrIS3_EEJS8_EEEvSA_OT_DpOT0_
#16 0x00000de3c857 _ZN4base8internal7InvokerINS0_9BindStateIMN2cc9ProxyMainEFvSt10unique_ptrINS3_28BeginMainFrameAndCommitStateESt14default_deleteIS6_EEEJNS_7WeakPtrIS4_EENS0_13PassedWrapperIS9_EEEEEFvvEE7RunIm
plIRKSB_RKSt5tupleIJSD_SF_EEJLm0ELm1EEEEvOT_OT0_NS_13IndexSequenceIJXspT1_EEEE
#17 0x000004ab9d5b base::debug::TaskAnnotator::RunTask()
#18 0x00000a51a85b blink::scheduler::TaskQueueManager::ProcessTaskFromWorkQueue()
#19 0x00000a51695a blink::scheduler::TaskQueueManager::DoWork()
#20 0x000004ab9d5b base::debug::TaskAnnotator::RunTask()
#21 0x000004a02360 base::MessageLoop::RunTask()
#22 0x000004a02809 base::MessageLoop::DeferOrRunPendingTask()
#23 0x000004a03143 base::MessageLoop::DoWork()
#24 0x000004a06563 base::MessagePumpDefault::Run()
#25 0x000004a01c1d base::MessageLoop::RunHandler()
#26 0x000004a421b9 base::RunLoop::Run()
#27 0x00000cca044c content::RendererMain()
#28 0x0000049adbfb content::RunZygote()
#29 0x0000049b037b content::ContentMainRunnerImpl::Run()
#30 0x0000049ad557 content::ContentMain()
#31 0x000005950443 content::LaunchTests()
#32 0x0000049c5374 main

This happens because layoutObject is NULL:
https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/layout/compositing/CompositedLayerMapping.cpp?q=blink::CompositedLayerMapping::containingSquashedLayer&sq=package:chromium&l=2609&dr=Ss

const GraphicsLayerPaintInfo* CompositedLayerMapping::containingSquashedLayer(
    const LayoutObject* layoutObject,
    const Vector<GraphicsLayerPaintInfo>& layers,
    unsigned maxSquashedLayerIndex) {
  for (size_t i = 0; i < layers.size() && i < maxSquashedLayerIndex; ++i) {
    if (layoutObject->isDescendantOf(layers[i].paintLayer->layoutObject()))
      return &layers[i];
  }
  return nullptr;
}

UBSan is not happy when trying to verify the vptr pointer at layoutObject->isDescendantOf call.
 
Blocking: 607996
Cc: thakis@chromium.org
In particular, the call with null layoutObject happens from
https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/layout/compositing/CompositingLayerAssigner.cpp?q=:getReasonsPreventingSquashing&sq=package:chromium&l=122

// FIXME: this is not efficient, since it walks up the tree. We should store
// these values on the CompositingInputsCache.
if (layer->clippingContainer() != squashingLayer.clippingContainer() &&
    !squashingLayer.compositedLayerMapping()->containingSquashedLayer(
        layer->clippingContainer(), squashingState.nextSquashedLayerIndex))
  return SquashingDisallowedReasonClippingContainerMismatch;

A naive fix posted: https://codereview.chromium.org/2473293002/
Project Member

Comment 3 by bugdroid1@chromium.org, Nov 4 2016

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

commit a0c9881a5470c0af5ba0faefb785e6c0751c669e
Author: krasin <krasin@chromium.org>
Date: Fri Nov 04 21:47:59 2016

Naive fix of a call on a nil object in CompositedLayerMapping::containingSquashedLayer.

layoutObject happens to be NULL in some cases. This CL fixes an undefined behavior by
avoiding making calls on a null object.

BUG= 662265 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

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

[modify] https://crrev.com/a0c9881a5470c0af5ba0faefb785e6c0751c669e/third_party/WebKit/Source/core/layout/compositing/CompositedLayerMapping.cpp

Status: Fixed (was: Untriaged)

Sign in to add a comment