summaryrefslogtreecommitdiffhomepage
diff options
context:
space:
mode:
authorAdin Scannell <ascannell@google.com>2020-08-26 14:40:30 -0700
committerAndrei Vagin <avagin@gmail.com>2020-09-09 17:53:10 -0700
commitf63cddc6b4826007ca2a755d30b2df65ea21c518 (patch)
treec2f65b4b73089c32f0374062cb974dd4672ad811
parentd872b342b2c2291420a9570edcf340040754bb44 (diff)
Support stdlib analyzers with nogo.
This immediately revealed an escape analysis violation (!), where the sync.Map was being used in a context that escapes were not allowed. This is a relatively minor fix and is included. PiperOrigin-RevId: 328611237
-rw-r--r--pkg/sentry/platform/kvm/bluepill_fault.go4
-rw-r--r--pkg/sentry/platform/kvm/kvm_const.go2
-rw-r--r--pkg/sentry/platform/kvm/machine.go40
-rw-r--r--tools/bazeldefs/defs.bzl11
-rw-r--r--tools/checkescape/BUILD2
-rw-r--r--tools/checkescape/checkescape.go19
-rw-r--r--tools/checkescape/test1/test1.go15
-rw-r--r--tools/checkescape/test2/test2.go6
-rw-r--r--tools/go_marshal/gomarshal/generator_interfaces.go2
-rw-r--r--tools/nogo/BUILD13
-rw-r--r--tools/nogo/build.go4
-rw-r--r--tools/nogo/config.go8
-rw-r--r--tools/nogo/data/data.go21
-rw-r--r--tools/nogo/defs.bzl185
-rw-r--r--tools/nogo/dump/BUILD (renamed from tools/nogo/data/BUILD)4
-rw-r--r--tools/nogo/dump/dump.go78
-rw-r--r--tools/nogo/nogo.go323
17 files changed, 560 insertions, 177 deletions
diff --git a/pkg/sentry/platform/kvm/bluepill_fault.go b/pkg/sentry/platform/kvm/bluepill_fault.go
index e34f46aeb..a182e4f22 100644
--- a/pkg/sentry/platform/kvm/bluepill_fault.go
+++ b/pkg/sentry/platform/kvm/bluepill_fault.go
@@ -98,6 +98,10 @@ func handleBluepillFault(m *machine, physical uintptr, phyRegions []physicalRegi
}
errno := m.setMemoryRegion(int(slot), physicalStart, length, virtualStart, flags)
if errno == 0 {
+ // Store the physical address in the slot. This is used to
+ // avoid calls to handleBluepillFault in the future (see
+ // machine.mapPhysical).
+ atomic.StoreUintptr(&m.usedSlots[slot], physical)
// Successfully added region; we can increment nextSlot and
// allow another set to proceed here.
atomic.StoreUint32(&m.nextSlot, slot+1)
diff --git a/pkg/sentry/platform/kvm/kvm_const.go b/pkg/sentry/platform/kvm/kvm_const.go
index 3bf918446..5c4b18899 100644
--- a/pkg/sentry/platform/kvm/kvm_const.go
+++ b/pkg/sentry/platform/kvm/kvm_const.go
@@ -56,6 +56,7 @@ const (
// KVM capability options.
const (
+ _KVM_CAP_MAX_MEMSLOTS = 0x0a
_KVM_CAP_MAX_VCPUS = 0x42
_KVM_CAP_ARM_VM_IPA_SIZE = 0xa5
_KVM_CAP_VCPU_EVENTS = 0x29
@@ -64,6 +65,7 @@ const (
// KVM limits.
const (
+ _KVM_NR_MEMSLOTS = 0x100
_KVM_NR_VCPUS = 0xff
_KVM_NR_INTERRUPTS = 0x100
_KVM_NR_CPUID_ENTRIES = 0x100
diff --git a/pkg/sentry/platform/kvm/machine.go b/pkg/sentry/platform/kvm/machine.go
index 6c54712d1..372a4cbd7 100644
--- a/pkg/sentry/platform/kvm/machine.go
+++ b/pkg/sentry/platform/kvm/machine.go
@@ -43,9 +43,6 @@ type machine struct {
// kernel is the set of global structures.
kernel ring0.Kernel
- // mappingCache is used for mapPhysical.
- mappingCache sync.Map
-
// mu protects vCPUs.
mu sync.RWMutex
@@ -63,6 +60,12 @@ type machine struct {
// maxVCPUs is the maximum number of vCPUs supported by the machine.
maxVCPUs int
+ // maxSlots is the maximum number of memory slots supported by the machine.
+ maxSlots int
+
+ // usedSlots is the set of used physical addresses (sorted).
+ usedSlots []uintptr
+
// nextID is the next vCPU ID.
nextID uint32
}
@@ -184,6 +187,7 @@ func newMachine(vm int) (*machine, error) {
PageTables: pagetables.New(newAllocator()),
})
+ // Pull the maximum vCPUs.
maxVCPUs, _, errno := syscall.RawSyscall(syscall.SYS_IOCTL, uintptr(m.fd), _KVM_CHECK_EXTENSION, _KVM_CAP_MAX_VCPUS)
if errno != 0 {
m.maxVCPUs = _KVM_NR_VCPUS
@@ -191,11 +195,19 @@ func newMachine(vm int) (*machine, error) {
m.maxVCPUs = int(maxVCPUs)
}
log.Debugf("The maximum number of vCPUs is %d.", m.maxVCPUs)
-
- // Create the vCPUs map/slices.
m.vCPUsByTID = make(map[uint64]*vCPU)
m.vCPUsByID = make([]*vCPU, m.maxVCPUs)
+ // Pull the maximum slots.
+ maxSlots, _, errno := syscall.RawSyscall(syscall.SYS_IOCTL, uintptr(m.fd), _KVM_CHECK_EXTENSION, _KVM_CAP_MAX_MEMSLOTS)
+ if errno != 0 {
+ m.maxSlots = _KVM_NR_MEMSLOTS
+ } else {
+ m.maxSlots = int(maxSlots)
+ }
+ log.Debugf("The maximum number of slots is %d.", m.maxSlots)
+ m.usedSlots = make([]uintptr, m.maxSlots)
+
// Apply the physical mappings. Note that these mappings may point to
// guest physical addresses that are not actually available. These
// physical pages are mapped on demand, see kernel_unsafe.go.
@@ -272,6 +284,20 @@ func newMachine(vm int) (*machine, error) {
return m, nil
}
+// hasSlot returns true iff the given address is mapped.
+//
+// This must be done via a linear scan.
+//
+//go:nosplit
+func (m *machine) hasSlot(physical uintptr) bool {
+ for i := 0; i < len(m.usedSlots); i++ {
+ if p := atomic.LoadUintptr(&m.usedSlots[i]); p == physical {
+ return true
+ }
+ }
+ return false
+}
+
// mapPhysical checks for the mapping of a physical range, and installs one if
// not available. This attempts to be efficient for calls in the hot path.
//
@@ -286,8 +312,8 @@ func (m *machine) mapPhysical(physical, length uintptr, phyRegions []physicalReg
panic("mapPhysical on unknown physical address")
}
- if _, ok := m.mappingCache.LoadOrStore(physicalStart, true); !ok {
- // Not present in the cache; requires setting the slot.
+ // Is this already mapped? Check the usedSlots.
+ if !m.hasSlot(physicalStart) {
if _, ok := handleBluepillFault(m, physical, phyRegions, flags); !ok {
panic("handleBluepillFault failed")
}
diff --git a/tools/bazeldefs/defs.bzl b/tools/bazeldefs/defs.bzl
index 4bbcda054..dad5fc3b2 100644
--- a/tools/bazeldefs/defs.bzl
+++ b/tools/bazeldefs/defs.bzl
@@ -147,7 +147,7 @@ def go_rule(rule, implementation, **kwargs):
Returns:
The result of invoking the rule.
"""
- attrs = kwargs.pop("attrs", [])
+ attrs = kwargs.pop("attrs", dict())
attrs["_go_context_data"] = attr.label(default = "@io_bazel_rules_go//:go_context_data")
attrs["_stdlib"] = attr.label(default = "@io_bazel_rules_go//:stdlib")
toolchains = kwargs.get("toolchains", []) + ["@io_bazel_rules_go//go:toolchain"]
@@ -158,12 +158,17 @@ def go_test_library(target):
return target.attr.embed[0]
return None
-def go_context(ctx):
+def go_context(ctx, std = False):
+ # We don't change anything for the standard library analysis. All Go files
+ # are available in all instances. Note that this includes the standard
+ # library sources, which are analyzed by nogo.
go_ctx = _go_context(ctx)
return struct(
go = go_ctx.go,
env = go_ctx.env,
- runfiles = depset([go_ctx.go] + go_ctx.sdk.tools + go_ctx.stdlib.libs),
+ nogo_args = [],
+ stdlib_srcs = go_ctx.sdk.srcs,
+ runfiles = depset([go_ctx.go] + go_ctx.sdk.srcs + go_ctx.sdk.tools + go_ctx.stdlib.libs),
goos = go_ctx.sdk.goos,
goarch = go_ctx.sdk.goarch,
tags = go_ctx.tags,
diff --git a/tools/checkescape/BUILD b/tools/checkescape/BUILD
index b8c3ddf44..6273aa779 100644
--- a/tools/checkescape/BUILD
+++ b/tools/checkescape/BUILD
@@ -8,7 +8,7 @@ go_library(
nogo = False,
visibility = ["//tools/nogo:__subpackages__"],
deps = [
- "//tools/nogo/data",
+ "//tools/nogo/dump",
"@org_golang_x_tools//go/analysis:go_tool_library",
"@org_golang_x_tools//go/analysis/passes/buildssa:go_tool_library",
"@org_golang_x_tools//go/ssa:go_tool_library",
diff --git a/tools/checkescape/checkescape.go b/tools/checkescape/checkescape.go
index f8def4823..aab3c36a1 100644
--- a/tools/checkescape/checkescape.go
+++ b/tools/checkescape/checkescape.go
@@ -66,7 +66,6 @@ import (
"go/token"
"go/types"
"io"
- "os"
"path/filepath"
"strconv"
"strings"
@@ -74,7 +73,7 @@ import (
"golang.org/x/tools/go/analysis"
"golang.org/x/tools/go/analysis/passes/buildssa"
"golang.org/x/tools/go/ssa"
- "gvisor.dev/gvisor/tools/nogo/data"
+ "gvisor.dev/gvisor/tools/nogo/dump"
)
const (
@@ -256,15 +255,14 @@ func (ec *EscapeCount) Record(reason EscapeReason) bool {
// used only to remove false positives for escape analysis. The call will be
// elided if escape analysis is able to put the object on the heap exclusively.
func loadObjdump() (map[LinePosition]string, error) {
- f, err := os.Open(data.Objdump)
+ cmd, out, err := dump.Command()
if err != nil {
return nil, err
}
- defer f.Close()
// Build the map.
m := make(map[LinePosition]string)
- r := bufio.NewReader(f)
+ r := bufio.NewReader(out)
var (
lastField string
lastPos LinePosition
@@ -329,6 +327,11 @@ func loadObjdump() (map[LinePosition]string, error) {
}
}
+ // Wait for the dump to finish.
+ if err := cmd.Wait(); err != nil {
+ return nil, err
+ }
+
return m, nil
}
@@ -413,12 +416,6 @@ func run(pass *analysis.Pass) (interface{}, error) {
return escapes(unknownPackage, "no package", inst, ec)
}
- // Atomic functions are instrinics. We can
- // assume that they don't escape.
- if x.Pkg.Pkg.Name() == "atomic" {
- return nil
- }
-
// Is this a local function? If yes, call the
// function to load the local function. The
// local escapes are the escapes found in the
diff --git a/tools/checkescape/test1/test1.go b/tools/checkescape/test1/test1.go
index 68d3f72cc..a1d36459f 100644
--- a/tools/checkescape/test1/test1.go
+++ b/tools/checkescape/test1/test1.go
@@ -17,7 +17,6 @@ package test1
import (
"fmt"
- "reflect"
)
// Interface is a generic interface.
@@ -163,20 +162,6 @@ func dynamicRec(f func()) {
Dynamic(f)
}
-// +mustescape:local,unknown
-//go:noinline
-//go:nosplit
-func Unknown() {
- _ = reflect.TypeOf((*Type)(nil)) // Does not actually escape.
-}
-
-// +mustescape:unknown
-//go:noinline
-//go:nosplit
-func unknownRec() {
- Unknown()
-}
-
//go:noinline
//go:nosplit
func internalFunc() {
diff --git a/tools/checkescape/test2/test2.go b/tools/checkescape/test2/test2.go
index 7fce3e3be..2d5865f47 100644
--- a/tools/checkescape/test2/test2.go
+++ b/tools/checkescape/test2/test2.go
@@ -81,12 +81,6 @@ func dynamicCrossPkg(f func()) {
test1.Dynamic(f)
}
-// +mustescape:unknown
-//go:noinline
-func unknownCrossPkg() {
- test1.Unknown()
-}
-
// +mustescape:stack
//go:noinline
func splitCrosssPkt() {
diff --git a/tools/go_marshal/gomarshal/generator_interfaces.go b/tools/go_marshal/gomarshal/generator_interfaces.go
index e3c3dac63..cf76b5241 100644
--- a/tools/go_marshal/gomarshal/generator_interfaces.go
+++ b/tools/go_marshal/gomarshal/generator_interfaces.go
@@ -224,7 +224,7 @@ func (g *interfaceGenerator) emitNoEscapeSliceDataPointer(srcPtr, dstVar string)
func (g *interfaceGenerator) emitKeepAlive(ptrVar string) {
g.emit("// Since we bypassed the compiler's escape analysis, indicate that %s\n", ptrVar)
g.emit("// must live until the use above.\n")
- g.emit("runtime.KeepAlive(%s)\n", ptrVar)
+ g.emit("runtime.KeepAlive(%s) // escapes: replaced by intrinsic.\n", ptrVar)
}
func (g *interfaceGenerator) expandBinaryExpr(b *strings.Builder, e *ast.BinaryExpr) {
diff --git a/tools/nogo/BUILD b/tools/nogo/BUILD
index e1bfb9a2c..fb35c5ffd 100644
--- a/tools/nogo/BUILD
+++ b/tools/nogo/BUILD
@@ -1,7 +1,18 @@
load("//tools:defs.bzl", "bzl_library", "go_library")
+load("//tools/nogo:defs.bzl", "nogo_dump_tool", "nogo_stdlib")
package(licenses = ["notice"])
+nogo_dump_tool(
+ name = "dump_tool",
+ visibility = ["//visibility:public"],
+)
+
+nogo_stdlib(
+ name = "stdlib",
+ visibility = ["//visibility:public"],
+)
+
go_library(
name = "nogo",
srcs = [
@@ -16,7 +27,7 @@ go_library(
deps = [
"//tools/checkescape",
"//tools/checkunsafe",
- "//tools/nogo/data",
+ "//tools/nogo/dump",
"@org_golang_x_tools//go/analysis:go_tool_library",
"@org_golang_x_tools//go/analysis/internal/facts:go_tool_library",
"@org_golang_x_tools//go/analysis/passes/asmdecl:go_tool_library",
diff --git a/tools/nogo/build.go b/tools/nogo/build.go
index 433d13738..37947b5c3 100644
--- a/tools/nogo/build.go
+++ b/tools/nogo/build.go
@@ -31,10 +31,10 @@ var (
)
// findStdPkg needs to find the bundled standard library packages.
-func (i *importer) findStdPkg(path string) (io.ReadCloser, error) {
+func findStdPkg(GOOS, GOARCH, path string) (io.ReadCloser, error) {
if path == "C" {
// Cgo builds cannot be analyzed. Skip.
return nil, ErrSkip
}
- return os.Open(fmt.Sprintf("external/go_sdk/pkg/%s_%s/%s.a", i.GOOS, i.GOARCH, path))
+ return os.Open(fmt.Sprintf("external/go_sdk/pkg/%s_%s/%s.a", GOOS, GOARCH, path))
}
diff --git a/tools/nogo/config.go b/tools/nogo/config.go
index 6958fca69..451cd4a4c 100644
--- a/tools/nogo/config.go
+++ b/tools/nogo/config.go
@@ -84,6 +84,14 @@ var analyzerConfig = map[*analysis.Analyzer]matcher{
externalExcluded(
".*protobuf/.*.go", // Bad conversions.
".*flate/huffman_bit_writer.go", // Bad conversion.
+
+ // Runtime internal violations.
+ ".*reflect/value.go",
+ ".*encoding/xml/xml.go",
+ ".*runtime/pprof/internal/profile/proto.go",
+ ".*fmt/scan.go",
+ ".*go/types/conversions.go",
+ ".*golang.org/x/net/dns/dnsmessage/message.go",
),
),
shadow.Analyzer: disableMatches(), // Disabled for now.
diff --git a/tools/nogo/data/data.go b/tools/nogo/data/data.go
deleted file mode 100644
index eb84d0d27..000000000
--- a/tools/nogo/data/data.go
+++ /dev/null
@@ -1,21 +0,0 @@
-// Copyright 2019 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 data contains shared data for nogo analysis.
-//
-// This is used to break a dependency cycle.
-package data
-
-// Objdump is the dumped binary under analysis.
-var Objdump string
diff --git a/tools/nogo/defs.bzl b/tools/nogo/defs.bzl
index 5377620b0..963084d53 100644
--- a/tools/nogo/defs.bzl
+++ b/tools/nogo/defs.bzl
@@ -2,6 +2,103 @@
load("//tools/bazeldefs:defs.bzl", "go_context", "go_importpath", "go_rule", "go_test_library")
+def _nogo_dump_tool_impl(ctx):
+ # Extract the Go context.
+ go_ctx = go_context(ctx)
+
+ # Construct the magic dump command.
+ #
+ # Note that in some cases, the input is being fed into the tool via stdin.
+ # Unfortunately, the Go objdump tool expects to see a seekable file [1], so
+ # we need the tool to handle this case by creating a temporary file.
+ #
+ # [1] https://github.com/golang/go/issues/41051
+ env_prefix = " ".join(["%s=%s" % (key, value) for (key, value) in go_ctx.env.items()])
+ dumper = ctx.actions.declare_file(ctx.label.name)
+ ctx.actions.write(dumper, "\n".join([
+ "#!/bin/bash",
+ "set -euo pipefail",
+ "if [[ $# -eq 0 ]]; then",
+ " T=$(mktemp -u -t libXXXXXX.a)",
+ " cat /dev/stdin > ${T}",
+ "else",
+ " T=$1;",
+ "fi",
+ "%s %s tool objdump ${T}" % (
+ env_prefix,
+ go_ctx.go.path,
+ ),
+ "if [[ $# -eq 0 ]]; then",
+ " rm -rf ${T}",
+ "fi",
+ "",
+ ]), is_executable = True)
+
+ # Include the full runfiles.
+ return [DefaultInfo(
+ runfiles = ctx.runfiles(files = go_ctx.runfiles.to_list()),
+ executable = dumper,
+ )]
+
+nogo_dump_tool = go_rule(
+ rule,
+ implementation = _nogo_dump_tool_impl,
+)
+
+# NogoStdlibInfo is the set of standard library facts.
+NogoStdlibInfo = provider(
+ "information for nogo analysis (standard library facts)",
+ fields = {
+ "facts": "serialized standard library facts",
+ },
+)
+
+def _nogo_stdlib_impl(ctx):
+ # Extract the Go context.
+ go_ctx = go_context(ctx)
+
+ # Build the standard library facts.
+ facts = ctx.actions.declare_file(ctx.label.name + ".facts")
+ config = struct(
+ Srcs = [f.path for f in go_ctx.stdlib_srcs],
+ GOOS = go_ctx.goos,
+ GOARCH = go_ctx.goarch,
+ Tags = go_ctx.tags,
+ FactOutput = facts.path,
+ )
+ config_file = ctx.actions.declare_file(ctx.label.name + ".cfg")
+ ctx.actions.write(config_file, config.to_json())
+ ctx.actions.run(
+ inputs = [config_file] + go_ctx.stdlib_srcs,
+ outputs = [facts],
+ tools = depset(go_ctx.runfiles.to_list() + ctx.files._dump_tool),
+ executable = ctx.files._nogo[0],
+ mnemonic = "GoStandardLibraryAnalysis",
+ progress_message = "Analyzing Go Standard Library",
+ arguments = go_ctx.nogo_args + [
+ "-dump_tool=%s" % ctx.files._dump_tool[0].path,
+ "-stdlib=%s" % config_file.path,
+ ],
+ )
+
+ # Return the stdlib facts as output.
+ return [NogoStdlibInfo(
+ facts = facts,
+ )]
+
+nogo_stdlib = go_rule(
+ rule,
+ implementation = _nogo_stdlib_impl,
+ attrs = {
+ "_nogo": attr.label(
+ default = "//tools/nogo/check:check",
+ ),
+ "_dump_tool": attr.label(
+ default = "//tools/nogo:dump_tool",
+ ),
+ },
+)
+
# NogoInfo is the serialized set of package facts for a nogo analysis.
#
# Each go_library rule will generate a corresponding nogo rule, which will run
@@ -33,6 +130,9 @@ def _nogo_aspect_impl(target, ctx):
else:
return [NogoInfo()]
+ # Extract the Go context.
+ go_ctx = go_context(ctx)
+
# If we're using the "library" attribute, then we need to aggregate the
# original library sources and dependencies into this target to perform
# proper type analysis.
@@ -45,10 +145,6 @@ def _nogo_aspect_impl(target, ctx):
if hasattr(info, "deps"):
deps = deps + info.deps
- # Construct the Go environment from the go_ctx.env dictionary.
- go_ctx = go_context(ctx)
- env_prefix = " ".join(["%s=%s" % (key, value) for (key, value) in go_ctx.env.items()])
-
# Start with all target files and srcs as input.
inputs = target.files.to_list() + srcs
@@ -64,26 +160,7 @@ def _nogo_aspect_impl(target, ctx):
else:
# Use the raw binary for go_binary and go_test targets.
target_objfile = binaries[0]
- disasm_file = ctx.actions.declare_file(target.label.name + ".out")
- dumper = ctx.actions.declare_file("%s-dumper" % ctx.label.name)
- ctx.actions.write(dumper, "\n".join([
- "#!/bin/bash",
- "%s %s tool objdump %s > %s\n" % (
- env_prefix,
- go_ctx.go.path,
- target_objfile.path,
- disasm_file.path,
- ),
- ]), is_executable = True)
- ctx.actions.run(
- inputs = [target_objfile],
- outputs = [disasm_file],
- tools = go_ctx.runfiles,
- mnemonic = "GoObjdump",
- progress_message = "Objdump %s" % target.label,
- executable = dumper,
- )
- inputs.append(disasm_file)
+ inputs.append(target_objfile)
# Extract the importpath for this package.
if ctx.rule.kind == "go_test":
@@ -96,25 +173,9 @@ def _nogo_aspect_impl(target, ctx):
else:
importpath = go_importpath(target)
- # The nogo tool requires a configfile serialized in JSON format to do its
- # work. This must line up with the nogo.Config fields.
- facts = ctx.actions.declare_file(target.label.name + ".facts")
- config = struct(
- ImportPath = importpath,
- GoFiles = [src.path for src in srcs if src.path.endswith(".go")],
- NonGoFiles = [src.path for src in srcs if not src.path.endswith(".go")],
- # Google's internal build system needs a bit more help to find std.
- StdZip = go_ctx.std_zip.short_path if hasattr(go_ctx, "std_zip") else "",
- GOOS = go_ctx.goos,
- GOARCH = go_ctx.goarch,
- Tags = go_ctx.tags,
- FactMap = {}, # Constructed below.
- ImportMap = {}, # Constructed below.
- FactOutput = facts.path,
- Objdump = disasm_file.path,
- )
-
# Collect all info from shadow dependencies.
+ fact_map = dict()
+ import_map = dict()
for dep in deps:
# There will be no file attribute set for all transitive dependencies
# that are not go_library or go_binary rules, such as a proto rules.
@@ -129,27 +190,46 @@ def _nogo_aspect_impl(target, ctx):
x_files = [f.path for f in info.binaries if f.path.endswith(".x")]
if not len(x_files):
x_files = [f.path for f in info.binaries if f.path.endswith(".a")]
- config.ImportMap[info.importpath] = x_files[0]
- config.FactMap[info.importpath] = info.facts.path
+ import_map[info.importpath] = x_files[0]
+ fact_map[info.importpath] = info.facts.path
# Ensure the above are available as inputs.
inputs.append(info.facts)
inputs += info.binaries
- # Write the configuration and run the tool.
+ # Add the standard library facts.
+ stdlib_facts = ctx.attr._nogo_stdlib[NogoStdlibInfo].facts
+ inputs.append(stdlib_facts)
+
+ # The nogo tool operates on a configuration serialized in JSON format.
+ facts = ctx.actions.declare_file(target.label.name + ".facts")
+ config = struct(
+ ImportPath = importpath,
+ GoFiles = [src.path for src in srcs if src.path.endswith(".go")],
+ NonGoFiles = [src.path for src in srcs if not src.path.endswith(".go")],
+ GOOS = go_ctx.goos,
+ GOARCH = go_ctx.goarch,
+ Tags = go_ctx.tags,
+ FactMap = fact_map,
+ ImportMap = import_map,
+ StdlibFacts = stdlib_facts.path,
+ FactOutput = facts.path,
+ )
config_file = ctx.actions.declare_file(target.label.name + ".cfg")
ctx.actions.write(config_file, config.to_json())
inputs.append(config_file)
-
- # Run the nogo tool itself.
ctx.actions.run(
inputs = inputs,
outputs = [facts],
- tools = go_ctx.runfiles,
+ tools = depset(go_ctx.runfiles.to_list() + ctx.files._dump_tool),
executable = ctx.files._nogo[0],
mnemonic = "GoStaticAnalysis",
progress_message = "Analyzing %s" % target.label,
- arguments = ["-config=%s" % config_file.path],
+ arguments = go_ctx.nogo_args + [
+ "-binary=%s" % target_objfile.path,
+ "-dump_tool=%s" % ctx.files._dump_tool[0].path,
+ "-package=%s" % config_file.path,
+ ],
)
# Return the package facts as output.
@@ -172,7 +252,12 @@ nogo_aspect = go_rule(
attrs = {
"_nogo": attr.label(
default = "//tools/nogo/check:check",
- allow_single_file = True,
+ ),
+ "_nogo_stdlib": attr.label(
+ default = "//tools/nogo:stdlib",
+ ),
+ "_dump_tool": attr.label(
+ default = "//tools/nogo:dump_tool",
),
},
)
diff --git a/tools/nogo/data/BUILD b/tools/nogo/dump/BUILD
index b7564cc44..dfa29d651 100644
--- a/tools/nogo/data/BUILD
+++ b/tools/nogo/dump/BUILD
@@ -3,8 +3,8 @@ load("//tools:defs.bzl", "go_library")
package(licenses = ["notice"])
go_library(
- name = "data",
- srcs = ["data.go"],
+ name = "dump",
+ srcs = ["dump.go"],
nogo = False,
visibility = ["//tools:__subpackages__"],
)
diff --git a/tools/nogo/dump/dump.go b/tools/nogo/dump/dump.go
new file mode 100644
index 000000000..f06567e0f
--- /dev/null
+++ b/tools/nogo/dump/dump.go
@@ -0,0 +1,78 @@
+// Copyright 2019 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 dump contains data dump tools.
+//
+// The interface used by the package corresponds to the tool generated by the
+// nogo_dump_tool rule.
+//
+// This package is separate in order to avoid a dependency cycle.
+package dump
+
+import (
+ "flag"
+ "fmt"
+ "io"
+ "os"
+ "os/exec"
+)
+
+var (
+ // Binary is the binary under analysis.
+ //
+ // See Reader, below.
+ binary = flag.String("binary", "", "binary under analysis")
+
+ // Reader is the input stream.
+ //
+ // This may be set instead of Binary.
+ Reader io.Reader
+
+ // Tool is the tool used to dump a binary.
+ tool = flag.String("dump_tool", "", "tool used to dump a binary")
+)
+
+// Command returns a command that will emit the dumped object on stdout.
+//
+// You must call Wait on the resulting command.
+func Command() (*exec.Cmd, io.Reader, error) {
+ var (
+ args []string
+ stdin io.Reader
+ )
+ if *binary != "" {
+ args = append(args, *binary)
+ *binary = "" // Clear.
+ } else if Reader != nil {
+ stdin = Reader
+ Reader = nil // Clear.
+ } else {
+ // We have no input stream or binary.
+ return nil, nil, fmt.Errorf("no binary or reader provided!")
+ }
+
+ // Construct our command.
+ cmd := exec.Command(*tool, args...)
+ cmd.Stdin = stdin
+ cmd.Stderr = os.Stderr
+ out, err := cmd.StdoutPipe()
+ if err != nil {
+ return nil, nil, err
+ }
+ if err := cmd.Start(); err != nil {
+ return nil, nil, err
+ }
+
+ return cmd, out, err
+}
diff --git a/tools/nogo/nogo.go b/tools/nogo/nogo.go
index ea1e97076..e44f32d4c 100644
--- a/tools/nogo/nogo.go
+++ b/tools/nogo/nogo.go
@@ -32,51 +32,97 @@ import (
"io/ioutil"
"log"
"os"
+ "path"
"path/filepath"
"reflect"
+ "strings"
"golang.org/x/tools/go/analysis"
"golang.org/x/tools/go/analysis/internal/facts"
"golang.org/x/tools/go/gcexportdata"
- "gvisor.dev/gvisor/tools/nogo/data"
+ "gvisor.dev/gvisor/tools/nogo/dump"
)
-// pkgConfig is serialized as the configuration.
+// stdlibConfig is serialized as the configuration.
//
-// This contains everything required for the analysis.
-type pkgConfig struct {
- ImportPath string
- GoFiles []string
- NonGoFiles []string
- Tags []string
+// This contains everything required for stdlib analysis.
+type stdlibConfig struct {
+ Srcs []string
GOOS string
GOARCH string
- ImportMap map[string]string
- FactMap map[string]string
+ Tags []string
FactOutput string
- Objdump string
- StdZip string
}
-// loadFacts finds and loads facts per FactMap.
-func (c *pkgConfig) loadFacts(path string) ([]byte, error) {
- realPath, ok := c.FactMap[path]
- if !ok {
- return nil, nil // No facts available.
+// packageConfig is serialized as the configuration.
+//
+// This contains everything required for single package analysis.
+type packageConfig struct {
+ ImportPath string
+ GoFiles []string
+ NonGoFiles []string
+ Tags []string
+ GOOS string
+ GOARCH string
+ ImportMap map[string]string
+ FactMap map[string]string
+ FactOutput string
+ StdlibFacts string
+}
+
+// loader is a fact-loader function.
+type loader func(string) ([]byte, error)
+
+// saver is a fact-saver function.
+type saver func([]byte) error
+
+// factLoader returns a function that loads facts.
+//
+// This resolves all standard library facts and imported package facts up
+// front. The returned loader function will never return an error, only
+// empty facts.
+//
+// This is done because all stdlib data is stored together, and we don't want
+// to load this data many times over.
+func (c *packageConfig) factLoader() (loader, error) {
+ allFacts := make(map[string][]byte)
+ if c.StdlibFacts != "" {
+ data, err := ioutil.ReadFile(c.StdlibFacts)
+ if err != nil {
+ return nil, fmt.Errorf("error loading stdlib facts from %q: %w", c.StdlibFacts, err)
+ }
+ var stdlibFacts map[string][]byte
+ if err := json.Unmarshal(data, &stdlibFacts); err != nil {
+ return nil, fmt.Errorf("error loading stdlib facts: %w", err)
+ }
+ for pkg, data := range stdlibFacts {
+ allFacts[pkg] = data
+ }
+ }
+ for pkg, file := range c.FactMap {
+ data, err := ioutil.ReadFile(file)
+ if err != nil {
+ return nil, fmt.Errorf("error loading %q: %w", file, err)
+ }
+ allFacts[pkg] = data
}
+ return func(path string) ([]byte, error) {
+ return allFacts[path], nil
+ }, nil
+}
- // Read the files file.
- data, err := ioutil.ReadFile(realPath)
- if err != nil {
- return nil, err
+// factSaver may be used directly as a saver.
+func (c *packageConfig) factSaver(factData []byte) error {
+ if c.FactOutput == "" {
+ return nil // Nothing to save.
}
- return data, nil
+ return ioutil.WriteFile(c.FactOutput, factData, 0644)
}
// shouldInclude indicates whether the file should be included.
//
// NOTE: This does only basic parsing of tags.
-func (c *pkgConfig) shouldInclude(path string) (bool, error) {
+func (c *packageConfig) shouldInclude(path string) (bool, error) {
ctx := build.Default
ctx.GOOS = c.GOOS
ctx.GOARCH = c.GOARCH
@@ -90,10 +136,11 @@ func (c *pkgConfig) shouldInclude(path string) (bool, error) {
// files, and the facts. Note that this importer implementation will always
// pass when a given package is not available.
type importer struct {
- pkgConfig
- fset *token.FileSet
- cache map[string]*types.Package
- lastErr error
+ *packageConfig
+ fset *token.FileSet
+ cache map[string]*types.Package
+ lastErr error
+ callback func(string) error
}
// Import implements types.Importer.Import.
@@ -104,6 +151,17 @@ func (i *importer) Import(path string) (*types.Package, error) {
// analyzers are specifically looking for this.
return types.Unsafe, nil
}
+
+ // Call the internal callback. This is used to resolve loading order
+ // for the standard library. See checkStdlib.
+ if i.callback != nil {
+ if err := i.callback(path); err != nil {
+ i.lastErr = err
+ return nil, err
+ }
+ }
+
+ // Actually load the data.
realPath, ok := i.ImportMap[path]
var (
rc io.ReadCloser
@@ -112,7 +170,7 @@ func (i *importer) Import(path string) (*types.Package, error) {
if !ok {
// Not found in the import path. Attempt to find the package
// via the standard library.
- rc, err = i.findStdPkg(path)
+ rc, err = findStdPkg(i.GOOS, i.GOARCH, path)
} else {
// Open the file.
rc, err = os.Open(realPath)
@@ -135,6 +193,139 @@ func (i *importer) Import(path string) (*types.Package, error) {
// ErrSkip indicates the package should be skipped.
var ErrSkip = errors.New("skipped")
+// checkStdlib checks the standard library.
+//
+// This constructs a synthetic package configuration for each library in the
+// standard library sources, and call checkPackage repeatedly.
+//
+// Note that not all parts of the source are expected to build. We skip obvious
+// test files, and cmd files, which should not be dependencies.
+func checkStdlib(config *stdlibConfig) ([]string, error) {
+ if len(config.Srcs) == 0 {
+ return nil, nil
+ }
+
+ // Ensure all paths are normalized.
+ for i := 0; i < len(config.Srcs); i++ {
+ config.Srcs[i] = path.Clean(config.Srcs[i])
+ }
+
+ // Calculate the root directory.
+ longestPrefix := path.Dir(config.Srcs[0])
+ for _, file := range config.Srcs[1:] {
+ for i := 0; i < len(file) && i < len(longestPrefix); i++ {
+ if file[i] != longestPrefix[i] {
+ // Truncate here; will stop the loop.
+ longestPrefix = longestPrefix[:i]
+ break
+ }
+ }
+ }
+ if len(longestPrefix) > 0 && longestPrefix[len(longestPrefix)-1] != '/' {
+ longestPrefix += "/"
+ }
+
+ // Aggregate all files by directory.
+ packages := make(map[string]*packageConfig)
+ for _, file := range config.Srcs {
+ d := path.Dir(file)
+ if len(longestPrefix) >= len(d) {
+ continue // Not a file.
+ }
+ pkg := path.Dir(file)[len(longestPrefix):]
+ // Skip cmd packages and obvious test files: see above.
+ if strings.HasPrefix(pkg, "cmd/") || strings.HasSuffix(file, "_test.go") {
+ continue
+ }
+ c, ok := packages[pkg]
+ if !ok {
+ c = &packageConfig{
+ ImportPath: pkg,
+ GOOS: config.GOOS,
+ GOARCH: config.GOARCH,
+ Tags: config.Tags,
+ }
+ packages[pkg] = c
+ }
+ // Add the files appropriately. Note that they will be further
+ // filtered by architecture and build tags below, so this need
+ // not be done immediately.
+ if strings.HasSuffix(file, ".go") {
+ c.GoFiles = append(c.GoFiles, file)
+ } else {
+ c.NonGoFiles = append(c.NonGoFiles, file)
+ }
+ }
+
+ // Closure to check a single package.
+ allFindings := make([]string, 0)
+ stdlibFacts := make(map[string][]byte)
+ var checkOne func(pkg string) error // Recursive.
+ checkOne = func(pkg string) error {
+ // Is this already done?
+ if _, ok := stdlibFacts[pkg]; ok {
+ return nil
+ }
+
+ // Lookup the configuration.
+ config, ok := packages[pkg]
+ if !ok {
+ return nil // Not known.
+ }
+
+ // Find the binary package, and provide to objdump.
+ rc, err := findStdPkg(config.GOOS, config.GOARCH, pkg)
+ if err != nil {
+ // If there's no binary for this package, it is likely
+ // not built with the distribution. That's fine, we can
+ // just skip analysis.
+ return nil
+ }
+
+ // Provide the input.
+ oldReader := dump.Reader
+ dump.Reader = rc // For analysis.
+ defer func() {
+ rc.Close()
+ dump.Reader = oldReader // Restore.
+ }()
+
+ // Run the analysis.
+ findings, err := checkPackage(config, func(factData []byte) error {
+ stdlibFacts[pkg] = factData
+ return nil
+ }, checkOne)
+ if err != nil {
+ // If we can't analyze a package from the standard library,
+ // then we skip it. It will simply not have any findings.
+ return nil
+ }
+ allFindings = append(allFindings, findings...)
+ return nil
+ }
+
+ // Check all packages.
+ //
+ // Note that this may call checkOne recursively, so it's not guaranteed
+ // to evaluate in the order provided here. We do ensure however, that
+ // all packages are evaluated.
+ for pkg := range packages {
+ checkOne(pkg)
+ }
+
+ // Write out all findings.
+ factData, err := json.Marshal(stdlibFacts)
+ if err != nil {
+ return nil, fmt.Errorf("error saving stdlib facts: %w", err)
+ }
+ if err := ioutil.WriteFile(config.FactOutput, factData, 0644); err != nil {
+ return nil, fmt.Errorf("error saving findings to %q: %v", config.FactOutput, err)
+ }
+
+ // Return all findings.
+ return allFindings, nil
+}
+
// checkPackage runs all analyzers.
//
// The implementation was adapted from [1], which was in turn adpated from [2].
@@ -143,11 +334,12 @@ var ErrSkip = errors.New("skipped")
//
// [1] bazelbuid/rules_go/tools/builders/nogo_main.go
// [2] golang.org/x/tools/go/checker/internal/checker
-func checkPackage(config pkgConfig) ([]string, error) {
+func checkPackage(config *packageConfig, factSaver saver, importCallback func(string) error) ([]string, error) {
imp := &importer{
- pkgConfig: config,
- fset: token.NewFileSet(),
- cache: make(map[string]*types.Package),
+ packageConfig: config,
+ fset: token.NewFileSet(),
+ cache: make(map[string]*types.Package),
+ callback: importCallback,
}
// Load all source files.
@@ -184,14 +376,15 @@ func checkPackage(config pkgConfig) ([]string, error) {
}
// Load all package facts.
- facts, err := facts.Decode(types, config.loadFacts)
+ loader, err := config.factLoader()
+ if err != nil {
+ return nil, fmt.Errorf("error loading facts: %w", err)
+ }
+ facts, err := facts.Decode(types, loader)
if err != nil {
return nil, fmt.Errorf("error decoding facts: %w", err)
}
- // Set the binary global for use.
- data.Objdump = config.Objdump
-
// Register fact types and establish dependencies between analyzers.
// The visit closure will execute recursively, and populate results
// will all required analysis results.
@@ -263,11 +456,9 @@ func checkPackage(config pkgConfig) ([]string, error) {
}
// Write the output file.
- if config.FactOutput != "" {
- factData := facts.Encode()
- if err := ioutil.WriteFile(config.FactOutput, factData, 0644); err != nil {
- return nil, fmt.Errorf("error: unable to open facts output %q: %v", config.FactOutput, err)
- }
+ factData := facts.Encode()
+ if err := factSaver(factData); err != nil {
+ return nil, fmt.Errorf("error: unable to save facts: %v", err)
}
// Convert all diagnostics to strings.
@@ -284,38 +475,56 @@ func checkPackage(config pkgConfig) ([]string, error) {
}
var (
- configFile = flag.String("config", "", "configuration file (in JSON format)")
+ packageFile = flag.String("package", "", "package configuration file (in JSON format)")
+ stdlibFile = flag.String("stdlib", "", "stdlib configuration file (in JSON format)")
)
-// Main is the entrypoint; it should be called directly from main.
-//
-// N.B. This package registers it's own flags.
-func Main() {
- // Parse all flags.
- flag.Parse()
-
+func loadConfig(file string, config interface{}) interface{} {
// Load the configuration.
- f, err := os.Open(*configFile)
+ f, err := os.Open(file)
if err != nil {
- log.Fatalf("unable to open configuration %q: %v", *configFile, err)
+ log.Fatalf("unable to open configuration %q: %v", file, err)
}
defer f.Close()
- config := new(pkgConfig)
dec := json.NewDecoder(f)
dec.DisallowUnknownFields()
if err := dec.Decode(config); err != nil {
log.Fatalf("unable to decode configuration: %v", err)
}
+ return config
+}
- // Process the package.
- findings, err := checkPackage(*config)
+// Main is the entrypoint; it should be called directly from main.
+//
+// N.B. This package registers it's own flags.
+func Main() {
+ // Parse all flags.
+ flag.Parse()
+
+ var (
+ findings []string
+ err error
+ )
+
+ // Check the configuration.
+ if *packageFile != "" && *stdlibFile != "" {
+ log.Fatalf("unable to perform stdlib and package analysis; provide only one!")
+ } else if *stdlibFile != "" {
+ c := loadConfig(*stdlibFile, new(stdlibConfig)).(*stdlibConfig)
+ findings, err = checkStdlib(c)
+ } else if *packageFile != "" {
+ c := loadConfig(*packageFile, new(packageConfig)).(*packageConfig)
+ findings, err = checkPackage(c, c.factSaver, nil)
+ } else {
+ log.Fatalf("please provide at least one of package or stdlib!")
+ }
+
+ // Handle findings & errors.
if err != nil {
log.Fatalf("error checking package: %v", err)
}
-
- // No findings?
if len(findings) == 0 {
- os.Exit(0)
+ return
}
// Print findings and exit with non-zero code.