Skip to content

Commit

Permalink
fix building for GOOS=darwin on Go 1.22.0
Browse files Browse the repository at this point in the history
It seems like building with Go 1.22.0 for GOOS=darwin started
running into some issues with the syscall package's use of ABIInternal
in assembly source code:

    > exec garble build
    [stderr]
    # syscall
    [...].s:16: ABI selector only permitted when compiling runtime, reference was to "runtime.entersyscall"

The error can be reproduced from another platform like GOOS=linux
as long as we have any test that cross-compiles std to GOOS=darwin.
We had crossbuild.txtar which only ensured we covered GOOS=windows
and GOOS=linux, so add a third case to ensure MacOS is covered too.

This will slow down the tests a bit, but is important for the sake
of ensuring that we catch these bugs early, even without MacOS on CI.
In fact, we hadn't caught this earlier for Go 1.22 precisely because
on CI we only tested on Go tip with GOOS=linux, for the sake of speed.

Adding the rest of the package import paths from objabi.allowAsmABIPkgs
to our runtimeAndDeps generated map solves this error.
  • Loading branch information
mvdan authored and lu4p committed Feb 8, 2024
1 parent 7a67952 commit 55921a0
Show file tree
Hide file tree
Showing 5 changed files with 24 additions and 9 deletions.
9 changes: 6 additions & 3 deletions go_std_tables.go

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

7 changes: 5 additions & 2 deletions scripts/gen-go-std-tables.sh
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,11 @@ var runtimeAndDeps = map[string]bool{
$(for path in ${runtime_and_deps}; do
echo "\"${path}\": true,"
done)
// Not a runtime dependency, but still uses tricks allowed by import path.
// Not a big deal either way, given that it's only imported in test packages.
// 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,
}
Expand Down
1 change: 1 addition & 0 deletions shared.go
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,7 @@ func (p *listedPackage) obfuscatedImportPath() string {
case "runtime", "reflect", "embed":
return p.ImportPath
}
// Intrinsics are matched by package import path as well.
if compilerIntrinsicsPkgs[p.ImportPath] {
return p.ImportPath
}
Expand Down
10 changes: 10 additions & 0 deletions testdata/script/crossbuild.txtar
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,16 @@
[arm] env GOARCH=arm64
exec garble build -gcflags=math/bits=-d=ssa/intrinsics/debug=1
stderr 'intrinsic substitution for Len64.*BitLen64'

# As a last step, also test building for MacOS if we're not already on it.
# We already cover Windows and Linux above, and MacOS is the other major OS.
# The way it is implemented in the standard library, in particular with syscalls,
# is different enough that it sometimes causes special bugs.
[darwin] stop
env GOOS=darwin
env GOARCH=arm64
exec garble build

-- go.mod --
module test/main

Expand Down
6 changes: 2 additions & 4 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' 'syscall.Listen'
! binsubstr out 'http.ListenAndServe' 'debug.WriteHeapDump' 'time.Now'

# 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' 'syscall.Listen'
binsubstr out_regular 'http.ListenAndServe' 'debug.WriteHeapDump' 'time.Now'

# 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,7 +79,6 @@ import (
"net/http"
"runtime/debug"
"time"
"syscall"
)

func main() {
Expand All @@ -88,5 +87,4 @@ func main() {
// as it is implemented by runtime via a linkname.
debug.WriteHeapDump(1)
time.Now()
syscall.Listen(0, 1)
}

0 comments on commit 55921a0

Please sign in to comment.