Smaller example

package main

import "hash/crc32"

func main() {
	colors := []string{"ffd179", "b786d2", "bed3f4"}

	// has is uint32(754903878), but panic occures only during crc32 call
	// and not, when you set this value explicitely
	hash := crc32.ChecksumIEEE([]byte("jHjDiYOWTOS53K9t3XbOU7QXAwD3"))
	index := hash % uint32(3)

	_ = colors[index] // PANIC: index out of range -4294967296
	return
}

The magic uint64 number -4294967296 is 0xFFFFFFFF 00000000, but all variables are 32bit unsigned ints. Thus
dropping high bits, shall be 0 or 0x00000000, however, due to optimizations, go compares -4294967296 with array length and panics.

More context

during execution, around index := hash % uint32(len(colors)) :

<main.main+108> sub x0, x0, x1

x0: 0xFFFFFFFF 2CFEEB46 -- register, which would be used as array index is .
x1: 0x2CFEEB46 -- (754903878) value needed for reminder calculation.

After substracting x1 from x0 part, we end with 0xFFFFFFFF00000000, where previous XORing set high bit's to 1.
As result out of bounds check fails, comparing remainder with array length fails and jumps to Panic.

uint32 type is ignored here. Everywhere.

Panics on go1.21.0 linux/arm64
Works on go1.21.0 linux/amd64

According to docs & playground proof:

^x bitwise complement is m ^ x with m = "all bits set to 1" for unsigned x and m = -1 for signed x

Following call makes sets x0 to -1, effectively 0xfffffffffffffff

return ^ieeeUpdate(^crc, p)     // hash/crc32.archUpdateIEEE mvn x0 x0

CC @golang/security.

No dependencies example

./bug.s

#include "textflag.h"

// func simple(crc uint32, p []byte) uint32
TEXT ·simple(SB),NOSPLIT,$0-36
	MOVWU	crc+0(FP), R9  // CRC value
	MOVWU	R9, ret+32(FP)
	RET

./bug.go

package main

func simple(crc uint32, p []byte) uint32

func main() {
	colors := []string{ "", "", ""}
	hash := archUpdateIEEE(0, []byte("jHjDiYOWTOS53K9t3XbOU7QXAwD3"))
	index := hash % uint32(3)
	_ = colors[index]
	return
}

func archUpdateIEEE(crc uint32, p []byte) uint32 {
	return ^simple(^crc, p)
}

Since you can reproduce the problem in assembly, it seems likely that this is a compiler or runtime change.

Can you build the Go repo from source and use git bisect to find the exact commit at which the regression began?

(attn @golang/compiler @golang/arm)

Simple reproducer:

package main

//go:noinline
func f() uint32 {
	return x - y
}
var x uint32 = 0
var y uint32 = 1
var z [2]int

func main() {
	_ = z[f() % 3]
}

Looks like there is a missing MOVWU in the % computation in 1.21.

This looks like https://go-review.googlesource.com/c/go/+/427454
@RuinanSun

Looks like the MOVWUreg is removed when its arg is a MSUBW, which is guaranteed to zero the upper 32 bits. But then later that arg gets changed to something that doesn't zero the upper 32 bits, a SUB.
So I think the logic that we don't need the zero extension if its arg does the zero extension itself, is invalid if we also have the rule that rewrites of 32-bit-typed values can change the upper 32 bits.
The rule is only valid if all our rewrite rules preserve the upper-32-bits-are-zeroed property.
Unless we do the MOVWUreg removal in a separate pass after all other rewrites are done.

The rule is only valid if all our rewrite rules preserve the upper-32-bits-are-zeroed property.

I've hacked up a change that fixes all the rules to preserve the semantics of the upper 32 bits. For example, when rewriting MULW with constant -1 argument, use NEGW and not NEG.
It's a pretty rote change but kind of large. Not something I would want to backport. I think we should revert CL 427454 on the 1.21 release branch and do my change for 1.22.

I worry other architectures have the same issue, as lots of those were started by copying the arm64 port. I will investigate.

@gopherbot please open a backport to 1.21.

Backport issue(s) opened: #62143 (for 1.21).

Remember to create the cherry-pick CL(s) as soon as the patch is submitted to master, according to https://go.dev/wiki/MinorReleases.

Change https://go.dev/cl/520916 mentions this issue: cmd/compile: ensure we keep top 32 bits zeroed for 32-bit arm64 ops

Thanks for the fix! @randall77

I've hacked up a change that fixes all the rules to preserve the semantics of the upper 32 bits.

Does it look like CL 427454 is still correct if we can hold this for all 32-bit rewrite rules?

The revert is fine, I just wonder if we can redo this work after your fix in 1.22.

The revert is fine, I just wonder if we can redo this work after your fix in 1.22.

CL 427454 is not reverted in the tip branch. I think Keith's fix is the right direction.