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

cmd/cgo/internal/testsanitizers: TestTSAN failures with SIGSEGV #62125

Open
gopherbot opened this issue Aug 18, 2023 · 10 comments
Open

cmd/cgo/internal/testsanitizers: TestTSAN failures with SIGSEGV #62125

gopherbot opened this issue Aug 18, 2023 · 10 comments
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided.
Milestone

Comments

@gopherbot
Copy link

gopherbot commented Aug 18, 2023

#!watchflakes
post <- pkg == "cmd/cgo/internal/testsanitizers" && test == "TestTSAN" && `exited with signal: segmentation fault`

Issue created automatically to collect these failures.

Example (log):

--- FAIL: TestTSAN (7.58s)
    --- FAIL: TestTSAN/tsan (1.49s)
        tsan_test.go:77: /workdir/tmp/TestTSAN2652834654/tsan exited with signal: segmentation fault

watchflakes

@gopherbot gopherbot added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Aug 18, 2023
@gopherbot
Copy link
Author

Found new dashboard test flakes for:

#!watchflakes
default <- pkg == "cmd/cgo/internal/testsanitizers" && test == "TestTSAN"
2023-08-17 23:32 linux-amd64-race go@60dd8311 cmd/cgo/internal/testsanitizers.TestTSAN (log)
--- FAIL: TestTSAN (7.58s)
    --- FAIL: TestTSAN/tsan (1.49s)
        tsan_test.go:77: /workdir/tmp/TestTSAN2652834654/tsan exited with signal: segmentation fault

watchflakes

@bcmills bcmills added the compiler/runtime Issues related to the Go compiler and/or runtime. label Aug 18, 2023
@bcmills bcmills changed the title cmd/cgo/internal/testsanitizers: TestTSAN failures cmd/cgo/internal/testsanitizers: TestTSAN failures with SIGSEGV Aug 18, 2023
@mknyszek mknyszek added this to the Backlog milestone Aug 23, 2023
@mknyszek
Copy link
Contributor

This isn't very actionable at the moment. We'll wait for more failures.

@mknyszek mknyszek added the WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. label Aug 30, 2023
@laboger
Copy link
Contributor

laboger commented Aug 31, 2023

It seems odd to be building TestTSAN with the -race option. Those tests link in the gcc thread sanitizer library and the -race option links in the LLVM thread sanitizer code from the race syso file. Should they be expected to work together?

@ianlancetaylor
Copy link
Contributor

That is a good point. This will link together two different copies of the TSAN code. The TSAN code has the same sources, so it will often work, but if there are version discrepancies anything could happen.

@gopherbot
Copy link
Author

Change https://go.dev/cl/524896 mentions this issue: cmd/cgo/internal/testsanitizers: don't test a sanitizer we are using

@bcmills
Copy link
Member

bcmills commented Sep 6, 2023

@laboger, what leads you to believe that the binaries built by the test are linking in both sanitizers?

As far as I can tell, the configure function doesn't take into account the settings with which the test binary itself was built. (But perhaps it should?)

Or maybe these settings are leaking in through an env file or a GOFLAGS environment variable?
(But I don't see a GOFLAGS variable in the environment logged in https://build.golang.org/log/790a7852a56dfa511475f6e00e42da18cbc23a26.)

@laboger
Copy link
Contributor

laboger commented Sep 7, 2023

@laboger, what leads you to believe that the binaries built by the test are linking in both sanitizers?

I was involved in adding the support for the -race option on ppc64le, so I know the -race option causes the .syso files from runtime/race to be linked into the binaries. Those syso files are built from LLVM's thread sanitizer code.

In the last year or so I had to debug some of the TestTSAN tests on ppc64le because we were seeing flaky results and discovered that those tests were using cgo to link in the sanitizer libraries that come with gcc.

For example, cmd/cgo/internal/testsanitizers/testdata/tsan.go and all the other tsan tests in this directory contain something like this:

package main

// This program produced false race reports when run under the C/C++
// ThreadSanitizer, as it did not understand the synchronization in
// the Go code.

/*
#cgo CFLAGS: -fsanitize=thread
#cgo LDFLAGS: -fsanitize=thread

These flags cause the gcc sanitizer library code to be linked in.

@bcmills
Copy link
Member

bcmills commented Sep 7, 2023

I know the -race option causes the .syso files from runtime/race to be linked into the binaries.

I agree that that is correct. However, I don't see how the -race option would be set for the binaries built by cmd/cgo/internal/testsanitizers.TestTSAN, unless it crept in via GOFLAGS or .config/go/env (which AFAICT is not the case for the failures reported here by watchflakes).

Those binaries are built by calling go build here:
https://cs.opensource.google/go/go/+/master:src/cmd/cgo/internal/testsanitizers/tsan_test.go;l=68;drc=a674ab1961ca75d902ebbdf0a3d363501fd017ae

@laboger
Copy link
Contributor

laboger commented Sep 7, 2023

In the case above where you show the failures, it is running on the linux-amd64-race builder? Doesn't that mean it is running everything with the -race option? Maybe I misunderstood what that means.

@bcmills
Copy link
Member

bcmills commented Sep 7, 2023

The -race builder does indeed build all of the packages and tests using the -race option. So the testsanitizers.test binary itself is built with -race and linked against the .syso files.

However, testsanitizers.test itself calls out to go build to build another binary, which it then runs in order to perform the actual test. And that other binary is not built with -race, because nothing in the test (or the builder environment) causes that to happen.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided.
Projects
Status: No status
Status: No status
Development

No branches or pull requests

5 participants