Go version

go version go1.22.3 darwin/arm64

Output of go env in your module/workspace:

GO111MODULE='on'
GOARCH='arm64'
GOBIN=''
GOCACHE='/Users/mathetake/Library/Caches/go-build'
GOENV='/Users/mathetake/Library/Application Support/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFLAGS=''
GOHOSTARCH='arm64'
GOHOSTOS='darwin'
GOINSECURE=''
GOMODCACHE='/Users/mathetake/go/pkg/mod'
GONOPROXY=''
GONOSUMDB=''
GOOS='darwin'
GOPATH='/Users/mathetake/go'
GOPRIVATE=''
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/usr/local/go'
GOSUMDB='sum.golang.org'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/usr/local/go/pkg/tool/darwin_arm64'
GOVCS=''
GOVERSION='go1.22.3'
GCCGO='gccgo'
AR='ar'
CC='clang'
CXX='clang++'
CGO_ENABLED='1'
GOMOD='/Users/mathetake/wazero/go.mod'
GOWORK=''
CGO_CFLAGS='-O2 -g'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
PKG_CONFIG='pkg-config'
GOGCCFLAGS='-fPIC -arch arm64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -ffile-prefix-map=/var/folders/zv/cj1015lx5bndpw3n_qsx9mh40000gn/T/go-build2746109211=/tmp/go-build -gno-record-gcc-switches -fno-common'

What did you do?

The following is the small repro that I managed to craft, and I run go test . and go test -race ..

type someType []uint64

func (s *someType) push(v uint64) {
    *s = append(*s, v)
}

func (s *someType) problematicFn(x1Lo, x1Hi, x2Lo, x2Hi uint64) {
    r1 := int32(int16(x1Lo>>0)) * int32(int16(x2Lo>>0))
    r3 := int32(int16(x1Lo>>32)) * int32(int16(x2Lo>>32))
    r4 := int32(int16(x1Lo>>48)) * int32(int16(x2Lo>>48))
    r5 := int32(int16(x1Hi>>0)) * int32(int16(x2Hi>>0))
    r7 := int32(int16(x1Hi>>32)) * int32(int16(x2Hi>>32))
    r8 := int32(int16(x1Hi>>48)) * int32(int16(x2Hi>>48))
    s.push(uint64(uint32(r1)) | (uint64(uint32(r3+r4)) << 32))
    s.push(uint64(uint32(r5)) | (uint64(uint32(r7+r8)) << 32))
}

func TestProblematicFn(t *testing.T) {
    s := &someType{}
    s.problematicFn(281479271743489, 281479271743489, 18446744073709551615, 18446744073709551615)
    if (*s)[0] != 18446744069414584319 {
        t.Fatal("unexpected result")
    }
    if (*s)[1] != 18446744073709551615 {
        t.Fatal("unexpected result")
    }
}

What did you see happen?

I got the assertion failure only for the case with -race. Note that this also happens for linux/arm64 but only when Go 1.22. When I briefly run with the different Go version, I got the consistent result but the assertion was failing, meaning that the compiled machine code has some undefined behavior. I suspect that seems the root case.

What did you expect to see?

Consistent results regardless of with or withour -race flag.

Comment From: gabyhelp

Related Issues

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

Comment From: ianlancetaylor

CC @randall77 @golang/arm

Comment From: randall77

I can reproduce. -race is not necessary, any function call causes the problem. Adding fmt.Printf("%x\n", r1) after calculating r1 is enough.

I see the problem, it's a missing zero-extension.

We start with this code:

v21 (15) = MULW <int32> v20 v17 (r1[int32])
v108 (+22) = MOVWUreg <uint64> v21

which is fine.

Then in late lower, we decide that because MULW already zeroes the high 32 bits, we don't need the zero extension. All uses of v108 are changed to use v21 directly.

But then during regalloc, we decide that we need to save/restore v21 around a call, and we use a 32-bit store to save it and a 32-bit signed load to restore it. Boom, that value no longer has zeroed high bits. That causes one of the ORs at the very end to set a bit it shouldn't.

We've seen this kind of error before #21963 #66066, and also kind of similar is #59367. My commit message text in CL 568616, which fixed #66066, reads

I am also worried about spilling and restoring. Spilling and restoring doesn't preserve the high bits, but instead sets them to a known value (often 0, but in some cases it could be sign-extended). I am unable to come up with a case that would cause a problem here, so leaving for another time.

This issue is the case I needed :)

The underlying confusion is that when a type only fills part of a register, the upper part is junk. So just because MULW clears the upper 32 bits, it doesn't mean that those upper bits will be preserved from the MULW to the use. The MULW would have to be typed 64 bits wide to preserve its upper bits. (Which is one possible fix here, but I will look further.)

Comment From: mathetake

cool, yeah I also noticed that this happens with or without fmt.Println, so I suspected something wrong around regalloc. Glad to hear my repro was what you needed!

Comment From: gopherbot

Change https://go.dev/cl/595675 mentions this issue: cmd/compile: don't elide zero extension on top of signed values

Comment From: randall77

@gopherbot please open backport issue for 1.22.

Comment From: gopherbot

Backport issue(s) opened: #68230 (for 1.22).

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

Comment From: thanm

DIscussed during the weekly release checkin -- does this bug need to be fixed in 1.21 as well?

Comment From: randall77

I don't believe so. The rules that triggered the bug were added in 1.22.