checklinkname: rudimentary type-checking of linkname directives

This CL introduces a 'checklinkname' analyzer, which provides rudimentary
type-checking that verifies that function signatures on the local and remote
sides of //go:linkname directives match expected values.

If the Go standard library changes the definitions of any of these function,
checklinkname will flag the change as a finding, providing an error informing
the gVisor team to adapt to the upstream changes. This allows us to eliminate
the majority of gVisor's forward-looking negative build tags, as we can catch
mismatches in testing [1].

The remaining forward-looking negative build tags are covering shared struct
definitions, which I hope to add to checklinkname in a future CL.

[1] Of course, semantics/requirements can change without the signature
changing, so we still must be careful, but this covers the common case.

PiperOrigin-RevId: 387873847

nogo.yaml

@@ -150,6 +150,17 @@ analyzers:
     external: # Enabled.
   checkescape:
     external: # Enabled.
+  checklinkname:
+    external: # Enabled.
+      suppress:
+        # We don't care to check every single linkname in the Go standard
+        # library. Suppress findings about stdlib linkname targets we haven't
+        # described in checklinkname.
+        #
+        # Note that we _do_ want to check the signature of the known linkname
+        # targets in the standard library, so we still need to run
+        # checklinkname on stdlib generally.
+        - "linkname to unknown symbol"
   SA4016:
     internal:
       exclude:

pkg/gohacks/gohacks_unsafe.go

@@ -15,7 +15,13 @@
 //go:build go1.13 && !go1.18
 // +build go1.13,!go1.18
 
-// Check type signatures when updating Go version.
+// //go:linkname directives type-checked by checklinkname. Any other
+// non-linkname assumptions outside the Go 1 compatibility guarantee should
+// have an accompanied vet check or version guard build tag.
+
+// Check type signatures and Noescape when updating Go version.
+//
+// TODO(b/165820485): add these checks to checklinkname.
 
 // Package gohacks contains utilities for subverting the Go compiler.
 package gohacks

pkg/procid/procid_amd64.s

@@ -15,6 +15,10 @@
 //go:build amd64 && go1.8 && !go1.18 && go1.1
 // +build amd64,go1.8,!go1.18,go1.1
 
+// //go:linkname directives type-checked by checklinkname. Any other
+// non-linkname assumptions outside the Go 1 compatibility guarantee should
+// have an accompanied vet check or version guard build tag.
+
 #include "textflag.h"
 
 TEXT ·Current(SB),NOSPLIT,$0-8

pkg/procid/procid_arm64.s

@@ -15,6 +15,10 @@
 //go:build arm64 && go1.8 && !go1.18 && go1.1
 // +build arm64,go1.8,!go1.18,go1.1
 
+// //go:linkname directives type-checked by checklinkname. Any other
+// non-linkname assumptions outside the Go 1 compatibility guarantee should
+// have an accompanied vet check or version guard build tag.
+
 #include "textflag.h"
 
 TEXT ·Current(SB),NOSPLIT,$0-8

pkg/sentry/platform/kvm/bluepill_unsafe.go

@@ -12,10 +12,12 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-//go:build go1.12 && !go1.18
-// +build go1.12,!go1.18
+//go:build go1.12
+// +build go1.12
 
-// Check go:linkname function signatures when updating Go version.
+// //go:linkname directives type-checked by checklinkname. Any other
+// non-linkname assumptions outside the Go 1 compatibility guarantee should
+// have an accompanied vet check or version guard build tag.
 
 package kvm
 
@@ -28,7 +30,7 @@ import (
 )
 
 //go:linkname throw runtime.throw
-func throw(string)
+func throw(s string)
 
 // vCPUPtr returns a CPU for the given address.
 //

pkg/sentry/platform/kvm/machine_unsafe.go

@@ -12,10 +12,12 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-//go:build go1.12 && !go1.18
-// +build go1.12,!go1.18
+//go:build go1.12
+// +build go1.12
 
