cmd/compile: separate data and function LSyms
Currently, obj.Ctxt's symbol table does not distinguish between ABI0
and ABIInternal symbols. This is *almost* okay, since a given symbol
name in the final object file is only going to belong to one ABI or
the other, but it requires that the compiler mark a Sym as being a
function symbol before it retrieves its LSym. If it retrieves the LSym
first, that LSym will be created as ABI0, and later marking the Sym as
a function symbol won't change the LSym's ABI.
Marking a Sym as a function symbol before looking up its LSym sounds
easy, except Syms have a dual purpose: they are used just as interned
strings (every function, variable, parameter, etc with the same
textual name shares a Sym), and *also* to store state for whatever
package global has that name. As a result, it's easy to slip up and
look up an LSym when a Sym is serving as the name of a local variable,
and then later mark it as a function when it's serving as the global
with the name.
In general, we were careful to avoid this, but #29610 demonstrates one
case where we messed up. Because of on-demand importing from indexed
export data, it's possible to compile a method wrapper for a type
imported from another package before importing an init function from
that package. If the argument of the method is named "init", the
"init" LSym will be created as a data symbol when compiling the
wrapper, before it gets marked as a function symbol.
To fix this, we separate obj.Ctxt's symbol tables for ABI0 and
ABIInternal symbols. This way, the compiler will simply get a
different LSym once the Sym takes on its package-global meaning as a
function.
This fixes the above ordering issue, and means we no longer need to go
out of our way to create the "init" function early and mark it as a
function symbol.
Fixes #29610.
Updates #27539.
Change-Id: Id9458b40017893d46ef9e4a3f9b47fc49e1ce8df
Reviewed-on: https://go-review.googlesource.com/c/157017
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
diff --git a/src/cmd/compile/internal/gc/main.go b/src/cmd/compile/internal/gc/main.go
index f44d19b..98ff2a3 100644
--- a/src/cmd/compile/internal/gc/main.go
+++ b/src/cmd/compile/internal/gc/main.go
@@ -563,11 +563,6 @@
errorexit()
}
- // The "init" function is the only user-spellable symbol that
- // we construct later. Mark it as a function now before
- // anything can ask for its Linksym.
- lookup("init").SetFunc(true)
-
// Phase 4: Decide how to capture closed variables.
// This needs to run before escape analysis,
// because variables captured by value do not escape.
diff --git a/src/cmd/compile/internal/gc/ssa.go b/src/cmd/compile/internal/gc/ssa.go
index db26f13..e201376 100644
--- a/src/cmd/compile/internal/gc/ssa.go
+++ b/src/cmd/compile/internal/gc/ssa.go
@@ -106,7 +106,7 @@
WasmDiv = sysvar("wasmDiv")
WasmTruncS = sysvar("wasmTruncS")
WasmTruncU = sysvar("wasmTruncU")
- SigPanic = sysvar("sigpanic")
+ SigPanic = sysfunc("sigpanic")
}
// buildssa builds an SSA function for fn.
diff --git a/src/cmd/compile/internal/types/sym.go b/src/cmd/compile/internal/types/sym.go
index 86f5022..13761c7 100644
--- a/src/cmd/compile/internal/types/sym.go
+++ b/src/cmd/compile/internal/types/sym.go
@@ -79,9 +79,7 @@
}
if sym.Func() {
// This is a function symbol. Mark it as "internal ABI".
- return Ctxt.LookupInit(sym.LinksymName(), func(s *obj.LSym) {
- s.SetABI(obj.ABIInternal)
- })
+ return Ctxt.LookupABI(sym.LinksymName(), obj.ABIInternal)
}
return Ctxt.Lookup(sym.LinksymName())
}
diff --git a/src/cmd/internal/obj/arm/asm5.go b/src/cmd/internal/obj/arm/asm5.go
index 316937b..b1fb1d3 100644
--- a/src/cmd/internal/obj/arm/asm5.go
+++ b/src/cmd/internal/obj/arm/asm5.go
@@ -1529,8 +1529,7 @@
return
}
- deferreturn = ctxt.Lookup("runtime.deferreturn")
- deferreturn.SetABI(obj.ABIInternal)
+ deferreturn = ctxt.LookupABI("runtime.deferreturn", obj.ABIInternal)
symdiv = ctxt.Lookup("runtime._div")
symdivu = ctxt.Lookup("runtime._divu")
diff --git a/src/cmd/internal/obj/link.go b/src/cmd/internal/obj/link.go
index 7df8e2e..f506f60 100644
--- a/src/cmd/internal/obj/link.go
+++ b/src/cmd/internal/obj/link.go
@@ -626,8 +626,9 @@
Flag_locationlists bool
Bso *bufio.Writer
Pathname string
- hashmu sync.Mutex // protects hash
+ hashmu sync.Mutex // protects hash, funchash
hash map[string]*LSym // name -> sym mapping
+ funchash map[string]*LSym // name -> sym mapping for ABIInternal syms
statichash map[string]*LSym // name -> sym mapping for static syms
PosTable src.PosTable
InlTree InlTree // global inlining tree used by gc/inl.go
diff --git a/src/cmd/internal/obj/sym.go b/src/cmd/internal/obj/sym.go
index 3fc17fa..15a501c 100644
--- a/src/cmd/internal/obj/sym.go
+++ b/src/cmd/internal/obj/sym.go
@@ -41,6 +41,7 @@
func Linknew(arch *LinkArch) *Link {
ctxt := new(Link)
ctxt.hash = make(map[string]*LSym)
+ ctxt.funchash = make(map[string]*LSym)
ctxt.statichash = make(map[string]*LSym)
ctxt.Arch = arch
ctxt.Pathname = objabi.WorkingDir()
@@ -74,6 +75,30 @@
return s
}
+// LookupABI looks up a symbol with the given ABI.
+// If it does not exist, it creates it.
+func (ctxt *Link) LookupABI(name string, abi ABI) *LSym {
+ var hash map[string]*LSym
+ switch abi {
+ case ABI0:
+ hash = ctxt.hash
+ case ABIInternal:
+ hash = ctxt.funchash
+ default:
+ panic("unknown ABI")
+ }
+
+ ctxt.hashmu.Lock()
+ s := hash[name]
+ if s == nil {
+ s = &LSym{Name: name}
+ s.SetABI(abi)
+ hash[name] = s
+ }
+ ctxt.hashmu.Unlock()
+ return s
+}
+
// Lookup looks up the symbol with name name.
// If it does not exist, it creates it.
func (ctxt *Link) Lookup(name string) *LSym {
diff --git a/src/cmd/internal/obj/wasm/wasmobj.go b/src/cmd/internal/obj/wasm/wasmobj.go
index 23283a1..fbea103 100644
--- a/src/cmd/internal/obj/wasm/wasmobj.go
+++ b/src/cmd/internal/obj/wasm/wasmobj.go
@@ -125,11 +125,13 @@
morestack = ctxt.Lookup("runtime.morestack")
morestackNoCtxt = ctxt.Lookup("runtime.morestack_noctxt")
gcWriteBarrier = ctxt.Lookup("runtime.gcWriteBarrier")
- sigpanic = ctxt.Lookup("runtime.sigpanic")
- sigpanic.SetABI(obj.ABIInternal)
- deferreturn = ctxt.Lookup("runtime.deferreturn")
- deferreturn.SetABI(obj.ABIInternal)
- jmpdefer = ctxt.Lookup(`"".jmpdefer`)
+ sigpanic = ctxt.LookupABI("runtime.sigpanic", obj.ABIInternal)
+ deferreturn = ctxt.LookupABI("runtime.deferreturn", obj.ABIInternal)
+ // jmpdefer is defined in assembly as ABI0, but what we're
+ // looking for is the *call* to jmpdefer from the Go function
+ // deferreturn, so we're looking for the ABIInternal version
+ // of jmpdefer that's called by Go.
+ jmpdefer = ctxt.LookupABI(`"".jmpdefer`, obj.ABIInternal)
}
func preprocess(ctxt *obj.Link, s *obj.LSym, newprog obj.ProgAlloc) {
diff --git a/src/cmd/internal/obj/x86/asm6.go b/src/cmd/internal/obj/x86/asm6.go
index 520f4be..c3da29c 100644
--- a/src/cmd/internal/obj/x86/asm6.go
+++ b/src/cmd/internal/obj/x86/asm6.go
@@ -2064,8 +2064,7 @@
case objabi.Hplan9:
plan9privates = ctxt.Lookup("_privates")
case objabi.Hnacl:
- deferreturn = ctxt.Lookup("runtime.deferreturn")
- deferreturn.SetABI(obj.ABIInternal)
+ deferreturn = ctxt.LookupABI("runtime.deferreturn", obj.ABIInternal)
}
for i := range avxOptab {
diff --git a/test/fixedbugs/issue29610.dir/a.go b/test/fixedbugs/issue29610.dir/a.go
new file mode 100644
index 0000000..ccbe451
--- /dev/null
+++ b/test/fixedbugs/issue29610.dir/a.go
@@ -0,0 +1,15 @@
+// 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.
+
+package a
+
+type I interface {
+ M(init bool)
+}
+
+var V I
+
+func init() {
+ V = nil
+}
diff --git a/test/fixedbugs/issue29610.dir/b.go b/test/fixedbugs/issue29610.dir/b.go
new file mode 100644
index 0000000..c2016de
--- /dev/null
+++ b/test/fixedbugs/issue29610.dir/b.go
@@ -0,0 +1,17 @@
+// 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.
+
+package b
+
+import "./a"
+
+type S struct {
+ a.I
+}
+
+var V a.I
+
+func init() {
+ V = S{}
+}
diff --git a/test/fixedbugs/issue29610.dir/main.go b/test/fixedbugs/issue29610.dir/main.go
new file mode 100644
index 0000000..29437bf
--- /dev/null
+++ b/test/fixedbugs/issue29610.dir/main.go
@@ -0,0 +1,11 @@
+// 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.
+
+package main
+
+import "./b"
+
+var v b.S
+
+func main() {}
diff --git a/test/fixedbugs/issue29610.go b/test/fixedbugs/issue29610.go
new file mode 100644
index 0000000..8d49ba6
--- /dev/null
+++ b/test/fixedbugs/issue29610.go
@@ -0,0 +1,13 @@
+// rundir
+
+// 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.
+
+// Issue 29610: Symbol import and initialization order caused function
+// symbols to be recorded as non-function symbols.
+
+// This uses rundir not because we actually want to run the final
+// binary, but because we need to at least link it.
+
+package ignored
