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

go/doc: Examples panics on a file containing a function without a body #42706

Closed
bmizerany opened this issue Nov 18, 2020 · 5 comments
Closed

go/doc: Examples panics on a file containing a function without a body #42706

bmizerany opened this issue Nov 18, 2020 · 5 comments
Labels
Milestone

Comments

@bmizerany
Copy link
Contributor

@bmizerany bmizerany commented Nov 18, 2020

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

go version go1.15.5 darwin/amd64

Does this issue reproduce with the latest release?

Yes.

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

go env Output
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/bmizerany/Library/Caches/go-build"
GOENV="/Users/bmizerany/Library/Application Support/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOINSECURE=""
GOMODCACHE="/Users/bmizerany/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="darwin"
GOPATH="/Users/bmizerany/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/local/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/darwin_amd64"
GCCGO="gccgo"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/c0/8wpgkv1d47n5lxdmw1q79nzm0000gn/T/go-build255996969=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

Wrote this example:

package example_test

func foo(int)

func Example() {
        foo(1)
}

Ran go test

What did you expect to see?

go test succeed or error with helpful message.

What did you see instead?

A panic:

$ ls
example_test.go

$ cat example_test.go 
package example_test

func foo(int)

func Example() {
	foo(1)
}

$ go test
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x8 pc=0x1121b37]

goroutine 1 [running]:
go/ast.Walk(0x1704080, 0xc00029c930, 0x17089e0, 0x0)
	/usr/local/go/src/go/ast/walk.go:224 +0x18b7
go/ast.Inspect(...)
	/usr/local/go/src/go/ast/walk.go:385
go/doc.playExample(0xc0002b4580, 0xc00029c810, 0x0)
	/usr/local/go/src/go/doc/example.go:240 +0x465
go/doc.Examples(0xc0001652b0, 0x1, 0x1, 0x9, 0x0, 0x4)
	/usr/local/go/src/go/doc/example.go:89 +0x5ec
cmd/go/internal/load.(*testFuncs).load(0xc0002d6150, 0xc000428040, 0x32, 0x163809d, 0x6, 0xc0002d61aa, 0xc0002d61ab, 0x0, 0x0)
	/usr/local/go/src/cmd/go/internal/load/test.go:585 +0x10c
cmd/go/internal/load.loadTestFuncs(0xc000080d80, 0x5, 0x5, 0x0)
	/usr/local/go/src/cmd/go/internal/load/test.go:472 +0x29e
cmd/go/internal/load.TestPackagesAndErrors(0xc000080d80, 0x0, 0xc000081680, 0xc000080d80, 0xc000081200)
	/usr/local/go/src/cmd/go/internal/load/test.go:271 +0x1cb6
cmd/go/internal/load.TestPackagesFor(0xc000080d80, 0x0, 0x1448576, 0x15a8100, 0xc00009a540, 0x10fbf1b, 0xc0000c4760)
	/usr/local/go/src/cmd/go/internal/load/test.go:46 +0x39
cmd/go/internal/test.builderTest(0xc0000bf7c0, 0xc000080d80, 0x0, 0x0, 0x0, 0x1703780, 0xc000152030)
	/usr/local/go/src/cmd/go/internal/test/test.go:838 +0x6e
cmd/go/internal/test.runTest(0x19ce8e0, 0xc0000a8030, 0x0, 0x0)
	/usr/local/go/src/cmd/go/internal/test/test.go:748 +0x4d0
main.main()
	/usr/local/go/src/cmd/go/main.go:190 +0x58d

NOTE: The func foo has no body. When I add the body like func foo(int) {} it does not panic.

@jayconrod jayconrod changed the title cmd/go: go test panics when function without body present go/doc: Examples panics on a file containing a function without a body Nov 18, 2020
@jayconrod jayconrod added this to the Backlog milestone Nov 18, 2020
@qbradq
Copy link
Contributor

@qbradq qbradq commented Nov 29, 2020

I looked into this a bit. parser.ParseFile and friends do not generate an error for the function with no body but the compiler does. This results in tools like go test and gopls to panic in this way. Would it be appropriate to try to add this as an error condition to the parser or should I just patch src/go/doc/example.go ?

@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Nov 29, 2020

A function with no body is valid Go. It is expected to be provided by assembler code or C code or via go:linkname or via some other mechanism. See https://golang.org/ref/spec#Function_declarations .

I haven't looked at this and I'm not sure where it is going wrong, but it would not be correct to modify the parser to reject functions with no bodies.

@qbradq
Copy link
Contributor

@qbradq qbradq commented Nov 29, 2020

Forgive me if this is not constructive. When I try to run an application with a function with no body I get a compilation error. Go Playground Example. This is why I assumed it was not valid. Does that imply an error in the compiler?

@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Nov 29, 2020

Function bodies may only be omitted in packages that contain assembler code or use cgo or use go:linkname.

qbradq added a commit to qbradq/go that referenced this issue Dec 1, 2020
qbradq added a commit to qbradq/go that referenced this issue Dec 1, 2020
This change guards a call to ast.Inspect with a nil check
on the first argument. This avoids a panic when inspecting a
reference to a function with an external body.

Fixes golang#42706
@gopherbot
Copy link

@gopherbot gopherbot commented Dec 4, 2020

Change https://golang.org/cl/275516 mentions this issue: go/doc: avoid panic on references functions with no body

mvdan added a commit to qbradq/go that referenced this issue Mar 30, 2021
mvdan added a commit to qbradq/go that referenced this issue Mar 30, 2021
This change guards a call to ast.Inspect with a nil check
on the first argument. This avoids a panic when inspecting a
reference to a function with an external body.

Fixes golang#42706
@gopherbot gopherbot closed this in c40dc67 Mar 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

5 participants