internal/vulncheck/internal/buildinfo: support stripped darwin binaries

With go1.22 and its prereleases, binaries are also stripped on darwin.
This cannot be observed by checking emptiness of the symbol table, yet
by non-existence of program symbols, "runtime.main" being a symbol that
every program should have.

Fixes golang/go#61051

Change-Id: If39214df9531bee66931a4155a2a8fbfbf3823cb
Reviewed-on: https://go-review.googlesource.com/c/vuln/+/522157
Run-TryBot: Zvonimir Pavlinovic <[email protected]>
Reviewed-by: Cherry Mui <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>

cmd/govulncheck/main_command_118_test.go

@@ -142,8 +142,7 @@ func TestCommand(t *testing.T) {
 	}
 	runTestSuite(t, filepath.Join(testDir, "testdata"), govulndbURI.String(), *update)
 	if runtime.GOOS != "darwin" {
-		// TODO(https://go.dev/issue/61051): binaries are not currently stripped on darwin.
-		// This is expected to change in Go 1.22.
+		// Binaries are not stripped on darwin with go1.21 and earlier. See #61051.
 		runTestSuite(t, filepath.Join(testDir, "testdata/strip"), govulndbURI.String(), *update)
 	}
 }

internal/vulncheck/internal/buildinfo/additions_buildinfo.go

@@ -192,7 +192,10 @@ func (x *peExe) PCLNTab() ([]byte, uint64) {
 
 // SymbolInfo is derived from cmd/internal/objfile/macho.go:symbols.
 func (x *machoExe) SymbolInfo(name string) (uint64, uint64, io.ReaderAt, error) {
-	sym := x.lookupSymbol(name)
+	sym, err := x.lookupSymbol(name)
+	if err != nil {
+		return 0, 0, nil, err
+	}
 	if sym == nil {
 		return 0, 0, nil, fmt.Errorf("no symbol %q", name)
 	}
@@ -203,15 +206,26 @@ func (x *machoExe) SymbolInfo(name string) (uint64, uint64, io.ReaderAt, error)
 	return sym.Value, seg.Addr, seg.ReaderAt, nil
 }
 
-func (x *machoExe) lookupSymbol(name string) *macho.Symbol {
+func (x *machoExe) lookupSymbol(name string) (*macho.Symbol, error) {
+	const mustExistSymbol = "runtime.main"
 	x.symbolsOnce.Do(func() {
 		x.symbols = make(map[string]*macho.Symbol, len(x.f.Symtab.Syms))
 		for _, s := range x.f.Symtab.Syms {
 			s := s // make a copy to prevent aliasing
 			x.symbols[s.Name] = &s
 		}
+		// In the presence of stripping, the symbol table for darwin
+		// binaries will not be empty, but the program symbols will
+		// be missing.
+		if _, ok := x.symbols[mustExistSymbol]; !ok {
+			x.symbolsErr = ErrNoSymbols
+		}
 	})
-	return x.symbols[name]
+
+	if x.symbolsErr != nil {
+		return nil, x.symbolsErr
+	}
+	return x.symbols[name], nil
 }
 
 func (x *machoExe) segmentContaining(addr uint64) *macho.Segment {

internal/vulncheck/internal/buildinfo/additions_scan_test.go

@@ -63,56 +63,3 @@ func TestExtractPackagesAndSymbols(t *testing.T) {
 			}
 		})
 }
-
-// TestStrippedBinary checks that there is no symbol table for
-// stripped binaries. This does not include darwin binaries.
-// For more info, see #61051.
-func TestStrippedBinary(t *testing.T) {
-	// We exclude darwin as its stripped binaries seem to
-	// preserve the symbol table. See TestStrippedDarwin.
-	testAll(t, []string{"linux", "windows", "freebsd"}, []string{"amd64", "386", "arm", "arm64"},
-		func(t *testing.T, goos, goarch string) {
-			binary, done := test.GoBuild(t, "testdata", "", true, "GOOS", goos, "GOARCH", goarch)
-			defer done()
-
-			f, err := os.Open(binary)
-			if err != nil {
-				t.Fatal(err)
-			}
-			defer f.Close()
-			_, syms, _, err := ExtractPackagesAndSymbols(f)
-			if err != nil {
-				t.Fatal(err)
-			}
-			if syms != nil {
-				t.Errorf("want empty symbol table; got %v symbols", len(syms))
-			}
-		})
-}
-
-// TestStrippedDarwin checks that the symbol table exists and
-// is complete on darwin even in the presence of stripping.
-// This test will become obsolete once #61051 is addressed.
-func TestStrippedDarwin(t *testing.T) {
-	t.Skip("to temporarily resolve #61511")
-	testAll(t, []string{"darwin"}, []string{"amd64", "386", "arm", "arm64"},
-		func(t *testing.T, goos, goarch string) {
-			binary, done := test.GoBuild(t, "testdata", "", true, "GOOS", goos, "GOARCH", goarch)
-			defer done()
-
-			f, err := os.Open(binary)
-			if err != nil {
-				t.Fatal(err)
-			}
-			defer f.Close()
-			_, syms, _, err := ExtractPackagesAndSymbols(f)
-			if err != nil {
-				t.Fatal(err)
-			}
-			got := syms["main"]
-			want := []string{"f", "g", "main"}
-			if !cmp.Equal(got, want) {
-				t.Errorf("\ngot  %q\nwant %q", got, want)
-			}
-		})
-}

