Mir

Merge lp:~albaguirre/mir/fix-1522581 into lp:mir

Proposed by Alberto Aguirre
Status: Merged
Approved by: Alexandros Frantzis
Approved revision: no longer in the source branch.
Merged at revision: 3181
Proposed branch: lp:~albaguirre/mir/fix-1522581
Merge into: lp:mir
Diff against target: 75 lines (+15/-10)
4 files modified
CMakeLists.txt (+3/-0)
cmake/FindGtestGmock.cmake (+7/-0)
cmake/MirCommon.cmake (+4/-6)
tools/tsan-suppressions (+1/-4)
To merge this branch: bzr merge lp:~albaguirre/mir/fix-1522581
Reviewer Review Type Date Requested Status
Alexandros Frantzis (community) Approve
Alan Griffiths Approve
Daniel van Vugt Approve
PS Jenkins bot (community) continuous-integration Approve
Review via email: mp+279533@code.launchpad.net

Commit message

Fix TSan build when using GCC

Description of the change

Fix TSan build when using GCC

To post a comment you must log in.
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
Alexandros Frantzis (afrantzis) wrote :

Builds and works properly on xenial.

Note that TSAN catches some data races in our tests, so there is some work to be done before we reenable a CI job for this.

review: Approve
Revision history for this message
Alexandros Frantzis (afrantzis) wrote :

Actually, (some of) these comments seem to be out-of-date?

# TSan does not support multi-threaded fork
# TSan may open fds so "surface_creation_does_not_leak_fds" will not work as written
# TSan deadlocks when running StreamTransportTest/0.SendsFullMessagesWhenInterrupted - disable it until understood

review: Needs Information
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
Daniel van Vugt (vanvugt) wrote :

Sounds good.

review: Approve
Revision history for this message
Alan Griffiths (alan-griffiths) wrote :

OK

review: Approve
Revision history for this message
Alexandros Frantzis (afrantzis) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'CMakeLists.txt'
2--- CMakeLists.txt 2015-12-02 14:55:35 +0000
3+++ CMakeLists.txt 2015-12-04 15:08:42 +0000
4@@ -103,6 +103,9 @@
5 # -fsanitize=thread is used, but doesn't.
6 #
7 # Linking everything with tsan is harmless and simple, so do that.
8+ if (CMAKE_VERSION VERSION_GREATER 3.2.1)
9+ cmake_policy(SET CMP0022 OLD)
10+ endif()
11 link_libraries(tsan)
12 endif()
13 set(CMAKE_MODULE_LINKER_FLAGS "${CMAKE_MODULE_LINKER_FLAGS} -fsanitize=thread")
14
15=== modified file 'cmake/FindGtestGmock.cmake'
16--- cmake/FindGtestGmock.cmake 2015-09-09 18:37:28 +0000
17+++ cmake/FindGtestGmock.cmake 2015-12-04 15:08:42 +0000
18@@ -23,11 +23,18 @@
19 set(GTEST_CXX_FLAGS "-fPIC")
20 if (cmake_build_type_lower MATCHES "threadsanitizer")
21 set(GTEST_CXX_FLAGS "${GTEST_CXX_FLAGS} -fsanitize=thread")
22+elseif (cmake_build_type_lower MATCHES "ubsanitizer")
23+ set(GTEST_CXX_FLAGS "${GTEST_CXX_FLAGS} -fsanitize=undefined")
24 endif()
25
26 set(GTEST_CMAKE_ARGS "-DCMAKE_CXX_FLAGS=${GTEST_CXX_FLAGS}")
27 list(APPEND GTEST_CMAKE_ARGS -DCMAKE_CXX_COMPILER=${CMAKE_CXX_COMPILER})
28 list(APPEND GTEST_CMAKE_ARGS -DCMAKE_C_COMPILER=${CMAKE_C_COMPILER})
29+if (cmake_build_type_lower MATCHES "threadsanitizer")
30+ #Skip compiler check, since if GCC is the compiler, we need to link against -ltsan
31+ #explicitly; specifying additional linker flags doesn't seem possible for external projects
32+ list(APPEND GTEST_CMAKE_ARGS -DCMAKE_CXX_COMPILER_WORKS=1)
33+endif()
34 if (${CMAKE_CROSSCOMPILING})
35 if(DEFINED MIR_NDK_PATH)
36 list(APPEND GTEST_CMAKE_ARGS -DCMAKE_TOOLCHAIN_FILE=${CMAKE_MODULE_PATH}/LinuxCrossCompile.cmake)
37
38=== modified file 'cmake/MirCommon.cmake'
39--- cmake/MirCommon.cmake 2015-10-07 02:34:05 +0000
40+++ cmake/MirCommon.cmake 2015-12-04 15:08:42 +0000
41@@ -67,16 +67,14 @@
42 endif()
43
44 if(cmake_build_type_lower MATCHES "threadsanitizer")
45- find_program(LLVM_SYMBOLIZER llvm-symbolizer-3.6)
46- if (LLVM_SYMBOLIZER)
47+ if (NOT CMAKE_COMPILER_IS_GNUCXX)
48+ find_program(LLVM_SYMBOLIZER llvm-symbolizer-3.6)
49+ if (LLVM_SYMBOLIZER)
50 set(TSAN_EXTRA_OPTIONS "external_symbolizer_path=${LLVM_SYMBOLIZER}")
51+ endif()
52 endif()
53 # Space after ${TSAN_EXTRA_OPTIONS} works around bug in TSAN env. variable parsing
54 list(APPEND test_env "TSAN_OPTIONS=\"suppressions=${CMAKE_SOURCE_DIR}/tools/tsan-suppressions second_deadlock_stack=1 halt_on_error=1 history_size=7 ${TSAN_EXTRA_OPTIONS} \"")
55- # TSan does not support multi-threaded fork
56- # TSan may open fds so "surface_creation_does_not_leak_fds" will not work as written
57- # TSan deadlocks when running StreamTransportTest/0.SendsFullMessagesWhenInterrupted - disable it until understood
58- set(test_exclusion_filter "UnresponsiveClient.does_not_hang_server:DemoInProcessServerWithStubClientPlatform.surface_creation_does_not_leak_fds:StreamTransportTest/0.SendsFullMessagesWhenInterrupted:BufferQueue/WithTwoOrMoreBuffers.client_framerate_matches_compositor*:BufferQueue/WithThreeOrMoreBuffers.slow_client_framerate_matches_compositor*:BufferQueue/WithThreeOrMoreBuffers.queue_size_scales_with_client_performance*")
59 endif()
60
61 if(SYSTEM_SUPPORTS_O_TMPFILE EQUAL 1)
62
63=== modified file 'tools/tsan-suppressions'
64--- tools/tsan-suppressions 2015-07-17 05:46:53 +0000
65+++ tools/tsan-suppressions 2015-12-04 15:08:42 +0000
66@@ -1,7 +1,4 @@
67 race:libglib*
68 race:libgio*
69 race:testing::internal::FunctionMockerBase
70-race:~posix_event
71-race:boost::asio::detail::posix_event::signal_and_unlock
72-#Suppress false positive when deleting main loop shared_ptr
73-race:std::__shared_ptr<mir::MainLoop, (__gnu_cxx::_Lock_policy)2>::operator->()
74+race:std::__future_base::_State_baseV2::wait()
75\ No newline at end of file

Subscribers

People subscribed via source and target branches