-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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.
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
Conversation
|
@a-nogikh there is no reviewer for the PR, can you check once? |
|
I'll take a closer look in a couple of days, somewhat short on time at the moment. |
|
We already have quite extensive path rewriting logic around here: syzkaller/pkg/cover/backend/dwarf.go Lines 553 to 611 in c6512ef
So syzkaller already trims Why wouldn't it work in your case? |
|
No, it won't work, here's my full cleanRules as you can see there are different kinds of prefixes as in below from vmlinux, android gki modules and vendor modules. Vendor/OEM android trees are different from AOSP one (there is no rule to say where to put kernel and oot module tree), like my KernelSrc is like /TOPDIR, android kernel is inside /TOPDIR/kernel_platform/common, soc-repo in /TOPDIR/kernel_platform/soc-repo and vendor (oot tree) in /TOPDIR/vendor. So these delimiters in the syzkaller code are not enough. I think we need to allow a flexible way to cleanPath in config file for different build versions. |
In some targets like android, we got addr2line info like: /proc/self/cwd/common/kernel/kcov.c:852 To cleanup the path, we need to remove /proc/self/cwd/ prefix, and add relative path to common directory, so we can have clean_rules like: "/proc/self/cwd/common:kernel_platform/common" More examples: "clean_rules" : [ "/proc/self/cwd/common/../vendor:vendor", "/proc/self/cwd/common/../soc-repo:kernel_platform/soc-repo", "/proc/self/cwd/vendor:vendor", "/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"` |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
|
close until there is better solution. |
In some targets like android, we got addr2line info like: /proc/self/cwd/common/kernel/kcov.c:852
To cleanup the path, we need to remove /proc/self/cwd/ prefix, and add relative path to common directory,
so we can have clean_rules like:
"/proc/self/cwd/common:kernel_platform/common"
More examples:
"clean_rules" : [
"/proc/self/cwd/common/../vendor:vendor",
"/proc/self/cwd/common/../soc-repo:kernel_platform/soc-repo",
"/proc/self/cwd/vendor:vendor",
"/proc/self/cwd/common:kernel_platform/common"
]
Before sending a pull request, please review Contribution Guidelines:
https://github.com/google/syzkaller/blob/master/docs/contributing.md