Skip to content

Commit 042c262

Browse files
authored
Merge pull request ERGO-Code#2301 from ERGO-Code/data-races-10
Fix data races in thread sanitizer
2 parents 3d0e800 + a3ccd3f commit 042c262

File tree

8 files changed

+310
-29
lines changed

8 files changed

+310
-29
lines changed

.bazelrc

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
build:asan --strip=never
2+
build:asan --copt -fsanitize=address
3+
build:asan --copt -O1
4+
build:asan --copt -g
5+
build:asan --copt -fno-omit-frame-pointer
6+
build:asan --linkopt -fsanitize=address
7+
8+
build:tsan --strip=never
9+
build:tsan --copt -fsanitize=thread
10+
build:tsan --copt -O1
11+
build:tsan --copt -g
12+
build:tsan --copt -fno-omit-frame-pointer
13+
build:tsan --linkopt -fsanitize=thread
14+
15+
build:lsan --strip=never
16+
build:lsan --copt -fsanitize=leak
17+
build:lsan --copt -O1
18+
build:lsan --copt -g
19+
build:lsan --copt -fno-omit-frame-pointer
20+
build:lsan --linkopt -fsanitize=leak
21+
22+
build:ubsan --strip=never
23+
build:ubsan --copt -fsanitize=undefined
24+
build:ubsan --copt -O1
25+
build:ubsan --copt -g
26+
build:ubsan --copt -fno-omit-frame-pointer
27+
build:ubsan --linkopt -fsanitize=undefined

.github/workflows/build-bazel.yml

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,13 @@ jobs:
2121
run: bazel build //...
2222

2323
- name: test all
24-
run: bazel test //...
24+
run: bazel test --test_output=all //...
2525

2626
- name: test example
27-
run: ./bazel-bin/call-highs-example
27+
run: ./bazel-bin/call-highs-example
28+
29+
- name: Upload bazel-testlogs
30+
uses: actions/upload-artifact@v4
31+
with:
32+
name: bazel-testlogs
33+
path: bazel-testlogs/
Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,108 @@
1+
name: sanitizers-bazel
2+
3+
on: [push, pull_request]
4+
5+
jobs:
6+
asan:
7+
runs-on: ${{ matrix.os }}
8+
strategy:
9+
matrix:
10+
os: [ubuntu-latest]
11+
12+
steps:
13+
- uses: actions/checkout@v4
14+
15+
- uses: bazelbuild/setup-bazelisk@v3
16+
17+
- name: Bazel clean
18+
run: bazel clean
19+
20+
- name: Bazel build
21+
run: bazel build -c dbg --config=asan //...
22+
23+
- name: Bazel test
24+
run: bazel test -c dbg --config=asan --runs_per_test 100 //...
25+
26+
- name: Upload bazel-testlogs
27+
uses: actions/upload-artifact@v4
28+
with:
29+
name: bazel-testlogs-asan
30+
path: bazel-testlogs/
31+
32+
tsan:
33+
runs-on: ${{ matrix.os }}
34+
strategy:
35+
matrix:
36+
os: [ubuntu-latest]
37+
38+
steps:
39+
- uses: actions/checkout@v4
40+
41+
- uses: bazelbuild/setup-bazelisk@v3
42+
43+
- name: Bazel clean
44+
run: bazel clean
45+
46+
- name: Bazel build
47+
run: bazel build -c dbg --config=tsan //...
48+
49+
- name: Bazel test
50+
run: bazel test -c dbg --config=tsan --runs_per_test 100 //...
51+
52+
- name: Upload bazel-testlogs
53+
uses: actions/upload-artifact@v4
54+
with:
55+
name: bazel-testlogs-tsan
56+
path: bazel-testlogs/
57+
58+
lsan:
59+
runs-on: ${{ matrix.os }}
60+
strategy:
61+
matrix:
62+
os: [ubuntu-latest]
63+
64+
steps:
65+
- uses: actions/checkout@v4
66+
67+
- uses: bazelbuild/setup-bazelisk@v3
68+
69+
- name: Bazel clean
70+
run: bazel clean
71+
72+
- name: Bazel build
73+
run: bazel build -c dbg --config=lsan //...
74+
75+
- name: Bazel test
76+
run: bazel test -c dbg --config=lsan --runs_per_test 100 //...
77+
78+
- name: Upload bazel-testlogs
79+
uses: actions/upload-artifact@v4
80+
with:
81+
name: bazel-testlogs-lsan
82+
path: bazel-testlogs/
83+
84+
ubsan:
85+
runs-on: ${{ matrix.os }}
86+
strategy:
87+
matrix:
88+
os: [ubuntu-latest]
89+
90+
steps:
91+
- uses: actions/checkout@v4
92+
93+
- uses: bazelbuild/setup-bazelisk@v3
94+
95+
- name: Bazel clean
96+
run: bazel clean
97+
98+
- name: Bazel build
99+
run: bazel build -c dbg --config=ubsan //...
100+
101+
- name: Bazel test
102+
run: bazel test -c dbg --config=ubsan --runs_per_test 100 //...
103+
104+
- name: Upload bazel-testlogs
105+
uses: actions/upload-artifact@v4
106+
with:
107+
name: bazel-testlogs-ubsan
108+
path: bazel-testlogs/

