-
Notifications
You must be signed in to change notification settings - Fork 17.9k
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/cover: cgo produces incorrect coverage profiles #9479
Comments
Are you sure you're running the latest version of the cover binary?
|
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 <r@golang.org>
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. |
It seems working good to me. |
BTW, OT, the file path should be ToSlash-ed? |
@mattn if it works well for you then you're just lucky. It only fails under certain circumstances. |
Change https://golang.org/cl/77155 mentions this issue: |
What version of Go are you using (go version)?
go1.4 darwin/amd64
What operating system and processor architecture are you using?
OSX Yosemite (10.10.1) x86_64
What did you do?
On OSX:
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?
I expected 16% of the lines of code to show green, consistent with the test tool's coverage report.
What did you see instead?

Only a 3 lines were highlighted green, and there were hundreds of lines colored red.
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.
The text was updated successfully, but these errors were encountered: