cmd/compile: late nil-check elimination changes nil-pointer access order

[jake-ciolek]

Go version

go 1.26.5, go 1.25.12, tip

Output of go env in your module/workspace:

GOARCH=amd64

What did you do?

Note, this was originally reported to security@golang.org, but the policy has changed and I was told to create a public issue.

package main

//go:noinline
func faultAt(p *[1 << 32]byte) byte {
	return p[0xdeadbeef] + p[0]
}

func main() {
	defer func() { recover() }()
	_ = faultAt(nil)
}

What did you see happen?

$ go run .
unexpected fault address 0xdeadbeef
fatal error: fault
[signal SIGSEGV: segmentation violation code=0x1 addr=0xdeadbeef pc=0x47a785]

What did you expect to see?

No SIGSEGV (doesn't happen on unoptimized builds).

[gabyhelp]

Related Issues

• cmd/compile: nilcheck is tightened in a branch allowing the other one to execute when it should have panicked #72860 (closed)
• cmd/compile: prove bug causes invalid indirect call #80517
• cmd/compile: late nilcheck elim doesn't elim later nilcheck #20962
• cmd/compile: nil pointer dereference does not panic #72918 (closed)
• cmd/compile: CMOV optimization allows GC to see invalid pointer #56990 (closed)
• cmd/compile: prove pass regression enables memory corruption #76355 (closed)
• cmd/compile: Program should arguably panic, but doesn't #39122 (closed)
• cmd/compile: redundant checks for nil pointers when calling an inlined method #11483 (closed)
• cmd/compile: `len(*f())` does not panic when f() returns (*[k]T)(nil) #72844 (closed)
• cmd/compile: generic code miscompilation returns stale value after clear #80520

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

[thepudds]

On the surface, this seems to be an old discrepancy. A git bisect landed on b1785a5 from 2016:

  first BAD:   b1785a5065 | 2016-03-08 02:32 | cmd/compile: Tinkering with schedule for debug and regalloc
  GOOD parent: 481fe59012 | 2016-03-08 02:21 | cmd/compile: fix load combining from a non-zero pointer offset

Maybe that commit is just jiggling some other latent issue, but go run seems to fault at that commit for the reported example:

$ go run /tmp/repro-nil.go
unexpected fault address 0xdeadbeef
fatal error: fault
[signal 0xb code=0x1 addr=0xdeadbeef pc=0x40100a]
[...]

$ go version
go version devel +b1785a5065 Tue Mar 8 02:32:20 2016 +0000 linux/amd64

That commit does not seem to fault with -gcflags='-N'. The parent commit did not fault in quick test.

CC @golang/compiler

[randall77]

Yes, this looks like we allow nil checks to move after other potentially faulting instructions.

In generic SSA, we keep nil checks sequenced before the use of the pointer, by having the load/store use the result of the nilcheck. See https://go-review.googlesource.com/c/go/+/537775. We do not do that in lowered SSA. Maybe we should, although the comment on that CL suggests we'd have to make sure we don't make regalloc worse by doing that.

This bug can only cause spurious loads. It will never allow a spurious store.