Repository: cilium/ebpf
Pull request #797 (2022-09-20 16:08:24)

btf: fix IntEncoding
We currently treat IntEncoding as if it was a set of flags, but the kernel and bpftool treat it like an enum. Additionally, clang doesn't generate Char encodings in the first place.

Remove the Is* methods on IntEncoding since they give users the wrong idea of how the type works. Tidy up GoFormatter to be more strict when outputting Int.

Updates #656


Patch:
diff --git a/btf/format.go b/btf/format.go
index 207d542..b1eaefe 100644
--- a/btf/format.go
+++ b/btf/format.go
@@ -119,7 +119,7 @@ func (gf *GoFormatter) writeTypeLit(typ Type, depth int) error {
 	var err error
 	switch v := skipQualifiers(typ).(type) {
 	case *Int:
-		gf.writeIntLit(v)
+		err = gf.writeIntLit(v)
 
 	case *Enum:
 		if !v.Signed {
@@ -166,19 +166,30 @@ func (gf *GoFormatter) writeTypeLit(typ Type, depth int) error {
 	return nil
 }
 
-func (gf *GoFormatter) writeIntLit(i *Int) {
-	// NB: Encoding.IsChar is ignored.
-	if i.Encoding.IsBool() && i.Size == 1 {
-		gf.w.WriteString("bool")
-		return
-	}
-
+func (gf *GoFormatter) writeIntLit(i *Int) error {
 	bits := i.Size * 8
-	if i.Encoding.IsSigned() {
+	switch i.Encoding {
+	case Bool:
+		if i.Size != 1 {
+			return fmt.Errorf("bool with size %d", i.Size)
+		}
+		gf.w.WriteString("bool")
+	case Signed:
 		fmt.Fprintf(&gf.w, "int%d", bits)
-	} else {
+	case Char:
+		if i.Size != 1 {
+			return fmt.Errorf("char with size %d", i.Size)
+		}
+		// BTF doesn't have a way to specify the signedness of a char. Assume
+		// we are dealing with unsigned, since this works nicely with []byte
+		// in Go code.
+		fallthrough
+	case Unsigned:
 		fmt.Fprintf(&gf.w, "uint%d", bits)
+	default:
+		return fmt.Errorf("can't encode %s", i.Encoding)
 	}
+	return nil
 }
 
 func (gf *GoFormatter) writeStructLit(size uint32, members []Member, depth int) error {
diff --git a/btf/types.go b/btf/types.go
index 81980e8..3e98585 100644
--- a/btf/types.go
+++ b/btf/types.go
@@ -77,36 +77,29 @@ func (v *Void) copy() Type                     { return (*Void)(nil) }
 
 type IntEncoding byte
 
+// Valid IntEncodings.
+//
+// These may look like they are flags, but they aren't.
 const (
-	Signed IntEncoding = 1 << iota
-	Char
-	Bool
+	Unsigned IntEncoding = 0
+	Signed   IntEncoding = 1
+	Char     IntEncoding = 2
+	Bool     IntEncoding = 4
 )
 
-func (ie IntEncoding) IsSigned() bool {
-	return ie&Signed != 0
-}
-
-func (ie IntEncoding) IsChar() bool {
-	return ie&Char != 0
-}
-
-func (ie IntEncoding) IsBool() bool {
-	return ie&Bool != 0
-}
-
 func (ie IntEncoding) String() string {
-	switch {
-	case ie.IsChar() && ie.IsSigned():
+	switch ie {
+	case Char:
+		// NB: There is no way to determine signedness for char.
 		return "char"
-	case ie.IsChar() && !ie.IsSigned():
-		return "uchar"
-	case ie.IsBool():
+	case Bool:
 		return "bool"
-	case ie.IsSigned():
+	case Signed:
 		return "signed"
-	default:
+	case Unsigned:
 		return "unsigned"
+	default:
+		return fmt.Sprintf("IntEncoding(%d)", byte(ie))
 	}
 }
 


Test patch:
diff --git a/btf/format_test.go b/btf/format_test.go
index 08d606d..40a4104 100644
--- a/btf/format_test.go
+++ b/btf/format_test.go
@@ -15,10 +15,7 @@ func TestGoTypeDeclaration(t *testing.T) {
 	}{
 		{&Int{Size: 1}, "type t uint8"},
 		{&Int{Size: 1, Encoding: Bool}, "type t bool"},
-		{&Int{Size: 2, Encoding: Bool}, "type t uint16"},
 		{&Int{Size: 1, Encoding: Char}, "type t uint8"},
-		{&Int{Size: 1, Encoding: Char | Signed}, "type t int8"},
-		{&Int{Size: 2, Encoding: Char}, "type t uint16"},
 		{&Int{Size: 2, Encoding: Signed}, "type t int16"},
 		{&Int{Size: 4, Encoding: Signed}, "type t int32"},
 		{&Int{Size: 8}, "type t uint64"},
@@ -223,15 +220,28 @@ func TestGoTypeDeclarationCycle(t *testing.T) {
 }
 
 func TestRejectBogusTypes(t *testing.T) {
-	var gf GoFormatter
-	_, err := gf.TypeDeclaration("t", &Struct{
-		Size: 1,
-		Members: []Member{
-			{Name: "foo", Type: &Int{Size: 2}, Offset: 0},
-		},
-	})
-	if err == nil {
-		t.Fatal("TypeDeclaration does not reject bogus struct")
+	tests := []struct {
+		typ Type
+	}{
+		{&Struct{
+			Size: 1,
+			Members: []Member{
+				{Name: "foo", Type: &Int{Size: 2}, Offset: 0},
+			},
+		}},
+		{&Int{Size: 2, Encoding: Bool}},
+		{&Int{Size: 1, Encoding: Char | Signed}},
+		{&Int{Size: 2, Encoding: Char}},
+	}
+	for _, test := range tests {
+		t.Run(fmt.Sprint(test.typ), func(t *testing.T) {
+			var gf GoFormatter
+
+			_, err := gf.TypeDeclaration("t", test.typ)
+			if err == nil {
+				t.Fatal("TypeDeclaration does not reject bogus type")
+			}
+		})
 	}
 }