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

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
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone
Backlog

Comments

@bmizerany
Copy link
Contributor

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 the NeedsFix The path to resolution is known, but the work has not been done. label Nov 18, 2020
@jayconrod jayconrod added this to the Backlog milestone Nov 18, 2020
@qbradq
Copy link
Contributor

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

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 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

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 failing test case for golang#42706
bd7c640
qbradq added a commit to qbradq/go that referenced this issue Dec 1, 2020
@qbradq
go/doc: avoid panic on reference to external function
c6183b7 
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
@qbradq qbradq mentioned this issue Dec 4, 2020
Closed
@gopherbot
Copy link

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

mvdan pushed a commit to qbradq/go that referenced this issue Mar 30, 2021
@qbradq @mvdan
Added failing test case for golang#42706
58eb10b
mvdan pushed a commit to qbradq/go that referenced this issue Mar 30, 2021
@qbradq @mvdan
go/doc: avoid panic on reference to external function
08072b9 
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
@golang golang locked and limited conversation to collaborators Mar 30, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Milestone
Backlog
Development

Successfully merging a pull request may close this issue.

go/doc: avoid panic on references to functions with no body qbradq/go
5 participants
@bmizerany @jayconrod @qbradq @ianlancetaylor @gopherbot