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

x/tools/go/analysis/passes/slog: nil panic in isAttr via analysisutil.IsNamed #61228

Closed
fsouza opened this issue Jul 7, 2023 · 3 comments
Closed

x/tools/go/analysis/passes/slog: nil panic in isAttr via analysisutil.IsNamed #61228

fsouza opened this issue Jul 7, 2023 · 3 comments
Assignees
@jba
Labels
Analysis Issues related to static analysis (vet, x/tools/go/analysis) NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Tools This label describes issues relating to any tools in the x/tools repository.
Milestone
Go1.21

Comments

@fsouza
Copy link
Contributor

fsouza commented Jul 7, 2023

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

$ go version
go version devel go1.21-39c5070712 Thu Jul 6 23:23:41 2023 +0000 darwin/arm64

Does this issue reproduce with the latest release?

Yes

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

go env Output
％ go env
GO111MODULE=''
GOARCH='arm64'
GOBIN='/Users/fsouza/bin'
GOCACHE='/Users/fsouza/Library/Caches/go-build'
GOENV='/Users/fsouza/Library/Application Support/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFLAGS='-modcacherw'
GOHOSTARCH='arm64'
GOHOSTOS='darwin'
GOINSECURE=''
GOMODCACHE='/Users/fsouza/.cache/go/path/pkg/mod'
GONOPROXY='git.sr.ht'
GONOSUMDB='git.sr.ht'
GOOS='darwin'
GOPATH='/Users/fsouza/.cache/go/path'
GOPRIVATE='git.sr.ht'
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/Users/fsouza/.cache/go/sdk/gotip'
GOSUMDB='sum.golang.org'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/Users/fsouza/.cache/go/sdk/gotip/pkg/tool/darwin_arm64'
GOVCS=''
GOVERSION='devel go1.21-39c5070712 Thu Jul 6 23:23:41 2023 +0000'
GCCGO='gccgo'
AR='ar'
CC='clang'
CXX='clang++'
CGO_ENABLED='1'
GOMOD='/Users/fsouza/Projects/os/p/s3-upload-proxy/go.mod'
GOWORK=''
CGO_CFLAGS='-O2 -g'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
PKG_CONFIG='pkg-config'
GOGCCFLAGS='-fPIC -arch arm64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -ffile-prefix-map=/var/folders/rz/8d0l4yp51sd3jh9f34x7pkg80000gn/T/go-build1879486314=/tmp/go-build -gno-record-gcc-switches -fno-common'

What did you do?

$ go test -c

The s3-upload-proxy package was using slog from golang.org/x/exp/slog, and I wanted to migrate it to the standard library one (see PR: fsouza/s3-upload-proxy#511). I ran into a weird issue when trying to run tests though, and since the error happens in a package that doesn't have any tests, I realized that the issue happened during compilation and tried to run go test -c to see what would happen, and voila, there's the error.

I tried creating a smaller reproducer, but failed to do so. Unfortunately I don't have much time in my hands to try to dig into this issue, so I figured I could report it in case someone else has more time. It's possible that I'm doing something very wrong here, but even in that case, I'm not sure if getting a panic from the build process is the right thing to do?

Steps to reproduce from "scratch":

％ git clone -b build-with-1.21rc2 https://github.com/fsouza/s3-upload-proxy.git
％ go -C s3-upload-proxy test -c

What did you expect to see?

Successful compilation of the test binary.

What did you see instead?

％ go test -c
# github.com/fsouza/s3-upload-proxy
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x2 addr=0x8 pc=0x1043fa738]

goroutine 44 [running]:
go/types.(*Package).Path(...)
        ../../../../.cache/go/sdk/gotip/src/go/types/package.go:33
cmd/vendor/golang.org/x/tools/go/analysis/passes/internal/analysisutil.IsNamed(...)
        ../../../../.cache/go/sdk/gotip/src/cmd/vendor/golang.org/x/tools/go/analysis/passes/internal/analysisutil/util.go:126
