LoggingTest.CheckCausesDistinctBreakpoints broken on win/clang bots |
|||
Issue descriptionTest was added here: https://codereview.chromium.org/2676483002 https://chromium.googlesource.com/chromium/src/+blame/6bab2561839d0c280b5490bfbfff6fc1ed4b88eb/base/logging_unittest.cc#198 It uses SEH which is always a bit suspicious, but it fails on 64-bit too: https://build.chromium.org/p/chromium.fyi/builders/CrWinClang64%20tester/builds/5450/steps/base_unittests/logs/LoggingTest.CheckCausesDistinctBreakpoints LoggingTest.CheckCausesDistinctBreakpoints (run #1): [ RUN ] LoggingTest.CheckCausesDistinctBreakpoints ../../base/logging_unittest.cc(245): error: Expected: (addr1) != (addr2), actual: 000000013F7FD8DF vs 000000013F7FD8DF ../../base/logging_unittest.cc(246): error: Expected: (addr1) != (addr3), actual: 000000013F7FD8DF vs 000000013F7FD8DF ../../base/logging_unittest.cc(247): error: Expected: (addr2) != (addr3), actual: 000000013F7FD8DF vs 000000013F7FD8DF [ FAILED ] LoggingTest.CheckCausesDistinctBreakpoints (0 ms) So either clang inlines things, or it makes a single island for all the int3s, and we might need something similar to primiano's https://codereview.chromium.org/2502953003/ (any reason we run this test only in official builds?)
,
Feb 20 2017
,
Feb 20 2017
Semi-related in trying to hack around this, it looks like clang-cl lacks support for useful intrinsics here including __ud2() and __fastfail().
,
Feb 20 2017
Coincidentally I implemented __fastfail two weeks ago, but we haven't had a clang roll in a while: http://llvm.org/viewvc/llvm-project?rev=294606&view=rev I don't think we have __ud2, but we can add it.
,
Feb 20 2017
https://codereview.chromium.org/2701253005/. I tried some other ways to uniquify (for example, __inbyte(__COUNTER__) might a short one), but those require intrinsic support from clang-cl. For the time being I think adding a null deref after debugbreak for clang is the best solution. Since this is going to be fiddly compiler-specific and platform-specific code anyway since we're carefully tailoring the implementation to the optimizer (especially once Primiano's change relands) I don't feel too bad about having a separate branch for clang-cl vs. cl.
,
Feb 20 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/92bbdc39154ce7cbbc939d9a4c5ee371691ab811 commit 92bbdc39154ce7cbbc939d9a4c5ee371691ab811 Author: scottmg <scottmg@chromium.org> Date: Mon Feb 20 21:06:25 2017 Make LoggingTest.CheckCausesDistinctBreakpoints work on clang win Clang coalesces the int3s of the simple implementation of CHECK as __debugbreak(). To avoid this, add an additional null deref following the __debugbreak() when __clang__ to uniquify crash locations. R=thakis@chromium.org BUG= 693713 Review-Url: https://codereview.chromium.org/2701253005 Cr-Commit-Position: refs/heads/master@{#451660} [modify] https://crrev.com/92bbdc39154ce7cbbc939d9a4c5ee371691ab811/base/logging.h
,
Mar 6 2017
Looks fixed now, thanks! |
|||
►
Sign in to add a comment |
|||
Comment 1 by scottmg@chromium.org
, Feb 17 2017