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>

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.

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.

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())
 }

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")

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

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 {

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) {

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 {

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
+}

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{}
+}

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() {}

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