Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1575297 > unrolled thread

[RFC/PATCH] arm64: Rename macro arguments to silence sparse

Back to article view | Back to linux.kernel

Contents

  [RFC/PATCH] arm64: Rename macro arguments to silence sparse Stephen Boyd <sboyd@codeaurora.org> - 2017-02-07 02:10 +0100
    Re: [RFC/PATCH] arm64: Rename macro arguments to silence sparse Stephen Boyd <sboyd@codeaurora.org> - 2017-02-07 02:10 +0100
      Re: [RFC/PATCH] arm64: Rename macro arguments to silence sparse Luc Van Oostenryck <luc.vanoostenryck@gmail.com> - 2017-02-07 03:00 +0100
        Re: [RFC/PATCH] arm64: Rename macro arguments to silence sparse Stephen Boyd <sboyd@codeaurora.org> - 2017-02-07 21:30 +0100
          Re: [RFC/PATCH] arm64: Rename macro arguments to silence sparse Van Oostenryck Luc <luc.vanoostenryck@gmail.com> - 2017-02-07 21:40 +0100

#1575297 — [RFC/PATCH] arm64: Rename macro arguments to silence sparse

When I compile files with sparse, I get these sorts of warnings:

arch/arm64/include/asm/lse.h:14:28: warning: Unknown escape 'l'
arch/arm64/include/asm/lse.h:14:37: warning: Unknown escape 'l'
arch/arm64/include/asm/alternative.h:172:28: warning: Unknown escape 'o'

This is because sparse is trying to tokenize these files and sees
a line like this:

 alternative_insn "\llsc", "\lse", ARM64_HAS_LSE_ATOMICS

It gets past alternative_insn part and then sees the start of a
string with the double quote character. So sparse starts to parse
the string (eat_string() in the sparse code) but the string has
an escape character '\' in it. Sparse sees the escape character,
so it checks to see if it's an escape sequence, but '\l' isn't.
This causes sparse to spit out this warning of an unknown escape
sequence 'l'.

In reality, sparse isn't going to use these macros anyway because
this whole thing is inside an __ASSEMBLER__ ifdef. One hacky
solution is to make sparse think it actually is an escape
sequence by starting the macro arguments with the 'n' character.
Then sparse will see a \n inside a string, which keeps it silent
and the assembler doesn't seem to mind either.

Cc: <sparse@vger.kernel.org>
Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
---
 arch/arm64/include/asm/alternative.h | 4 ++--
 arch/arm64/include/asm/lse.h         | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/arm64/include/asm/alternative.h b/arch/arm64/include/asm/alternative.h
index 6e1cb8c5af4d..dd393db554c8 100644
--- a/arch/arm64/include/asm/alternative.h
+++ b/arch/arm64/include/asm/alternative.h
@@ -168,8 +168,8 @@ alternative_endif
 #define _ALTERNATIVE_CFG(insn1, insn2, cap, cfg, ...)	\
 	alternative_insn insn1, insn2, cap, IS_ENABLED(cfg)
 
-.macro user_alt, label, oldinstr, newinstr, cond
-9999:	alternative_insn "\oldinstr", "\newinstr", \cond
+.macro user_alt, label, noldinstr, newinstr, cond
+9999:	alternative_insn "\noldinstr", "\newinstr", \cond
 	_ASM_EXTABLE 9999b, \label
 .endm
 
diff --git a/arch/arm64/include/asm/lse.h b/arch/arm64/include/asm/lse.h
index fc756e22c84c..36206d75943d 100644
--- a/arch/arm64/include/asm/lse.h
+++ b/arch/arm64/include/asm/lse.h
@@ -10,8 +10,8 @@
 
 .arch_extension	lse
 
-.macro alt_lse, llsc, lse
-	alternative_insn "\llsc", "\lse", ARM64_HAS_LSE_ATOMICS
+.macro alt_lse, nllsc, nlse
+	alternative_insn "\nllsc", "\nlse", ARM64_HAS_LSE_ATOMICS
 .endm
 
 #else	/* __ASSEMBLER__ */
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

[toc] | [next] | [standalone]

#1575305

