cmd/vet,x/tools/go/analysis/passes/printf: broken with GODEBUG=gotypesalias=1

[findleyr]

Repro, originating from @adonovan's investigation of #68744:

> cat bad.go
package main

import "fmt"

func main() {
        fmt.Printf("%s", 123)
        wrapf("%s", 123)
}

func wrapf(format string, args ...any) {
        fmt.Printf(format, args...)
}
> go version
go version go1.23rc2 linux/amd64
> go vet bad.go 
# command-line-arguments
# [command-line-arguments]
./bad.go:6:2: fmt.Printf format %s has arg 123 of wrong type int
./bad.go:7:2: command-line-arguments.wrapf format %s has arg 123 of wrong type int
> GODEBUG=gotypesalias=0 go vet bad.go 
# command-line-arguments
# [command-line-arguments]
./bad.go:6:2: fmt.Printf format %s has arg 123 of wrong type int
./bad.go:7:2: command-line-arguments.wrapf format %s has arg 123 of wrong type int
> GODEBUG=gotypesalias=1 go vet bad.go 
# command-line-arguments
# [command-line-arguments]
./bad.go:6:2: fmt.Printf format %s has arg 123 of wrong type int

When GODEBUG=gotypesalias=1 (the default in 1.23), the printf analyzer fails to report a diagnostic for the wrapf printf wrapper, due to a missing call to types.Unalias.

Separately, it appears the default vet tool distributed with 1.23rc2 (or built on the release branch with make.bash) does not have GODEBUG=gotypesalias=1. Otherwise, this should have been caught by the vet integration tests. I will file a separate issue for that.

It may be the case that the two bugs mentioned in the previous two paragraphs effectively cancel themselves out for the Go distribution. However, I think we should at least block the release until we understand both better.

[gabyhelp]

Related Issues and Documentation

• cmd/vet: printf analyzer cannot analyze anonymous functions #44350
• cmd/vet: false positives and wrong types across files, needs automatic recursive checking #20514 (closed)
• cmd/vet: Printf format %.T has unrecognized flag #30363
• go/types, types2: audit type assertions for potential Alias bugs #67547 (closed)
• cmd/vet: report type and format specifier mismatch in Printf* #4404 (closed)
• go vet incorrectly recognizes format verb in string #38131 (closed)
• cmd/vet: false positive about wrong argument for verb #23552 (closed)
• cmd/vet: false positive in Printf recursive Stringer check with non-matching String() functions #30441 (closed)
• cmd/vet: false positive printf-like function detection #26486 (closed)
• cmd/vet: false positive about wrong number of arguments in printf #23564 (closed)

_{(Emoji vote if this was helpful or unhelpful; more detailed feedback welcome in this discussion.)}

[gopherbot]

Change https://go.dev/cl/603938 mentions this issue: go/analysis/passes/printf: add missing Unalias call

[findleyr]

Now that we understand it better, I don't think this needs to block the release. A missing vet diagnostic for printf wrappers (that use any, not interface{}) seems to be benign enough to fix in 1.23.1.

[dmitshur]

Thanks. For this to be fixed in go1.23.1, it'll still need to be fixed at tip first and then backported, so moving this to Go1.24 milestone and re-adding release-blocker. Please use the usual process (https://go.dev/wiki/MinorReleases) to create a separate backport tracking issue in the Go1.23.1 milestone.

[findleyr]

@gopherbot please backport this issue to 1.23. It is a regression that may affect vet if #68797 is also backported.

[gopherbot]

Backport issue(s) opened: #68813 (for 1.23).

Remember to create the cherry-pick CL(s) as soon as the patch is submitted to master, according to https://go.dev/wiki/MinorReleases.

[findleyr]

To clarify: right now the version of vet distributed with go1.23rc2 DOES work as expected. It is only if we fix the incorrect GODEBUG value to have gotypesalias=1 that we would observe the printf bug, and even if that happened I don't think the nature of this bug is severe enough to block this release. I'm not even sure we should backport the fix to 1.23, unless we decide to also backport a fix for #68797.

One of the motivations for marking this as a release blocker was that we weren't sure if GODEBUG was being set correctly during compilation of ALL programs. It turns out that the issue with GODEBUG is restricted to toolchain programs, as a result of the bootstrap process. That is a much smaller problem.

[gopherbot]

Change https://go.dev/cl/609435 mentions this issue: [gopls-release-branch.0.16] go/analysis/passes/printf: add missing Unalias call

[dmitshur]

Checking on this issue as it's a release blocker for 1.24. Any updates? Thanks.

[gopherbot]

Change https://go.dev/cl/610736 mentions this issue: vendor/golang.org/x/tools: update to v0.24.1-0.20240904143311-70f56264139c

[gopherbot]

Change https://go.dev/cl/610737 mentions this issue: vet: add regression test for printf checker regression

[gopherbot]

Change https://go.dev/cl/610739 mentions this issue: all: fix printf(var) mistakes detected by latest printf checker

[gopherbot]

Change https://go.dev/cl/610797 mentions this issue: all: fix printf(var) mistakes detected by latest printf checker