elf: link library sections

So far, we don't resolve function references in sections that we
link to the main program, which is problematic when using something
like -ffunction-sections. Update loader.c to consistently test calling
static and non-static functions in the same and in a different section.

Fix the linker to also process linked code sections.

Author: lmb

elf_reader_test.go

@@ -100,8 +100,8 @@ func TestLoadCollectionSpec(t *testing.T) {
 				t.Fatal("Can't run program:", err)
 			}
 
-			if ret != 1 {
-				t.Error("Expected return value to be 1, got", ret)
+			if ret != 5 {
+				t.Error("Expected return value to be 5, got", ret)
 			}
 		})
 	}

linker.go

@@ -10,35 +10,51 @@ import (
 // link resolves bpf-to-bpf calls.
 //
 // Each library may contain multiple functions / labels, and is only linked
-// if the program being edited references one of these functions.
+// if prog references one of these functions.
 //
-// Libraries must not require linking themselves.
+// Libraries also linked.
 func link(prog *ProgramSpec, libs []*ProgramSpec) error {
-	for _, lib := range libs {
-		insns, err := linkSection(prog.Instructions, lib.Instructions)
-		if err != nil {
-			return xerrors.Errorf("linking %s: %w", lib.Name, err)
-		}
+	var (
+		linked  = make(map[*ProgramSpec]bool)
+		pending = []asm.Instructions{prog.Instructions}
+		insns   asm.Instructions
+	)
+	for len(pending) > 0 {
+		insns, pending = pending[0], pending[1:]
+		for _, lib := range libs {
+			if linked[lib] {
+				continue
+			}
 
-		if len(insns) == len(prog.Instructions) {
-			continue
-		}
+			needed, err := needSection(insns, lib.Instructions)
+			if err != nil {
+				return xerrors.Errorf("linking %s: %w", lib.Name, err)
+			}
 
-		prog.Instructions = insns
-		if prog.BTF != nil && lib.BTF != nil {
-			if err := btf.ProgramAppend(prog.BTF, lib.BTF); err != nil {
-				return xerrors.Errorf("linking BTF of %s: %w", lib.Name, err)
+			if !needed {
+				continue
+			}
+
+			linked[lib] = true
+			prog.Instructions = append(prog.Instructions, lib.Instructions...)
+			pending = append(pending, lib.Instructions)
+
+			if prog.BTF != nil && lib.BTF != nil {
+				if err := btf.ProgramAppend(prog.BTF, lib.BTF); err != nil {
+					return xerrors.Errorf("linking BTF of %s: %w", lib.Name, err)
+				}
 			}
 		}
 	}
+
 	return nil
 }
 
-func linkSection(insns, section asm.Instructions) (asm.Instructions, error) {
+func needSection(insns, section asm.Instructions) (bool, error) {
 	// A map of symbols to the libraries which contain them.
 	symbols, err := section.SymbolOffsets()
 	if err != nil {
-		return nil, err
+		return false, err
 	}
 
 	for _, ins := range insns {
@@ -61,11 +77,10 @@ func linkSection(insns, section asm.Instructions) (asm.Instructions, error) {
 		}
 
 		// At this point we know that at least one function in the
-		// library is called from insns. Merge the two sections.
-		// The rewrite of ins.Constant happens in asm.Instruction.Marshal.
-		return append(insns, section...), nil
+		// library is called from insns, so we have to link it.
+		return true, nil
 	}
 
-	// None of the functions in the section are called. Do nothing.
-	return insns, nil
+	// None of the functions in the section are called.
+	return false, nil
 }

linker_test.go

@@ -20,14 +20,22 @@ func TestLink(t *testing.T) {
 		License: "MIT",
 	}
 
-	lib := &ProgramSpec{
-		Instructions: asm.Instructions{
-			asm.LoadImm(asm.R0, 1337, asm.DWord).Sym("my_func"),
-			asm.Return(),
+	libs := []*ProgramSpec{
+		{
+			Instructions: asm.Instructions{
+				asm.LoadImm(asm.R0, 1337, asm.DWord).Sym("my_other_func"),
+				asm.Return(),
+			},
+		},
+		{
+			Instructions: asm.Instructions{
+				asm.Call.Label("my_other_func").Sym("my_func"),
+				asm.Return(),
+			},
 		},
 	}
 
-	err := link(spec, []*ProgramSpec{lib})
+	err := link(spec, libs)
 	if err != nil {
 		t.Fatal(err)
 	}

testdata/invalid_map.elf

@@ -44,20 +44,20 @@ struct {
 } btf_map __section(".maps");
 #endif
 
-static int __attribute__((noinline)) helper_func2(uint32_t arg) {
+static int __attribute__((noinline)) static_fn(uint32_t arg) {
 	return arg;
 }
 
-int __attribute__((noinline)) helper_func(uint32_t arg) {
-	// Enforce bpf-to-bpf call in .text section
-	return helper_func2(arg);
+int __attribute__((noinline)) global_fn2(uint32_t arg) {
+	return arg++;
 }
 
-// Static functions in other sections get different relocations
-// than global functions.
-static int __attribute__((noinline)) helper_func_static() {
-	uint32_t key = 0;
-	return !!map_lookup_elem(&hash_map, (void*)&key);
+int __attribute__((noinline)) __section("other") global_fn3(uint32_t arg) {
+	return arg+1;
+}
+
+int __attribute__((noinline)) global_fn(uint32_t arg) {
+	return static_fn(arg) + global_fn2(arg) + global_fn3(arg);
 }
 
 #if __clang_major__ >= 9
@@ -77,10 +77,7 @@ __section("xdp") int xdp_prog() {
 	map_lookup_elem(&hash_map, (void*)&key1);
 	map_lookup_elem(&hash_map2, (void*)&key2);
 	map_lookup_elem(&hash_map2, (void*)&key3);
-	if (helper_func_static()) {
-		return 2;
-	}
-	return helper_func(arg);
+	return static_fn(arg) + global_fn(arg);
 }
 
 // This function has no relocations, and is thus parsed differently.