On 02/06/2017 05:01 PM, Stephen Boyd wrote:
> When I compile files with sparse, I get these sorts of warnings:
>
> arch/arm64/include/asm/lse.h:14:28: warning: Unknown escape 'l'
> arch/arm64/include/asm/lse.h:14:37: warning: Unknown escape 'l'
> arch/arm64/include/asm/alternative.h:172:28: warning: Unknown escape 'o'
>
> This is because sparse is trying to tokenize these files and sees
> a line like this:
>
>  alternative_insn "\llsc", "\lse", ARM64_HAS_LSE_ATOMICS
>
> It gets past alternative_insn part and then sees the start of a
> string with the double quote character. So sparse starts to parse
> the string (eat_string() in the sparse code) but the string has
> an escape character '\' in it. Sparse sees the escape character,
> so it checks to see if it's an escape sequence, but '\l' isn't.
> This causes sparse to spit out this warning of an unknown escape
> sequence 'l'.
>
> In reality, sparse isn't going to use these macros anyway because
> this whole thing is inside an __ASSEMBLER__ ifdef. One hacky
> solution is to make sparse think it actually is an escape
> sequence by starting the macro arguments with the 'n' character.
> Then sparse will see a \n inside a string, which keeps it silent
> and the assembler doesn't seem to mind either.
>
> Cc: <sparse@vger.kernel.org>

Sorry, supposed to be <linux-sparse@vger.kernel.org>

> Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
> ---
>  arch/arm64/include/asm/alternative.h | 4 ++--
>  arch/arm64/include/asm/lse.h         | 4 ++--
>  2 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm64/include/asm/alternative.h b/arch/arm64/include/asm/alternative.h
> index 6e1cb8c5af4d..dd393db554c8 100644
> --- a/arch/arm64/include/asm/alternative.h
> +++ b/arch/arm64/include/asm/alternative.h
> @@ -168,8 +168,8 @@ alternative_endif
>  #define _ALTERNATIVE_CFG(insn1, insn2, cap, cfg, ...)	\
>  	alternative_insn insn1, insn2, cap, IS_ENABLED(cfg)
>  
> -.macro user_alt, label, oldinstr, newinstr, cond
> -9999:	alternative_insn "\oldinstr", "\newinstr", \cond
> +.macro user_alt, label, noldinstr, newinstr, cond
> +9999:	alternative_insn "\noldinstr", "\newinstr", \cond
>  	_ASM_EXTABLE 9999b, \label
>  .endm
>  
> diff --git a/arch/arm64/include/asm/lse.h b/arch/arm64/include/asm/lse.h
> index fc756e22c84c..36206d75943d 100644
> --- a/arch/arm64/include/asm/lse.h
> +++ b/arch/arm64/include/asm/lse.h
> @@ -10,8 +10,8 @@
>  
>  .arch_extension	lse
>  
> -.macro alt_lse, llsc, lse
> -	alternative_insn "\llsc", "\lse", ARM64_HAS_LSE_ATOMICS
> +.macro alt_lse, nllsc, nlse
> +	alternative_insn "\nllsc", "\nlse", ARM64_HAS_LSE_ATOMICS
>  .endm
>  
>  #else	/* __ASSEMBLER__ */

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

[toc] | [prev] | [next] | [standalone]

#1575324

On Mon, Feb 06, 2017 at 05:08:17PM -0800, Stephen Boyd wrote:
> On 02/06/2017 05:01 PM, Stephen Boyd wrote:
> > When I compile files with sparse, I get these sorts of warnings:
> >
> > arch/arm64/include/asm/lse.h:14:28: warning: Unknown escape 'l'
> > arch/arm64/include/asm/lse.h:14:37: warning: Unknown escape 'l'
> > arch/arm64/include/asm/alternative.h:172:28: warning: Unknown escape 'o'
> >
> > This is because sparse is trying to tokenize these files and sees
> > a line like this:
> >
> >  alternative_insn "\llsc", "\lse", ARM64_HAS_LSE_ATOMICS
> >
> > It gets past alternative_insn part and then sees the start of a
> > string with the double quote character. So sparse starts to parse
> > the string (eat_string() in the sparse code) but the string has
> > an escape character '\' in it. Sparse sees the escape character,
> > so it checks to see if it's an escape sequence, but '\l' isn't.
> > This causes sparse to spit out this warning of an unknown escape
> > sequence 'l'.
> >
> > In reality, sparse isn't going to use these macros anyway because
> > this whole thing is inside an __ASSEMBLER__ ifdef.

Yes, annoying. Conversion of escaped characters is supposed to be
done just after preprocessing. It's definitively a bug.

Luc


From 786877f6fa5a3b49c92ef08b28c19e58b75ba8e8 Mon Sep 17 00:00:00 2001
From: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
Date: Tue, 7 Feb 2017 02:37:00 +0100
Subject: [PATCH] add testcase for wrong early escape conversion

Reported-by: Stephen Boyd <sboyd@codeaurora.org>
Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 validation/preprocessor/early-escape.c | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)
 create mode 100644 validation/preprocessor/early-escape.c

