cmd/compile: riscv64 miscompiles struct copy, corrupting a []byte slice field

[jasonmontleon]

Go version

go version go1.26.4-X:nodwarf5 linux/amd64 (golang-1.26.4-2.fc44.x86_64)

Output of go env in your module/workspace:

AR='ar'
CC='gcc'
CGO_CFLAGS='-O2 -g'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_ENABLED='1'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
CXX='g++'
GCCGO='gccgo'
GO111MODULE=''
GOAMD64='v1'
GOARCH='amd64'
GOAUTH='netrc'
GOBIN=''
GOCACHE='/home/jason/.cache/go-build'
GOCACHEPROG=''
GODEBUG=''
GOENV='/home/jason/.config/go/env'
GOEXE=''
GOEXPERIMENT='nodwarf5'
GOFIPS140='off'
GOFLAGS=''
GOGCCFLAGS='-fPIC -m64 -pthread -Wl,--no-gc-sections -fmessage-length=0 -ffile-prefix-map=/tmp/go-build2783132525=/tmp/go-build -gno-record-gcc-switches'
GOHOSTARCH='amd64'
GOHOSTOS='linux'
GOINSECURE=''
GOMOD='/dev/null'
GOMODCACHE='/home/jason/go/pkg/mod'
GONOPROXY=''
GONOSUMDB=''
GOOS='linux'
GOPATH='/home/jason/go'
GOPRIVATE=''
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/usr/lib/golang'
GOSUMDB='sum.golang.org'
GOTELEMETRY='local'
GOTELEMETRYDIR='/home/jason/.config/go/telemetry'
GOTMPDIR=''
GOTOOLCHAIN='local'
GOTOOLDIR='/usr/lib/golang/pkg/tool/linux_amd64'
GOVCS=''
GOVERSION='go1.26.4-X:nodwarf5'
GOWORK=''
PKG_CONFIG='pkg-config'

What did you do?

I was looking into a couple kubernetes test failures on riscv64.
https://riscv-kojipkgs.fedoraproject.org/koji/buildinfo?buildID=92143

In particular

--- FAIL: TestValidateControllerRevisionUpdate (0.01s)
    --- FAIL: TestValidateControllerRevisionUpdate/changed_revision (0.00s)
        validation_test.go:1495: changed revision: unexpected error: [data: Invalid value: null: field is immutable]
FAIL

The test passes on other architectures and on riscv64 with -N and/or -l: -gcflags=all='-N -l'

$ CGO_ENABLED=0 GOOS=linux GOARCH=riscv64 GOFLAGS=-mod=vendor go test -gcflags=all='-N' -run 'TestValidateControllerRevisionUpdate$' ./pkg/apis/apps/validation
ok  	k8s.io/kubernetes/pkg/apis/apps/validation	(cached)

$ CGO_ENABLED=0 GOOS=linux GOARCH=riscv64 GOFLAGS=-mod=vendor go test -gcflags=all='-l' -run 'TestValidateControllerRevisionUpdate$' ./pkg/apis/apps/validation
ok  	k8s.io/kubernetes/pkg/apis/apps/validation	(cached)

$ CGO_ENABLED=0 GOOS=linux GOARCH=riscv64 GOFLAGS=-mod=vendor go test -run 'TestValidateControllerRevisionUpdate$' ./pkg/apis/apps/validation
--- FAIL: TestValidateControllerRevisionUpdate (0.05s)
    --- FAIL: TestValidateControllerRevisionUpdate/changed_revision (0.00s)
        validation_test.go:1500: changed revision: unexpected error: [data: Invalid value: null: field is immutable]
FAIL
FAIL	k8s.io/kubernetes/pkg/apis/apps/validation	0.478s
FAIL

claude cooked up the the following as a reproducer, although in this case only -N seems to cause it to work.

package main

import "fmt"

// ---- plain-Go analogs of the Kubernetes types (no external deps) ----

type myTime struct { // mimics time.Time inside metav1.Time
	wall uint64
	ext  int64
	loc  *int
}

// ObjectMeta mimics k8s.io/apimachinery metav1.ObjectMeta field-for-field
type ObjectMeta struct {
	Name, GenerateName, Namespace, SelfLink, UID, ResourceVersion string
	Generation                 int64
	CreationTimestamp          myTime
	DeletionTimestamp          *myTime
	DeletionGracePeriodSeconds *int64
	Labels                     map[string]string
	Annotations                map[string]string
	OwnerReferences            []int
	Finalizers                 []string
	ManagedFields              []int
}

