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/cover: doesn't work with tip when using //go: annotations #18285

Closed
mikioh opened this issue Dec 12, 2016 · 7 comments
Closed

cmd/cover: doesn't work with tip when using //go: annotations #18285

mikioh opened this issue Dec 12, 2016 · 7 comments
Assignees
Milestone

Comments

@mikioh
Copy link
Contributor

@mikioh mikioh commented Dec 12, 2016

With go1.7.4:

go test -short -cover runtime
PASS
coverage: 58.7% of statements
ok      runtime 38.548s

With devel +0716fef:

go test -short -cover runtime
# testmain
runtime.nanotime1: relocation target libc_clock_gettime not defined
runtime.pipe1: relocation target libc_pipe not defined
runtime.usleep2: relocation target libc_usleep not defined
runtime.osyield1: relocation target libc_sched_yield not defined
runtime.nanotime1: undefined: "libc_clock_gettime"
runtime.pipe1: undefined: "libc_pipe"
runtime.usleep2: undefined: "libc_usleep"
runtime.osyield1: undefined: "libc_sched_yield"
FAIL    runtime [build failed]
@mikioh mikioh added the OS-Solaris label Dec 12, 2016
@dhananjay92
Copy link
Contributor

@dhananjay92 dhananjay92 commented Dec 12, 2016

They look like linker errors. Does it work without -cover?

@mikioh
Copy link
Contributor Author

@mikioh mikioh commented Dec 13, 2016

@dhananjay92,

Does it work without -cover?

See https://build.golang.org.

The cmd/cover package just generates annotated source files. So just guessing that some modification to runtime package during Go 1.8 development cycle breaks the existing plumbing work for shared libraries.

dhananjay92 added a commit to dhananjay92/go that referenced this issue Dec 17, 2016
Change-Id: Icb633c7b3e13504a5d26c8d947c76cb0dd800269
@dhananjay92
Copy link
Contributor

@dhananjay92 dhananjay92 commented Dec 17, 2016

Okay, root cause is that the compiler directives in src/runtime/os3_solaris.go disappear during coverage instrumentation.

Confirmed with,

$ go tool cover -mode=count runtime/os3_solaris.go
# .. notice that the compiler directives are present ...

$ ../pkg/tool/linux_amd64/cover -mode=count runtime/os3_solaris.go
# ... notice absence of compiler directives ...

This is happening because of my own change: golang.org/cl/30161. Coverage instrumentation removes all comments that are not present as nodes in the AST; because they were interfering with inserted code.

Parser doesn't keep these compiler directives as nodes in AST, but instead as CommentGroup in ast.File.Comments with all other comments. The fix that solves both this and #17315 would be to either keep all compiler directive into AST as nodes, Or handle such un-attached compiler directives explicitly like cover does for // +build tags.

Working on the fix.

PS: The issue will go away with dhananjay92@4a16ac8 because doing that makes parser keep those comments in AST. Parser only keeps comments into the tree that can potentially be document-comments

@dhananjay92
Copy link
Contributor

@dhananjay92 dhananjay92 commented Dec 17, 2016

I think Ian had told me some time earlier that the compiler directives are associated with something (a function or variable, etc). If that's the case, fix would be dhananjay92@4a16ac8.

Let me confirm with him again.

@mikioh mikioh changed the title runtime: cmd/cover doesn't work with tip on solaris/amd64 cmd/cover: doesn't work with tip on solaris/amd64 Dec 19, 2016
@mikioh
Copy link
Contributor Author

@mikioh mikioh commented Dec 19, 2016

@dhananjay92,

Thanks for the investigation. As you mentioned above, a few packages that import runtime symbols by using //go:cgo_import_dynamic or //go:linkname annotation, for example vendor/golang_org/x/net/lif, golang.org/x/net/lif or golang.org/x/sys, surely crash when using cmd/cover.

/CC @ianlancetaylor

@gopherbot
Copy link

@gopherbot gopherbot commented Dec 19, 2016

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

@dhananjay92
Copy link
Contributor

@dhananjay92 dhananjay92 commented Dec 19, 2016

golang.org/cl/34563 takes care of such compiler directives that aren't attached to any node in the AST.

My another attempt to let printer print such compiler directives, failed. Because ast/printer can either print from ast.File.Comment or from nodes in the tree, but not from both.

@mikioh mikioh changed the title cmd/cover: doesn't work with tip on solaris/amd64 cmd/cover: doesn't work with tip when using //go: annotations Dec 19, 2016
@mikioh mikioh removed the OS-Solaris label Dec 19, 2016
@mikioh mikioh added this to the Go1.8 milestone Dec 19, 2016
@gopherbot gopherbot closed this in 7eee512 Dec 20, 2016
@golang golang locked and limited conversation to collaborators Dec 31, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants
You can’t perform that action at this time.