Skip to content

Commit

Permalink
Fix invocation of assembler for go1.22
Browse files Browse the repository at this point in the history
In go1.19 through go1.22-devel (as of golang/go@6382893) a series of
changes were made to the way assembly files' symabis are produced.

https://go-review.googlesource.com/c/go/+/523337

Most significantly, the packagename now must be passed to the assembler
via the -p flag, even when generating only the symabis. The go build
system does this, but Bazel Go rules have not, and this finally breaks
in go1.22-devel as the compatibility code is removed.

Without specifying -p to the assembler, the output symabis file will
contain something like:

```
def <unlinkable>.s2Decode ABI0
```

instead of

```
def github.com/klauspost/compress/s2.s2Decode ABI0
```

The result is that the compiler will default to using ABIInternal
instead of ABI0 if it cannot resolve a match in symabis, which will
cause a link failure:

```
Use --sandbox_debug to see verbose messages from the sandbox and retain the sandbox build root for debugging
github.com/klauspost/compress/s2.Decode: relocation target github.com/klauspost/compress/s2.s2Decode not defined for ABIInternal (but is defined for ABI0)
link: error running subcommand external/go_sdk/pkg/tool/darwin_arm64/link: exit status 2
```

We conservatively only do this for go minor releases later than 1.21.
  • Loading branch information
jquirke committed Nov 20, 2023
1 parent c009a2b commit 8dc4747
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 2 deletions.
9 changes: 8 additions & 1 deletion go/tools/builders/asm.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ var ASM_DEFINES = []string{
// by the compiler. This is only needed in go1.12+ when there is at least one
// .s file. If the symabis file is not needed, no file will be generated,
// and "", nil will be returned.
func buildSymabisFile(goenv *env, sFiles, hFiles []fileInfo, asmhdr string) (string, error) {
func buildSymabisFile(goenv *env, packagePath string, sFiles, hFiles []fileInfo, asmhdr string) (string, error) {
if len(sFiles) == 0 {
return "", nil
}
Expand Down Expand Up @@ -94,6 +94,13 @@ func buildSymabisFile(goenv *env, sFiles, hFiles []fileInfo, asmhdr string) (str
seenHdrDirs[hdrDir] = true
}
}
// The package path has to be specified as of Go 1.22 or the resulting
// object will be unlinkable, but the -p flag is only required in
// preparing symabis since Go1.22, however, go build has been
// emitting -p for both symabi and actual assembly since at least Go1.19
if packagePath != "" && isGo119OrHigher() {
asmargs = append(asmargs, "-p", packagePath)
}
asmargs = append(asmargs, ASM_DEFINES...)
asmargs = append(asmargs, "-gensymabis", "-o", symabisName, "--")
for _, sFile := range sFiles {
Expand Down
2 changes: 1 addition & 1 deletion go/tools/builders/compilepkg.go
Original file line number Diff line number Diff line change
Expand Up @@ -471,7 +471,7 @@ func compileArchive(
}
var symabisPath string
if !haveCgo {
symabisPath, err = buildSymabisFile(goenv, srcs.sSrcs, srcs.hSrcs, asmHdrPath)
symabisPath, err = buildSymabisFile(goenv, packagePath, srcs.sSrcs, srcs.hSrcs, asmHdrPath)
if symabisPath != "" {
if !goenv.shouldPreserveWorkDir {
defer os.Remove(symabisPath)
Expand Down

0 comments on commit 8dc4747

Please sign in to comment.