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

Issue 604534 link

Starred by 1 user
Status: Fixed
Owner:
wangxianzhu@chromium.org
Closed: Apr 2016
Cc:
kcc@chromium.org
█ krasin@chromium.org
wangxianzhu@chromium.org
p...@chromium.org
thakis@chromium.org
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug



Sign in to add a comment

UBSAN vptr bot is broken (multiple browser tests)

Project Member Reported by krasin@chromium.org, Apr 18 2016 
As of now, UBSAN vptr buildbot is very broken. Multiple browser tests fail:
https://build.chromium.org/p/chromium.fyi/builders/ClangToTLinuxUBSanVptr%20tester/builds/414

It began on 9 Apr 2016:
https://build.chromium.org/p/chromium.fyi/builders/ClangToTLinuxUBSanVptr%20tester/builds/391

To reproduce:

export GYP_DEFINES=
ninja -C out/Release content_browsertests
export DISPLAY=:0.0
./out/Release/content_browsertests  --gtest_filter=AccessibilityHitTestingBrowserTest.HitTestOutsideDocumentBoundsReturnsRoot --no-sandbox --single_process --renderer-cmd-prefix="xterm -e gdb --args" zygote-cmd-prefix="xterm -e gdb --args" --utility-cmd-prefix="xterm -e gdb --args" --gpu-launcher="xterm -e gdb --args"

And then "r", then "bt" in one of the terminals opened.

#0  0x000000000b25d4db in insertOnlyThisLayerAfterStyleChange () at ../../third_party/WebKit/Source/core/paint/PaintLayer.cpp:1340
#1  0x000000000b975e9c in createLayer () at ../../third_party/WebKit/Source/core/layout/LayoutBoxModelObject.cpp:340
#2  0x000000000b96ee3b in styleDidChange () at ../../third_party/WebKit/Source/core/layout/LayoutBoxModelObject.cpp:210
#3  0x000000000b8ce575 in styleDidChange () at ../../third_party/WebKit/Source/core/layout/LayoutBox.cpp:239
#4  0x000000000b7c4878 in styleDidChange () at ../../third_party/WebKit/Source/core/layout/LayoutBlock.cpp:324
#5  0x000000000b86b7a7 in styleDidChange () at ../../third_party/WebKit/Source/core/layout/LayoutBlockFlow.cpp:2019
#6  0x000000000bb2f657 in setStyle () at ../../third_party/WebKit/Source/core/layout/LayoutObject.cpp:1958
#7  0x0000000009912807 in attach () at ../../third_party/WebKit/Source/core/dom/Document.cpp:2128
#8  0x000000000ad174e7 in installNewDocument () at ../../third_party/WebKit/Source/core/frame/LocalDOMWindow.cpp:350
#9  0x000000000b040b64 in createWriterFor () at ../../third_party/WebKit/Source/core/loader/DocumentLoader.cpp:673
#10 0x000000000b03f605 in ensureWriter () at ../../third_party/WebKit/Source/core/loader/DocumentLoader.cpp:454
#11 0x000000000b0381cb in commitData () at ../../third_party/WebKit/Source/core/loader/DocumentLoader.cpp:469
#12 0x000000000b036b53 in finishedLoading () at ../../third_party/WebKit/Source/core/loader/DocumentLoader.cpp:283
#13 0x000000000b0435ec in maybeLoadEmpty () at ../../third_party/WebKit/Source/core/loader/DocumentLoader.cpp:615
#14 0x000000000b043c22 in startLoadingMainResource () at ../../third_party/WebKit/Source/core/loader/DocumentLoader.cpp:626
#15 0x000000000b09418b in init () at ../../third_party/WebKit/Source/core/loader/FrameLoader.cpp:200
#16 0x00000000081f9bfb in init () at ../../third_party/WebKit/Source/core/frame/LocalFrame.h:233
#17 initializeCoreFrame () at ../../third_party/WebKit/Source/web/WebLocalFrameImpl.cpp:1516
#18 0x00000000015df01d in CreateMainFrame () at ../../content/renderer/render_frame_impl.cc:841
#19 0x00000000016f2593 in Initialize () at ../../content/renderer/render_view_impl.cc:710
#20 0x0000000001701198 in Create () at ../../content/renderer/render_view_impl.cc:1127
#21 0x00000000016dae75 in DispatchToMethodImpl<content::RenderThreadImpl*, void (content::RenderThreadImpl::*)(ViewMsg_New_Params const&), ViewMsg_New_Params, 0> () at ../../base/tuple.h:166
#22 DispatchToMethod<content::RenderThreadImpl*, void (content::RenderThreadImpl::*)(ViewMsg_New_Params const&), ViewMsg_New_Params> () at ../../base/tuple.h:173
#23 DispatchToMethod<content::RenderThreadImpl, void (content::RenderThreadImpl::*)(ViewMsg_New_Params const&), void, std::tuple<ViewMsg_New_Params> > () at ../../ipc/ipc_message_templates.h:26
#24 Dispatch<content::RenderThreadImpl, content::RenderThreadImpl, void, void (content::RenderThreadImpl::*)(ViewMsg_New_Params const&)> () at ../../ipc/ipc_message_templates.h:121
#25 0x00000000016d864e in OnControlMessageReceived () at ../../content/renderer/render_thread_impl.cc:1754
#26 0x000000000ce1c14b in OnMessageReceived () at ../../content/child/child_thread_impl.cc:648
#27 0x000000000565631c in OnDispatchMessage () at ../../ipc/ipc_channel_proxy.cc:282
#28 0x0000000002dec02a in Run () at ../../base/callback.h:397
#29 RunTask () at ../../base/debug/task_annotator.cc:51
#30 0x000000000cff99e5 in ProcessTaskFromWorkQueue () at ../../components/scheduler/base/task_queue_manager.cc:289
#31 0x000000000cff398f in DoWork () at ../../components/scheduler/base/task_queue_manager.cc:201
#32 0x000000000cffe445 in Run () at ../../base/bind_internal.h:372
#33 0x0000000002dec02a in Run () at ../../base/callback.h:397
#34 RunTask () at ../../base/debug/task_annotator.cc:51
#35 0x0000000002c90cc1 in RunTask () at ../../base/message_loop/message_loop.cc:479
#36 0x0000000002c91f28 in DeferOrRunPendingTask () at ../../base/message_loop/message_loop.cc:488
#37 0x0000000002c928e3 in DoWork () at ../../base/message_loop/message_loop.cc:600
#38 0x0000000002c98fd4 in Run () at ../../base/message_loop/message_pump_default.cc:33
#39 0x0000000002c8f88f in RunHandler () at ../../base/message_loop/message_loop.cc:443
#40 0x0000000002cf3986 in base::RunLoop::Run() () at ../../base/run_loop.cc:35
#41 0x0000000002c8c681 in Run () at ../../base/message_loop/message_loop.cc:295
#42 0x000000000f7e68df in RendererMain () at ../../content/renderer/renderer_main.cc:219
#43 0x000000000f1d8065 in RunNamedProcessTypeMain () at ../../content/app/content_main_runner.cc:387
#44 0x000000000f1dafed in Run () at ../../content/app/content_main_runner.cc:755
#45 0x000000000f1d561d in ContentMain () at ../../content/app/content_main.cc:22
#46 0x000000000208cf0c in LaunchTests () at ../../content/public/test/test_launcher.cc:523
#47 0x0000000001ec027f in main () at ../../content/test/content_test_launcher.cc:131

