-
Notifications
You must be signed in to change notification settings - Fork 17.8k
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
all: racy usage not detected due to assembly code #61204
Comments
This happens because A few possible solutions:
I stumbled across this because I was looking at the compiler's |
Another (possibly worse) variation of this problem, this time using just In this case, it's the compiler generating the call, so we might be able to just add the missing instrumentation around the call. |
Fixing problem for
and
|
Right. We'd have to do that for all of the algorithms in I'm kind of surprised using a race-instrumented version of |
I guess because the function isn't inlined from a race package ( |
I'm surprised because there are contexts in the runtime where the race detector will crash or self-loop, regardless of inlining. I guess we just don't use |
This also happens with other packages that have assembly code, e.g. crypto/aes, see #63817. |
Adding relevant info from #63817. Example of data race not being detected while using Going over the standard library, assembly was used in:
Similar search for
So all of these probably need to be evaluated for possible missing race detection. There seem to be three main issues:
I guess there are few ways to make the behavior visible to race detector:
Not using assembly can be a significant performance hit in some scenarios, so switching off assembly doesn't seem ideal. The other approach is to call the race detector functions directly, e.g. Third approach is to add some magic incantation to assembly code to allow the compiler to add appropriate race detection, e.g. rough draft (ignore whether I got the content correct):
Of course, writing those annotations in comments is significantly more annoying than using some nicer Go API. For starters I can add the necessary annotations to standard library using |
It seems unfortunate to have manual annotations (including manual calls to Another — probably much slower but higher-fidelity — option might be to have the runtime move the arguments to assembly functions to pages with access protection, and then use a fault handler to step through the assembly routine to observe (and record) the actual access patterns. |
I definitely agree that manual annotations are fragile, but I don't think they are more fragile than assembly itself. But, yeah, if there's a way to implement it without having to annotate, then that would be even better. I wasn't able to figure out a way that would work reliably and (reasonably) fast. |
Sidenote, I did implement helpers for my own purposes:
Most of the data being read is either a slice or a pointer to a struct. |
Change https://go.dev/cl/546555 mentions this issue: |
Yay, we can't just do this, because IOW, the build tags condition |
Change https://go.dev/cl/601117 mentions this issue: |
So the racy usage could be detected after re-writing "==" to runtime.memequal call. Updates #61204 Change-Id: Idb4ac37e55813cc87f9d16aa656fb447edf69ea1 Reviewed-on: https://go-review.googlesource.com/c/go/+/601117 Reviewed-by: Egon Elbre <egonelbre@gmail.com> Auto-Submit: Cuong Manh Le <cuong.manhle.vn@gmail.com> Reviewed-by: Michael Knyszek <mknyszek@google.com> Reviewed-by: Cherry Mui <cherryyz@google.com> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
What version of Go are you using (
go version
)?Does this issue reproduce with the latest release?
Yes. Reproduced back to Go 1.10.
What did you do?
Run this program with
-race
. (Thanks to @cherrymui for putting together the example.)What did you expect to see?
A race report like the following:
What did you see instead?
With the call to
bytes.IndexByte
, there's no race report. There is a race report if you comment out that line and uncomment the line that calls the pure GoIndexByte
implementation.cc @golang/runtime
The text was updated successfully, but these errors were encountered: