Re: [RFC PATCH] bug: always show source-tree-relative paths in WARN()/BUG()
On Thu, Jul 11, 2019 at 6:50 PM Masahiro Yamada wrote: > GCC 8 added this flag. > So, it will be eventually all solved in the GCC world. Ack. > Clang has not supported it yet... That's what it appeared like. I've bugged our Clang-loving toolchain folks to see if we can get parity. > Trimming absolute path at run-time > is no help for reducing the kernel image. Sure, but that's not my stated goal. It would indeed be nicer though. I guess if no one else speaks up with a favorable word toward my RFC, I'll just see what I can do on the toolchain side. Thanks for the help, Brian
Re: [RFC PATCH] bug: always show source-tree-relative paths in WARN()/BUG()
On Fri, Jul 12, 2019 at 10:23 AM Brian Norris wrote: > > On Thu, Jul 11, 2019 at 6:14 PM Masahiro Yamada > wrote: > > BTW, did you see this? > > > > commit a73619a845d5625079cc1b3b820f44c899618388 > > Author: Masahiro Yamada > > Date: Fri Mar 30 13:15:26 2018 +0900 > > > > kbuild: use -fmacro-prefix-map to make __FILE__ a relative path > > Oh, wow, no I did not. If my reading is correct, that's GCC only? I've > been using various combinations of newer (5.2) and older (4.14.y -- > didn't have that patch) kernels, older GCC (doesn't have that feature > AFAICT), and newer Clang (doesn't appear to have that feature). So I'm > not totally sure if I ever actually tried a combo that *could* make > use of that. But I may give it another shot. > > In the event that this is GCC-specific...I don't suppose I could > convince anybody to expend any effort (e.g., taking a patch like mine) > to solve it for the non-GCC world? > > Thanks for the tip, > Brian GCC 8 added this flag. So, it will be eventually all solved in the GCC world. Clang has not supported it yet... Trimming absolute path at run-time is no help for reducing the kernel image. Turning __FILE__ into a relative path at compile-time is better. I hope Clang people will consider to support it. I guess implementing this feature should not be so hard. -- Best Regards Masahiro Yamada
Re: [RFC PATCH] bug: always show source-tree-relative paths in WARN()/BUG()
On Thu, Jul 11, 2019 at 6:14 PM Masahiro Yamada wrote: > BTW, did you see this? > > commit a73619a845d5625079cc1b3b820f44c899618388 > Author: Masahiro Yamada > Date: Fri Mar 30 13:15:26 2018 +0900 > > kbuild: use -fmacro-prefix-map to make __FILE__ a relative path Oh, wow, no I did not. If my reading is correct, that's GCC only? I've been using various combinations of newer (5.2) and older (4.14.y -- didn't have that patch) kernels, older GCC (doesn't have that feature AFAICT), and newer Clang (doesn't appear to have that feature). So I'm not totally sure if I ever actually tried a combo that *could* make use of that. But I may give it another shot. In the event that this is GCC-specific...I don't suppose I could convince anybody to expend any effort (e.g., taking a patch like mine) to solve it for the non-GCC world? Thanks for the tip, Brian
Re: [RFC PATCH] bug: always show source-tree-relative paths in WARN()/BUG()
On Fri, Jul 12, 2019 at 10:06 AM Brian Norris wrote:
>
> When building out-of-tree (e.g., 'make O=...'), __FILE__ ends up being
> an absolute path, and so WARN() and BUG() end up putting path names from
> the build system into the log text. For example:
>
> # echo BUG > /sys/kernel/debug/provoke-crash/DIRECT
> ...
> kernel BUG at /mnt/host/source/[...]/drivers/misc/lkdtm/bugs.c:71!
>
> Not only is this excessively verbose, it also adds extra noise into
> tools that might parse this output. (For example, if builder paths
> change across versions, we suddenly get a "new" crash signature.)
>
> All in all, this looks much better as:
>
> kernel BUG at drivers/misc/lkdtm/bugs.c:71!
>
> It appears the Kbuild system is fairly entrenched in using
> $(KBUILD_OUTPUT) for the ${CWD}, which necessarily means that the
> preprocessor will get handed an absolute path. It seems the only
> solution then, is to do some sort of post-processing on __FILE__.
>
> It so happens that lib/dynamic_debug.c already solves this sort of
> problem, so I steal its solution for use in panic/warn/bug code as well.
BTW, did you see this?
commit a73619a845d5625079cc1b3b820f44c899618388
Author: Masahiro Yamada
Date: Fri Mar 30 13:15:26 2018 +0900
kbuild: use -fmacro-prefix-map to make __FILE__ a relative path
> Signed-off-by: Brian Norris
> ---
> I'd be happy to entertain better solutions to this problem, but so far,
> I haven't been creative enough to come up with one.
>
> I'm also unsure of who best to address this to. If anyone has better
> pointers, I'm all ears.
>
> include/linux/bug.h | 2 ++
> kernel/panic.c | 21 +++--
> lib/bug.c | 3 ++-
> lib/dynamic_debug.c | 18 --
> 4 files changed, 27 insertions(+), 17 deletions(-)
>
> diff --git a/include/linux/bug.h b/include/linux/bug.h
> index fe5916550da8..6ab59e53801d 100644
> --- a/include/linux/bug.h
> +++ b/include/linux/bug.h
> @@ -76,4 +76,6 @@ static inline __must_check bool check_data_corruption(bool
> v) { return v; }
> corruption; \
> }))
>
> +const char *trim_filepath_prefix(const char *path);
> +
> #endif /* _LINUX_BUG_H */
> diff --git a/kernel/panic.c b/kernel/panic.c
> index 4d9f55bf7d38..0bed3101f049 100644
> --- a/kernel/panic.c
> +++ b/kernel/panic.c
> @@ -546,6 +546,23 @@ struct warn_args {
> va_list args;
> };
>
> +/**
> + * trim_filepath_prefix - retrieve source-root relative path from a __FILE__
> + * @path: a __FILE__-like path argument.
> + * Return: path relative to source root.
> + */
> +const char *trim_filepath_prefix(const char *path)
> +{
> + int skip = strlen(__FILE__) - strlen("kernel/panic.c");
> +
> + BUILD_BUG_ON(strlen(__FILE__) < strlen("kernel/panic.c"));
> +
> + if (strncmp(path, __FILE__, skip))
> + skip = 0; /* prefix mismatch, don't skip */
> +
> + return path + skip;
> +}
> +
> void __warn(const char *file, int line, void *caller, unsigned taint,
> struct pt_regs *regs, struct warn_args *args)
> {
> @@ -556,8 +573,8 @@ void __warn(const char *file, int line, void *caller,
> unsigned taint,
>
> if (file)
> pr_warn("WARNING: CPU: %d PID: %d at %s:%d %pS\n",
> - raw_smp_processor_id(), current->pid, file, line,
> - caller);
> + raw_smp_processor_id(), current->pid,
> + trim_filepath_prefix(file), line, caller);
> else
> pr_warn("WARNING: CPU: %d PID: %d at %pS\n",
> raw_smp_processor_id(), current->pid, caller);
> diff --git a/lib/bug.c b/lib/bug.c
> index 1077366f496b..2aa91d330451 100644
> --- a/lib/bug.c
> +++ b/lib/bug.c
> @@ -191,7 +191,8 @@ enum bug_trap_type report_bug(unsigned long bugaddr,
> struct pt_regs *regs)
> printk(KERN_DEFAULT CUT_HERE);
>
> if (file)
> - pr_crit("kernel BUG at %s:%u!\n", file, line);
> + pr_crit("kernel BUG at %s:%u!\n", trim_filepath_prefix(file),
> + line);
> else
> pr_crit("Kernel BUG at %pB [verbose debug info
> unavailable]\n",
> (void *)bugaddr);
> diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
> index 8a16c2d498e9..0896f067ba17 100644
> --- a/lib/dynamic_debug.c
> +++ b/lib/dynamic_debug.c
> @@ -13,6 +13,7 @@
>
> #define pr_fmt(fmt) KBUILD_MODNAME ":%s: " fmt, __func__
>
> +#include
> #include
> #include
> #include
> @@ -67,17 +68,6 @@ static LIST_HEAD(ddebug_tables);
> static int verbose;
> module_param(verbose, int, 0644);
>
> -/* Return the path relative to source root */
> -static inline const char *trim_prefix(const char *path)
> -{
> - int skip = strlen(__FILE__) - strlen("lib/dynamic_debug.c");
> -
> - if (strncmp(path, __FILE__, skip))
> - skip = 0; /* prefix mismatch, don't skip */
[RFC PATCH] bug: always show source-tree-relative paths in WARN()/BUG()
When building out-of-tree (e.g., 'make O=...'), __FILE__ ends up being
an absolute path, and so WARN() and BUG() end up putting path names from
the build system into the log text. For example:
# echo BUG > /sys/kernel/debug/provoke-crash/DIRECT
...
kernel BUG at /mnt/host/source/[...]/drivers/misc/lkdtm/bugs.c:71!
Not only is this excessively verbose, it also adds extra noise into
tools that might parse this output. (For example, if builder paths
change across versions, we suddenly get a "new" crash signature.)
All in all, this looks much better as:
kernel BUG at drivers/misc/lkdtm/bugs.c:71!
It appears the Kbuild system is fairly entrenched in using
$(KBUILD_OUTPUT) for the ${CWD}, which necessarily means that the
preprocessor will get handed an absolute path. It seems the only
solution then, is to do some sort of post-processing on __FILE__.
It so happens that lib/dynamic_debug.c already solves this sort of
problem, so I steal its solution for use in panic/warn/bug code as well.
Signed-off-by: Brian Norris
---
I'd be happy to entertain better solutions to this problem, but so far,
I haven't been creative enough to come up with one.
I'm also unsure of who best to address this to. If anyone has better
pointers, I'm all ears.
include/linux/bug.h | 2 ++
kernel/panic.c | 21 +++--
lib/bug.c | 3 ++-
lib/dynamic_debug.c | 18 --
4 files changed, 27 insertions(+), 17 deletions(-)
diff --git a/include/linux/bug.h b/include/linux/bug.h
index fe5916550da8..6ab59e53801d 100644
--- a/include/linux/bug.h
+++ b/include/linux/bug.h
@@ -76,4 +76,6 @@ static inline __must_check bool check_data_corruption(bool v)
{ return v; }
corruption; \
}))
+const char *trim_filepath_prefix(const char *path);
+
#endif /* _LINUX_BUG_H */
diff --git a/kernel/panic.c b/kernel/panic.c
index 4d9f55bf7d38..0bed3101f049 100644
--- a/kernel/panic.c
+++ b/kernel/panic.c
@@ -546,6 +546,23 @@ struct warn_args {
va_list args;
};
+/**
+ * trim_filepath_prefix - retrieve source-root relative path from a __FILE__
+ * @path: a __FILE__-like path argument.
+ * Return: path relative to source root.
+ */
+const char *trim_filepath_prefix(const char *path)
+{
+ int skip = strlen(__FILE__) - strlen("kernel/panic.c");
+
+ BUILD_BUG_ON(strlen(__FILE__) < strlen("kernel/panic.c"));
+
+ if (strncmp(path, __FILE__, skip))
+ skip = 0; /* prefix mismatch, don't skip */
+
+ return path + skip;
+}
+
void __warn(const char *file, int line, void *caller, unsigned taint,
struct pt_regs *regs, struct warn_args *args)
{
@@ -556,8 +573,8 @@ void __warn(const char *file, int line, void *caller,
unsigned taint,
if (file)
pr_warn("WARNING: CPU: %d PID: %d at %s:%d %pS\n",
- raw_smp_processor_id(), current->pid, file, line,
- caller);
+ raw_smp_processor_id(), current->pid,
+ trim_filepath_prefix(file), line, caller);
else
pr_warn("WARNING: CPU: %d PID: %d at %pS\n",
raw_smp_processor_id(), current->pid, caller);
diff --git a/lib/bug.c b/lib/bug.c
index 1077366f496b..2aa91d330451 100644
--- a/lib/bug.c
+++ b/lib/bug.c
@@ -191,7 +191,8 @@ enum bug_trap_type report_bug(unsigned long bugaddr, struct
pt_regs *regs)
printk(KERN_DEFAULT CUT_HERE);
if (file)
- pr_crit("kernel BUG at %s:%u!\n", file, line);
+ pr_crit("kernel BUG at %s:%u!\n", trim_filepath_prefix(file),
+ line);
else
pr_crit("Kernel BUG at %pB [verbose debug info unavailable]\n",
(void *)bugaddr);
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 8a16c2d498e9..0896f067ba17 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -13,6 +13,7 @@
#define pr_fmt(fmt) KBUILD_MODNAME ":%s: " fmt, __func__
+#include
#include
#include
#include
@@ -67,17 +68,6 @@ static LIST_HEAD(ddebug_tables);
static int verbose;
module_param(verbose, int, 0644);
-/* Return the path relative to source root */
-static inline const char *trim_prefix(const char *path)
-{
- int skip = strlen(__FILE__) - strlen("lib/dynamic_debug.c");
-
- if (strncmp(path, __FILE__, skip))
- skip = 0; /* prefix mismatch, don't skip */
-
- return path + skip;
-}
-
static struct { unsigned flag:8; char opt_char; } opt_array[] = {
{ _DPRINTK_FLAGS_PRINT, 'p' },
{ _DPRINTK_FLAGS_INCL_MODNAME, 'm' },
@@ -164,7 +154,7 @@ static int ddebug_change(const struct ddebug_query *query,
!match_wildcard(query->filename,
kbasename(dp->filename)) &&
!match_wildcard(query->filename,
-