// RawExtension mimics runtime.RawExtension; first field is a []byte slice
type RawExtension struct {
	Raw    []byte
	Object interface{}
}

// CR mimics apps.ControllerRevision: big struct, then RawExtension, then int64
type CR struct {
	ObjectMeta ObjectMeta
	Data       RawExtension
	Revision   int64
}

func main() {
	raw := RawExtension{Raw: []byte("0123456789")} // length 10

	// Two back-to-back composite literals, BOTH initialized from the same `raw`.
	a := CR{ObjectMeta: ObjectMeta{Name: "n", Namespace: "ns", ResourceVersion: "1"}, Data: raw, Revision: 0}
	b := CR{ObjectMeta: ObjectMeta{Name: "n", Namespace: "ns", ResourceVersion: "1"}, Data: raw, Revision: 1}

	fmt.Printf("a.Data.Raw = %q  (len %d)\n", a.Data.Raw, len(a.Data.Raw))
	fmt.Printf("b.Data.Raw = %q  (len %d)\n", b.Data.Raw, len(b.Data.Raw))

	if len(a.Data.Raw) == 10 && len(b.Data.Raw) == 10 {
		fmt.Println("RESULT: OK  - both slices intact (correct codegen)")
	} else {
		fmt.Println("RESULT: BUG - a slice was corrupted by the compiler (miscompile)")
	}
}

$ GOARCH=amd64 go run repro.go
a.Data.Raw = "0123456789"  (len 10)
b.Data.Raw = "0123456789"  (len 10)
RESULT: OK  - both slices intact (correct codegen)

$ CGO_ENABLED=0 GOOS=linux GOARCH=riscv64 go run -gcflags=all='-N' repro.go
a.Data.Raw = "0123456789"  (len 10)
b.Data.Raw = "0123456789"  (len 10)
RESULT: OK  - both slices intact (correct codegen)

$ CGO_ENABLED=0 GOOS=linux GOARCH=riscv64 go run repro.go
a.Data.Raw = "0123456789"  (len 10)
b.Data.Raw = "n"  (len 1)
RESULT: BUG - a slice was corrupted by the compiler (miscompile)

$ CGO_ENABLED=0 GOOS=linux GOARCH=riscv64 go run -gcflags=all='-l' repro.go
a.Data.Raw = "0123456789"  (len 10)
b.Data.Raw = "n"  (len 1)
RESULT: BUG - a slice was corrupted by the compiler (miscompile)

What did you see happen?

A kubernetes test fails with inlining and optimization enabled.

What did you expect to see?

The tests succeeding without disabling optimization/inlining.

[gabyhelp]

Related Issues

• cmd/compile: memcombine should learn allignement of byte slices - easy mode - types #71778
• cmd/compile: memcombine should learn alignment of byte slices - hard mode - arithmetic #71780
• cmd/compile: duplicate instructions after rewrite func results #74788 (closed)
• cmd/compile: commit `eb7f515` causing regression #74836 (closed)
• runtime: go1.26 SIGILL on riscv64 with vector support #78045 (closed)
• cmd/compile: riscv performance degredation #74606 (closed)

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

[jasonmontleon]

Dug a little more using claude and it gave the following explanation:

---

The fault is in cmd/compile/internal/liveness/mergelocals.go — the optimization that lets two local variables share one stack slot when their lifetimes don't overlap. It overlaps raw with b even though they're live at the same time, so building b clobbers raw, corrupting the []byte slice header during the b.Data = raw copy (b.Data.Raw becomes {ptr=&"n", len=1} instead of the real 10-byte slice).

Why its lifetime analysis is wrong. Liveness for an address-taken local is tracked by following the uses of its materialized address (MOVaddr {raw} → v114):

• amd64: both struct copies read the address straight from v114, so both are attributed to raw. raw's interval [4,36) overlaps b → not merged (correct).
• riscv64 (Go 1.26.x): regalloc copies the address into a register once and reuses it — v38 = Copy v114 — and both copies read through v38. The tracker follows v114's uses, reaches the Copy, records it, and stops. It never realizes v38 is also the address of raw, so the two LoweredMoves that actually read raw are attributed only to a/b. raw's interval is truncated to [4,27), looks disjoint from b [30,49) → merged (miscompile).

amd64 (correct):

raw  [4 ------------------------------------------------- 36)
b                               [26 ------------------------------------------ 44)
                                |_shared: positions 26–35_|   ← OVERLAP

Positions 26 through 35 lie in both ranges. At, say, instruction 30, raw and b are both live → the compiler correctly refuses to merge them.

riscv64 (the bug):

