linux-kernel - Re: [PATCH v2 4/5] math64: Silence a clang -Wshorten-64-to-32 warning

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAP-5=fXrhsZYJwjJzqb-zMg+UoC-bKoYCjstq8yD9wHNCfbS5g@mail.gmail.com>
Date: Thu, 1 May 2025 13:15:30 -0700
From: Ian Rogers <irogers@...gle.com>
To: David Laight <david.laight.linux@...il.com>
Cc: Yury Norov <yury.norov@...il.com>, Rasmus Villemoes <linux@...musvillemoes.dk>, 
	Arnd Bergmann <arnd@...db.de>, Nathan Chancellor <nathan@...nel.org>, 
	Nick Desaulniers <nick.desaulniers+lkml@...il.com>, Bill Wendling <morbo@...gle.com>, 
	Justin Stitt <justinstitt@...gle.com>, Adrian Hunter <adrian.hunter@...el.com>, 
	Thomas Gleixner <tglx@...utronix.de>, Jakub Kicinski <kuba@...nel.org>, 
	Jacob Keller <jacob.e.keller@...el.com>, linux-arch@...r.kernel.org, 
	linux-kernel@...r.kernel.org, llvm@...ts.linux.dev, Leo Yan <leo.yan@....com>
Subject: Re: [PATCH v2 4/5] math64: Silence a clang -Wshorten-64-to-32 warning

On Thu, May 1, 2025 at 1:07 PM David Laight
<david.laight.linux@...il.com> wrote:
>
> On Wed, 30 Apr 2025 10:15:33 -0700
> Ian Rogers <irogers@...gle.com> wrote:
>
> > The clang warning -Wshorten-64-to-32 can be useful to catch
> > inadvertent truncation. In some instances this truncation can lead to
> > changing the sign of a result, for example, truncation to return an
> > int to fit a sort routine. Silence the warning by making the implicit
> > truncation explicit. This isn't to say the code is currently incorrect
> > but without silencing the warning it is hard to spot the erroneous
> > cases.
>
> Except that the extra casts make the reader think something 'extra'
> is going on.
> For readability you want as few casts as possible.

Agreed except when not having the cast can introduce bugs which is why
the cast is always required in other languages. Consider in Java:
```
class a {
  public static void main(String args[]) {
     long x = args.length;
     int y = x;
 }
}
$ javac a.java
a.java:4: error: incompatible types: possible lossy conversion from long to int
     int y = x;
             ^
1 error
```
Having -Wshorten-64-to-32 enabled for building with clang would allow
possible mistakes to be spotted, but that's not currently possible
without wading through warnings that this series cleans up.

I also don't really think anyone will be confused about the purpose of
the cast in something like:
```
al = (u32)a;
```

Thanks,
Ian

>         David
>
>
> >
> > Signed-off-by: Ian Rogers <irogers@...gle.com>
> > ---
> >  include/linux/math64.h | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/include/linux/math64.h b/include/linux/math64.h
> > index 6aaccc1626ab..f32fcb2a2331 100644
> > --- a/include/linux/math64.h
> > +++ b/include/linux/math64.h
> > @@ -179,7 +179,7 @@ static __always_inline u64 mul_u64_u64_shr(u64 a, u64 mul, unsigned int shift)
> >  #ifndef mul_u64_u32_shr
> >  static __always_inline u64 mul_u64_u32_shr(u64 a, u32 mul, unsigned int shift)
> >  {
> > -     u32 ah = a >> 32, al = a;
> > +     u32 ah = a >> 32, al = (u32)a;
> >       u64 ret;
> >
> >       ret = mul_u32_u32(al, mul) >> shift;
>