diff options
author | Adin Scannell <ascannell@google.com> | 2020-08-31 17:07:35 -0700 |
---|---|---|
committer | gVisor bot <gvisor-bot@google.com> | 2020-08-31 17:09:20 -0700 |
commit | 101c97d6f851abbd3024605102757da66a36551f (patch) | |
tree | 5e5154b4449c7a7dec9887cbc6e49329617e03fc | |
parent | 170560cec01f99d49f4c2f09f2a5655dd376fac7 (diff) |
Change nogo failures to test failures, instead of build failures.
PiperOrigin-RevId: 329408633
-rw-r--r-- | tools/checkescape/BUILD | 1 | ||||
-rw-r--r-- | tools/checkescape/checkescape.go | 832 | ||||
-rw-r--r-- | tools/checkescape/test1/test1.go | 1 | ||||
-rw-r--r-- | tools/checkescape/test2/test2.go | 1 | ||||
-rw-r--r-- | tools/go_marshal/test/escape/escape.go | 4 | ||||
-rw-r--r-- | tools/nogo/BUILD | 1 | ||||
-rw-r--r-- | tools/nogo/config.go | 5 | ||||
-rw-r--r-- | tools/nogo/defs.bzl | 70 | ||||
-rw-r--r-- | tools/nogo/dump/BUILD | 10 | ||||
-rw-r--r-- | tools/nogo/dump/dump.go | 78 | ||||
-rw-r--r-- | tools/nogo/nogo.go | 126 | ||||
-rw-r--r-- | tools/nogo/register.go | 3 |
12 files changed, 627 insertions, 505 deletions
diff --git a/tools/checkescape/BUILD b/tools/checkescape/BUILD index 6273aa779..8956be621 100644 --- a/tools/checkescape/BUILD +++ b/tools/checkescape/BUILD @@ -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", diff --git a/tools/checkescape/checkescape.go b/tools/checkescape/checkescape.go index aab3c36a1..d98f5c3a1 100644 --- a/tools/checkescape/checkescape.go +++ b/tools/checkescape/checkescape.go @@ -61,19 +61,20 @@ package checkescape import ( "bufio" "bytes" + "flag" "fmt" "go/ast" "go/token" "go/types" "io" + "os" + "os/exec" "path/filepath" - "strconv" "strings" "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/dump" ) const ( @@ -90,81 +91,20 @@ const ( exempt = "// escapes" ) -// escapingBuiltins are builtins known to escape. -// -// These are lowered at an earlier stage of compilation to explicit function -// calls, but are not available for recursive analysis. -var escapingBuiltins = []string{ - "append", - "makemap", - "newobject", - "mallocgc", -} - -// Analyzer defines the entrypoint. -var Analyzer = &analysis.Analyzer{ - Name: "checkescape", - Doc: "surfaces recursive escape analysis results", - Run: run, - Requires: []*analysis.Analyzer{buildssa.Analyzer}, - FactTypes: []analysis.Fact{(*packageEscapeFacts)(nil)}, -} - -// packageEscapeFacts is the set of all functions in a package, and whether or -// not they recursively pass escape analysis. -// -// All the type names for receivers are encoded in the full key. The key -// represents the fully qualified package and type name used at link time. -type packageEscapeFacts struct { - Funcs map[string][]Escape -} - -// AFact implements analysis.Fact.AFact. -func (*packageEscapeFacts) AFact() {} - -// CallSite is a single call site. -// -// These can be chained. -type CallSite struct { - LocalPos token.Pos - Resolved LinePosition -} - -// Escape is a single escape instance. -type Escape struct { - Reason EscapeReason - Detail string - Chain []CallSite -} - -// LinePosition is a low-resolution token.Position. -// -// This is used to match against possible exemptions placed in the source. -type LinePosition struct { - Filename string - Line int -} +var ( + // Binary is the binary under analysis. + // + // See Reader, below. + binary = flag.String("binary", "", "binary under analysis") -// String implements fmt.Stringer.String. -func (e *LinePosition) String() string { - return fmt.Sprintf("%s:%d", e.Filename, e.Line) -} + // Reader is the input stream. + // + // This may be set instead of Binary. + Reader io.Reader -// String implements fmt.Stringer.String. -// -// Note that this string will contain new lines. -func (e *Escape) String() string { - var b bytes.Buffer - fmt.Fprintf(&b, "%s", e.Reason.String()) - for i, cs := range e.Chain { - if i == len(e.Chain)-1 { - fmt.Fprintf(&b, "\n @ %s → %s", cs.Resolved.String(), e.Detail) - } else { - fmt.Fprintf(&b, "\n + %s", cs.Resolved.String()) - } - } - return b.String() -} + // Tool is the tool used to dump a binary. + tool = flag.String("dump_tool", "", "tool used to dump a binary") +) // EscapeReason is an escape reason. // @@ -172,12 +112,12 @@ func (e *Escape) String() string { type EscapeReason int const ( - interfaceInvoke EscapeReason = iota - unknownPackage - allocation + allocation EscapeReason = iota builtin + interfaceInvoke dynamicCall stackSplit + unknownPackage reasonCount // Count for below. ) @@ -188,17 +128,17 @@ const ( func (e EscapeReason) String() string { switch e { case interfaceInvoke: - return "interface: function invocation via interface" + return "interface: call to potentially allocating function" case unknownPackage: return "unknown: no package information available" case allocation: - return "heap: call to runtime heap allocation" + return "heap: explicit allocation" case builtin: - return "builtin: call to runtime builtin" + return "builtin: call to potentially allocating builtin" case dynamicCall: - return "dynamic: call via dynamic function" + return "dynamic: call to potentially allocating function" case stackSplit: - return "stack: stack split on function entry" + return "stack: possible split on function entry" default: panic(fmt.Sprintf("unknown reason: %d", e)) } @@ -227,46 +167,241 @@ var escapeTypes = func() map[string]EscapeReason { return result }() -// EscapeCount counts escapes. +// escapingBuiltins are builtins known to escape. // -// It is used to avoid accumulating too many escapes for the same reason, for -// the same function. We limit each class to 3 instances (arbitrarily). -type EscapeCount struct { - byReason [reasonCount]uint32 +// These are lowered at an earlier stage of compilation to explicit function +// calls, but are not available for recursive analysis. +var escapingBuiltins = []string{ + "append", + "makemap", + "newobject", + "mallocgc", } -// maxRecordsPerReason is the number of explicit records. +// packageEscapeFacts is the set of all functions in a package, and whether or +// not they recursively pass escape analysis. // -// See EscapeCount (and usage), and Record implementation. -const maxRecordsPerReason = 5 - -// Record records the reason or returns false if it should not be added. -func (ec *EscapeCount) Record(reason EscapeReason) bool { - ec.byReason[reason]++ - if ec.byReason[reason] > maxRecordsPerReason { - return false +// All the type names for receivers are encoded in the full key. The key +// represents the fully qualified package and type name used at link time. +// +// Note that each Escapes object is a summary. Local findings may be reported +// using more detailed information. +type packageEscapeFacts struct { + Funcs map[string]Escapes +} + +// AFact implements analysis.Fact.AFact. +func (*packageEscapeFacts) AFact() {} + +// Analyzer includes specific results. +var Analyzer = &analysis.Analyzer{ + Name: "checkescape", + Doc: "escape analysis checks based on +checkescape annotations", + Run: runSelectEscapes, + Requires: []*analysis.Analyzer{buildssa.Analyzer}, + FactTypes: []analysis.Fact{(*packageEscapeFacts)(nil)}, +} + +// EscapeAnalyzer includes all local escape results. +var EscapeAnalyzer = &analysis.Analyzer{ + Name: "checkescape", + Doc: "complete local escape analysis results (requires Analyzer facts)", + Run: runAllEscapes, + Requires: []*analysis.Analyzer{buildssa.Analyzer}, +} + +// LinePosition is a low-resolution token.Position. +// +// This is used to match against possible exemptions placed in the source. +type LinePosition struct { + Filename string + Line int +} + +// String implements fmt.Stringer.String. +func (e LinePosition) String() string { + return fmt.Sprintf("%s:%d", e.Filename, e.Line) +} + +// Simplified returns the simplified name. +func (e LinePosition) Simplified() string { + return fmt.Sprintf("%s:%d", filepath.Base(e.Filename), e.Line) +} + +// CallSite is a single call site. +// +// These can be chained. +type CallSite struct { + LocalPos token.Pos + Resolved LinePosition +} + +// IsValid indicates whether the CallSite is valid or not. +func (cs *CallSite) IsValid() bool { + return cs.LocalPos.IsValid() +} + +// Escapes is a collection of escapes. +// +// We record at most one escape for each reason, but record the number of +// escapes that were omitted. +// +// This object should be used to summarize all escapes for a single line (local +// analysis) or a single function (package facts). +// +// All fields are exported for gob. +type Escapes struct { + CallSites [reasonCount][]CallSite + Details [reasonCount]string + Omitted [reasonCount]int +} + +// add is called by Add and Merge. +func (es *Escapes) add(r EscapeReason, detail string, omitted int, callSites ...CallSite) { + if es.CallSites[r] != nil { + // We will either be replacing the current escape or dropping + // the added one. Either way, we increment omitted by the + // appropriate amount. + es.Omitted[r]++ + // If the callSites in the other is only a single element, then + // we will universally favor this. This provides the cleanest + // set of escapes to summarize, and more importantly: if there + if len(es.CallSites) == 1 || len(callSites) != 1 { + return + } + } + es.Details[r] = detail + es.CallSites[r] = callSites + es.Omitted[r] += omitted +} + +// Add adds a single escape. +func (es *Escapes) Add(r EscapeReason, detail string, callSites ...CallSite) { + es.add(r, detail, 0, callSites...) +} + +// IsEmpty returns true iff this Escapes is empty. +func (es *Escapes) IsEmpty() bool { + for _, cs := range es.CallSites { + if cs != nil { + return false + } } return true } +// Filter filters out all escapes except those matches the given reasons. +// +// If local is set, then non-local escapes will also be filtered. +func (es *Escapes) Filter(reasons []EscapeReason, local bool) { +FilterReasons: + for r := EscapeReason(0); r < reasonCount; r++ { + for i := 0; i < len(reasons); i++ { + if r == reasons[i] { + continue FilterReasons + } + } + // Zap this reason. + es.CallSites[r] = nil + es.Details[r] = "" + es.Omitted[r] = 0 + } + if !local { + return + } + for r := EscapeReason(0); r < reasonCount; r++ { + // Is does meet our local requirement? + if len(es.CallSites[r]) > 1 { + es.CallSites[r] = nil + es.Details[r] = "" + es.Omitted[r] = 0 + } + } +} + +// MergeWithCall merges these escapes with another. +// +// If callSite is nil, no call is added. +func (es *Escapes) MergeWithCall(other Escapes, callSite CallSite) { + for r := EscapeReason(0); r < reasonCount; r++ { + if other.CallSites[r] != nil { + // Construct our new call chain. + newCallSites := other.CallSites[r] + if callSite.IsValid() { + newCallSites = append([]CallSite{callSite}, newCallSites...) + } + // Add (potentially replacing) the underlying escape. + es.add(r, other.Details[r], other.Omitted[r], newCallSites...) + } + } +} + +// Reportf will call Reportf for each class of escapes. +func (es *Escapes) Reportf(pass *analysis.Pass) { + var b bytes.Buffer // Reused for all escapes. + for r := EscapeReason(0); r < reasonCount; r++ { + if es.CallSites[r] == nil { + continue + } + b.Reset() + fmt.Fprintf(&b, "%s ", r.String()) + if es.Omitted[r] > 0 { + fmt.Fprintf(&b, "(%d omitted) ", es.Omitted[r]) + } + for _, cs := range es.CallSites[r][1:] { + fmt.Fprintf(&b, "→ %s ", cs.Resolved.String()) + } + fmt.Fprintf(&b, "→ %s", es.Details[r]) + pass.Reportf(es.CallSites[r][0].LocalPos, b.String()) + } +} + +// MergeAll merges a sequence of escapes. +func MergeAll(others []Escapes) (es Escapes) { + for _, other := range others { + es.MergeWithCall(other, CallSite{}) + } + return +} + // loadObjdump reads the objdump output. // // This records if there is a call any function for every source line. It is // 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) { - cmd, out, err := dump.Command() +// +// Note that the map uses <basename.go>:<line> because that is all that is +// provided in the objdump format. Since this is all local, it is sufficient. +func loadObjdump() (map[string][]string, error) { + var ( + args []string + stdin io.Reader + ) + if *binary != "" { + args = append(args, *binary) + } else if Reader != nil { + stdin = Reader + } else { + // We have no input stream or binary. + return 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, err } + if err := cmd.Start(); err != nil { + return nil, err + } // Build the map. - m := make(map[LinePosition]string) + m := make(map[string][]string) r := bufio.NewReader(out) - var ( - lastField string - lastPos LinePosition - ) +NextLine: for { line, err := r.ReadString('\n') if err != nil && err != io.EOF { @@ -286,41 +421,62 @@ func loadObjdump() (map[LinePosition]string, error) { if !strings.Contains(fields[3], "CALL") { continue } + site := strings.TrimSpace(fields[0]) + var callStr string // Friendly string. + if len(fields) > 5 { + callStr = strings.Join(fields[5:], " ") + } + if len(callStr) == 0 { + // Just a raw call? is this asm? + callStr = strings.Join(fields[3:], " ") + } // Ignore strings containing duffzero, which is just // used by stack allocations for types that are large // enough to warrant Duff's device. - if strings.Contains(line, "runtime.duffzero") { + if strings.Contains(callStr, "runtime.duffzero") || + strings.Contains(callStr, "runtime.duffcopy") { continue } // Ignore the racefuncenter call, which is used for // race builds. This does not escape. - if strings.Contains(line, "runtime.racefuncenter") { + if strings.Contains(callStr, "runtime.racefuncenter") { continue } - // Calculate the filename and line. Note that per the - // example above, the filename is not a fully qualified - // base, just the basename (what we require). - if fields[0] != lastField { - parts := strings.SplitN(fields[0], ":", 2) - lineNum, err := strconv.ParseInt(parts[1], 10, 64) - if err != nil { - return nil, err - } - lastPos = LinePosition{ - Filename: parts[0], - Line: int(lineNum), - } - lastField = fields[0] + // Ignore the write barriers. + if strings.Contains(callStr, "runtime.gcWriteBarrier") { + continue } - if _, ok := m[lastPos]; ok { - continue // Already marked. + + // Ignore retpolines. + if strings.Contains(callStr, "runtime.retpoline") { + continue } - // Save the actual call for the detail. - m[lastPos] = strings.Join(fields[3:], " ") + // Ignore stack sanity check (does not split). + if strings.Contains(callStr, "runtime.stackcheck") { + continue + } + + // Ignore tls functions. + if strings.Contains(callStr, "runtime.settls") { + continue + } + + // Does this exist already? + existing, ok := m[site] + if !ok { + existing = make([]string, 0, 1) + } + for _, other := range existing { + if callStr == other { + continue NextLine + } + } + existing = append(existing, callStr) + m[site] = existing // Update. } if err == io.EOF { break @@ -340,65 +496,148 @@ type poser interface { Pos() token.Pos } +// runSelectEscapes runs with only select escapes. +func runSelectEscapes(pass *analysis.Pass) (interface{}, error) { + return run(pass, false) +} + +// runAllEscapes runs with all escapes included. +func runAllEscapes(pass *analysis.Pass) (interface{}, error) { + return run(pass, true) +} + +// findReasons extracts reasons from the function. +func findReasons(pass *analysis.Pass, fdecl *ast.FuncDecl) ([]EscapeReason, bool, map[EscapeReason]bool) { + // Is there a comment? + if fdecl.Doc == nil { + return nil, false, nil + } + var ( + reasons []EscapeReason + local bool + testReasons = make(map[EscapeReason]bool) // reason -> local? + ) + // Scan all lines. + found := false + for _, c := range fdecl.Doc.List { + // Does the comment contain a +checkescape line? + if !strings.HasPrefix(c.Text, magic) && !strings.HasPrefix(c.Text, testMagic) { + continue + } + if c.Text == magic { + // Default: hard reasons, local only. + reasons = hardReasons + local = true + } else if strings.HasPrefix(c.Text, magicParams) { + // Extract specific reasons. + types := strings.Split(c.Text[len(magicParams):], ",") + found = true // For below. + for i := 0; i < len(types); i++ { + if types[i] == "local" { + // Limit search to local escapes. + local = true + } else if types[i] == "all" { + // Append all reasons. + reasons = append(reasons, allReasons...) + } else if types[i] == "hard" { + // Append all hard reasons. + reasons = append(reasons, hardReasons...) + } else { + r, ok := escapeTypes[types[i]] + if !ok { + // This is not a valid escape reason. + pass.Reportf(fdecl.Pos(), "unknown reason: %v", types[i]) + continue + } + reasons = append(reasons, r) + } + } + } else if strings.HasPrefix(c.Text, testMagic) { + types := strings.Split(c.Text[len(testMagic):], ",") + local := false + for i := 0; i < len(types); i++ { + if types[i] == "local" { + local = true + } else { + r, ok := escapeTypes[types[i]] + if !ok { + // This is not a valid escape reason. + pass.Reportf(fdecl.Pos(), "unknown reason: %v", types[i]) + continue + } + if v, ok := testReasons[r]; ok && v { + // Already registered as local. + continue + } + testReasons[r] = local + } + } + } + } + if len(reasons) == 0 && found { + // A magic annotation was provided, but no reasons. + pass.Reportf(fdecl.Pos(), "no reasons provided") + } + return reasons, local, testReasons +} + // run performs the analysis. -func run(pass *analysis.Pass) (interface{}, error) { +func run(pass *analysis.Pass, localEscapes bool) (interface{}, error) { calls, err := loadObjdump() if err != nil { return nil, err } - pef := packageEscapeFacts{ - Funcs: make(map[string][]Escape), - } + allEscapes := make(map[string][]Escapes) + mergedEscapes := make(map[string]Escapes) linePosition := func(inst, parent poser) LinePosition { p := pass.Fset.Position(inst.Pos()) if (p.Filename == "" || p.Line == 0) && parent != nil { p = pass.Fset.Position(parent.Pos()) } return LinePosition{ - Filename: filepath.Base(p.Filename), + Filename: p.Filename, Line: p.Line, } } - hasCall := func(inst poser) (string, bool) { - p := linePosition(inst, nil) - s, ok := calls[p] - return s, ok - } callSite := func(inst ssa.Instruction) CallSite { return CallSite{ LocalPos: inst.Pos(), Resolved: linePosition(inst, inst.Parent()), } } - escapes := func(reason EscapeReason, detail string, inst ssa.Instruction, ec *EscapeCount) []Escape { - if !ec.Record(reason) { - return nil // Skip. - } - es := Escape{ - Reason: reason, - Detail: detail, - Chain: []CallSite{callSite(inst)}, + hasCall := func(inst poser) (string, bool) { + p := linePosition(inst, nil) + s, ok := calls[p.Simplified()] + if !ok { + return "", false } - return []Escape{es} + // Join all calls together. + return strings.Join(s, " or "), true } - resolve := func(sub []Escape, inst ssa.Instruction, ec *EscapeCount) (es []Escape) { - for _, e := range sub { - if !ec.Record(e.Reason) { - continue // Skip. + state := pass.ResultOf[buildssa.Analyzer].(*buildssa.SSA) + + // Build the exception list. + exemptions := make(map[LinePosition]string) + for _, f := range pass.Files { + for _, cg := range f.Comments { + for _, c := range cg.List { + p := pass.Fset.Position(c.Slash) + if strings.HasPrefix(strings.ToLower(c.Text), exempt) { + exemptions[LinePosition{ + Filename: p.Filename, + Line: p.Line, + }] = c.Text[len(exempt):] + } } - es = append(es, Escape{ - Reason: e.Reason, - Detail: e.Detail, - Chain: append([]CallSite{callSite(inst)}, e.Chain...), - }) } - return es } - state := pass.ResultOf[buildssa.Analyzer].(*buildssa.SSA) - - var loadFunc func(*ssa.Function) []Escape // Used below. - analyzeInstruction := func(inst ssa.Instruction, ec *EscapeCount) []Escape { + var loadFunc func(*ssa.Function) Escapes // Used below. + analyzeInstruction := func(inst ssa.Instruction) (es Escapes) { + cs := callSite(inst) + if _, ok := exemptions[cs.Resolved]; ok { + return // No escape. + } switch x := inst.(type) { case *ssa.Call: if x.Call.IsInvoke() { @@ -407,13 +646,15 @@ func run(pass *analysis.Pass) (interface{}, error) { // not, since we don't know the underlying // type. call, _ := hasCall(inst) - return escapes(interfaceInvoke, call, inst, ec) + es.Add(interfaceInvoke, call, cs) + return } switch x := x.Call.Value.(type) { case *ssa.Function: if x.Pkg == nil { // Can't resolve the package. - return escapes(unknownPackage, "no package", inst, ec) + es.Add(unknownPackage, "no package", cs) + return } // Is this a local function? If yes, call the @@ -421,7 +662,8 @@ func run(pass *analysis.Pass) (interface{}, error) { // local escapes are the escapes found in the // local function. if x.Pkg.Pkg == pass.Pkg { - return resolve(loadFunc(x), inst, ec) + es.MergeWithCall(loadFunc(x), cs) + return } // Recursively collect information from @@ -430,22 +672,26 @@ func run(pass *analysis.Pass) (interface{}, error) { if !pass.ImportPackageFact(x.Pkg.Pkg, &imp) { // Unable to import the dependency; we must // declare these as escaping. - return escapes(unknownPackage, "no analysis", inst, ec) + es.Add(unknownPackage, "no analysis", cs) + return } // The escapes of this instruction are the // escapes of the called function directly. - return resolve(imp.Funcs[x.RelString(x.Pkg.Pkg)], inst, ec) + // Note that this may record many escapes. + es.MergeWithCall(imp.Funcs[x.RelString(x.Pkg.Pkg)], cs) + return case *ssa.Builtin: // Ignore elided escapes. if _, has := hasCall(inst); !has { - return nil + return } // Check if the builtin is escaping. for _, name := range escapingBuiltins { if x.Name() == name { - return escapes(builtin, name, inst, ec) + es.Add(builtin, name, cs) + return } } default: @@ -454,82 +700,87 @@ func run(pass *analysis.Pass) (interface{}, error) { // dispatches. We cannot actually look up what // this refers to using static analysis alone. call, _ := hasCall(inst) - return escapes(dynamicCall, call, inst, ec) + es.Add(dynamicCall, call, cs) } case *ssa.Alloc: // Ignore non-heap allocations. if !x.Heap { - return nil + return } // Ignore elided escapes. call, has := hasCall(inst) if !has { - return nil + return } // This is a real heap allocation. - return escapes(allocation, call, inst, ec) + es.Add(allocation, call, cs) case *ssa.MakeMap: - return escapes(builtin, "makemap", inst, ec) + es.Add(builtin, "makemap", cs) case *ssa.MakeSlice: - return escapes(builtin, "makeslice", inst, ec) + es.Add(builtin, "makeslice", cs) case *ssa.MakeClosure: - return escapes(builtin, "makeclosure", inst, ec) + es.Add(builtin, "makeclosure", cs) case *ssa.MakeChan: - return escapes(builtin, "makechan", inst, ec) + es.Add(builtin, "makechan", cs) } - return nil // No escapes. + return } - var analyzeBasicBlock func(*ssa.BasicBlock, *EscapeCount) []Escape // Recursive. - analyzeBasicBlock = func(block *ssa.BasicBlock, ec *EscapeCount) (rval []Escape) { + var analyzeBasicBlock func(*ssa.BasicBlock) []Escapes // Recursive. + analyzeBasicBlock = func(block *ssa.BasicBlock) (rval []Escapes) { for _, inst := range block.Instrs { - rval = append(rval, analyzeInstruction(inst, ec)...) + if es := analyzeInstruction(inst); !es.IsEmpty() { + rval = append(rval, es) + } } - return rval // N.B. may be empty. + return } - loadFunc = func(fn *ssa.Function) []Escape { + loadFunc = func(fn *ssa.Function) Escapes { // Is this already available? name := fn.RelString(pass.Pkg) - if es, ok := pef.Funcs[name]; ok { + if es, ok := mergedEscapes[name]; ok { return es } // In the case of a true cycle, we assume that the current - // function itself has no escapes until the rest of the - // analysis is complete. This will trip the above in the case - // of a cycle of any kind. - pef.Funcs[name] = nil + // function itself has no escapes. + // + // When evaluating the function again, the proper escapes will + // be filled in here. + allEscapes[name] = nil + mergedEscapes[name] = Escapes{} // Perform the basic analysis. - var ( - es []Escape - ec EscapeCount - ) + var es []Escapes if fn.Recover != nil { - es = append(es, analyzeBasicBlock(fn.Recover, &ec)...) + es = append(es, analyzeBasicBlock(fn.Recover)...) } for _, block := range fn.Blocks { - es = append(es, analyzeBasicBlock(block, &ec)...) + es = append(es, analyzeBasicBlock(block)...) } // Check for a stack split. if call, has := hasCall(fn); has { - es = append(es, Escape{ - Reason: stackSplit, - Detail: call, - Chain: []CallSite{CallSite{ - LocalPos: fn.Pos(), - Resolved: linePosition(fn, fn.Parent()), - }}, + var ss Escapes + ss.Add(stackSplit, call, CallSite{ + LocalPos: fn.Pos(), + Resolved: linePosition(fn, fn.Parent()), }) + es = append(es, ss) } // Save the result and return. - pef.Funcs[name] = es - return es + // + // Note that we merge the result when saving to the facts. It + // doesn't really matter the specific escapes, as long as we + // have recorded all the appropriate classes of escapes. + summary := MergeAll(es) + allEscapes[name] = es + mergedEscapes[name] = summary + return summary } // Complete all local functions. @@ -537,173 +788,76 @@ func run(pass *analysis.Pass) (interface{}, error) { loadFunc(fn) } - // Build the exception list. - exemptions := make(map[LinePosition]string) - for _, f := range pass.Files { - for _, cg := range f.Comments { - for _, c := range cg.List { - p := pass.Fset.Position(c.Slash) - if strings.HasPrefix(strings.ToLower(c.Text), exempt) { - exemptions[LinePosition{ - Filename: filepath.Base(p.Filename), - Line: p.Line, - }] = c.Text[len(exempt):] - } - } - } + if !localEscapes { + // Export all findings for future packages. We only do this in + // non-local escapes mode, and expect to run this analysis + // after the SelectAnalysis. + pass.ExportPackageFact(&packageEscapeFacts{ + Funcs: mergedEscapes, + }) } - // Delete everything matching the excemtions. - // - // This has the implication that exceptions are applied recursively, - // since this now modified set is what will be saved. - for name, escapes := range pef.Funcs { - var newEscapes []Escape - for _, escape := range escapes { - isExempt := false - for line, _ := range exemptions { - // Note that an exemption applies if it is - // marked as an exemption anywhere in the call - // chain. It need not be marked as escapes in - // the function itself, nor in the top-level - // caller. - for _, callSite := range escape.Chain { - if callSite.Resolved == line { - isExempt = true - break - } - } - if isExempt { - break - } - } - if !isExempt { - // Record this escape; not an exception. - newEscapes = append(newEscapes, escape) - } - } - pef.Funcs[name] = newEscapes // Update. - } - - // Export all findings for future packages. - pass.ExportPackageFact(&pef) - // Scan all functions for violations. for _, f := range pass.Files { // Scan all declarations. for _, decl := range f.Decls { - fdecl, ok := decl.(*ast.FuncDecl) // Function declaration? + fdecl, ok := decl.(*ast.FuncDecl) if !ok { continue } - // Is there a comment? - if fdecl.Doc == nil { - continue - } var ( reasons []EscapeReason - found bool local bool - testReasons = make(map[EscapeReason]bool) // reason -> local? + testReasons map[EscapeReason]bool ) - // Does the comment contain a +checkescape line? - for _, c := range fdecl.Doc.List { - if !strings.HasPrefix(c.Text, magic) && !strings.HasPrefix(c.Text, testMagic) { - continue - } - if c.Text == magic { - // Default: hard reasons, local only. - reasons = hardReasons - local = true - } else if strings.HasPrefix(c.Text, magicParams) { - // Extract specific reasons. - types := strings.Split(c.Text[len(magicParams):], ",") - found = true // For below. - for i := 0; i < len(types); i++ { - if types[i] == "local" { - // Limit search to local escapes. - local = true - } else if types[i] == "all" { - // Append all reasons. - reasons = append(reasons, allReasons...) - } else if types[i] == "hard" { - // Append all hard reasons. - reasons = append(reasons, hardReasons...) - } else { - r, ok := escapeTypes[types[i]] - if !ok { - // This is not a valid escape reason. - pass.Reportf(fdecl.Pos(), "unknown reason: %v", types[i]) - continue - } - reasons = append(reasons, r) - } - } - } else if strings.HasPrefix(c.Text, testMagic) { - types := strings.Split(c.Text[len(testMagic):], ",") - local := false - for i := 0; i < len(types); i++ { - if types[i] == "local" { - local = true - } else { - r, ok := escapeTypes[types[i]] - if !ok { - // This is not a valid escape reason. - pass.Reportf(fdecl.Pos(), "unknown reason: %v", types[i]) - continue - } - if v, ok := testReasons[r]; ok && v { - // Already registered as local. - continue - } - testReasons[r] = local - } - } - } - } - if len(reasons) == 0 && found { - // A magic annotation was provided, but no reasons. - pass.Reportf(fdecl.Pos(), "no reasons provided") - continue + if localEscapes { + // Find all hard escapes. + reasons = hardReasons + } else { + // Find all declared reasons. + reasons, local, testReasons = findReasons(pass, fdecl) } // Scan for matches. fn := pass.TypesInfo.Defs[fdecl.Name].(*types.Func) - name := state.Pkg.Prog.FuncValue(fn).RelString(pass.Pkg) - es, ok := pef.Funcs[name] - if !ok { + fv := state.Pkg.Prog.FuncValue(fn) + if fv == nil { + continue + } + name := fv.RelString(pass.Pkg) + all, allOk := allEscapes[name] + merged, mergedOk := mergedEscapes[name] + if !allOk || !mergedOk { pass.Reportf(fdecl.Pos(), "internal error: function %s not found.", name) continue } - for _, e := range es { - for _, r := range reasons { - // Is does meet our local requirement? - if local && len(e.Chain) > 1 { - continue - } - // Does this match the reason? Emit - // with a full stack trace that - // explains why this violates our - // constraints. - if e.Reason == r { - pass.Reportf(e.Chain[0].LocalPos, "%s", e.String()) - } - } + + // Filter reasons and report. + // + // For the findings, we use all escapes. + for _, es := range all { + es.Filter(reasons, local) + es.Reportf(pass) } // Scan for test (required) matches. + // + // For tests we need only the merged escapes. testReasonsFound := make(map[EscapeReason]bool) - for _, e := range es { + for r := EscapeReason(0); r < reasonCount; r++ { + if merged.CallSites[r] == nil { + continue + } // Is this local? - local, ok := testReasons[e.Reason] - wantLocal := len(e.Chain) == 1 - testReasonsFound[e.Reason] = wantLocal + wantLocal, ok := testReasons[r] + isLocal := len(merged.CallSites[r]) == 1 + testReasonsFound[r] = isLocal if !ok { continue } - if local == wantLocal { - delete(testReasons, e.Reason) + if isLocal == wantLocal { + delete(testReasons, r) } } for reason, local := range testReasons { @@ -711,10 +865,8 @@ func run(pass *analysis.Pass) (interface{}, error) { pass.Reportf(fdecl.Pos(), fmt.Sprintf("testescapes not found: reason=%s, local=%t", reason, local)) } if len(testReasons) > 0 { - // Dump all reasons found to help in debugging. - for _, e := range es { - pass.Reportf(e.Chain[0].LocalPos, "escape found: %s", e.String()) - } + // Report for debugging. + merged.Reportf(pass) } } } diff --git a/tools/checkescape/test1/test1.go b/tools/checkescape/test1/test1.go index a1d36459f..27991649f 100644 --- a/tools/checkescape/test1/test1.go +++ b/tools/checkescape/test1/test1.go @@ -175,6 +175,7 @@ func Split() { // +mustescape:stack //go:noinline +//go:nosplit func splitRec() { Split() } diff --git a/tools/checkescape/test2/test2.go b/tools/checkescape/test2/test2.go index 2d5865f47..067d5a1f4 100644 --- a/tools/checkescape/test2/test2.go +++ b/tools/checkescape/test2/test2.go @@ -83,6 +83,7 @@ func dynamicCrossPkg(f func()) { // +mustescape:stack //go:noinline +//go:nosplit func splitCrosssPkt() { test1.Split() } diff --git a/tools/go_marshal/test/escape/escape.go b/tools/go_marshal/test/escape/escape.go index 6a46ddbf8..3a1a64e9c 100644 --- a/tools/go_marshal/test/escape/escape.go +++ b/tools/go_marshal/test/escape/escape.go @@ -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) diff --git a/tools/nogo/BUILD b/tools/nogo/BUILD index fb35c5ffd..9f1fcd9c7 100644 --- a/tools/nogo/BUILD +++ b/tools/nogo/BUILD @@ -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", diff --git a/tools/nogo/config.go b/tools/nogo/config.go index 451cd4a4c..cfe7b4aa4 100644 --- a/tools/nogo/config.go +++ b/tools/nogo/config.go @@ -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(), +} diff --git a/tools/nogo/defs.bzl b/tools/nogo/defs.bzl index 963084d53..480438047 100644 --- a/tools/nogo/defs.bzl +++ b/tools/nogo/defs.bzl @@ -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) + + # 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, + )) - # Draw a sweet unicode checkmark with the package name (in green). + # 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) diff --git a/tools/nogo/dump/BUILD b/tools/nogo/dump/BUILD deleted file mode 100644 index dfa29d651..000000000 --- a/tools/nogo/dump/BUILD +++ /dev/null @@ -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__"], -) diff --git a/tools/nogo/dump/dump.go b/tools/nogo/dump/dump.go deleted file mode 100644 index f06567e0f..000000000 --- a/tools/nogo/dump/dump.go +++ /dev/null @@ -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 -} diff --git a/tools/nogo/nogo.go b/tools/nogo/nogo.go index e44f32d4c..40e48540d 100644 --- a/tools/nogo/nogo.go +++ b/tools/nogo/nogo.go @@ -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) } diff --git a/tools/nogo/register.go b/tools/nogo/register.go index 62b499661..34b173937 100644 --- a/tools/nogo/register.go +++ b/tools/nogo/register.go @@ -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 } |