diff options
Diffstat (limited to 'tools')
| -rw-r--r-- | tools/checklinkname/BUILD | 16 | ||||
| -rw-r--r-- | tools/checklinkname/README.md | 54 | ||||
| -rw-r--r-- | tools/checklinkname/check_linkname.go | 229 | ||||
| -rw-r--r-- | tools/checklinkname/known.go | 110 | ||||
| -rw-r--r-- | tools/checklinkname/test/BUILD | 9 | ||||
| -rw-r--r-- | tools/checklinkname/test/test_unsafe.go | 34 | ||||
| -rw-r--r-- | tools/nogo/BUILD | 1 | ||||
| -rw-r--r-- | tools/nogo/analyzers.go | 2 | ||||
| -rw-r--r-- | tools/nogo/filter/main.go | 2 |
9 files changed, 456 insertions, 1 deletions
diff --git a/tools/checklinkname/BUILD b/tools/checklinkname/BUILD new file mode 100644 index 000000000..0f1b07e24 --- /dev/null +++ b/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", + ], +) diff --git a/tools/checklinkname/README.md b/tools/checklinkname/README.md new file mode 100644 index 000000000..06b3c302d --- /dev/null +++ b/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. diff --git a/tools/checklinkname/check_linkname.go b/tools/checklinkname/check_linkname.go new file mode 100644 index 000000000..5373dd762 --- /dev/null +++ b/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 +} diff --git a/tools/checklinkname/known.go b/tools/checklinkname/known.go new file mode 100644 index 000000000..54e5155fc --- /dev/null +++ b/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()", + }, + }, +} diff --git a/tools/checklinkname/test/BUILD b/tools/checklinkname/test/BUILD new file mode 100644 index 000000000..b29bd84f2 --- /dev/null +++ b/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"], +) diff --git a/tools/checklinkname/test/test_unsafe.go b/tools/checklinkname/test/test_unsafe.go new file mode 100644 index 000000000..a7504591c --- /dev/null +++ b/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 diff --git a/tools/nogo/BUILD b/tools/nogo/BUILD index 27fe48680..d72821377 100644 --- a/tools/nogo/BUILD +++ b/tools/nogo/BUILD @@ -35,6 +35,7 @@ go_library( visibility = ["//:sandbox"], deps = [ "//tools/checkescape", + "//tools/checklinkname", "//tools/checklocks", "//tools/checkunsafe", "//tools/nogo/objdump", diff --git a/tools/nogo/analyzers.go b/tools/nogo/analyzers.go index 2b3c03fec..6705fc905 100644 --- a/tools/nogo/analyzers.go +++ b/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, } diff --git a/tools/nogo/filter/main.go b/tools/nogo/filter/main.go index d50336b9b..4a925d03c 100644 --- a/tools/nogo/filter/main.go +++ b/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 ( |