Repository: cilium/ebpf
Pull request #781 (2022-09-01 16:47:50)

btf: handle signed enums and fix default signedness
btf: handle signed enums and fix default signedness

    Enums now carry signedness encoded in the KindFlag, with the default being
    an unsigned enum. This breaks our assumption that enums are signed by default.

    Add Enum.Signed and switch EnumValue.Value to uint64. I used uint64 instead of
    uint32 since we'll likely need 64 bits to support BTF_KIND_ENUM64 in the future.

    Users of bpf2go that generated enums will see their generated types changing from
    signed to unsigned types.

    Updates #780

btf: reject invalid structs when formatting

    GoFormatter currently outputs nonsensical padding given a struct definition
    like the following:

        &Struct{
            Size: 1,
            Members: []Member{
                 {Name: "foo", Type: &Int{Size: 2}, Offset: 0},
            },
        }

    The reason is that the member is large than the enclosing struct. Return
    an error in such cases instead of formatting a bogus type.

btf: fix the output of GoFormatter for small enums

    When adding support for sized enums I only adjusted the code that
    writes out a type declaration, but forgot to change the output
    of enum type literals. This would lead to the wrong type being used
    for an enum that is used as a struct field.

    Fix this by moving the code from writeTypeDecl to writeTypeLit.

lmb: Source: https://patchwork.kernel.org/project/netdevbpf/patch/20220607062600.3716578-1-yhs@fb.com/

