Fix invocation of assembler for go1.22 #3756
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
sywhang
reviewed
Nov 20, 2023
jquirke
changed the title
fmeum
reviewed
Nov 20, 2023
jquirke
force-pushed
the
go122asm
branch
2 times, most recently
from
ff65758
to
8dc4747
Compare
fmeum
approved these changes
Nov 20, 2023
jquirke
added a commit
to jquirke/rules_go
that referenced
this pull request
Nov 20, 2023
I'm testing the PR rebased on v0.42.0 in Uber internal diff D11928203 |
jquirke
force-pushed
the
go122asm
branch
2 times, most recently
from
4d64f5b
to
afac8b2
Compare
|
I tested this on BuildBuddy repo + Go SDK 1.22.0 and it worked. Without this PR it would fail the same way stated in the PR summary. Thanks for submitting this. 🙏 |
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.
fmeum
approved these changes
Feb 7, 2024
cgrindel-self-hosted-renovate bot
referenced
this pull request
in cgrindel/bazel-starlib
Feb 10, 2024
This PR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [io_bazel_rules_go](https://togithub.com/bazelbuild/rules_go) | http_archive | minor | `v0.45.1` -> `v0.46.0` | --- ### Release Notes <details> <summary>bazelbuild/rules_go (io_bazel_rules_go)</summary> ### [`v0.46.0`](https://togithub.com/bazelbuild/rules_go/releases/tag/v0.46.0) [Compare Source](https://togithub.com/bazelbuild/rules_go/compare/v0.45.1...v0.46.0) #### `WORKSPACE` code load("@​bazel_tools//tools/build_defs/repo:http.bzl", "http_archive") http_archive( name = "io_bazel_rules_go", sha256 = "80a98277ad1311dacd837f9b16db62887702e9f1d1c4c9f796d0121a46c8e184", urls = [ "https://mirror.bazel.build/github.com/bazelbuild/rules_go/releases/download/v0.46.0/rules_go-v0.46.0.zip", "https://github.com/bazelbuild/rules_go/releases/download/v0.46.0/rules_go-v0.46.0.zip", ], ) load("@​io_bazel_rules_go//go:deps.bzl", "go_register_toolchains", "go_rules_dependencies") go_rules_dependencies() go_register_toolchains(version = "1.22.0") #### What's Changed - Support custom `GOARM` architecture levels via platform constraints by [@​LINKIWI](https://togithub.com/LINKIWI) in [https://github.com/bazelbuild/rules_go/pull/3837](https://togithub.com/bazelbuild/rules_go/pull/3837) - Emit nogo facts into a separate archive by [@​fmeum](https://togithub.com/fmeum) in [https://github.com/bazelbuild/rules_go/pull/3789](https://togithub.com/bazelbuild/rules_go/pull/3789) - go_test: ensure external source compilation has data by [@​sluongng](https://togithub.com/sluongng) in [https://github.com/bazelbuild/rules_go/pull/3848](https://togithub.com/bazelbuild/rules_go/pull/3848) - Fix invocation of assembler for go1.22 by [@​jquirke](https://togithub.com/jquirke) in [https://github.com/bazelbuild/rules_go/pull/3756](https://togithub.com/bazelbuild/rules_go/pull/3756) - nogo: Create a go_register_nogo wrapper for WORKSPACE users. by [@​DolceTriade](https://togithub.com/DolceTriade) in [https://github.com/bazelbuild/rules_go/pull/3842](https://togithub.com/bazelbuild/rules_go/pull/3842) - prepare minor release 0.46 by [@​tyler-french](https://togithub.com/tyler-french) in [https://github.com/bazelbuild/rules_go/pull/3854](https://togithub.com/bazelbuild/rules_go/pull/3854) #### New Contributors - [@​LINKIWI](https://togithub.com/LINKIWI) made their first contribution in [https://github.com/bazelbuild/rules_go/pull/3837](https://togithub.com/bazelbuild/rules_go/pull/3837) - [@​jquirke](https://togithub.com/jquirke) made their first contribution in [https://github.com/bazelbuild/rules_go/pull/3756](https://togithub.com/bazelbuild/rules_go/pull/3756) **Full Changelog**: bazel-contrib/rules_go@v0.45.1...v0.46.0 </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Enabled. ♻ **Rebasing**: Whenever PR is behind base branch, or you tick the rebase/retry checkbox. 👻 **Immortal**: This PR will be recreated if closed unmerged. Get [config help](https://togithub.com/renovatebot/renovate/discussions) if that's undesired. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR has been generated by [Renovate Bot](https://togithub.com/renovatebot/renovate). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNi4xMDAuMCIsInVwZGF0ZWRJblZlciI6IjM2LjEwMC4wIiwidGFyZ2V0QnJhbmNoIjoibWFpbiJ9--> Co-authored-by: Self-hosted Renovate Bot <361546+cgrindel-self-hosted-renovate[bot]@users.noreply.github.enterprise.com>
renovate bot
referenced
this pull request
in kreempuff/rules_unreal_engine
Feb 10, 2024
[](https://renovatebot.com) This PR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [io_bazel_rules_go](https://togithub.com/bazelbuild/rules_go) | http_archive | minor | `v0.45.1` -> `v0.46.0` | --- > [!WARNING] > Some dependencies could not be looked up. Check the Dependency Dashboard for more information. --- ### Release Notes <details> <summary>bazelbuild/rules_go (io_bazel_rules_go)</summary> ### [`v0.46.0`](https://togithub.com/bazelbuild/rules_go/releases/tag/v0.46.0) [Compare Source](https://togithub.com/bazelbuild/rules_go/compare/v0.45.1...v0.46.0) #### `WORKSPACE` code load("@​bazel_tools//tools/build_defs/repo:http.bzl", "http_archive") http_archive( name = "io_bazel_rules_go", sha256 = "80a98277ad1311dacd837f9b16db62887702e9f1d1c4c9f796d0121a46c8e184", urls = [ "https://mirror.bazel.build/github.com/bazelbuild/rules_go/releases/download/v0.46.0/rules_go-v0.46.0.zip", "https://github.com/bazelbuild/rules_go/releases/download/v0.46.0/rules_go-v0.46.0.zip", ], ) load("@​io_bazel_rules_go//go:deps.bzl", "go_register_toolchains", "go_rules_dependencies") go_rules_dependencies() go_register_toolchains(version = "1.22.0") #### What's Changed - Support custom `GOARM` architecture levels via platform constraints by [@​LINKIWI](https://togithub.com/LINKIWI) in [https://github.com/bazelbuild/rules_go/pull/3837](https://togithub.com/bazelbuild/rules_go/pull/3837) - Emit nogo facts into a separate archive by [@​fmeum](https://togithub.com/fmeum) in [https://github.com/bazelbuild/rules_go/pull/3789](https://togithub.com/bazelbuild/rules_go/pull/3789) - go_test: ensure external source compilation has data by [@​sluongng](https://togithub.com/sluongng) in [https://github.com/bazelbuild/rules_go/pull/3848](https://togithub.com/bazelbuild/rules_go/pull/3848) - Fix invocation of assembler for go1.22 by [@​jquirke](https://togithub.com/jquirke) in [https://github.com/bazelbuild/rules_go/pull/3756](https://togithub.com/bazelbuild/rules_go/pull/3756) - nogo: Create a go_register_nogo wrapper for WORKSPACE users. by [@​DolceTriade](https://togithub.com/DolceTriade) in [https://github.com/bazelbuild/rules_go/pull/3842](https://togithub.com/bazelbuild/rules_go/pull/3842) - prepare minor release 0.46 by [@​tyler-french](https://togithub.com/tyler-french) in [https://github.com/bazelbuild/rules_go/pull/3854](https://togithub.com/bazelbuild/rules_go/pull/3854) #### New Contributors - [@​LINKIWI](https://togithub.com/LINKIWI) made their first contribution in [https://github.com/bazelbuild/rules_go/pull/3837](https://togithub.com/bazelbuild/rules_go/pull/3837) - [@​jquirke](https://togithub.com/jquirke) made their first contribution in [https://github.com/bazelbuild/rules_go/pull/3756](https://togithub.com/bazelbuild/rules_go/pull/3756) **Full Changelog**: bazel-contrib/rules_go@v0.45.1...v0.46.0 </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Enabled. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR has been generated by [Mend Renovate](https://www.mend.io/free-developer-tools/renovate/). View repository job log [here](https://developer.mend.io/github/kreempuff/rules_unreal_engine). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy4xNzMuMCIsInVwZGF0ZWRJblZlciI6IjM3LjE3My4wIiwidGFyZ2V0QnJhbmNoIjoibWFpbiJ9-->
cgrindel-self-hosted-renovate bot
referenced
this pull request
in cgrindel/rules_swift_package_manager
Feb 10, 2024
This PR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [io_bazel_rules_go](https://togithub.com/bazelbuild/rules_go) | http_archive | minor | `v0.45.1` -> `v0.46.0` | --- ### Release Notes <details> <summary>bazelbuild/rules_go (io_bazel_rules_go)</summary> ### [`v0.46.0`](https://togithub.com/bazelbuild/rules_go/releases/tag/v0.46.0) [Compare Source](https://togithub.com/bazelbuild/rules_go/compare/v0.45.1...v0.46.0) #### `WORKSPACE` code load("@​bazel_tools//tools/build_defs/repo:http.bzl", "http_archive") http_archive( name = "io_bazel_rules_go", sha256 = "80a98277ad1311dacd837f9b16db62887702e9f1d1c4c9f796d0121a46c8e184", urls = [ "https://mirror.bazel.build/github.com/bazelbuild/rules_go/releases/download/v0.46.0/rules_go-v0.46.0.zip", "https://github.com/bazelbuild/rules_go/releases/download/v0.46.0/rules_go-v0.46.0.zip", ], ) load("@​io_bazel_rules_go//go:deps.bzl", "go_register_toolchains", "go_rules_dependencies") go_rules_dependencies() go_register_toolchains(version = "1.22.0") #### What's Changed - Support custom `GOARM` architecture levels via platform constraints by [@​LINKIWI](https://togithub.com/LINKIWI) in [https://github.com/bazelbuild/rules_go/pull/3837](https://togithub.com/bazelbuild/rules_go/pull/3837) - Emit nogo facts into a separate archive by [@​fmeum](https://togithub.com/fmeum) in [https://github.com/bazelbuild/rules_go/pull/3789](https://togithub.com/bazelbuild/rules_go/pull/3789) - go_test: ensure external source compilation has data by [@​sluongng](https://togithub.com/sluongng) in [https://github.com/bazelbuild/rules_go/pull/3848](https://togithub.com/bazelbuild/rules_go/pull/3848) - Fix invocation of assembler for go1.22 by [@​jquirke](https://togithub.com/jquirke) in [https://github.com/bazelbuild/rules_go/pull/3756](https://togithub.com/bazelbuild/rules_go/pull/3756) - nogo: Create a go_register_nogo wrapper for WORKSPACE users. by [@​DolceTriade](https://togithub.com/DolceTriade) in [https://github.com/bazelbuild/rules_go/pull/3842](https://togithub.com/bazelbuild/rules_go/pull/3842) - prepare minor release 0.46 by [@​tyler-french](https://togithub.com/tyler-french) in [https://github.com/bazelbuild/rules_go/pull/3854](https://togithub.com/bazelbuild/rules_go/pull/3854) #### New Contributors - [@​LINKIWI](https://togithub.com/LINKIWI) made their first contribution in [https://github.com/bazelbuild/rules_go/pull/3837](https://togithub.com/bazelbuild/rules_go/pull/3837) - [@​jquirke](https://togithub.com/jquirke) made their first contribution in [https://github.com/bazelbuild/rules_go/pull/3756](https://togithub.com/bazelbuild/rules_go/pull/3756) **Full Changelog**: bazel-contrib/rules_go@v0.45.1...v0.46.0 </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Enabled. ♻ **Rebasing**: Whenever PR is behind base branch, or you tick the rebase/retry checkbox. 👻 **Immortal**: This PR will be recreated if closed unmerged. Get [config help](https://togithub.com/renovatebot/renovate/discussions) if that's undesired. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR has been generated by [Renovate Bot](https://togithub.com/renovatebot/renovate). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNi4xMDAuMCIsInVwZGF0ZWRJblZlciI6IjM2LjEwMC4wIiwidGFyZ2V0QnJhbmNoIjoibWFpbiJ9--> Co-authored-by: Self-hosted Renovate Bot <361546+cgrindel-self-hosted-renovate[bot]@users.noreply.github.enterprise.com>
rail
pushed a commit
to rail/rules_go
that referenced
this pull request
Feb 27, 2024
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.
](https://renovatebot.com){kind=link}
8 tasks
akalenyu
added a commit
to akalenyu/containerized-data-importer
that referenced
this pull request
May 8, 2024
More info at bazel-contrib/rules_go#3756 Signed-off-by: Alex Kalenyuk <[email protected]>
kubevirt-bot
pushed a commit
to kubevirt/containerized-data-importer
that referenced
this pull request
May 16, 2024
* Use new builder with 1.22.3 Signed-off-by: Alex Kalenyuk <[email protected]> * Update rules_go to v0.46.0 More info at bazel-contrib/rules_go#3756 Signed-off-by: Alex Kalenyuk <[email protected]> * Bump go to version 1.22 in go.mod & make deps-update Signed-off-by: Alex Kalenyuk <[email protected]> * make generate Signed-off-by: Alex Kalenyuk <[email protected]> * Run prom metric linter entirely in builder We we're executing the go build command directly on the host instead of in the container Signed-off-by: Alex Kalenyuk <[email protected]> * Bump golangci version due to panic Bumping to avoid a panic with 1.22.3 ``` ERRO [runner] Panic: SA1027: package "main" (isInitialPkg: true, needAnalyzeSource: true): runtime error: invalid memory address or nil pointer dereference: goroutine 20883 [running]: runtime/debug.Stack() /gimme/.gimme/versions/go1.22.3.linux.amd64/src/runtime/debug/stack.go:24 +0x5e ``` Signed-off-by: Alex Kalenyuk <[email protected]> --------- Signed-off-by: Alex Kalenyuk <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What type of PR is this?
Bug fix
What does this PR do? Why is it needed?
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.
Now, without specifying -p to the assembler when generating symabis, the output symabis file will contain something like:
instead of
The result is that the compiler will default to using ABIInternal instead of ABI0 if it cannot resolve the imported symbol's ABI in symabis, and default the symbol as ABIInternal in the assembled object files NonPkgDefs.
This will inevitably cause a link failure as the consuming object files declare their expecatation of ABI0 in their NonPkgRefs.
Which issues(s) does this PR fix?
Fixes #3755, #3849
Other notes for review