Skip to content

build: arm64 builders don't support SHA-512 extensions #69593

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
FiloSottile opened this issue Sep 23, 2024 · 7 comments
Closed

build: arm64 builders don't support SHA-512 extensions #69593

FiloSottile opened this issue Sep 23, 2024 · 7 comments
Assignees
@FiloSottile
Labels
arch-arm64 Builders x/build issues (builders, bots, dashboards) FixPending Issues that have a fix which has not yet been reviewed or submitted. NeedsFix The path to resolution is known, but the work has not been done.
Milestone
Go1.25

Comments

@FiloSottile
Copy link
Contributor

The arm64 builders don't support the SHA-512 Armv8 extensions, so our crypto/sha512 assembly is untested, in violation of the Assembly Policy.

/cc @golang/security @golang/release
@gabyhelp
Copy link

Related Issues and Documentation

(Emoji vote if this was helpful or unhelpful; more detailed feedback welcome in this discussion.)

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/614656 mentions this issue: crypto/sha256,crypto/sha512: test fallback implementations

@gabyhelp gabyhelp mentioned this issue Sep 23, 2024
Open
@dr2chase dr2chase added the NeedsFix The path to resolution is known, but the work has not been done. label Sep 24, 2024
cpu pushed a commit to cpu/go that referenced this issue Oct 1, 2024
@FiloSottile
crypto/sha256,crypto/sha512: test fallback implementations
53b8834 
This will be required for golang#69536 but is also good hygiene and required
by go.dev/wiki/AssemblyPolicy.

> The code must be tested in our CI. This means there need to be
> builders that support the instructions, and if there are multiple (or
> fallback) paths they must be tested separately.

The new crypto/internal/impl registry lets us select alternative
implementations from both the same package and importers (such as
crypto/sha256 tests once we have crypto/internal/fips/sha256, or
crypto/hmac).

Updates golang#69592
Updates golang#69593

Change-Id: Ifea22a9fc9ccffcaf4924ff6bd08da7c9bd39e99
Cq-Include-Trybots: luci.golang.try:gotip-linux-arm64-longtest,gotip-linux-amd64-longtest,gotip-linux-ppc64le_power8,gotip-linux-ppc64_power8
cpu pushed a commit to cpu/go that referenced this issue Oct 16, 2024
@FiloSottile
crypto/sha256,crypto/sha512: test fallback implementations
8388169 
This will be required for golang#69536 but is also good hygiene and required
by go.dev/wiki/AssemblyPolicy.

> The code must be tested in our CI. This means there need to be
> builders that support the instructions, and if there are multiple (or
> fallback) paths they must be tested separately.

The new crypto/internal/impl registry lets us select alternative
implementations from both the same package and importers (such as
crypto/sha256 tests once we have crypto/internal/fips/sha256, or
crypto/hmac).

Updates golang#69592
Updates golang#69593

Change-Id: Ifea22a9fc9ccffcaf4924ff6bd08da7c9bd39e99
Cq-Include-Trybots: luci.golang.try:gotip-linux-arm64-longtest,gotip-linux-amd64-longtest,gotip-linux-ppc64le_power8,gotip-linux-ppc64_power8
gopherbot pushed a commit that referenced this issue Oct 23, 2024
@FiloSottile @gopherbot
crypto/sha256,crypto/sha512: test fallback implementations
f04f4c2 
This will be required for #69536 but is also good hygiene and required
by go.dev/wiki/AssemblyPolicy.

> The code must be tested in our CI. This means there need to be
> builders that support the instructions, and if there are multiple (or
> fallback) paths they must be tested separately.

The new crypto/internal/impl registry lets us select alternative
implementations from both the same package and importers (such as
crypto/sha256 tests once we have crypto/internal/fips/sha256, or
crypto/hmac).

Updates #69592
Updates #69593

Change-Id: Ifea22a9fc9ccffcaf4924ff6bd08da7c9bd39e99
Cq-Include-Trybots: luci.golang.try:gotip-linux-arm64-longtest,gotip-linux-amd64-longtest,gotip-linux-ppc64le_power8,gotip-linux-ppc64_power8
Reviewed-on: https://go-review.googlesource.com/c/go/+/614656
LUCI-TryBot-Result: Go LUCI <[email protected]>
Reviewed-by: Daniel McCarney <[email protected]>
Reviewed-by: Michael Pratt <[email protected]>
Reviewed-by: Roland Shoemaker <[email protected]>
Auto-Submit: Filippo Valsorda <[email protected]>
@FiloSottile FiloSottile mentioned this issue Oct 23, 2024
Closed
@rolandshoemaker
Copy link
Member

The new GCP C4A instances appear to support the SHA-512 instructions.

@FiloSottile
Copy link
Contributor Author

FiloSottile commented Apr 24, 2025 •
edited by gabyhelp
Loading

I ran into this again for CL 667675, but I realized we have Apple M builders, which do have the Armv8.2 instructions, so we can use those. It's unfortunate we don't have a darwin/arm64 longtest, though.

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/667755 mentions this issue: crypto/internal/cryptotest: test Armv8.2 on darwin/arm64

@prattmic
Copy link
Member

As part of #73397, I've turned up new darwin-arm64 testing infra, increasing our capacity from 5 to 30 machines. We were just discussing that a good next step would be to add darwin-arm64 longtest and race builders. Feel free to open an issue.

@FiloSottile FiloSottile mentioned this issue Apr 24, 2025
Closed
@FiloSottile
Copy link
Contributor Author

Done, thank you! #73488

@dmitshur dmitshur added arch-arm64 Builders x/build issues (builders, bots, dashboards) FixPending Issues that have a fix which has not yet been reviewed or submitted. labels Apr 24, 2025
@dmitshur dmitshur moved this to In Progress in Go Release Apr 24, 2025
@dmitshur dmitshur added this to the Go1.25 milestone Apr 24, 2025
@github-project-automation github-project-automation bot moved this from In Progress to Done in Go Release May 21, 2025
Sirherobrine23 pushed a commit to Sirherobrine23/go that referenced this issue May 24, 2025
@FiloSottile @Sirherobrine23
crypto/internal/cryptotest: test Armv8.2 on darwin/arm64
bd9a510 
Fixes golang#69593

Cq-Include-Trybots: luci.golang.try:gotip-darwin-arm64_15
Change-Id: I6a6a4656302d65b582df582fa12bb72b88b0316d
Reviewed-on: https://go-review.googlesource.com/c/go/+/667755
Reviewed-by: David Chase <[email protected]>
Reviewed-by: Roland Shoemaker <[email protected]>
LUCI-TryBot-Result: Go LUCI <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@FiloSottile FiloSottile

Labels
arch-arm64 Builders x/build issues (builders, bots, dashboards) FixPending Issues that have a fix which has not yet been reviewed or submitted. NeedsFix The path to resolution is known, but the work has not been done.
Projects
Go Release
Status: Done
Milestone
Go1.25
Development

No branches or pull requests
7 participants
@FiloSottile @prattmic @dmitshur @dr2chase @rolandshoemaker @gopherbot @gabyhelp