Merge pull request #26 from parca-dev/parse-modules

proc: Fix parsing Kernel Module lines where refcount is "-"

interpreter/loaderinfo.go

@@ -55,7 +55,7 @@ func (i *LoaderInfo) GetSymbolAsRanges(symbol libpf.SymbolName) ([]util.Range, e
 	start := uint64(sym.Address)
 	return []util.Range{{
 		Start: start,
-		End:   start + uint64(sym.Size)},
+		End:   start + sym.Size},
 	}, nil
 }
 

interpreter/luajit/offsets.go

@@ -323,10 +323,10 @@ func (o *offsetData) findTraceInfoFromLuaOpen() (*libpf.Symbol, error) {
 	slices.Sort(funcAddrs)
 
 	// Its a tiny function, give it reasonable default.
-	traceInfoSize := 100
+	traceInfoSize := uint64(100)
 	for i, addr := range funcAddrs {
 		if addr == traceInfoAddr && i != len(funcAddrs)-1 {
-			traceInfoSize = int(funcAddrs[i+1] - funcAddrs[i])
+			traceInfoSize = funcAddrs[i+1] - funcAddrs[i]
 			break
 		}
 	}

libpf/pfelf/file.go

@@ -786,7 +786,7 @@ func (f *File) readAndMatchSymbol(n uint32, name libpf.SymbolName) (libpf.Symbol
 	return libpf.Symbol{
 		Name:    name,
 		Address: libpf.SymbolValue(sym.Value),
-		Size:    int(sym.Size),
+		Size:    sym.Size,
 	}, true
 }
 
@@ -946,7 +946,7 @@ func (f *File) loadSymbolTable(name string) (*libpf.SymbolMap, error) {
 		symMap.Add(libpf.Symbol{
 			Name:    libpf.SymbolName(name),
 			Address: libpf.SymbolValue(sym.Value),
-			Size:    int(sym.Size),
+			Size:    sym.Size,
 		})
 	}
 	symMap.Finalize()

libpf/pfelf/pfelf.go

@@ -256,7 +256,7 @@ func symbolMapFromELFSymbols(syms []elf.Symbol) *libpf.SymbolMap {
 		symmap.Add(libpf.Symbol{
 			Name:    libpf.SymbolName(sym.Name),
 			Address: libpf.SymbolValue(sym.Value),
-			Size:    int(sym.Size),
+			Size:    sym.Size,
 		})
 	}
 	symmap.Finalize()

libpf/symbol.go

@@ -32,7 +32,7 @@ type SymbolFinder interface {
 type Symbol struct {
 	Name    SymbolName
 	Address SymbolValue
-	Size    int
+	Size    uint64
 }
 
 var _ SymbolFinder = &SymbolMap{}

proc/proc.go

@@ -106,51 +106,104 @@ func GetKernelModules(modulesPath string,
 	symmap.Add(libpf.Symbol{
 		Name:    "vmlinux",
 		Address: stext.Address,
-		Size:    int(etext.Address - stext.Address),
+		Size:    uint64(etext.Address - stext.Address),
 	})
 
-	atLeastOneValidAddress := false
-	count := 0
+	modules, err := parseKernelModules(bufio.NewScanner(file))
+	if err != nil {
+		return nil, fmt.Errorf("failed to parse kernel modules: %v", err)
+	}
 
-	var scanner = bufio.NewScanner(file)
-	for scanner.Scan() {
-		var size, address uint64
-		var refcount int64
-		var name, dependencies, state string
+	for _, kmod := range modules {
+		symmap.Add(libpf.Symbol{
+			Name:    libpf.SymbolName(kmod.name),
+			Address: libpf.SymbolValue(kmod.address),
+			Size:    kmod.size,
+		})
+	}
+
+	symmap.Finalize()
+
+	return &symmap, nil
+}
+
+func parseKernelModules(scanner *bufio.Scanner) ([]kernelModule, error) {
+	var (
+		modules                []kernelModule
+		atLeastOneValidAddress = false
+		count                  = 0
+	)
 
+	for scanner.Scan() {
 		line := scanner.Text()
 
 		count++
 
-		nFields, err := fmt.Sscanf(line, "%s %d %d %s %s 0x%x",
-			&name, &size, &refcount, &dependencies, &state, &address)
+		kmod, err := parseKernelModuleLine(line)
 		if err != nil {
-			log.Warnf("err parsing line in modules: '%s'", err)
-			continue
-		}
-		if nFields < 6 {
-			log.Warnf("unexpected line in modules: '%s'", line)
-			continue
+			return nil, fmt.Errorf("failed to parse kernel module line: %v", err)
 		}
-		if address == 0 {
+		if kmod.address == 0 {
 			continue
 		}
 		atLeastOneValidAddress = true
 
-		symmap.Add(libpf.Symbol{
-			Name:    libpf.SymbolName(name),
-			Address: libpf.SymbolValue(address),
-			Size:    int(size),
-		})
+		modules = append(modules, kmod)
 	}
 
 	if count > 0 && !atLeastOneValidAddress {
 		return nil, errors.New("addresses from all modules is zero - check process permissions")
 	}
 
-	symmap.Finalize()
+	return modules, nil
+}
 
-	return &symmap, nil
+type kernelModule struct {
+	name    string
+	size    uint64
+	address uint64
+}
+
+func parseKernelModuleLine(line string) (kernelModule, error) {
+	// The format is: "name size refcount dependencies state address"
+	parts := strings.SplitN(line, " ", 6)
+	if len(parts) < 6 {
+		return kernelModule{}, fmt.Errorf("unexpected line in modules: '%s'", line)
+	}
+
+	size, err := parseSize(parts[1])
+	if err != nil {
+		return kernelModule{}, fmt.Errorf("failed to parse size value: '%s'", parts[1])
+	}
+
+	address, err := parseAddress(parts[5])
+	if err != nil {
+		return kernelModule{}, fmt.Errorf("failed to parse address value: '%s'", parts[5])
+	}
+
+	return kernelModule{
+		name:    parts[0],
+		size:    size,
+		address: address,
+	}, nil
+}
+
+func parseAddress(addressStr string) (uint64, error) {
+	address, err := strconv.ParseUint(strings.TrimPrefix(addressStr, "0x"), 16, 64)
+	if err != nil {
+		return 0, fmt.Errorf("failed to parse address as hex value: '%s'", addressStr)
+	}
+
+	return address, nil
+}
+
+func parseSize(sizeStr string) (uint64, error) {
+	size, err := strconv.ParseUint(sizeStr, 10, 64)
+	if err != nil {
+		return 0, fmt.Errorf("failed to parse size int value: %q", sizeStr)
+	}
+
+	return size, nil
 }
 
 // IsPIDLive checks if a PID belongs to a live process. It will never produce a false negative but

proc/proc_test.go

@@ -4,6 +4,8 @@
 package proc
 
 import (
+	"bufio"
+	"bytes"
 	"testing"
 
 	"go.opentelemetry.io/ebpf-profiler/libpf"
@@ -38,3 +40,66 @@ func TestParseKallSyms(t *testing.T) {
 	assertSymbol(t, symmap, "cpu_tss_rw", 0x6000)
 	assertSymbol(t, symmap, "hid_add_device", 0xffffffffc033e550)
 }
+
+func TestParseKernelModules(t *testing.T) {
+	content := []byte(`i40e 589824 - - Live 0xffffffffc0321000
+mpt3sas 405504 - - Live 0xffffffffc02ab000
+ahci 45056 - - Live 0xffffffffc0294000
+libahci 49152 - - Live 0xffffffffc027f000
+sp5100_tco 12288 - - Live 0xffffffffc0274000
+watchdog 40960 - - Live 0xffffffffc025f000
+k10temp 12288 - - Live 0xffffffffc0254000`)
+
+	kmods, err := parseKernelModules(bufio.NewScanner(bytes.NewReader(content)))
+	require.NoError(t, err)
+
+	require.Len(t, kmods, 7)
+	require.Equal(t, []kernelModule{
+		{
+			name:    "i40e",
+			size:    589824,
+			address: 0xffffffffc0321000,
+		},
+		{
+			name:    "mpt3sas",
+			size:    405504,
+			address: 0xffffffffc02ab000,
+		},
+		{
+			name:    "ahci",
+			size:    45056,
+			address: 0xffffffffc0294000,
+		},
+		{
+			name:    "libahci",
+			size:    49152,
+			address: 0xffffffffc027f000,
+		},
+		{
+			name:    "sp5100_tco",
+			size:    12288,
+			address: 0xffffffffc0274000,
+		},
+		{
+			name:    "watchdog",
+			size:    40960,
+			address: 0xffffffffc025f000,
+		},
+		{
+			name:    "k10temp",
+			size:    12288,
+			address: 0xffffffffc0254000,
+		},
+	}, kmods)
+}
+
+func TestParseKernelModuleLine(t *testing.T) {
+	line := "i40e 589824 - - Live 0xffffffffc0364000"
+	kmod, err := parseKernelModuleLine(line)
+	require.NoError(t, err)
+	require.Equal(t, kernelModule{
+		name:    "i40e",
+		size:    589824,
+		address: 0xffffffffc0364000,
+	}, kmod)
+}

processmanager/synthdeltas.go

@@ -48,7 +48,7 @@ func createVDSOSyntheticRecordArm64(ef *pfelf.File) sdtypes.IntervalData {
 			deltas = append(
 				deltas,
 				sdtypes.StackDelta{Address: addr, Info: sdtypes.UnwindInfoSignal},
-				sdtypes.StackDelta{Address: addr + uint64(sym.Size), Info: sdtypes.UnwindInfoLR},
+				sdtypes.StackDelta{Address: addr + sym.Size, Info: sdtypes.UnwindInfoLR},
 			)
 			return
 		}
@@ -60,7 +60,7 @@ func createVDSOSyntheticRecordArm64(ef *pfelf.File) sdtypes.IntervalData {
 
 		var frameStart uint64
 		var frameSize int
-		for offs := uint64(0); offs < uint64(sym.Size); offs += 4 {
+		for offs := uint64(0); offs < sym.Size; offs += 4 {
 			inst, err := aa.Decode(code[offs:])
 			if err != nil {
 				continue