runtime: FuncForPC with invalid PC causes fatal error

[rhysh]

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

$ go1.11 version
go version go1.11.4 darwin/amd64
$ go version
go version devel +4b3f04c63b Thu Jan 10 18:15:48 2019 +0000 darwin/amd64
$ go-tip version
go version devel +70931c087b Mon Jan 14 19:44:57 2019 +0000 darwin/amd64

Does this issue reproduce with the latest release?

The issue is present in go1.12beta2 and in the current HEAD of master.

I bisected and found that 956879d (for #29582) introduced the issue.

CC @randall77 @ianlancetaylor

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

go env Output

$ go env
GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/rhys/Library/Caches/go-build"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOOS="darwin"
GOPATH="/Users/rhys/go"
GOPROXY=""
GORACE=""
GOROOT="/usr/local/go"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/darwin_amd64"
GCCGO="gccgo"
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/49/zmds5zsn75z1283vtzxyfr5hj7yjq4/T/go-build282935932=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

I have a package that calls runtime.FuncForPC several times, sometimes with PCs that don't correspond to function bodies. Previously, the returned *runtime.Func would correspond to a nearby function. Now, runtime.FuncForPC throws a fatal error.

package main

import (
	"fmt"
	"runtime"
)

func main() {
	var stack [1]uintptr
	runtime.Callers(1, stack[:])

	prev := ""
	for i := 0; i < 1000; i++ {
		fn := runtime.FuncForPC(stack[0] + uintptr(i))
		name := fn.Name()
		if prev != name {
			fmt.Println(name)
			prev = name
		}
	}
}

What did you expect to see?

I expected calls to runtime.FuncForPC with invalid arguments to return nil or a nearby *runtime.Func.

$ go1.11 run /tmp/repro.go 
main.main
main.init

What did you see instead?

The function instead throws a fatal error, immediately halting the program.

$ go run /tmp/repro.go 
main.main
fmt.Println
main.main
runtime: invalid pc-encoded table f=main.main pc=0x1092048 targetpc=0x1092048 tab=[0/0]0x0
	value=-1 until pc=0x1091faf
	value=0 until pc=0x1091fe7
	value=-1 until pc=0x1092048
fatal error: invalid runtime symbol table

goroutine 1 [running]:
runtime.throw(0x10c8598, 0x1c)
	/usr/local/go/src/runtime/panic.go:617 +0x72 fp=0xc000080d98 sp=0xc000080d68 pc=0x1027b52
runtime.pcvalue(0x1157f78, 0x115f200, 0x76c07, 0x1092048, 0x0, 0x1092001, 0x115f200)
	/usr/local/go/src/runtime/symtab.go:700 +0x53f fp=0xc000080e40 sp=0xc000080d98 pc=0x1041cbf
runtime.pcdatavalue(0x1157f78, 0x115f200, 0x1, 0x1092048, 0x0, 0x1157fd8)
	/usr/local/go/src/runtime/symtab.go:764 +0x87 fp=0xc000080e88 sp=0xc000080e40 pc=0x10422e7
runtime.FuncForPC(0x1092048, 0x1157fd8)
	/usr/local/go/src/runtime/symtab.go:477 +0xc0 fp=0xc000080f08 sp=0xc000080e88 pc=0x10413c0
main.main()
	/tmp/repro.go:14 +0xa6 fp=0xc000080f98 sp=0xc000080f08 pc=0x1091f56
runtime.main()
	/usr/local/go/src/runtime/proc.go:200 +0x20c fp=0xc000080fe0 sp=0xc000080f98 pc=0x102948c
runtime.goexit()
	/usr/local/go/src/runtime/asm_amd64.s:1337 +0x1 fp=0xc000080fe8 sp=0xc000080fe0 pc=0x1050521
exit status 2

$ go-tip run /tmp/repro.go 
main.main
fmt.Println
main.main
runtime: invalid pc-encoded table f=main.main pc=0x1092048 targetpc=0x1092048 tab=[0/0]0x0
	value=-1 until pc=0x1091faf
	value=0 until pc=0x1091fe7
	value=-1 until pc=0x1092048
fatal error: invalid runtime symbol table

goroutine 1 [running]:
runtime.throw(0x10c8598, 0x1c)
	/usr/local/go/src/runtime/panic.go:617 +0x72 fp=0xc000082d98 sp=0xc000082d68 pc=0x1027b52
runtime.pcvalue(0x1157f78, 0x115f200, 0x76c07, 0x1092048, 0x0, 0x1092001, 0x115f200)
	/usr/local/go/src/runtime/symtab.go:700 +0x53f fp=0xc000082e40 sp=0xc000082d98 pc=0x1041cbf
runtime.pcdatavalue(0x1157f78, 0x115f200, 0x1, 0x1092048, 0x0, 0x1157fd8)
	/usr/local/go/src/runtime/symtab.go:764 +0x87 fp=0xc000082e88 sp=0xc000082e40 pc=0x10422e7
runtime.FuncForPC(0x1092048, 0x1157fd8)
	/usr/local/go/src/runtime/symtab.go:477 +0xc0 fp=0xc000082f08 sp=0xc000082e88 pc=0x10413c0
main.main()
	/tmp/repro.go:14 +0xa6 fp=0xc000082f98 sp=0xc000082f08 pc=0x1091f56
runtime.main()
	/usr/local/go/src/runtime/proc.go:200 +0x20c fp=0xc000082fe0 sp=0xc000082f98 pc=0x102948c
runtime.goexit()
	/usr/local/go/src/runtime/asm_amd64.s:1337 +0x1 fp=0xc000082fe8 sp=0xc000082fe0 pc=0x1050521
exit status 2

[randall77]

Not really supported behavior, but we should probably fix it.

For PCs which occur after the last instruction of a function, but before the first instruction of the next function (which exist because we pad to align function starts to 16 bytes), we get in trouble. findfunc returns the previous function, but the pcln tables don't cover those instructions. We now need the pcln tables to figure out whether that pc is in an inlined function or not.

We should really have findfunc return nil for those instructions. Unfortunately, we don't have the function size anywhere convenient at runtime. I'll see if I can find a way to make that happen.

The other solution is to extend the pcln tables to cover the padding instructions, and/or have the pcln reader not barf if we read off the end of the table.

[randall77]

CL out using that very last idea I mentioned.
We already protect ourselves for bad arguments to FileLine, might as well do FuncForPC also.

[gopherbot]

Change https://golang.org/cl/157799 mentions this issue: runtime: keep FuncForPC from crashing for PCs between functions