Message-ID: <CAP-5=fVMk+1+Jv3OavMwHmm1b0YNZPnmETOW7nTbW9-7tS4MNw@mail.gmail.com>
Date: Wed, 2 Apr 2025 15:32:57 -0700
From: Ian Rogers <irogers@...gle.com>
To: David Laight <david.laight.linux@...il.com>
Cc: Leo Yan <leo.yan@....com>, Yury Norov <yury.norov@...il.com>, 
	Rasmus Villemoes <linux@...musvillemoes.dk>, Peter Zijlstra <peterz@...radead.org>, 
	Ingo Molnar <mingo@...hat.com>, Arnaldo Carvalho de Melo <acme@...nel.org>, Namhyung Kim <namhyung@...nel.org>, 
	Mark Rutland <mark.rutland@....com>, 
	Alexander Shishkin <alexander.shishkin@...ux.intel.com>, Jiri Olsa <jolsa@...nel.org>, 
	Adrian Hunter <adrian.hunter@...el.com>, Kan Liang <kan.liang@...ux.intel.com>, 
	Thomas Gleixner <tglx@...utronix.de>, Darren Hart <dvhart@...radead.org>, 
	Davidlohr Bueso <dave@...olabs.net>, André Almeida <andrealmeid@...lia.com>, 
	John Garry <john.g.garry@...cle.com>, Will Deacon <will@...nel.org>, 
	James Clark <james.clark@...aro.org>, Mike Leach <mike.leach@...aro.org>, 
	Leo Yan <leo.yan@...ux.dev>, Yicong Yang <yangyicong@...ilicon.com>, 
	Jonathan Cameron <jonathan.cameron@...wei.com>, Nathan Chancellor <nathan@...nel.org>, 
	Bill Wendling <morbo@...gle.com>, Justin Stitt <justinstitt@...gle.com>, 
	Josh Poimboeuf <jpoimboe@...nel.org>, Al Viro <viro@...iv.linux.org.uk>, 
	Kyle Meyer <kyle.meyer@....com>, Ben Gainey <ben.gainey@....com>, 
	Athira Rajeev <atrajeev@...ux.vnet.ibm.com>, Kajol Jain <kjain@...ux.ibm.com>, 
	Aditya Gupta <adityag@...ux.ibm.com>, Eder Zulian <ezulian@...hat.com>, 
	Dapeng Mi <dapeng1.mi@...ux.intel.com>, Kuan-Wei Chiu <visitorckw@...il.com>, 
	He Zhe <zhe.he@...driver.com>, Dirk Gouders <dirk@...ders.net>, 
	Brian Geffon <bgeffon@...gle.com>, Ravi Bangoria <ravi.bangoria@....com>, 
	Howard Chu <howardchu95@...il.com>, Charlie Jenkins <charlie@...osinc.com>, 
	Colin Ian King <colin.i.king@...il.com>, Dominique Martinet <asmadeus@...ewreck.org>, 
	Jann Horn <jannh@...gle.com>, Masahiro Yamada <masahiroy@...nel.org>, Arnd Bergmann <arnd@...db.de>, 
	Yang Jihong <yangjihong@...edance.com>, Dmitry Vyukov <dvyukov@...gle.com>, 
	Andi Kleen <ak@...ux.intel.com>, Graham Woodward <graham.woodward@....com>, 
	Ilkka Koskinen <ilkka@...amperecomputing.com>, 
	Anshuman Khandual <anshuman.khandual@....com>, Zhongqiu Han <quic_zhonhan@...cinc.com>, 
	Hao Ge <gehao@...inos.cn>, Tengda Wu <wutengda@...weicloud.com>, 
	Gabriele Monaco <gmonaco@...hat.com>, Chun-Tse Shao <ctshao@...gle.com>, 
	Casey Chen <cachen@...estorage.com>, "Dr. David Alan Gilbert" <linux@...blig.org>, 
	Li Huafei <lihuafei1@...wei.com>, "Steinar H. Gunderson" <sesse@...gle.com>, Levi Yun <yeoreum.yun@....com>, 
	Weilin Wang <weilin.wang@...el.com>, Thomas Falcon <thomas.falcon@...el.com>, 
	Thomas Richter <tmricht@...ux.ibm.com>, Andrew Kreimer <algonell@...il.com>, 
	Krzysztof Łopatowski <krzysztof.m.lopatowski@...il.com>, 
	Christophe Leroy <christophe.leroy@...roup.eu>, 
	Jean-Philippe Romain <jean-philippe.romain@...s.st.com>, Junhao He <hejunhao3@...wei.com>, 
	"Masami Hiramatsu (Google)" <mhiramat@...nel.org>, Xu Yang <xu.yang_2@....com>, 
	Steve Clevenger <scclevenger@...amperecomputing.com>, Zixian Cai <fzczx123@...il.com>, 
	Stephen Brennan <stephen.s.brennan@...cle.com>, Yujie Liu <yujie.liu@...el.com>, 
	linux-kernel@...r.kernel.org, linux-perf-users@...r.kernel.org, 
	linux-arm-kernel@...ts.infradead.org, llvm@...ts.linux.dev
