Skip to content

Commit

Permalink
slices: prevent Clone keeping alive the array when cloning empty slices
Browse files Browse the repository at this point in the history
Fixes golang#68488

Change-Id: I39aba22cdfe8ca0bbe69db7c64f1bca75fa067fa
Reviewed-on: https://go-review.googlesource.com/c/go/+/598875
Reviewed-by: Keith Randall <[email protected]>
Commit-Queue: Ian Lance Taylor <[email protected]>
Reviewed-by: Keith Randall <[email protected]>
LUCI-TryBot-Result: Go LUCI <[email protected]>
Auto-Submit: Ian Lance Taylor <[email protected]>
Reviewed-by: Ian Lance Taylor <[email protected]>
  • Loading branch information
Jorropo authored and gopherbot committed Sep 29, 2024
1 parent eb6f2c2 commit 2bffb8b
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 2 deletions.
9 changes: 7 additions & 2 deletions src/slices/slices.go
Original file line number Diff line number Diff line change
Expand Up @@ -346,8 +346,13 @@ func Replace[S ~[]E, E any](s S, i, j int, v ...E) S {
// The elements are copied using assignment, so this is a shallow clone.
// The result may have additional unused capacity.
func Clone[S ~[]E, E any](s S) S {
// The s[:0:0] preserves nil in case it matters.
return append(s[:0:0], s...)
// Preserve nilness in case it matters.
if s == nil {
return nil
}
// Avoid s[:0:0] as it leads to unwanted liveness when cloning a
// zero-length slice of a large array; see https://go.dev/issue/68488.
return append(S{}, s...)
}

// Compact replaces consecutive runs of equal elements with a single copy.
Expand Down
10 changes: 10 additions & 0 deletions src/slices/slices_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
. "slices"
"strings"
"testing"
"unsafe"
)

var equalIntTests = []struct {
Expand Down Expand Up @@ -1450,3 +1451,12 @@ func TestRepeatPanics(t *testing.T) {
}
}
}

func TestIssue68488(t *testing.T) {
s := make([]int, 3)
clone := Clone(s[1:1])
switch unsafe.SliceData(clone) {
case &s[0], &s[1], &s[2]:
t.Error("clone keeps alive s due to array overlap")
}
}

0 comments on commit 2bffb8b

Please sign in to comment.