Skip to content

Commit

Permalink
btf: fix panic when copying nil Type
Browse files Browse the repository at this point in the history
Copying a nil Type panics since we unconditionally invoke Type.copy().
Fix this and adjust the tests.

Signed-off-by: Lorenz Bauer <lmb@isovalent.com>
  • Loading branch information
lmb committed Jul 23, 2024
1 parent 218b9f9 commit b689d28
Show file tree
Hide file tree
Showing 3 changed files with 28 additions and 31 deletions.
4 changes: 4 additions & 0 deletions btf/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
45 changes: 22 additions & 23 deletions btf/types_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"reflect"
"testing"

"github.com/cilium/ebpf/internal/testutils"
"github.com/go-quicktest/qt"
"github.com/google/go-cmp/cmp"
)
Expand Down Expand Up @@ -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) {
Expand Down
10 changes: 2 additions & 8 deletions map.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
}

Expand Down

0 comments on commit b689d28

Please sign in to comment.