cmd/compile: inconsistent integer arithmetic result on Go 1.22+arm64 with/without -race

[mathetake]

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.

[gabyhelp]

Related Issues

• cmd/compile: miscompilation of simple arithmetic program targeting darwin/arm64 #62723 (closed)
• cmd/compile: unstable arithmetic #52293 (closed)
• cmd/compile: -race instrumentation leads to different behaviour due to 16 to 32-bit expansion #21963 (closed)
• runtime: Inconsistency Multiplication, Division, and Addition Result #51031 (closed)
• cmd/compile: bad code generation for ARM (bounds test fails) #41780 (closed)
• go compiler: Incorrect results on GOOS=linux GOARCH=arm64 running on Google t2a-standard-8 (Ampere Altra) #66431 (closed)
• cmd/compile: compiler error when building a binary with race detector enabled #64606 (closed)
• runtime: floating point error on arm64 #44528 (closed)
• Floating point semantics: unstable semantics inside/outside of a loop and among architectures #61061 (closed)
• Parsing float64 to uint64 yields different results on GOARCH="arm64" #52217 (closed)

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

[ianlancetaylor]

CC @randall77 @golang/arm

[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.)

[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!

[gopherbot]

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

[randall77]

@gopherbot please open backport issue for 1.22.

[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.

[thanm]

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

[randall77]

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