Mix checklocks and atomic analyzers.

This change makes the checklocks analyzer considerable more powerful, adding:
* The ability to traverse complex structures, e.g. to have multiple nested
  fields as part of the annotation.
* The ability to resolve simple anonymous functions and closures, and perform
  lock analysis across these invocations. This does not apply to closures that
  are passed elsewhere, since it is not possible to know the context in which
  they might be invoked.
* The ability to annotate return values in addition to receivers and other
  parameters, with the same complex structures noted above.
* Ignoring locking semantics for "fresh" objects, i.e. objects that are
  allocated in the local frame (typically a new-style function).
* Sanity checking of locking state across block transitions and returns, to
  ensure that no unexpected locks are held.

Note that initially, most of these findings are excluded by a comprehensive
nogo.yaml. The findings that are included are fundamental lock violations.
The changes here should be relatively low risk, minor refactorings to either
include necessary annotations to simplify the code structure (in general
removing closures in favor of methods) so that the analyzer can be easily
track the lock state.

This change additional includes two changes to nogo itself:
* Sanity checking of all types to ensure that the binary and ast-derived
  types have a consistent objectpath, to prevent the bug above from occurring
  silently (and causing much confusion). This also requires a trick in
  order to ensure that serialized facts are consumable downstream. This can
  be removed with https://go-review.googlesource.com/c/tools/+/331789 merged.
* A minor refactoring to isolation the objdump settings in its own package.
  This was originally used to implement the sanity check above, but this
  information is now being passed another way. The minor refactor is preserved
  however, since it cleans up the code slightly and is minimal risk.

PiperOrigin-RevId: 382613300

2 files changed, 69 insertions, 110 deletions

