Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.Sign up
GitHub is where the world builds software
Millions of developers and companies build, ship, and maintain their software on GitHub — the largest and most advanced development platform in the world.
cmd/cover: cgo produces incorrect coverage profiles #9479
What version of Go are you using (go version)?
What operating system and processor architecture are you using?
What did you do?
export CGO_LDFLAGS="-framework Accelerate" go get -u -t github.com/gonum/blas go test -coverprofile=coverage.out github.com/gonum/blas/cblas/ go tool cover -html=coverage.out
These commands would be different on other OSes, because the Accelerate framework is not available on them. Alternate installation instructions are available on the github.com/gonum/blas README.md. The important thing is that I'm running the cover tool's html option with a package that uses cgo.
What did you expect to see?
The issue #6333 seems to be related. That one appears to be focused on the command line tool, which is working correctly in this case (I think). The html report is not.
I double checked by removing everything and go get-ing everything again. This produces the same result:
jonlawlor: ~/go/src/github.com/gonum/blas $ go get -u golang.org/x/tools/cmd/cover jonlawlor: ~/go/src/github.com/gonum/blas $ go version go version go1.4 darwin/amd64 jonlawlor: ~/go/src/github.com/gonum/blas $ export CGO_LDFLAGS="-framework Accelerate" jonlawlor: ~/go/src/github.com/gonum/blas $ go get -u -t github.com/gonum/blas jonlawlor: ~/go/src/github.com/gonum/blas $ go test -coverprofile=coverage.out github.com/gonum/blas/cblas/ --- FAIL: TestIdamax (0.00s) level1double.go:1155: idamax: mismatch NaN: expected 0, found 1 level1double.go:1155: idamax: mismatch NaNInc: expected 0, found 1 FAIL coverage: 16.0% of statements FAIL github.com/gonum/blas/cblas 0.207s jonlawlor: ~/go/src/github.com/gonum/blas $ go tool cover -html=coverage.out
(except that there was some work in gonum/blas which makes it fail a test)
I really have tried to eliminate any PBCAK, but if I've missed something I apologize.
The problem here is that the line and column numbers in the cover profile refer to the cgo-rewritten file, but the file named is the original Go source file. For this particular case, a profile line refers to column 157 of line 91 of blas.go, whereas the actual data for that counter comes from 157 of line 91 of blas.cgo1.go. Line 91 of blas.go has only 122 characters; the cgo rewrite lengthens the line.
When the HTML processor for the cover tool reads, as instructed, blas.go, line 91 has only 122 characters but the block termination is written to happen at column 157. The tool decides the block never terminates, and the HTML output is compromised.
We are fixing the cover tool to terminate the block at the end of the line so the rest of the file will work. This is a simple fix, but not fully correct: the column numbers for blocks that start or stop on all rewritten lines will be incorrect. They always were, for cgo files.
A real fix will require much more complex computation to refer the column numbers back to the original Go source file, which is the one that the HMTL annotation refers to. This is not impossible, but it's close to infeasible.
We will leave this bug open to record this possibility, but don't expect it to be fixed. The updated cover tool will at least not fail so badly. Only the column numbers will sometimes be wrong. Lines will always be right.
When cover is used with cgo packages, the coverage profile is produced from the cgo-generated Go source files, so the profile will reference line and column numbers from that generated source, even though it names the original file. This is okay in general because the cgo-generated Go source files are very similar to the original, with one significant exception: the original source may refer to C identifiers such as C.foo(), but the cgo tool generates source that rewrites these mentions to something like _Cfunc_foo. This means that column numbers in coverage profiles might be higher than they should be, so be lenient when interpreting them. Update golang/go#9479 Change-Id: Ic3abef07471614101ce0c686d35b85e7e5e6a777 Reviewed-on: https://go-review.googlesource.com/2410 Reviewed-by: Rob Pike <email@example.com>
It occurs to me that a feasible solution is to run the cover analysis on two files, the original and the cgo output. Rewrite the cgo output file as usual, but append the position data from the scan of the original. To work without bringing the diff algorithm in, which I would like to avoid, it would require that all cgo rewriting is within a line, that is, that it does not add or delete any lines. It does this today (otherwise coverage wouldn't work at all), but I am a little nervous to add this requirement.