[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 <[email protected]>
TryBot-Result: Gobot Gobot <[email protected]>
Reviewed-by: Ian Lance Taylor <[email protected]>
Reviewed-on: https://go-review.googlesource.com/102793
Run-TryBot: Andrew Bonventre <[email protected]>
Reviewed-by: Keith Randall <[email protected]>

Author: randall77

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

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
+	}()
+}

misc/cgo/testplugin/test.bash

@@ -85,3 +85,8 @@ GOPATH=$(pwd) go build -gcflags "$GO_GCFLAGS" -o issue22175 src/issue22175/main.
 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

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

src/cmd/link/internal/ld/pcln.go

@@ -312,12 +312,47 @@ func (ctxt *Link) pclntab() {
 		}
 		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)

src/runtime/os_windows.go

@@ -304,8 +304,6 @@ func osinit() {
 
 	disableWER()
 
-	externalthreadhandlerp = funcPC(externalthreadhandler)
-
 	initExceptionHandler()
 
 	stdcall2(_SetConsoleCtrlHandler, funcPC(ctrlhandler), 1)

src/runtime/proc.go

@@ -393,6 +393,11 @@ func releaseSudog(s *sudog) {
 
 // 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 @@ func setsSP(pc uintptr) bool {
 		// 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

src/runtime/runtime2.go

@@ -635,8 +635,8 @@ type _func struct {
 	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

src/runtime/stack.go

@@ -619,7 +619,7 @@ func adjustframe(frame *stkframe, arg unsafe.Pointer) bool {
 	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 @@ func shrinkstack(gp *g) {
 	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

src/runtime/symtab.go

@@ -133,7 +133,7 @@ again:
 		}
 		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 @@ const (
 	_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 @@ func findfunc(pc uintptr) funcInfo {
 		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 @@ func findfunc(pc uintptr) funcInfo {
 			throw("findfunc: bad findfunctab entry idx")
 		}
 	} else {
-
 		// linear search to find func with pc >= entry.
 		for datap.ftab[idx+1].entry <= pc {
 			idx++