From: Guenter Roeck <linux@roeck-us.net>
Add name of functions triggering warning backtraces to the __bug_table
object section to enable support for suppressing WARNING backtraces.
To limit image size impact, the pointer to the function name is only added
to the __bug_table section if both CONFIG_KUNIT_SUPPRESS_BACKTRACE and
CONFIG_DEBUG_BUGVERBOSE are enabled. Otherwise, the __func__ assembly
parameter is replaced with a (dummy) NULL parameter to avoid an image size
increase due to unused __func__ entries (this is necessary because __func__
is not a define but a virtual variable).
Tested-by: Linux Kernel Functional Testing <lkft@linaro.org>
Acked-by: Dan Carpenter <dan.carpenter@linaro.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
Signed-off-by: Alessandro Carminati <acarmina@redhat.com>
---
arch/x86/include/asm/bug.h | 21 ++++++++++++++++-----
1 file changed, 16 insertions(+), 5 deletions(-)
diff --git a/arch/x86/include/asm/bug.h b/arch/x86/include/asm/bug.h
index e85ac0c7c039..f6e13fc675ab 100644
--- a/arch/x86/include/asm/bug.h
+++ b/arch/x86/include/asm/bug.h
@@ -35,18 +35,28 @@
#ifdef CONFIG_DEBUG_BUGVERBOSE
+#ifdef CONFIG_KUNIT_SUPPRESS_BACKTRACE
+# define HAVE_BUG_FUNCTION
+# define __BUG_FUNC_PTR __BUG_REL(%c1)
+# define __BUG_FUNC __func__
+#else
+# define __BUG_FUNC_PTR
+# define __BUG_FUNC NULL
+#endif /* CONFIG_KUNIT_SUPPRESS_BACKTRACE */
+
#define _BUG_FLAGS(ins, flags, extra) \
do { \
asm_inline volatile("1:\t" ins "\n" \
".pushsection __bug_table,\"aw\"\n" \
"2:\t" __BUG_REL(1b) "\t# bug_entry::bug_addr\n" \
"\t" __BUG_REL(%c0) "\t# bug_entry::file\n" \
- "\t.word %c1" "\t# bug_entry::line\n" \
- "\t.word %c2" "\t# bug_entry::flags\n" \
- "\t.org 2b+%c3\n" \
+ "\t" __BUG_FUNC_PTR "\t# bug_entry::function\n" \
+ "\t.word %c2" "\t# bug_entry::line\n" \
+ "\t.word %c3" "\t# bug_entry::flags\n" \
+ "\t.org 2b+%c4\n" \
".popsection\n" \
extra \
- : : "i" (__FILE__), "i" (__LINE__), \
+ : : "i" (__FILE__), "i" (__BUG_FUNC), "i" (__LINE__),\
"i" (flags), \
"i" (sizeof(struct bug_entry))); \
} while (0)
@@ -92,7 +102,8 @@ do { \
do { \
__auto_type __flags = BUGFLAG_WARNING|(flags); \
instrumentation_begin(); \
- _BUG_FLAGS(ASM_UD2, __flags, ANNOTATE_REACHABLE(1b)); \
+ if (!KUNIT_IS_SUPPRESSED_WARNING(__func__)) \
+ _BUG_FLAGS(ASM_UD2, __flags, ANNOTATE_REACHABLE(1b)); \
instrumentation_end(); \
} while (0)
--
2.34.1On Thu, Mar 13, 2025 at 11:43:21AM +0000, Alessandro Carminati wrote:
> From: Guenter Roeck <linux@roeck-us.net>
>
> Add name of functions triggering warning backtraces to the __bug_table
> object section to enable support for suppressing WARNING backtraces.
>
> To limit image size impact, the pointer to the function name is only added
> to the __bug_table section if both CONFIG_KUNIT_SUPPRESS_BACKTRACE and
> CONFIG_DEBUG_BUGVERBOSE are enabled. Otherwise, the __func__ assembly
> parameter is replaced with a (dummy) NULL parameter to avoid an image size
> increase due to unused __func__ entries (this is necessary because __func__
> is not a define but a virtual variable).
>
> Tested-by: Linux Kernel Functional Testing <lkft@linaro.org>
> Acked-by: Dan Carpenter <dan.carpenter@linaro.org>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: Borislav Petkov <bp@alien8.de>
> Cc: Dave Hansen <dave.hansen@linux.intel.com>
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> Signed-off-by: Alessandro Carminati <acarmina@redhat.com>
> ---
> arch/x86/include/asm/bug.h | 21 ++++++++++++++++-----
> 1 file changed, 16 insertions(+), 5 deletions(-)
>
> diff --git a/arch/x86/include/asm/bug.h b/arch/x86/include/asm/bug.h
> index e85ac0c7c039..f6e13fc675ab 100644
> --- a/arch/x86/include/asm/bug.h
> +++ b/arch/x86/include/asm/bug.h
> @@ -35,18 +35,28 @@
>
> #ifdef CONFIG_DEBUG_BUGVERBOSE
>
> +#ifdef CONFIG_KUNIT_SUPPRESS_BACKTRACE
> +# define HAVE_BUG_FUNCTION
> +# define __BUG_FUNC_PTR __BUG_REL(%c1)
> +# define __BUG_FUNC __func__
> +#else
> +# define __BUG_FUNC_PTR
> +# define __BUG_FUNC NULL
> +#endif /* CONFIG_KUNIT_SUPPRESS_BACKTRACE */
> +
> #define _BUG_FLAGS(ins, flags, extra) \
> do { \
> asm_inline volatile("1:\t" ins "\n" \
> ".pushsection __bug_table,\"aw\"\n" \
> "2:\t" __BUG_REL(1b) "\t# bug_entry::bug_addr\n" \
> "\t" __BUG_REL(%c0) "\t# bug_entry::file\n" \
> - "\t.word %c1" "\t# bug_entry::line\n" \
> - "\t.word %c2" "\t# bug_entry::flags\n" \
> - "\t.org 2b+%c3\n" \
> + "\t" __BUG_FUNC_PTR "\t# bug_entry::function\n" \
> + "\t.word %c2" "\t# bug_entry::line\n" \
> + "\t.word %c3" "\t# bug_entry::flags\n" \
> + "\t.org 2b+%c4\n" \
> ".popsection\n" \
> extra \
> - : : "i" (__FILE__), "i" (__LINE__), \
> + : : "i" (__FILE__), "i" (__BUG_FUNC), "i" (__LINE__),\
> "i" (flags), \
> "i" (sizeof(struct bug_entry))); \
> } while (0)
> @@ -92,7 +102,8 @@ do { \
> do { \
> __auto_type __flags = BUGFLAG_WARNING|(flags); \
> instrumentation_begin(); \
> - _BUG_FLAGS(ASM_UD2, __flags, ANNOTATE_REACHABLE(1b)); \
> + if (!KUNIT_IS_SUPPRESSED_WARNING(__func__)) \
> + _BUG_FLAGS(ASM_UD2, __flags, ANNOTATE_REACHABLE(1b)); \
> instrumentation_end(); \
> } while (0)
NAK, this grows the BUG site for now appreciable reason.
On 4/1/25 10:08, Peter Zijlstra wrote:
> On Thu, Mar 13, 2025 at 11:43:21AM +0000, Alessandro Carminati wrote:
>> From: Guenter Roeck <linux@roeck-us.net>
>>
>> Add name of functions triggering warning backtraces to the __bug_table
>> object section to enable support for suppressing WARNING backtraces.
>>
>> To limit image size impact, the pointer to the function name is only added
>> to the __bug_table section if both CONFIG_KUNIT_SUPPRESS_BACKTRACE and
>> CONFIG_DEBUG_BUGVERBOSE are enabled. Otherwise, the __func__ assembly
>> parameter is replaced with a (dummy) NULL parameter to avoid an image size
>> increase due to unused __func__ entries (this is necessary because __func__
>> is not a define but a virtual variable).
>>
>> Tested-by: Linux Kernel Functional Testing <lkft@linaro.org>
>> Acked-by: Dan Carpenter <dan.carpenter@linaro.org>
>> Cc: Thomas Gleixner <tglx@linutronix.de>
>> Cc: Ingo Molnar <mingo@redhat.com>
>> Cc: Borislav Petkov <bp@alien8.de>
>> Cc: Dave Hansen <dave.hansen@linux.intel.com>
>> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
>> Signed-off-by: Alessandro Carminati <acarmina@redhat.com>
>> ---
>> arch/x86/include/asm/bug.h | 21 ++++++++++++++++-----
>> 1 file changed, 16 insertions(+), 5 deletions(-)
>>
>> diff --git a/arch/x86/include/asm/bug.h b/arch/x86/include/asm/bug.h
>> index e85ac0c7c039..f6e13fc675ab 100644
>> --- a/arch/x86/include/asm/bug.h
>> +++ b/arch/x86/include/asm/bug.h
>> @@ -35,18 +35,28 @@
>>
>> #ifdef CONFIG_DEBUG_BUGVERBOSE
>>
>> +#ifdef CONFIG_KUNIT_SUPPRESS_BACKTRACE
>> +# define HAVE_BUG_FUNCTION
>> +# define __BUG_FUNC_PTR __BUG_REL(%c1)
>> +# define __BUG_FUNC __func__
>> +#else
>> +# define __BUG_FUNC_PTR
>> +# define __BUG_FUNC NULL
>> +#endif /* CONFIG_KUNIT_SUPPRESS_BACKTRACE */
>> +
>> #define _BUG_FLAGS(ins, flags, extra) \
>> do { \
>> asm_inline volatile("1:\t" ins "\n" \
>> ".pushsection __bug_table,\"aw\"\n" \
>> "2:\t" __BUG_REL(1b) "\t# bug_entry::bug_addr\n" \
>> "\t" __BUG_REL(%c0) "\t# bug_entry::file\n" \
>> - "\t.word %c1" "\t# bug_entry::line\n" \
>> - "\t.word %c2" "\t# bug_entry::flags\n" \
>> - "\t.org 2b+%c3\n" \
>> + "\t" __BUG_FUNC_PTR "\t# bug_entry::function\n" \
>> + "\t.word %c2" "\t# bug_entry::line\n" \
>> + "\t.word %c3" "\t# bug_entry::flags\n" \
>> + "\t.org 2b+%c4\n" \
>> ".popsection\n" \
>> extra \
>> - : : "i" (__FILE__), "i" (__LINE__), \
>> + : : "i" (__FILE__), "i" (__BUG_FUNC), "i" (__LINE__),\
>> "i" (flags), \
>> "i" (sizeof(struct bug_entry))); \
>> } while (0)
>> @@ -92,7 +102,8 @@ do { \
>> do { \
>> __auto_type __flags = BUGFLAG_WARNING|(flags); \
>> instrumentation_begin(); \
>> - _BUG_FLAGS(ASM_UD2, __flags, ANNOTATE_REACHABLE(1b)); \
>> + if (!KUNIT_IS_SUPPRESSED_WARNING(__func__)) \
>> + _BUG_FLAGS(ASM_UD2, __flags, ANNOTATE_REACHABLE(1b)); \
>> instrumentation_end(); \
>> } while (0)
>
> NAK, this grows the BUG site for now appreciable reason.
Only if CONFIG_KUNIT_SUPPRESS_BACKTRACE is enabled. Why does that warrant a NACK ?
Guenter
On Tue, Apr 01, 2025 at 10:53:46AM -0700, Guenter Roeck wrote:
> > > #define _BUG_FLAGS(ins, flags, extra) \
> > > do { \
> > > asm_inline volatile("1:\t" ins "\n" \
> > > ".pushsection __bug_table,\"aw\"\n" \
> > > "2:\t" __BUG_REL(1b) "\t# bug_entry::bug_addr\n" \
> > > "\t" __BUG_REL(%c0) "\t# bug_entry::file\n" \
> > > - "\t.word %c1" "\t# bug_entry::line\n" \
> > > - "\t.word %c2" "\t# bug_entry::flags\n" \
> > > - "\t.org 2b+%c3\n" \
> > > + "\t" __BUG_FUNC_PTR "\t# bug_entry::function\n" \
> > > + "\t.word %c2" "\t# bug_entry::line\n" \
> > > + "\t.word %c3" "\t# bug_entry::flags\n" \
> > > + "\t.org 2b+%c4\n" \
> > > ".popsection\n" \
> > > extra \
> > > - : : "i" (__FILE__), "i" (__LINE__), \
> > > + : : "i" (__FILE__), "i" (__BUG_FUNC), "i" (__LINE__),\
> > > "i" (flags), \
> > > "i" (sizeof(struct bug_entry))); \
> > > } while (0)
Also this, why do you need this extra function in the bug entry? Isn't
that trivial from the trap site itself? symbol information should be
able to get you the function from the trap ip.
None of this makes any sense.
On Tue, Apr 01, 2025 at 10:53:46AM -0700, Guenter Roeck wrote:
> > > @@ -92,7 +102,8 @@ do { \
> > > do { \
> > > __auto_type __flags = BUGFLAG_WARNING|(flags); \
> > > instrumentation_begin(); \
> > > - _BUG_FLAGS(ASM_UD2, __flags, ANNOTATE_REACHABLE(1b)); \
> > > + if (!KUNIT_IS_SUPPRESSED_WARNING(__func__)) \
> > > + _BUG_FLAGS(ASM_UD2, __flags, ANNOTATE_REACHABLE(1b)); \
> > > instrumentation_end(); \
> > > } while (0)
> >
> > NAK, this grows the BUG site for now appreciable reason.
>
> Only if CONFIG_KUNIT_SUPPRESS_BACKTRACE is enabled. Why does that warrant a NACK ?
And isn't that something distros will want enabled? All I'm seeing is
bloating every single UD2 site, and no real justification. As Josh said,
this should be done on the other side of the trap if at all.
On Tue, Apr 01, 2025 at 10:53:46AM -0700, Guenter Roeck wrote: > On 4/1/25 10:08, Peter Zijlstra wrote: > > > + if (!KUNIT_IS_SUPPRESSED_WARNING(__func__)) \ > > > + _BUG_FLAGS(ASM_UD2, __flags, ANNOTATE_REACHABLE(1b)); \ > > > instrumentation_end(); \ > > > } while (0) > > > > NAK, this grows the BUG site for now appreciable reason. > > Only if CONFIG_KUNIT_SUPPRESS_BACKTRACE is enabled. Why does that > warrant a NACK ? I agree with Peter, this bloats the code around thousands of UD2 sites. It would be much better to do the checking after the exception. In fact it looks like you're already doing that in report_bug()? if (warning && KUNIT_IS_SUPPRESSED_WARNING(function)) return BUG_TRAP_TYPE_WARN; Why check it twice? -- Josh
On Thu, 13 Mar 2025 at 19:44, Alessandro Carminati <acarmina@redhat.com> wrote:
>
> From: Guenter Roeck <linux@roeck-us.net>
>
> Add name of functions triggering warning backtraces to the __bug_table
> object section to enable support for suppressing WARNING backtraces.
>
> To limit image size impact, the pointer to the function name is only added
> to the __bug_table section if both CONFIG_KUNIT_SUPPRESS_BACKTRACE and
> CONFIG_DEBUG_BUGVERBOSE are enabled. Otherwise, the __func__ assembly
> parameter is replaced with a (dummy) NULL parameter to avoid an image size
> increase due to unused __func__ entries (this is necessary because __func__
> is not a define but a virtual variable).
>
> Tested-by: Linux Kernel Functional Testing <lkft@linaro.org>
> Acked-by: Dan Carpenter <dan.carpenter@linaro.org>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: Borislav Petkov <bp@alien8.de>
> Cc: Dave Hansen <dave.hansen@linux.intel.com>
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> Signed-off-by: Alessandro Carminati <acarmina@redhat.com>
> ---
Reviewed-by: David Gow <davidgow@google.com>
Cheers,
-- David
> arch/x86/include/asm/bug.h | 21 ++++++++++++++++-----
> 1 file changed, 16 insertions(+), 5 deletions(-)
>
> diff --git a/arch/x86/include/asm/bug.h b/arch/x86/include/asm/bug.h
> index e85ac0c7c039..f6e13fc675ab 100644
> --- a/arch/x86/include/asm/bug.h
> +++ b/arch/x86/include/asm/bug.h
> @@ -35,18 +35,28 @@
>
> #ifdef CONFIG_DEBUG_BUGVERBOSE
>
> +#ifdef CONFIG_KUNIT_SUPPRESS_BACKTRACE
> +# define HAVE_BUG_FUNCTION
> +# define __BUG_FUNC_PTR __BUG_REL(%c1)
> +# define __BUG_FUNC __func__
> +#else
> +# define __BUG_FUNC_PTR
> +# define __BUG_FUNC NULL
> +#endif /* CONFIG_KUNIT_SUPPRESS_BACKTRACE */
> +
> #define _BUG_FLAGS(ins, flags, extra) \
> do { \
> asm_inline volatile("1:\t" ins "\n" \
> ".pushsection __bug_table,\"aw\"\n" \
> "2:\t" __BUG_REL(1b) "\t# bug_entry::bug_addr\n" \
> "\t" __BUG_REL(%c0) "\t# bug_entry::file\n" \
> - "\t.word %c1" "\t# bug_entry::line\n" \
> - "\t.word %c2" "\t# bug_entry::flags\n" \
> - "\t.org 2b+%c3\n" \
> + "\t" __BUG_FUNC_PTR "\t# bug_entry::function\n" \
> + "\t.word %c2" "\t# bug_entry::line\n" \
> + "\t.word %c3" "\t# bug_entry::flags\n" \
> + "\t.org 2b+%c4\n" \
> ".popsection\n" \
> extra \
> - : : "i" (__FILE__), "i" (__LINE__), \
> + : : "i" (__FILE__), "i" (__BUG_FUNC), "i" (__LINE__),\
> "i" (flags), \
> "i" (sizeof(struct bug_entry))); \
> } while (0)
> @@ -92,7 +102,8 @@ do { \
> do { \
> __auto_type __flags = BUGFLAG_WARNING|(flags); \
> instrumentation_begin(); \
> - _BUG_FLAGS(ASM_UD2, __flags, ANNOTATE_REACHABLE(1b)); \
> + if (!KUNIT_IS_SUPPRESSED_WARNING(__func__)) \
> + _BUG_FLAGS(ASM_UD2, __flags, ANNOTATE_REACHABLE(1b)); \
> instrumentation_end(); \
> } while (0)
>
> --
> 2.34.1
>
© 2016 - 2025 Red Hat, Inc.