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

internal/coverage/decodemeta: mishandled function literal flag in covdata read #57942

Closed
thanm opened this issue Jan 20, 2023 · 4 comments
Closed
Assignees
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@thanm
Copy link
Contributor

thanm commented Jan 20, 2023

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

$ go version
go version devel go1.21-46d19b3ac9 Thu Jan 19 10:19:48 2023 -0500 linux/amd64

Does this issue reproduce with the latest release?

Only on tip / go 1.20

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

linux/amd64

What did you do?

Build this program with "-cover" and run to collect coverage profile.

https://go.dev/play/p/Gs7UJrWz86r?v=gotip

E.g.

$ go build -o prog.exe -cover prog.go
$ mkdir /tmp/covdata
$ GOCOVERDIR=/tmp/covdata ./prog.exe
$ go tool covdata func -i=tmp/covdata
...

What did you expect to see?

Coverage profile includes "main".

What did you see instead?

Coverage profile is missing "main":

$ go tool covdata func -i=/tmp/covdata
/tmp/prog.go:5:	*Atyp.Set	100.0%
/tmp/prog.go:9:	Atyp.Get	100.0%
total                        (statements)	100.0%
$

What's happening here is that there is a coding error in the handling of the function literal flag in the meta-data decoder., this code here:

	lit := d.r.ReadULEB128()
	if lit != 0 {
		f.Lit = true
	}

is written incorrectly. The update to f.Lit should be unconditional (since we can't make any assumptions about the state of the FuncDesc passed in). If clients always pass in a zero FuncDesc everything will appear to work ok, but if not, the state of the "Lit" field will be whatever it was set to on the last go around.

@thanm thanm self-assigned this Jan 20, 2023
@thanm thanm added the NeedsFix The path to resolution is known, but the work has not been done. label Jan 20, 2023
@thanm thanm added this to the Go1.20 milestone Jan 20, 2023
@gopherbot
Copy link

Change https://go.dev/cl/462955 mentions this issue: internal/coverage/decodemeta: fix coding error in func literal handling

@cherrymui
Copy link
Member

Do you plan to get this in to Go 1.20? If so, you may want to comment on #57854. Thanks.

@thanm
Copy link
Contributor Author

thanm commented Jan 20, 2023

Thanks Cherry. Yes, I personally think this change is appropriate for 1.20. I'll add a note.

@thanm thanm reopened this Jan 20, 2023
@thanm thanm closed this as completed Jan 20, 2023
@gopherbot
Copy link

Change https://go.dev/cl/463418 mentions this issue: [release-branch.go1.20] internal/coverage/decodemeta: fix coding error in func literal handling

gopherbot pushed a commit that referenced this issue Jan 26, 2023
…r in func literal handling

Fix a coding error in coverage meta-data decoding in the method
decodemeta.CoverageMetaDataDecoder.ReadFunc. The code was not
unconditionally assigning the "function literal" field of the
coverage.FuncDesc object passed in, resulting in bad values depending
on what the state of the field happened to be in the object.

Fixes #57942.

Change-Id: I6dfd7d7f7af6004f05c622f9a7116e9f6018cf4f
Reviewed-on: https://go-review.googlesource.com/c/go/+/462955
Run-TryBot: Than McIntosh <thanm@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
(cherry picked from commit 620399e)
Reviewed-on: https://go-review.googlesource.com/c/go/+/463418
Reviewed-by: Than McIntosh <thanm@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Auto-Submit: Matthew Dempsky <mdempsky@google.com>
@golang golang locked and limited conversation to collaborators Jan 25, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

3 participants