Repository: cilium/ebpf
Pull request #525 (2021-12-13 11:14:04)

btf: only return exact matches from AnyTypesByName
Change AnyTypesByName so that essential names stay an implementation
detail of the btf package: we need spec.namedTypes for fast
flavour lookup, but it's not clear that we should expose the flavour
name distinction to the users.

Fix this by only returning types which have an exact name match. This
means that `AnyTypesByName("ambiguous")` does not return ambiguous___flavour.


Patch:
diff --git a/internal/btf/btf.go b/internal/btf/btf.go
index ea9e6b3..83af0ea 100644
--- a/internal/btf/btf.go
+++ b/internal/btf/btf.go
@@ -29,10 +29,6 @@ var (
 // ID represents the unique ID of a BTF object.
 type ID uint32
 
-// essentialName represents the name of a BTF type stripped of any flavor
-// suffixes after a ___ delimiter.
-type essentialname string
-
 // Spec represents decoded BTF.
 type Spec struct {
 	// Data from .BTF.
@@ -44,7 +40,7 @@ type Spec struct {
 
 	// Types indexed by essential name.
 	// Includes all struct flavors and types with the same name.
-	namedTypes map[essentialname][]Type
+	namedTypes map[essentialName][]Type
 
 	// Data from .BTF.ext.
 	funcInfos map[string]FuncInfo
@@ -516,10 +512,10 @@ func fixupDatasec(rawTypes []rawType, rawStrings stringTable, sectionSizes map[s
 func (s *Spec) Copy() *Spec {
 	types, _ := copyTypes(s.types, nil)
 
-	namedTypes := make(map[essentialname][]Type)
+	namedTypes := make(map[essentialName][]Type)
 	for _, typ := range types {
 		if name := typ.TypeName(); name != "" {
-			en := essentialName(name)
+			en := newEssentialName(name)
 			namedTypes[en] = append(namedTypes[en], typ)
 		}
 	}
@@ -632,14 +628,7 @@ func (s *Spec) TypeByID(id TypeID) (Type, error) {
 	return s.types[id], nil
 }
 
-// AnyTypesByName returns a list of BTF Types with the given name. If name
-// contains a struct flavor suffix (e.g. 'thread_struct___v46'), an exact match
-// will be performed. Otherwise, returns all types that carry the given name,
-// including all struct flavors.
-//
-// Multiple Types of the same name can exist in case multiple 'struct flavors'
-// are described in BTF info. These are used to deal with changes in kernel
-// data structures.
+// AnyTypesByName returns a list of BTF Types with the given name.
 //
 // If the BTF blob describes multiple compilation units like vmlinux, multiple
 // Types with the same name and kind can exist, but might not describe the same
@@ -647,34 +636,21 @@ func (s *Spec) TypeByID(id TypeID) (Type, error) {
 //
 // Returns an error wrapping ErrNotFound if no matching Type exists in the Spec.
 func (s *Spec) AnyTypesByName(name string) ([]Type, error) {
-	en := essentialName(name)
-
-	// Named types are indexed by essential name.
-	types, ok := s.namedTypes[en]
-	if !ok {
+	types := s.namedTypes[newEssentialName(name)]
+	if len(types) == 0 {
 		return nil, fmt.Errorf("type name %s: %w", name, ErrNotFound)
 	}
 
-	if string(en) == name {
-		// Return a copy to prevent changes to namedTypes.
-		cpy := make([]Type, len(types))
-		copy(cpy, types)
-		return cpy, nil
-	}
-
-	// If name contains a flavor suffix, require an exact match.
-	var filtered []Type
+	// Return a copy to prevent changes to namedTypes.
+	result := make([]Type, 0, len(types))
 	for _, t := range types {
+		// Match against the full name, not just the essential one
+		// in case the type being looked up is a struct flavor.
 		if t.TypeName() == name {
-			filtered = append(filtered, t)
+			result = append(result, t)
 		}
 	}
-
-	if len(filtered) == 0 {
-		return nil, fmt.Errorf("type flavor %s: %w", name, ErrNotFound)
-	}
-
-	return filtered, nil
+	return result, nil
 }
 
 // TypeByName searches for a Type with a specific name. Since multiple
@@ -714,12 +690,6 @@ func (s *Spec) TypeByName(name string, typ interface{}) error {
 			continue
 		}
 
-		// Match against the full name, not just the essential one
-		// in case the type being looked up is a struct flavor.
-		if typ.TypeName() != name {
-			continue
-		}
-
 		if candidate != nil {
 			return fmt.Errorf("type %s: multiple candidates for %T", name, typ)
 		}
diff --git a/internal/btf/core.go b/internal/btf/core.go
index 8624cf9..9590830 100644
--- a/internal/btf/core.go
+++ b/internal/btf/core.go
@@ -240,7 +240,7 @@ func coreRelocate(local, target *Spec, relos CoreRelos) (COREFixups, error) {
 		}
 
 		relos := relosByID[id]
-		targets := target.namedTypes[essentialName(localTypeName)]
+		targets := target.namedTypes[newEssentialName(localTypeName)]
 		fixups, err := coreCalculateFixups(localType, targets, relos)
 		if err != nil {
 			return nil, fmt.Errorf("relocate %s: %w", localType, err)
@@ -704,9 +704,9 @@ func coreFindEnumValue(local Type, localAcc coreAccessor, target Type) (localVal
 		return nil, nil, errImpossibleRelocation
 	}
 
-	localName := essentialName(localValue.Name)
+	localName := newEssentialName(localValue.Name)
 	for i, targetValue := range targetEnum.Values {
-		if essentialName(targetValue.Name) != localName {
+		if newEssentialName(targetValue.Name) != localName {
 			continue
 		}
 
@@ -831,7 +831,7 @@ func coreAreMembersCompatible(localType Type, targetType Type) error {
 			return nil
 		}
 
-		if essentialName(a) == essentialName(b) {
+		if newEssentialName(a) == newEssentialName(b) {
 			return nil
 		}
 
diff --git a/internal/btf/types.go b/internal/btf/types.go
index c294170..a6b5a10 100644
--- a/internal/btf/types.go
+++ b/internal/btf/types.go
@@ -763,7 +763,7 @@ func (dq *typeDeque) all() []*Type {
 // Returns a map of named types (so, where NameOff is non-zero) and 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, rawStrings stringTable) (types []Type, namedTypes map[essentialname][]Type, err error) {
+func inflateRawTypes(rawTypes []rawType, rawStrings stringTable) (types []Type, namedTypes map[essentialName][]Type, err error) {
 	type fixupDef struct {
 		id           TypeID
 		expectedKind btfKind
@@ -802,7 +802,7 @@ func inflateRawTypes(rawTypes []rawType, rawStrings stringTable) (types []Type,
 
 	types = make([]Type, 0, len(rawTypes))
 	types = append(types, (*Void)(nil))
-	namedTypes = make(map[essentialname][]Type)
+	namedTypes = make(map[essentialName][]Type)
 
 	for i, raw := range rawTypes {
 		var (
@@ -946,7 +946,7 @@ func inflateRawTypes(rawTypes []rawType, rawStrings stringTable) (types []Type,
 
 		types = append(types, typ)
 
-		if name := essentialName(typ.TypeName()); name != "" {
+		if name := newEssentialName(typ.TypeName()); name != "" {
 			namedTypes[name] = append(namedTypes[name], typ)
 		}
 	}
@@ -973,16 +973,20 @@ func inflateRawTypes(rawTypes []rawType, rawStrings stringTable) (types []Type,
 	return types, namedTypes, nil
 }
 
-// essentialName returns name without a ___ suffix.
+// essentialName represents the name of a BTF type stripped of any flavor
+// suffixes after a ___ delimiter.
+type essentialName string
+
+// newEssentialName returns name without a ___ suffix.
 //
 // CO-RE has the concept of 'struct flavors', which are used to deal with
 // changes in kernel data structures. Anything after three underscores
 // in a type name is ignored for the purpose of finding a candidate type
 // in the kernel's BTF.
-func essentialName(name string) essentialname {
+func newEssentialName(name string) essentialName {
 	lastIdx := strings.LastIndex(name, "___")
 	if lastIdx > 0 {
-		return essentialname(name[:lastIdx])
+		return essentialName(name[:lastIdx])
 	}
-	return essentialname(name)
+	return essentialName(name)
 }


Test patch:
diff --git a/internal/btf/btf_test.go b/internal/btf/btf_test.go
index ea2b2d3..f4e881d 100644
--- a/internal/btf/btf_test.go
+++ b/internal/btf/btf_test.go
@@ -58,8 +58,8 @@ func TestAnyTypesByName(t *testing.T) {
 			t.Fatal(err)
 		}
 
-		if len(types) != 2 {
-			t.Fatalf("expected to receive exactly 2 types from querying ambiguous type, got: %v", types)
+		if len(types) != 1 {
+			t.Fatalf("expected to receive exactly 1 types from querying ambiguous type, got: %v", types)
 		}
 
 		types, err = spec.AnyTypesByName("ambiguous___flavour")