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

runtime/race: eliminate dependency on cmd/cgo #6508

Open
dvyukov opened this issue Sep 28, 2013 · 11 comments
Open

runtime/race: eliminate dependency on cmd/cgo #6508

dvyukov opened this issue Sep 28, 2013 · 11 comments

Comments

@dvyukov
Copy link
Member

dvyukov commented Sep 28, 2013

There is a circular dependency between runtime/race and cmd/cgo in -race build.
Everything depends on runtime/race, but runtime/race is a cgo package.
Currently it's resolved by a hack in go tool:

    // If we are not doing a cross-build, then record the binary we'll
    // generate for cgo as a dependency of the build of any package
    // using cgo, to make sure we do not overwrite the binary while
    // a package is using it.  If this is a cross-build, then the cgo we
    // are writing is not the cgo we need to use.
    if goos == runtime.GOOS && goarch == runtime.GOARCH && !buildRace {

We pretend that packages do not depend on cmd/cgo. But as the result we can overwrite
cmd/cgo binary while it is used.

A better solution is to make runtime/race to be not dependent on cmd/cgo.
@rsc
Copy link
Contributor

rsc commented Nov 27, 2013

Comment 1:

Labels changed: added go1.3maybe.

@rsc
Copy link
Contributor

rsc commented Dec 4, 2013

Comment 2:

Labels changed: added release-none, removed go1.3maybe.

@rsc
Copy link
Contributor

rsc commented Dec 4, 2013

Comment 3:

Labels changed: added repo-main.

@dvyukov
Copy link
Member Author

dvyukov commented Mar 6, 2014

Comment 4:

This issue was updated by revision a1695d2.

Update issue #6688
R=iant, rsc, bradfitz
CC=golang-codereviews
https://golang.org/cl/55100044

@odeke-em
Copy link
Member

Gentle ping @dvyukov @ianlancetaylor how are we doing here? Is this issue still a concern? Could I be of help here with your guidance?

@dvyukov
Copy link
Member Author

dvyukov commented Dec 23, 2017

This is still an issue. Fixing it will require wiping all dependencies on libc from race runtime. I've removed the main one -- on malloc/free -- some time ago, but there are still some minor ones.
If we eliminate dependency on libc/cgo, it will also allow using -race without enabling cgo, which would be nice as well.
But nobody actively complained about this as far as I know, so this was never high on my list.

@odeke-em
Copy link
Member

Gotcha gotcha, thanks @dvyukov! Oh right, you opened #9918 which is still open.

@AlekSi
Copy link
Contributor

AlekSi commented Sep 30, 2023

If we eliminate dependency on libc/cgo, it will also allow using -race without enabling cgo, which would be nice as well.
But nobody actively complained about this as far as I know, so this was never high on my list.

In the meantime, many people actively complained about that issue :) Many (including me!) seem to want to build scratch-based Docker images with race-detector-enabled binaries copied from the previous Docker build stage.

What are minor things that are left?

@dvyukov
Copy link
Member Author

dvyukov commented Sep 30, 2023

What are errors when cgo is disabled for -race build?

@AlekSi
Copy link
Contributor

AlekSi commented Sep 30, 2023

fmt.Fprintf(os.Stderr, "go: %s requires cgo; enable cgo by setting CGO_ENABLED=1\n", modeFlag)

If that check is commented out, it fails with:

runtime/race/internal/amd64v1(.text): relocation target getuid not defined
runtime/race/internal/amd64v1(.text): relocation target pthread_self not defined
runtime/race/internal/amd64v1(.text): relocation target abort not defined
runtime/race/internal/amd64v1(.text): relocation target mkdir not defined
runtime/race/internal/amd64v1(.text): relocation target isatty not defined
runtime/race/internal/amd64v1(.text): relocation target pthread_attr_getstack not defined
runtime/race/internal/amd64v1(.text): relocation target sigaction not defined
runtime/race/internal/amd64v1(.text): relocation target getrusage not defined
runtime/race/internal/amd64v1(.text): relocation target syslog not defined
runtime/race/internal/amd64v1(.text): relocation target getrlimit not defined
runtime/race/internal/amd64v1(.text): relocation target pipe not defined
runtime/race/internal/amd64v1(.text): relocation target sched_getaffinity not defined
runtime/race/internal/amd64v1(.text): relocation target __sched_cpucount not defined
runtime/race/internal/amd64v1(.text): relocation target pthread_attr_init not defined
runtime/race/internal/amd64v1(.text): relocation target pthread_getattr_np not defined
runtime/race/internal/amd64v1(.text): relocation target pthread_attr_destroy not defined
runtime/race/internal/amd64v1(.text): relocation target exit not defined
runtime/race/internal/amd64v1(.text): relocation target __errno_location not defined
runtime/race/internal/amd64v1(.text): relocation target setrlimit not defined
runtime/race/internal/amd64v1(.text): relocation target sysconf not defined
runtime/race/internal/amd64v1(.text): relocation target clock_gettime not defined
/go/pkg/tool/linux_amd64/link: too many errors

which looks similar to #9918 (comment)

@dvyukov
Copy link
Member Author

dvyukov commented Oct 4, 2023

To fix most of these one would need to add more ifdef's similar to this one:
https://github.com/llvm/llvm-project/blob/ca003ee06d0eac7e8facc179181298a05e4d03ed/compiler-rt/lib/tsan/rtl/tsan_platform_linux.cpp#L152

I would assume most uses of these libc functions can be simply removed as not needed for the Go version (e.g. isatty/pthread_self).

Some may need to replaced with calls to internal analogs. E.g. for exit we have Die in sanitizer runtimes. Not sure why we ever need to call exit.

Few may be harder to remove. These need to be dealt on a case-by-case basis.

Once all of these are removed, we also need a test in llvm upstream to prevent introduction of new libc calls in future.
The test can be added here: https://github.com/llvm/llvm-project/blob/ca003ee06d0eac7e8facc179181298a05e4d03ed/compiler-rt/lib/tsan/go/buildgo.sh

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants