Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

gh-112536: Set up TSAN CI for free-threading #116555

Closed
wants to merge 19 commits into from

Conversation

corona10
Copy link
Member

@corona10 corona10 commented Mar 10, 2024

@corona10

This comment was marked as resolved.

@corona10 corona10 marked this pull request as draft March 10, 2024 05:36
@corona10
Copy link
Member Author

corona10 commented Mar 10, 2024

Hmm, I locally passed the thread-sanitizer test with clang, I am taking a look why CI has an issue.

python.exe(48341,0x1e0bcdc40) malloc: nano zone abandoned due to inability to reserve vm space.
Using random seed: 2688388199
Raised RLIMIT_NOFILE: 256 -> 1024
0:00:00 load avg: 7.94 Run 44 tests in parallel using 4 worker processes
0:00:01 load avg: 7.94 [ 1/44] test_binop passed
0:00:02 load avg: 7.94 [ 2/44] test_binascii passed
0:00:04 load avg: 7.94 [ 3/44] test_bisect passed
0:00:07 load avg: 7.94 [ 4/44] test_base64 passed
0:00:09 load avg: 7.94 [ 5/44] test_array passed
0:00:09 load avg: 7.71 [ 6/44] test_cmath passed
0:00:14 load avg: 7.71 [ 7/44] test_bz2 passed
0:00:18 load avg: 7.81 [ 8/44] test_complex passed
0:00:24 load avg: 7.82 [ 9/44] test_dataclasses passed
0:00:24 load avg: 7.60 [10/44] test_codecs passed
0:00:26 load avg: 7.60 [11/44] test_collections passed
0:00:41 load avg: 7.10 [12/44] test_difflib passed
0:00:42 load avg: 7.10 [13/44] test_bytes passed
0:00:46 load avg: 7.33 [14/44] test_float passed

@corona10
Copy link
Member Author

checking for builtin __atomic_load_n and __atomic_store_n functions... no
configure: error: --disable-gil requires mimalloc memory allocator (--with-mimalloc).

Weired..

@corona10
Copy link
Member Author

corona10 commented Mar 10, 2024

I need to check on my Linux machine later.

./Include/cpython/pyatomic_gcc.h:531:3: warning: ‘atomic_thread_fence’ is not supported with ‘-fsanitize=thread’ [-Wtsan]
  531 | { __atomic_thread_fence(__ATOMIC_SEQ_CST); }

Comment on lines +485 to +488
- name: Set up GCC-10 for ASAN
uses: egor-tensin/setup-gcc@v1
with:
version: 11
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The version says "11" but the step name says "GCC-10"

- name: Display build info
run: make pythoninfo
- name: Tests
run: ./python -m test --pgo -j4 # Reduce test scope
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we'll want a configuration specifically for tsan. Maybe a --tsan option? We'll only want to run tests that actually use threading.

Brett had a list of tests that fail with PYTHON_GIL=0 -- those are probably a good starting point:

swtaarrs@1fe9165#diff-590c02490d066378045dd48d62e86dc85d3fb4ad934b6a2b2b0b1112e59eaefb

@@ -446,6 +446,7 @@ _PyMem_ArenaFree(void *Py_UNUSED(ctx), void *ptr,
/***************************/

static int
_Py_NO_SANITIZE_THREAD
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've had a better experience using suppression lists rather than marking functions as _Py_NO_SANITIZE_THREAD:

  1. Add a file supressions.txt somwhere (maybe Tools/tsan/supressions.txt?)
  2. Set the environment variable TSAN_OPTIONS="suppressions=<path_to_supressions.txt>"

See https://github.com/google/sanitizers/wiki/ThreadSanitizerSuppressions

with:
version: 11
- name: Configure CPython
run: ./configure --disable-gil --with-thread-sanitizer
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we want --with-pydebug?

@corona10
Copy link
Member Author

Actual PR for this issue will be: #116872

@corona10 corona10 closed this Mar 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants