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/go: go test -coverpkg=./... -race does not rebuild sync/atomic #18486

Closed
hirochachacha opened this issue Jan 1, 2017 · 15 comments
Closed
Milestone

Comments

@hirochachacha
Copy link
Contributor

Please answer these questions before submitting your issue. Thanks!

What version of Go are you using (go version)?

go version devel +e776975 Sat Dec 31 18:54:27 2016 +0000 linux/amd64

What operating system and processor architecture are you using (go env)?

GOARCH="amd64"
GOBIN=""
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/home/travis/gopath"
GORACE=""
GOROOT="/home/travis/.gimme/versions/go"
GOTOOLDIR="/home/travis/.gimme/versions/go/pkg/tool/linux_amd64"
GCCGO="gccgo"
CC="gcc"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build901867199=/tmp/go-build"
CXX="g++"
CGO_ENABLED="1"
PKG_CONFIG="pkg-config"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"

What did you do?

If possible, provide a recipe for reproducing the error.
A complete runnable program is good.
A link on play.golang.org is best.

  1. create an empty package https://github.com/hirochachacha/empty
  2. add .travis.yml
language: go
go:
    - 1.5
    - 1.6
    - 1.7
    - tip
script:
- go test -coverpkg=./... -race -v
  1. then, I see a build error. https://travis-ci.org/hirochachacha/empty
$ go test -coverpkg=./... -race -v
# github.com/hirochachacha/empty
/tmp/go-build559712700/github.com/hirochachacha/empty/_obj/empty.go:3: can't find import: "sync/atomic"

What did you expect to see?

no errors.

What did you see instead?

an error.

I could not reproduce it on my mac and linux.
However, I can reproduce it on a docker container.

docker run -it quay.io/travisci/travis-go /bin/bash

and,

su - travis
GIMME_OUTPUT=$(gimme tip) && eval "$GIMME_OUTPUT"
export GOPATH=$HOME/gopath
export PATH=$HOME/gopath/bin:$PATH
go get github.com/hirochachacha/empty
cd $HOME/gopath/src/github.com/hirochachacha/empty
go test -coverpkg=./... -race -v

I have no clue. It may be travis's issue? or something I am missing.

Thanks.

@davecheney
Copy link
Contributor

davecheney commented Jan 1, 2017 via email

@hirochachacha
Copy link
Contributor Author

@davecheney Good to know, thanks! But the problem is caused on the tip.

@JayNakrani
Copy link
Contributor

JayNakrani commented Jan 1, 2017

-coverpkg=./... and -race will work alone, but both of the flags together fail.

$ go test -coverpkg=./... -v
?   	github.com/hirochachacha/empty	[no test files]

$ go test -race -v
?   	github.com/hirochachacha/empty	[no test files]

$ go test -coverpkg=./... -race -v
# github.com/hirochachacha/empty
/tmp/go-build463635544/github.com/hirochachacha/empty/_obj/empty.go:3: can't find import: "sync/atomic"

$ cat /tmp/go-build595576066/github.com/hirochachacha/empty/_obj/empty.go -n
     1	package empty
     2	
     3	import _cover_atomic_ "sync/atomic"
     4	
     5	var _ = _cover_atomic_.AddUint32
     6	
     7	var GoCover_0 = struct {
     8		Count     [0]uint32
     9		Pos       [3 * 0]uint32
    10		NumStmt   [0]uint16
    11	} {
    12		Pos: [3 * 0]uint32{
    13		},
    14		NumStmt: [0]uint16{
    15		},
    16	}

I've isolated the issue to, cmd/compile not being able to compile a program importing std-lib with -race. It can compile it without -race though.

Smaller repro case,

$ cat /tmp/p.go 
package p

import _ "sync/atomic"

$ go tool compile -o /dev/null /tmp/p.go 
# ... succeeds ...

$ go tool compile -o /dev/null -race /tmp/p.go 
/tmp/p.go:3: can't find import: "sync/atomic"

Looking into why cmd/compile fails to compile it with -race.

@davecheney
Copy link
Contributor

Is there a GOROOT/pkg/linux_amd64_race/sync/atomic.a file?

@JayNakrani
Copy link
Contributor

Nope. And that's the issue. findpkg() is adding _race to the import path. I am not sure what the correct behavior should be for std-imports. Should it be importing non-instrumented version of std libs?

@davecheney
Copy link
Contributor

Yes, only instrument the package under test.

@JayNakrani
Copy link
Contributor

Okay, thanks. I'll tackle this in 1.9.
Because this is not a regression from 1.7, the fix can probably wait for 1.9.

@hirochachacha
Copy link
Contributor Author

I think this is a regression. please see https://travis-ci.org/hirochachacha/empty.

@hirochachacha
Copy link
Contributor Author

hirochachacha commented Jan 2, 2017

I think this is a regression. please see https://travis-ci.org/hirochachacha/empty.

I understand.

gimme 1.7 download binary package which have "pkg/linux_amd64_race" directory.
gimme tip download source code and run ./make.bash.

./make.bash doesn't create "pkg/$GOOS_$GOARCH_race". (while ./all.bash create the directory)
So this is not a platform specific issue nor a regression. I can reproduce it on my mac.

go test should create "pkg/$GOOS_$GOARCH_race" if it is missing?

@davecheney
Copy link
Contributor

davecheney commented Jan 2, 2017 via email

@JayNakrani
Copy link
Contributor

JayNakrani commented Jan 3, 2017

Yes, only instrument the package under test.

I thought about this and it doesn't add up. When compiling with -race, it has to import race instrumented versions of all packages. Without that, it wouldn't catch race between a function in user's package and another function in std-lib.

/cc @dvyukov, to confirm.

If that's the case, then cmd/compile is working as expected; Although, it could give better error message mentioning that it was looking for race/msan instrumented version of the package.

@davecheney
Copy link
Contributor

davecheney commented Jan 3, 2017 via email

@JayNakrani
Copy link
Contributor

Ah, okay. Sent out the CL for better error.

After golang.org/cl/34798:

$ compile -o /dev/null -race /tmp/p.go
/tmp/p.go:3: can't find race-instrumented version of import: "fmt"

@gopherbot
Copy link

CL https://golang.org/cl/34798 mentions this issue.

@rsc rsc added this to the Go1.8 milestone Jan 4, 2017
@rsc rsc changed the title 'go test -coverpkg=./... -race' on the empty package fails on travis CI. cmd/go: go test -coverpkg=./... -race does not rebuild sync/atomic Jan 4, 2017
@gopherbot
Copy link

CL https://golang.org/cl/34830 mentions this issue.

@golang golang locked and limited conversation to collaborators Jan 4, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants