WARN_ON() is buggy for 32 bit systems

[Christophe Leroy]

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
>