btf: fix panic when copying nil Type

Copying a nil Type panics since we unconditionally invoke Type.copy(). Fix this and adjust the tests. Signed-off-by: Lorenz Bauer <[email protected]>
cilium · Jul 23, 2024 · b689d28 · b689d28
1 parent 218b9f9
commit b689d28
Show file tree

Hide file tree

Showing 3 changed files with 28 additions and 31 deletions.
diff --git a/btf/types.go b/btf/types.go
@@ -682,6 +682,10 @@ func Copy(typ Type) Type {
 }
 
 func copyType(typ Type, ids map[Type]TypeID, copies map[Type]Type, copiedIDs map[Type]TypeID) Type {
+	if typ == nil {
+		return nil
+	}
+
 	cpy, ok := copies[typ]
 	if ok {
 		// This has been copied previously, no need to continue.

diff --git a/btf/types_test.go b/btf/types_test.go
@@ -8,6 +8,7 @@ import (
 	"reflect"
 	"testing"
 
+	"github.com/cilium/ebpf/internal/testutils"
 	"github.com/go-quicktest/qt"
 	"github.com/google/go-cmp/cmp"
 )
@@ -39,33 +40,31 @@ func TestSizeof(t *testing.T) {
 }
 
 func TestCopy(t *testing.T) {
-	_ = Copy((*Void)(nil))
+	i := &Int{Size: 4}
 
-	in := &Int{Size: 4}
-	out := Copy(in)
-
-	in.Size = 8
-	if size := out.(*Int).Size; size != 4 {
-		t.Error("Copy doesn't make a copy, expected size 4, got", size)
-	}
-
-	t.Run("cyclical", func(t *testing.T) {
-		_ = Copy(newCyclicalType(2))
+	got := Copy(&Struct{
+		Members: []Member{
+			{Name: "a", Type: i},
+			{Name: "b", Type: i},
+		},
 	})
+	members := got.(*Struct).Members
+	qt.Check(t, qt.Equals(members[0].Type.(*Int), members[1].Type.(*Int)), qt.Commentf("identity should be preserved"))
 
-	t.Run("identity", func(t *testing.T) {
-		u16 := &Int{Size: 2}
-
-		out := Copy(&Struct{
-			Members: []Member{
-				{Name: "a", Type: u16},
-				{Name: "b", Type: u16},
-			},
+	for _, test := range []struct {
+		name string
+		typ  Type
+	}{
+		{"nil", nil},
+		{"void", (*Void)(nil)},
+		{"int", i},
+		{"cyclical", newCyclicalType(2)},
+	} {
+		t.Run(test.name, func(t *testing.T) {
+			cpy := Copy(test.typ)
+			qt.Assert(t, testutils.IsDeepCopy(cpy, test.typ))
 		})
-
-		outStruct := out.(*Struct)
-		qt.Assert(t, qt.Equals(outStruct.Members[0].Type, outStruct.Members[1].Type))
-	})
+	}
 }
 
 func TestAs(t *testing.T) {

diff --git a/map.go b/map.go
@@ -102,6 +102,8 @@ func (ms *MapSpec) Copy() *MapSpec {
 
 	cpy := *ms
 	cpy.Contents = slices.Clone(cpy.Contents)
+	cpy.Key = btf.Copy(cpy.Key)
+	cpy.Value = btf.Copy(cpy.Value)
 
 	if cpy.InnerMap == ms {
 		cpy.InnerMap = &cpy
@@ -114,14 +116,6 @@ func (ms *MapSpec) Copy() *MapSpec {
 		cpy.Extra = &extra
 	}
 
-	if cpy.Key != nil {
-		cpy.Key = btf.Copy(cpy.Key)
-	}
-
-	if cpy.Value != nil {
-		cpy.Value = btf.Copy(cpy.Value)
-	}
-
 	return &cpy
 }