New issue
Advanced search Search tips

Issue 717554 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: May 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 3
Type: Bug

Blocking:
issue 593874
issue 709456



Sign in to add a comment

BrowserActionsBarBrowserTest.OverflowedBrowserActionPopupTestRemoval failing in asan builds with new libc++

Project Member Reported by thakis@chromium.org, May 2 2017

Issue description

I'm trying to land a libc++ roll (https://codereview.chromium.org/2854703002/).

Almost everything works fine with it, but  BrowserActionsBarBrowserTest.OverflowedBrowserActionPopupTestRemoval fails in asan builds, like so:

thakis@thakis:~/src/chrome/src$ out/asan/browser_tests --gtest_filter=BrowserActionsBarBrowserTest.OverflowedBrowserActionPopupTestRemoval 2>&1 | tools/valgrind/asan/asan_symbolize.py 
IMPORTANT DEBUGGING NOTE: each test is run inside its own process.
For debugging a test inside a debugger, use the
--gtest_filter=<your_test_name> flag along with either
--single_process (to run the test in one launcher/browser process) or
--single-process (to do the above, and also run Chrome in single-process mode).
Using sharding settings from environment. This is shard 0/1
Using 1 parallel jobs.
Note: Google Test filter = BrowserActionsBarBrowserTest.OverflowedBrowserActionPopupTestRemoval
[==========] Running 1 test from 1 test case.
[----------] Global test environment set-up.
[----------] 1 test from BrowserActionsBarBrowserTest, where TypeParam =
[ RUN      ] BrowserActionsBarBrowserTest.OverflowedBrowserActionPopupTestRemoval
[127263:127263:0502/111635.766961:WARNING:password_store_factory.cc(249)] Using basic (unencrypted) store for password storage. See https://chromium.googlesource.com/chromium/src/+/master/docs/linux_password_storage.md for more information about password storage options.
BrowserTestBase received signal: Segmentation fault. Backtrace:
    #0 0x0000007ea911 in __interceptor_backtrace ??:0:0
    #1 0x0000082e2afc in base::debug::StackTrace::StackTrace(unsigned long) /usr/local/google/home/thakis/src/chrome/src/out/asan/../../base/debug/stack_trace_posix.cc:745:41
    #2 0x00000979d5ce in content::(anonymous namespace)::DumpStackTraceSignalHandler(int) /usr/local/google/home/thakis/src/chrome/src/out/asan/../../content/public/test/browser_test_base.cc:80:5
    #3 0x7f45b7956cb0 in killpg ??:?
    #4 0x7f45b7956cb0 in ?? ??:0
    #5 0x00000fbc96b4 in BrowserActionsContainer::Layout() /usr/local/google/home/thakis/src/chrome/src/out/asan/../../chrome/browser/ui/views/toolbar/browser_actions_container.cc:364:13
    #6 0x00000fbc7cce in BrowserActionsContainer::Redraw(bool) /usr/local/google/home/thakis/src/chrome/src/out/asan/../../chrome/browser/ui/views/toolbar/browser_actions_container.cc:214:3
    #7 0x00000f91141e in ToolbarActionsBar::UndoPopOut() /usr/local/google/home/thakis/src/chrome/src/out/asan/../../chrome/browser/ui/toolbar/toolbar_actions_bar.cc:534:16
    #8 0x00000fcb0dce in ExtensionActionViewController::OnPopupClosed() /usr/local/google/home/thakis/src/chrome/src/out/asan/../../chrome/browser/ui/extensions/extension_action_view_controller.cc:366:29
    #9 0x00000fcaf02e in ExtensionActionViewController::SetDelegate(ToolbarActionViewDelegate*) /usr/local/google/home/thakis/src/chrome/src/out/asan/../../chrome/browser/ui/extensions/extension_action_view_controller.cc:78:7
    #10 0x00000fbd26f1 in ToolbarActionView::~ToolbarActionView() /usr/local/google/home/thakis/src/chrome/src/out/asan/../../chrome/browser/ui/views/toolbar/toolbar_action_view.cc:76:21
    #11 0x00000fbd290e in ToolbarActionView::~ToolbarActionView() /usr/local/google/home/thakis/src/chrome/src/out/asan/../../chrome/browser/ui/views/toolbar/toolbar_action_view.cc:75:41
    #12 0x00000fbc7454 in operator() /usr/local/google/home/thakis/src/chrome/src/out/asan/../../buildtools/third_party/libc++/trunk/include/memory:2270:5
    #13 0x00000fbc7454 in reset /usr/local/google/home/thakis/src/chrome/src/out/asan/../../buildtools/third_party/libc++/trunk/include/memory:2583:0
    #14 0x00000fbc7454 in ~unique_ptr /usr/local/google/home/thakis/src/chrome/src/out/asan/../../buildtools/third_party/libc++/trunk/include/memory:2537:0
    #15 0x00000fbc7454 in destroy /usr/local/google/home/thakis/src/chrome/src/out/asan/../../buildtools/third_party/libc++/trunk/include/memory:1853:0
    #16 0x00000fbc7454 in __destroy<std::__1::unique_ptr<ToolbarActionView, std::__1::default_delete<ToolbarActionView> > > /usr/local/google/home/thakis/src/chrome/src/out/asan/../../buildtools/third_party/libc++/trunk/include/memory:1721:0
    #17 0x00000fbc7454 in destroy<std::__1::unique_ptr<ToolbarActionView, std::__1::default_delete<ToolbarActionView> > > /usr/local/google/home/thakis/src/chrome/src/out/asan/../../buildtools/third_party/libc++/trunk/include/memory:1589:0
    #18 0x00000fbc7454 in __destruct_at_end /usr/local/google/home/thakis/src/chrome/src/out/asan/../../buildtools/third_party/libc++/trunk/include/vector:418:0
    #19 0x00000fbc7454 in __destruct_at_end /usr/local/google/home/thakis/src/chrome/src/out/asan/../../buildtools/third_party/libc++/trunk/include/vector:804:0
    #20 0x00000fbc7454 in erase /usr/local/google/home/thakis/src/chrome/src/out/asan/../../buildtools/third_party/libc++/trunk/include/vector:1680:0
    #21 0x00000fbc7454 in BrowserActionsContainer::RemoveViewForAction(ToolbarActionViewController*) /usr/local/google/home/thakis/src/chrome/src/out/asan/../../chrome/browser/ui/views/toolbar/browser_actions_container.cc:174:0
    #22 0x00000f9131c6 in ToolbarActionsBar::OnToolbarActionRemoved(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&) /usr/local/google/home/thakis/src/chrome/src/out/asan/../../chrome/browser/ui/toolbar/toolbar_actions_bar.cc:661:14
    #23 0x00000e99484a in ToolbarActionsModel::RemoveItem(ToolbarActionsModel::ToolbarItem const&) /usr/local/google/home/thakis/src/chrome/src/out/asan/../../chrome/browser/ui/toolbar/toolbar_actions_model.cc:428:16
    #24 0x00000e990d01 in RemoveExtension /usr/local/google/home/thakis/src/chrome/src/out/asan/../../chrome/browser/ui/toolbar/toolbar_actions_model.cc:452:3
    #25 0x00000e990d01 in ToolbarActionsModel::OnExtensionUnloaded(content::BrowserContext*, extensions::Extension const*, extensions::UnloadedExtensionInfo::Reason) /usr/local/google/home/thakis/src/chrome/src/out/asan/../../chrome/browser/ui/toolbar/toolbar_actions_model.cc:234:0
    #26 0x0000056dd2e6 in extensions::ExtensionRegistry::TriggerOnUnloaded(extensions::Extension const*, extensions::UnloadedExtensionInfo::Reason) /usr/local/google/home/thakis/src/chrome/src/out/asan/../../extensions/browser/extension_registry.cc:71:14
    #27 0x00000ebcf383 in NotifyExtensionUnloaded /usr/local/google/home/thakis/src/chrome/src/out/asan/../../chrome/browser/extensions/extension_service.cc:1175:14
    #28 0x00000ebcf383 in ExtensionService::UnloadExtension(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, extensions::UnloadedExtensionInfo::Reason) /usr/local/google/home/thakis/src/chrome/src/out/asan/../../chrome/browser/extensions/extension_service.cc:1441:0
    #29 0x000002819662 in BrowserActionsBarBrowserTest_OverflowedBrowserActionPopupTestRemoval_Test::RunTestOnMainThread() /usr/local/google/home/thakis/src/chrome/src/out/asan/../../chrome/browser/ui/toolbar/browser_actions_bar_browsertest.cc:527:26


