]> Cypherpunks repositories - gostls13.git/commitdiff
[release-branch.go1.10] runtime: identify special functions by flag instead of address
authorKeith Randall <khr@golang.org>
Wed, 14 Mar 2018 22:21:37 +0000 (15:21 -0700)
committerAndrew Bonventre <andybons@golang.org>
Thu, 29 Mar 2018 06:09:45 +0000 (06:09 +0000)
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>
misc/cgo/testplugin/src/issue24351/main.go [new file with mode: 0644]
misc/cgo/testplugin/src/issue24351/plugin.go [new file with mode: 0644]
misc/cgo/testplugin/test.bash
src/cmd/internal/objabi/funcid.go [new file with mode: 0644]
src/cmd/link/internal/ld/pcln.go
src/runtime/os_windows.go
src/runtime/proc.go
src/runtime/runtime2.go
src/runtime/stack.go
src/runtime/symtab.go
src/runtime/traceback.go

diff --git a/misc/cgo/testplugin/src/issue24351/main.go b/misc/cgo/testplugin/src/issue24351/main.go
new file mode 100644 (file)
index 0000000..4107adf
--- /dev/null
@@ -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 (file)
index 0000000..db17e0a
--- /dev/null
@@ -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
+       }()
+}
index 18e3803bf42aee1d2deec227e764885ba6d39cd2..df38204a4e9512353a138dc429acb17110b7485c 100755 (executable)
@@ -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
diff --git a/src/cmd/internal/objabi/funcid.go b/src/cmd/internal/objabi/funcid.go
new file mode 100644 (file)
index 0000000..55f1328
--- /dev/null
@@ -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
+)
index 9d82677059b824cf0c0c1a79751d7082cf2dea56..8708924d92a2709b5b18f17e6e08859b106bf5d9 100644 (file)
@@ -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)
index 7aeadd9ef1252f597c41ce5a5dcba88a44138178..2653a94169f585c2ece1751bf7b96a1ae60777b9 100644 (file)
@@ -304,8 +304,6 @@ func osinit() {
 
        disableWER()
 
-       externalthreadhandlerp = funcPC(externalthreadhandler)
-
        initExceptionHandler()
 
        stdcall2(_SetConsoleCtrlHandler, funcPC(ctrlhandler), 1)
index 2e958f7fc597b359a174da414fb9cd688d0005e9..6e4097517df7b9b4dbfe12510e94635405704c05 100644 (file)
@@ -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
index 556f13d1c13822eb4243ca09486512f8ed7f9f45..a783f60ae463fb250001b12473939901fc8882df 100644 (file)
@@ -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
index 6149838b6d62752f8c8264effed6ac613ff47f51..1bfd9dd27f3ffbecec63865d9ab773a35c86bcea 100644 (file)
@@ -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
index bdf98b9e9dc63798d41feca071490c91e4d5e45b..f50284fb6f5efc02ccbe1ce38666d5e1cd48c972 100644 (file)
@@ -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++
index 747176c278202757fb62591f2cd0b891570b1818..06df9385fd03ee4eaba0bff769ef400a8cf29ff7 100644 (file)
@@ -35,56 +35,14 @@ import (
 
 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 @@ func gentraceback(pc0, sp0, lr0 uintptr, gp *g, skip int, pcbuf *uintptr, max in
        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 @@ func gentraceback(pc0, sp0, lr0 uintptr, gp *g, skip int, pcbuf *uintptr, max in
                        // 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 @@ func gentraceback(pc0, sp0, lr0 uintptr, gp *g, skip int, pcbuf *uintptr, max in
                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 @@ func gentraceback(pc0, sp0, lr0 uintptr, gp *g, skip int, pcbuf *uintptr, max in
                                // 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 @@ func gentraceback(pc0, sp0, lr0 uintptr, gp *g, skip int, pcbuf *uintptr, max in
                                // 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 @@ func gentraceback(pc0, sp0, lr0 uintptr, gp *g, skip int, pcbuf *uintptr, max in
                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 @@ func gentraceback(pc0, sp0, lr0 uintptr, gp *g, skip int, pcbuf *uintptr, max in
                        }
                }
 
-               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 @@ func tracebackHexdump(stk stack, frame *stkframe, bad uintptr) {
 
 // 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