Repository: cilium/ebpf
Pull request #989 (2023-03-28 14:36:17)

btf: remove type alias for []Type
Remove "type types []Type" since the only use case is the ByID method, which doesn't take split BTF into account. Use *Spec instead where necessary.

lmb: Removed the drive-by fix, I'll open a separate PR.

Patch:
diff --git a/btf/btf.go b/btf/btf.go
index c512502..96fd21e 100644
--- a/btf/btf.go
+++ b/btf/btf.go
@@ -111,7 +111,7 @@ func LoadSpecFromReader(rd io.ReaderAt) (*Spec, error) {
 	file, err := internal.NewSafeELFFile(rd)
 	if err != nil {
 		if bo := guessRawBTFByteOrder(rd); bo != nil {
-			return loadRawSpec(io.NewSectionReader(rd, 0, math.MaxInt64), bo, nil, nil)
+			return loadRawSpec(io.NewSectionReader(rd, 0, math.MaxInt64), bo, nil)
 		}
 
 		return nil, err
@@ -135,7 +135,7 @@ func LoadSpecAndExtInfosFromReader(rd io.ReaderAt) (*Spec, *ExtInfos, error) {
 		return nil, nil, err
 	}
 
-	extInfos, err := loadExtInfosFromELF(file, spec.types, spec.strings)
+	extInfos, err := loadExtInfosFromELF(file, spec)
 	if err != nil && !errors.Is(err, ErrNotFound) {
 		return nil, nil, err
 	}
@@ -215,7 +215,7 @@ func loadSpecFromELF(file *internal.SafeELFFile) (*Spec, error) {
 		return nil, fmt.Errorf("compressed BTF is not supported")
 	}
 
-	spec, err := loadRawSpec(btfSection.ReaderAt, file.ByteOrder, nil, nil)
+	spec, err := loadRawSpec(btfSection.ReaderAt, file.ByteOrder, nil)
 	if err != nil {
 		return nil, err
 	}
@@ -228,8 +228,19 @@ func loadSpecFromELF(file *internal.SafeELFFile) (*Spec, error) {
 	return spec, nil
 }
 
-func loadRawSpec(btf io.ReaderAt, bo binary.ByteOrder,
-	baseTypes types, baseStrings *stringTable) (*Spec, error) {
+func loadRawSpec(btf io.ReaderAt, bo binary.ByteOrder, base *Spec) (*Spec, error) {
+	var (
+		baseStrings *stringTable
+		baseTypes   []Type
+		firstTypeID TypeID
+	)
+
+	if base != nil {
+		baseStrings = base.strings
+		baseTypes = base.types
+		firstTypeID = TypeID(len(baseTypes))
+	}
+
 	rawTypes, rawStrings, err := parseBTF(btf, bo, baseStrings)
 	if err != nil {
 		return nil, err
@@ -240,7 +251,7 @@ func loadRawSpec(btf io.ReaderAt, bo binary.ByteOrder,
 		return nil, err
 	}
 
-	typeIDs, typesByName := indexTypes(types, TypeID(len(baseTypes)))
+	typeIDs, typesByName := indexTypes(types, firstTypeID)
 
 	return &Spec{
 		namedTypes:  typesByName,
@@ -333,7 +344,7 @@ func loadKernelSpec() (_ *Spec, fallback bool, _ error) {
 	if err == nil {
 		defer fh.Close()
 
-		spec, err := loadRawSpec(fh, internal.NativeEndian, nil, nil)
+		spec, err := loadRawSpec(fh, internal.NativeEndian, nil)
 		return spec, false, err
 	}
 
@@ -746,7 +757,7 @@ func LoadSplitSpecFromReader(r io.ReaderAt, base *Spec) (*Spec, error) {
 		return nil, fmt.Errorf("parse split BTF: base must be loaded from an ELF")
 	}
 
-	return loadRawSpec(r, internal.NativeEndian, base.types, base.strings)
+	return loadRawSpec(r, internal.NativeEndian, base)
 }
 
 // TypesIterator iterates over types of a given spec.
diff --git a/btf/ext_info.go b/btf/ext_info.go
index c647ade..5179cfe 100644
--- a/btf/ext_info.go
+++ b/btf/ext_info.go
@@ -24,7 +24,7 @@ type ExtInfos struct {
 // loadExtInfosFromELF parses ext infos from the .BTF.ext section in an ELF.
 //
 // Returns an error wrapping ErrNotFound if no ext infos are present.
-func loadExtInfosFromELF(file *internal.SafeELFFile, ts types, strings *stringTable) (*ExtInfos, error) {
+func loadExtInfosFromELF(file *internal.SafeELFFile, spec *Spec) (*ExtInfos, error) {
 	section := file.Section(".BTF.ext")
 	if section == nil {
 		return nil, fmt.Errorf("btf ext infos: %w", ErrNotFound)
@@ -34,11 +34,11 @@ func loadExtInfosFromELF(file *internal.SafeELFFile, ts types, strings *stringTa
 		return nil, fmt.Errorf("compressed ext_info is not supported")
 	}
 
-	return loadExtInfos(section.ReaderAt, file.ByteOrder, ts, strings)
+	return loadExtInfos(section.ReaderAt, file.ByteOrder, spec, spec.strings)
 }
 
 // loadExtInfos parses bare ext infos.
-func loadExtInfos(r io.ReaderAt, bo binary.ByteOrder, ts types, strings *stringTable) (*ExtInfos, error) {
+func loadExtInfos(r io.ReaderAt, bo binary.ByteOrder, spec *Spec, strings *stringTable) (*ExtInfos, error) {
 	// Open unbuffered section reader. binary.Read() calls io.ReadFull on
 	// the header structs, resulting in one syscall per header.
 	headerRd := io.NewSectionReader(r, 0, math.MaxInt64)
@@ -60,7 +60,7 @@ func loadExtInfos(r io.ReaderAt, bo binary.ByteOrder, ts types, strings *stringT
 
 	funcInfos := make(map[string][]funcInfo, len(btfFuncInfos))
 	for section, bfis := range btfFuncInfos {
-		funcInfos[section], err = newFuncInfos(bfis, ts)
+		funcInfos[section], err = newFuncInfos(bfis, spec)
 		if err != nil {
 			return nil, fmt.Errorf("section %s: func infos: %w", section, err)
 		}
@@ -93,7 +93,7 @@ func loadExtInfos(r io.ReaderAt, bo binary.ByteOrder, ts types, strings *stringT
 
 	coreRelos := make(map[string][]coreRelocationInfo, len(btfCORERelos))
 	for section, brs := range btfCORERelos {
-		coreRelos[section], err = newRelocationInfos(brs, ts, strings)
+		coreRelos[section], err = newRelocationInfos(brs, spec, strings)
 		if err != nil {
 			return nil, fmt.Errorf("section %s: CO-RE relocations: %w", section, err)
 		}
@@ -345,8 +345,8 @@ type bpfFuncInfo struct {
 	TypeID  TypeID
 }
 
-func newFuncInfo(fi bpfFuncInfo, ts types) (*funcInfo, error) {
-	typ, err := ts.ByID(fi.TypeID)
+func newFuncInfo(fi bpfFuncInfo, spec *Spec) (*funcInfo, error) {
+	typ, err := spec.TypeByID(fi.TypeID)
 	if err != nil {
 		return nil, err
 	}
@@ -367,10 +367,10 @@ func newFuncInfo(fi bpfFuncInfo, ts types) (*funcInfo, error) {
 	}, nil
 }
 
-func newFuncInfos(bfis []bpfFuncInfo, ts types) ([]funcInfo, error) {
+func newFuncInfos(bfis []bpfFuncInfo, spec *Spec) ([]funcInfo, error) {
 	fis := make([]funcInfo, 0, len(bfis))
 	for _, bfi := range bfis {
-		fi, err := newFuncInfo(bfi, ts)
+		fi, err := newFuncInfo(bfi, spec)
 		if err != nil {
 			return nil, fmt.Errorf("offset %d: %w", bfi.InsnOff, err)
 		}
@@ -670,8 +670,8 @@ type coreRelocationInfo struct {
 	offset asm.RawInstructionOffset
 }
 
-func newRelocationInfo(relo bpfCORERelo, ts types, strings *stringTable) (*coreRelocationInfo, error) {
-	typ, err := ts.ByID(relo.TypeID)
+func newRelocationInfo(relo bpfCORERelo, spec *Spec, strings *stringTable) (*coreRelocationInfo, error) {
+	typ, err := spec.TypeByID(relo.TypeID)
 	if err != nil {
 		return nil, err
 	}
@@ -697,10 +697,10 @@ func newRelocationInfo(relo bpfCORERelo, ts types, strings *stringTable) (*coreR
 	}, nil
 }
 
-func newRelocationInfos(brs []bpfCORERelo, ts types, strings *stringTable) ([]coreRelocationInfo, error) {
+func newRelocationInfos(brs []bpfCORERelo, spec *Spec, strings *stringTable) ([]coreRelocationInfo, error) {
 	rs := make([]coreRelocationInfo, 0, len(brs))
 	for _, br := range brs {
-		relo, err := newRelocationInfo(br, ts, strings)
+		relo, err := newRelocationInfo(br, spec, strings)
 		if err != nil {
 			return nil, fmt.Errorf("offset %d: %w", br.InsnOff, err)
 		}
diff --git a/btf/handle.go b/btf/handle.go
index b85ed24..87ec2d7 100644
--- a/btf/handle.go
+++ b/btf/handle.go
@@ -120,7 +120,7 @@ func (h *Handle) Spec() (*Spec, error) {
 	}
 
 	if !h.needsKernelBase {
-		return loadRawSpec(bytes.NewReader(btfBuffer), internal.NativeEndian, nil, nil)
+		return loadRawSpec(bytes.NewReader(btfBuffer), internal.NativeEndian, nil)
 	}
 
 	base, fallback, err := kernelSpec()
@@ -133,7 +133,7 @@ func (h *Handle) Spec() (*Spec, error) {
 		return nil, fmt.Errorf("can't load split BTF without access to /sys")
 	}
 
-	return loadRawSpec(bytes.NewReader(btfBuffer), internal.NativeEndian, base.types, base.strings)
+	return loadRawSpec(bytes.NewReader(btfBuffer), internal.NativeEndian, base)
 }
 
 // Close destroys the handle.
diff --git a/btf/types.go b/btf/types.go
index 08d59a3..3b73363 100644
--- a/btf/types.go
+++ b/btf/types.go
@@ -66,18 +66,6 @@ var (
 	_ Type = (*cycle)(nil)
 )
 
-// types is a list of Type.
-//
-// The order determines the ID of a type.
-type types []Type
-
-func (ts types) ByID(id TypeID) (Type, error) {
-	if int(id) > len(ts) {
-		return nil, fmt.Errorf("type ID %d: %w", id, ErrNotFound)
-	}
-	return ts[id], nil
-}
-
 // Void is the unit type of BTF.
 type Void struct{}
 
@@ -752,7 +740,7 @@ type typeDeque = internal.Deque[*Type]
 // Returns  a slice of types indexed by TypeID. Since BTF ignores compilation
 // units, multiple types may share the same name. A Type may form a cyclic graph
 // by pointing at itself.
-func inflateRawTypes(rawTypes []rawType, baseTypes types, rawStrings *stringTable) ([]Type, error) {
+func inflateRawTypes(rawTypes []rawType, baseTypes []Type, rawStrings *stringTable) ([]Type, error) {
 	types := make([]Type, 0, len(rawTypes)+1) // +1 for Void added to base types
 
 	typeIDOffset := TypeID(1) // Void is TypeID(0), so the rest starts from TypeID(1)


Test patch:
diff --git a/btf/btf_test.go b/btf/btf_test.go
index 29adc82..76bbe2e 100644
--- a/btf/btf_test.go
+++ b/btf/btf_test.go
@@ -42,7 +42,7 @@ var vmlinuxTestdata = internal.Memoize(func() (specAndRawBTF, error) {
 		return specAndRawBTF{}, err
 	}
 
-	spec, err := loadRawSpec(bytes.NewReader(b), binary.LittleEndian, nil, nil)
+	spec, err := loadRawSpec(bytes.NewReader(b), binary.LittleEndian, nil)
 	if err != nil {
 		return specAndRawBTF{}, err
 	}
@@ -260,7 +260,7 @@ func BenchmarkParseVmlinux(b *testing.B) {
 			b.Fatal(err)
 		}
 
-		if _, err := loadRawSpec(rd, binary.LittleEndian, nil, nil); err != nil {
+		if _, err := loadRawSpec(rd, binary.LittleEndian, nil); err != nil {
 			b.Fatal("Can't load BTF:", err)
 		}
 	}
diff --git a/btf/fuzz_test.go b/btf/fuzz_test.go
index cf89e40..5c4a113 100644
--- a/btf/fuzz_test.go
+++ b/btf/fuzz_test.go
@@ -26,7 +26,7 @@ func FuzzSpec(f *testing.F) {
 			t.Skip("data is too short")
 		}
 
-		spec, err := loadRawSpec(bytes.NewReader(data), internal.NativeEndian, nil, nil)
+		spec, err := loadRawSpec(bytes.NewReader(data), internal.NativeEndian, nil)
 		if err != nil {
 			if spec != nil {
 				t.Fatal("spec is not nil")
@@ -56,6 +56,8 @@ func FuzzExtInfo(f *testing.F) {
 	}
 	f.Add(buf.Bytes(), []byte("\x00foo\x00barfoo\x00"))
 
+	emptySpec := NewSpec()
+
 	f.Fuzz(func(t *testing.T, data, strings []byte) {
 		if len(data) < binary.Size(btfExtHeader{}) {
 			t.Skip("data is too short")
@@ -66,7 +68,7 @@ func FuzzExtInfo(f *testing.F) {
 			t.Skip("invalid string table")
 		}
 
-		info, err := loadExtInfos(bytes.NewReader(data), internal.NativeEndian, nil, table)
+		info, err := loadExtInfos(bytes.NewReader(data), internal.NativeEndian, emptySpec, table)
 		if err != nil {
 			if info != nil {
 				t.Fatal("info is not nil")
diff --git a/btf/marshal_test.go b/btf/marshal_test.go
index 6013045..e5c3057 100644
--- a/btf/marshal_test.go
+++ b/btf/marshal_test.go
@@ -30,7 +30,7 @@ func TestBuild(t *testing.T) {
 	var buf bytes.Buffer
 	qt.Assert(t, marshalTypes(&buf, want, nil, nil), qt.IsNil)
 
-	have, err := loadRawSpec(bytes.NewReader(buf.Bytes()), internal.NativeEndian, nil, nil)
+	have, err := loadRawSpec(bytes.NewReader(buf.Bytes()), internal.NativeEndian, nil)
 	qt.Assert(t, err, qt.IsNil, qt.Commentf("Couldn't parse BTF"))
 	qt.Assert(t, have.types, qt.DeepEquals, want)
 }
@@ -74,7 +74,7 @@ limitTypes:
 	var buf bytes.Buffer
 	qt.Assert(t, marshalTypes(&buf, types, nil, nil), qt.IsNil)
 
-	rebuilt, err := loadRawSpec(bytes.NewReader(buf.Bytes()), binary.LittleEndian, nil, nil)
+	rebuilt, err := loadRawSpec(bytes.NewReader(buf.Bytes()), binary.LittleEndian, nil)
 	qt.Assert(t, err, qt.IsNil, qt.Commentf("round tripping BTF failed"))
 
 	if n := len(rebuilt.types); n > math.MaxUint16 {
diff --git a/btf/types_test.go b/btf/types_test.go
index ddf1400..307ed2a 100644
--- a/btf/types_test.go
+++ b/btf/types_test.go
@@ -209,7 +209,7 @@ func TestTagMarshaling(t *testing.T) {
 			err := marshalTypes(&buf, []Type{&Void{}, typ}, nil, nil)
 			qt.Assert(t, err, qt.IsNil)
 
-			s, err := loadRawSpec(bytes.NewReader(buf.Bytes()), internal.NativeEndian, nil, nil)
+			s, err := loadRawSpec(bytes.NewReader(buf.Bytes()), internal.NativeEndian, nil)
 			qt.Assert(t, err, qt.IsNil)
 
 			have, err := s.TypeByID(1)