(This is with https://codereview.chromium.org/2857643002/ to get better stacks.)

Given that everything else works, this feels more like an extensions code bug to me than a libc++ issue, maybe similar to issue 599467.

rdevlin, does this happen to ring any bells?
 
This (blindly guessed) change seems to help:

thakis@thakis:~/src/chrome/src$ git diff
diff --git a/chrome/browser/ui/toolbar/toolbar_actions_bar.cc b/chrome/browser/ui/toolbar/toolbar_actions_bar.cc
index fd0bbf023704..feeedc302ddd 100644
--- a/chrome/browser/ui/toolbar/toolbar_actions_bar.cc
+++ b/chrome/browser/ui/toolbar/toolbar_actions_bar.cc
@@ -658,9 +658,9 @@ void ToolbarActionsBar::OnToolbarActionRemoved(const std::string& action_id) {
   std::unique_ptr<ToolbarActionViewController> removed_action =
       std::move(*iter);
   toolbar_actions_.erase(iter);
-  delegate_->RemoveViewForAction(removed_action.get());
   if (popped_out_action_ == removed_action.get())
     UndoPopOut();
+  delegate_->RemoveViewForAction(removed_action.get());
   removed_action.reset();
 
   // If the extension is being upgraded we don't want the bar to shrink

Bug 599467 might be related too?
Owner: thakis@chromium.org
Running comment 1 through try bots: https://codereview.chromium.org/2857653002/
Project Member

Comment 4 by bugdroid1@chromium.org, May 24 2017

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

commit 1899d987d016dc588d677d7a8b851379aca2bdaa
Author: thomasanderson <thomasanderson@chromium.org>
Date: Wed May 24 18:31:49 2017

Fix nullptr dereference of ToolbarActionView

BrowserActionsBarBrowserTest.OverflowedBrowserActionPopupTestRemoval
fails when using the latest libc++ and libc++abi [1].

Frame 20 in the stack trace in [2] nulls the pointer but did not
remove the ToolbarActionView from the vector (yet).  Frame 3 then
tries to use the null pointer.

This CL is necessary to roll buildtools (contains libc++ and
libc++abi) to the latest revision.

[1] https://codereview.chromium.org/2888383005/
[2] https://paste.googleplex.com/5158886495485952

BUG= 593874 , 717554 
TBR=finnur@chromium.org

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

[modify] https://crrev.com/1899d987d016dc588d677d7a8b851379aca2bdaa/chrome/browser/ui/views/toolbar/browser_actions_container.cc

Comment 5 by thakis@chromium.org, May 24 2017

Owner: thomasanderson@chromium.org
Status: Fixed (was: Untriaged)

Comment 6 by thakis@chromium.org, Sep 14 2017

Blocking: 593874

Sign in to add a comment