.github/workflows/sanitizers-cmake.yml

Lines changed: 57 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,59 @@
11
name: sanitizers-cmake
2-
on: [] #push
2+
on: [push, pull_request]
33

44
jobs:
5-
sanitizer_release:
5+
gcc_relwithdebinfo:
6+
runs-on: ${{ matrix.os }}
7+
strategy:
8+
fail-fast: false
9+
matrix:
10+
os: [ubuntu-latest]
11+
sanitizer: [Address, Thread, Leak]
12+
steps:
13+
- uses: actions/checkout@v4
14+
15+
- name: Create Build Environment
16+
run: cmake -E make_directory ${{runner.workspace}}/build
17+
18+
- name: Configure CMake and Build
19+
shell: bash
20+
working-directory: ${{runner.workspace}}/build
21+
run: |
22+
cmake $GITHUB_WORKSPACE -DCMAKE_BUILD_TYPE=RelWithDebInfo -DDEBUG_MEMORY=${{ matrix.sanitizer }} -DCMAKE_C_COMPILER=gcc -DCMAKE_CXX_COMPILER=g++
23+
cmake --build . --parallel
24+
25+
- name: Run
26+
working-directory: ${{runner.workspace}}/build
27+
shell: bash
28+
run: ./bin/highs $GITHUB_WORKSPACE/check/instances/afiro.mps
29+
30+
gcc_debug:
31+
runs-on: ${{ matrix.os }}
32+
strategy:
33+
fail-fast: false
34+
matrix:
35+
# os: [ubuntu-latest, macos-latest]
36+
os: [ubuntu-latest]
37+
sanitizer: [Address, Thread, Leak]
38+
steps:
39+
- uses: actions/checkout@v4
40+
41+
- name: Create Build Environment
42+
run: cmake -E make_directory ${{runner.workspace}}/build
43+
44+
- name: Configure CMake and Build
45+
shell: bash
46+
working-directory: ${{runner.workspace}}/build
47+
run: |
48+
cmake $GITHUB_WORKSPACE -DCMAKE_BUILD_TYPE=Debug -DDEBUG_MEMORY=${{ matrix.sanitizer }} -DCMAKE_C_COMPILER=gcc -DCMAKE_CXX_COMPILER=g++
49+
cmake --build . --parallel
50+
51+
- name: Run
52+
working-directory: ${{runner.workspace}}/build
53+
shell: bash
54+
run: ./bin/highs $GITHUB_WORKSPACE/check/instances/afiro.mps
55+
56+
clang_relwithdebinfo:
657
runs-on: ${{ matrix.os }}
758
strategy:
859
fail-fast: false
@@ -20,15 +71,15 @@ jobs:
2071
shell: bash
2172
working-directory: ${{runner.workspace}}/build
2273
run: |
23-
cmake $GITHUB_WORKSPACE -DCMAKE_BUILD_TYPE=RelWithDebInfo -DDEBUG_MEMORY=${{ matrix.sanitizer }}
74+
cmake $GITHUB_WORKSPACE -DCMAKE_BUILD_TYPE=RelWithDebInfo -DDEBUG_MEMORY=${{ matrix.sanitizer }} -DCMAKE_C_COMPILER=clang -DCMAKE_CXX_COMPILER=clang++
2475
cmake --build . --parallel
2576
2677
- name: Run
2778
working-directory: ${{runner.workspace}}/build
2879
shell: bash
2980
run: ./bin/highs $GITHUB_WORKSPACE/check/instances/afiro.mps
3081

