2022-05-09 09:59:29

by Josh Poimboeuf

[permalink] [raw]
Subject: [PATCH] bug: Use normal relative pointers in 'struct bug_entry'

With CONFIG_GENERIC_BUG_RELATIVE_POINTERS, the addr/file relative
pointers are calculated weirdly: based on the beginning of the bug_entry
struct address, rather than their respective pointer addresses.

Make the relative pointers less surprising to both humans and tools by
calculating them the normal way.

Signed-off-by: Josh Poimboeuf <[email protected]>
---
arch/arm64/include/asm/asm-bug.h | 4 ++--
arch/powerpc/include/asm/bug.h | 5 +++--
arch/riscv/include/asm/bug.h | 4 ++--
arch/s390/include/asm/bug.h | 5 +++--
arch/x86/include/asm/bug.h | 2 +-
lib/bug.c | 15 +++++++--------
6 files changed, 18 insertions(+), 17 deletions(-)

diff --git a/arch/arm64/include/asm/asm-bug.h b/arch/arm64/include/asm/asm-bug.h
index 03f52f84a4f3..c762038ba400 100644
--- a/arch/arm64/include/asm/asm-bug.h
+++ b/arch/arm64/include/asm/asm-bug.h
@@ -14,7 +14,7 @@
14472: .string file; \
.popsection; \
\
- .long 14472b - 14470b; \
+ .long 14472b - .; \
.short line;
#else
#define _BUGVERBOSE_LOCATION(file, line)
@@ -25,7 +25,7 @@
#define __BUG_ENTRY(flags) \
.pushsection __bug_table,"aw"; \
.align 2; \
- 14470: .long 14471f - 14470b; \
+ 14470: .long 14471f - .; \
_BUGVERBOSE_LOCATION(__FILE__, __LINE__) \
.short flags; \
.popsection; \
diff --git a/arch/powerpc/include/asm/bug.h b/arch/powerpc/include/asm/bug.h
index ecbae1832de3..76252576d889 100644
--- a/arch/powerpc/include/asm/bug.h
+++ b/arch/powerpc/include/asm/bug.h
@@ -13,7 +13,8 @@
#ifdef CONFIG_DEBUG_BUGVERBOSE
.macro __EMIT_BUG_ENTRY addr,file,line,flags
.section __bug_table,"aw"
-5001: .4byte \addr - 5001b, 5002f - 5001b
+5001: .4byte \addr - .
+ .4byte 5002f - .
.short \line, \flags
.org 5001b+BUG_ENTRY_SIZE
.previous
@@ -24,7 +25,7 @@
#else
.macro __EMIT_BUG_ENTRY addr,file,line,flags
.section __bug_table,"aw"
-5001: .4byte \addr - 5001b
+5001: .4byte \addr - .
.short \flags
.org 5001b+BUG_ENTRY_SIZE
.previous
diff --git a/arch/riscv/include/asm/bug.h b/arch/riscv/include/asm/bug.h
index d3804a2f9aad..1aaea81fb141 100644
--- a/arch/riscv/include/asm/bug.h
+++ b/arch/riscv/include/asm/bug.h
@@ -30,8 +30,8 @@
typedef u32 bug_insn_t;

