pkg/cover: pass modules as []host.KernelModule

Pass modules as []host.KernelModule to cover.MakeReportGenerator.
This avoids make(map) in callers that don't pass modules.

Store modules as []*KernelModule.
This avoids clumsy assignments to the map to update Path
and allows to store modules as *KernelModule rather than by name
(we are not scripting, pointer is more flexible and handy representation).

5 files changed, 68 insertions, 85 deletions

diff --git a/pkg/cover/backend/backend.go b/pkg/cover/backend/backend.go
index 0b28cbab2..29b413943 100644
--- a/pkg/cover/backend/backend.go
+++ b/pkg/cover/backend/backend.go
@@ -4,8 +4,6 @@
 package backend
 
 import (
-	"fmt"
-
 	"github.com/google/syzkaller/sys/targets"
 )
 
@@ -21,7 +19,7 @@ type Impl struct {
 	Frames    []Frame
 	Symbolize func(pcs []uint64, obj string) ([]Frame, error)
 	RestorePC func(pc uint32) uint64
-	Modules   map[string]KernelModule
+	Modules   []*KernelModule
 }
 
 type CompileUnit struct {
@@ -45,7 +43,7 @@ type ObjectUnit struct {
 }
 
 type Frame struct {
-	Module string
+	Module *KernelModule
 	PC     uint64
 	Name   string
 	Path   string
@@ -61,19 +59,16 @@ type Range struct {
 
 const LineEnd = 1 << 30
 
-func Make(target *targets.Target, vm, objDir, srcDir, buildDir string,
-	moduleObj []string, modules map[string]KernelModule) (*Impl, error) {
-	if objDir == "" {
-		return nil, fmt.Errorf("kernel obj directory is not specified")
-	}
+func Make(target *targets.Target, vm, srcDir, buildDir string,
+	moduleObj []string, modules []*KernelModule) (*Impl, error) {
 	if vm == "gvisor" {
-		return makeGvisor(target, objDir, srcDir, buildDir, modules)
+		return makeGvisor(target, srcDir, buildDir, modules)
 	}
-	return makeELF(target, objDir, srcDir, buildDir, moduleObj, modules)
+	return makeELF(target, srcDir, buildDir, moduleObj, modules)
 }
 
-var GroupPCsByModule = func(pcs []uint64, modules map[string]KernelModule) map[string][]uint64 {
-	groupPCs := make(map[string][]uint64)
-	groupPCs[""] = pcs
+var GroupPCsByModule = func(pcs []uint64, modules []*KernelModule) map[*KernelModule][]uint64 {
+	groupPCs := make(map[*KernelModule][]uint64)
+	groupPCs[modules[0]] = pcs
 	return groupPCs
 }
diff --git a/pkg/cover/backend/elf.go b/pkg/cover/backend/elf.go
index 635bfdc12..96251cc50 100644
--- a/pkg/cover/backend/elf.go
+++ b/pkg/cover/backend/elf.go
@@ -29,24 +29,16 @@ func init() {
 	GroupPCsByModule = groupPCsByModule
 }
 
-func makeELF(target *targets.Target, objDir, srcDir, buildDir string,
-	moduleObj []string, modules map[string]KernelModule) (*Impl, error) {
+func makeELF(target *targets.Target, srcDir, buildDir string,
+	moduleObj []string, modules []*KernelModule) (*Impl, error) {
 	var allCoverPoints [2][]uint64
 	var allSymbols []*Symbol
 	var allRanges []pcRange
 	var allUnits []*CompileUnit
 
-	if _, ok := modules[""]; !ok {
-		if target.OS == targets.Linux {
-			dirs := append([]string{objDir}, moduleObj...)
-			getKernelModules(dirs, modules)
-		}
-	}
-	kernelObject := filepath.Join(objDir, target.KernelObject)
-	modules[""] = KernelModule{
-		Path: kernelObject,
+	if target.OS == targets.Linux {
+		getKernelModules(moduleObj, modules)
 	}
-
 	for _, module := range modules {
 		if module.Path == "" {
 			continue
@@ -74,11 +66,7 @@ func makeELF(target *targets.Target, objDir, srcDir, buildDir string,
 				textAddr = ^uint64(0)
 			}
 			if module.Name == "" {
-				modules[""] = KernelModule{
-					Name: "",
-					Addr: textAddr,
-					Path: kernelObject,
-				}
+				module.Addr = textAddr
 			}
 			if target.Arch == targets.AMD64 {
 				if module.Name == "" {
@@ -135,6 +123,7 @@ func makeELF(target *targets.Target, objDir, srcDir, buildDir string,
 		if len(unit.PCs) == 0 {
 			continue // drop the unit
 		}
+		objDir := modules[0].Path // TODO: won't work for out-of-tree modules
 		unit.Name, unit.Path = cleanPath(unit.Name, objDir, srcDir, buildDir)
 		allUnits[nunit] = unit
 		nunit++
@@ -147,29 +136,30 @@ func makeELF(target *targets.Target, objDir, srcDir, buildDir string,
 		Units:   allUnits,
 		Symbols: allSymbols,
 		Symbolize: func(pcs []uint64, obj string) ([]Frame, error) {
+			objDir := modules[0].Path // TODO: won't work for out-of-tree modules
 			return symbolize(target, objDir, srcDir, buildDir, obj, pcs)
 		},
 		RestorePC: func(pc uint32) uint64 {
-			return PreviousInstructionPC(target, RestorePC(pc, uint32(modules[""].Addr>>32)))
+			return PreviousInstructionPC(target, RestorePC(pc, uint32(modules[0].Addr>>32)))
 		},
 		Modules: modules,
 	}
 	return impl, nil
 }
 
-func getKernelModules(dirs []string, modules map[string]KernelModule) {
+func getKernelModules(dirs []string, modules []*KernelModule) {
+	byName := make(map[string]*KernelModule)
+	for _, mod := range modules {
+		byName[mod.Name] = mod
+	}
 	files := findModulePaths(dirs)
 	for _, path := range files {
 		name := strings.TrimSuffix(filepath.Base(path), ".ko")
-		if module, ok := modules[name]; ok {
+		if module := byName[name]; module != nil {
 			if module.Path != "" {
 				continue
 			}
-			modules[name] = KernelModule{
-				Name: name,
-				Addr: module.Addr,
-				Path: path,
-			}
+			module.Path = path
 			continue
 		}
 		name, err := getModuleName(path)
@@ -180,15 +170,11 @@ func getKernelModules(dirs []string, modules map[string]KernelModule) {
 		if name == "" {
 			continue
 		}
-		if module, ok := modules[name]; ok {
+		if module := byName[name]; module != nil {
 			if module.Path != "" {
 				continue
 			}
-			modules[name] = KernelModule{
-				Name: name,
-				Addr: module.Addr,
-				Path: path,
-			}
+			module.Path = path
 		}
 	}
 	log.Logf(0, "kernel modules: %v", modules)
@@ -265,30 +251,26 @@ func searchModuleName(data []byte) string {
 	return string(data[pos+len(key) : end])
 }
 
-func groupPCsByModule(pcs []uint64, modules map[string]KernelModule) map[string][]uint64 {
-	var smodules []KernelModule
-	groupPCs := make(map[string][]uint64)
-	for _, module := range modules {
-		groupPCs[module.Name] = make([]uint64, 0)
-		smodules = append(smodules, module)
-	}
+func groupPCsByModule(pcs []uint64, modules []*KernelModule) map[*KernelModule][]uint64 {
+	groupPCs := make(map[*KernelModule][]uint64)
+	smodules := append([]*KernelModule{}, modules...)
 	sort.Slice(smodules, func(i, j int) bool {
 		return smodules[i].Addr > smodules[j].Addr
 	})
 	for _, pc := range pcs {
 		if pc > smodules[0].Addr {
 			if smodules[0].Name == "" {
-				groupPCs[smodules[0].Name] = append(groupPCs[smodules[0].Name], pc)
+				groupPCs[smodules[0]] = append(groupPCs[smodules[0]], pc)
 			} else {
-				groupPCs[smodules[0].Name] = append(groupPCs[smodules[0].Name], pc-smodules[0].Addr)
+				groupPCs[smodules[0]] = append(groupPCs[smodules[0]], pc-smodules[0].Addr)
 			}
 		} else {
 			for i := 0; i < len(modules)-1; i++ {
 				if pc < smodules[i].Addr && pc >= smodules[i+1].Addr {
 					if smodules[i+1].Name == "" {
-						groupPCs[smodules[i+1].Name] = append(groupPCs[smodules[i+1].Name], pc)
+						groupPCs[smodules[i+1]] = append(groupPCs[smodules[i+1]], pc)
 					} else {
-						groupPCs[smodules[i+1].Name] = append(groupPCs[smodules[i+1].Name], pc-smodules[i+1].Addr)
+						groupPCs[smodules[i+1]] = append(groupPCs[smodules[i+1]], pc-smodules[i+1].Addr)
 					}
 					break
 				}
@@ -364,7 +346,7 @@ func buildSymbols(symbols []*Symbol, ranges []pcRange, coverPoints [2][]uint64)
 	return symbols
 }
 
-func readSymbols(file *elf.File, module KernelModule) ([]*Symbol, uint64, uint64, map[uint64]bool, error) {
+func readSymbols(file *elf.File, module *KernelModule) ([]*Symbol, uint64, uint64, map[uint64]bool, error) {
 	text := file.Section(".text")
 	if text == nil {
 		return nil, 0, 0, nil, fmt.Errorf("no .text section in the object file")
@@ -408,7 +390,7 @@ func readSymbols(file *elf.File, module KernelModule) ([]*Symbol, uint64, uint64
 	return symbols, text.Addr, tracePC, traceCmp, nil
 }
 
-func readTextRanges(file *elf.File, module KernelModule) ([]pcRange, []*CompileUnit, error) {
+func readTextRanges(file *elf.File, module *KernelModule) ([]pcRange, []*CompileUnit, error) {
 	text := file.Section(".text")
 	if text == nil {
 		return nil, nil, fmt.Errorf("no .text section in the object file")
@@ -604,7 +586,7 @@ func getRelSymbolName(file *elf.File, index uint32) (string, error) {
 // Currently it is amd64-specific: looks for e8 opcode and correct offset.
 // Running objdump on the whole object file is too slow.
 func readCoverPoints(file *elf.File, tracePC uint64, traceCmp map[uint64]bool,
-	module KernelModule) ([2][]uint64, error) {
+	module *KernelModule) ([2][]uint64, error) {
 	var pcs [2][]uint64
 	const callLen = 5
 	if module.Name == "" {
diff --git a/pkg/cover/backend/gvisor.go b/pkg/cover/backend/gvisor.go
index e590c4956..e4170ffb6 100644
--- a/pkg/cover/backend/gvisor.go
+++ b/pkg/cover/backend/gvisor.go
@@ -14,19 +14,14 @@ import (
 	"github.com/google/syzkaller/sys/targets"
 )
 
-func makeGvisor(target *targets.Target, objDir, srcDir, buildDir string,
-	modules map[string]KernelModule) (*Impl, error) {
+func makeGvisor(target *targets.Target, srcDir, buildDir string, modules []*KernelModule) (*Impl, error) {
+	if len(modules) != 1 {
+		return nil, fmt.Errorf("gvisor coverage does not support modules")
+	}
+	bin := modules[0].Path
 	// pkg/build stores runsc as 'vmlinux' (we pretent to be linux), but a local build will have it as 'runsc'.
-	bin := filepath.Join(objDir, target.KernelObject)
 	if !osutil.IsExist(bin) {
-		bin = filepath.Join(objDir, "runsc")
-	}
-	if modules == nil {
-		modules = make(map[string]KernelModule)
-	}
-	modules[""] = KernelModule{
-		Name: "",
-		Path: bin,
+		bin = filepath.Join(filepath.Dir(bin), "runsc")
 	}
 	frames, err := gvisorSymbolize(bin, srcDir)
 	if err != nil {
@@ -56,7 +51,6 @@ func makeGvisor(target *targets.Target, objDir, srcDir, buildDir string,
 		RestorePC: func(pc uint32) uint64 {
 			return uint64(pc)
 		},
-		Modules: modules,
 	}
 	return impl, nil
 }
diff --git a/pkg/cover/report.go b/pkg/cover/report.go
index b838d5f5f..222bcdfa6 100644
--- a/pkg/cover/report.go
+++ b/pkg/cover/report.go
@@ -5,9 +5,11 @@ package cover
 
 import (
 	"fmt"
+	"path/filepath"
 	"sort"
 
 	"github.com/google/syzkaller/pkg/cover/backend"
+	"github.com/google/syzkaller/pkg/host"
 	"github.com/google/syzkaller/pkg/mgrconfig"
 	"github.com/google/syzkaller/sys/targets"
 )
@@ -15,7 +17,6 @@ import (
 type ReportGenerator struct {
 	target    *targets.Target
 	srcDir    string
-	objDir    string
 	buildDir  string
 	subsystem []mgrconfig.Subsystem
 	*backend.Impl
@@ -29,8 +30,23 @@ type Prog struct {
 var RestorePC = backend.RestorePC
 
 func MakeReportGenerator(target *targets.Target, vm, objDir, srcDir, buildDir string, subsystem []mgrconfig.Subsystem,
-	moduleObj []string, modules map[string]backend.KernelModule) (*ReportGenerator, error) {
-	impl, err := backend.Make(target, vm, objDir, srcDir, buildDir, moduleObj, modules)
+	moduleObj []string, hostModules []host.KernelModule) (*ReportGenerator, error) {
+	if objDir == "" {
+		return nil, fmt.Errorf("kernel obj directory is not specified")
+	}
+	moduleObj = append([]string{objDir}, moduleObj...)
+	modules := []*backend.KernelModule{
+		{
+			Path: filepath.Join(objDir, target.KernelObject),
+		},
+	}
+	for _, mod := range hostModules {
+		modules = append(modules, &backend.KernelModule{
+			Name: mod.Name,
+			Addr: mod.Addr,
+		})
+	}
+	impl, err := backend.Make(target, vm, srcDir, buildDir, moduleObj, modules)
 	if err != nil {
 		return nil, err
 	}
@@ -41,7 +57,6 @@ func MakeReportGenerator(target *targets.Target, vm, objDir, srcDir, buildDir st
 	rg := &ReportGenerator{
 		target:    target,
 		srcDir:    srcDir,
-		objDir:    objDir,
 		buildDir:  buildDir,
 		subsystem: subsystem,
 		Impl:      impl,
@@ -95,11 +110,9 @@ func (rg *ReportGenerator) prepareFileMap(progs []Prog) (map[string]*file, error
 	for _, frame := range rg.Frames {
 		f := getFile(files, frame.Name, frame.Path)
 		ln := f.lines[frame.StartLine]
-		var pc uint64
-		if frame.Module == "" {
-			pc = frame.PC
-		} else {
-			pc = frame.PC + rg.Modules[frame.Module].Addr
+		pc := frame.PC
+		if frame.Module.Name != "" {
+			pc += frame.Module.Addr
 		}
 		coveredBy := progPCs[pc]
 		if len(coveredBy) == 0 {
@@ -182,17 +195,16 @@ func (rg *ReportGenerator) lazySymbolize(progs []Prog) error {
 	if len(pcs) == 0 {
 		return nil
 	}
-	groupPCs := backend.GroupPCsByModule(pcs, rg.Modules)
-	for name, pcs := range groupPCs {
+	for mod, pcs := range backend.GroupPCsByModule(pcs, rg.Modules) {
 		if len(pcs) == 0 {
 			continue
 		}
-		frames, err := rg.Symbolize(pcs, rg.Modules[name].Path)
+		frames, err := rg.Symbolize(pcs, mod.Path)
 		if err != nil {
 			return err
 		}
 		for i := range frames {
-			frames[i].Module = name
+			frames[i].Module = mod
 		}
 		rg.Frames = append(rg.Frames, frames...)
 	}
diff --git a/pkg/cover/report_test.go b/pkg/cover/report_test.go
index 239ca106f..25dc43a62 100644
--- a/pkg/cover/report_test.go
+++ b/pkg/cover/report_test.go
@@ -191,7 +191,7 @@ func generateReport(t *testing.T, target *targets.Target, test Test) ([]byte, []
 		},
 	}
 
-	rg, err := MakeReportGenerator(target, "", dir, dir, dir, subsystem, nil, make(map[string]backend.KernelModule))
+	rg, err := MakeReportGenerator(target, "", dir, dir, dir, subsystem, nil, nil)
 	if err != nil {
 		return nil, nil, err
 	}