btf,linker: Apply ignore suffix rule to kfuncs

The ultimate goal of this change is to apply the ignore suffix rule to
kfuncs. This allows a user to define multiple flavors of kfuncs as they
may appear in different kernel versions, without name collisions.
During kfunc relocation, we strip the ___... suffix before looking up
kfunc candidates in the vmlinux BTF.

This required changing the behavior of btf.Spec.AnyTypesByName, which
would internally call newEssentialName on the provided name to lookup
types but then still apply name filtering on the name with the suffix.

By moving the suffix stripping out of the btf.Spec method responsibility
the caller can decide if they want to find exact matches or if they
want to apply the ignore suffix rule.

This also surfaced a test with a wrong assumption. We were asserting
that if the kernel contained a `ambiguous` and `ambiguous___flavor` type
that we should match both during relocation. But this is not how the
ignore suffix rule works, suffixes are only stripped off of user
provided type names (those in ELF BTF), the logic should not be applied
to kernel types. So removed tests with the bad assumptions and added
a new test to assert the correct ambiguous relocation behavior.

Signed-off-by: Dylan Reimerink <dylan.reimerink@isovalent.com>

Author: dylandreimerink

btf/btf.go

@@ -408,7 +408,7 @@ func (s *Spec) TypeID(typ Type) (TypeID, error) {
 //
 // Returns an error wrapping ErrNotFound if no matching Type exists in the Spec.
 func (s *Spec) AnyTypesByName(name string) ([]Type, error) {
-	types, err := s.d.typesByName(newEssentialName(name))
+	types, err := s.d.typesByName(name)
 	if err != nil {
 		return nil, err
 	}

btf/btf_test.go

@@ -90,78 +90,13 @@ func TestAnyTypesByName(t *testing.T) {
 	testutils.Files(t, testutils.Glob(t, "testdata/relocs-*.elf"), func(t *testing.T, file string) {
 		spec := parseELFBTF(t, file)
 
-		types, err := spec.AnyTypesByName("ambiguous")
+		types, err := spec.AnyTypesByName("u")
 		if err != nil {
 			t.Fatal(err)
 		}
 
 		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")
-		if err != nil {
-			t.Fatal(err)
-		}
-
-		if len(types) != 1 {
-			t.Fatalf("expected to receive exactly 1 type from querying ambiguous flavour, got: %v", types)
-		}
-	})
-}
-
-func TestAnyTypesByNameExactMatch(t *testing.T) {
-	spec := specFromTypes(t, []Type{
-		&Int{Name: "foo___one", Size: 4},
-		&Int{Name: "foo___two", Size: 4},
-		&Int{Name: "foo", Size: 4},
-	})
-
-	types, err := spec.AnyTypesByName("foo")
-	qt.Assert(t, qt.IsNil(err))
-	qt.Assert(t, qt.HasLen(types, 1))
-	qt.Assert(t, qt.Equals(types[0].TypeName(), "foo"))
-}
-
-func TestAnyTypesByNameNoExactMatch(t *testing.T) {
-	spec := specFromTypes(t, []Type{
-		&Int{Name: "foo___flavour", Size: 4},
-	})
-
-	types, err := spec.AnyTypesByName("foo")
-	qt.Assert(t, qt.ErrorIs(err, ErrNotFound))
-	qt.Assert(t, qt.IsNil(types))
-}
-
-func TestAnyTypeByNameNoExactMatch(t *testing.T) {
-	spec := specFromTypes(t, []Type{
-		&Int{Name: "foo___flavour", Size: 4},
-	})
-
-	typ, err := spec.AnyTypeByName("foo")
-	qt.Assert(t, qt.ErrorIs(err, ErrNotFound))
-	qt.Assert(t, qt.IsNil(typ))
-}
-
-func TestTypeByNameAmbiguous(t *testing.T) {
-	testutils.Files(t, testutils.Glob(t, "testdata/relocs-*.elf"), func(t *testing.T, file string) {
-		spec := parseELFBTF(t, file)
-
-		var typ *Struct
-		if err := spec.TypeByName("ambiguous", &typ); err != nil {
-			t.Fatal(err)
-		}
-
-		if name := typ.TypeName(); name != "ambiguous" {
-			t.Fatal("expected type name 'ambiguous', got:", name)
-		}
-
-		if err := spec.TypeByName("ambiguous___flavour", &typ); err != nil {
-			t.Fatal(err)
-		}
-
-		if name := typ.TypeName(); name != "ambiguous___flavour" {
-			t.Fatal("expected type name 'ambiguous___flavour', got:", name)
+			t.Fatalf("expected to receive exactly 1 types from querying type, got: %v", types)
 		}
 	})
 }

btf/core.go

@@ -12,6 +12,7 @@ import (
 	"github.com/cilium/ebpf/asm"
 	"github.com/cilium/ebpf/internal"
 	"github.com/cilium/ebpf/internal/platform"
+	"github.com/davecgh/go-spew/spew"
 )
 
 // Code in this file is derived from libbpf, which is available under a BSD
@@ -264,7 +265,7 @@ func CORERelocate(relos []*CORERelocation, targets []*Spec, bo binary.ByteOrder,
 
 		var targetTypes []Type
 		for _, target := range targets {
-			namedTypes, err := target.d.typesByName(essentialName)
+			namedTypes, err := target.d.typesByName(string(essentialName))
 			if errors.Is(err, ErrNotFound) {
 				continue
 			} else if err != nil {
@@ -458,6 +459,7 @@ func coreCalculateFixup(relo *CORERelocation, target Type, bo binary.ByteOrder,
 		}
 
 		localField, targetField, err := coreFindField(local, relo.accessor, target)
+		spew.Dump(localField, targetField, err)
 		if errors.Is(err, errImpossibleRelocation) {
 			return poison()
 		}

btf/core_test.go

@@ -545,11 +545,6 @@ func TestCORERelocation(t *testing.T) {
 			t.Skip("No ext_infos")
 		}
 
-		errs := map[string]error{
-			"err_ambiguous":         errAmbiguousRelocation,
-			"err_ambiguous_flavour": errAmbiguousRelocation,
-		}
-
 		for section := range extInfos.Funcs {
 			name := strings.TrimPrefix(section, "socket/")
 			t.Run(name, func(t *testing.T) {
@@ -559,13 +554,6 @@ func TestCORERelocation(t *testing.T) {
 				}
 
 				fixups, err := CORERelocate(relos, []*Spec{spec}, spec.byteOrder(), spec.TypeID)
-				if want := errs[name]; want != nil {
-					if !errors.Is(err, want) {
-						t.Fatal("Expected", want, "got", err)
-					}
-					return
-				}
-
 				if err != nil {
 					t.Fatal("Can't relocate against itself:", err)
 				}
@@ -582,6 +570,60 @@ func TestCORERelocation(t *testing.T) {
 	})
 }
 
+// TestCORERelocationAmbiguous test that when the kernel contains multiple types with the same name but of different
+// layout, and the relocation result would return different values for these types, we throw a errAmbiguousRelocation
+// error.
+func TestCORERelocationAmbiguous(t *testing.T) {
+	b, err := NewBuilder(
+		[]Type{
+			&Struct{
+				Name: "ambiguous",
+				Members: []Member{
+					{Name: "a", Type: &Int{Size: 4}},
+					{Name: "b", Type: &Int{Size: 4}},
+					{Name: "c", Type: &Int{Size: 4}},
+				},
+			},
+			&Struct{
+				Name: "ambiguous",
+				Members: []Member{
+					{Name: "a", Type: &Int{Size: 4}},
+					{Name: "c", Type: &Int{Size: 4}},
+					{Name: "b", Type: &Int{Size: 4}},
+				},
+			},
+		},
+		nil,
+	)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	fakeVmlinux, err := b.Spec()
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	relos := []*CORERelocation{
+		{
+			typ: &Struct{
+				Name: "ambiguous",
+				Members: []Member{
+					{Name: "a", Type: &Int{Size: 4}},
+					{Name: "b", Type: &Int{Size: 4}},
+				},
+			},
+			kind:     reloTypeIDTarget,
+			accessor: coreAccessor{0},
+		},
+	}
+
+	_, err = CORERelocate(relos, []*Spec{fakeVmlinux}, fakeVmlinux.byteOrder(), fakeVmlinux.TypeID)
+	if !errors.Is(err, errAmbiguousRelocation) {
+		t.Fatalf("Expected errAmbiguousRelocation, got %s", err)
+	}
+}
+
 func TestCOREReloFieldSigned(t *testing.T) {
 	for _, typ := range []Type{&Int{}, &Enum{}} {
 		t.Run(fmt.Sprintf("%T with invalid target", typ), func(t *testing.T) {

btf/testdata/relocs-eb.elf

@@ -271,21 +271,3 @@ __section("socket/fields") int fields() {
 
 	return 0;
 }
-
-struct ambiguous {
-	int _1;
-	char _2;
-};
-
-struct ambiguous___flavour {
-	char _1;
-	int _2;
-};
-
-__section("socket/err_ambiguous") int err_ambiguous() {
-	return bpf_core_type_id_kernel(struct ambiguous);
-}
-
-__section("socket/err_ambiguous_flavour") int err_ambiguous_flavour() {
-	return bpf_core_type_id_kernel(struct ambiguous___flavour);
-}

btf/testdata/relocs-el.elf

@@ -233,18 +233,18 @@ func (d *decoder) typeID(typ Type) (TypeID, error) {
 	return id, nil
 }
 
-// typesByName returns all types which have the given essential name.
+// TypesByName returns all types which have the given name.
 //
 // Returns ErrNotFound if no matching Type exists.
-func (d *decoder) typesByName(name essentialName) ([]Type, error) {
+func (d *decoder) typesByName(name string) ([]Type, error) {
 	var types []Type
 	for id := range d.namedTypes.Find(string(name)) {
 		typ, err := d.typeByID(id)
 		if err != nil {
 			return nil, err
 		}
 
-		if newEssentialName(typ.TypeName()) == name {
+		if typ.TypeName() == name {
 			// Deal with hash collisions by checking against the name.
 			types = append(types, typ)
 		}

btf/testdata/relocs.c

@@ -308,7 +308,7 @@ fixups:
 		// findTargetInKernel returns [btf.ErrNotFound] if the target can't be found
 		// or if BTF is not enabled.
 		target := btf.Type((*btf.Func)(nil))
-		spec, module, err := findTargetInKernel(kfm.Func.Name, &target, cache)
+		spec, module, err := findTargetInKernel(essentialName(kfm.Func.Name), &target, cache)
 		if errors.Is(err, btf.ErrNotFound) {
 			if kfm.Binding == elf.STB_WEAK {
 				if ins.IsKfuncCall() {
@@ -344,6 +344,11 @@ fixups:
 		}
 
 		if err := btf.CheckTypeCompatibility(kfm.Func.Type, target.(*btf.Func).Type); err != nil {
+			if kfm.Binding == elf.STB_WEAK {
+				iter.Next()
+				continue
+			}
+
 			return nil, &incompatibleKfuncError{kfm.Func.Name, err}
 		}
 
@@ -569,7 +574,7 @@ func applyTypedKsymFixups(fixups []ksymFixup, cache *btf.Cache) (modules handles
 		sym, ok := symbols[varName]
 		if !ok {
 			var target *btf.Var
-			spec, module, err := findTargetInKernel(varName, &target, cache)
+			spec, module, err := findTargetInKernel(essentialName(varName), &target, cache)
 			if errors.Is(err, btf.ErrNotFound) && fixup.Binding == elf.STB_WEAK {
 				continue
 			}
@@ -602,3 +607,19 @@ func applyTypedKsymFixups(fixups []ksymFixup, cache *btf.Cache) (modules handles
 
 	return modules, nil
 }
+
+// essentialName returns name without a ___ suffix.
+//
+// When resolving types in the kernels BTF, the "ignore suffix" rule is applied where we strip the ___ suffix from
+// a name before we search for in in the kernel BTF. This allows multiple flavors of a type to be defined in a
+// program without name collisions, yet attempt to resolve multiple types.
+func essentialName(name string) string {
+	if name == "" {
+		return ""
+	}
+	lastIdx := strings.LastIndex(name, "___")
+	if lastIdx > 0 {
+		return name[:lastIdx]
+	}
+	return name
+}