Skip to content

Commit 928a41d

Browse files
da-woodsscoder
authored andcommitted
Add address/undefined behaviour/thread sanitizer tests (#7334)
Adapt/re-use the pydebug test run to also add test with the various clang sanitizers. Address sanitizer and undefined behaviour sanitizer are run in parallel to save a little time. Thread sanitizer is run individual for both regular and freethreading builds. At present I don't think thread sanitizer is hugely useful (since the vast majority of tests run single-threaded in series). I have some plans to improve that in future. Address sanitizer will stop if it hits an error. The others will keep going and write to some log files (and examination of those log files triggers pass/failure)
1 parent 7eb8405 commit 928a41d

File tree

12 files changed

+260
-25
lines changed

12 files changed

+260
-25
lines changed
Lines changed: 145 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,145 @@
1+
name: Run with compiled Python
2+
3+
on:
4+
workflow_call:
5+
inputs:
6+
sanitize:
7+
required: false
8+
default:
9+
type: string
10+
compiler:
11+
required: false
12+
default:
13+
type: string
14+
cpp_compiler:
15+
required: false
16+
default:
17+
type: string
18+
name:
19+
required: true
20+
type: string
21+
22+
jobs:
23+
do_run:
24+
name: ${{inputs.name}}
25+
runs-on: ubuntu-latest
26+
27+
env:
28+
BACKEND: c,cpp
29+
PYTHON_VERSION: 3.x-dev
30+
CONFIGURE_ARGS: --with-pydebug
31+
SANITIZER_CFLAGS: ""
32+
33+
steps:
34+
- name: Checkout repo
35+
uses: actions/checkout@v5
36+
37+
- name: Set compiler
38+
if: ${{inputs.compiler}}
39+
run: |
40+
CC=${{inputs.compiler}}
41+
CXX=${{inputs.cpp_compiler}}
42+
echo EXTERNAL_OVERRIDE_CC=1 >> $GITHUB_ENV
43+
clangv=$($CC -v 2> >(grep "clang version"))
44+
echo $clangv
45+
if [[ $clangv == *"version 18"* && "${{inputs.sanitize}}" == *"thread"* ]]; then
46+
# Python uses clang-17 instead of 18 because of bugs so do the same
47+
CC=clang-17
48+
CXX=clang-17
49+
fi
50+
echo "CC=$CC" >> $GITHUB_ENV
51+
echo "CXX=$CXX" >> $GITHUB_ENV
52+
53+
54+
- name: Set up sanitizer args
55+
if: ${{inputs.sanitize}}
56+
run: |
57+
SANITIZER_CFLAGS=""
58+
CONFIGURE_ARGS=""
59+
EXTRA_CONFIGURE_CFLAGS=""
60+
EXCLUDE="run[.] memoryview[.] --no-examples"
61+
if [[ "${{inputs.sanitize}}" == *"address"* ]]; then
62+
CONFIGURE_ARGS="$CONFIGURE_ARGS --with-address-sanitizer --without-pymalloc"
63+
SANITIZER_CFLAGS="$SANITIZER_CFLAGS -fsanitize=address"
64+
echo "ASAN_OPTIONS=detect_leaks=false log_path=${{ github.workspace }}/san_log" >> $GITHUB_ENV
65+
fi
66+
# TODO - memory sanitizer requires rebuilding almost all of CPython's dependencies
67+
# with memory sanitizer too, so isn't really usable for us.
68+
if [[ "${{inputs.sanitize}}" == *"memory"* ]]; then
69+
CONFIGURE_ARGS="$CONFIGURE_ARGS --with-memory-sanitizer"
70+
SANITIZER_CFLAGS="$SANITIZER_CFLAGS -fsanitize=memory"
71+
fi
72+
if [[ "${{inputs.sanitize}}" == *"undefined"* ]]; then
73+
CONFIGURE_ARGS="$CONFIGURE_ARGS --with-undefined-behavior-sanitizer"
74+
# We call functions through slightly incorrect pointer types a lot so disable this check for now for now
75+
EXTRA_CONFIGURE_CFLAGS="$EXTRA_CONFIGURE_CFLAGS -fno-sanitize=function"
76+
# omit vptr because it's largely C++-only and requires linking with clang++ (which breaks other things)
77+
SANITIZER_CFLAGS="$SANITIZER_CFLAGS -fsanitize=undefined -fno-sanitize=function -fno-sanitize=vptr -fno-omit-frame-pointer"
78+
echo "print_stacktrace=1" >> $GITHUB_ENV
79+
EXCLUDE="$EXCLUDE --excludefile tests/ubsan_bugs.txt"
80+
echo "UBSAN_OPTIONS=log_path=${{ github.workspace }}/san_log" >> $GITHUB_ENV
81+
fi
82+
if [[ "${{inputs.sanitize}}" == *"thread"* ]]; then
83+
CONFIGURE_ARGS="$CONFIGURE_ARGS --with-thread-sanitizer"
84+
SANITIZER_CFLAGS="$SANITIZER_CFLAGS -fsanitize=thread"
85+
if [[ "${{inputs.sanitize}}" == *"ft"* ]]; then
86+
echo "PYTHON_VERSION=3.xt-dev" >> $GITHUB_ENV
87+
CONFIGURE_ARGS="$CONFIGURE_ARGS --disable-gil"
88+
TSAN_SUPPRESSIONS="${GITHUB_WORKSPACE}/cpython_main/Tools/tsan/suppressions_free_threading.txt"
89+
else
90+
TSAN_SUPPRESSIONS="${GITHUB_WORKSPACE}/cpython_main/Tools/tsan/suppressions.txt"
91+
fi
92+
if [[ "${{github.event.label.name}}" != "full_sanitizers" ]]; then
93+
# For thread sanitizer run on a much smaller list of tests
94+
EXCLUDE="tag:threads"
95+
fi
96+
EXCLUDE="$EXCLUDE --excludefile tests/tsan_bugs.txt"
97+
echo "TSAN_OPTIONS=suppressions=$TSAN_SUPPRESSIONS log_path=${{ github.workspace }}/san_log" >> $GITHUB_ENV
98+
# Having too many workers seems to lead to an exit without a diagnostic message - possibly memory?
99+
echo "TEST_PARALLELISM=-j3" >> $GITHUB_ENV
100+
fi
101+
# Even running Python like this is slow, so only run a subset of tests
102+
# (i.e. compile-only tests tell us nothing)
103+
echo "EXCLUDE=$EXCLUDE --no-refnanny" >> $GITHUB_ENV
104+
# https://github.com/google/sanitizers/issues/934
105+
echo "LD_PRELOAD=$(realpath "$(clang -print-file-name=libstdc++.so)")" >> $GITHUB_ENV
106+
echo "CONFIGURE_ARGS=$CONFIGURE_ARGS" >> $GITHUB_ENV
107+
echo "SANITIZER_CFLAGS=$SANITIZER_CFLAGS" >> $GITHUB_ENV
108+
echo "EXTRA_CONFIGURE_CFLAGS=$EXTRA_CONFIGURE_CFLAGS" >> $GITHUB_ENV
109+
110+
- name: Install build dependencies
111+
run: |
112+
sudo apt-get update -y -q
113+
sudo apt-get install -y -q libbz2-dev lzma-dev libreadline-dev libgmp-dev
114+
115+
- name: Build Python
116+
run: |
117+
git clone https://github.com/python/cpython/ cpython_main
118+
cd cpython_main
119+
./configure ${CONFIGURE_ARGS} --prefix=${GITHUB_WORKSPACE}/cpython_install CFLAGS="-O2 $EXTRA_CONFIGURE_CFLAGS"
120+
make -j8
121+
make install
122+
${GITHUB_WORKSPACE}/cpython_install/bin/python3 -m venv ${GITHUB_WORKSPACE}/venv_pydebug
123+
124+
- name: Run CI
125+
run: |
126+
cd "${GITHUB_WORKSPACE}/"
127+
source venv_pydebug/bin/activate
128+
bash ./Tools/ci-run.sh
129+
130+
- name: Archive logs
131+
if: ${{ inputs.sanitize && always() }}
132+
uses: actions/upload-artifact@v4
133+
with:
134+
name: ${{inputs.sanitize}}-logs
135+
path: san_log.*
136+
if-no-files-found: ignore
137+
138+
# The check of the sanitizer logs does a bit of filtering of unwanted diagnostics
139+
# and finishes with an error code if anything bad is found.
140+
- name: Check logs
141+
if: ${{ inputs.sanitize && always() }}
142+
run: |
143+
cd "${GITHUB_WORKSPACE}/"
144+
source venv_pydebug/bin/activate
145+
python Tools/examine_sanitizer_logs.py san_log.*

Cython/Utility/Exceptions.c

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,11 @@ if (likely(__Pyx_init_assertions_enabled() == 0)); else
1818
static int __pyx_assertions_enabled_flag;
1919
#define __pyx_assertions_enabled() (__pyx_assertions_enabled_flag)
2020

21+
#if __clang__ || __GNUC__
22+
// "Assertions enabled" may be written multiple times when using subinterpreters.
23+
// However, it should always be written to the same value to isn't a "real" race.
24+
__attribute__((no_sanitize("thread")))
25+
#endif
2126
static int __Pyx_init_assertions_enabled(void) {
2227
PyObject *builtins, *debug, *debug_str;
2328
int flag;

Cython/Utility/Optimize.c

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1191,6 +1191,13 @@ static {{c_ret_type}} __Pyx_Fallback_{{cfunc_name}}(PyObject *op1, PyObject *op2
11911191
}
11921192

11931193
#if CYTHON_USE_PYLONG_INTERNALS
1194+
{{if op == 'Lshift'}}
1195+
#if __clang__ || __GNUC__
1196+
// left-shift by more than the width of the number is undefined behaviour.
1197+
// We do check it (and test that it gives the right answer though).
1198+
__attribute__((no_sanitize("shift")))
1199+
#endif
1200+
{{endif}}
11941201
static {{c_ret_type}} __Pyx_Unpacked_{{cfunc_name}}(PyObject *op1, PyObject *op2, long intval, int inplace, int zerodivision_check) {
11951202
CYTHON_MAYBE_UNUSED_VAR(inplace);
11961203
CYTHON_UNUSED_VAR(zerodivision_check);

Tools/ci-run.sh

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ GCC_VERSION=${GCC_VERSION:=10}
77
# Set up compilers
88
if [[ $TEST_CODE_STYLE == "1" ]]; then
99
echo "Skipping compiler setup: Code style run"
10-
elif [[ $OSTYPE == "linux-gnu"* ]]; then
10+
elif [[ $OSTYPE == "linux-gnu"* && ! "$EXTERNAL_OVERRIDE_CC" ]]; then
1111
echo "Setting up linux compiler"
1212
echo "Installing requirements [apt]"
1313
sudo apt-add-repository -y "ppa:ubuntu-toolchain-r/test"
@@ -102,7 +102,7 @@ if [[ $PYTHON_VERSION != *"t" && $PYTHON_VERSION != *"t-dev" ]]; then
102102
fi
103103
if [[ $PYTHON_VERSION != *"-dev" ]]; then
104104
python -m pip install --pre -r test-requirements.txt || exit 1
105-
else
105+
elif [[ ! "$SANITIZER_CFLAGS" ]]; then
106106
# Install packages one by one, allowing failures due to missing recent wheels.
107107
cat test-requirements.txt | while read package; do python -m pip install --pre --only-binary ":all:" "$package" || true; done
108108
fi
@@ -146,7 +146,7 @@ if [[ $OSTYPE == "msys" ]]; then # for MSVC cl
146146
# (off by default) 5045 warns that the compiler will insert Spectre mitigations for memory load if the /Qspectre switch is specified
147147
# (off by default) 4820 warns about the code in Python\3.9.6\x64\include ...
148148
CFLAGS="-Od /Z7 /MP /W4 /wd4711 /wd4127 /wd5045 /wd4820"
149-
elif [[ $OSTYPE == "darwin"* ]]; then
149+
elif [[ $OSTYPE == "darwin"* || $CC == "clang" ]]; then
150150
CFLAGS="-O0 -g2 -Wall -Wextra -Wcast-qual -Wconversion -Wdeprecated -Wunused-result"
151151
else
152152
CFLAGS="-Og -g2 -Wall -Wextra -Wcast-qual -Wconversion -Wdeprecated -Wunused-result"
@@ -162,6 +162,10 @@ elif [[ $ODD_VERSION == "0" ]]; then
162162
CFLAGS="$CFLAGS -UNDEBUG"
163163
fi
164164

165+
if [[ "$SANITIZER_CFLAGS" ]]; then
166+
CFLAGS="$CFLAGS $SANITIZER_CFLAGS"
167+
fi
168+
165169
if [[ $NO_CYTHON_COMPILE != "1" && $PYTHON_VERSION != "pypy"* ]]; then
166170

167171
BUILD_CFLAGS="$CFLAGS -O2"
@@ -229,9 +233,13 @@ if [[ $COVERAGE == "1" ]]; then
229233
RUNTESTS_ARGS="$RUNTESTS_ARGS --coverage --coverage-html --coverage-md --cython-only"
230234
fi
231235
if [[ $TEST_CODE_STYLE != "1" ]]; then
232-
RUNTESTS_ARGS="$RUNTESTS_ARGS -j7"
236+
if [[ ! $TEST_PARALLELISM ]]; then
237+
TEST_PARALLELISM=-j7
238+
fi
239+
RUNTESTS_ARGS="$RUNTESTS_ARGS $TEST_PARALLELISM"
233240
fi
234241

242+
235243
if [[ $PYTHON_VERSION == "graalpy"* ]]; then
236244
# [DW] - the Graal JIT and Cython don't seem to get on too well. Disabling the
237245
# JIT actually makes it faster! And reduces the number of cores each process uses.

Tools/examine_sanitizer_logs.py

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
import sys
2+
import re
3+
4+
POS_MATCH = re.compile(r"^[^:]+:\d+:\d+: ")
5+
6+
def check_file(filename) -> int:
7+
failed_count = 0
8+
with open(filename) as f:
9+
for line in f:
10+
if not POS_MATCH.match(line) or line.startswith("WARNING: ThreadSanitizer"):
11+
continue
12+
if line.startswith("conftest.c"):
13+
# this is in the Python setup - we don't care
14+
continue
15+
if "applying zero offset to null pointer" in line:
16+
# This is OK in C++ and dropped in clang21 (on DW's laptop) so treat it as fine.
17+
continue
18+
# anything not specifically included is a failure
19+
failed_count += 1
20+
print(line)
21+
return failed_count
22+
23+
failed_count = 0
24+
25+
if len(sys.argv) == 2 and sys.argv[1].endswith('.*'):
26+
# No issues so the pattern has not expanded
27+
print(f"No logs found with pattern '{sys.argv[1]}'")
28+
exit(0)
29+
for arg in sys.argv[1:]:
30+
print(f"Looking at file '{arg}':")
31+
failed_count += check_file(arg)
32+
33+
exit(failed_count)

runtests.py

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2150,7 +2150,7 @@ def test_embed(self):
21502150

21512151
def load_listfile(filename):
21522152
# just reuse the FileListExclude implementation
2153-
return list(FileListExcluder(filename))
2153+
return FileListExcluder(filename)
21542154

21552155
class MissingDependencyExcluder(object):
21562156
def __init__(self, deps):
@@ -2451,6 +2451,10 @@ def main():
24512451
"--listfile", dest="listfile",
24522452
action="append",
24532453
help="specify a file containing a list of tests to run")
2454+
parser.add_argument(
2455+
"--excludefile", dest="excludefile",
2456+
action="append",
2457+
help="specify a file containing a list of tests to run")
24542458
parser.add_argument(
24552459
"-j", "--shard_count", dest="shard_count", metavar="N",
24562460
type=int, default=1,
@@ -2590,7 +2594,7 @@ def main():
25902594

25912595
if options.listfile:
25922596
for listfile in options.listfile:
2593-
cmd_args.extend(load_listfile(listfile))
2597+
cmd_args.extend(load_listfile(listfile).excludes.keys())
25942598

25952599
if options.capture and not options.for_debugging:
25962600
keep_alive_interval = 10
@@ -2914,6 +2918,10 @@ def runtests(options, cmd_args, coverage=None):
29142918
if options.exclude:
29152919
exclude_selectors += [ string_selector(r) for r in options.exclude ]
29162920

2921+
if options.excludefile:
2922+
for excludefile in options.excludefile:
2923+
exclude_selectors.append(load_listfile(excludefile))
2924+
29172925
if not COMPILER_HAS_INT128:
29182926
exclude_selectors += [RegExSelector('int128')]
29192927

0 commit comments

Comments
 (0)