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 b9f97ba
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 6 deletions.
16 changes: 11 additions & 5 deletions 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,12 @@ 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
if packagePath != "" && isGoMinorVersionEqualHigher(22) {
asmargs = append(asmargs, "-p", packagePath)
}
asmargs = append(asmargs, ASM_DEFINES...)
asmargs = append(asmargs, "-gensymabis", "-o", symabisName, "--")
for _, sFile := range sFiles {
Expand All @@ -110,7 +116,7 @@ func asmFile(goenv *env, srcPath, packagePath string, asmFlags []string, outPath
// The package path has to be specified as of Go 1.19 or the resulting
// object will be unlinkable, but the -p flag is also only available
// since Go 1.19.
if packagePath != "" && isGo119OrHigher() {
if packagePath != "" && isGoMinorVersionEqualHigher(19) {
args = append(args, "-p", packagePath)
}
args = append(args, ASM_DEFINES...)
Expand All @@ -123,16 +129,16 @@ func asmFile(goenv *env, srcPath, packagePath string, asmFlags []string, outPath

var goMinorVersionRegexp = regexp.MustCompile(`^go1\.(\d+)`)

func isGo119OrHigher() bool {
func isGoMinorVersionEqualHigher(requiredMinor int) bool {
match := goMinorVersionRegexp.FindStringSubmatch(runtime.Version())
if match == nil {
// Developer version or something with an unparseable version string,
// assume Go 1.19 or higher.
// assume it matches the conditions
return true
}
minorVersion, err := strconv.Atoi(match[1])
if err != nil {
return true
}
return minorVersion >= 19
return minorVersion >= requiredMinor
}
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 b9f97ba

Please sign in to comment.