Patch:
diff --git a/btf/btf_types.go b/btf/btf_types.go
index 1ab0917..d64916d 100644
--- a/btf/btf_types.go
+++ b/btf/btf_types.go
@@ -237,7 +237,7 @@ type btfVariable struct {
 
 type btfEnum struct {
 	NameOff uint32
-	Val     int32
+	Val     uint32
 }
 
 type btfParam struct {
diff --git a/btf/format.go b/btf/format.go
index 5769565..207d542 100644
--- a/btf/format.go
+++ b/btf/format.go
@@ -63,39 +63,25 @@ func (gf *GoFormatter) writeTypeDecl(name string, typ Type) error {
 		return fmt.Errorf("need a name for type %s", typ)
 	}
 
-	switch v := skipQualifiers(typ).(type) {
-	case *Enum:
-		fmt.Fprintf(&gf.w, "type %s ", name)
-		switch v.Size {
-		case 1:
-			gf.w.WriteString("int8")
-		case 2:
-			gf.w.WriteString("int16")
-		case 4:
-			gf.w.WriteString("int32")
-		case 8:
-			gf.w.WriteString("int64")
-		default:
-			return fmt.Errorf("%s: invalid enum size %d", typ, v.Size)
-		}
-
-		if len(v.Values) == 0 {
-			return nil
-		}
-
-		gf.w.WriteString("; const ( ")
-		for _, ev := range v.Values {
-			id := gf.enumIdentifier(name, ev.Name)
-			fmt.Fprintf(&gf.w, "%s %s = %d; ", id, name, ev.Value)
-		}
-		gf.w.WriteString(")")
+	typ = skipQualifiers(typ)
+	fmt.Fprintf(&gf.w, "type %s ", name)
+	if err := gf.writeTypeLit(typ, 0); err != nil {
+		return err
+	}
 
+	e, ok := typ.(*Enum)
+	if !ok || len(e.Values) == 0 {
 		return nil
+	}
 
-	default:
-		fmt.Fprintf(&gf.w, "type %s ", name)
-		return gf.writeTypeLit(v, 0)
+	gf.w.WriteString("; const ( ")
+	for _, ev := range e.Values {
+		id := gf.enumIdentifier(name, ev.Name)
+		fmt.Fprintf(&gf.w, "%s %s = %d; ", id, name, ev.Value)
 	}
+	gf.w.WriteString(")")
+
+	return nil
 }
 
 // writeType outputs the name of a named type or a literal describing the type.
@@ -136,7 +122,21 @@ func (gf *GoFormatter) writeTypeLit(typ Type, depth int) error {
 		gf.writeIntLit(v)
 
 	case *Enum:
-		gf.w.WriteString("int32")
+		if !v.Signed {
+			gf.w.WriteRune('u')
+		}
+		switch v.Size {
+		case 1:
+			gf.w.WriteString("int8")
+		case 2:
+			gf.w.WriteString("int16")
+		case 4:
+			gf.w.WriteString("int32")
+		case 8:
+			gf.w.WriteString("int64")
+		default:
+			err = fmt.Errorf("invalid enum size %d", v.Size)
+		}
 
 	case *Typedef:
 		err = gf.writeType(v.Type, depth)
@@ -199,11 +199,15 @@ func (gf *GoFormatter) writeStructLit(size uint32, members []Member, depth int)
 			gf.writePadding(n)
 		}
 
-		size, err := Sizeof(m.Type)
+		fieldSize, err := Sizeof(m.Type)
 		if err != nil {
 			return fmt.Errorf("field %d: %w", i, err)
 		}
-		prevOffset = offset + uint32(size)
+
+		prevOffset = offset + uint32(fieldSize)
+		if prevOffset > size {
+			return fmt.Errorf("field %d of size %d exceeds type size %d", i, fieldSize, size)
+		}
 
 		if err := gf.writeStructField(m, depth); err != nil {
 			return fmt.Errorf("field %d: %w", i, err)
diff --git a/btf/types.go b/btf/types.go
index 38ad3dd..b34aaee 100644
--- a/btf/types.go
+++ b/btf/types.go
@@ -273,7 +273,9 @@ type Member struct {
 type Enum struct {
 	Name string
 	// Size of the enum value in bytes.
-	Size   uint32
+	Size uint32
+	// True if the values should be interpreted as signed integers.
+	Signed bool
 	Values []EnumValue
 }
 
@@ -288,7 +290,7 @@ func (e *Enum) TypeName() string { return e.Name }
 // Is is not a valid Type
 type EnumValue struct {
 	Name  string
-	Value int32
+	Value uint64
 }
 
 func (e *Enum) size() uint32    { return e.Size }
@@ -993,17 +995,20 @@ func inflateRawTypes(rawTypes []rawType, baseTypes types, rawStrings *stringTabl
 		case kindEnum:
 			rawvals := raw.data.([]btfEnum)
 			vals := make([]EnumValue, 0, len(rawvals))
+			signed := raw.KindFlag()
 			for i, btfVal := range rawvals {
 				name, err := rawStrings.Lookup(btfVal.NameOff)
 				if err != nil {
 					return nil, fmt.Errorf("get name for enum value %d: %s", i, err)
 				}
-				vals = append(vals, EnumValue{
-					Name:  name,
-					Value: btfVal.Val,
-				})
+				value := uint64(btfVal.Val)
+				if signed {
+					// Sign extend values to 64 bit.
+					value = uint64(int32(btfVal.Val))
+				}
+				vals = append(vals, EnumValue{name, value})
 			}
-			typ = &Enum{name, raw.Size(), vals}
+			typ = &Enum{name, raw.Size(), signed, vals}
 
 		case kindForward:
 			if raw.KindFlag() {
diff --git a/internal/sys/types.go b/internal/sys/types.go
index fd2ec24..9fb6e08 100644
--- a/internal/sys/types.go
+++ b/internal/sys/types.go
@@ -6,14 +6,14 @@ import (
 	"unsafe"
 )
 
-type AdjRoomMode int32
+type AdjRoomMode uint32
 
 const (
 	BPF_ADJ_ROOM_NET AdjRoomMode = 0
 	BPF_ADJ_ROOM_MAC AdjRoomMode = 1
 )
 
-type AttachType int32
+type AttachType uint32
 
 const (
 	BPF_CGROUP_INET_INGRESS            AttachType = 0
@@ -62,7 +62,7 @@ const (
 	__MAX_BPF_ATTACH_TYPE              AttachType = 43
 )
 
-type Cmd int32
+type Cmd uint32
 
 const (
 	BPF_MAP_CREATE                  Cmd = 0
@@ -104,7 +104,7 @@ const (
 	BPF_PROG_BIND_MAP               Cmd = 35
 )
 
-type FunctionId int32
+type FunctionId uint32
 
 const (
 	BPF_FUNC_unspec                         FunctionId = 0
@@ -304,14 +304,14 @@ const (
 	__BPF_FUNC_MAX_ID                       FunctionId = 194
 )
 
-type HdrStartOff int32
+type HdrStartOff uint32
 
 const (
 	BPF_HDR_START_MAC HdrStartOff = 0
 	BPF_HDR_START_NET HdrStartOff = 1
 )
 
-type LinkType int32
+type LinkType uint32
 
 const (
 	BPF_LINK_TYPE_UNSPEC         LinkType = 0
@@ -326,7 +326,7 @@ const (
 	MAX_BPF_LINK_TYPE            LinkType = 9
 )
 
-type MapType int32
+type MapType uint32
 
 const (
 	BPF_MAP_TYPE_UNSPEC                MapType = 0
@@ -362,7 +362,7 @@ const (
 	BPF_MAP_TYPE_BLOOM_FILTER          MapType = 30
 )
 
-type ProgType int32
+type ProgType uint32
 
 const (
 	BPF_PROG_TYPE_UNSPEC                  ProgType = 0
@@ -399,7 +399,7 @@ const (
 	BPF_PROG_TYPE_SYSCALL                 ProgType = 31
 )
 
-type RetCode int32
+type RetCode uint32
 
 const (
 	BPF_OK          RetCode = 0
@@ -408,14 +408,14 @@ const (
 	BPF_LWT_REROUTE RetCode = 128
 )
 
-type SkAction int32
+type SkAction uint32
 
 const (
 	SK_DROP SkAction = 0
 	SK_PASS SkAction = 1
 )
 
-type StackBuildIdStatus int32
+type StackBuildIdStatus uint32
 
 const (
 	BPF_STACK_BUILD_ID_EMPTY StackBuildIdStatus = 0
@@ -423,13 +423,13 @@ const (
 	BPF_STACK_BUILD_ID_IP    StackBuildIdStatus = 2
 )
 
-type StatsType int32
+type StatsType uint32
 
 const (
 	BPF_STATS_RUN_TIME StatsType = 0
 )
 
-type XdpAction int32
+type XdpAction uint32
 
 const (
 	XDP_ABORTED  XdpAction = 0


Test patch:
diff --git a/btf/core_test.go b/btf/core_test.go
index 136da3d..9767578 100644
--- a/btf/core_test.go
+++ b/btf/core_test.go
@@ -209,7 +209,7 @@ func TestCOREFindEnumValue(t *testing.T) {
 		name                    string
 		local, target           Type
 		acc                     coreAccessor
-		localValue, targetValue int32
+		localValue, targetValue uint64
 	}{
 		{"a to b", a, b, coreAccessor{0}, 23, 0},
 		{"b to a", b, a, coreAccessor{1}, 123, 42},
diff --git a/btf/format_test.go b/btf/format_test.go
index 0cbe519..08d606d 100644
--- a/btf/format_test.go
+++ b/btf/format_test.go
@@ -24,8 +24,18 @@ func TestGoTypeDeclaration(t *testing.T) {
 		{&Int{Size: 8}, "type t uint64"},
 		{&Typedef{Name: "frob", Type: &Int{Size: 8}}, "type t uint64"},
 		{&Int{Size: 16}, "type t uint128"},
-		{&Enum{Values: []EnumValue{{"FOO", 32}}, Size: 4}, "type t int32; const ( tFOO t = 32; )"},
-		{&Enum{Values: []EnumValue{{"BAR", 1}}, Size: 1}, "type t int8; const ( tBAR t = 1; )"},
+		{&Enum{Values: []EnumValue{{"FOO", 32}}, Size: 4}, "type t uint32; const ( tFOO t = 32; )"},
+		{&Enum{Values: []EnumValue{{"BAR", 1}}, Size: 1, Signed: true}, "type t int8; const ( tBAR t = 1; )"},
+		{
+			&Struct{
+				Name: "enum literals",
+				Size: 2,
+				Members: []Member{
+					{Name: "enum", Type: &Enum{Values: []EnumValue{{"BAR", 1}}, Size: 2}, Offset: 0},
+				},
+			},
+			"type t struct { enum uint16; }",
+		},
 		{&Array{Nelems: 2, Type: &Int{Size: 1}}, "type t [2]uint8"},
 		{
 			&Union{
@@ -155,12 +165,12 @@ func TestGoTypeDeclarationNamed(t *testing.T) {
 		named  []Type
 		output string
 	}{
-		{e1, []Type{e1}, "type t int32"},
+		{e1, []Type{e1}, "type t uint32"},
 		{s1, []Type{e1, s1}, "type t struct { frob E1; }"},
 		{s2, []Type{e1}, "type t struct { frood struct { frob E1; }; }"},
 		{s2, []Type{e1, s1}, "type t struct { frood S1; }"},
-		{td, nil, "type t int32"},
-		{td, []Type{td}, "type t int32"},
+		{td, nil, "type t uint32"},
+		{td, []Type{td}, "type t uint32"},
 		{arr, []Type{td}, "type t [1]TD"},
 	}
 
@@ -212,6 +222,19 @@ 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")
+	}
+}
+
 func mustGoTypeDeclaration(tb testing.TB, typ Type, names map[Type]string, id func(string) string) string {
 	tb.Helper()
 
diff --git a/cmd/bpf2go/test/test_bpfeb.go b/cmd/bpf2go/test/test_bpfeb.go
index d750782..1b285e0 100644
--- a/cmd/bpf2go/test/test_bpfeb.go
+++ b/cmd/bpf2go/test/test_bpfeb.go
@@ -20,7 +20,7 @@ type testBarfoo struct {
 	Boo testE
 }
 
-type testE int32
+type testE uint32
 
 const (
 	testEHOOPY testE = 0
diff --git a/cmd/bpf2go/test/test_bpfel.go b/cmd/bpf2go/test/test_bpfel.go
index 659466c..236d310 100644
--- a/cmd/bpf2go/test/test_bpfel.go
+++ b/cmd/bpf2go/test/test_bpfel.go
@@ -20,7 +20,7 @@ type testBarfoo struct {
 	Boo testE
 }
 
-type testE int32
+type testE uint32
 
 const (
 	testEHOOPY testE = 0