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

Jump to bottom

runtime/race: update syso files with tsan atomics for And/Or #62624

Closed
mauri870 opened this issue Sep 13, 2023 · 11 comments
Closed

runtime/race: update syso files with tsan atomics for And/Or #62624

mauri870 opened this issue Sep 13, 2023 · 11 comments
Assignees
@mauri870 @cherrymui
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. NeedsFix The path to resolution is known, but the work has not been done. RaceDetector
Milestone
Go1.22

Comments

@mauri870
Copy link
Member

mauri870 commented Sep 13, 2023 •
edited

In order to implement the race variants for the new sync/atomic And/Or operators proposed in #61395 we need to build new syso files as well as implementing these atomic operations for the llvm thread sanitizer.

I already have a patch under review llvm/llvm-project#65695 that adds support for the new functions, but that is still hanging, awaiting to be merged. I would appreciate it if someone with closer ties to llvm could draw the maintainers attention to it.

Once the patch is merged we need to use x/build/cmd/racebuild with llvm HEAD to build the new syso files in runtime/race and commit them.
@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Sep 13, 2023
@mauri870
Copy link
Member Author

@gopherbot add NeedsFix, RaceDetector

@gopherbot gopherbot added NeedsFix The path to resolution is known, but the work has not been done. RaceDetector labels Sep 13, 2023
@mauri870
Copy link
Member Author

It's worth noting that previous updates to the syso files have caused issues, for example #37485. Since the race runtimes haven't been recompiled for almost a year we need to be careful when doing so.

@mauri870 mauri870 mentioned this issue Sep 13, 2023
Closed
@gopherbot
Copy link

Change https://go.dev/cl/528315 mentions this issue: runtime/internal/atomic: add 386/amd64 And/Or operators

@mknyszek mknyszek modified the milestones: Backlog, Go1.22 Sep 20, 2023
@mauri870
Copy link
Member Author

The upstream llvm support for the and/or tsan operators was cherry picked as llvm/llvm-project@51bfeff, I'll try to move forward with this as soon as I get gomote access.

@mauri870
Copy link
Member Author

cc @cherrymui

@cherrymui cherrymui self-assigned this Nov 11, 2023
@cherrymui
Copy link
Member

Working on it. Apparently there are a few things needed to be changed in racebuild, as the builders and environments changed. Hopefully I'll get this done soon (in a couple of days).

@mauri870
Copy link
Member Author

mauri870 commented Nov 14, 2023 •
edited

Thanks @cherrymui. Additionally, I remember reading somewhere (I can't seem to find the source now) that some platforms still rely on tsan v2 that don't have these new operators. Can you confirm that? And if so how to proceed? Thanks

Here is what I have on my notes (no source, sorry!)

On 1.19, the race detector has been upgraded to use thread sanitizer version v3 on all supported platforms except windows/amd64 and openbsd/amd64, which remain on v2. Race detector support for openbsd/amd64 has been removed from thread sanitizer upstream, so it is unlikely to ever be updated from v2.

@cherrymui
Copy link
Member

@mauri870 good point! windows/amd64 is already updated to v3 (https://go.dev/cl/420197). It is probably true that openbsd will remain v2. On openbsd, is it possible to write the new operations as a CAS loop and just use the CAS operation from TSAN? Thanks

@mauri870
Copy link
Member Author

Well it seems like we will have some additional work on openbsd, but it should be possible to have a race_openbsd_amd64.s that uses a tsan CAS, afaik race only works on openbsd/amd64.

@gopherbot
Copy link

Change https://go.dev/cl/543035 mentions this issue: runtime/race: update race syso files to support atomic And, Or

gopherbot pushed a commit that referenced this issue Nov 16, 2023
@cherrymui
runtime/race: update race syso files to support atomic And, Or
6b43a17 
TSAN recently got support for Go's new atomic And and Or
operations (#61395). This CL updates the race syso files to
include the change. Also regenerate cgo dynamic imports on darwin.

OpenBSD/AMD64 is not updated, as TSAN no longer supports OpenBSD
(#52090).

Linux/PPC64 is not updated, as I'm running into some builder
issues. Still working on it.

For #61395.
For #62624.

Change-Id: Ifc90ea79284f29a356f9e8a5f144f6c690881395
Reviewed-on: https://go-review.googlesource.com/c/go/+/543035
Run-TryBot: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Than McIntosh <thanm@google.com>
Reviewed-by: Mauri de Souza Meneguzzo <mauri870@gmail.com>
@gopherbot
Copy link

Change https://go.dev/cl/543397 mentions this issue: runtine/race: update race syso for PPC64LE

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@mauri870 mauri870

@cherrymui cherrymui

Labels
compiler/runtime Issues related to the Go compiler and/or runtime. NeedsFix The path to resolution is known, but the work has not been done. RaceDetector
Projects
Go Compiler / Runtime
Status: Done
Milestone
Go1.22
Development

No branches or pull requests
4 participants
@mknyszek @gopherbot @mauri870 @cherrymui