cmd/vendor/golang.org/x/tools/go/analysis/passes/slog.isAttr(...)
        ../../../../.cache/go/sdk/gotip/src/cmd/vendor/golang.org/x/tools/go/analysis/passes/slog/slog.go:142
cmd/vendor/golang.org/x/tools/go/analysis/passes/slog.run.func1({0x1044fa680?, 0x14000191c40})
        ../../../../.cache/go/sdk/gotip/src/cmd/vendor/golang.org/x/tools/go/analysis/passes/slog/slog.go:90 +0x268
cmd/vendor/golang.org/x/tools/go/ast/inspector.(*Inspector).Preorder(0x1400081a000, {0x140006a3be8?, 0x1?, 0x104236e2c?}, 0x140006a3bf8)
        ../../../../.cache/go/sdk/gotip/src/cmd/vendor/golang.org/x/tools/go/ast/inspector/inspector.go:81 +0x98
cmd/vendor/golang.org/x/tools/go/analysis/passes/slog.run(0x140007240d0)
        ../../../../.cache/go/sdk/gotip/src/cmd/vendor/golang.org/x/tools/go/analysis/passes/slog/slog.go:55 +0x8c
cmd/vendor/golang.org/x/tools/go/analysis/unitchecker.run.func5.1()
        ../../../../.cache/go/sdk/gotip/src/cmd/vendor/golang.org/x/tools/go/analysis/unitchecker/unitchecker.go:354 +0x734
sync.(*Once).doSlow(0x1044ab720?, 0x140001a5470?)
        ../../../../.cache/go/sdk/gotip/src/sync/once.go:74 +0x100
sync.(*Once).Do(...)
        ../../../../.cache/go/sdk/gotip/src/sync/once.go:65
cmd/vendor/golang.org/x/tools/go/analysis/unitchecker.run.func5(0x10468f060)
        ../../../../.cache/go/sdk/gotip/src/cmd/vendor/golang.org/x/tools/go/analysis/unitchecker/unitchecker.go:305 +0x134
cmd/vendor/golang.org/x/tools/go/analysis/unitchecker.run.func6.1(0x0?)
        ../../../../.cache/go/sdk/gotip/src/cmd/vendor/golang.org/x/tools/go/analysis/unitchecker/unitchecker.go:376 +0x34
created by cmd/vendor/golang.org/x/tools/go/analysis/unitchecker.run.func6 in goroutine 1
        ../../../../.cache/go/sdk/gotip/src/cmd/vendor/golang.org/x/tools/go/analysis/unitchecker/unitchecker.go:375 +0x4c
@seankhliao
Copy link
Member

Reproduced on tip.
The panic is in vet, run as part of tests.

Simpler reproducer:

package main

import (
	"errors"
	"log/slog"
)

func main() {
	err := errors.New("xxx")
	slog.Error("foo", err)
}

cc @jba

@seankhliao seankhliao added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Jul 7, 2023
@seankhliao seankhliao added this to the Go1.21 milestone Jul 7, 2023
@seankhliao seankhliao added the Analysis Issues related to static analysis (vet, x/tools/go/analysis) label Jul 7, 2023
@jba jba self-assigned this Jul 7, 2023
@bcmills bcmills changed the title cmd/go: nil pointer dereference when compiling tests after migrating from golang.org/x/exp/slog to log/slog x/tools/go/analysis/passes/slog: nil panic in isAttr via analysisutil.IsNamed Jul 10, 2023
@gopherbot gopherbot added the Tools This label describes issues relating to any tools in the x/tools repository. label Jul 10, 2023
@gopherbot
Copy link

Change https://go.dev/cl/508822 mentions this issue: src/cmd: re-vendor tools

@jba
Copy link
Contributor

jba commented Jul 12, 2023

Tools was vendored in via a different CL. This should be fixed.

@jba jba closed this as completed Jul 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@jba jba

Labels
Analysis Issues related to static analysis (vet, x/tools/go/analysis) NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Tools This label describes issues relating to any tools in the x/tools repository.
Projects
None yet
Milestone
Go1.21
Development

No branches or pull requests
4 participants
@fsouza @gopherbot @seankhliao @jba