pkg/dwarf/frame: fix FrameDescriptionEntries's Append (#3433)

The current implementation has a bug to remove duplicates. It can be implemented by using fast-slow pointers.
go-delve · Jul 5, 2023 · f016055 · f016055
1 parent 53998cb
commit f016055
Show file tree

Hide file tree

Showing 2 changed files with 94 additions and 8 deletions.
diff --git a/pkg/dwarf/frame/entries.go b/pkg/dwarf/frame/entries.go
@@ -91,18 +91,21 @@ func (fdes FrameDescriptionEntries) Append(otherFDEs FrameDescriptionEntries) Fr
 	sort.SliceStable(r, func(i, j int) bool {
 		return r[i].Begin() < r[j].Begin()
 	})
+	if len(r) < 2 { // fast path, no duplicates
+		return r
+	}
+
 	// remove duplicates
-	uniqFDEs := fdes[:0]
-	for _, fde := range fdes {
-		if len(uniqFDEs) > 0 {
-			last := uniqFDEs[len(uniqFDEs)-1]
-			if last.Begin() == fde.Begin() && last.End() == fde.End() {
-				continue
+	slow := 1
+	for fast := 1; fast < len(r); fast++ {
+		if r[fast].Begin() != r[fast-1].Begin() || r[fast].End() != r[fast-1].End() {
+			if slow != fast {
+				r[slow] = r[fast]
 			}
+			slow++
 		}
-		uniqFDEs = append(uniqFDEs, fde)
 	}
-	return r
+	return r[:slow]
 }
 
 // ptrEnc represents a pointer encoding value, used during eh_frame decoding

diff --git a/pkg/dwarf/frame/entries_test.go b/pkg/dwarf/frame/entries_test.go
@@ -56,6 +56,89 @@ func TestFDEForPC(t *testing.T) {
 	}
 }
 
+func TestAppend(t *testing.T) {
+	equal := func(x, y FrameDescriptionEntries) bool {
+		if len(x) != len(y) {
+			return false
+		}
+		for i := range x {
+			if x[i].Begin() != y[i].Begin() || x[i].End() != y[i].End() {
+				return false
+			}
+		}
+		return true
+	}
+	var appendTests = []struct {
+		name string
+		f1   FrameDescriptionEntries
+		f2   FrameDescriptionEntries
+		want FrameDescriptionEntries
+	}{
+		{
+			name: "nil",
+			f1:   nil,
+			f2:   nil,
+			want: nil,
+		},
+
+		{
+			name: "one",
+			f1: FrameDescriptionEntries{
+				&FrameDescriptionEntry{begin: 10, size: 40},
+			},
+			f2: FrameDescriptionEntries{
+				&FrameDescriptionEntry{begin: 10, size: 40},
+			},
+			want: FrameDescriptionEntries{
+				&FrameDescriptionEntry{begin: 10, size: 40},
+			},
+		},
+		{
+			name: "1 item",
+			f1: FrameDescriptionEntries{
+				&FrameDescriptionEntry{begin: 10, size: 40},
+				&FrameDescriptionEntry{begin: 10, size: 40},
+				&FrameDescriptionEntry{begin: 50, size: 50},
+			},
+			f2: FrameDescriptionEntries{
+				&FrameDescriptionEntry{begin: 10, size: 40},
+				&FrameDescriptionEntry{begin: 50, size: 50},
+			},
+			want: FrameDescriptionEntries{
+				&FrameDescriptionEntry{begin: 10, size: 40},
+				&FrameDescriptionEntry{begin: 50, size: 50},
+			},
+		},
+		{
+			name: "many",
+			f1: FrameDescriptionEntries{
+				&FrameDescriptionEntry{begin: 10, size: 40},
+				&FrameDescriptionEntry{begin: 100, size: 100},
+				&FrameDescriptionEntry{begin: 50, size: 50},
+				&FrameDescriptionEntry{begin: 50, size: 50},
+				&FrameDescriptionEntry{begin: 300, size: 10},
+				&FrameDescriptionEntry{begin: 300, size: 10},
+			},
+			f2: FrameDescriptionEntries{
+				&FrameDescriptionEntry{begin: 10, size: 40},
+				&FrameDescriptionEntry{begin: 100, size: 100},
+				&FrameDescriptionEntry{begin: 100, size: 100},
+			},
+			want: FrameDescriptionEntries{
+				&FrameDescriptionEntry{begin: 10, size: 40},
+				&FrameDescriptionEntry{begin: 50, size: 50},
+				&FrameDescriptionEntry{begin: 100, size: 100},
+				&FrameDescriptionEntry{begin: 300, size: 10},
+			},
+		},
+	}
+	for _, test := range appendTests {
+		if got := test.f1.Append(test.f2); !equal(got, test.want) {
+			t.Errorf("%v.Append(%v) = %v, want %v", test.f1, test.f2, got, test.want)
+		}
+	}
+}
+
 func BenchmarkFDEForPC(b *testing.B) {
 	f, err := os.Open("testdata/frame")
 	if err != nil {