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.

Sign up for GitHub

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

Jump to bottom

cmd/vet: lock copy warning and unsafe.Offsetof #52700

Closed
oakad opened this issue May 4, 2022 · 4 comments
Closed

cmd/vet: lock copy warning and unsafe.Offsetof #52700

oakad opened this issue May 4, 2022 · 4 comments
Labels
Analysis Issues related to static analysis (vet, x/tools/go/analysis) FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone
Backlog

Comments

@oakad
Copy link

oakad commented May 4, 2022

go 1.18.1

This may not be a bug but it was not obvious to me how to avoid the warning.

Consider the following situation:

type B struct {
	sync.Mutex
}

type D struct {
	some int
	base B
}

const baseOffset = unsafe.Offsetof(D{}.base)

This now triggers a warning with go vet: "call of unsafe.Offsetof copies lock value: B".

Clearly:

  1. Either go vet should be made to recognize Offsetof usage
  2. Or, Offsetof should be modified somehow to avoid triggering the warning
@dr2chase dr2chase added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label May 4, 2022
@dr2chase dr2chase added this to the Backlog milestone May 4, 2022
@dr2chase
Copy link
Contributor

dr2chase commented May 4, 2022

@matloob @timothy-king can one of you give this a look? I went looking for a duplicate, I'd swear I'd seen recent mention of something very similar but I couldn't find it.

@zpavlinovic zpavlinovic added the Analysis Issues related to static analysis (vet, x/tools/go/analysis) label May 4, 2022
@zpavlinovic
Copy link
Contributor

I believe reflect.{Alignof, Offsetof} should be excluded from the analysis of calls in the copy-lock checker as these functions do not really evaluate expressions (#12946, #20262).

Interestingly, reflect.Sizeof is already covered by the checker (in addition to len, cap, and new).

@gopherbot
Copy link

Change https://go.dev/cl/404234 mentions this issue: passes/copylock: suppress reports on Offsetof and Alignof

@timothy-king
Copy link
Contributor

This is roughly the same as #21800. The fix goes back to https://go-review.googlesource.com/c/go/+/66210/. Given the age of the issue and the other fix, I don't think a fix needs to be back ported.

@golang golang locked and limited conversation to collaborators May 5, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
Analysis Issues related to static analysis (vet, x/tools/go/analysis) FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Milestone
Backlog
Development

No branches or pull requests
5 participants
@oakad @timothy-king @zpavlinovic @dr2chase @gopherbot