]> Cypherpunks repositories - gostls13.git/commitdiff
cmd/link: fix deferreturn detector
authorKeith Randall <keithr@alum.mit.edu>
Fri, 7 Jun 2019 17:22:53 +0000 (13:22 -0400)
committerKeith Randall <khr@golang.org>
Fri, 7 Jun 2019 18:51:04 +0000 (18:51 +0000)
The logic for detecting deferreturn calls is wrong.

We used to look for a relocation whose symbol is runtime.deferreturn
and has an offset of 0. But on some architectures, the relocation
offset is not zero. These include arm (the offset is 0xebfffffe) and
s390x (the offset is 6).

This ends up setting the deferreturn offset at 0, so we end up using
the entry point live map instead of the deferreturn live map in a
frame which defers and then segfaults.

Instead, use the IsDirectCall helper to find calls.

Fixes #32477
Update #6980

Change-Id: Iecb530a7cf6eabd7233be7d0731ffa78873f3a54
Reviewed-on: https://go-review.googlesource.com/c/go/+/181258
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
src/cmd/link/internal/ld/pcln.go
test/fixedbugs/issue32477.go [new file with mode: 0644]

index 6c0a9e9ebcb78bceddae44cdeb1f81ec96fe4654..cd8151022a8c435b5b697f1cccd143a46dc68193 100644 (file)
@@ -318,7 +318,7 @@ func (ctxt *Link) pclntab() {
                                // set the resumption point to PC_B.
                                lastWasmAddr = uint32(r.Add)
                        }
-                       if r.Sym != nil && r.Sym.Name == "runtime.deferreturn" && r.Add == 0 {
+                       if r.Type.IsDirectJump() && r.Sym != nil && r.Sym.Name == "runtime.deferreturn" {
                                if ctxt.Arch.Family == sys.Wasm {
                                        deferreturn = lastWasmAddr
                                } else {
diff --git a/test/fixedbugs/issue32477.go b/test/fixedbugs/issue32477.go
new file mode 100644 (file)
index 0000000..8b3c175
--- /dev/null
@@ -0,0 +1,71 @@
+// run
+
+// Copyright 2019 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.
+
+// Make sure we use the deferreturn live map instead of
+// the entry live map when handling a segv in a function
+// that defers.
+
+package main
+
+import "runtime"
+
+var finalized bool
+var err string
+
+type HeapObj [8]int64
+
+const filler int64 = 0x123456789abcdef0
+
+func (h *HeapObj) init() {
+       for i := 0; i < len(*h); i++ {
+               h[i] = filler
+       }
+}
+func (h *HeapObj) check() {
+       for i := 0; i < len(*h); i++ {
+               if h[i] != filler {
+                       err = "filler overwritten"
+               }
+       }
+}
+
+func gc(shouldFinalize bool) {
+       runtime.GC()
+       runtime.GC()
+       runtime.GC()
+       if shouldFinalize != finalized {
+               err = "heap object finalized at the wrong time"
+       }
+}
+
+func main() {
+       h := new(HeapObj)
+       h.init()
+       runtime.SetFinalizer(h, func(h *HeapObj) {
+               finalized = true
+       })
+
+       gc(false)
+       g(h)
+       if err != "" {
+               panic(err)
+       }
+}
+
+func g(h *HeapObj) {
+       gc(false)
+       h.check()
+       // h is now unused
+       defer func() {
+               // h should not be live here. Previously we used to
+               // use the function entry point as the place to get
+               // the live map when handling a segv.
+               gc(true)
+               recover()
+       }()
+       *(*int)(nil) = 0 // trigger a segv
+       return
+}