Subject: Re: [PATCH v1 09/48] perf tests: Silence -Wshorten-64-to-32 warnings

On Wed, Apr 2, 2025 at 3:01 PM David Laight
<david.laight.linux@...il.com> wrote:
>
> On Wed, 2 Apr 2025 17:38:07 +0100
> Leo Yan <leo.yan@....com> wrote:
>
> > Hi Ian,
> ...
> > If we use casts just to silence warnings, we also lose the opportunity
> > to improve code quality.  The changes in this series that fix type
> > mismatches are good, but I think the use of casts is not particularly
> > helpful - we're simply switching from implicit compiler casts to
> > explicit ones.
>
> It is certainly my experience (a lot of years) that casts between
> integers of different sizes just make code harder to read and possibly
> even more buggy.
>
> If the compiler really knew the valid range of the value (rather than
> just the type) then perhaps a warning that significant bits were being
> discarded might be more reasonable.
> But even then you don't want anything for code like:
>         hi_32 = val_64 >> 32;
>         lo_32 = val_64;

There is an instance of this in:
https://lore.kernel.org/lkml/20250401182347.3422199-3-irogers@google.com/

The particular problem that Leo Yan [1] found in the code base is if a
compare function like:

  int cmp(long *a, long *b)
  {
      return *a - *b;
  }

which are typically passed to routines like list_sort, qsort or
bsearch. The subtract is potentially 64-bit and the implicit cast to
32-bit loses the sign information. Generally the problem is more
opaque than this as here you can quite easily see the types of "a" and
"b". If we say we don't warn for implicit truncating assignments then:

  int cmp(long *a, long *b)
  {
      int ret = *a - *b;
      return ret;
  }

becomes valid, but it is identical and as broken as the code before.
I'm happy for a better alternative than clang's `-Wshorten-64-to-32`
but haven't found it and I agree it's unfortunate for the churn it
kicks up.

As an aside, I wrote a Java check for this as I found a similar bug in
Android. That check found instances where the subtract and truncate
were happening in a cache that would sort elements deleting the
oldest. Unfortunately the truncation meant it could also be deleting
the newest elements. In Java lossy conversions require a cast so I
could target the warning on casts within compare routines. C of course
has given us a good way to shoot ourselves in the foot with implicit
conversion and -Wshorten-64-to-32 seems to be a good way of avoiding
that particular foot cannon. Making ordering routines use a less-than
comparison avoids the problem in C++.

Thanks,
Ian

[1] https://lore.kernel.org/lkml/20250331172759.115604-1-leo.yan@arm.com/
>     David

Powered by blists - more mailing lists