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 <zpavlinovic@google.com> Reviewed-by: Cherry Mui <cherryyz@google.com> TryBot-Result: Gopher Robot <gobot@golang.org>
diff --git a/cmd/govulncheck/main_command_118_test.go b/cmd/govulncheck/main_command_118_test.go index 7f056c4..69bd755 100644 --- a/cmd/govulncheck/main_command_118_test.go +++ b/cmd/govulncheck/main_command_118_test.go
@@ -142,8 +142,7 @@ } 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) } }
diff --git a/internal/vulncheck/internal/buildinfo/additions_buildinfo.go b/internal/vulncheck/internal/buildinfo/additions_buildinfo.go index 1bdd38c..642920b 100644 --- a/internal/vulncheck/internal/buildinfo/additions_buildinfo.go +++ b/internal/vulncheck/internal/buildinfo/additions_buildinfo.go
@@ -192,7 +192,10 @@ // 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 @@ 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 {
diff --git a/internal/vulncheck/internal/buildinfo/additions_scan_test.go b/internal/vulncheck/internal/buildinfo/additions_scan_test.go index 0948d48..7e238ce 100644 --- a/internal/vulncheck/internal/buildinfo/additions_scan_test.go +++ b/internal/vulncheck/internal/buildinfo/additions_scan_test.go
@@ -63,56 +63,3 @@ } }) } - -// 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) - } - }) -}
diff --git a/internal/vulncheck/internal/buildinfo/additions_stripped_122_test.go b/internal/vulncheck/internal/buildinfo/additions_stripped_122_test.go new file mode 100644 index 0000000..9862037 --- /dev/null +++ b/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)) + } + }) +}
diff --git a/internal/vulncheck/internal/buildinfo/additions_stripped_test.go b/internal/vulncheck/internal/buildinfo/additions_stripped_test.go new file mode 100644 index 0000000..ce10de6 --- /dev/null +++ b/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) + } + }) +}
diff --git a/internal/vulncheck/internal/buildinfo/buildinfo.go b/internal/vulncheck/internal/buildinfo/buildinfo.go index 1be795e..f29dffa 100644 --- a/internal/vulncheck/internal/buildinfo/buildinfo.go +++ b/internal/vulncheck/internal/buildinfo/buildinfo.go
@@ -174,6 +174,7 @@ 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) {