New issue
Advanced search Search tips

Issue 793548 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Dec 2017
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 2
Type: Bug



Sign in to add a comment

Linux CFI build broken (ExtensionStorageMonitorTest)

Project Member Reported by est...@chromium.org, Dec 9 2017

Issue description

broken by 37bd8dbf991d69ecf3bc3. Here's the stack:

../../components/keyed_service/content/browser_context_keyed_service_factory.cc:118:26: runtime error: control flow integrity check for type 'content::BrowserContext' failed during base-to-derived cast (vtable address 0xffffd8dfb7bc2885)
0xffffd8dfb7bc2885: note: invalid vtable
<memory cannot be printed>
    #0 0xcdc22fc in BrowserContextKeyedServiceFactory::ContextShutdown(base::SupportsUserData*) components/keyed_service/content/browser_context_keyed_service_factory.cc:118:26
    #1 0xc43f283 in KeyedServiceFactory::SetTestingFactory(base::SupportsUserData*, std::__1::function<std::__1::unique_ptr<KeyedService, std::__1::default_delete<KeyedService> > (base::SupportsUserData*)>) components/keyed_service/core/keyed_service_factory.cc:42:3
    #2 0xcdc1c0c in BrowserContextKeyedServiceFactory::SetTestingFactory(content::BrowserContext*, std::__1::unique_ptr<KeyedService, std::__1::default_delete<KeyedService> > (*)(content::BrowserContext*)) components/keyed_service/content/browser_context_keyed_service_factory.cc:24:24
    #3 0x6ccea84 in operator() buildtools/third_party/libc++/trunk/include/memory:2233:5
    #4 0x6ccea84 in reset buildtools/third_party/libc++/trunk/include/memory:2546:0
    #5 0x6ccea84 in ~unique_ptr buildtools/third_party/libc++/trunk/include/memory:2500:0
    #6 0x6ccea84 in extensions::ExtensionStorageMonitorTest::~ExtensionStorageMonitorTest() chrome/browser/extensions/extension_storage_monitor_browsertest.cc:89:0
    #7 0x6ccd1f2 in extensions::ExtensionStorageMonitorTest_TwoHostedAppsInSameOrigin_Test::~ExtensionStorageMonitorTest_TwoHostedAppsInSameOrigin_Test() chrome/browser/extensions/extension_storage_monitor_browsertest.cc:381:1
    #8 0x6ccd21d in extensions::ExtensionStorageMonitorTest_TwoHostedAppsInSameOrigin_Test::~ExtensionStorageMonitorTest_TwoHostedAppsInSameOrigin_Test() chrome/browser/extensions/extension_storage_monitor_browsertest.cc:381:1
    #9 0x715b42b in testing::TestInfo::Run() third_party/googletest/src/googletest/src/gtest.cc:2660:3
    #10 0x715bc91 in testing::TestCase::Run() third_party/googletest/src/googletest/src/gtest.cc:2773:28
    #11 0x71611b2 in testing::internal::UnitTestImpl::RunAllTests() third_party/googletest/src/googletest/src/gtest.cc:4673:43
    #12 0x7160e4c in testing::UnitTest::Run() third_party/googletest/src/googletest/src/gtest.cc:4281:10
    #13 0x9d7046c in base::TestSuite::Run() base/test/test_suite.cc:272:16
    #14 0x9c0d98f in ChromeTestSuiteRunner::RunTestSuite(int, char**) chrome/test/base/chrome_test_launcher.cc:72:38
    #15 0xa64fc7a in content::LaunchTests(content::TestLauncherDelegate*, unsigned long, int, char**) content/public/test/test_launcher.cc:632:31
    #16 0x9c0dd7c in LaunchChromeTests(unsigned long, content::TestLauncherDelegate*, int, char**) chrome/test/base/chrome_test_launcher.cc:169:10
    #17 0x9c0d8db in main.cfi chrome/test/base/browser_tests_main.cc:36:10
    #18 0x7fe4ed9a1f44 in __libc_start_main /build/eglibc-SvCtMH/eglibc-2.19/csu/libc-start.c:287:0
 
Project Member

Comment 1 by bugdroid1@chromium.org, Dec 11 2017

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

commit 5bd7d06491325bf3f8a1e96fe70bbc302e23be4b
Author: Evan Stade <estade@chromium.org>
Date: Mon Dec 11 18:12:30 2017

Fix Linux CFI bot by fixing ExtensionStorageMonitorTest.

Reland 37bd8dbf991d69ecf3bc3804 with changes:

1. Destroy the NotificationDisplayServiceTester before the profile goes
   down (to fix the error).
2. Make the extension storage monitor depend on the notification service
   (for correctness --- doesn't help fix this error).
3. Don't explicitly remove notifications when the profile goes down. The
   NotificationDisplayService should remove all transient notifications
   for its profile when it's destroyed, which happens when the profile
   is destroyed. This part is necessary because when you remove a testing
   factory it doesn't reinstate the normal, non-testing factory, so we
   get a null deref. See KeyedServiceFactory::testing_factories_

TBR=stevenjb@chromium.org

Bug:  793548 
Change-Id: I834091ad6c53aaf923756e723a234013f92b1cd9
Reviewed-on: https://chromium-review.googlesource.com/817657
Commit-Queue: Evan Stade <estade@chromium.org>
Reviewed-by: Evan Stade <estade@chromium.org>
Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#523138}
[modify] https://crrev.com/5bd7d06491325bf3f8a1e96fe70bbc302e23be4b/chrome/browser/extensions/extension_storage_monitor.cc
[modify] https://crrev.com/5bd7d06491325bf3f8a1e96fe70bbc302e23be4b/chrome/browser/extensions/extension_storage_monitor.h
[modify] https://crrev.com/5bd7d06491325bf3f8a1e96fe70bbc302e23be4b/chrome/browser/extensions/extension_storage_monitor_browsertest.cc
[modify] https://crrev.com/5bd7d06491325bf3f8a1e96fe70bbc302e23be4b/chrome/browser/extensions/extension_storage_monitor_factory.cc
[modify] https://crrev.com/5bd7d06491325bf3f8a1e96fe70bbc302e23be4b/chrome/browser/notifications/notification_display_service_tester.cc
[modify] https://crrev.com/5bd7d06491325bf3f8a1e96fe70bbc302e23be4b/chrome/browser/notifications/notification_display_service_tester.h
[modify] https://crrev.com/5bd7d06491325bf3f8a1e96fe70bbc302e23be4b/chrome/browser/notifications/stub_notification_display_service.cc

Comment 2 by est...@chromium.org, Dec 13 2017

Status: Fixed (was: Started)

Sign in to add a comment