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

Add support for thread sanitizer (TSAN) via --with-thread-sanitizer #112536

Closed
Tracked by #108219
colesbury opened this issue Nov 29, 2023 · 11 comments
Closed
Tracked by #108219

Add support for thread sanitizer (TSAN) via --with-thread-sanitizer #112536

colesbury opened this issue Nov 29, 2023 · 11 comments
Assignees
Labels
3.13 bugs and security fixes build The build process and cross-build topic-free-threading type-feature A feature request or enhancement

Comments

@colesbury
Copy link
Contributor

colesbury commented Nov 29, 2023

Feature or enhancement

GCC and Clang provide ThreadSanitizer, a tool that detects data races. We should add support for thread sanitizer to CPython. Note that Python already supports the memory and address sanitizers.

  • Add --with-thread-sanitizer as a configure option
  • Define _Py_THREAD_SANITIZER if thread-sanitizer is enabled (see, e.g., _Py_ADDRESS_SANITIZER in pyport.h)
  • Move the definition of _Py_NO_SANITIZE_THREAD from obmalloc.c to a place that's more widely available (like pyport.h). We're going to need this in a few places

Eventually, it would be helpful to have a continuous build for the combination of --disable-gil --with-thread-sanitizer. Note that we probably won't want to run all the tests. ThreadSanitizer is slow and also not very useful for single-threaded tests. We should collect a subset of our tests that use threading for the ThreadSanitizer continuous build.

Linked PRs

@samety
Copy link
Contributor

samety commented Dec 3, 2023

Hey,
I would like to pick this up.
As a first step tried copying the struct for ASAN to TSAN
Here is a draft PR: #112648

Happy to get some initial feedback on it and then add remaining things like _Py_THREAD_SANITIZER etc.

@samety
Copy link
Contributor

samety commented Dec 17, 2023

Added --with-thread-sanitizer as a configure option in pr 112648.

Still looking for a good way to enable TSAN for the continuous build, I will make another PR when I find one.

@erlend-aasland erlend-aasland added the build The build process and cross-build label Jan 28, 2024
corona10 added a commit to corona10/cpython that referenced this issue Mar 10, 2024
corona10 added a commit to corona10/cpython that referenced this issue Mar 10, 2024
corona10 added a commit to corona10/cpython that referenced this issue Mar 11, 2024
corona10 added a commit to corona10/cpython that referenced this issue Mar 13, 2024
corona10 added a commit to corona10/cpython that referenced this issue Mar 15, 2024
corona10 added a commit to corona10/cpython that referenced this issue Mar 15, 2024
corona10 added a commit to corona10/cpython that referenced this issue Mar 15, 2024
@pitrou
Copy link
Member

pitrou commented Mar 16, 2024

Do we want to backport these at some point?

@corona10
Copy link
Member

corona10 commented Mar 16, 2024

@pitrou We can go for the default build. I will submit the PR soon

pitrou pushed a commit to pitrou/cpython that referenced this issue Mar 18, 2024
…16872)

(cherry picked from commit 20578a1)

Co-authored-by: Donghee Na <donghee.na@python.org>
pitrou added a commit that referenced this issue Mar 18, 2024
(cherry picked from commit 20578a1)

Co-authored-by: Donghee Na <donghee.na@python.org>
@corona10
Copy link
Member

@pitrou cc @colesbury
By the way, once the backport patches are completed, let's close this issue.
Expanding TSAN test can be tracked from the new issue.
(I prefer to cut off the issues as the checkpoint)

@pitrou
Copy link
Member

pitrou commented Mar 19, 2024

Ok, closing as done now.

@pitrou pitrou closed this as completed Mar 19, 2024
vstinner pushed a commit to vstinner/cpython that referenced this issue Mar 20, 2024
vstinner pushed a commit to vstinner/cpython that referenced this issue Mar 20, 2024
---------

Co-authored-by: Antoine Pitrou <antoine@python.org>
vstinner pushed a commit to vstinner/cpython that referenced this issue Mar 20, 2024
These may all exercise some non-trivial aspects of thread synchronization.
adorilson pushed a commit to adorilson/cpython that referenced this issue Mar 25, 2024
adorilson pushed a commit to adorilson/cpython that referenced this issue Mar 25, 2024
adorilson pushed a commit to adorilson/cpython that referenced this issue Mar 25, 2024
---------

Co-authored-by: Antoine Pitrou <antoine@python.org>
adorilson pushed a commit to adorilson/cpython that referenced this issue Mar 25, 2024
These may all exercise some non-trivial aspects of thread synchronization.
colesbury added a commit to colesbury/cpython that referenced this issue Apr 9, 2024
…abled

The `__has_feature(thread_sanitizer)` is a Clang-ism. Although new
versions of GCC implement `__has_feature`, the `defined(__has_feature)`
check still fails on GCC so we don't use that code path.
colesbury added a commit that referenced this issue Apr 10, 2024
…117702)

The `__has_feature(thread_sanitizer)` is a Clang-ism. Although new
versions of GCC implement `__has_feature`, the `defined(__has_feature)`
check still fails on GCC so we don't use that code path.
miss-islington pushed a commit to miss-islington/cpython that referenced this issue Apr 10, 2024
…abled (pythonGH-117702)

The `__has_feature(thread_sanitizer)` is a Clang-ism. Although new
versions of GCC implement `__has_feature`, the `defined(__has_feature)`
check still fails on GCC so we don't use that code path.
(cherry picked from commit 79eec66)

Co-authored-by: Sam Gross <colesbury@gmail.com>
colesbury added a commit that referenced this issue Apr 10, 2024
…nabled (GH-117702) (#117713)

gh-112536: Define `_Py_THREAD_SANITIZER` on GCC when TSan is enabled (GH-117702)

The `__has_feature(thread_sanitizer)` is a Clang-ism. Although new
versions of GCC implement `__has_feature`, the `defined(__has_feature)`
check still fails on GCC so we don't use that code path.
(cherry picked from commit 79eec66)

Co-authored-by: Sam Gross <colesbury@gmail.com>
diegorusso pushed a commit to diegorusso/cpython that referenced this issue Apr 17, 2024
diegorusso pushed a commit to diegorusso/cpython that referenced this issue Apr 17, 2024
diegorusso pushed a commit to diegorusso/cpython that referenced this issue Apr 17, 2024
diegorusso pushed a commit to diegorusso/cpython that referenced this issue Apr 17, 2024
---------

Co-authored-by: Antoine Pitrou <antoine@python.org>
diegorusso pushed a commit to diegorusso/cpython that referenced this issue Apr 17, 2024
These may all exercise some non-trivial aspects of thread synchronization.
diegorusso pushed a commit to diegorusso/cpython that referenced this issue Apr 17, 2024
…abled (python#117702)

The `__has_feature(thread_sanitizer)` is a Clang-ism. Although new
versions of GCC implement `__has_feature`, the `defined(__has_feature)`
check still fails on GCC so we don't use that code path.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.13 bugs and security fixes build The build process and cross-build topic-free-threading type-feature A feature request or enhancement
Projects
None yet
Development

No branches or pull requests

5 participants