internal/vulncheck/internal/buildinfo/additions_stripped_122_test.go

@@ -0,0 +1,38 @@
+// Copyright 2023 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+//go:build go1.22
+// +build go1.22
+
+package buildinfo
+
+import (
+	"os"
+	"testing"
+
+	"golang.org/x/vuln/internal/test"
+)
+
+// TestStrippedBinary checks that there is no symbol table for
+// stripped binaries.
+func TestStrippedBinary(t *testing.T) {
+	testAll(t, []string{"linux", "windows", "freebsd", "darwin"}, []string{"amd64", "386", "arm", "arm64"},
+		func(t *testing.T, goos, goarch string) {
+			binary, done := test.GoBuild(t, "testdata", "", true, "GOOS", goos, "GOARCH", goarch)
+			defer done()
+
+			f, err := os.Open(binary)
+			if err != nil {
+				t.Fatal(err)
+			}
+			defer f.Close()
+			_, syms, _, err := ExtractPackagesAndSymbols(f)
+			if err != nil {
+				t.Fatal(err)
+			}
+			if syms != nil {
+				t.Errorf("want empty symbol table; got %v symbols", len(syms))
+			}
+		})
+}

internal/vulncheck/internal/buildinfo/additions_stripped_test.go

@@ -0,0 +1,68 @@
+// Copyright 2023 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+//go:build go1.18 && !go1.22
+// +build go1.18,!go1.22
+
+package buildinfo
+
+import (
+	"os"
+	"testing"
+
+	"github.com/google/go-cmp/cmp"
+	"golang.org/x/vuln/internal/test"
+)
+
+// TestStrippedBinary checks that there is no symbol table for
+// stripped binaries. This does not include darwin binaries.
+// For more info, see #61051.
+func TestStrippedBinary(t *testing.T) {
+	// We exclude darwin as its stripped binaries seem to
+	// preserve the symbol table. See TestStrippedDarwin.
+	testAll(t, []string{"linux", "windows", "freebsd"}, []string{"amd64", "386", "arm", "arm64"},
+		func(t *testing.T, goos, goarch string) {
+			binary, done := test.GoBuild(t, "testdata", "", true, "GOOS", goos, "GOARCH", goarch)
+			defer done()
+
+			f, err := os.Open(binary)
+			if err != nil {
+				t.Fatal(err)
+			}
+			defer f.Close()
+			_, syms, _, err := ExtractPackagesAndSymbols(f)
+			if err != nil {
+				t.Fatal(err)
+			}
+			if syms != nil {
+				t.Errorf("want empty symbol table; got %v symbols", len(syms))
+			}
+		})
+}
+
+// TestStrippedDarwin checks that the symbol table exists and
+// is complete on darwin even in the presence of stripping.
+// For more info, see #61051.
+func TestStrippedDarwin(t *testing.T) {
+	testAll(t, []string{"darwin"}, []string{"amd64", "386"},
+		func(t *testing.T, goos, goarch string) {
+			binary, done := test.GoBuild(t, "testdata", "", true, "GOOS", goos, "GOARCH", goarch)
+			defer done()
+
+			f, err := os.Open(binary)
+			if err != nil {
+				t.Fatal(err)
+			}
+			defer f.Close()
+			_, syms, _, err := ExtractPackagesAndSymbols(f)
+			if err != nil {
+				t.Fatal(err)
+			}
+			got := syms["main"]
+			want := []string{"f", "g", "main"}
+			if !cmp.Equal(got, want) {
+				t.Errorf("\ngot  %q\nwant %q", got, want)
+			}
+		})
+}

internal/vulncheck/internal/buildinfo/buildinfo.go

@@ -174,6 +174,7 @@ type machoExe struct {
 
 	symbols     map[string]*macho.Symbol // Addition: symbols in the binary
 	symbolsOnce sync.Once                // Addition: for computing symbols
+	symbolsErr  error                    // Addition: error for computing symbols
 }
 
 func (x *machoExe) ReadData(addr, size uint64) ([]byte, error) {