31-
sanitizer_debug:
82+
clang_debug:
3283
runs-on: ${{ matrix.os }}
3384
strategy:
3485
fail-fast: false
@@ -46,10 +97,10 @@ jobs:
4697
shell: bash
4798
working-directory: ${{runner.workspace}}/build
4899
run: |
49-
cmake $GITHUB_WORKSPACE -DCMAKE_BUILD_TYPE=Debug -DDEBUG_MEMORY=${{ matrix.sanitizer }}
100+
cmake $GITHUB_WORKSPACE -DCMAKE_BUILD_TYPE=Debug -DDEBUG_MEMORY=${{ matrix.sanitizer }} -DCMAKE_C_COMPILER=clang -DCMAKE_CXX_COMPILER=clang++
50101
cmake --build . --parallel
51102
52103
- name: Run
53104
working-directory: ${{runner.workspace}}/build
54105
shell: bash
55-
run: ./bin/highs $GITHUB_WORKSPACE/check/instances/afiro.mps
106+
run: ./bin/highs $GITHUB_WORKSPACE/check/instances/afiro.mps

BUILD.bazel

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,26 @@ cc_library(
7373
],
7474
)
7575

76+
cc_library(
77+
name = "highs-runtime-opts",
78+
hdrs = ["app/HighsRuntimeOptions.h",
79+
"app/CLI11.hpp"],
80+
visibility = ["//visibility:public"],
81+
deps = [
82+
"//:highs",
83+
],
84+
)
85+
86+
cc_binary(
87+
name = "highs-bin",
88+
srcs = ["app/RunHighs.cpp"],
89+
visibility = ["//visibility:public"],
90+
deps = [
91+
"//:highs",
92+
"//:highs-runtime-opts",
93+
],
94+
)
95+
7696
cc_binary(
7797
name = "call-highs-example",
7898
srcs = ["examples/call_highs_from_cpp.cpp"],

CMakeLists.txt

Lines changed: 53 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -520,6 +520,12 @@ set(CPACK_PACKAGE_VENDOR "University of Edinburgh")
520520

521521
configure_file(${PROJECT_SOURCE_DIR}/highs/HConfig.h.in ${HIGHS_BINARY_DIR}/HConfig.h)
522522

523+
if (NOT DEBUG_MEMORY STREQUAL "Off")
524+
if (CMAKE_CXX_COMPILER_ID STREQUAL "Clang")
525+
add_compile_options($<$<AND:$<OR:$<CONFIG:Debug>,$<CONFIG:RelWithDebInfo>>,$<OR:$<COMPILE_LANGUAGE:C>,$<COMPILE_LANGUAGE:CXX>>>:-fno-sanitize=function,vptr>)
526+
endif()
527+
endif()
528+
523529
if (DEBUG_MEMORY STREQUAL "Address")
524530
set (CMAKE_CXX_FLAGS_RELWITHDEBINFO "${CMAKE_CXX_FLAGS_RELWITHDEBINFO} \
525531
-fsanitize=address,undefined \
@@ -530,6 +536,11 @@ if (DEBUG_MEMORY STREQUAL "Address")
530536
-fno-omit-frame-pointer \
531537
-fsanitize-address-use-after-scope")
532538

539+
# add_compile_options("-fsanitize=address,undefined")
540+
# add_compile_options("-fno-omit-frame-pointer")
541+
# add_link_options("-fsanitize=address,undefined")
542+
# add_link_options("-lubsan")
543+
533544
set (CMAKE_CXX_FLAGS_DEBUG "${CMAKE_CXX_FLAGS_DEBUG} \
534545
-fsanitize=address,undefined \
535546
-fno-omit-frame-pointer \
@@ -540,34 +551,56 @@ if (DEBUG_MEMORY STREQUAL "Address")
540551
-fno-optimize-sibling-calls ")
541552

542553
elseif (DEBUG_MEMORY STREQUAL "Thread")
543-
set (CMAKE_CXX_FLAGS_RELWITHDEBINFO "${CMAKE_CXX_FLAGS_RELWITHDEBINFO} \
544-
-fsanitize=thread,undefined \
554+
set (CMAKE_CXX_FLAGS_RELWITHDEBINFO "${CMAKE_CXX_FLAGS_RELWITHDEBINFO} -fsanitize=thread,undefined \
545555
-fno-omit-frame-pointer")
546-
set (CMAKE_EXE_LINKER_FLAGS_RELWITHDEBINFO "${CMAKE_EXE_LINKER_FLAGS_RELWITHDEBINFO} \
547-
-fsanitize=thread,undefined \
556+
set (CMAKE_EXE_LINKER_FLAGS_RELWITHDEBINFO "${CMAKE_EXE_LINKER_FLAGS_RELWITHDEBINFO} -fsanitize=thread,undefined \
548557
-fno-omit-frame-pointer")
549558

550-
set (CMAKE_CXX_FLAGS_DEBUG "${CMAKE_CXX_FLAGS_DEBUG} \
551-
-fsanitize=thread,undefined \
559+
set (CMAKE_CXX_FLAGS_DEBUG "${CMAKE_CXX_FLAGS_DEBUG} -fsanitize=thread \
552560
-fno-omit-frame-pointer ")
553-
set (CMAKE_EXE_LINKER_FLAGS_DEBUG "${CMAKE_EXE_LINKER_FLAGS_DEBUG} \
554-
-fsanitize=thread,undefined \
561+
set (CMAKE_EXE_LINKER_FLAGS_DEBUG "${CMAKE_EXE_LINKER_FLAGS_DEBUG} -fsanitize=thread \
555562
-fno-omit-frame-pointer ")
556563

557564
elseif (DEBUG_MEMORY STREQUAL "Leak")
558-
set (CMAKE_CXX_FLAGS_RELWITHDEBINFO "${CMAKE_CXX_FLAGS_RELWITHDEBINFO} \
559-
-fsanitize=leak,undefined \
560-
-fno-omit-frame-pointer")
561-
set (CMAKE_EXE_LINKER_FLAGS_RELWITHDEBINFO "${CMAKE_EXE_LINKER_FLAGS_RELWITHDEBINFO} \
562-
-fsanitize=leak,undefined \
563-
-fno-omit-frame-pointer")
565+
if (CMAKE_CXX_COMPILER_ID STREQUAL "Clang")
566+
set (CMAKE_CXX_FLAGS_RELWITHDEBINFO "${CMAKE_CXX_FLAGS_RELWITHDEBINFO} \
567+
-fsanitize=leak \
568+
-fno-omit-frame-pointer")
569+
set (CMAKE_EXE_LINKER_FLAGS_RELWITHDEBINFO "${CMAKE_EXE_LINKER_FLAGS_RELWITHDEBINFO} \
570+
-fsanitize=leak \
571+
-fno-omit-frame-pointer")
572+
set (CMAKE_CXX_FLAGS_DEBUG "${CMAKE_CXX_FLAGS_DEBUG} -fsanitize=leak \
573+
-fno-omit-frame-pointer ")
574+
set (CMAKE_EXE_LINKER_FLAGS_DEBUG "${CMAKE_EXE_LINKER_FLAGS_DEBUG} -fsanitize=leak \
575+
-fno-omit-frame-pointer ")
576+
else()
577+
set (CMAKE_CXX_FLAGS_RELWITHDEBINFO "${CMAKE_CXX_FLAGS_RELWITHDEBINFO} \
578+
-fsanitize=leak,undefined \
579+
-fno-omit-frame-pointer")
580+
set (CMAKE_EXE_LINKER_FLAGS_RELWITHDEBINFO "${CMAKE_EXE_LINKER_FLAGS_RELWITHDEBINFO} \
581+
-fsanitize=leak,undefined \
582+
-fno-omit-frame-pointer")
583+
set (CMAKE_CXX_FLAGS_DEBUG "${CMAKE_CXX_FLAGS_DEBUG} -fsanitize=leak,undefined \
584+
-fno-omit-frame-pointer ")
585+
set (CMAKE_EXE_LINKER_FLAGS_DEBUG "${CMAKE_EXE_LINKER_FLAGS_DEBUG} -fsanitize=leak,undefined \
586+
-fno-omit-frame-pointer ")
587+
endif()
588+
elseif (DEBUG_MEMORY STREQUAL "Memory")
589+
if (CMAKE_CXX_COMPILER_ID STREQUAL "Clang")
590+
set (CMAKE_CXX_FLAGS_RELWITHDEBINFO "${CMAKE_CXX_FLAGS_RELWITHDEBINFO} \
591+
-fsanitize=memory \
592+
-fno-omit-frame-pointer")
593+
set (CMAKE_EXE_LINKER_FLAGS_RELWITHDEBINFO "${CMAKE_EXE_LINKER_FLAGS_RELWITHDEBINFO} \
594+
-fsanitize=memory \
595+
-fno-omit-frame-pointer")
596+
set (CMAKE_CXX_FLAGS_DEBUG "${CMAKE_CXX_FLAGS_DEBUG} -fsanitize=memory \
597+
-fno-omit-frame-pointer ")
598+
set (CMAKE_EXE_LINKER_FLAGS_DEBUG "${CMAKE_EXE_LINKER_FLAGS_DEBUG} -fsanitize=memory \
599+
-fno-omit-frame-pointer ")
564600

565-
set (CMAKE_CXX_FLAGS_DEBUG "${CMAKE_CXX_FLAGS_DEBUG} \
566-
-fsanitize=leak,undefined \
567-
-fno-omit-frame-pointer ")
568-
set (CMAKE_EXE_LINKER_FLAGS_DEBUG "${CMAKE_EXE_LINKER_FLAGS_DEBUG} \
569-
-fsanitize=leak,undefined \
570-
-fno-omit-frame-pointer ")
601+
else()
602+
message(FATAL_ERROR "Memory sanitizer only available with clang.")
603+
endif()
571604
endif()
572605

573606
# HiGHS coverage update in progress

check/TestHighsParallel.cpp

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,10 +24,18 @@ int64_t fib(const int64_t n) {
2424
if (n <= 20) return fib_sequential(n);
2525

2626
int64_t n1;
27-
parallel::spawn([&]() { n1 = fib(n - 1); });
27+
parallel::spawn([&]() {
28+
n1 = fib(n - 1);
29+
TSAN_ANNOTATE_HAPPENS_BEFORE(&n1);
30+
});
31+
2832
int64_t n2 = fib(n - 2);
33+
TSAN_ANNOTATE_HAPPENS_BEFORE(&n2);
34+
2935
parallel::sync();
3036

37+
TSAN_ANNOTATE_HAPPENS_AFTER(&n1);
38+
TSAN_ANNOTATE_HAPPENS_AFTER(&n2);
3139
// printf("fib(%ld) = %ld + %ld = %ld\n", n, n1, n2, n1 + n2);
3240
return n1 + n2;
3341
}

0 commit comments

Comments
 (0)