Change nogo failures to test failures, instead of build failures.

PiperOrigin-RevId: 329408633
This commit is contained in:
Adin Scannell
2020-08-31 17:09:20 -07:00
committed by gVisor bot
parent 170560cec0
commit 101c97d6f8
12 changed files with 705 additions and 583 deletions
-1
View File
@@ -8,7 +8,6 @@ go_library(
nogo = False,
visibility = ["//tools/nogo:__subpackages__"],
deps = [
"//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",
File diff suppressed because it is too large Load Diff
+1
View File
@@ -175,6 +175,7 @@ func Split() {
// +mustescape:stack
//go:noinline
//go:nosplit
func splitRec() {
Split()
}
+1
View File
@@ -83,6 +83,7 @@ func dynamicCrossPkg(f func()) {
// +mustescape:stack
//go:noinline
//go:nosplit
func splitCrosssPkt() {
test1.Split()
}
+4
View File
@@ -64,6 +64,7 @@ func doCopyOut(t *dummyTask) {
// +mustescape:builtin
// +mustescape:stack
//go:nosplit
func doMarshalBytesDirect(t *dummyTask) {
var stat test.Stat
buf := t.CopyScratchBuffer(stat.SizeBytes())
@@ -73,6 +74,7 @@ func doMarshalBytesDirect(t *dummyTask) {
// +mustescape:builtin
// +mustescape:stack
//go:nosplit
func doMarshalUnsafeDirect(t *dummyTask) {
var stat test.Stat
buf := t.CopyScratchBuffer(stat.SizeBytes())
@@ -82,6 +84,7 @@ func doMarshalUnsafeDirect(t *dummyTask) {
// +mustescape:local,heap
// +mustescape:stack
//go:nosplit
func doMarshalBytesViaMarshallable(t *dummyTask) {
var stat test.Stat
t.MarshalBytes(usermem.Addr(0xf000ba12), &stat)
@@ -89,6 +92,7 @@ func doMarshalBytesViaMarshallable(t *dummyTask) {
// +mustescape:local,heap
// +mustescape:stack
//go:nosplit
func doMarshalUnsafeViaMarshallable(t *dummyTask) {
var stat test.Stat
t.MarshalUnsafe(usermem.Addr(0xf000ba12), &stat)
-1
View File
@@ -27,7 +27,6 @@ go_library(
deps = [
"//tools/checkescape",
"//tools/checkunsafe",
"//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",
+5
View File
@@ -122,3 +122,8 @@ var analyzerConfig = map[*analysis.Analyzer]matcher{
checkescape.Analyzer: internalMatches(),
checkunsafe.Analyzer: internalMatches(),
}
var escapesConfig = map[*analysis.Analyzer]matcher{
// Informational only: include all packages.
checkescape.EscapeAnalyzer: alwaysMatches(),
}
+48 -22
View File
@@ -50,6 +50,7 @@ NogoStdlibInfo = provider(
"information for nogo analysis (standard library facts)",
fields = {
"facts": "serialized standard library facts",
"findings": "package findings (if relevant)",
},
)
@@ -59,18 +60,18 @@ def _nogo_stdlib_impl(ctx):
# Build the standard library facts.
facts = ctx.actions.declare_file(ctx.label.name + ".facts")
findings = ctx.actions.declare_file(ctx.label.name + ".findings")
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],
outputs = [facts, findings],
tools = depset(go_ctx.runfiles.to_list() + ctx.files._dump_tool),
executable = ctx.files._nogo[0],
mnemonic = "GoStandardLibraryAnalysis",
@@ -78,12 +79,15 @@ def _nogo_stdlib_impl(ctx):
arguments = go_ctx.nogo_args + [
"-dump_tool=%s" % ctx.files._dump_tool[0].path,
"-stdlib=%s" % config_file.path,
"-findings=%s" % findings.path,
"-facts=%s" % facts.path,
],
)
# Return the stdlib facts as output.
return [NogoStdlibInfo(
facts = facts,
findings = findings,
)]
nogo_stdlib = go_rule(
@@ -108,6 +112,7 @@ NogoInfo = provider(
"information for nogo analysis",
fields = {
"facts": "serialized package facts",
"findings": "package findings (if relevant)",
"importpath": "package import path",
"binaries": "package binary files",
"srcs": "original source files (for go_test support)",
@@ -203,6 +208,8 @@ def _nogo_aspect_impl(target, ctx):
# The nogo tool operates on a configuration serialized in JSON format.
facts = ctx.actions.declare_file(target.label.name + ".facts")
findings = ctx.actions.declare_file(target.label.name + ".findings")
escapes = ctx.actions.declare_file(target.label.name + ".escapes")
config = struct(
ImportPath = importpath,
GoFiles = [src.path for src in srcs if src.path.endswith(".go")],
@@ -213,14 +220,13 @@ def _nogo_aspect_impl(target, ctx):
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)
ctx.actions.run(
inputs = inputs,
outputs = [facts],
outputs = [facts, findings, escapes],
tools = depset(go_ctx.runfiles.to_list() + ctx.files._dump_tool),
executable = ctx.files._nogo[0],
mnemonic = "GoStaticAnalysis",
@@ -229,17 +235,30 @@ def _nogo_aspect_impl(target, ctx):
"-binary=%s" % target_objfile.path,
"-dump_tool=%s" % ctx.files._dump_tool[0].path,
"-package=%s" % config_file.path,
"-findings=%s" % findings.path,
"-facts=%s" % facts.path,
"-escapes=%s" % escapes.path,
],
)
# Return the package facts as output.
return [NogoInfo(
facts = facts,
importpath = importpath,
binaries = binaries,
srcs = srcs,
deps = deps,
)]
return [
NogoInfo(
facts = facts,
findings = findings,
importpath = importpath,
binaries = binaries,
srcs = srcs,
deps = deps,
),
OutputGroupInfo(
# Expose all findings (should just be a single file). This can be
# used for build analysis of the nogo findings.
nogo_findings = depset([findings]),
# Expose all escape analysis findings (see above).
nogo_escapes = depset([escapes]),
),
]
nogo_aspect = go_rule(
aspect,
@@ -250,15 +269,9 @@ nogo_aspect = go_rule(
"embed",
],
attrs = {
"_nogo": attr.label(
default = "//tools/nogo/check:check",
),
"_nogo_stdlib": attr.label(
default = "//tools/nogo:stdlib",
),
"_dump_tool": attr.label(
default = "//tools/nogo:dump_tool",
),
"_nogo": attr.label(default = "//tools/nogo/check:check"),
"_nogo_stdlib": attr.label(default = "//tools/nogo:stdlib"),
"_dump_tool": attr.label(default = "//tools/nogo:dump_tool"),
},
)
@@ -270,13 +283,26 @@ def _nogo_test_impl(ctx):
# this way so that any test applied is effectively pushed down to all
# upstream dependencies through the aspect.
inputs = []
findings = []
runner = ctx.actions.declare_file("%s-executer" % ctx.label.name)
runner_content = ["#!/bin/bash"]
for dep in ctx.attr.deps:
# Extract the findings.
info = dep[NogoInfo]
inputs.append(info.facts)
inputs.append(info.findings)
findings.append(info.findings)
# Draw a sweet unicode checkmark with the package name (in green).
# Include all source files, transitively. This will make this target
# "directly affected" for the purpose of build analysis.
inputs += info.srcs
# If there are findings, dump them and fail.
runner_content.append("if [[ -s \"%s\" ]]; then cat \"%s\" && exit 1; fi" % (
info.findings.short_path,
info.findings.short_path,
))
# Otherwise, draw a sweet unicode checkmark with the package name (in green).
runner_content.append("echo -e \"\\033[0;32m\\xE2\\x9C\\x94\\033[0;31m\\033[0m %s\"" % info.importpath)
runner_content.append("exit 0\n")
ctx.actions.write(runner, "\n".join(runner_content), is_executable = True)
-10
View File
@@ -1,10 +0,0 @@
load("//tools:defs.bzl", "go_library")
package(licenses = ["notice"])
go_library(
name = "dump",
srcs = ["dump.go"],
nogo = False,
visibility = ["//tools:__subpackages__"],
)
-78
View File
@@ -1,78 +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 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
}
+73 -53
View File
@@ -40,18 +40,19 @@ import (
"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/dump"
// Special case: flags live here and change overall behavior.
"gvisor.dev/gvisor/tools/checkescape"
)
// stdlibConfig is serialized as the configuration.
//
// This contains everything required for stdlib analysis.
type stdlibConfig struct {
Srcs []string
GOOS string
GOARCH string
Tags []string
FactOutput string
Srcs []string
GOOS string
GOARCH string
Tags []string
}
// packageConfig is serialized as the configuration.
@@ -66,7 +67,6 @@ type packageConfig struct {
GOARCH string
ImportMap map[string]string
FactMap map[string]string
FactOutput string
StdlibFacts string
}
@@ -111,14 +111,6 @@ func (c *packageConfig) factLoader() (loader, error) {
}, nil
}
// factSaver may be used directly as a saver.
func (c *packageConfig) factSaver(factData []byte) error {
if c.FactOutput == "" {
return nil // Nothing to save.
}
return ioutil.WriteFile(c.FactOutput, factData, 0644)
}
// shouldInclude indicates whether the file should be included.
//
// NOTE: This does only basic parsing of tags.
@@ -200,9 +192,9 @@ var ErrSkip = errors.New("skipped")
//
// 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) {
func checkStdlib(config *stdlibConfig, ac map[*analysis.Analyzer]matcher) ([]string, []byte, error) {
if len(config.Srcs) == 0 {
return nil, nil
return nil, nil, nil
}
// Ensure all paths are normalized.
@@ -283,23 +275,21 @@ func checkStdlib(config *stdlibConfig) ([]string, error) {
}
// Provide the input.
oldReader := dump.Reader
dump.Reader = rc // For analysis.
oldReader := checkescape.Reader
checkescape.Reader = rc // For analysis.
defer func() {
rc.Close()
dump.Reader = oldReader // Restore.
checkescape.Reader = oldReader // Restore.
}()
// Run the analysis.
findings, err := checkPackage(config, func(factData []byte) error {
stdlibFacts[pkg] = factData
return nil
}, checkOne)
findings, factData, err := checkPackage(config, ac, 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
}
stdlibFacts[pkg] = factData
allFindings = append(allFindings, findings...)
return nil
}
@@ -316,14 +306,11 @@ func checkStdlib(config *stdlibConfig) ([]string, error) {
// 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 nil, nil, fmt.Errorf("error saving stdlib facts: %w", err)
}
// Return all findings.
return allFindings, nil
return allFindings, factData, nil
}
// checkPackage runs all analyzers.
@@ -334,7 +321,7 @@ func checkStdlib(config *stdlibConfig) ([]string, error) {
//
// [1] bazelbuid/rules_go/tools/builders/nogo_main.go
// [2] golang.org/x/tools/go/checker/internal/checker
func checkPackage(config *packageConfig, factSaver saver, importCallback func(string) error) ([]string, error) {
func checkPackage(config *packageConfig, ac map[*analysis.Analyzer]matcher, importCallback func(string) error) ([]string, []byte, error) {
imp := &importer{
packageConfig: config,
fset: token.NewFileSet(),
@@ -347,14 +334,14 @@ func checkPackage(config *packageConfig, factSaver saver, importCallback func(st
for _, file := range config.GoFiles {
include, err := config.shouldInclude(file)
if err != nil {
return nil, fmt.Errorf("error evaluating file %q: %v", file, err)
return nil, nil, fmt.Errorf("error evaluating file %q: %v", file, err)
}
if !include {
continue
}
s, err := parser.ParseFile(imp.fset, file, nil, parser.ParseComments)
if err != nil {
return nil, fmt.Errorf("error parsing file %q: %v", file, err)
return nil, nil, fmt.Errorf("error parsing file %q: %v", file, err)
}
syntax = append(syntax, s)
}
@@ -372,17 +359,17 @@ func checkPackage(config *packageConfig, factSaver saver, importCallback func(st
}
types, err := typeConfig.Check(config.ImportPath, imp.fset, syntax, typesInfo)
if err != nil && imp.lastErr != ErrSkip {
return nil, fmt.Errorf("error checking types: %w", err)
return nil, nil, fmt.Errorf("error checking types: %w", err)
}
// Load all package facts.
loader, err := config.factLoader()
if err != nil {
return nil, fmt.Errorf("error loading facts: %w", err)
return nil, 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)
return nil, nil, fmt.Errorf("error decoding facts: %w", err)
}
// Register fact types and establish dependencies between analyzers.
@@ -404,7 +391,7 @@ func checkPackage(config *packageConfig, factSaver saver, importCallback func(st
}
// Prepare the matcher.
m := analyzerConfig[a]
m := ac[a]
report := func(d analysis.Diagnostic) {
if m.ShouldReport(d, imp.fset) {
diagnostics[a] = append(diagnostics[a], d)
@@ -445,22 +432,16 @@ func checkPackage(config *packageConfig, factSaver saver, importCallback func(st
return nil // Success.
}
// Visit all analysis recursively.
for a, _ := range analyzerConfig {
// Visit all analyzers recursively.
for a, _ := range ac {
if imp.lastErr == ErrSkip {
continue // No local analysis.
}
if err := visit(a); err != nil {
return nil, err // Already has context.
return nil, nil, err // Already has context.
}
}
// Write the output file.
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.
findings := make([]string, 0, len(diagnostics))
for a, ds := range diagnostics {
@@ -471,12 +452,16 @@ func checkPackage(config *packageConfig, factSaver saver, importCallback func(st
}
// Return all findings.
return findings, nil
factData := facts.Encode()
return findings, factData, nil
}
var (
packageFile = flag.String("package", "", "package configuration file (in JSON format)")
stdlibFile = flag.String("stdlib", "", "stdlib 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)")
findingsOutput = flag.String("findings", "", "output file (or stdout, if not specified)")
factsOutput = flag.String("facts", "", "output file for facts (optional)")
escapesOutput = flag.String("escapes", "", "output file for escapes (optional)")
)
func loadConfig(file string, config interface{}) interface{} {
@@ -503,6 +488,7 @@ func Main() {
var (
findings []string
factData []byte
err error
)
@@ -510,15 +496,50 @@ func Main() {
if *packageFile != "" && *stdlibFile != "" {
log.Fatalf("unable to perform stdlib and package analysis; provide only one!")
} else if *stdlibFile != "" {
// Perform basic analysis.
c := loadConfig(*stdlibFile, new(stdlibConfig)).(*stdlibConfig)
findings, err = checkStdlib(c)
findings, factData, err = checkStdlib(c, analyzerConfig)
} else if *packageFile != "" {
// Perform basic analysis.
c := loadConfig(*packageFile, new(packageConfig)).(*packageConfig)
findings, err = checkPackage(c, c.factSaver, nil)
findings, factData, err = checkPackage(c, analyzerConfig, nil)
// Do we need to do escape analysis?
if *escapesOutput != "" {
escapes, _, err := checkPackage(c, escapesConfig, nil)
if err != nil {
log.Fatalf("error performing escape analysis: %v", err)
}
f, err := os.OpenFile(*escapesOutput, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0644)
if err != nil {
log.Fatalf("unable to open output %q: %v", *escapesOutput, err)
}
defer f.Close()
for _, escape := range escapes {
fmt.Fprintf(f, "%s\n", escape)
}
}
} else {
log.Fatalf("please provide at least one of package or stdlib!")
}
// Save facts.
if *factsOutput != "" {
if err := ioutil.WriteFile(*factsOutput, factData, 0644); err != nil {
log.Fatalf("error saving findings to %q: %v", *factsOutput, err)
}
}
// Open the output file.
var w io.Writer = os.Stdout
if *findingsOutput != "" {
f, err := os.OpenFile(*findingsOutput, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0644)
if err != nil {
log.Fatalf("unable to open output %q: %v", *findingsOutput, err)
}
defer f.Close()
w = f
}
// Handle findings & errors.
if err != nil {
log.Fatalf("error checking package: %v", err)
@@ -527,9 +548,8 @@ func Main() {
return
}
// Print findings and exit with non-zero code.
// Print findings.
for _, finding := range findings {
fmt.Fprintf(os.Stdout, "%s\n", finding)
fmt.Fprintf(w, "%s\n", finding)
}
os.Exit(1)
}
+3
View File
@@ -26,6 +26,9 @@ func analyzers() (all []*analysis.Analyzer) {
for a, _ := range analyzerConfig {
all = append(all, a)
}
for a, _ := range escapesConfig {
all = append(all, a)
}
return all
}