Skip to content

Commit

Permalink
internal/vulncheck/internal/buildinfo: support stripped darwin binaries
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
zpavlinovic committed Sep 19, 2023
1 parent 3881ca8 commit 712bbae
Show file tree
Hide file tree
Showing 6 changed files with 125 additions and 58 deletions.
3 changes: 1 addition & 2 deletions cmd/govulncheck/main_command_118_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
Expand Down
20 changes: 17 additions & 3 deletions internal/vulncheck/internal/buildinfo/additions_buildinfo.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand All @@ -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 {
Expand Down
53 changes: 0 additions & 53 deletions internal/vulncheck/internal/buildinfo/additions_scan_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
})
}
Original file line number Diff line number Diff line change
@@ -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))
}
})
}
68 changes: 68 additions & 0 deletions internal/vulncheck/internal/buildinfo/additions_stripped_test.go
Original file line number Diff line number Diff line change
@@ -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)
}
})
}
1 change: 1 addition & 0 deletions internal/vulncheck/internal/buildinfo/buildinfo.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down

0 comments on commit 712bbae

Please sign in to comment.