diff --git a/validation/preprocessor/early-escape.c b/validation/preprocessor/early-escape.c
new file mode 100644
index 000000000..c7beba5d8
--- /dev/null
+++ b/validation/preprocessor/early-escape.c
@@ -0,0 +1,23 @@
+#if 0
+"\l"
+#endif
+
+/*
+ * check-description:
+ *	Following the C standard, escape conversion must be
+ *	done in phase 5, just after preprocessing and just
+ *	before string concatenation. So we're not supposed
+ *	to receive a diagnostic for an unknown escape char
+ *	for a token which is excluded by the preprocessor.
+ * check-name: early-escape
+ * check-command: sparse -E $file
+ * check-known-to-fail
+ *
+ * check-output-start
+
+
+ * check-output-end
+ *
+ * check-error-start
+ * check-error-end
+ */
-- 
2.11.0

[toc] | [prev] | [next] | [standalone]

#1576028

On 02/06/2017 05:50 PM, Luc Van Oostenryck wrote:
> On Mon, Feb 06, 2017 at 05:08:17PM -0800, Stephen Boyd wrote:
>> On 02/06/2017 05:01 PM, Stephen Boyd wrote:
>>> When I compile files with sparse, I get these sorts of warnings:
>>>
>>> arch/arm64/include/asm/lse.h:14:28: warning: Unknown escape 'l'
>>> arch/arm64/include/asm/lse.h:14:37: warning: Unknown escape 'l'
>>> arch/arm64/include/asm/alternative.h:172:28: warning: Unknown escape 'o'
>>>
>>> This is because sparse is trying to tokenize these files and sees
>>> a line like this:
>>>
>>>  alternative_insn "\llsc", "\lse", ARM64_HAS_LSE_ATOMICS
>>>
>>> It gets past alternative_insn part and then sees the start of a
>>> string with the double quote character. So sparse starts to parse
>>> the string (eat_string() in the sparse code) but the string has
>>> an escape character '\' in it. Sparse sees the escape character,
>>> so it checks to see if it's an escape sequence, but '\l' isn't.
>>> This causes sparse to spit out this warning of an unknown escape
>>> sequence 'l'.
>>>
>>> In reality, sparse isn't going to use these macros anyway because
>>> this whole thing is inside an __ASSEMBLER__ ifdef.
> Yes, annoying. Conversion of escaped characters is supposed to be
> done just after preprocessing. It's definitively a bug.

Ok. Thanks for the fixes to sparse. My hack patch can be safely ignored.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

[toc] | [prev] | [next] | [standalone]

#1576037

On Tue, Feb 7, 2017 at 9:11 PM, Stephen Boyd <sboyd@codeaurora.org> wrote:
> On 02/06/2017 05:50 PM, Luc Van Oostenryck wrote:
>> On Mon, Feb 06, 2017 at 05:08:17PM -0800, Stephen Boyd wrote:
>>> On 02/06/2017 05:01 PM, Stephen Boyd wrote:
>>>> When I compile files with sparse, I get these sorts of warnings:
>>>>
>>>> arch/arm64/include/asm/lse.h:14:28: warning: Unknown escape 'l'
>>>> arch/arm64/include/asm/lse.h:14:37: warning: Unknown escape 'l'
>>>> arch/arm64/include/asm/alternative.h:172:28: warning: Unknown escape 'o'
>>>>
>>>> This is because sparse is trying to tokenize these files and sees
>>>> a line like this:
>>>>
>>>>  alternative_insn "\llsc", "\lse", ARM64_HAS_LSE_ATOMICS
>>>>
>>>> It gets past alternative_insn part and then sees the start of a
>>>> string with the double quote character. So sparse starts to parse
>>>> the string (eat_string() in the sparse code) but the string has
>>>> an escape character '\' in it. Sparse sees the escape character,
>>>> so it checks to see if it's an escape sequence, but '\l' isn't.
>>>> This causes sparse to spit out this warning of an unknown escape
>>>> sequence 'l'.
>>>>
>>>> In reality, sparse isn't going to use these macros anyway because
>>>> this whole thing is inside an __ASSEMBLER__ ifdef.
>> Yes, annoying. Conversion of escaped characters is supposed to be
>> done just after preprocessing. It's definitively a bug.
>
> Ok. Thanks for the fixes to sparse. My hack patch can be safely ignored.

Well, I don't know.
Maybe it's still useful for everyone using the official version of sparse
or an older one.

Luc Van Oostenryck

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web