Skip to content

Commit

Permalink
obfuscate syscall again to fix x/sys/unix
Browse files Browse the repository at this point in the history
When updating Garble to support Go 1.22.0, CI on MacOS spotted
that the syscall package was failing to build given that it uses
assembly code which is only allowed in some std packages.

That allowlist is based on import paths, and we were obfuscating
the syscall package's import path, so that was breaking GOOS=darwin.
As a fix, I added syscall to runtimeAndDeps to not obfuscate it.

That wasn't a great fix; it's not part of runtime and its dependencies,
and there's no reason we should avoid obfuscating the package contents.
Not obfuscating the contents in fact broke x/sys/unix,
as it contains a copy of syscall.Rlimit which it type converted with.

Undo that fix and reinstate the gogarble.txtar syscall test.
Implement the fix where we only leave syscall's import path alone.
Add a regression test, and add a note about adding x/net and x/sys
to check-third-party.sh so that we can catch these bugs earlier.

Fixes burrowers#830.
  • Loading branch information
mvdan authored and pagran committed Feb 18, 2024
1 parent 69bc62c commit 66b6140
Show file tree
Hide file tree
Showing 6 changed files with 27 additions and 15 deletions.
6 changes: 0 additions & 6 deletions go_std_tables.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions scripts/check-third-party.sh
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,9 @@ modules=(
github.com/andybalholm/brotli v1.0.4

# TODO: consider github.com/mattn/go-sqlite3 to cover a DB and more cgo

# TODO: add x/net and x/unix as they include hacks like go:linkname
# or copying syscall types.
)

SCRIPT_DIR=$(cd -- "$(dirname -- "${BASH_SOURCE[0]}")" &>/dev/null && pwd)
Expand Down
6 changes: 0 additions & 6 deletions scripts/gen-go-std-tables.sh
Original file line number Diff line number Diff line change
Expand Up @@ -34,12 +34,6 @@ var runtimeAndDeps = map[string]bool{
$(for path in ${runtime_and_deps}; do
echo "\"${path}\": true,"
done)
// Not runtime dependencies, but still use tricks allowed by import path.
// TODO: collect directly from cmd/internal/objabi/pkgspecial.go,
// in this particular case from allowAsmABIPkgs.
"reflect": true,
"syscall": true,
"runtime/internal/startlinetest": true,
}
var runtimeLinknamed = []string{
Expand Down
6 changes: 5 additions & 1 deletion shared.go
Original file line number Diff line number Diff line change
Expand Up @@ -183,8 +183,12 @@ func (p *listedPackage) obfuscatedImportPath() string {
// * runtime: it is special in many ways
// * reflect: its presence turns down dead code elimination
// * embed: its presence enables using //go:embed
// * others like syscall are allowed by import path to have more ABI tricks
//
// TODO: collect directly from cmd/internal/objabi/pkgspecial.go,
// in this particular case from allowAsmABIPkgs.
switch p.ImportPath {
case "runtime", "reflect", "embed":
case "runtime", "reflect", "embed", "syscall", "runtime/internal/startlinetest":
return p.ImportPath
}
// Intrinsics are matched by package import path as well.
Expand Down
6 changes: 4 additions & 2 deletions testdata/script/gogarble.txtar
Original file line number Diff line number Diff line change
Expand Up @@ -36,11 +36,11 @@ exec garble build std
# Also ensure we are obfuscating low-level std packages.
exec garble build -o=out ./stdimporter
! stderr . # no warnings
! binsubstr out 'http.ListenAndServe' 'debug.WriteHeapDump' 'time.Now'
! binsubstr out 'http.ListenAndServe' 'debug.WriteHeapDump' 'time.Now' 'syscall.Listen'

# The same low-level std packages appear in plain sight in regular builds.
go build -o=out_regular ./stdimporter
binsubstr out_regular 'http.ListenAndServe' 'debug.WriteHeapDump' 'time.Now'
binsubstr out_regular 'http.ListenAndServe' 'debug.WriteHeapDump' 'time.Now' 'syscall.Listen'

# Also check that a full rebuild is reproducible, via a new GOCACHE.
# This is slow, but necessary to uncover bugs hidden by the build cache.
Expand Down Expand Up @@ -79,6 +79,7 @@ import (
"net/http"
"runtime/debug"
"time"
"syscall"
)

func main() {
Expand All @@ -87,4 +88,5 @@ func main() {
// as it is implemented by runtime via a linkname.
debug.WriteHeapDump(1)
time.Now()
syscall.Listen(0, 1)
}
15 changes: 15 additions & 0 deletions testdata/script/implement.txtar
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,21 @@ func main() {
fmt.Println(T("foo"))
fmt.Println(T("foo").unexportedMethod())
}

-- main_linux.go --
package main

import "syscall"

// golang.org/x/sys/unix has a copy of syscall.Rlimit and converts with it.
// Note that syscall.Rlimit is only declared in some unix GOOSes.
type Rlimit2 struct {
Cur uint64
Max uint64
}

var _ = (*syscall.Rlimit)(new(Rlimit2))

-- lib1/lib1.go --
package lib1

Expand Down

0 comments on commit 66b6140

Please sign in to comment.