DrMemory: Uninitialized read in CSSSelectorParse |
||||
Issue descriptionr397094 looks cuprit. esprehn@, can you look into this? Started on: https://build.chromium.org/p/chromium.memory.fyi/builders/Windows%20Content%20Browser%20%28DrMemory%20full%29%20%285%29/builds/8260 other bots are failing due to this too. UNINITIALIZED READ: reading 0x001ce0e0-0x001ce0e1 1 byte(s) # 0 wtf.dll!WTF::equalIgnoringASCIICase [third_party\webkit\source\wtf\text\stringview.cpp:60] # 1 webcore_shared.dll!blink::CSSSelectorParser::consumeAttributeFlags [third_party\webkit\source\core\css\parser\cssselectorparser.cpp:633] # 2 webcore_shared.dll!blink::CSSSelectorParser::consumeAttribute [third_party\webkit\source\core\css\parser\cssselectorparser.cpp:457] # 3 webcore_shared.dll!blink::CSSSelectorParser::consumeSimpleSelector [third_party\webkit\source\core\css\parser\cssselectorparser.cpp:346] # 4 webcore_shared.dll!blink::CSSSelectorParser::consumeCompoundSelector [third_party\webkit\source\core\css\parser\cssselectorparser.cpp:306] # 5 webcore_shared.dll!blink::CSSSelectorParser::consumeComplexSelector [third_party\webkit\source\core\css\parser\cssselectorparser.cpp:176] # 6 webcore_shared.dll!blink::CSSSelectorParser::consumeComplexSelectorList [third_party\webkit\source\core\css\parser\cssselectorparser.cpp:108] # 7 webcore_shared.dll!blink::CSSSelectorParser::parseSelector [third_party\webkit\source\core\css\parser\cssselectorparser.cpp:91] # 8 webcore_shared.dll!blink::CSSParserImpl::consumeStyleRule [third_party\webkit\source\core\css\parser\cssparserimpl.cpp:685] # 9 webcore_shared.dll!blink::CSSParserImpl::consumeQualifiedRule [third_party\webkit\source\core\css\parser\cssparserimpl.cpp:425] #10 webcore_shared.dll!blink::CSSParserImpl::consumeRuleList<> [third_party\webkit\source\core\css\parser\cssparserimpl.cpp:340] #11 webcore_shared.dll!blink::CSSParserImpl::parseStyleSheet [third_party\webkit\source\core\css\parser\cssparserimpl.cpp:173] #12 webcore_shared.dll!blink::CSSParser::parseSheet [third_party\webkit\source\core\css\parser\cssparser.cpp:53] #13 webcore_shared.dll!blink::StyleSheetContents::parseStringAtPosition [third_party\webkit\source\core\css\stylesheetcontents.cpp:366] #14 webcore_shared.dll!blink::StyleSheetContents::parseString [third_party\webkit\source\core\css\stylesheetcontents.cpp:360] #15 webcore_shared.dll!blink::parseUASheet [third_party\webkit\source\core\css\cssdefaultstylesheets.cpp:67] #16 webcore_shared.dll!blink::CSSDefaultStyleSheets::CSSDefaultStyleSheets [third_party\webkit\source\core\css\cssdefaultstylesheets.cpp:97] #17 webcore_shared.dll!blink::CSSDefaultStyleSheets::instance [third_party\webkit\source\core\css\cssdefaultstylesheets.cpp:48] #18 webcore_shared.dll!blink::StyleResolver::finishAppendAuthorStyleSheets [third_party\webkit\source\core\css\resolver\styleresolver.cpp:273] #19 webcore_shared.dll!blink::StyleEngine::appendActiveAuthorStyleSheets [third_party\webkit\source\core\dom\styleengine.cpp:375] #20 webcore_shared.dll!blink::StyleEngine::createResolver [third_party\webkit\source\core\dom\styleengine.cpp:391] #21 webcore_shared.dll!blink::Document::ensureStyleResolver [third_party\webkit\source\core\dom\document.cpp:2137] #22 webcore_shared.dll!blink::Document::updateStyleAndLayoutTree [third_party\webkit\source\core\dom\document.cpp:1757] #23 webcore_shared.dll!blink::FrameView::updateStyleAndLayoutIfNeededRecursiveInternal [third_party\webkit\source\core\frame\frameview.cpp:2605] #24 webcore_shared.dll!blink::FrameView::updateStyleAndLayoutIfNeededRecursive [third_party\webkit\source\core\frame\frameview.cpp:2588] #25 webcore_shared.dll!blink::FrameView::updateLifecyclePhasesInternal [third_party\webkit\source\core\frame\frameview.cpp:2446] #26 webcore_shared.dll!blink::FrameView::updateAllLifecyclePhases [third_party\webkit\source\core\frame\frameview.cpp:2401] #27 blink_web.dll!blink::PageWidgetDelegate::updateAllLifecyclePhases [third_party\webkit\source\web\pagewidgetdelegate.cpp:61] #28 blink_web.dll!blink::WebViewImpl::updateAllLifecyclePhases [third_party\webkit\source\web\webviewimpl.cpp:2037] #29 content.dll!content::RenderWidgetCompositor::UpdateLayerTreeHost [content\renderer\gpu\render_widget_compositor.cc:925] #30 cc.dll!base::internal::Invoker<>::Run [base\bind_internal.h:363] #31 base.dll!base::debug::TaskAnnotator::RunTask [base\debug\task_annotator.cc:51] #32 scheduler.dll!scheduler::TaskQueueManager::ProcessTaskFromWorkQueue [components\scheduler\base\task_queue_manager.cc:289] #33 scheduler.dll!scheduler::TaskQueueManager::DoWork [components\scheduler\base\task_queue_manager.cc:201] #34 scheduler.dll!base::internal::Invoker<>::Run [base\bind_internal.h:363] #35 base.dll!base::debug::TaskAnnotator::RunTask [base\debug\task_annotator.cc:51] #36 base.dll!base::MessageLoop::RunTask [base\message_loop\message_loop.cc:475] #37 base.dll!base::MessageLoop::DeferOrRunPendingTask [base\message_loop\message_loop.cc:484] #38 base.dll!base::MessageLoop::DoWork [base\message_loop\message_loop.cc:601] #39 base.dll!base::MessagePumpDefault::Run [base\message_loop\message_pump_default.cc:33] Note: @0:00:31.605 in thread 3352
,
Jun 1 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/2a699298dde743832faf90bb1926ffea29199eb6 commit 2a699298dde743832faf90bb1926ffea29199eb6 Author: oshima <oshima@chromium.org> Date: Wed Jun 01 19:50:21 2016 Suppress UNINIT READ in CSSSelectorParser BUG= 616556 TBR=esprehn@chromium.org Review-Url: https://codereview.chromium.org/2028203002 Cr-Commit-Position: refs/heads/master@{#397212} [modify] https://crrev.com/2a699298dde743832faf90bb1926ffea29199eb6/tools/valgrind/drmemory/suppressions.txt
,
Jun 1 2016
Evgenii, I ran all of content_browsertests through MSan, and didn't reproduce the UNINITIALIZED READ that DrMemory claims. Is MSan failing to detect the bug? The relevant field is a 1-bit bitfield.
,
Jun 1 2016
Hard to say. Bitfields are supported in MSan, but both tools have all kinds of false negatives due to approximate shadow propagation, and may take shortcuts in different places. If you want to dig into this, start by understanding where this uninit came from, and then see what MSan thinks about the state of memory at the same points in the program with __msan_check_mem_is_initialized(ptr, size).
,
Jun 2 2016
fwiw, the internet suggests DrMemory has some issues with bitfields that may lead to false positives. http://drmemory.org/docs/page_options.html https://blog.mozilla.org/jseward/2015/10/05/dr-memory-a-memory-checking-tool-for-windows/ esprehn@: It's not clear to me why StringView::m_is8Bit uses a bitfield since afaict it doesn't affect packing, changing the type there might also help.
,
Jun 2 2016
timloh@ I had plans add m_isAtomic so that I could replace equal(const StringImpl* a, const StringImpl* b)'s fast path with a single equal(StringView, StringView).
,
Jun 2 2016
+Derek to say if this looks like one of DrMemory's false positives. Elliott and I haven't been able to find an actual uninitialized path.
,
Jun 12 2016
I'm going to mark this as wont fix, I can't repro it and no one else can either. I also can't find any path that has the bad read. Note also that since https://chromium.googlesource.com/chromium/src/+/580cbf5d215fee758dd5c447cf03885b07bc9d2e there's no more bitfields. We should probably remove the suppression too. |
||||
►
Sign in to add a comment |
||||
Comment 1 by osh...@chromium.org
, Jun 1 2016