x/tools/gopls/internal/analysis/modernize: slices.Delete modernizer is unsound (clears out s[len:cap])

[gandrejczuk]

gopls version

The issue is for latest modernize

go env

See below

What did you do?

$ cat test.go
// You can edit this code!
// Click here and start typing.
package main

import (
        "fmt"
)

func p(v int) *int {
        return &v
}

func main() {
        fmt.Println("Hello, 世界")
        x := []*int{p(1), p(2), p(3), p(4)}

        for i, member := range x {
                println(i)
                if *member == 2 {
                        x = append(x[:i], x[i+1:]...)
                        // Modernize will generate following code which will panic
                        // when last elemeny will be accessed
                        // x = slices.Delete(x, i, i+1)
                }
        }

        fmt.Printf("%v\n", x)
}
$ go run test.go
Hello, 世界
0
1
2
3
[0xc0000120f0 0xc000012100 0xc000012108]
$ go run golang.org/x/tools/gopls/internal/analysis/modernize/cmd/modernize@latest -fix test.go
$ cat test.go
// You can edit this code!
// Click here and start typing.
package main

import (
        "fmt"
        "slices"
)

func p(v int) *int {
        return &v
}

func main() {
        fmt.Println("Hello, 世界")
        x := []*int{p(1), p(2), p(3), p(4)}

        for i, member := range x {
                println(i)
                if *member == 2 {
                        x = slices.Delete(x, i, i+1)
                        // Modernize will generate following code which will panic
                        // when last elemeny will be accessed
                        // x = slices.Delete(x, i, i+1)
                }
        }

        fmt.Printf("%v\n", x)
}
$ go run test.go
Hello, 世界
0
1
2
3
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x4912d7]

goroutine 1 [running]:
main.main()
        test.go:20 +0x1d7
exit status 2

What did you see happen?

Hello, 世界
0
1
2
3
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x4930b7]

goroutine 1 [running]:
main.main()
	/tmp/sandbox3797330105/prog.go:20 +0x1d7

What did you expect to see?

Hello, 世界
0
1
2
3
[0xc000010080 0xc000010090 0xc000010098]

Program exited.

Editor and settings

No response

Logs

No response

[adonovan]

Thanks for the bug report and test case. The fundamental issue here is that slices.Delete clears out the array elements between len and cap (or the new length and the old), ostensibly to aid garbage collection, whereas append does not, and without further analysis--that is I think far beyond our scope--it is impossible to tell whether they are needed.

Here's a more minimal example:

https://go.dev/play/p/a_-7zP0N-Xq

	{
		slice := []int{0, 1, 2, 3, 4, 5}
		orig := slice
		slice = append(slice[:2], slice[3:]...)
		fmt.Println(orig) // "[0 1 3 4 5 5]"
	}
	{
		slice := []int{0, 1, 2, 3, 4, 5}
		orig := slice
		slice = slices.Delete(slice, 2, 3)
		fmt.Println(orig) // "[0 1 3 4 5 0]"
	}

I propose we disable the slices.Delete modernizer. Sigh.

[gabyhelp]

Related Issues

• x/tools/gopls/internal/analysis/modernize: crash in minmax #73566 (closed)
• x/tools/gopls: invalid memory address or nil pointer dereference on highlight #68927 (closed)
• x/tools/gopls/internal/analysis/modernize: Panic in modernize/sortslice.go #71786 (closed)
• x/tools/gopls/internal/analysis/modernize: append([]T{}, x...) -> slices.Clone(x) fix may unsoundly return nil #73557
• x/tools/gopls: panic in analysis #60551 (closed)
• x/tools/gopls: SIGSEGV in cache.missingModuleError #42349 (closed)
• gopls: automated issue report (crash) #63702 (closed)
• x/tools/gopls: Highlight: nil deref panic on CompositeLiteral with no type #68918 (closed)
• x/tools/cmd/gopls: nil pointer dereference when creating new file #34366 (closed)
• runtime: memory corruption after append to huge slice #29495 (closed)

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

[gopherbot]

Change https://go.dev/cl/673516 mentions this issue: gopls/internal/analysis/modernize: disable slicesdelete pass