[PATCH] loongarch: Use '.ascii' instead of '.string' in __BUGVERBOSE_LOCATION

Nathan Chancellor posted 1 patch 3 months, 3 weeks ago
arch/loongarch/include/asm/bug.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
[PATCH] loongarch: Use '.ascii' instead of '.string' in __BUGVERBOSE_LOCATION
Posted by Nathan Chancellor 3 months, 3 weeks ago
After commit 5d476f66e6ad ("bugs/LoongArch: Concatenate 'cond_str' with
'__FILE__' in __BUG_ENTRY(), to extend WARN_ON/BUG_ON output"), building
loongarch with clang's integrated assembler fails with:

  block/blk-throttle.c:205:2: error: unexpected token
    205 |         WARN_ON_ONCE(!bio);
        |         ^
  include/asm-generic/bug.h:119:3: note: expanded from macro 'WARN_ON_ONCE'
    119 |                 __WARN_FLAGS("["#condition"] ",                 \
        |                 ^
  arch/loongarch/include/asm/bug.h:47:2: note: expanded from macro '__WARN_FLAGS'
     47 |         __BUG_FLAGS(cond_str, BUGFLAG_WARNING|(flags), ANNOTATE_REACHABLE(10001b));\
        |         ^
  arch/loongarch/include/asm/bug.h:42:23: note: expanded from macro '__BUG_FLAGS'
     42 |         asm_inline volatile (__stringify(ASM_BUG_FLAGS(cond_str, flags)) extra);
        |                              ^
  ...
  <inline asm>:1:134: note: instantiated into assembly here
      1 |         .pushsection __bug_table, "aw"; .align 2; 10000: .long 10001f - .; .pushsection .rodata.str, "aMS", @progbits, 1; 10002: .string "[""!bio""] " "block/blk-throttle.c"; .popsection; .long 10002b - .; .short 205; .short (1 << 0)|((1 << 1) | ((9) << 8)); .popsection; 10001: break 1;.pushsection .discard.annotate_insn,"M",@progbits,8
        |                                                                                                                                             ^

clang's integrated assembler only supports concatenating strings with
'.ascii'. There was discussion of allowing '.string' / '.asciz' but it
was decided against [1] because it was undesirable to match the behavior
of released binutils at the time, where

  .asciz "foo" "bar"

was treated like

  "foo\0bar\0"

instead of the more intuitive to C programmers

  "foobar\0"

and diverging from that behavior was seen as risky and unnecessary. GNU
binutils updated its handling of '.asciz' and '.string' to the more
intuitive option in 2.36 [2] after this was brought up to them in
passing.

Convert LoongArch's __BUGVERBOSE_LOCATION from '.string' to '.ascii'
with a manually added NUL, similar to commit 707f853d7fa3 ("module:
Provide EXPORT_SYMBOL_GPL_FOR_MODULES() helper") for the same reason,
which clears up the build error and results in no changes in output for
builds with GNU binutils.

Fixes: 5d476f66e6ad ("bugs/LoongArch: Concatenate 'cond_str' with '__FILE__' in __BUG_ENTRY(), to extend WARN_ON/BUG_ON output")
Link: https://reviews.llvm.org/D91460#2398228 [1]
Link: https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=3d955acb36f483c05724181da5ffba46b1303c43 [2]
Signed-off-by: Nathan Chancellor <nathan@kernel.org>
---
 arch/loongarch/include/asm/bug.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/loongarch/include/asm/bug.h b/arch/loongarch/include/asm/bug.h
index cad807b100ad..d090a5bec5eb 100644
--- a/arch/loongarch/include/asm/bug.h
+++ b/arch/loongarch/include/asm/bug.h
@@ -11,7 +11,7 @@
 #else
 #define __BUGVERBOSE_LOCATION(file, line)			\
 		.pushsection .rodata.str, "aMS", @progbits, 1;	\
-	10002:	.string file;					\
+	10002:	.ascii file "\0";				\
 		.popsection;					\
 								\
 		.long 10002b - .;				\

---
base-commit: d298bb98d65f964288bb87feef014da1baafedda
change-id: 20250616-loongarch-fix-warn-cond-llvm-ias-b954f02c446c

Best regards,
--  
Nathan Chancellor <nathan@kernel.org>
Re: [PATCH] loongarch: Use '.ascii' instead of '.string' in __BUGVERBOSE_LOCATION
Posted by Huacai Chen 3 months, 1 week ago
On Tue, Jun 17, 2025 at 7:23 AM Nathan Chancellor <nathan@kernel.org> wrote:
>
> After commit 5d476f66e6ad ("bugs/LoongArch: Concatenate 'cond_str' with
> '__FILE__' in __BUG_ENTRY(), to extend WARN_ON/BUG_ON output"), building
> loongarch with clang's integrated assembler fails with:
>
>   block/blk-throttle.c:205:2: error: unexpected token
>     205 |         WARN_ON_ONCE(!bio);
>         |         ^
>   include/asm-generic/bug.h:119:3: note: expanded from macro 'WARN_ON_ONCE'
>     119 |                 __WARN_FLAGS("["#condition"] ",                 \
>         |                 ^
>   arch/loongarch/include/asm/bug.h:47:2: note: expanded from macro '__WARN_FLAGS'
>      47 |         __BUG_FLAGS(cond_str, BUGFLAG_WARNING|(flags), ANNOTATE_REACHABLE(10001b));\
>         |         ^
>   arch/loongarch/include/asm/bug.h:42:23: note: expanded from macro '__BUG_FLAGS'
>      42 |         asm_inline volatile (__stringify(ASM_BUG_FLAGS(cond_str, flags)) extra);
>         |                              ^
>   ...
>   <inline asm>:1:134: note: instantiated into assembly here
>       1 |         .pushsection __bug_table, "aw"; .align 2; 10000: .long 10001f - .; .pushsection .rodata.str, "aMS", @progbits, 1; 10002: .string "[""!bio""] " "block/blk-throttle.c"; .popsection; .long 10002b - .; .short 205; .short (1 << 0)|((1 << 1) | ((9) << 8)); .popsection; 10001: break 1;.pushsection .discard.annotate_insn,"M",@progbits,8
>         |                                                                                                                                             ^
>
> clang's integrated assembler only supports concatenating strings with
> '.ascii'. There was discussion of allowing '.string' / '.asciz' but it
> was decided against [1] because it was undesirable to match the behavior
> of released binutils at the time, where
>
>   .asciz "foo" "bar"
>
> was treated like
>
>   "foo\0bar\0"
>
> instead of the more intuitive to C programmers
>
>   "foobar\0"
>
> and diverging from that behavior was seen as risky and unnecessary. GNU
> binutils updated its handling of '.asciz' and '.string' to the more
> intuitive option in 2.36 [2] after this was brought up to them in
> passing.
>
> Convert LoongArch's __BUGVERBOSE_LOCATION from '.string' to '.ascii'
> with a manually added NUL, similar to commit 707f853d7fa3 ("module:
> Provide EXPORT_SYMBOL_GPL_FOR_MODULES() helper") for the same reason,
> which clears up the build error and results in no changes in output for
> builds with GNU binutils.
>
> Fixes: 5d476f66e6ad ("bugs/LoongArch: Concatenate 'cond_str' with '__FILE__' in __BUG_ENTRY(), to extend WARN_ON/BUG_ON output")
5d476f66e6ad is not in upstream, so I prefer squash this patch to 5d476f66e6ad.

Huacai

> Link: https://reviews.llvm.org/D91460#2398228 [1]
> Link: https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=3d955acb36f483c05724181da5ffba46b1303c43 [2]
> Signed-off-by: Nathan Chancellor <nathan@kernel.org>
> ---
>  arch/loongarch/include/asm/bug.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/loongarch/include/asm/bug.h b/arch/loongarch/include/asm/bug.h
> index cad807b100ad..d090a5bec5eb 100644
> --- a/arch/loongarch/include/asm/bug.h
> +++ b/arch/loongarch/include/asm/bug.h
> @@ -11,7 +11,7 @@
>  #else
>  #define __BUGVERBOSE_LOCATION(file, line)                      \
>                 .pushsection .rodata.str, "aMS", @progbits, 1;  \
> -       10002:  .string file;                                   \
> +       10002:  .ascii file "\0";                               \
>                 .popsection;                                    \
>                                                                 \
>                 .long 10002b - .;                               \
>
> ---
> base-commit: d298bb98d65f964288bb87feef014da1baafedda
> change-id: 20250616-loongarch-fix-warn-cond-llvm-ias-b954f02c446c
>
> Best regards,
> --
> Nathan Chancellor <nathan@kernel.org>
>
Re: [PATCH] loongarch: Use '.ascii' instead of '.string' in __BUGVERBOSE_LOCATION
Posted by Nathan Chancellor 3 months, 1 week ago
On Sun, Jun 29, 2025 at 11:59:57AM +0800, Huacai Chen wrote:
> On Tue, Jun 17, 2025 at 7:23 AM Nathan Chancellor <nathan@kernel.org> wrote:
> > Fixes: 5d476f66e6ad ("bugs/LoongArch: Concatenate 'cond_str' with '__FILE__' in __BUG_ENTRY(), to extend WARN_ON/BUG_ON output")
> 
> 5d476f66e6ad is not in upstream, so I prefer squash this patch to 5d476f66e6ad.

Sure, that's fine with me, assuming Ingo is fine with rebasing that
branch. I just want my builds back to green :)

Cheers,
Nathan