Skip to content

Commit

Permalink
Fix breaking change with assembler in go1.22
Browse files Browse the repository at this point in the history
In go1.22, a breaking change to the build system is introduced.
https://go-review.googlesource.com/c/go/+/523337

Namely, that the packagename now *must* passed to the assembler, even
when generating only the symabis. The go build system has always done
this, but Bazel Go rules have not, and this finally breaks in go1.22.

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 ff65758
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 ff65758

Please sign in to comment.