cmd/internal/obj: tweak implicit {ArgsPointerMaps,ArgInfo} behavior

This CL changes cmd/internal/obj to also implicitly set
ArgsPointerMaps and ArgInfo for assembly functions that are explicitly
package qualified (e.g., "pkg·name", not just "·name"). This is a
prerequisite for changing cmd/asm to stop emitting `"".`-prefixed
symbol names.

Change-Id: I4e14bc24c87cf4d7114a7aed9beaf0c8d1f9c07f
Reviewed-on: https://go-review.googlesource.com/c/go/+/523335
Auto-Submit: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>

diff --git a/src/cmd/internal/obj/plist.go b/src/cmd/internal/obj/plist.go
index 9cdf0800f09fbe7f90fb6d33bcfd8013a4976088..1471c6267f34d080832a56f0976171eecb93d891 100644 (file)
--- a/src/cmd/internal/obj/plist.go
+++ b/src/cmd/internal/obj/plist.go
@@ -96,8 +96,9 @@ func Flushplist(ctxt *Link, plist *Plist, newprog ProgAlloc) {
 
        // Add reference to Go arguments for assembly functions without them.
        if ctxt.IsAsm {
+               pkgPrefix := objabi.PathToPrefix(ctxt.Pkgpath) + "."
                for _, s := range text {
-                       if !strings.HasPrefix(s.Name, "\"\".") {
+                       if !strings.HasPrefix(s.Name, `"".`) && !strings.HasPrefix(s.Name, pkgPrefix) {
                                continue
                        }
                        // The current args_stackmap generation in the compiler assumes
@@ -107,6 +108,16 @@ func Flushplist(ctxt *Link, plist *Plist, newprog ProgAlloc) {
                        if s.ABI() != ABI0 {
                                continue
                        }
+                       // runtime.addmoduledata is a host ABI function, so it doesn't
+                       // need FUNCDATA anyway. Moreover, cmd/link has special logic
+                       // for linking it in eccentric build modes, which breaks if it
+                       // has FUNCDATA references (e.g., cmd/cgo/internal/testplugin).
+                       //
+                       // TODO(cherryyz): Fix cmd/link's handling of plugins (see
+                       // discussion on CL 523355).
+                       if s.Name == "runtime.addmoduledata" {
+                               continue
+                       }
                        foundArgMap, foundArgInfo := false, false
                        for p := s.Func().Text; p != nil; p = p.Link {
                                if p.As == AFUNCDATA && p.From.Type == TYPE_CONST {

diff --git a/src/cmd/link/internal/ld/pcln.go b/src/cmd/link/internal/ld/pcln.go
index 03e3981ec8fbbac5ab49080446ccfb6726215db2..5734b92507e15b554efe10d8a848912f63bcea3b 100644 (file)
--- a/src/cmd/link/internal/ld/pcln.go
+++ b/src/cmd/link/internal/ld/pcln.go
@@ -727,6 +727,17 @@ func writeFuncs(ctxt *Link, sb *loader.SymbolBuilder, funcs []loader.Sym, inlSym
                for j := range funcdata {
                        dataoff := off + int64(4*j)
                        fdsym := funcdata[j]
+
+                       // cmd/internal/obj optimistically populates ArgsPointerMaps and
+                       // ArgInfo for assembly functions, hoping that the compiler will
+                       // emit appropriate symbols from their Go stub declarations. If
+                       // it didn't though, just ignore it.
+                       //
+                       // TODO(cherryyz): Fix arg map generation (see discussion on CL 523335).
+                       if fdsym != 0 && (j == abi.FUNCDATA_ArgsPointerMaps || j == abi.FUNCDATA_ArgInfo) && ldr.IsFromAssembly(s) && ldr.Data(fdsym) == nil {
+                               fdsym = 0
+                       }
+
                        if fdsym == 0 {
                                sb.SetUint32(ctxt.Arch, dataoff, ^uint32(0)) // ^0 is a sentinel for "no value"
                                continue

diff --git a/src/runtime/funcdata.h b/src/runtime/funcdata.h
index edc0316fb0bbc20dfdbf694e29aa38ae22a5a52f..4bbc58ea48823ee02a9f4ed96473205626cab9e0 100644 (file)
--- a/src/runtime/funcdata.h
+++ b/src/runtime/funcdata.h
@@ -35,9 +35,9 @@
 // defines the pointer map for the function's arguments.
 // GO_ARGS should be the first instruction in a function that uses it.
 // It can be omitted if there are no arguments at all.
-// GO_ARGS is inserted implicitly by the linker for any function whose
-// name starts with a middle-dot and that also has a Go prototype; it
-// is therefore usually not necessary to write explicitly.
+// GO_ARGS is inserted implicitly by the assembler for any function
+// whose package-qualified symbol name belongs to the current package;
+// it is therefore usually not necessary to write explicitly.
 #define GO_ARGS        FUNCDATA $FUNCDATA_ArgsPointerMaps, go_args_stackmap(SB)
 
 // GO_RESULTS_INITIALIZED indicates that the assembly function