[release-branch.go1.10] runtime: identify special functions by flag instead of address
When there are plugins, there may not be a unique copy of runtime
functions like goexit, mcall, etc. So identifying them by entry
address is problematic. Instead, keep track of each special function
using a field in the symbol table. That way, multiple copies of
the same runtime function will be treated identically.
Fixes #24351
Fixes #23133
Change-Id: Iea3232df8a6af68509769d9ca618f530cc0f84fd
Reviewed-on: https://go-review.googlesource.com/100739
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-on: https://go-review.googlesource.com/102793
Run-TryBot: Andrew Bonventre <andybons@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
diff --git a/misc/cgo/testplugin/src/issue24351/main.go b/misc/cgo/testplugin/src/issue24351/main.go
new file mode 100644
index 0000000..4107adf
--- /dev/null
+++ b/misc/cgo/testplugin/src/issue24351/main.go
@@ -0,0 +1,21 @@
+// Copyright 2018 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package main
+
+import "plugin"
+
+func main() {
+ p, err := plugin.Open("issue24351.so")
+ if err != nil {
+ panic(err)
+ }
+ f, err := p.Lookup("B")
+ if err != nil {
+ panic(err)
+ }
+ c := make(chan bool)
+ f.(func(chan bool))(c)
+ <-c
+}
diff --git a/misc/cgo/testplugin/src/issue24351/plugin.go b/misc/cgo/testplugin/src/issue24351/plugin.go
new file mode 100644
index 0000000..db17e0a
--- /dev/null
+++ b/misc/cgo/testplugin/src/issue24351/plugin.go
@@ -0,0 +1,14 @@
+// Copyright 2018 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package main
+
+import "fmt"
+
+func B(c chan bool) {
+ go func() {
+ fmt.Println(1.5)
+ c <- true
+ }()
+}
diff --git a/misc/cgo/testplugin/test.bash b/misc/cgo/testplugin/test.bash
index 18e3803..df38204 100755
--- a/misc/cgo/testplugin/test.bash
+++ b/misc/cgo/testplugin/test.bash
@@ -85,3 +85,8 @@
GOPATH=$(pwd) go build -gcflags "$GO_GCFLAGS" -buildmode=plugin -o issue.22295.so issue22295.pkg
GOPATH=$(pwd) go build -gcflags "$GO_GCFLAGS" -o issue22295 src/issue22295.pkg/main.go
./issue22295
+
+# Test for issue 24351
+GOPATH=$(pwd) go build -gcflags "$GO_GCFLAGS" -buildmode=plugin -o issue24351.so src/issue24351/plugin.go
+GOPATH=$(pwd) go build -gcflags "$GO_GCFLAGS" -o issue24351 src/issue24351/main.go
+./issue24351
diff --git a/src/cmd/internal/objabi/funcid.go b/src/cmd/internal/objabi/funcid.go
new file mode 100644
index 0000000..55f1328
--- /dev/null
+++ b/src/cmd/internal/objabi/funcid.go
@@ -0,0 +1,34 @@
+// Copyright 2018 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package objabi
+
+// A FuncID identifies particular functions that need to be treated
+// specially by the runtime.
+// Note that in some situations involving plugins, there may be multiple
+// copies of a particular special runtime function.
+// Note: this list must match the list in runtime/symtab.go.
+type FuncID uint32
+
+const (
+ FuncID_normal FuncID = iota // not a special function
+ FuncID_goexit
+ FuncID_jmpdefer
+ FuncID_mcall
+ FuncID_morestack
+ FuncID_mstart
+ FuncID_rt0_go
+ FuncID_asmcgocall
+ FuncID_sigpanic
+ FuncID_runfinq
+ FuncID_bgsweep
+ FuncID_forcegchelper
+ FuncID_timerproc
+ FuncID_gcBgMarkWorker
+ FuncID_systemstack_switch
+ FuncID_systemstack
+ FuncID_cgocallback_gofunc
+ FuncID_gogo
+ FuncID_externalthreadhandler
+)
diff --git a/src/cmd/link/internal/ld/pcln.go b/src/cmd/link/internal/ld/pcln.go
index 9d82677..8708924 100644
--- a/src/cmd/link/internal/ld/pcln.go
+++ b/src/cmd/link/internal/ld/pcln.go
@@ -312,12 +312,47 @@
}
off = int32(ftab.SetUint32(ctxt.Arch, int64(off), args))
- // frame int32
- // This has been removed (it was never set quite correctly anyway).
- // Nothing should use it.
- // Leave an obviously incorrect value.
- // TODO: Remove entirely.
- off = int32(ftab.SetUint32(ctxt.Arch, int64(off), 0x1234567))
+ // funcID uint32
+ funcID := objabi.FuncID_normal
+ switch s.Name {
+ case "runtime.goexit":
+ funcID = objabi.FuncID_goexit
+ case "runtime.jmpdefer":
+ funcID = objabi.FuncID_jmpdefer
+ case "runtime.mcall":
+ funcID = objabi.FuncID_mcall
+ case "runtime.morestack":
+ funcID = objabi.FuncID_morestack
+ case "runtime.mstart":
+ funcID = objabi.FuncID_mstart
+ case "runtime.rt0_go":
+ funcID = objabi.FuncID_rt0_go
+ case "runtime.asmcgocall":
+ funcID = objabi.FuncID_asmcgocall
+ case "runtime.sigpanic":
+ funcID = objabi.FuncID_sigpanic
+ case "runtime.runfinq":
+ funcID = objabi.FuncID_runfinq
+ case "runtime.bgsweep":
+ funcID = objabi.FuncID_bgsweep
+ case "runtime.forcegchelper":
+ funcID = objabi.FuncID_forcegchelper
+ case "runtime.timerproc":
+ funcID = objabi.FuncID_timerproc
+ case "runtime.gcBgMarkWorker":
+ funcID = objabi.FuncID_gcBgMarkWorker
+ case "runtime.systemstack_switch":
+ funcID = objabi.FuncID_systemstack_switch
+ case "runtime.systemstack":
+ funcID = objabi.FuncID_systemstack
+ case "runtime.cgocallback_gofunc":
+ funcID = objabi.FuncID_cgocallback_gofunc
+ case "runtime.gogo":
+ funcID = objabi.FuncID_gogo
+ case "runtime.externalthreadhandler":
+ funcID = objabi.FuncID_externalthreadhandler
+ }
+ off = int32(ftab.SetUint32(ctxt.Arch, int64(off), uint32(funcID)))
if pcln != &pclntabZpcln {
renumberfiles(ctxt, pcln.File, &pcln.Pcfile)
diff --git a/src/runtime/os_windows.go b/src/runtime/os_windows.go
index 7aeadd9..2653a94 100644
--- a/src/runtime/os_windows.go
+++ b/src/runtime/os_windows.go
@@ -304,8 +304,6 @@
disableWER()
- externalthreadhandlerp = funcPC(externalthreadhandler)
-
initExceptionHandler()
stdcall2(_SetConsoleCtrlHandler, funcPC(ctrlhandler), 1)
diff --git a/src/runtime/proc.go b/src/runtime/proc.go
index 2e958f7..6e40975 100644
--- a/src/runtime/proc.go
+++ b/src/runtime/proc.go
@@ -393,6 +393,11 @@
// funcPC returns the entry PC of the function f.
// It assumes that f is a func value. Otherwise the behavior is undefined.
+// CAREFUL: In programs with plugins, funcPC can return different values
+// for the same function (because there are actually multiple copies of
+// the same function in the address space). To be safe, don't use the
+// results of this function in any == expression. It is only safe to
+// use the result as an address at which to start executing code.
//go:nosplit
func funcPC(f interface{}) uintptr {
return **(**uintptr)(add(unsafe.Pointer(&f), sys.PtrSize))
@@ -3798,8 +3803,8 @@
// so assume the worst and stop traceback
return true
}
- switch f.entry {
- case gogoPC, systemstackPC, mcallPC, morestackPC:
+ switch f.funcID {
+ case funcID_gogo, funcID_systemstack, funcID_mcall, funcID_morestack:
return true
}
return false
diff --git a/src/runtime/runtime2.go b/src/runtime/runtime2.go
index 556f13d..a783f60 100644
--- a/src/runtime/runtime2.go
+++ b/src/runtime/runtime2.go
@@ -635,8 +635,8 @@
entry uintptr // start pc
nameoff int32 // function name
- args int32 // in/out args size
- _ int32 // previously legacy frame size; kept for layout compatibility
+ args int32 // in/out args size
+ funcID funcID // set for certain special runtime functions
pcsp int32
pcfile int32
diff --git a/src/runtime/stack.go b/src/runtime/stack.go
index 6149838..1bfd9dd 100644
--- a/src/runtime/stack.go
+++ b/src/runtime/stack.go
@@ -619,7 +619,7 @@
if stackDebug >= 2 {
print(" adjusting ", funcname(f), " frame=[", hex(frame.sp), ",", hex(frame.fp), "] pc=", hex(frame.pc), " continpc=", hex(frame.continpc), "\n")
}
- if f.entry == systemstack_switchPC {
+ if f.funcID == funcID_systemstack_switch {
// A special routine at the bottom of stack of a goroutine that does an systemstack call.
// We will allow it to be copied even though we don't
// have full GC info for it (because it is written in asm).
@@ -1110,7 +1110,8 @@
if debug.gcshrinkstackoff > 0 {
return
}
- if gp.startpc == gcBgMarkWorkerPC {
+ f := findfunc(gp.startpc)
+ if f.valid() && f.funcID == funcID_gcBgMarkWorker {
// We're not allowed to shrink the gcBgMarkWorker
// stack (see gcBgMarkWorker for explanation).
return
diff --git a/src/runtime/symtab.go b/src/runtime/symtab.go
index bdf98b9..f50284f 100644
--- a/src/runtime/symtab.go
+++ b/src/runtime/symtab.go
@@ -133,7 +133,7 @@
}
se.pcExpander.init(ncallers[0], se.wasPanic)
ncallers = ncallers[1:]
- se.wasPanic = se.pcExpander.funcInfo.valid() && se.pcExpander.funcInfo.entry == sigpanicPC
+ se.wasPanic = se.pcExpander.funcInfo.valid() && se.pcExpander.funcInfo.funcID == funcID_sigpanic
if se.skip > 0 {
for ; se.skip > 0; se.skip-- {
se.pcExpander.next()
@@ -349,6 +349,35 @@
_ArgsSizeUnknown = -0x80000000
)
+// A FuncID identifies particular functions that need to be treated
+// specially by the runtime.
+// Note that in some situations involving plugins, there may be multiple
+// copies of a particular special runtime function.
+// Note: this list must match the list in cmd/internal/objabi/funcid.go.
+type funcID uint32
+
+const (
+ funcID_normal funcID = iota // not a special function
+ funcID_goexit
+ funcID_jmpdefer
+ funcID_mcall
+ funcID_morestack
+ funcID_mstart
+ funcID_rt0_go
+ funcID_asmcgocall
+ funcID_sigpanic
+ funcID_runfinq
+ funcID_bgsweep
+ funcID_forcegchelper
+ funcID_timerproc
+ funcID_gcBgMarkWorker
+ funcID_systemstack_switch
+ funcID_systemstack
+ funcID_cgocallback_gofunc
+ funcID_gogo
+ funcID_externalthreadhandler
+)
+
// moduledata records information about the layout of the executable
// image. It is written by the linker. Any changes here must be
// matched changes to the code in cmd/internal/ld/symtab.go:symtab.
@@ -648,7 +677,6 @@
idx = uint32(len(datap.ftab) - 1)
}
if pc < datap.ftab[idx].entry {
-
// With multiple text sections, the idx might reference a function address that
// is higher than the pc being searched, so search backward until the matching address is found.
@@ -659,7 +687,6 @@
throw("findfunc: bad findfunctab entry idx")
}
} else {
-
// linear search to find func with pc >= entry.
for datap.ftab[idx+1].entry <= pc {
idx++
diff --git a/src/runtime/traceback.go b/src/runtime/traceback.go
index 747176c..06df938 100644
--- a/src/runtime/traceback.go
+++ b/src/runtime/traceback.go
@@ -35,56 +35,14 @@
const usesLR = sys.MinFrameSize > 0
-var (
- // initialized in tracebackinit
- goexitPC uintptr
- jmpdeferPC uintptr
- mcallPC uintptr
- morestackPC uintptr
- mstartPC uintptr
- rt0_goPC uintptr
- asmcgocallPC uintptr
- sigpanicPC uintptr
- runfinqPC uintptr
- bgsweepPC uintptr
- forcegchelperPC uintptr
- timerprocPC uintptr
- gcBgMarkWorkerPC uintptr
- systemstack_switchPC uintptr
- systemstackPC uintptr
- cgocallback_gofuncPC uintptr
- skipPC uintptr
-
- gogoPC uintptr
-
- externalthreadhandlerp uintptr // initialized elsewhere
-)
+var skipPC uintptr
func tracebackinit() {
// Go variable initialization happens late during runtime startup.
// Instead of initializing the variables above in the declarations,
// schedinit calls this function so that the variables are
// initialized and available earlier in the startup sequence.
- goexitPC = funcPC(goexit)
- jmpdeferPC = funcPC(jmpdefer)
- mcallPC = funcPC(mcall)
- morestackPC = funcPC(morestack)
- mstartPC = funcPC(mstart)
- rt0_goPC = funcPC(rt0_go)
- asmcgocallPC = funcPC(asmcgocall)
- sigpanicPC = funcPC(sigpanic)
- runfinqPC = funcPC(runfinq)
- bgsweepPC = funcPC(bgsweep)
- forcegchelperPC = funcPC(forcegchelper)
- timerprocPC = funcPC(timerproc)
- gcBgMarkWorkerPC = funcPC(gcBgMarkWorker)
- systemstack_switchPC = funcPC(systemstack_switch)
- systemstackPC = funcPC(systemstack)
- cgocallback_gofuncPC = funcPC(cgocallback_gofunc)
skipPC = funcPC(skipPleaseUseCallersFrames)
-
- // used by sigprof handler
- gogoPC = funcPC(gogo)
}
// Traceback over the deferred function calls.
@@ -137,9 +95,6 @@
if skip > 0 && callback != nil {
throw("gentraceback callback cannot be used with non-zero skip")
}
- if goexitPC == 0 {
- throw("gentraceback before goexitPC initialization")
- }
g := getg()
if g == gp && g == g.m.curg {
// The starting sp has been passed in as a uintptr, and the caller may
@@ -241,7 +196,7 @@
// g0, this systemstack is at the top of the stack.
// if we're not on g0 or there's a no curg, then this is a regular call.
sp := frame.sp
- if flags&_TraceJumpStack != 0 && f.entry == systemstackPC && gp == g.m.g0 && gp.m.curg != nil {
+ if flags&_TraceJumpStack != 0 && f.funcID == funcID_systemstack && gp == g.m.g0 && gp.m.curg != nil {
sp = gp.m.curg.sched.sp
frame.sp = sp
cgoCtxt = gp.m.curg.cgoCtxt
@@ -256,7 +211,7 @@
if topofstack(f, gp.m != nil && gp == gp.m.g0) {
frame.lr = 0
flr = funcInfo{}
- } else if usesLR && f.entry == jmpdeferPC {
+ } else if usesLR && f.funcID == funcID_jmpdefer {
// jmpdefer modifies SP/LR/PC non-atomically.
// If a profiling interrupt arrives during jmpdefer,
// the stack unwind may see a mismatched register set
@@ -287,7 +242,7 @@
// But if callback is set, we're doing a garbage collection and must
// get everything, so crash loudly.
doPrint := printing
- if doPrint && gp.m.incgo {
+ if doPrint && gp.m.incgo && f.funcID == funcID_sigpanic {
// We can inject sigpanic
// calls directly into C code,
// in which case we'll see a C
@@ -396,8 +351,8 @@
// if there's room, pcbuf[1] is a skip PC that encodes the number of skipped frames in pcbuf[0]
if n+1 < max {
n++
- skipPC := funcPC(skipPleaseUseCallersFrames) + uintptr(logicalSkipped)
- (*[1 << 20]uintptr)(unsafe.Pointer(pcbuf))[n] = skipPC
+ pc := skipPC + uintptr(logicalSkipped)
+ (*[1 << 20]uintptr)(unsafe.Pointer(pcbuf))[n] = pc
}
}
}
@@ -466,7 +421,7 @@
n++
skipped:
- if f.entry == cgocallback_gofuncPC && len(cgoCtxt) > 0 {
+ if f.funcID == funcID_cgocallback_gofunc && len(cgoCtxt) > 0 {
ctxt := cgoCtxt[len(cgoCtxt)-1]
cgoCtxt = cgoCtxt[:len(cgoCtxt)-1]
@@ -478,7 +433,7 @@
}
}
- waspanic = f.entry == sigpanicPC
+ waspanic = f.funcID == funcID_sigpanic
// Do not unwind past the bottom of the stack.
if !flr.valid() {
@@ -931,30 +886,32 @@
// Does f mark the top of a goroutine stack?
func topofstack(f funcInfo, g0 bool) bool {
- pc := f.entry
- return pc == goexitPC ||
- pc == mstartPC ||
- pc == mcallPC ||
- pc == morestackPC ||
- pc == rt0_goPC ||
- externalthreadhandlerp != 0 && pc == externalthreadhandlerp ||
+ return f.funcID == funcID_goexit ||
+ f.funcID == funcID_mstart ||
+ f.funcID == funcID_mcall ||
+ f.funcID == funcID_morestack ||
+ f.funcID == funcID_rt0_go ||
+ f.funcID == funcID_externalthreadhandler ||
// asmcgocall is TOS on the system stack because it
// switches to the system stack, but in this case we
// can come back to the regular stack and still want
// to be able to unwind through the call that appeared
// on the regular stack.
- (g0 && pc == asmcgocallPC)
+ (g0 && f.funcID == funcID_asmcgocall)
}
// isSystemGoroutine reports whether the goroutine g must be omitted in
// stack dumps and deadlock detector.
func isSystemGoroutine(gp *g) bool {
- pc := gp.startpc
- return pc == runfinqPC && !fingRunning ||
- pc == bgsweepPC ||
- pc == forcegchelperPC ||
- pc == timerprocPC ||
- pc == gcBgMarkWorkerPC
+ f := findfunc(gp.startpc)
+ if !f.valid() {
+ return false
+ }
+ return f.funcID == funcID_runfinq && !fingRunning ||
+ f.funcID == funcID_bgsweep ||
+ f.funcID == funcID_forcegchelper ||
+ f.funcID == funcID_timerproc ||
+ f.funcID == funcID_gcBgMarkWorker
}
// SetCgoTraceback records three C functions to use to gather
