* WARN_ON() is buggy for 32 bit systems
@ 2022-01-26 11:56 Dan Carpenter
2022-01-26 12:21 ` Christophe Leroy
0 siblings, 1 reply; 5+ messages in thread
From: Dan Carpenter @ 2022-01-26 11:56 UTC (permalink / raw)
To: Michael Ellerman; +Cc: linuxppc-dev
Hi Michael,
Commit e432fe97f3e5 ("powerpc/bug: Cast to unsigned long before passing
to inline asm") breaks WARN_ON() for 32 bit systems.
arch/powerpc/include/asm/bug.h
109 #define WARN_ON(x) ({ \
110 bool __ret_warn_on = false; \
111 do { \
112 if (__builtin_constant_p((x))) { \
113 if (!(x)) \
114 break; \
115 __WARN(); \
116 __ret_warn_on = true; \
117 } else { \
118 __label__ __label_warn_on; \
119 \
120 WARN_ENTRY(PPC_TLNEI " %4, 0", \
121 BUGFLAG_WARNING | BUGFLAG_TAINT(TAINT_WARN), \
122 __label_warn_on, \
123 "r" ((__force long)(x))); \
^^^^
If the code is "if (WARN_ON(some_u64)) {" then the cast to long will
truncate away the high bits so it's wrong. (Or at least that's how it
works on x86, I'm working on a work around for Smatch to be able to
parse this WARN_ON(). I don't know anything about PowerPC.)
124 break; \
125 __label_warn_on: \
126 __ret_warn_on = true; \
127 } \
128 } while (0); \
129 unlikely(__ret_warn_on); \
130 })
regards,
dan carpenter
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: WARN_ON() is buggy for 32 bit systems
2022-01-26 11:56 WARN_ON() is buggy for 32 bit systems Dan Carpenter
@ 2022-01-26 12:21 ` Christophe Leroy
2022-01-26 13:49 ` Dan Carpenter
0 siblings, 1 reply; 5+ messages in thread
From: Christophe Leroy @ 2022-01-26 12:21 UTC (permalink / raw)
To: Dan Carpenter, Michael Ellerman; +Cc: linuxppc-dev@lists.ozlabs.org
Hi Dan,
Le 26/01/2022 à 12:56, Dan Carpenter a écrit :
>
> Hi Michael,
>
> Commit e432fe97f3e5 ("powerpc/bug: Cast to unsigned long before passing
> to inline asm") breaks WARN_ON() for 32 bit systems.
I think you missed commit db87a7199229 ("powerpc/bug: Remove specific
powerpc BUG_ON() and WARN_ON() on PPC32")
>
> arch/powerpc/include/asm/bug.h
> 109 #define WARN_ON(x) ({ \
> 110 bool __ret_warn_on = false; \
> 111 do { \
> 112 if (__builtin_constant_p((x))) { \
> 113 if (!(x)) \
> 114 break; \
> 115 __WARN(); \
> 116 __ret_warn_on = true; \
> 117 } else { \
> 118 __label__ __label_warn_on; \
> 119 \
> 120 WARN_ENTRY(PPC_TLNEI " %4, 0", \
> 121 BUGFLAG_WARNING | BUGFLAG_TAINT(TAINT_WARN), \
> 122 __label_warn_on, \
> 123 "r" ((__force long)(x))); \
> ^^^^
> If the code is "if (WARN_ON(some_u64)) {" then the cast to long will
> truncate away the high bits so it's wrong. (Or at least that's how it
> works on x86, I'm working on a work around for Smatch to be able to
> parse this WARN_ON(). I don't know anything about PowerPC.)
The code is enclosed in a #ifdef CONFIG_PPC64, it is not used for PPC32:
/arch/powerpc/include/asm/bug.h
99 #ifdef CONFIG_PPC64
100 #define BUG_ON(x) do { \
...
109 #define WARN_ON(x) ({ \
...
132 #define HAVE_ARCH_BUG_ON
133 #define HAVE_ARCH_WARN_ON
134 #endif
Christophe
>
> 124 break; \
> 125 __label_warn_on: \
> 126 __ret_warn_on = true; \
> 127 } \
> 128 } while (0); \
> 129 unlikely(__ret_warn_on); \
> 130 })
>
> regards,
> dan carpenter
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: WARN_ON() is buggy for 32 bit systems
2022-01-26 12:21 ` Christophe Leroy
@ 2022-01-26 13:49 ` Dan Carpenter
2022-01-27 11:10 ` Michael Ellerman
0 siblings, 1 reply; 5+ messages in thread
From: Dan Carpenter @ 2022-01-26 13:49 UTC (permalink / raw)
To: Christophe Leroy; +Cc: linuxppc-dev@lists.ozlabs.org
On Wed, Jan 26, 2022 at 12:21:49PM +0000, Christophe Leroy wrote:
> The code is enclosed in a #ifdef CONFIG_PPC64, it is not used for PPC32:
>
> /arch/powerpc/include/asm/bug.h
> 99 #ifdef CONFIG_PPC64
Ah...
You know, life would be a lot easier for me personally if we added an
#ifndef __CHECKER__ as well... I can't compile PowerPC code so I can't
test a patch like that.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: WARN_ON() is buggy for 32 bit systems
2022-01-26 13:49 ` Dan Carpenter
@ 2022-01-27 11:10 ` Michael Ellerman
2022-01-27 13:37 ` Dan Carpenter
0 siblings, 1 reply; 5+ messages in thread
From: Michael Ellerman @ 2022-01-27 11:10 UTC (permalink / raw)
To: Dan Carpenter, Christophe Leroy; +Cc: linuxppc-dev@lists.ozlabs.org
Dan Carpenter <dan.carpenter@oracle.com> writes:
> On Wed, Jan 26, 2022 at 12:21:49PM +0000, Christophe Leroy wrote:
>> The code is enclosed in a #ifdef CONFIG_PPC64, it is not used for PPC32:
>>
>> /arch/powerpc/include/asm/bug.h
>> 99 #ifdef CONFIG_PPC64
>
> Ah...
>
> You know, life would be a lot easier for me personally if we added an
> #ifndef __CHECKER__ as well... I can't compile PowerPC code so I can't
> test a patch like that.
Ubuntu & Fedora both have cross compilers packaged, or there's cross
compilers on kernel.org. But I assume you mean you'd rather not bother
compiling for powerpc, which is fair enough.
Do you mean something like below?
I'm not sure about that, as it would prevent sparse from checking the
actual BUG_ON code we're using, vs the generic version which we never
use on 64-bit. Is there a smatch specific macro we could check?
cheers
diff --git a/arch/powerpc/include/asm/bug.h b/arch/powerpc/include/asm/bug.h
index 02c08d1492f8..5cbfe9d8232d 100644
--- a/arch/powerpc/include/asm/bug.h
+++ b/arch/powerpc/include/asm/bug.h
@@ -96,7 +96,7 @@ __label_warn_on: \
break; \
} while (0)
-#ifdef CONFIG_PPC64
+#if defined(CONFIG_PPC64) && !defined(__CHECKER__)
#define BUG_ON(x) do { \
if (__builtin_constant_p(x)) { \
if (x) \
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: WARN_ON() is buggy for 32 bit systems
2022-01-27 11:10 ` Michael Ellerman
@ 2022-01-27 13:37 ` Dan Carpenter
0 siblings, 0 replies; 5+ messages in thread
From: Dan Carpenter @ 2022-01-27 13:37 UTC (permalink / raw)
To: Michael Ellerman; +Cc: linuxppc-dev@lists.ozlabs.org
On Thu, Jan 27, 2022 at 10:10:32PM +1100, Michael Ellerman wrote:
> Dan Carpenter <dan.carpenter@oracle.com> writes:
> > On Wed, Jan 26, 2022 at 12:21:49PM +0000, Christophe Leroy wrote:
> >> The code is enclosed in a #ifdef CONFIG_PPC64, it is not used for PPC32:
> >>
> >> /arch/powerpc/include/asm/bug.h
> >> 99 #ifdef CONFIG_PPC64
> >
> > Ah...
> >
> > You know, life would be a lot easier for me personally if we added an
> > #ifndef __CHECKER__ as well... I can't compile PowerPC code so I can't
> > test a patch like that.
>
> Ubuntu & Fedora both have cross compilers packaged, or there's cross
> compilers on kernel.org. But I assume you mean you'd rather not bother
> compiling for powerpc, which is fair enough.
>
> Do you mean something like below?
Yes, please.
>
> I'm not sure about that, as it would prevent sparse from checking the
> actual BUG_ON code we're using, vs the generic version which we never
> use on 64-bit. Is there a smatch specific macro we could check?
There isn't a Smatch define. This shouldn't affect Sparse at all unless
there was a bug in the WARN_ON() macro.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2022-01-27 13:38 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-01-26 11:56 WARN_ON() is buggy for 32 bit systems Dan Carpenter
2022-01-26 12:21 ` Christophe Leroy
2022-01-26 13:49 ` Dan Carpenter
2022-01-27 11:10 ` Michael Ellerman
2022-01-27 13:37 ` Dan Carpenter
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).