raw  [4 ---------------------------------- 27)
b                                                    [30 ----------------------------- 49)
                                              |_gap_|                    ← NO overlap

raw's range was computed as ending at 27, and b's starts at 30. They don't touch, so the compiler concludes "raw is already dead before b is born — safe to put them in the same stack slot." That merge is the miscompile.

What it should be (raw's true live range):

raw  [4 ------------------------------------------------- 40)
b                               [30 ------------------------------------------ 49)
                                |______shared: 30–39______|      ← reality: OVERLAP → DON'T merge

---

and proposed the following patch

diff --git a/src/cmd/compile/internal/liveness/mergelocals.go b/src/cmd/compile/internal/liveness/mergelocals.go
index 6967ee016e..a42ea412e6 100644
--- a/src/cmd/compile/internal/liveness/mergelocals.go
+++ b/src/cmd/compile/internal/liveness/mergelocals.go
@@ -522,6 +522,20 @@ func (cs *cstate) populateIndirectUseTable(cands []*ir.Name) ([]*ir.Name, []cand
 						fmt.Fprintf(os.Stderr, "=-= add indirectUE(%s) count=%d on %s\n", nc.n.Sym().Name, nc.count, v.LongString())
 					}
 					blockIndirectUE[v.ID] = append(blockIndirectUE[v.ID], nc.n)
+					// If v merely forwards the address of nc.n (e.g. a
+					// register-to-register OpCopy inserted by regalloc),
+					// then v's result is itself a pointer to nc.n, so v's
+					// own downstream uses are also indirect uses of nc.n.
+					// Seed pendingUses for v to keep tracking them.
+					// Without this, an access made through the copy is not
+					// attributed to nc.n; its live interval is truncated
+					// and it may be given an unsafe overlapping stack slot
+					// (miscompile).
+					if v.Op == ssa.OpCopy && v.Uses != 0 {
+						if _, dup := pendingUses[v.ID]; !dup {
+							pendingUses[v.ID] = nameCount{n: nc.n, count: v.Uses}
+						}
+					}
 					nc.count--
 					if nc.count == 0 {
 						// That was the last use of the value. Clean

Rebuilding the golang package with this patch I can build the kubernetes successfully on both amd64 and riscv64 without issue, so it seems at least feasible.

[randall77]

I can verify the bug. It also looks like it is fixed at tip. Bisect of the fix points to https://go-review.googlesource.com/c/go/+/760900, which seems kind of like an accidental fix. The underlying problem is probably still there.

AI's analysis of the problem seems correct. raw and b incorrectly share the same stack frame area, as raw's contents are needed to initialize b.

I'm not sure I buy its fix. The problem seems to be deeper than that. Have to think about it.

It does seem like there's nothing particular about riscv64 here. This might hit other archs also.

[randall77]

The fundamental problem in mergelocals is that it thinks that all in-block uses are ok. But uses are only ok if they do not pass on the address somehow. That could be with an OpCopy, but could also be with whatever AddPtr or OffPtr lower to.
Only loads, stores (and not the value being stored), and various copy/zero ops are ok, in that they do not leak their input address on to their users.
Not sure how we would capture that. We could have a bit on ops+arg# that says whether that input arg is safe in this respect.

[randall77]

@thanm

[randall77]

Here's a reproducer, on arm64 and maybe others:

package main

type T [2][2]int64

//go:noinline
func f(i, j int) int64 {
	if i < 0 || i > 1 || j < 0 || j > 1 {
		return 3
	}
	var x T
	x[i][i] = 33

	var y T
	y[i][i] = 44
	r := y[j][j]

	return r + x[i][i]
}

func main() {
	if x := f(1, 1); x != 77 {
		println("bad", x)
	}
}

This prints bad 88. x and y get allocated to the same stack slot, which makes the y[i][i] = 44 overwrite the x[i][i] = 33 assignment.

[thanm]

Thanks @randall77. I am on the road right at the moment but should be able to take a look at the end of the week.

Is your feeling that this is something unique to (or more likely with) riscv64, or just a generic way in which copy ops are handled during liveness analysis?

[randall77]

It seems to be a general problem. Addresses of (parts of) a mergeable variable can live beyond the last explicit reference to that variable. The mergelocals pass doesn't currently handle that case correctly.

It's a bit tricky to make a reproducer, because mergeable variables can't be addrtaken. But it can be done by convincing the compiler to do some address arithmetic that ends up being CSEd in the right places.

[gopherbot]

Change https://go.dev/cl/796001 mentions this issue: cmd/compile: tighten mergelocals address use analysis