MasterHostScanCacheTest.TestRecoversFromCrashAndCleansUpWhenDeleted fails when using libc++ |
|||||
Issue descriptionI'm trying to make cros/desktop builds use libc++, https://chromium-review.googlesource.com/c/591775/ But: [ RUN ] DisplayManagerTest.NoOverlappedDisplays [8727:8727:0728/132931.856792:26235650082:FATAL:display.cc(64)] Check failed: id1 != id2 (2200000004 vs. 2200000004) #0 0x000000b615ac base::debug::StackTrace::StackTrace() #1 0x000000b6e0ac logging::LogMessage::~LogMessage() #2 0x0000011c1ac1 display::CompareDisplayIds() #3 0x0000011c9011 std::__1::__sort<>() #4 0x0000011c8e0c display::DisplayLayoutBuilder::Build() #5 0x000000480283 ash::DisplayManagerTest_NoOverlappedDisplays_Test::TestBody() #6 0x0000009e5c16 testing::Test::Run() #7 0x0000009e65a0 testing::TestInfo::Run() #8 0x0000009e6a87 testing::TestCase::Run() #9 0x0000009ecfa7 testing::internal::UnitTestImpl::RunAllTests() #10 0x0000009ecc27 testing::UnitTest::Run() #11 0x000000be24be base::TestSuite::Run() #12 0x000000be3a0b base::LaunchUnitTests() #13 0x00000054d3d4 main #14 0x7f801f39ef45 __libc_start_main #15 0x00000045f99c <unknown> Almost all other tests pass. I'm guessing that the code under test makes some invalid assumption about the c++ standard library (maybe it assumes that std::sort() is stable while it should use std::stable_sort, or something like that). khorimoto, this code is fairly new, do you have any guess what could be the cause?
,
Jul 28 2017
If iteration order is important, maybe the code should use a std::map instead of an std::unordered_map?
,
Jul 28 2017
Or if the order is only important in the test, maybe get all the elements out of the map and then explicitly sort() them before comparing?
,
Jul 28 2017
khorimoto, since you're familiar with the code, could you try to come up with a fix for this? (Ideally today :-) )
,
Jul 28 2017
I'm working on a fix right now :)
,
Jul 29 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/49b06439f30cef366621a69be797968d60f514d2 commit 49b06439f30cef366621a69be797968d60f514d2 Author: Kyle Horimoto <khorimoto@google.com> Date: Sat Jul 29 00:16:47 2017 Remove std::unordered_map sorting assumption from MasterHostScanCacheTest. This assumption prevented thakis@ from being able to use libc++ (see crbug.com/750342 ). Bug: 750342 Change-Id: Ifb7467ded104d2627bbfc08a973aba7520c36619 Reviewed-on: https://chromium-review.googlesource.com/592466 Commit-Queue: Kyle Horimoto <khorimoto@chromium.org> Reviewed-by: Nico Weber <thakis@chromium.org> Cr-Commit-Position: refs/heads/master@{#490551} [modify] https://crrev.com/49b06439f30cef366621a69be797968d60f514d2/chromeos/components/tether/master_host_scan_cache.cc [modify] https://crrev.com/49b06439f30cef366621a69be797968d60f514d2/chromeos/components/tether/master_host_scan_cache_unittest.cc
,
Jul 29 2017
Merge requested for M61. I know this bug is for adding libc++, but the CL to fix it actually introduces a performance improvement by using references instead of copies.
,
Jul 30 2017
Your change meets the bar and is auto-approved for M61. Please go ahead and merge the CL to branch 3163 manually. Please contact milestone owner if you have questions. Owners: amineer@(Android), cmasso@(iOS), ketakid @(ChromeOS), govind@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Jul 31 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/e30cff5e925496fcfa1ab9d00d1aa2c77104edb1 commit e30cff5e925496fcfa1ab9d00d1aa2c77104edb1 Author: Kyle Horimoto <khorimoto@google.com> Date: Mon Jul 31 18:14:20 2017 Remove std::unordered_map sorting assumption from MasterHostScanCacheTest. This assumption prevented thakis@ from being able to use libc++ (see crbug.com/750342 ). TBR=khorimoto@google.com (cherry picked from commit 49b06439f30cef366621a69be797968d60f514d2) Bug: 750342 Change-Id: Ifb7467ded104d2627bbfc08a973aba7520c36619 Reviewed-on: https://chromium-review.googlesource.com/592466 Commit-Queue: Kyle Horimoto <khorimoto@chromium.org> Reviewed-by: Nico Weber <thakis@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#490551} Reviewed-on: https://chromium-review.googlesource.com/594687 Reviewed-by: Kyle Horimoto <khorimoto@chromium.org> Cr-Commit-Position: refs/branch-heads/3163@{#167} Cr-Branched-From: ff259bab28b35d242e10186cd63af7ed404fae0d-refs/heads/master@{#488528} [modify] https://crrev.com/e30cff5e925496fcfa1ab9d00d1aa2c77104edb1/chromeos/components/tether/master_host_scan_cache.cc [modify] https://crrev.com/e30cff5e925496fcfa1ab9d00d1aa2c77104edb1/chromeos/components/tether/master_host_scan_cache_unittest.cc
,
Jan 22 2018
|
|||||
►
Sign in to add a comment |
|||||
Comment 1 by khorimoto@chromium.org
, Jul 28 2017