arm64/debug: don't duplicate filenames.

Rather than explicitly pushing the filename into .rodata.str, use a compiler generated string literal and use the address of that as an input constraint to the inline assembly. This allows the compiler to emit only one version of the string without relying on the linker to deduplicate.

Cc Dave P Martin <[email protected]>

Cc Catalin Marinas <[email protected]>

Cc Will Deacon <[email protected]>

Signed-off-by Jamie Iles <[email protected]>

---
 arch/arm64/include/asm/bug.h | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

arch/arm64/include/asm/bug.h +5 -7

index a02a57186f56..a7b05625ef63 100644
@@ -23,12 +23,8 @@
 #ifdef CONFIG_DEBUG_BUGVERBOSE
 #define _BUGVERBOSE_LOCATION(file, line) __BUGVERBOSE_LOCATION(file, line)
 #define __BUGVERBOSE_LOCATION(file, line)				\
-		".pushsection .rodata.str,\"aMS\", at progbits,1\n"	\
-	"2:	.string \"" file "\"\n\t"				\
-		".popsection\n\t"					\
-									\
-		".long 2b - 0b\n\t"					\
-		".short " #line "\n\t"
+		".long %[file] - 0b\n\t"				\
+		".short %[line]\n\t"
 #else
 #define _BUGVERBOSE_LOCATION(file, line)
 #endif

@@ -50,7 +46,9 @@ _BUGVERBOSE_LOCATION(__FILE__, __LINE__)		\
 #define __BUG_FLAGS(flags)				\
 	asm volatile (					\
 		__BUG_ENTRY(flags)			\
-		"brk %[imm]" :: [imm] "i" (BUG_BRK_IMM)	\
+		"brk %[imm]" :: [imm] "i" (BUG_BRK_IMM),\
+			        [line] "i" (__LINE__),  \
+			        [file] "i" (__FILE__)   \
 	);
 
 

Signature · 7 lines

2.15.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Quoted 5 lines · Jamie Iles

Rather than explicitly pushing the filename into .rodata.str, use a compiler generated string literal and use the address of that as an input constraint to the inline assembly. This allows the compiler to emit only one version of the string without relying on the linker to deduplicate.

But if the linker does deduplicate, why does it matter?

Or is the linker not removing duplicates that some from the same source file?

Hi Dave,

Quoted 8 lines · Dave Martin

Quoted 5 lines · Jamie Iles

Rather than explicitly pushing the filename into .rodata.str, use a compiler generated string literal and use the address of that as an input constraint to the inline assembly. This allows the compiler to emit only one version of the string without relying on the linker to deduplicate.

But if the linker does deduplicate, why does it matter?

It's not broken, but at the moment we have multiple copies in the object file then we're relying on the linker. If we use the same pattern as X86, we can let GCC assign sections+attributes for us and only get one copy in the object file.

Quoted 2 lines

Or is the linker not removing duplicates that some from the same source file?

In my testing, yes, the linker is merging them.

Quoted 17 lines · Jamie Iles

Hi Dave,

Quoted 8 lines · Dave Martin

Quoted 5 lines · Jamie Iles

Rather than explicitly pushing the filename into .rodata.str, use a compiler generated string literal and use the address of that as an input constraint to the inline assembly. This allows the compiler to emit only one version of the string without relying on the linker to deduplicate.

But if the linker does deduplicate, why does it matter?

It's not broken, but at the moment we have multiple copies in the object file then we're relying on the linker. If we use the same pattern as X86, we can let GCC assign sections+attributes for us and only get one copy in the object file.

Quoted 4 lines

Quoted 2 lines

Or is the linker not removing duplicates that some from the same source file?

In my testing, yes, the linker is merging them.

So, playing devil's advocate here, what is this fixing?

It doesn't really matter where in the toolchain the duplicates get removed, just that it happens somewhere...

It looks like ld -r doesn't do the deduplication, so there may be a fair amount of duplicates before the final linux of vmlinux -- which I guess could be an issue if BUG() is used in macros that get expanded all over the place.

Quoted 4 lines · Dave Martin

Quoted 3 lines · Jamie Iles

Hi Dave,

On Thu, Nov 09, 2017 at 12:35:04PM +0000, Dave Martin wrote:

Quoted 6 lines

Quoted 4 lines

Quoted 2 lines

Or is the linker not removing duplicates that some from the same source file?

In my testing, yes, the linker is merging them.

So, playing devil's advocate here, what is this fixing?

It's caused some confusion looking at object files, it's certainly not a critical bug!

Quoted 7 lines

It doesn't really matter where in the toolchain the duplicates get removed, just that it happens somewhere...

It looks like ld -r doesn't do the deduplication, so there may be a fair amount of duplicates before the final linux of vmlinux -- which I guess could be an issue if BUG() is used in macros that get expanded all over the place.

Ah, interesting. Testing with a loadable module, this means that there are duplicates in modules. With mainline:

strings ./fs/ext4/ext4.ko | grep -c "inode\.c"
61

and with my patch:

strings ./fs/ext4/ext4.ko | grep -c "inode\.c"
4

Thanks,

Jamie

Rather than explicitly pushing the filename into .rodata.str, use a compiler generated string literal and use the address of that as an input constraint to the inline assembly. This allows the compiler to emit only one version of the string without relying on the linker to deduplicate.

Cc Dave P Martin <[email protected]>

Cc Catalin Marinas <[email protected]>

Cc Will Deacon <[email protected]>

Signed-off-by Jamie Iles <[email protected]>

---
 arch/arm64/include/asm/bug.h | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

arch/arm64/include/asm/bug.h +2 -6

index a02a57186f56..a7b05625ef63 100644
@@ -23,12 +23,8 @@
 #ifdef CONFIG_DEBUG_BUGVERBOSE
 #define _BUGVERBOSE_LOCATION(file, line) __BUGVERBOSE_LOCATION(file, line)
 #define __BUGVERBOSE_LOCATION(file, line)				\
-		".pushsection .rodata.str,\"aMS\", at progbits,1\n"	\
-	"2:	.string \"" file "\"\n\t"				\
-		".popsection\n\t"					\
-									\
-		".long 2b - 0b\n\t"					\
-		".short " #line "\n\t"
+		".long %[file] - 0b\n\t"				\
+		".short %[line]\n\t"
 #else
 #define _BUGVERBOSE_LOCATION(file, line)
 #endif