elf: assign Name from BTF map definitions

When definining a map using BTF-style __type, etc. macros MapSpec.Name
is not populated. The tests don't catch this since we never added a test
for these map definitions.

Set MapSpec.Name and rework the tests to use go-cmp. This means we don't
have to manually maintain the comparison logic anymore.

elf_reader.go

@@ -540,6 +540,7 @@ func mapSpecFromBTF(spec *btf.Spec, name string) (*MapSpec, error) {
 	}
 
 	return &MapSpec{
+		Name:       SanitizeName(name, -1),
 		Type:       MapType(mapType),
 		KeySize:    keySize,
 		ValueSize:  valueSize,

elf_reader_test.go

@@ -3,86 +3,105 @@ package ebpf
 import (
 	"flag"
 	"path/filepath"
-	"reflect"
 	"testing"
 
 	"github.com/cilium/ebpf/internal"
+	"github.com/cilium/ebpf/internal/btf"
 	"github.com/cilium/ebpf/internal/testutils"
+
+	"github.com/google/go-cmp/cmp"
+	"github.com/google/go-cmp/cmp/cmpopts"
 )
 
 func TestLoadCollectionSpec(t *testing.T) {
+	const BPF_F_NO_PREALLOC = 1
+
+	coll := &CollectionSpec{
+		Maps: map[string]*MapSpec{
+			"hash_map": {
+				Name:       "hash_map",
+				Type:       Hash,
+				KeySize:    4,
+				ValueSize:  8,
+				MaxEntries: 1,
+				Flags:      BPF_F_NO_PREALLOC,
+			},
+			"hash_map2": {
+				Name:       "hash_map2",
+				Type:       Hash,
+				KeySize:    4,
+				ValueSize:  8,
+				MaxEntries: 2,
+			},
+			"array_of_hash_map": {
+				Name:       "array_of_hash_map",
+				Type:       ArrayOfMaps,
+				KeySize:    4,
+				MaxEntries: 2,
+			},
+		},
+		Programs: map[string]*ProgramSpec{
+			"xdp_prog": {
+				Name:    "xdp_prog",
+				Type:    XDP,
+				License: "MIT",
+			},
+			"no_relocation": {
+				Name:    "no_relocation",
+				Type:    SocketFilter,
+				License: "MIT",
+			},
+		},
+	}
+
+	opts := cmp.Options{
+		cmpopts.IgnoreTypes(new(btf.Map), new(btf.Program)),
+		cmpopts.IgnoreFields(ProgramSpec{}, "Instructions", "ByteOrder"),
+		cmpopts.IgnoreMapEntries(func(key string, _ *MapSpec) bool {
+			switch key {
+			case ".bss", ".data", ".rodata":
+				return true
+
+			default:
+				return false
+			}
+		}),
+	}
+
 	testutils.TestFiles(t, "testdata/loader-*.elf", func(t *testing.T, file string) {
-		spec, err := LoadCollectionSpec(file)
+		have, err := LoadCollectionSpec(file)
 		if err != nil {
 			t.Fatal("Can't parse ELF:", err)
 		}
 
-		hashMapSpec := &MapSpec{
-			Name:       "hash_map",
-			Type:       Hash,
-			KeySize:    4,
-			ValueSize:  2,
-			MaxEntries: 1,
-		}
-		checkMapSpec(t, spec.Maps, "hash_map", hashMapSpec)
-		checkMapSpec(t, spec.Maps, "array_of_hash_map", &MapSpec{
-			Name:       "hash_map",
-			Type:       ArrayOfMaps,
-			KeySize:    4,
-			MaxEntries: 2,
-		})
-		spec.Maps["array_of_hash_map"].InnerMap = spec.Maps["hash_map"]
-
-		hashMap2Spec := &MapSpec{
-			Name:       "",
-			Type:       Hash,
-			KeySize:    4,
-			ValueSize:  1,
-			MaxEntries: 2,
-			Flags:      1,
+		if diff := cmp.Diff(coll, have, opts...); diff != "" {
+			t.Errorf("MapSpec mismatch (-want +got):\n%s", diff)
 		}
-		checkMapSpec(t, spec.Maps, "hash_map2", hashMap2Spec)
-		checkMapSpec(t, spec.Maps, "hash_of_hash_map", &MapSpec{
-			Type:       HashOfMaps,
-			KeySize:    4,
-			MaxEntries: 2,
-		})
-		spec.Maps["hash_of_hash_map"].InnerMap = spec.Maps["hash_map2"]
-
-		checkProgramSpec(t, spec.Programs, "xdp_prog", &ProgramSpec{
-			Type:          XDP,
-			License:       "MIT",
-			KernelVersion: 0,
-		})
-		checkProgramSpec(t, spec.Programs, "no_relocation", &ProgramSpec{
-			Type:          SocketFilter,
-			License:       "MIT",
-			KernelVersion: 0,
-		})
 
-		if rodata := spec.Maps[".rodata"]; rodata != nil {
-			err := spec.RewriteConstants(map[string]interface{}{
+		if rodata := have.Maps[".rodata"]; rodata != nil {
+			err := have.RewriteConstants(map[string]interface{}{
 				"arg": uint32(1),
 			})
 			if err != nil {
 				t.Fatal("Can't rewrite constant:", err)
 			}
 
-			err = spec.RewriteConstants(map[string]interface{}{
+			err = have.RewriteConstants(map[string]interface{}{
 				"totallyBogus": uint32(1),
 			})
 			if err == nil {
 				t.Error("Rewriting a bogus constant doesn't fail")
 			}
 		}
 
-		t.Log(spec.Programs["xdp_prog"].Instructions)
+		t.Log(have.Programs["xdp_prog"].Instructions)
 
-		if spec.Programs["xdp_prog"].ByteOrder != internal.NativeEndian {
+		if have.Programs["xdp_prog"].ByteOrder != internal.NativeEndian {
 			return
 		}
 
-		coll, err := NewCollectionWithOptions(spec, CollectionOptions{
+		have.Maps["array_of_hash_map"].InnerMap = have.Maps["hash_map"]
+		coll, err := NewCollectionWithOptions(have, CollectionOptions{
 			Programs: ProgramOptions{
 				LogLevel: 1,
 			},
@@ -104,81 +123,6 @@ func TestLoadCollectionSpec(t *testing.T) {
 	})
 }
 
-func checkMapSpec(t *testing.T, maps map[string]*MapSpec, name string, want *MapSpec) {
-	t.Helper()
-
-	have, ok := maps[name]
-	if !ok {
-		t.Errorf("Missing map %s", name)
-		return
-	}
-
-	mapSpecEqual(t, name, have, want)
-}
-
-func mapSpecEqual(t *testing.T, name string, have, want *MapSpec) {
-	t.Helper()
-
-	if have.Type != want.Type {
-		t.Errorf("%s: expected type %v, got %v", name, want.Type, have.Type)
-	}
-
-	if have.KeySize != want.KeySize {
-		t.Errorf("%s: expected key size %v, got %v", name, want.KeySize, have.KeySize)
-	}
-
-	if have.ValueSize != want.ValueSize {
-		t.Errorf("%s: expected value size %v, got %v", name, want.ValueSize, have.ValueSize)
-	}
-
-	if have.MaxEntries != want.MaxEntries {
-		t.Errorf("%s: expected max entries %v, got %v", name, want.MaxEntries, have.MaxEntries)
-	}
-
-	if have.Flags != want.Flags {
-		t.Errorf("%s: expected flags %v, got %v", name, want.Flags, have.Flags)
-	}
-
-	switch {
-	case have.InnerMap != nil && want.InnerMap == nil:
-		t.Errorf("%s: extraneous InnerMap", name)
-	case have.InnerMap == nil && want.InnerMap != nil:
-		t.Errorf("%s: missing InnerMap", name)
-	case have.InnerMap != nil && want.InnerMap != nil:
-		mapSpecEqual(t, name+".InnerMap", have.InnerMap, want.InnerMap)
-	}
-}
-
-func checkProgramSpec(t *testing.T, progs map[string]*ProgramSpec, name string, want *ProgramSpec) {
-	t.Helper()
-
-	have, ok := progs[name]
-	if !ok {
-		t.Fatalf("Missing program %s", name)
-		return
-	}
-
-	if have.ByteOrder == nil {
-		t.Errorf("%s: nil ByteOrder", name)
-	}
-
-	if have.License != want.License {
-		t.Errorf("%s: expected %v license, got %v", name, want.License, have.License)
-	}
-
-	if have.Type != want.Type {
-		t.Errorf("%s: expected %v program, got %v", name, want.Type, have.Type)
-	}
-
-	if want.Instructions != nil && !reflect.DeepEqual(have.Instructions, want.Instructions) {
-		t.Log("Expected program")
-		t.Log(want.Instructions)
-		t.Log("Actual program")
-		t.Log(want.Instructions)
-		t.Error("Instructions do not match")
-	}
-}
-
 func TestCollectionSpecDetach(t *testing.T) {
 	coll := Collection{
 		Maps: map[string]*Map{

go.mod

@@ -2,4 +2,7 @@ module github.com/cilium/ebpf
 
 go 1.14
 
-require golang.org/x/sys v0.0.0-20200124204421-9fbb57f87de9
+require (
+	github.com/google/go-cmp v0.5.2
+	golang.org/x/sys v0.0.0-20200124204421-9fbb57f87de9
+)

go.sum

@@ -1,2 +1,6 @@
+github.com/google/go-cmp v0.5.2 h1:X2ev0eStA3AbceY54o37/0PQ/UWqKEiiO2dKL5OPaFM=
+github.com/google/go-cmp v0.5.2/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE=
 golang.org/x/sys v0.0.0-20200124204421-9fbb57f87de9 h1:1/DFK4b7JH8DmkqhUk48onnSfrPzImPoVxuomtbT2nk=
 golang.org/x/sys v0.0.0-20200124204421-9fbb57f87de9/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
+golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543 h1:E7g+9GITq07hpfrRu66IVDexMakfv52eLZ2CXBWiKr4=
+golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=

testdata/common.h

@@ -7,7 +7,7 @@ typedef unsigned long uint64_t;
 #define __uint(name, val) int(*name)[val]
 #define __type(name, val) typeof(val) *name
 
-#define BPF_MAP_TYPE_ARRAY (1)
+#define BPF_MAP_TYPE_HASH (1)
 #define BPF_MAP_TYPE_PERF_EVENT_ARRAY (4)
 #define BPF_MAP_TYPE_ARRAY_OF_MAPS (12)
 #define BPF_MAP_TYPE_HASH_OF_MAPS (13)

testdata/invalid_map.c

@@ -11,7 +11,7 @@ struct {
 } invalid_map __section("maps") = {
 	.def =
 		{
-			.type        = BPF_MAP_TYPE_ARRAY,
+			.type        = BPF_MAP_TYPE_HASH,
 			.key_size    = 4,
 			.value_size  = 2,
 			.max_entries = 1,

testdata/loader.c

@@ -5,52 +5,45 @@
 
 char __license[] __section("license") = "MIT";
 
+#if __clang_major__ >= 9
+// Clang < 9 doesn't emit the necessary BTF for this to work.
+struct {
+	__uint(type, BPF_MAP_TYPE_HASH);
+	__type(key, uint32_t);
+	__type(value, uint64_t);
+	__uint(max_entries, 1);
+	__uint(map_flags, BPF_F_NO_PREALLOC);
+} hash_map __section(".maps");
+
+struct {
+	__uint(type, BPF_MAP_TYPE_HASH);
+	__uint(key_size, sizeof(uint32_t));
+	__uint(value_size, sizeof(uint64_t));
+	__uint(max_entries, 2);
+} hash_map2 __section(".maps");
+#else
 struct bpf_map_def hash_map __section("maps") = {
-	.type        = BPF_MAP_TYPE_ARRAY,
-	.key_size    = 4,
-	.value_size  = 2,
+	.type        = BPF_MAP_TYPE_HASH,
+	.key_size    = sizeof(uint32_t),
+	.value_size  = sizeof(uint64_t),
 	.max_entries = 1,
-	.map_flags   = 0,
+	.map_flags   = BPF_F_NO_PREALLOC,
 };
 
 struct bpf_map_def hash_map2 __section("maps") = {
-	.type        = BPF_MAP_TYPE_ARRAY,
-	.key_size    = 4,
-	.value_size  = 1,
+	.type        = BPF_MAP_TYPE_HASH,
+	.key_size    = sizeof(uint32_t),
+	.value_size  = sizeof(uint64_t),
 	.max_entries = 2,
-	.map_flags   = BPF_F_NO_PREALLOC,
 };
+#endif
 
 struct bpf_map_def array_of_hash_map __section("maps") = {
 	.type        = BPF_MAP_TYPE_ARRAY_OF_MAPS,
 	.key_size    = sizeof(uint32_t),
 	.max_entries = 2,
 };
 
-struct bpf_map_def hash_of_hash_map __section("maps") = {
-	.type        = BPF_MAP_TYPE_HASH_OF_MAPS,
-	.key_size    = sizeof(uint32_t),
-	.max_entries = 2,
-};
-
-#if __clang_major__ >= 9
-// Clang < 9 doesn't emit the necessary BTF for this to work.
-struct {
-	__uint(type, BPF_MAP_TYPE_ARRAY);
-	__type(key, uint32_t);
-	__type(value, uint32_t);
-	__uint(max_entries, 1);
-	__uint(map_flags, BPF_F_NO_PREALLOC);
-} btf_map __section(".maps");
-
-struct {
-	__uint(type, BPF_MAP_TYPE_ARRAY);
-	__uint(key_size, 4);
-	__uint(value_size, 4);
-	__uint(max_entries, 1);
-} btf_map2 __section(".maps");
-#endif
-
 static int __attribute__((noinline)) static_fn(uint32_t arg) {
 	return arg;
 }