Repository: cilium/ebpf
Pull request #633 (2022-04-21 16:36:55)

btf: stop relying on TypeID when printing Types
btf: stop relying on TypeID when printing Types

Using the TypeID when printing is convenient, but it precludes us from
removing TypeID from the Type interface. Rework the printing code to
use the pointer address to identify a specific Type.

At the same time, make printing a lot more powerful. We now support
printing cyclical types without having to worry about infinite recursion.
For example, formatting a typedef with %v will now ouput

    Typedef:"foo"[Pointer]

We can include details of Pointer by formatting with %1v:

    Typedef:"foo"[Pointer[target=Void]]

Updates #643


Patch:
diff --git a/internal/btf/types.go b/internal/btf/types.go
index 524526f..198e601 100644
--- a/internal/btf/types.go
+++ b/internal/btf/types.go
@@ -2,6 +2,7 @@ package btf
 
 import (
 	"fmt"
+	"io"
 	"math"
 	"reflect"
 	"strings"
@@ -19,6 +20,17 @@ func (tid TypeID) ID() TypeID {
 
 // Type represents a type described by BTF.
 type Type interface {
+	// Type can be formatted using the %s and %v verbs. %s outputs only the
+	// identity of the type, without any detail. %v outputs additional detail.
+	//
+	// Use the '+' flag to include the address of the type.
+	//
+	// Use the width to specify how many levels of detail to output, for example
+	// %1v will output detail for the root type and a short description of its
+	// children. %2v would output details of the root type and its children
+	// as well as a short description of the grandchildren.
+	fmt.Formatter
+
 	// The type ID of the Type within this BTF spec.
 	ID() TypeID
 
@@ -32,8 +44,6 @@ type Type interface {
 	// Enumerate all nested Types. Repeated calls must visit nested
 	// types in the same order.
 	walk(*typeDeque)
-
-	String() string
 }
 
 var (
@@ -52,12 +62,12 @@ var (
 // Void is the unit type of BTF.
 type Void struct{}
 
-func (v *Void) ID() TypeID       { return 0 }
-func (v *Void) String() string   { return "void#0" }
-func (v *Void) TypeName() string { return "" }
-func (v *Void) size() uint32     { return 0 }
-func (v *Void) copy() Type       { return (*Void)(nil) }
-func (v *Void) walk(*typeDeque)  {}
+func (v *Void) ID() TypeID                     { return 0 }
+func (v *Void) Format(fs fmt.State, verb rune) { formatType(fs, verb, v) }
+func (v *Void) TypeName() string               { return "" }
+func (v *Void) size() uint32                   { return 0 }
+func (v *Void) copy() Type                     { return (*Void)(nil) }
+func (v *Void) walk(*typeDeque)                {}
 
 type IntEncoding byte
 
@@ -79,6 +89,21 @@ func (ie IntEncoding) IsBool() bool {
 	return ie&Bool != 0
 }
 
+func (ie IntEncoding) String() string {
+	switch {
+	case ie.IsChar() && ie.IsSigned():
+		return "char"
+	case ie.IsChar() && !ie.IsSigned():
+		return "uchar"
+	case ie.IsBool():
+		return "bool"
+	case ie.IsSigned():
+		return "signed"
+	default:
+		return "unsigned"
+	}
+}
+
 // Int is an integer of a given length.
 type Int struct {
 	TypeID
@@ -94,29 +119,15 @@ type Int struct {
 	Bits       byte
 }
 
-func (i *Int) String() string {
-	var s strings.Builder
-
-	switch {
-	case i.Encoding.IsChar():
-		s.WriteString("char")
-	case i.Encoding.IsBool():
-		s.WriteString("bool")
-	default:
-		if !i.Encoding.IsSigned() {
-			s.WriteRune('u')
-		}
-		s.WriteString("int")
-		fmt.Fprintf(&s, "%d", i.Size*8)
+func (i *Int) Format(fs fmt.State, verb rune) {
+	extra := []interface{}{
+		i.Encoding,
+		"size=", i.Size * 8,
 	}
-
-	fmt.Fprintf(&s, "#%d", i.TypeID)
-
 	if i.Bits > 0 {
-		fmt.Fprintf(&s, "[bits=%d]", i.Bits)
+		extra = append(extra, "bits=", i.Bits)
 	}
-
-	return s.String()
+	formatType(fs, verb, i, extra...)
 }
 
 func (i *Int) TypeName() string { return i.Name }
@@ -137,8 +148,8 @@ type Pointer struct {
 	Target Type
 }
 
-func (p *Pointer) String() string {
-	return fmt.Sprintf("pointer#%d[target=#%d]", p.TypeID, p.Target.ID())
+func (p *Pointer) Format(fs fmt.State, verb rune) {
+	formatType(fs, verb, p, "target=", p.Target)
 }
 
 func (p *Pointer) TypeName() string    { return "" }
@@ -156,8 +167,8 @@ type Array struct {
 	Nelems uint32
 }
 
-func (arr *Array) String() string {
-	return fmt.Sprintf("array#%d[type=#%d n=%d]", arr.TypeID, arr.Type.ID(), arr.Nelems)
+func (arr *Array) Format(fs fmt.State, verb rune) {
+	formatType(fs, verb, arr, "type=", arr.Type, "n=", arr.Nelems)
 }
 
 func (arr *Array) TypeName() string { return "" }
@@ -177,8 +188,8 @@ type Struct struct {
 	Members []Member
 }
 
-func (s *Struct) String() string {
-	return fmt.Sprintf("struct#%d[%q]", s.TypeID, s.Name)
+func (s *Struct) Format(fs fmt.State, verb rune) {
+	formatType(fs, verb, s, "fields=", len(s.Members))
 }
 
 func (s *Struct) TypeName() string { return s.Name }
@@ -210,8 +221,8 @@ type Union struct {
 	Members []Member
 }
 
-func (u *Union) String() string {
-	return fmt.Sprintf("union#%d[%q]", u.TypeID, u.Name)
+func (u *Union) Format(fs fmt.State, verb rune) {
+	formatType(fs, verb, u, "fields=", len(u.Members))
 }
 
 func (u *Union) TypeName() string { return u.Name }
@@ -267,8 +278,8 @@ type Enum struct {
 	Values []EnumValue
 }
 
-func (e *Enum) String() string {
-	return fmt.Sprintf("enum#%d[%q]", e.TypeID, e.Name)
+func (e *Enum) Format(fs fmt.State, verb rune) {
+	formatType(fs, verb, e, "values=", len(e.Values))
 }
 
 func (e *Enum) TypeName() string { return e.Name }
@@ -317,8 +328,8 @@ type Fwd struct {
 	Kind FwdKind
 }
 
-func (f *Fwd) String() string {
-	return fmt.Sprintf("fwd#%d[%s %q]", f.TypeID, f.Kind, f.Name)
+func (f *Fwd) Format(fs fmt.State, verb rune) {
+	formatType(fs, verb, f, f.Kind)
 }
 
 func (f *Fwd) TypeName() string { return f.Name }
@@ -336,8 +347,8 @@ type Typedef struct {
 	Type Type
 }
 
-func (td *Typedef) String() string {
-	return fmt.Sprintf("typedef#%d[%q #%d]", td.TypeID, td.Name, td.Type.ID())
+func (td *Typedef) Format(fs fmt.State, verb rune) {
+	formatType(fs, verb, td, td.Type)
 }
 
 func (td *Typedef) TypeName() string { return td.Name }
@@ -354,8 +365,8 @@ type Volatile struct {
 	Type Type
 }
 
-func (v *Volatile) String() string {
-	return fmt.Sprintf("volatile#%d[#%d]", v.TypeID, v.Type.ID())
+func (v *Volatile) Format(fs fmt.State, verb rune) {
+	formatType(fs, verb, v, v.Type)
 }
 
 func (v *Volatile) TypeName() string { return "" }
@@ -373,8 +384,8 @@ type Const struct {
 	Type Type
 }
 
-func (c *Const) String() string {
-	return fmt.Sprintf("const#%d[#%d]", c.TypeID, c.Type.ID())
+func (c *Const) Format(fs fmt.State, verb rune) {
+	formatType(fs, verb, c, c.Type)
 }
 
 func (c *Const) TypeName() string { return "" }
@@ -392,8 +403,8 @@ type Restrict struct {
 	Type Type
 }
 
-func (r *Restrict) String() string {
-	return fmt.Sprintf("restrict#%d[#%d]", r.TypeID, r.Type.ID())
+func (r *Restrict) Format(fs fmt.State, verb rune) {
+	formatType(fs, verb, r, r.Type)
 }
 
 func (r *Restrict) TypeName() string { return "" }
@@ -413,8 +424,8 @@ type Func struct {
 	Linkage FuncLinkage
 }
 
-func (f *Func) String() string {
-	return fmt.Sprintf("func#%d[%s %q proto=#%d]", f.TypeID, f.Linkage, f.Name, f.Type.ID())
+func (f *Func) Format(fs fmt.State, verb rune) {
+	formatType(fs, verb, f, f.Linkage, "proto=", f.Type)
 }
 
 func (f *Func) TypeName() string { return f.Name }
@@ -432,14 +443,8 @@ type FuncProto struct {
 	Params []FuncParam
 }
 
-func (fp *FuncProto) String() string {
-	var s strings.Builder
-	fmt.Fprintf(&s, "proto#%d[", fp.TypeID)
-	for _, param := range fp.Params {
-		fmt.Fprintf(&s, "%q=#%d, ", param.Name, param.Type.ID())
-	}
-	fmt.Fprintf(&s, "return=#%d]", fp.Return.ID())
-	return s.String()
+func (fp *FuncProto) Format(fs fmt.State, verb rune) {
+	formatType(fs, verb, fp, "args=", len(fp.Params), "return=", fp.Return)
 }
 
 func (fp *FuncProto) TypeName() string { return "" }
@@ -471,8 +476,8 @@ type Var struct {
 	Linkage VarLinkage
 }
 
-func (v *Var) String() string {
-	return fmt.Sprintf("var#%d[%s %q]", v.TypeID, v.Linkage, v.Name)
+func (v *Var) Format(fs fmt.State, verb rune) {
+	formatType(fs, verb, v, v.Linkage)
 }
 
 func (v *Var) TypeName() string { return v.Name }
@@ -491,8 +496,8 @@ type Datasec struct {
 	Vars []VarSecinfo
 }
 
-func (ds *Datasec) String() string {
-	return fmt.Sprintf("section#%d[%q]", ds.TypeID, ds.Name)
+func (ds *Datasec) Format(fs fmt.State, verb rune) {
+	formatType(fs, verb, ds)
 }
 
 func (ds *Datasec) TypeName() string { return ds.Name }
@@ -530,8 +535,8 @@ type Float struct {
 	Size uint32
 }
 
-func (f *Float) String() string {
-	return fmt.Sprintf("float%d#%d[%q]", f.Size*8, f.TypeID, f.Name)
+func (f *Float) Format(fs fmt.State, verb rune) {
+	formatType(fs, verb, f, "size=", f.Size*8)
 }
 
 func (f *Float) TypeName() string { return f.Name }
@@ -1053,3 +1058,91 @@ func UnderlyingType(typ Type) Type {
 	// Return the original argument, since we can't find an underlying type.
 	return typ
 }
+
+type formatState struct {
+	fmt.State
+	depth int
+}
+
+// formattableType is a subset of Type, to ease unit testing of formatType.
+type formattableType interface {
+	fmt.Formatter
+	TypeName() string
+}
+
+// formatType formats a type in a canonical form.
+//
+// Handles cyclical types by only printing cycles up to a certain depth. Elements
+// in extra are separated by spaces unless the preceding element is a string
+// ending in '='.
+func formatType(f fmt.State, verb rune, t formattableType, extra ...interface{}) {
+	if verb != 'v' && verb != 's' {
+		fmt.Fprintf(f, "{UNRECOGNIZED: %c}", verb)
+		return
+	}
+
+	// This is the same as %T, but elides the package name. Assumes that
+	// formattableType is implemented by a pointer receiver.
+	goTypeName := reflect.TypeOf(t).Elem().Name()
+	_, _ = io.WriteString(f, goTypeName)
+
+	if name := t.TypeName(); name != "" {
+		// Output BTF type name if present.
+		fmt.Fprintf(f, ":%q", name)
+	}
+
+	if f.Flag('+') {
+		// Output address if requested.
+		fmt.Fprintf(f, ":%#p", t)
+	}
+
+	if verb == 's' {
+		// %s omits details.
+		return
+	}
+
+	var depth int
+	if ps, ok := f.(*formatState); ok {
+		depth = ps.depth
+		f = ps.State
+	}
+
+	maxDepth, ok := f.Width()
+	if !ok {
+		maxDepth = 0
+	}
+
+	if depth > maxDepth {
+		// We've reached the maximum depth. This avoids infinite recursion even
+		// for cyclical types.
+		return
+	}
+
+	if len(extra) == 0 {
+		return
+	}
+
+	wantSpace := false
+	_, _ = io.WriteString(f, "[")
+	for _, arg := range extra {
+		if wantSpace {
+			_, _ = io.WriteString(f, " ")
+		}
+
+		switch v := arg.(type) {
+		case string:
+			_, _ = io.WriteString(f, v)
+			wantSpace = v[len(v)-1] != '='
+			continue
+
+		case formattableType:
+			v.Format(&formatState{f, depth + 1}, verb)
+
+		default:
+			fmt.Fprint(f, arg)
+		}
+
+		wantSpace = true
+	}
+	_, _ = io.WriteString(f, "]")
+}


Test patch:
diff --git a/internal/btf/fuzz_test.go b/internal/btf/fuzz_test.go
index 89393b4..1da3284 100644
--- a/internal/btf/fuzz_test.go
+++ b/internal/btf/fuzz_test.go
@@ -6,6 +6,8 @@ package btf
 import (
 	"bytes"
 	"encoding/binary"
+	"fmt"
+	"io"
 	"testing"
 
 	"github.com/cilium/ebpf/internal"
@@ -32,9 +34,16 @@ func FuzzSpec(f *testing.F) {
 			if spec != nil {
 				t.Fatal("spec is not nil")
 			}
-		} else if spec == nil {
+			return
+		}
+
+		if spec == nil {
 			t.Fatal("spec is nil")
 		}
+
+		for _, typ := range spec.types {
+			fmt.Fprintf(io.Discard, "%+10v", typ)
+		}
 	})
 }
 
diff --git a/internal/btf/types_test.go b/internal/btf/types_test.go
index a53f2dd..f6f3d57 100644
--- a/internal/btf/types_test.go
+++ b/internal/btf/types_test.go
@@ -2,6 +2,7 @@ package btf
 
 import (
 	"fmt"
+	"reflect"
 	"testing"
 
 	qt "github.com/frankban/quicktest"
@@ -224,6 +225,61 @@ func TestTypeDeque(t *testing.T) {
 	})
 }
 
+type testFormattableType struct {
+	name  string
+	extra []interface{}
+}
+
+var _ formattableType = (*testFormattableType)(nil)
+
+func (tft *testFormattableType) TypeName() string { return tft.name }
+func (tft *testFormattableType) Format(fs fmt.State, verb rune) {
+	formatType(fs, verb, tft, tft.extra...)
+}
+
+func TestFormatType(t *testing.T) {
+	t1 := &testFormattableType{"", []interface{}{"extra"}}
+	t1Addr := fmt.Sprintf("%#p", t1)
+	goType := reflect.TypeOf(t1).Elem().Name()
+
+	t2 := &testFormattableType{"foo", []interface{}{t1}}
+
+	tests := []struct {
+		t        formattableType
+		fmt      string
+		contains []string
+		omits    []string
+	}{
+		// %s doesn't contain address or extra.
+		{t1, "%s", []string{goType}, []string{t1Addr, "extra"}},
+		// %+s doesn't contain extra.
+		{t1, "%+s", []string{goType, t1Addr}, []string{"extra"}},
+		// %v does contain extra.
+		{t1, "%v", []string{goType, "extra"}, []string{t1Addr}},
+		// %+v does contain address.
+		{t1, "%+v", []string{goType, "extra", t1Addr}, nil},
+		// %v doesn't print nested types' extra.
+		{t2, "%v", []string{goType, t2.name}, []string{"extra"}},
+		// %1v does print nested types' extra.
+		{t2, "%1v", []string{goType, t2.name, "extra"}, nil},
+	}
+
+	for _, test := range tests {
+		t.Run(test.fmt, func(t *testing.T) {
+			str := fmt.Sprintf(test.fmt, test.t)
+			t.Log(str)
+
+			for _, want := range test.contains {
+				qt.Assert(t, str, qt.Contains, want)
+			}
+
+			for _, notWant := range test.omits {
+				qt.Assert(t, str, qt.Not(qt.Contains), notWant)
+			}
+		})
+	}
+}
+
 func newCyclicalType(n int) Type {
 	ptr := &Pointer{}
 	prev := Type(ptr)