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 f44d19b4390ef69cb0a612633c9cacd1709066fd..98ff2a3d27a1c3c81f710f8bec0c80f03459b098 100644 (file)
--- a/src/cmd/compile/internal/gc/main.go
+++ b/src/cmd/compile/internal/gc/main.go
@@ -563,11 +563,6 @@ func Main(archInit func(*Arch)) {
                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 db26f135f59b733c8a5a91f33b73dcfc7be7f453..e20137669aaac831cc8b9e8eba1781e985dbb68e 100644 (file)
--- a/src/cmd/compile/internal/gc/ssa.go
+++ b/src/cmd/compile/internal/gc/ssa.go
@@ -106,7 +106,7 @@ func initssaconfig() {
        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 86f5022b5c84fc3e6d0777747123fefb4b3e1417..13761c7615b401764b79f4ea0ff060b8f632961e 100644 (file)
--- a/src/cmd/compile/internal/types/sym.go
+++ b/src/cmd/compile/internal/types/sym.go
@@ -79,9 +79,7 @@ func (sym *Sym) Linksym() *obj.LSym {
        }
        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 316937bde0892f08507465cc18196b99e7f56ddc..b1fb1d394411a20d2edb981bdd3bf3186d67d0d0 100644 (file)
--- a/src/cmd/internal/obj/arm/asm5.go
+++ b/src/cmd/internal/obj/arm/asm5.go
@@ -1529,8 +1529,7 @@ func buildop(ctxt *obj.Link) {
                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 7df8e2e516857a444b641fa340ceae4cfbbe2687..f506f60d065d4fe019610646ffe0f22213718ba0 100644 (file)
--- a/src/cmd/internal/obj/link.go
+++ b/src/cmd/internal/obj/link.go
@@ -626,8 +626,9 @@ type Link struct {
        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 3fc17fa850711be0218cc24f845f22337f0819d8..15a501c3aa514ae12035643c9cdb97a8c7466fff 100644 (file)
--- a/src/cmd/internal/obj/sym.go
+++ b/src/cmd/internal/obj/sym.go
@@ -41,6 +41,7 @@ import (
 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 @@ func (ctxt *Link) LookupStatic(name string) *LSym {
        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 23283a12cf657f3d17518c3bba17d311ccc3b154..fbea103dcb08030f19bb1eb74292761c66544e34 100644 (file)
--- a/src/cmd/internal/obj/wasm/wasmobj.go
+++ b/src/cmd/internal/obj/wasm/wasmobj.go
@@ -125,11 +125,13 @@ func instinit(ctxt *obj.Link) {
        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 520f4be8f5653a9eaabfb61df33812c4861e90c6..c3da29ce2cbd4e78d766bafca4861ea503cf0466 100644 (file)
--- a/src/cmd/internal/obj/x86/asm6.go
+++ b/src/cmd/internal/obj/x86/asm6.go
@@ -2064,8 +2064,7 @@ func instinit(ctxt *obj.Link) {
        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 (file)
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 (file)
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 (file)
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 (file)
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