-// Check go:linkname function signatures when updating Go version.
+// //go:linkname directives type-checked by checklinkname. Any other
+// non-linkname assumptions outside the Go 1 compatibility guarantee should
+// have an accompanied vet check or version guard build tag.
 
 package kvm
 

pkg/sentry/platform/ptrace/subprocess_unsafe.go

@@ -12,10 +12,12 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-//go:build go1.12 && !go1.18
-// +build go1.12,!go1.18
+//go:build go1.12
+// +build go1.12
 
-// Check go:linkname function signatures when updating Go version.
+// //go:linkname directives type-checked by checklinkname. Any other
+// non-linkname assumptions outside the Go 1 compatibility guarantee should
+// have an accompanied vet check or version guard build tag.
 
 package ptrace
 

pkg/sync/goyield_unsafe.go

@@ -3,10 +3,12 @@
 // Use of this source code is governed by a BSD-style
 // license that can be found in the LICENSE file.
 
-//go:build go1.14 && !go1.18
-// +build go1.14,!go1.18
+//go:build go1.14
+// +build go1.14
 
-// Check go:linkname function signatures when updating Go version.
+// //go:linkname directives type-checked by checklinkname. Any other
+// non-linkname assumptions outside the Go 1 compatibility guarantee should
+// have an accompanied vet check or version guard build tag.
 
 package sync
 

pkg/sync/runtime_unsafe.go

@@ -6,8 +6,13 @@
 //go:build go1.13 && !go1.18
 // +build go1.13,!go1.18
 
-// Check go:linkname function signatures, type definitions, and constants when
-// updating Go version.
+// //go:linkname directives type-checked by checklinkname. Any other
+// non-linkname assumptions outside the Go 1 compatibility guarantee should
+// have an accompanied vet check or version guard build tag.
+
+// Check type definitions and constants when updating Go version.
+//
+// TODO(b/165820485): add these checks to checklinkname.
 
 package sync
 
