From 218aa20a07bfac511cf9e25d47d6792d300e9814 Mon Sep 17 00:00:00 2001 From: Lorenz Bauer Date: Tue, 23 Jul 2024 11:43:20 +0100 Subject: [PATCH] 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 --- btf/types.go | 4 ++++ btf/types_test.go | 45 ++++++++++++++++++++++----------------------- map.go | 10 ++-------- 3 files changed, 28 insertions(+), 31 deletions(-) diff --git a/btf/types.go b/btf/types.go index 3cb9184f0..a3397460b 100644 --- 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 index e1a7b754f..95ddb9dae 100644 --- 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 index 94421baae..61c7d0a80 100644 --- 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 }