cmd/compile: memory corruption when setting outer values in functional iterators

[scr-oath]

Go version

go version go1.23.2 darwin/arm64

Output of go env in your module/workspace:

GO111MODULE='on'
GOARCH='arm64'
GOBIN=''
GOCACHE='/Users/scr/Library/Caches/go-build'
GOENV='/Users/scr/Library/Application Support/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFLAGS=''
GOHOSTARCH='arm64'
GOHOSTOS='darwin'
GOINSECURE=''
GOMODCACHE='/Users/scr/.asdf/installs/golang/1.23.2/packages/pkg/mod'
GONOPROXY='go.ouryahoo.com'
GONOSUMDB='go.ouryahoo.com'
GOOS='darwin'
GOPATH='/Users/scr/.asdf/installs/golang/1.23.2/packages'
GOPRIVATE='go.ouryahoo.com'
GOPROXY='https://goproxy.ouryahoo.com:4443/goproxy,https://proxy.golang.org,direct'
GOROOT='/Users/scr/.asdf/installs/golang/1.23.2/go'
GOSUMDB='sum.golang.org'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/Users/scr/.asdf/installs/golang/1.23.2/go/pkg/tool/darwin_arm64'
GOVCS=''
GOVERSION='go1.23.2'
GODEBUG=''
GOTELEMETRY='local'
GOTELEMETRYDIR='/Users/scr/Library/Application Support/go/telemetry'
GCCGO='gccgo'
GOARM64='v8.0'
AR='ar'
CC='clang'
CXX='clang++'
CGO_ENABLED='1'
GOMOD='/Users/scr/GolandProjects/scr/show-iteration-failure/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/wj/29xj08zd0vn3trjdyh197h_00000gq/T/go-build942010698=/tmp/go-build -gno-record-gcc-switches -fno-common'

What did you do?

Wrote and ran this test

package bug

import (
	"slices"
	"testing"
)

func Bug() string {
	var ret string
	prefixes := []string{"prefix"}
	stringValues := []string{"foo", "bar", "baz"}
	for prefix := range slices.Values(prefixes) {
		var joined string
		for stringValue := range slices.Values(stringValues) {
			if joined == "" {
				joined = stringValue
			} else {
				joined = joined + "_" + stringValue
			}
		}
		ret = prefix + "+" + joined
	}
	return ret
}

func Test_Bug(t *testing.T) {
	got := Bug()
	want := "prefix+foo_bar_baz"
	if got != want {
		t.Errorf("Bug() = %v, want %v", got, want)
	}
}

What did you see happen?

=== RUN   Test_Bug
    bug_test.go:30: Bug() = prefix+foo_bar_IJ�, want prefix+foo_bar_baz
--- FAIL: Test_Bug (0.00s)

FAIL

What did you expect to see?

I expected not to see a result that contained uninitialized memory - it's different every time and seems to refer to bad bytes.

[gabyhelp]

Related Issues and Documentation

• iter: `iter.Seq` returns incorrect values for subsequent next calls #64646 (closed)
• cmd/compile: ICE compiling rangefunc for < 1.23 in combination with Cgo #67497 (closed)
• memory corruption while using appending to a slice across multiple goroutines #37484 (closed)
• go/types, types2: scope for `:=`-defined range iteration variables is incorrect #51437 (closed)
• Bug with []byte struct field at append slice in for range loop #35340 (closed)
• cmd/compile: ranging exclusively values over slices and arrays that are not cheaply indexable keeps alive 3 registers instead of 2 #61629 (closed)
• cmd/compile: fix unsafe.{SliceData,StringData} escape analysis memory corruption
• affected/package: #57101 (closed)
• fatal error: found bad pointer in Go heap AND fatal error: bad map state #41141 (closed)
• strings.Split a empty string, and len([]string) equal 1 #39806 (closed)

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

[prattmic]

cc @golang/compiler

[dmitris]

Playground version: https://go.dev/play/p/SrpUKjbzHq4
If you uncomment any of the two fmt.Println(joined) lines, it works as expected (ex. https://go.dev/play/p/sCV-WRD9S5y), same if using Sprintf instead of combining strings with +.

[scr-oath]

Curiously enough, changing

joined = joined + "_" + stringValue

to

joined += "_" + stringValue

Seems to avoid this problem - maybe that's a clue?

[fengyoulin]

It seems that currently nested rangefunc, function inline and escape analysis do not work well together.

I can reproduce this problem on the master branch code using the following code:

package main

import (
	"fmt"
	"slices"
)

func Bug(s1, s2, s3 []string) string {
	var c1 string
	for v1 := range slices.Values(s1) {
		var c2 string
		for v2 := range slices.Values(s2) {
			var c3 string
			for v3 := range slices.Values(s3) {
				c3 = c3 + v3
			}
			c2 = c2 + v2 + c3
		}
		c1 = c1 + v1 + c2
	}
	return c1
}

func main() {
	got := Bug([]string{"1", "2", "3"}, []string{"a", "b", "c"}, []string{"A", "B", "C"})
	want := "1aABCbABCcABC2aABCbABCcABC3aABCbABCcABC"
	if got != want {
		fmt.Println(fmt.Errorf("got %v, want %v", got, want))
	}
}

But, you can avoid this problem, by -gcflags='-l'.
Some calls to runtime.concatstring3 use on stack buffers, that should have been moved to the heap.

[dr2chase]

I suspect this involves outlives and containsClosure in escape/solve.go and I am looking there.

[cuonglm]

I suspect this involves outlives and containsClosure in escape/solve.go and I am looking there.

This seems to be the nested rangefunc name generation. For example, the parent is Bug-range1, the child is Bug-range2, and with current rule, Bug-range1 does not contain Bug-range2, while it should.

[dr2chase]

I think I have a fix, sorry.

I just make an explicit parent pointer, initialize it in deadlocals, use that.

[gopherbot]

Change https://go.dev/cl/622715 mentions this issue: cmd/compile: fix wrong escape analysis for nested rangefunc

[cuonglm]

I think I have a fix, sorry.

I just make an explicit parent pointer, initialize it in deadlocals, use that.

Ops, I have sent https://go-review.googlesource.com/c/go/+/622715.

Feel free to abadon if you prefer your fix.

[gopherbot]

Change https://go.dev/cl/622656 mentions this issue: cmd/compile: use a non-fragile test for "does f contain closure c?"

[cuonglm]

@dr2chase @randall77 Should we backport this?

[zigo101]

@dmitshur

[randall77]

Probably.
@gopherbot Please open a backport issue to 1.23.

This causes a serious miscompilation in certain range-over-func situations.

[gopherbot]

Backport issue(s) opened: #70198 (for 1.23).

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