Simpler reproducer:

func main() {
	num := "765"
	maxSymbol := ""
	for i := 0; i < len(num); i++ {
		newMax := max(string(num[i]), maxSymbol)
		fmt.Printf("max(%q, %q)=%q\n", string(num[i]), maxSymbol, newMax)
		maxSymbol = newMax
	}
}

$ go run main.go
max("7", "")="7"
max("6", "6")="6"
max("5", "5")="5"

Making a tmp variable fixes this issue.

func main() {
	num := "765"
	maxSymbol := ""
	for i := 0; i < len(num); i++ {
		tmp := string(num[i])
		newMax := max(tmp, maxSymbol)
		fmt.Printf("max(%q, %q)=%q\n", tmp, maxSymbol, newMax)
		maxSymbol = newMax
	}
}

$ go run main.go
max("7", "")="7"
max("6", "7")="7"
max("5", "7")="7"

EDIT: The same thing happens with min.

CC @golang/compiler

Note that the commit title is not accurate. string(num[i]) is not a string(rune) conversion but rather a string(byte) conversion.

This appears to be an escape analysis issue.

@gopherbot Please backport to Go 1.21. This is a silent miscompilation / memory corruption issue.

Backport issue(s) opened: #64567 (for 1.21).

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

I find it weird that in this reproducer:

func main() {
	num := "765"
	maxSymbol := ""
	for i := 0; i < len(num); i++ {
		newMax := max(string(num[i]), maxSymbol)
		fmt.Printf("max(%q, %q)=%q\n", string(num[i]), maxSymbol, newMax)
		maxSymbol = newMax
	}
}

If you stop a debugger in the newMax assignment line where i == 1, maxSymbol has a value of 7, if you step to the next line maxSymbol suddenly has a value of 6.

Change https://go.dev/cl/547715 mentions this issue: cmd/compile: fix escape analysis of string min/max

Thanks all for the report. FWIW, since there's some interest in the issue, here's an explanation of what's going on.

Starting from this reproducer:

package main

import "fmt"

func main() {
	m := "0"
	for _, c := range "321" {
		m = max(string(c), m)
		fmt.Println(m)
	}
}

The compiler transforms this into basically:

func main() {
        m := "0"
	for _, c := range "321" {
                var tmp = intstring(new([4]byte), int64(c))
		m = strmax(tmp, m)
		fmt.Println(m)
	}
}

where intstring and strmax are provided by the runtime.

However, the compiler was not correctly taking into consideration that strmax returns one of its arguments. That is, it didn't realize m could point to the memory allocated by new([4]byte). As a result, it mistakenly thought it was safe to reuse that memory allocation across loop iterations, so it further transformed the code into:

func main() {
        m := "0"
        var buf [4]byte
	for _, c := range "321" {
                var tmp = intstring(&buf, int64(c)) // BAD: reuses buf, even though m may refer to it
		m = strmax(tmp, m)
		fmt.Println(m)
	}
}

Thank you for taking the time to provide such a detailed explanation, much appreciated!

Change https://go.dev/cl/547757 mentions this issue: [release-branch.go1.21] cmd/compile: fix escape analysis of string min/max