The code from the top stack frame does not look particularly related to UBSAN vptr:
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit/Source/core/paint/PaintLayer.cpp&sq=package:chromium&type=cs&l=1340&q=third_party/WebKit/Source/core/paint/PaintLayer.cpp:1340

if (PaintLayer* enclosingSelfPaintingLayer = m_parent->enclosingSelfPaintingLayer())
            mergeNeedsPaintPhaseFlagsFrom(*enclosingSelfPaintingLayer);

Looking...

Comment 1 by krasin@chromium.org, Apr 18 2016 

Correction:

export GYP_DEFINES='clang=1 clang_use_chrome_plugins=0 component=static_library release_extra_cflags=-fno-sanitize-recover=undefined sanitizer_coverage=edge ubsan_vptr=1 symbol_level=1 dcheck_always_on=true'

Comment 2 by krasin@chromium.org, Apr 18 2016 

Slightly simpler command line that only opens a single xterm / GDB:

./out/Release/content_browsertests  --gtest_filter=AccessibilityHitTestingBrowserTest.HitTestOutsideDocumentBoundsReturnsRoot --no-sandbox  --renderer-cmd-prefix="xterm -e gdb --args"

Comment 3 by krasin@chromium.org, Apr 19 2016

Cc: wangxianzhu@chromium.org krasin@chromium.org
Owner: wangxianzhu@chromium.org
Bisect pointed out to this commit:

commit eb3bc319810c236e1a030d0f0c04867e1cdb8862
Author: wangxianzhu <wangxianzhu@chromium.org>
Date:   Fri Apr 8 17:53:50 2016 -0700

    Update PaintLayer::needsPaintPhaseXXX flags when add/remove layer on style change
    
    When addding/removing a layer on style change, we may not
    do paint invalidation to update the needsPaintPhaseXXX flags, so
    we need to update the flags manually.
    
    In the future, the logic can be simplified by updating the flags
    during pre-painting tree walk.
    
    BUG= 598978 
    
    Review URL: https://codereview.chromium.org/1862313002
    
    Cr-Commit-Position: refs/heads/master@{#386264}


Xianzhu, can you please take a quick look? Does the stack trace give you any insight what could be broken? If not, I will continue to dive into this (just reassign the bug back to me).

Comment 4 by wangxianzhu@chromium.org, Apr 19 2016

Status: Started (was: Untriaged)

Comment 5 by wangxianzhu@chromium.org, Apr 19 2016

Labels: -Pri-3 M-51 Pri-1
Project Member

Comment 6 by bugdroid1@chromium.org, Apr 19 2016 

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

commit c24d1e3d26bcdd4fc4afbc579ec65fbaadd4fbfa
Author: wangxianzhu <wangxianzhu@chromium.org>
Date: Tue Apr 19 19:50:10 2016

Check null m_parent in PaintLayer::insertOnlyThisLayerAfterStyleChange()

PaintLayer::m_parent may be nullptr if an orphan LayoutObject is set
style causing it to create a layer.

The problem was only detected by ubsan_vptr because though there was
call to PaintLayer::enclosingSelfPaintingLayer() with null 'this', the
method returns null without any actual harm :)

BUG= 604534 
TEST=All tests pass with ubsan_vptr

Review URL: https://codereview.chromium.org/1901193002

Cr-Commit-Position: refs/heads/master@{#388286}

[modify] https://crrev.com/c24d1e3d26bcdd4fc4afbc579ec65fbaadd4fbfa/third_party/WebKit/Source/core/paint/PaintLayer.cpp

Comment 7 by wangxianzhu@chromium.org, Apr 19 2016

Labels: -M-51
Status: Fixed (was: Started)
Removing M-51 because the bug actually does no harm.

Comment 8 by krasin@chromium.org, Apr 19 2016 

Thank you for fixing it so quickly!
►

Sign in to add a comment