Skip to content

all: add clean_rules config to help CleanPath for specific targets #5842

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
jiangenj wants to merge 1 commit into from
Closed

all: add clean_rules config to help CleanPath for specific targets #5842

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions pkg/cover/backend/backend.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ type SecRange struct {
const LineEnd = 1 << 30

func Make(target *targets.Target, vm string, kernelDirs *mgrconfig.KernelDirs, splitBuild bool,
moduleObj []string, modules []*vminfo.KernelModule) (*Impl, error) {
moduleObj []string, modules []*vminfo.KernelModule, cleanRules []string) (*Impl, error) {
if kernelDirs.Obj == "" {
return nil, fmt.Errorf("kernel obj directory is not specified")
}
Expand All @@ -84,7 +84,7 @@ func Make(target *targets.Target, vm string, kernelDirs *mgrconfig.KernelDirs, s
// details.
delimiters = []string{"/aosp/", "/private/"}
}
return makeELF(target, kernelDirs, delimiters, moduleObj, modules)
return makeELF(target, kernelDirs, delimiters, moduleObj, modules, cleanRules)
}

func GetPCBase(cfg *mgrconfig.Config) (uint64, error) {
Expand Down
29 changes: 22 additions & 7 deletions pkg/cover/backend/dwarf.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ type dwarfParams struct {
readModuleCoverPoints func(*targets.Target, *vminfo.KernelModule, *symbolInfo) ([2][]uint64, error)
readTextRanges func(*vminfo.KernelModule) ([]pcRange, []*CompileUnit, error)
getCompilerVersion func(string) string
cleanRules []string
}

type Arch struct {
Expand Down Expand Up @@ -141,6 +142,7 @@ func makeDWARFUnsafe(params *dwarfParams) (*Impl, error) {
kernelDirs := params.kernelDirs
splitBuildDelimiters := params.splitBuildDelimiters
modules := params.hostModules
cleanRules := params.cleanRules

// Here and below index 0 refers to coverage callbacks (__sanitizer_cov_trace_pc(_guard))
// and index 1 refers to comparison callbacks (__sanitizer_cov_trace_cmp*).
Expand Down Expand Up @@ -228,7 +230,7 @@ func makeDWARFUnsafe(params *dwarfParams) (*Impl, error) {
continue // drop the unit
}
// TODO: objDir won't work for out-of-tree modules.
unit.Name, unit.Path = CleanPath(unit.Name, kernelDirs, splitBuildDelimiters)
unit.Name, unit.Path = CleanPath(unit.Name, kernelDirs, splitBuildDelimiters, cleanRules)
allUnits[nunit] = unit
nunit++
}
Expand All @@ -241,7 +243,7 @@ func makeDWARFUnsafe(params *dwarfParams) (*Impl, error) {
Units: allUnits,
Symbols: allSymbols,
Symbolize: func(pcs map[*vminfo.KernelModule][]uint64) ([]*Frame, error) {
return symbolize(target, &interner, kernelDirs, splitBuildDelimiters, pcs)
return symbolize(target, &interner, kernelDirs, splitBuildDelimiters, pcs, cleanRules)
},
CallbackPoints: allCoverPoints[0],
PreciseCoverage: preciseCoverage,
Expand Down Expand Up @@ -399,7 +401,7 @@ func readTextRanges(debugInfo *dwarf.Data, module *vminfo.KernelModule, pcFix pc
}

func symbolizeModule(target *targets.Target, interner *symbolizer.Interner, kernelDirs *mgrconfig.KernelDirs,
splitBuildDelimiters []string, mod *vminfo.KernelModule, pcs []uint64) ([]*Frame, error) {
splitBuildDelimiters []string, mod *vminfo.KernelModule, pcs []uint64, cleanRules []string) ([]*Frame, error) {
procs := min(runtime.GOMAXPROCS(0)/2, len(pcs)/1000)
const (
minProcs = 1
Expand Down Expand Up @@ -450,7 +452,7 @@ func symbolizeModule(target *targets.Target, interner *symbolizer.Interner, kern
err0 = res.err
}
for _, frame := range res.frames {
name, path := CleanPath(frame.File, kernelDirs, splitBuildDelimiters)
name, path := CleanPath(frame.File, kernelDirs, splitBuildDelimiters, cleanRules)
pc := frame.PC
if mod.Name != "" {
pc = frame.PC + mod.Addr
Expand Down Expand Up @@ -478,7 +480,7 @@ func symbolizeModule(target *targets.Target, interner *symbolizer.Interner, kern
}

func symbolize(target *targets.Target, interner *symbolizer.Interner, kernelDirs *mgrconfig.KernelDirs,
splitBuildDelimiters []string, pcs map[*vminfo.KernelModule][]uint64) ([]*Frame, error) {
splitBuildDelimiters []string, pcs map[*vminfo.KernelModule][]uint64, cleanRules []string) ([]*Frame, error) {
var frames []*Frame
type frameResult struct {
frames []*Frame
Expand All @@ -487,7 +489,7 @@ func symbolize(target *targets.Target, interner *symbolizer.Interner, kernelDirs
frameC := make(chan frameResult, len(pcs))
for mod, pcs1 := range pcs {
go func(mod *vminfo.KernelModule, pcs []uint64) {
frames, err := symbolizeModule(target, interner, kernelDirs, splitBuildDelimiters, mod, pcs)
frames, err := symbolizeModule(target, interner, kernelDirs, splitBuildDelimiters, mod, pcs, cleanRules)
frameC <- frameResult{frames: frames, err: err}
}(mod, pcs1)
}
Expand Down Expand Up @@ -582,9 +584,22 @@ func cleanPathAndroid(path, srcDir string, delimiters []string, existFn func(str
return "", ""
}

func CleanPath(path string, kernelDirs *mgrconfig.KernelDirs, splitBuildDelimiters []string) (string, string) {
func CleanPath(path string, kernelDirs *mgrconfig.KernelDirs, splitBuildDelimiters,
cleanRules []string) (string, string) {
filename := ""

// Assume out-of-tree modules need to apply clean rules.
for _, rule := range cleanRules {
tokens := strings.Split(rule, ":")
old := tokens[0]
new := tokens[1]
if strings.HasPrefix(path, old) {
path = strings.Replace(path, old, new, 1)
filename = filepath.Join(kernelDirs.Src, path)
return strings.TrimLeft(filepath.Clean(path), "/\\"), filename
}
}

path = filepath.Clean(path)
aname, apath := cleanPathAndroid(path, kernelDirs.Src, splitBuildDelimiters, osutil.IsExist)
if aname != "" {
Expand Down
3 changes: 2 additions & 1 deletion pkg/cover/backend/elf.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import (
)

func makeELF(target *targets.Target, kernelDirs *mgrconfig.KernelDirs, splitBuildDelimiters, moduleObj []string,
hostModules []*vminfo.KernelModule) (*Impl, error) {
hostModules []*vminfo.KernelModule, cleanRules []string) (*Impl, error) {
return makeDWARF(&dwarfParams{
target: target,
kernelDirs: kernelDirs,
Expand All @@ -30,6 +30,7 @@ func makeELF(target *targets.Target, kernelDirs *mgrconfig.KernelDirs, splitBuil
readModuleCoverPoints: elfReadModuleCoverPoints,
readTextRanges: elfReadTextRanges,
getCompilerVersion: elfGetCompilerVersion,
cleanRules: cleanRules,
})
}

Expand Down
3 changes: 2 additions & 1 deletion pkg/cover/report.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,8 @@ func GetPCBase(cfg *mgrconfig.Config) (uint64, error) {
}

func MakeReportGenerator(cfg *mgrconfig.Config, modules []*vminfo.KernelModule) (*ReportGenerator, error) {
impl, err := backend.Make(cfg.SysTarget, cfg.Type, cfg.KernelDirs(), cfg.AndroidSplitBuild, cfg.ModuleObj, modules)
impl, err := backend.Make(cfg.SysTarget, cfg.Type, cfg.KernelDirs(), cfg.AndroidSplitBuild, cfg.ModuleObj,
modules, cfg.CleanRules)
if err != nil {
return nil, err
}
Expand Down
11 changes: 11 additions & 0 deletions pkg/mgrconfig/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,17 @@ type Config struct {

// Implementation details beyond this point. Filled after parsing.
Derived `json:"-"`

// cleanPath rules like "old_path:new_path" which replaces old_path by new_path
// it's especially useful for out-of-tree kernel modules symbol path
// old_path is relative path to kernel_src, for example in Android S
// "clean_rules" : [
// "/proc/self/cwd/common:kernel_platform/common"
// ]
// in cleanPath function, if path in elf is /proc/self/cwd/common/drivers/something.c
// then path will be changed to kernel_platform/common/drivers/something.c,
// and filename will be kernel_src + /kernel_platform/common/drivers/something.c
CleanRules []string `json:"clean_rules,omitempty"`
Copy link
Collaborator

@dvyukov dvyukov Apr 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Configuration for modules looks more and more like a pile of hacks on top of hacks.
Main kernel has src+obj dirs. For modules you added only obj dirs (ModuleObj). And now source dirs are specified separately and in a different way with new CleanRules concept. There is also no mapping between module objs dirs and clean rules.

Please think of a consistent way to configure all this.
E.g. is it possible to provide a set of parameters for the main kernel (e.g. src+obj) and the same set of parameters for separate module build dirs? Then the configuration would be a consistent set of kernel build tree directories.

Copy link
Contributor Author

@jiangenj jiangenj Apr 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @dvyukov
There are multiple paths for module_srcs, we don't know which module_src to use for th e module, so I think module_src +module_obj won't work.

Besides, I checked with google android team in https://issuetracker.google.com/issues/408907076.

Is your debugging process broken because of this? The reason we chose /proc/self/cwd is this:
https://cs.android.com/android/kernel/superproject/+/common-android-mainline:build/kernel/kleaf/impl/kernel_env.bzl;l=576-584;drc=e8bc2d8f8faa5bde296f1ccd2345ee66b9a6bf5a
        # Replace ${{ROOT_DIR}} with "/proc/self/cwd" in the file name
        # references in the binaries (e.g. debug info).
        # "/proc/self/cwd" is an absolute path that resolves to a directory
        # where debugger runs. And ${{ROOT_DIR}} layout should be the same as
        # layout on the top of the repo, so if you start a debugger from the
        # top directory, all paths should resolve correctly even on another
        # machine.
As for DW_AT_comp_dir, unfortunately it is deleted by bazel unless you use --config=local. Even in that case, the path is still not correct, though you can find the files in out/cache. I don't think that would be something Kleaf aims to fix. What do you use DW_AT_comp_dir for?

so, I think /proc/self/cwd will always be there and there is no clean way to get correct map for pattern like /proc/self/cwd/common/../soc-repo:kernel_platform/soc-repo.

And in current syzkaller, we have

	var delimiters []string
	if cfg.AndroidSplitBuild {
		// Path prefixes used by Android Pixel kernels. See
		// https://source.android.com/docs/setup/build/building-pixel-kernels for more
		// details.
		delimiters = []string{"/aosp/", "/private/"}
	}

which might work only for aosp kernel if we put kernel in aosp tree, but it won't work for vendor/oems who put kernel in different dirs.

Copy link
Collaborator

@dvyukov dvyukov Apr 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are multiple paths for module_srcs, we don't know which module_src to use for th e module

They have different prefixes, why wouldn't we know?
That's what CleanPath already does for the main kernel: if path starts with build dir, it replaces it with src dir.
If we have a set of paths (build/obj/src) for each module, all of them can be replaced.

so, I think /proc/self/cwd will always be there and there is no clean way to get correct map for pattern like /proc/self/cwd/common/../soc-repo:kernel_platform/soc-repo.

But you just did it with the current patch and clean path rules. That's no different.

And in current syzkaller, we have

Yes, that's another ugly hack we have. We just keep piling special hacks for each new case.

}

// These options are not guaranteed to be backward/forward compatible and
Expand Down
2 changes: 1 addition & 1 deletion pkg/report/linux.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -487,7 +487,7 @@ func symbolizeLine(symbFunc func(bin string, pc uint64) ([]symbolizer.Frame, err
}
var symbolized []byte
for _, frame := range frames {
path, _ := backend.CleanPath(frame.File, &ctx.kernelDirs, nil)
path, _ := backend.CleanPath(frame.File, &ctx.kernelDirs, nil, ctx.config.cleanRules)
info := fmt.Sprintf(" %v:%v", path, frame.Line)
modified := append([]byte{}, line...)
if buildID != "" {
Expand Down
2 changes: 2 additions & 0 deletions pkg/report/report.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,7 @@ func NewReporter(cfg *mgrconfig.Config) (*Reporter, error) {
kernelDirs: *cfg.KernelDirs(),
ignores: ignores,
kernelModules: localModules,
cleanRules: cfg.CleanRules,
}
rep, suppressions, err := ctor(config)
if err != nil {
Expand Down Expand Up @@ -172,6 +173,7 @@ type config struct {
kernelDirs mgrconfig.KernelDirs
ignores []*regexp.Regexp
kernelModules []*vminfo.KernelModule
cleanRules []string
}

type fn func(cfg *config) (reporterImpl, []string, error)
Expand Down
Loading