diff --git a/tools/checkescape/BUILD b/tools/checkescape/BUILD
index 940538b9e..109b5410c 100644
--- a/tools/checkescape/BUILD
+++ b/tools/checkescape/BUILD
@@ -8,6 +8,7 @@ go_library(
     nogo = False,
     visibility = ["//tools/nogo:__subpackages__"],
     deps = [
+        "//tools/nogo/objdump",
         "@org_golang_x_tools//go/analysis:go_default_library",
         "@org_golang_x_tools//go/analysis/passes/buildssa:go_default_library",
         "@org_golang_x_tools//go/ssa:go_default_library",
diff --git a/tools/checkescape/checkescape.go b/tools/checkescape/checkescape.go
index c788654a8..ddd1212d7 100644
--- a/tools/checkescape/checkescape.go
+++ b/tools/checkescape/checkescape.go
@@ -61,21 +61,19 @@ package checkescape
 import (
 	"bufio"
 	"bytes"
-	"flag"
 	"fmt"
 	"go/ast"
 	"go/token"
 	"go/types"
 	"io"
 	"log"
-	"os"
-	"os/exec"
 	"path/filepath"
 	"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/objdump"
 )
 
 const (
@@ -92,21 +90,6 @@ const (
 	exempt = "// escapes"
 )
 
-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
-
-	// objdumpTool is the tool used to dump a binary.
-	objdumpTool = flag.String("objdump_tool", "", "tool used to dump a binary")
-)
-
 // EscapeReason is an escape reason.
 //
 // This is a simple enum.
@@ -374,31 +357,6 @@ func MergeAll(others []Escapes) (es Escapes) {
 // 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(*objdumpTool, 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
-	}
-
 	// Identify calls by address or name. Note that this is also
 	// constructed dynamically below, as we encounted the addresses.
 	// This is because some of the functions (duffzero) may have
@@ -431,78 +389,83 @@ func loadObjdump() (map[string][]string, error) {
 	// Build the map.
 	nextFunc := "" // For funcsAllowed.
 	m := make(map[string][]string)
-	r := bufio.NewReader(out)
-NextLine:
-	for {
-		line, err := r.ReadString('\n')
-		if err != nil && err != io.EOF {
-			return nil, err
-		}
-		fields := strings.Fields(line)
-
-		// Is this an "allowed" function definition?
-		if len(fields) >= 2 && fields[0] == "TEXT" {
-			nextFunc = strings.TrimSuffix(fields[1], "(SB)")
-			if _, ok := funcsAllowed[nextFunc]; !ok {
-				nextFunc = "" // Don't record addresses.
-			}
-		}
-		if nextFunc != "" && len(fields) > 2 {
-			// Save the given address (in hex form, as it appears).
-			addrsAllowed[fields[1]] = struct{}{}
-		}
-
-		// We recognize lines corresponding to actual code (not the
-		// symbol name or other metadata) and annotate them if they
-		// correspond to an explicit CALL instruction. We assume that
-		// the lack of a CALL for a given line is evidence that escape
-		// analysis has eliminated an allocation.
-		//
-		// Lines look like this (including the first space):
-		//  gohacks_unsafe.go:33  0xa39                   488b442408              MOVQ 0x8(SP), AX
-		if len(fields) >= 5 && line[0] == ' ' {
-			if !strings.Contains(fields[3], "CALL") {
-				continue
+	if err := objdump.Load(func(origR io.Reader) error {
+		r := bufio.NewReader(origR)
+	NextLine:
+		for {
+			line, err := r.ReadString('\n')
+			if err != nil && err != io.EOF {
+				return err
 			}
-			site := fields[0]
-			target := strings.TrimSuffix(fields[4], "(SB)")
+			fields := strings.Fields(line)
 
-			// Ignore strings containing allowed functions.
-			if _, ok := funcsAllowed[target]; ok {
-				continue
+			// Is this an "allowed" function definition?
+			if len(fields) >= 2 && fields[0] == "TEXT" {
+				nextFunc = strings.TrimSuffix(fields[1], "(SB)")
+				if _, ok := funcsAllowed[nextFunc]; !ok {
+					nextFunc = "" // Don't record addresses.
+				}
 			}
-			if _, ok := addrsAllowed[target]; ok {
-				continue
+			if nextFunc != "" && len(fields) > 2 {
+				// Save the given address (in hex form, as it appears).
+				addrsAllowed[fields[1]] = struct{}{}
 			}
-			if len(fields) > 5 {
-				// This may be a future relocation. Some
-				// objdump versions describe this differently.
-				// If it contains any of the functions allowed
-				// above as a string, we let it go.
-				softTarget := strings.Join(fields[5:], " ")
-				for name := range funcsAllowed {
-					if strings.Contains(softTarget, name) {
-						continue NextLine
+
+			// We recognize lines corresponding to actual code (not the
+			// symbol name or other metadata) and annotate them if they
+			// correspond to an explicit CALL instruction. We assume that
+			// the lack of a CALL for a given line is evidence that escape
+			// analysis has eliminated an allocation.
+			//
+			// Lines look like this (including the first space):
+			//  gohacks_unsafe.go:33  0xa39                   488b442408              MOVQ 0x8(SP), AX
+			if len(fields) >= 5 && line[0] == ' ' {
+				if !strings.Contains(fields[3], "CALL") {
+					continue
+				}
+				site := fields[0]
+				target := strings.TrimSuffix(fields[4], "(SB)")
+
+				// Ignore strings containing allowed functions.
+				if _, ok := funcsAllowed[target]; ok {
+					continue
+				}
+				if _, ok := addrsAllowed[target]; ok {
+					continue
+				}
+				if len(fields) > 5 {
+					// This may be a future relocation. Some
+					// objdump versions describe this differently.
+					// If it contains any of the functions allowed
+					// above as a string, we let it go.
+					softTarget := strings.Join(fields[5:], " ")
+					for name := range funcsAllowed {
+						if strings.Contains(softTarget, name) {
+							continue NextLine
+						}
 					}
 				}
-			}
 
-			// Does this exist already?
-			existing, ok := m[site]
-			if !ok {
-				existing = make([]string, 0, 1)
-			}
-			for _, other := range existing {
-				if target == other {
-					continue NextLine
+				// Does this exist already?
+				existing, ok := m[site]
+				if !ok {
+					existing = make([]string, 0, 1)
+				}
+				for _, other := range existing {
+					if target == other {
+						continue NextLine
+					}
 				}
+				existing = append(existing, target)
+				m[site] = existing // Update.
+			}
+			if err == io.EOF {
+				break
 			}
-			existing = append(existing, target)
-			m[site] = existing // Update.
-		}
-		if err == io.EOF {
-			break
 		}
+		return nil
+	}); err != nil {
+		return nil, err
 	}
 
 	// Zap any accidental false positives.
@@ -518,11 +481,6 @@ NextLine:
 		final[site] = filteredCalls
 	}
 
-	// Wait for the dump to finish.
-	if err := cmd.Wait(); err != nil {
-		return nil, err
-	}
-
 	return final, nil
 }