#ifdef CONFIG_GENERIC_BUG_RELATIVE_POINTERS
-#define __BUG_ENTRY_ADDR RISCV_INT " 1b - 2b"
-#define __BUG_ENTRY_FILE RISCV_INT " %0 - 2b"
+#define __BUG_ENTRY_ADDR RISCV_INT " 1b - ."
+#define __BUG_ENTRY_FILE RISCV_INT " %0 - ."
#else
#define __BUG_ENTRY_ADDR RISCV_PTR " 1b"
#define __BUG_ENTRY_FILE RISCV_PTR " %0"
diff --git a/arch/s390/include/asm/bug.h b/arch/s390/include/asm/bug.h
index 0b25f28351ed..aebe1e22c7be 100644
--- a/arch/s390/include/asm/bug.h
+++ b/arch/s390/include/asm/bug.h
@@ -15,7 +15,8 @@
"1: .asciz \""__FILE__"\"\n" \
".previous\n" \
".section __bug_table,\"awM\",@progbits,%2\n" \
- "2: .long 0b-2b,1b-2b\n" \
+ "2: .long 0b-.\n" \
+ " .long 1b-.\n" \
" .short %0,%1\n" \
" .org 2b+%2\n" \
".previous\n" \
@@ -30,7 +31,7 @@
asm_inline volatile( \
"0: mc 0,0\n" \
".section __bug_table,\"awM\",@progbits,%1\n" \
- "1: .long 0b-1b\n" \
+ "1: .long 0b-.\n" \
" .short %0\n" \
" .org 1b+%1\n" \
".previous\n" \
diff --git a/arch/x86/include/asm/bug.h b/arch/x86/include/asm/bug.h
index aaf0cb0db4ae..a3ec87d198ac 100644
--- a/arch/x86/include/asm/bug.h
+++ b/arch/x86/include/asm/bug.h
@@ -18,7 +18,7 @@
#ifdef CONFIG_X86_32
# define __BUG_REL(val) ".long " __stringify(val)
#else
-# define __BUG_REL(val) ".long " __stringify(val) " - 2b"
+# define __BUG_REL(val) ".long " __stringify(val) " - ."
#endif

#ifdef CONFIG_DEBUG_BUGVERBOSE
diff --git a/lib/bug.c b/lib/bug.c
index 45a0584f6541..c223a2575b72 100644
--- a/lib/bug.c
+++ b/lib/bug.c
@@ -6,8 +6,7 @@

CONFIG_BUG - emit BUG traps. Nothing happens without this.
CONFIG_GENERIC_BUG - enable this code.
- CONFIG_GENERIC_BUG_RELATIVE_POINTERS - use 32-bit pointers relative to
- the containing struct bug_entry for bug_addr and file.
+ CONFIG_GENERIC_BUG_RELATIVE_POINTERS - use 32-bit relative pointers for bug_addr and file
CONFIG_DEBUG_BUGVERBOSE - emit full file+line information for each BUG

CONFIG_BUG and CONFIG_DEBUG_BUGVERBOSE are potentially user-settable
@@ -53,10 +52,10 @@ extern struct bug_entry __start___bug_table[], __stop___bug_table[];

static inline unsigned long bug_addr(const struct bug_entry *bug)
{
-#ifndef CONFIG_GENERIC_BUG_RELATIVE_POINTERS
- return bug->bug_addr;
+#ifdef CONFIG_GENERIC_BUG_RELATIVE_POINTERS
+ return (unsigned long)&bug->bug_addr_disp + bug->bug_addr_disp;
#else
- return (unsigned long)bug + bug->bug_addr_disp;
+ return bug->bug_addr;
#endif
}

@@ -131,10 +130,10 @@ void bug_get_file_line(struct bug_entry *bug, const char **file,
unsigned int *line)
{
#ifdef CONFIG_DEBUG_BUGVERBOSE
-#ifndef CONFIG_GENERIC_BUG_RELATIVE_POINTERS
- *file = bug->file;
+#ifdef CONFIG_GENERIC_BUG_RELATIVE_POINTERS
+ *file = (const char *)&bug->file_disp + bug->file_disp;
#else
- *file = (const char *)bug + bug->file_disp;
+ *file = bug->file;
#endif
*line = bug->line;
#else
--
2.34.1



2022-05-09 12:45:05

by Michael Ellerman

[permalink] [raw]
Subject: Re: [PATCH] bug: Use normal relative pointers in 'struct bug_entry'

Josh Poimboeuf <[email protected]> writes:
> With CONFIG_GENERIC_BUG_RELATIVE_POINTERS, the addr/file relative
> pointers are calculated weirdly: based on the beginning of the bug_entry
> struct address, rather than their respective pointer addresses.
>
> Make the relative pointers less surprising to both humans and tools by
> calculating them the normal way.
>
> Signed-off-by: Josh Poimboeuf <[email protected]>
> ---
...
> diff --git a/arch/powerpc/include/asm/bug.h b/arch/powerpc/include/asm/bug.h
> index ecbae1832de3..76252576d889 100644
> --- a/arch/powerpc/include/asm/bug.h
> +++ b/arch/powerpc/include/asm/bug.h
> @@ -13,7 +13,8 @@
> #ifdef CONFIG_DEBUG_BUGVERBOSE
> .macro __EMIT_BUG_ENTRY addr,file,line,flags
> .section __bug_table,"aw"
> -5001: .4byte \addr - 5001b, 5002f - 5001b
> +5001: .4byte \addr - .
> + .4byte 5002f - .
> .short \line, \flags
> .org 5001b+BUG_ENTRY_SIZE
> .previous
> @@ -24,7 +25,7 @@
> #else
> .macro __EMIT_BUG_ENTRY addr,file,line,flags
> .section __bug_table,"aw"
> -5001: .4byte \addr - 5001b
> +5001: .4byte \addr - .
> .short \flags
> .org 5001b+BUG_ENTRY_SIZE
> .previous

Embarrassingly, we have another copy of the logic, used in the C
versions, they need updating too:

diff --git a/arch/powerpc/include/asm/bug.h b/arch/powerpc/include/asm/bug.h
index ecbae1832de3..3fde35fd67f8 100644
--- a/arch/powerpc/include/asm/bug.h
+++ b/arch/powerpc/include/asm/bug.h
@@ -49,14 +49,14 @@
#ifdef CONFIG_DEBUG_BUGVERBOSE
#define _EMIT_BUG_ENTRY \
".section __bug_table,\"aw\"\n" \
- "2:\t.4byte 1b - 2b, %0 - 2b\n" \
+ "2:\t.4byte 1b - ., %0 - .\n" \
"\t.short %1, %2\n" \
".org 2b+%3\n" \
".previous\n"
#else
#define _EMIT_BUG_ENTRY \
".section __bug_table,\"aw\"\n" \
- "2:\t.4byte 1b - 2b\n" \
+ "2:\t.4byte 1b - .\n" \
"\t.short %2\n" \
".org 2b+%3\n" \
".previous\n"


With that added it seems to be working correctly for me.

Acked-by: Michael Ellerman <[email protected]> (powerpc)


cheers

2022-05-09 19:29:16

by Josh Poimboeuf

[permalink] [raw]
Subject: Re: [PATCH] bug: Use normal relative pointers in 'struct bug_entry'

On Mon, May 09, 2022 at 10:31:14PM +1000, Michael Ellerman wrote:
> Embarrassingly, we have another copy of the logic, used in the C
> versions, they need updating too:

Oops, thanks for finding that.

> With that added it seems to be working correctly for me.
>
> Acked-by: Michael Ellerman <[email protected]> (powerpc)

Thanks!

--
Josh

2022-05-10 11:08:22

by Catalin Marinas

[permalink] [raw]
Subject: Re: [PATCH] bug: Use normal relative pointers in 'struct bug_entry'

On Thu, May 05, 2022 at 06:09:45PM -0700, Josh Poimboeuf wrote:
> With CONFIG_GENERIC_BUG_RELATIVE_POINTERS, the addr/file relative
> pointers are calculated weirdly: based on the beginning of the bug_entry
> struct address, rather than their respective pointer addresses.
>
> Make the relative pointers less surprising to both humans and tools by
> calculating them the normal way.
>
> Signed-off-by: Josh Poimboeuf <[email protected]>
> ---
> arch/arm64/include/asm/asm-bug.h | 4 ++--

Acked-by: Catalin Marinas <[email protected]>