Repository: cilium/ebpf
Pull request #849 (2022-11-09 12:48:13)

Set Program.name when constructing from file descriptor
When a Program is constructed from a file descriptor, only the file descriptor and the program type are set. This results in the name not being present for `.String()`. Because of this the resulting string is different depending on how the Program was constructed.

As the program info is used in `newProgramFromFD()` already anyway, it can be used for setting the name as well.



lmb: Thanks for your PR! Can you add a small test please?
aibor: I've added tests for the name and for the type right in the test function for programs by FD. Let me know if it would be better suited somewhere else.

Patch:
diff --git a/btf/core.go b/btf/core.go
index f952b65..a0d2c1f 100644
--- a/btf/core.go
+++ b/btf/core.go
@@ -251,7 +251,7 @@ func coreCalculateFixups(relos []*CORERelocation, targetSpec *Spec, targets []Ty
 		for _, relo := range relos {
 			fixup, err := coreCalculateFixup(relo, target, targetID, bo)
 			if err != nil {
-				return nil, fmt.Errorf("target %s: %w", target, err)
+				return nil, fmt.Errorf("target %s: %s: %w", target, relo.kind, err)
 			}
 			if fixup.poison || fixup.isNonExistant() {
 				score++
@@ -320,7 +320,7 @@ func coreCalculateFixup(relo *CORERelocation, target Type, targetID TypeID, bo b
 	switch relo.kind {
 	case reloTypeIDTarget, reloTypeSize, reloTypeExists:
 		if len(relo.accessor) > 1 || relo.accessor[0] != 0 {
-			return zero, fmt.Errorf("%s: unexpected accessor %v", relo.kind, relo.accessor)
+			return zero, fmt.Errorf("unexpected accessor %v", relo.accessor)
 		}
 
 		err := coreAreTypesCompatible(local, target)
@@ -328,7 +328,7 @@ func coreCalculateFixup(relo *CORERelocation, target Type, targetID TypeID, bo b
 			return poison()
 		}
 		if err != nil {
-			return zero, fmt.Errorf("relocation %s: %w", relo.kind, err)
+			return zero, err
 		}
 
 		switch relo.kind {
@@ -358,7 +358,7 @@ func coreCalculateFixup(relo *CORERelocation, target Type, targetID TypeID, bo b
 			return poison()
 		}
 		if err != nil {
-			return zero, fmt.Errorf("relocation %s: %w", relo.kind, err)
+			return zero, err
 		}
 
 		switch relo.kind {
@@ -395,7 +395,7 @@ func coreCalculateFixup(relo *CORERelocation, target Type, targetID TypeID, bo b
 			return poison()
 		}
 		if err != nil {
-			return zero, fmt.Errorf("target %s: %w", target, err)
+			return zero, err
 		}
 
 		maybeSkipValidation := func(f COREFixup, err error) (COREFixup, error) {
@@ -451,7 +451,7 @@ func coreCalculateFixup(relo *CORERelocation, target Type, targetID TypeID, bo b
 		}
 	}
 
-	return zero, fmt.Errorf("relocation %s: %w", relo.kind, ErrNotSupported)
+	return zero, ErrNotSupported
 }
 
 /* coreAccessor contains a path through a struct. It contains at least one index.
@@ -552,6 +552,10 @@ type coreField struct {
 }
 
 func (cf *coreField) adjustOffsetToNthElement(n int) error {
+	if n == 0 {
+		return nil
+	}
+
 	size, err := Sizeof(cf.Type)
 	if err != nil {
 		return err
@@ -608,6 +612,10 @@ func coreFindField(localT Type, localAcc coreAccessor, targetT Type) (coreField,
 	local := coreField{Type: localT}
 	target := coreField{Type: targetT}
 
+	if err := coreAreMembersCompatible(local.Type, target.Type); err != nil {
+		return coreField{}, coreField{}, fmt.Errorf("fields: %w", err)
+	}
+
 	// The first index is used to offset a pointer of the base type like
 	// when accessing an array.
 	if err := local.adjustOffsetToNthElement(localAcc[0]); err != nil {
@@ -618,10 +626,6 @@ func coreFindField(localT Type, localAcc coreAccessor, targetT Type) (coreField,
 		return coreField{}, coreField{}, err
 	}
 
-	if err := coreAreMembersCompatible(local.Type, target.Type); err != nil {
-		return coreField{}, coreField{}, fmt.Errorf("fields: %w", err)
-	}
-
 	var localMaybeFlex, targetMaybeFlex bool
 	for i, acc := range localAcc[1:] {
 		switch localType := local.Type.(type) {
diff --git a/prog.go b/prog.go
index 06e91e9..34ce5fb 100644
--- a/prog.go
+++ b/prog.go
@@ -400,7 +400,7 @@ func newProgramFromFD(fd *sys.FD) (*Program, error) {
 		return nil, fmt.Errorf("discover program type: %w", err)
 	}
 
-	return &Program{"", fd, "", "", info.Type}, nil
+	return &Program{"", fd, info.Name, "", info.Type}, nil
 }
 
 func (p *Program) String() string {


Test patch:
diff --git a/btf/core_test.go b/btf/core_test.go
index f98eb4f..bde875f 100644
--- a/btf/core_test.go
+++ b/btf/core_test.go
@@ -302,6 +302,13 @@ func TestCOREFindField(t *testing.T) {
 			coreAccessor{0, 0},
 			errImpossibleRelocation,
 		},
+		{
+			"unsized type",
+			bStruct, &Func{},
+			// non-zero accessor to force calculating the offset.
+			coreAccessor{1},
+			errImpossibleRelocation,
+		},
 	}
 
 	for _, test := range invalid {
diff --git a/prog_test.go b/prog_test.go
index dfc2eb1..9e1f7b4 100644
--- a/prog_test.go
+++ b/prog_test.go
@@ -617,6 +617,15 @@ func TestProgramFromFD(t *testing.T) {
 		t.Fatal(err)
 	}
 
+	// Name and type are supposed to be copied from program info.
+	if haveObjName() == nil && prog2.name != "test" {
+		t.Errorf("Expected program to have name test, got '%s'", prog2.name)
+	}
+
+	if prog2.typ != SocketFilter {
+		t.Errorf("Expected program to have type SocketFilter, got '%s'", prog2.typ)
+	}
+
 	// Both programs refer to the same fd now. Closing either of them will
 	// release the fd to the OS, which then might re-use that fd for another
 	// test. Once we close the second map we might close the re-used fd