aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorCherry Zhang <cherryyz@google.com>2021-03-12 23:21:09 -0500
committerAlexander Rakoczy <alex@golang.org>2021-03-25 18:33:03 +0000
commitac59d7abb9ab5ec31cb0fe1eaccbdf3b17edbde0 (patch)
tree79ded1fa3e28b9c65b5512f2ed091ab6fcb56363
parent33fb47921f65d941b232da2ab542191c9978b4dc (diff)
downloadgo-ac59d7abb9ab5ec31cb0fe1eaccbdf3b17edbde0.tar.gz
go-ac59d7abb9ab5ec31cb0fe1eaccbdf3b17edbde0.zip
[release-branch.go1.16] cmd/compile, cmd/link: dynamically export writable static tmps
Static tmps are private to a package, but with plugins a package can be shared among multiple DSOs. They need to have a consistent view of the static tmps, especially for writable ones. So export them. (Read-only static tmps have the same values anyway, so it doesn't matter. Also Mach-O doesn't support dynamically exporting read-only symbols anyway.) Updates #44956. Fixes #45030. Change-Id: I921e25b7ab73cd5d5347800eccdb7931e3448779 Reviewed-on: https://go-review.googlesource.com/c/go/+/301793 Trust: Cherry Zhang <cherryyz@google.com> Run-TryBot: Cherry Zhang <cherryyz@google.com> TryBot-Result: Go Bot <gobot@golang.org> Reviewed-by: Than McIntosh <thanm@google.com> (cherry picked from commit de012bc095359e1b552d4ea6fb6b2995f3ab04f5) Reviewed-on: https://go-review.googlesource.com/c/go/+/302449
-rw-r--r--misc/cgo/testplugin/plugin_test.go7
-rw-r--r--misc/cgo/testplugin/testdata/issue44956/base/base.go7
-rw-r--r--misc/cgo/testplugin/testdata/issue44956/main.go47
-rw-r--r--misc/cgo/testplugin/testdata/issue44956/plugin1.go9
-rw-r--r--misc/cgo/testplugin/testdata/issue44956/plugin2.go11
-rw-r--r--src/cmd/compile/internal/gc/sinit.go4
-rw-r--r--src/cmd/link/internal/ld/symtab.go13
7 files changed, 93 insertions, 5 deletions
diff --git a/misc/cgo/testplugin/plugin_test.go b/misc/cgo/testplugin/plugin_test.go
index 2d991012c8..8869528015 100644
--- a/misc/cgo/testplugin/plugin_test.go
+++ b/misc/cgo/testplugin/plugin_test.go
@@ -209,3 +209,10 @@ func TestMethod2(t *testing.T) {
goCmd(t, "build", "-o", "method2.exe", "./method2/main.go")
run(t, "./method2.exe")
}
+
+func TestIssue44956(t *testing.T) {
+ goCmd(t, "build", "-buildmode=plugin", "-o", "issue44956p1.so", "./issue44956/plugin1.go")
+ goCmd(t, "build", "-buildmode=plugin", "-o", "issue44956p2.so", "./issue44956/plugin2.go")
+ goCmd(t, "build", "-o", "issue44956.exe", "./issue44956/main.go")
+ run(t, "./issue44956.exe")
+}
diff --git a/misc/cgo/testplugin/testdata/issue44956/base/base.go b/misc/cgo/testplugin/testdata/issue44956/base/base.go
new file mode 100644
index 0000000000..609aa0dff4
--- /dev/null
+++ b/misc/cgo/testplugin/testdata/issue44956/base/base.go
@@ -0,0 +1,7 @@
+// Copyright 2021 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 base
+
+var X = &map[int]int{123: 456}
diff --git a/misc/cgo/testplugin/testdata/issue44956/main.go b/misc/cgo/testplugin/testdata/issue44956/main.go
new file mode 100644
index 0000000000..287a60585e
--- /dev/null
+++ b/misc/cgo/testplugin/testdata/issue44956/main.go
@@ -0,0 +1,47 @@
+// Copyright 2021 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.
+
+// Issue 44956: writable static temp is not exported correctly.
+// In the test below, package base is
+//
+// X = &map{...}
+//
+// which compiles to
+//
+// X = &stmp // static
+// stmp = makemap(...) // in init function
+//
+// plugin1 and plugin2 both import base. plugin1 doesn't use
+// base.X, so that symbol is deadcoded in plugin1.
+//
+// plugin1 is loaded first. base.init runs at that point, which
+// initialize base.stmp.
+//
+// plugin2 is then loaded. base.init already ran, so it doesn't run
+// again. When base.stmp is not exported, plugin2's base.X points to
+// its own private base.stmp, which is not initialized, fail.
+
+package main
+
+import "plugin"
+
+func main() {
+ _, err := plugin.Open("issue44956p1.so")
+ if err != nil {
+ panic("FAIL")
+ }
+
+ p2, err := plugin.Open("issue44956p2.so")
+ if err != nil {
+ panic("FAIL")
+ }
+ f, err := p2.Lookup("F")
+ if err != nil {
+ panic("FAIL")
+ }
+ x := f.(func() *map[int]int)()
+ if x == nil || (*x)[123] != 456 {
+ panic("FAIL")
+ }
+}
diff --git a/misc/cgo/testplugin/testdata/issue44956/plugin1.go b/misc/cgo/testplugin/testdata/issue44956/plugin1.go
new file mode 100644
index 0000000000..499fa31abf
--- /dev/null
+++ b/misc/cgo/testplugin/testdata/issue44956/plugin1.go
@@ -0,0 +1,9 @@
+// Copyright 2021 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 _ "testplugin/issue44956/base"
+
+func main() {}
diff --git a/misc/cgo/testplugin/testdata/issue44956/plugin2.go b/misc/cgo/testplugin/testdata/issue44956/plugin2.go
new file mode 100644
index 0000000000..a73542ca71
--- /dev/null
+++ b/misc/cgo/testplugin/testdata/issue44956/plugin2.go
@@ -0,0 +1,11 @@
+// Copyright 2021 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 "testplugin/issue44956/base"
+
+func F() *map[int]int { return base.X }
+
+func main() {}
diff --git a/src/cmd/compile/internal/gc/sinit.go b/src/cmd/compile/internal/gc/sinit.go
index 212fcc022d..4d0837bc74 100644
--- a/src/cmd/compile/internal/gc/sinit.go
+++ b/src/cmd/compile/internal/gc/sinit.go
@@ -363,15 +363,15 @@ func staticname(t *types.Type) *Node {
n := newname(lookup(fmt.Sprintf("%s%d", obj.StaticNamePref, statuniqgen)))
statuniqgen++
addvar(n, t, PEXTERN)
- n.Sym.Linksym().Set(obj.AttrLocal, true)
return n
}
-// readonlystaticname returns a name backed by a (writable) static data symbol.
+// readonlystaticname returns a name backed by a read-only static data symbol.
func readonlystaticname(t *types.Type) *Node {
n := staticname(t)
n.MarkReadonly()
n.Sym.Linksym().Set(obj.AttrContentAddressable, true)
+ n.Sym.Linksym().Set(obj.AttrLocal, true)
return n
}
diff --git a/src/cmd/link/internal/ld/symtab.go b/src/cmd/link/internal/ld/symtab.go
index c98e4de03f..f54cf9ea2f 100644
--- a/src/cmd/link/internal/ld/symtab.go
+++ b/src/cmd/link/internal/ld/symtab.go
@@ -31,6 +31,7 @@
package ld
import (
+ "cmd/internal/obj"
"cmd/internal/objabi"
"cmd/link/internal/loader"
"cmd/link/internal/sym"
@@ -102,10 +103,14 @@ func putelfsym(ctxt *Link, x loader.Sym, typ elf.SymType, curbind elf.SymBind) {
elfshnum = xosect.Elfsect.(*ElfShdr).shnum
}
+ sname := ldr.SymExtname(x)
+
// One pass for each binding: elf.STB_LOCAL, elf.STB_GLOBAL,
// maybe one day elf.STB_WEAK.
bind := elf.STB_GLOBAL
- if ldr.IsFileLocal(x) || ldr.AttrVisibilityHidden(x) || ldr.AttrLocal(x) {
+ if ldr.IsFileLocal(x) && !isStaticTmp(sname) || ldr.AttrVisibilityHidden(x) || ldr.AttrLocal(x) {
+ // Static tmp is package local, but a package can be shared among multiple DSOs.
+ // They need to have a single view of the static tmp that are writable.
bind = elf.STB_LOCAL
}
@@ -140,8 +145,6 @@ func putelfsym(ctxt *Link, x loader.Sym, typ elf.SymType, curbind elf.SymBind) {
other |= 3 << 5
}
- sname := ldr.SymExtname(x)
-
// When dynamically linking, we create Symbols by reading the names from
// the symbol tables of the shared libraries and so the names need to
// match exactly. Tools like DTrace will have to wait for now.
@@ -823,3 +826,7 @@ func setCarrierSize(typ sym.SymKind, sz int64) {
}
CarrierSymByType[typ].Size = sz
}
+
+func isStaticTmp(name string) bool {
+ return strings.Contains(name, "."+obj.StaticNamePref)
+}