@@ -109,10 +114,10 @@ type maptype struct {
 // These functions are only used within the sync package.
 
 //go:linkname semacquire sync.runtime_Semacquire
-func semacquire(s *uint32)
+func semacquire(addr *uint32)
 
 //go:linkname semrelease sync.runtime_Semrelease
-func semrelease(s *uint32, handoff bool, skipframes int)
+func semrelease(addr *uint32, handoff bool, skipframes int)
 
 //go:linkname canSpin sync.runtime_canSpin
 func canSpin(i int) bool

pkg/tcpip/link/rawfile/blockingpoll_yield_unsafe.go

@@ -12,12 +12,13 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-//go:build ((linux && amd64) || (linux && arm64)) && go1.12 && !go1.18
+//go:build ((linux && amd64) || (linux && arm64)) && go1.12
 // +build linux,amd64 linux,arm64
 // +build go1.12
-// +build !go1.18
 
-// Check go:linkname function signatures when updating Go version.
+// //go:linkname directives type-checked by checklinkname. Any other
+// non-linkname assumptions outside the Go 1 compatibility guarantee should
+// have an accompanied vet check or version guard build tag.
 
 package rawfile
 

tools/checklinkname/BUILD

@@ -0,0 +1,16 @@
+load("//tools:defs.bzl", "go_library")
+
+package(licenses = ["notice"])
+
+go_library(
+    name = "checklinkname",
+    srcs = [
+        "check_linkname.go",
+        "known.go",
+    ],
+    nogo = False,
+    visibility = ["//tools/nogo:__subpackages__"],
+    deps = [
+        "@org_golang_x_tools//go/analysis:go_default_library",
+    ],
+)

tools/checklinkname/README.md

@@ -0,0 +1,54 @@
+# `checklinkname` Analyzer
+
+`checklinkname` is an analyzer to provide rudimentary type-checking for
+`//go:linkname` directives. Since `//go:linkname` only affects linker behavior,
+there is no built-in type safety and it is the programmer's responsibility to
+ensure the types on either side are compatible.
+
+`checklinkname` helps with this by checking that uses match expectations, as
+defined in this package.
+
+`known.go` contains the set of known linkname targets. For most functions, we
+expect identical types on both sides of the linkname. In a few cases, the types
+may be slightly different (e.g., local redefinition of internal type). It is
+still the responsibility of the programmer to ensure the signatures in
+`known.go` are compatible and safe.
+
+## Findings
+
+Here are the most common findings from this package, and how to resolve them.
+
+### `runtime.foo signature got "BAR" want "BAZ"; stdlib type changed?`
+
+The definition of `runtime.foo` in the standard library does not match the
+expected type in `known.go`. This means that the function signature in the
+standard library changed.
+
+Addressing this will require creating a new linkname directive in a new Go
+version build-tagged in any packages using this symbol. Be sure to also check to
+ensure use with the new version is safe, as function constraints may have
+changed in addition to the signature.
+
+<!-- TODO(b/165820485): This isn't yet explicitly supported. -->
+
+`known.go` will also need to be updated to accept the new signature for the new
+version of Go.
+
+### `Cannot find known symbol "runtime.foo"`
+
+The standard library has removed runtime.foo entirely. Handling is similar to
+above, except existing code must transition away from the symbol entirely (note
+that is may simply be renamed).
+
+### `linkname to unknown symbol "mypkg.foo"; add this symbol to checklinkname.knownLinknames type-check against the remote type`
+
+A package has added a new linkname directive for a symbol not listed in
+`known.go`. Address this by adding a new entry for the target symbol. The
+`local` field should be the expected type in your package, while `remote` should
+be expected type in the remote package (e.g., in the standard library). These
+are typically identical, in which case `remote` can be omitted.
+
+### `usage: //go:linkname localname [linkname]`
+
+Malformed `//go:linkname` directive. This should be accompanied by a build
+failure in the package.

tools/checklinkname/check_linkname.go

@@ -0,0 +1,229 @@
+// Copyright 2021 The gVisor Authors.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+// Package checklinkname ensures that linkname declarations match their source.
+package checklinkname
+
+import (
+	"fmt"
+	"go/ast"
+	"go/token"
+	"go/types"
+	"strings"
+
+	"golang.org/x/tools/go/analysis"
+)
+
+// Analyzer implements the checklinkname analyzer.
+var Analyzer = &analysis.Analyzer{
+	Name: "checklinkname",
+	Doc:  "verifies that linkname declarations match their source",
+	Run:  run,
+}
+
+// go:linkname can be rather confusing. https://pkg.go.dev/cmd/compile says:
+//
+// //go:linkname localname [importpath.name]
+//
+// This special directive does not apply to the Go code that follows it.
+// Instead, the //go:linkname directive instructs the compiler to use
+// “importpath.name” as the object file symbol name for the variable or
+// function declared as “localname” in the source code. If the
+// “importpath.name” argument is omitted, the directive uses the symbol's
+// default object file symbol name and only has the effect of making the symbol
+// accessible to other packages. Because this directive can subvert the type
+// system and package modularity, it is only enabled in files that have
+// imported "unsafe".
+//
+// In this package we use the term "local" to refer to the symbol name in the
+// same package as the //go:linkname directive, whose name will be changed by
+// the linker. We use the term "remote" to refer to the symbol name that we are
+// changing to.
+//
+// In the general case, the local symbol is a function declaration, and the
+// remote symbol is a real function in the standard library.
+
+// linknameSignatures describes a the type signatures of the symbols in a
+// //go:linkname directive.
+type linknameSignatures struct {
+	local  string
+	remote string // equivalent to local if "".
+}
+
+func (l *linknameSignatures) Remote() string {
+	if l.remote == "" {
+		return l.local
+	}
+	return l.remote
+}
+
+// linknameSymbols describes the symbol namess in a single //go:linkname
+// directive.
+type linknameSymbols struct {
+	pos    token.Pos
+	local  string
+	remote string
+}
+
+func findLinknames(pass *analysis.Pass, f *ast.File) []linknameSymbols {
+	var names []linknameSymbols
+
+	for _, cg := range f.Comments {
+		for _, c := range cg.List {
+			if len(c.Text) <= 2 || !strings.HasPrefix(c.Text[2:], "go:linkname ") {
+				continue
+			}
+
+			f := strings.Fields(c.Text)
+			if len(f) < 2 || len(f) > 3 {
+				// Malformed linkname. This is the same error the compiler emits.
+				pass.Reportf(c.Slash, "usage: //go:linkname localname [linkname]")
+			}
+
+			if len(f) == 2 {
+				// "If the “importpath.name” argument is
+				// omitted, the directive uses the symbol's
+				// default object file symbol name and only has
+				// the effect of making the symbol accessible
+				// to other packages."
+				// -https://golang.org/cmd/compile
+				//
+				// There is no type-checking to be done here.
+				continue
+			}
+
+			names = append(names, linknameSymbols{
+				pos:    c.Slash,
+				local:  f[1],
+				remote: f[2],
+			})
+		}
+	}
+
+	return names
+}
+
+func splitSymbol(pkg *types.Package, symbol string) (packagePath, name string) {
+	// Note that some runtime symbols can have multiple dots. e.g.,
+	// runtime..init_task.
+	s := strings.SplitN(symbol, ".", 2)
+
+	switch len(s) {
+	case 1:
+		// Package name omitted, use current package.
+		return pkg.Path(), symbol
+	case 2:
+		return s[0], s[1]
+	default:
+		panic("unreachable")
+	}
+}
+
+func findObject(pkg *types.Package, symbol string) (types.Object, error) {
+	packagePath, symbolName := splitSymbol(pkg, symbol)
+	return findPackageObject(pkg, packagePath, symbolName)
+}
+
+func findPackageObject(pkg *types.Package, packagePath, symbolName string) (types.Object, error) {
+	if pkg.Path() == packagePath {
+		o := pkg.Scope().Lookup(symbolName)
+		if o == nil {
+			return nil, fmt.Errorf("%q not found in %q (names: %+v)", symbolName, packagePath, pkg.Scope().Names())
+		}
+		return o, nil
+	}
+
+	for _, p := range pkg.Imports() {
+		if o, err := findPackageObject(p, packagePath, symbolName); err == nil {
+			return o, nil
+		}
+	}
+
+	return nil, fmt.Errorf("package %q not found", packagePath)
+}
+
+// checkOneLinkname verifies that the type of sym.local matches the type from
+// knownLinknames.
+func checkOneLinkname(pass *analysis.Pass, f *ast.File, sym linknameSymbols) {
+	remotePackage, remoteName := splitSymbol(pass.Pkg, sym.remote)
+
+	m, ok := knownLinknames[remotePackage]
+	if !ok {
+		pass.Reportf(sym.pos, "linkname to unknown symbol %q; add this symbol to checklinkname.knownLinknames type-check against the remote type", sym.remote)
+		return
+	}
+
+	linkname, ok := m[remoteName]
+	if !ok {
+		pass.Reportf(sym.pos, "linkname to unknown symbol %q; add this symbol to checklinkname.knownLinknames type-check against the remote type", sym.remote)
+		return
+	}
+
+	local, err := findObject(pass.Pkg, sym.local)
+	if err != nil {
+		pass.Reportf(sym.pos, "Unable to find symbol %q: %v", sym.local, err)
+		return
+	}
+
+	localSig, ok := local.Type().(*types.Signature)
+	if !ok {
+		pass.Reportf(local.Pos(), "%q object is not a signature: %+#v", sym.local, local)
+		return
+	}
+
+	if linkname.local != localSig.String() {
+		pass.Reportf(local.Pos(), "%q signature got %q want %q; mismatched types?", sym.local, localSig.String(), linkname.local)
+		return
+	}
+}
+
+// checkOneRemote verifies that the type of sym matches wantSig.
+func checkOneRemote(pass *analysis.Pass, sym, wantSig string) {
+	o := pass.Pkg.Scope().Lookup(sym)
+	if o == nil {
+		pass.Reportf(pass.Files[0].Package, "Cannot find known symbol %q", sym)
+		return
+	}
+
+	sig, ok := o.Type().(*types.Signature)
+	if !ok {
+		pass.Reportf(o.Pos(), "%q object is not a signature: %+#v", sym, o)
+		return
+	}
+
+	if sig.String() != wantSig {
+		pass.Reportf(o.Pos(), "%q signature got %q want %q; stdlib type changed?", sym, sig.String(), wantSig)
+		return
+	}
+}
+
+func run(pass *analysis.Pass) (interface{}, error) {
+	// First, check if any remote symbols are in this package.
+	p, ok := knownLinknames[pass.Pkg.Path()]
+	if ok {
+		for sym, l := range p {
+			checkOneRemote(pass, sym, l.Remote())
+		}
+	}
+
+	// Then check for local //go:linkname directives in this package.
+	for _, f := range pass.Files {
+		names := findLinknames(pass, f)
+		for _, n := range names {
+			checkOneLinkname(pass, f, n)
+		}
+	}
+
+	return nil, nil
+}

tools/checklinkname/known.go

@@ -0,0 +1,110 @@
+// Copyright 2021 The gVisor Authors.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package checklinkname
+
+// knownLinknames is the set of the symbols for which we can do a rudimentary
+// type-check on.
+//
+// When analyzing the remote package (e.g., runtime), we verify the symbol
+// signature matches 'remote'. When analyzing local packages with //go:linkname
+// directives, we verify the symbol signature matches 'local'.
+//
+// Usually these are identical, but may differ slightly if equivalent
+// replacement types are used in the local packages, such as a copy of a struct
+// or uintptr instead of a pointer type.
+//
+// NOTE: It is the responsibility of the developer to verify the safety of the
+// signatures used here! This analyzer only checks that types match this map;
+// it does not verify compatibility of the entries themselves.
+//
+// //go:linkname directives with no corresponding entry here will trigger a
+// finding.
+//
+// We preform only rudimentary string-based type-checking due to limitations in
+// the analysis framework. Ideally, from the local package we'd lookup the
+// remote symbol's types.Object and perform robust type-checking.
+// Unfortunately, remote symbols are typically loaded from the remote package's
+// gcexportdata. Since //go:linkname targets are usually not exported symbols,
+// they are no included in gcexportdata and we cannot load their types.Object.
+//
+// TODO(b/165820485): Add option to specific per-version signatures.
+var knownLinknames = map[string]map[string]linknameSignatures{
+	"runtime": map[string]linknameSignatures{
+		"entersyscall": linknameSignatures{
+			local: "func()",
+		},
+		"entersyscallblock": linknameSignatures{
+			local: "func()",
+		},
+		"exitsyscall": linknameSignatures{
+			local: "func()",
+		},
+		"fastrand": linknameSignatures{
+			local: "func() uint32",
+		},
+		"gopark": linknameSignatures{
+			// TODO(b/165820485): add verification of waitReason
+			// size and reason and traceEv values.
+			local:  "func(unlockf func(uintptr, unsafe.Pointer) bool, lock unsafe.Pointer, reason uint8, traceEv byte, traceskip int)",
+			remote: "func(unlockf func(*runtime.g, unsafe.Pointer) bool, lock unsafe.Pointer, reason runtime.waitReason, traceEv byte, traceskip int)",
+		},
+		"goready": linknameSignatures{
+			local:  "func(gp uintptr, traceskip int)",
+			remote: "func(gp *runtime.g, traceskip int)",
+		},
+		"goyield": linknameSignatures{
+			local: "func()",
+		},
+		"memmove": linknameSignatures{
+			local: "func(to unsafe.Pointer, from unsafe.Pointer, n uintptr)",
+		},
+		"throw": linknameSignatures{
+			local: "func(s string)",
+		},
+	},
+	"sync": map[string]linknameSignatures{
+		"runtime_canSpin": linknameSignatures{
+			local: "func(i int) bool",
+		},
+		"runtime_doSpin": linknameSignatures{
+			local: "func()",
+		},
+		"runtime_Semacquire": linknameSignatures{
+			// The only difference here is the parameter names. We
+			// can't just change our local use to match remote, as
+			// the stdlib runtime and sync packages also disagree
+			// on the name, and the analyzer checks that use as
+			// well.
+			local:  "func(addr *uint32)",
+			remote: "func(s *uint32)",
+		},
+		"runtime_Semrelease": linknameSignatures{
+			// See above.
+			local:  "func(addr *uint32, handoff bool, skipframes int)",
+			remote: "func(s *uint32, handoff bool, skipframes int)",
+		},
+	},
+	"syscall": map[string]linknameSignatures{
+		"runtime_BeforeFork": linknameSignatures{
+			local: "func()",
+		},
+		"runtime_AfterFork": linknameSignatures{
+			local: "func()",
+		},
+		"runtime_AfterForkInChild": linknameSignatures{
+			local: "func()",
+		},
+	},
+}

tools/checklinkname/test/BUILD

@@ -0,0 +1,9 @@
+load("//tools:defs.bzl", "go_library")
+
+package(licenses = ["notice"])
+
+go_library(
+    name = "test",
+    testonly = 1,
+    srcs = ["test_unsafe.go"],
+)

tools/checklinkname/test/test_unsafe.go

@@ -0,0 +1,34 @@
+// Copyright 2021 The gVisor Authors.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+// Package test provides linkname test targets.
+package test
+
+import (
+	_ "unsafe" // for go:linkname.
+)
+
+//go:linkname DetachedLinkname runtime.fastrand
+
+//go:linkname attachedLinkname runtime.entersyscall
+func attachedLinkname()
+
+// AttachedLinkname reexports attachedLinkname because go vet doesn't like an
+// exported go:linkname without a comment starting with "// AttachedLinkname".
+func AttachedLinkname() {
+	attachedLinkname()
+}
+
+// DetachedLinkname has a linkname elsewhere in the file.
+func DetachedLinkname() uint32

tools/nogo/BUILD

@@ -35,6 +35,7 @@ go_library(
     visibility = ["//:sandbox"],
     deps = [
         "//tools/checkescape",
+        "//tools/checklinkname",
         "//tools/checklocks",
         "//tools/checkunsafe",
         "//tools/nogo/objdump",

tools/nogo/analyzers.go

@@ -47,6 +47,7 @@ import (
 	"honnef.co/go/tools/stylecheck"
 
 	"gvisor.dev/gvisor/tools/checkescape"
+	"gvisor.dev/gvisor/tools/checklinkname"
 	"gvisor.dev/gvisor/tools/checklocks"
 	"gvisor.dev/gvisor/tools/checkunsafe"
 )
@@ -80,6 +81,7 @@ var AllAnalyzers = []*analysis.Analyzer{
 	unusedresult.Analyzer,
 	checkescape.Analyzer,
 	checkunsafe.Analyzer,
+	checklinkname.Analyzer,
 	checklocks.Analyzer,
 }
 

tools/nogo/filter/main.go

@@ -12,7 +12,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-// Binary check is the nogo entrypoint.
+// Binary filter is the filters and reports nogo findings.
 package main
 
 import (