[PATCH v2 0/5] Silence some clang -Wshorten-64-to-32 warnings

[PATCH v2 0/5] Silence some clang -Wshorten-64-to-32 warnings

[Ian Rogers]

Clang's shorten-64-to-32 can be useful to spot certain kinds of bugs
that can be more prevalent in C code due to implicit 64 to 32-bit
casting. Add some explicit casts to header files so as to avoid the
warning when these headers are used.

This patch started out as a single patch in a series for the perf tool
where a bug could have been identified were -Wshorten-64-to-32
enabled:
https://lore.kernel.org/lkml/20250401182347.3422199-3-irogers@google.com/

v2: Rebase and try to address Arnd Bergmann's comments wrt the commit
    message. Arnd also mentioned doing larger refactors, changing
    return types and adding helper functions. I've held off doing this
    given concerns over breaking printf flags.

Ian Rogers (5):
  bitfield: Silence a clang -Wshorten-64-to-32 warning
  bitmap: Silence a clang -Wshorten-64-to-32 warning
  bitops: Silence a clang -Wshorten-64-to-32 warning
  math64: Silence a clang -Wshorten-64-to-32 warning
  hash.h: Silence a clang -Wshorten-64-to-32 warning

 include/asm-generic/bitops/fls64.h | 2 +-
 include/linux/bitfield.h           | 2 +-
 include/linux/bitmap.h             | 2 +-
 include/linux/hash.h               | 2 +-
 include/linux/math64.h             | 2 +-
 5 files changed, 5 insertions(+), 5 deletions(-)

-- 
2.49.0.906.g1f30a19c02-goog

[PATCH v2 1/5] bitfield: Silence a clang -Wshorten-64-to-32 warning

[Ian Rogers]

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.

Signed-off-by: Ian Rogers <irogers@google.com>
---
 include/linux/bitfield.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/bitfield.h b/include/linux/bitfield.h
index 63928f173223..cc5cfed041bb 100644
--- a/include/linux/bitfield.h
+++ b/include/linux/bitfield.h
@@ -176,7 +176,7 @@ static __always_inline __##type type##_encode_bits(base v, base field)	\
 {									\
 	if (__builtin_constant_p(v) && (v & ~field_mask(field)))	\
 		__field_overflow();					\
-	return to((v & field_mask(field)) * field_multiplier(field));	\
+	return to((__##type)((v & field_mask(field)) * field_multiplier(field))); \
 }									\
 static __always_inline __##type type##_replace_bits(__##type old,	\
 					base val, base field)		\
-- 
2.49.0.906.g1f30a19c02-goog

Re: [PATCH v2 1/5] bitfield: Silence a clang -Wshorten-64-to-32 warning

[David Laight]

On Wed, 30 Apr 2025 10:15:30 -0700
Ian Rogers <irogers@google.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.

I suspect that is going to add an explicit mask in many cases.
Probably for u8, u16 and u32 if the return value from the old
code would return a u64 and the value is assigned to a u64.

Now if 'field_mask()' and 'field_multiplier()' are constants
then the compiler might optimise away the extra mask.
But in that case it shouldn't be bleating about the truncation
because it knows it can't happen.

So get the compiler fixed to do proper 'value tracking' and only
report about truncation when the high bits might be non-zero.
It also needs to not warn when the high bits are saved separately
(int h = v64 >> 32, l = v64;).

Then you have to worry about stopping the compiler bleating
for the return values of read (and similar).

Once you've got the compiler fixed to remove most of the false
positives worry about the Linux code.

	David

> 
> Signed-off-by: Ian Rogers <irogers@google.com>
> ---
>  include/linux/bitfield.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/include/linux/bitfield.h b/include/linux/bitfield.h
> index 63928f173223..cc5cfed041bb 100644
> --- a/include/linux/bitfield.h
> +++ b/include/linux/bitfield.h
> @@ -176,7 +176,7 @@ static __always_inline __##type type##_encode_bits(base v, base field)	\
>  {									\
>  	if (__builtin_constant_p(v) && (v & ~field_mask(field)))	\
>  		__field_overflow();					\
> -	return to((v & field_mask(field)) * field_multiplier(field));	\
> +	return to((__##type)((v & field_mask(field)) * field_multiplier(field))); \
>  }									\
>  static __always_inline __##type type##_replace_bits(__##type old,	\
>  					base val, base field)		\

Re: [PATCH v2 1/5] bitfield: Silence a clang -Wshorten-64-to-32 warning

[kernel test robot]

Hi Ian,

kernel test robot noticed the following build warnings:

[auto build test WARNING on arnd-asm-generic/master]
[also build test WARNING on soc/for-next linus/master v6.15-rc5 next-20250506]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Ian-Rogers/bitfield-Silence-a-clang-Wshorten-64-to-32-warning/20250501-011830
base:   https://git.kernel.org/pub/scm/linux/kernel/git/arnd/asm-generic.git master
patch link:    https://lore.kernel.org/r/20250430171534.132774-2-irogers%40google.com
patch subject: [PATCH v2 1/5] bitfield: Silence a clang -Wshorten-64-to-32 warning
config: mips-randconfig-r111-20250501 (https://download.01.org/0day-ci/archive/20250507/202505071638.ZLhUrspy-lkp@intel.com/config)
compiler: clang version 21.0.0git (https://github.com/llvm/llvm-project f819f46284f2a79790038e1f6649172789734ae8)
reproduce: (https://download.01.org/0day-ci/archive/20250507/202505071638.ZLhUrspy-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202505071638.ZLhUrspy-lkp@intel.com/

sparse warnings: (new ones prefixed by >>)
   lib/bitfield_kunit.c: note: in included file (through include/linux/fortify-string.h, include/linux/string.h, include/linux/bitmap.h, ...):
>> include/linux/bitfield.h:200:1: sparse: sparse: cast to restricted __le16
>> include/linux/bitfield.h:200:1: sparse: sparse: cast from restricted __le16
>> include/linux/bitfield.h:200:1: sparse: sparse: cast to restricted __be16
>> include/linux/bitfield.h:200:1: sparse: sparse: cast from restricted __be16
>> include/linux/bitfield.h:200:1: sparse: sparse: cast to restricted __le16
>> include/linux/bitfield.h:200:1: sparse: sparse: cast from restricted __le16
>> include/linux/bitfield.h:200:1: sparse: sparse: cast to restricted __be16
>> include/linux/bitfield.h:200:1: sparse: sparse: cast from restricted __be16
>> include/linux/bitfield.h:200:1: sparse: sparse: cast to restricted __le16
>> include/linux/bitfield.h:200:1: sparse: sparse: cast from restricted __le16
>> include/linux/bitfield.h:200:1: sparse: sparse: cast to restricted __be16
>> include/linux/bitfield.h:200:1: sparse: sparse: cast from restricted __be16
>> include/linux/bitfield.h:200:1: sparse: sparse: cast to restricted __le16
>> include/linux/bitfield.h:200:1: sparse: sparse: cast from restricted __le16
>> include/linux/bitfield.h:200:1: sparse: sparse: cast to restricted __be16
>> include/linux/bitfield.h:200:1: sparse: sparse: cast from restricted __be16
>> include/linux/bitfield.h:200:1: sparse: sparse: cast to restricted __le16
>> include/linux/bitfield.h:200:1: sparse: sparse: cast from restricted __le16
>> include/linux/bitfield.h:200:1: sparse: sparse: cast to restricted __be16
>> include/linux/bitfield.h:200:1: sparse: sparse: cast from restricted __be16
>> include/linux/bitfield.h:200:1: sparse: sparse: cast to restricted __le16
>> include/linux/bitfield.h:200:1: sparse: sparse: cast from restricted __le16
>> include/linux/bitfield.h:200:1: sparse: sparse: cast to restricted __be16
>> include/linux/bitfield.h:200:1: sparse: sparse: cast from restricted __be16
>> include/linux/bitfield.h:201:1: sparse: sparse: cast to restricted __le32
>> include/linux/bitfield.h:201:1: sparse: sparse: cast from restricted __le32
>> include/linux/bitfield.h:201:1: sparse: sparse: cast to restricted __be32
>> include/linux/bitfield.h:201:1: sparse: sparse: cast from restricted __be32
>> include/linux/bitfield.h:201:1: sparse: sparse: cast to restricted __le32
>> include/linux/bitfield.h:201:1: sparse: sparse: cast from restricted __le32
>> include/linux/bitfield.h:201:1: sparse: sparse: cast to restricted __be32
>> include/linux/bitfield.h:201:1: sparse: sparse: cast from restricted __be32
>> include/linux/bitfield.h:201:1: sparse: sparse: cast to restricted __le32
>> include/linux/bitfield.h:201:1: sparse: sparse: cast from restricted __le32
>> include/linux/bitfield.h:201:1: sparse: sparse: cast to restricted __be32
>> include/linux/bitfield.h:201:1: sparse: sparse: cast from restricted __be32
>> include/linux/bitfield.h:201:1: sparse: sparse: cast to restricted __le32
>> include/linux/bitfield.h:201:1: sparse: sparse: cast from restricted __le32
>> include/linux/bitfield.h:201:1: sparse: sparse: cast to restricted __be32
>> include/linux/bitfield.h:201:1: sparse: sparse: cast from restricted __be32
>> include/linux/bitfield.h:201:1: sparse: sparse: cast to restricted __le32
>> include/linux/bitfield.h:201:1: sparse: sparse: cast from restricted __le32
>> include/linux/bitfield.h:201:1: sparse: sparse: cast to restricted __be32
>> include/linux/bitfield.h:201:1: sparse: sparse: cast from restricted __be32
>> include/linux/bitfield.h:201:1: sparse: sparse: cast to restricted __le32
>> include/linux/bitfield.h:201:1: sparse: sparse: cast from restricted __le32
>> include/linux/bitfield.h:201:1: sparse: sparse: cast to restricted __be32
>> include/linux/bitfield.h:201:1: sparse: sparse: cast from restricted __be32
>> include/linux/bitfield.h:202:1: sparse: sparse: cast to restricted __le64
>> include/linux/bitfield.h:202:1: sparse: sparse: cast from restricted __le64
>> include/linux/bitfield.h:202:1: sparse: sparse: cast to restricted __be64
>> include/linux/bitfield.h:202:1: sparse: sparse: cast from restricted __be64
>> include/linux/bitfield.h:202:1: sparse: sparse: cast to restricted __le64
>> include/linux/bitfield.h:202:1: sparse: sparse: cast from restricted __le64
>> include/linux/bitfield.h:202:1: sparse: sparse: cast to restricted __be64
>> include/linux/bitfield.h:202:1: sparse: sparse: cast from restricted __be64
>> include/linux/bitfield.h:202:1: sparse: sparse: cast to restricted __le64
>> include/linux/bitfield.h:202:1: sparse: sparse: cast from restricted __le64
>> include/linux/bitfield.h:202:1: sparse: sparse: cast to restricted __be64
>> include/linux/bitfield.h:202:1: sparse: sparse: cast from restricted __be64
>> include/linux/bitfield.h:202:1: sparse: sparse: cast to restricted __le64
>> include/linux/bitfield.h:202:1: sparse: sparse: cast from restricted __le64
>> include/linux/bitfield.h:202:1: sparse: sparse: cast to restricted __be64
>> include/linux/bitfield.h:202:1: sparse: sparse: cast from restricted __be64
>> include/linux/bitfield.h:202:1: sparse: sparse: cast to restricted __le64
>> include/linux/bitfield.h:202:1: sparse: sparse: cast from restricted __le64
>> include/linux/bitfield.h:202:1: sparse: sparse: cast to restricted __be64
>> include/linux/bitfield.h:202:1: sparse: sparse: cast from restricted __be64
>> include/linux/bitfield.h:202:1: sparse: sparse: cast to restricted __le64
>> include/linux/bitfield.h:202:1: sparse: sparse: cast from restricted __le64
>> include/linux/bitfield.h:202:1: sparse: sparse: cast to restricted __be64
>> include/linux/bitfield.h:202:1: sparse: sparse: cast from restricted __be64
   lib/bitfield_kunit.c: note: in included file (through include/linux/thread_info.h, include/asm-generic/preempt.h, arch/mips/include/generated/asm/preempt.h, ...):
   include/linux/bitops.h:274:19: sparse: sparse: cast truncates bits from constant value (8000000000000000 becomes 0)
   include/linux/bitops.h:279:37: sparse: sparse: cast truncates bits from constant value (8000000000000000 becomes 0)
   include/linux/bitops.h:274:19: sparse: sparse: cast truncates bits from constant value (7f00000000000000 becomes 0)
   include/linux/bitops.h:279:37: sparse: sparse: cast truncates bits from constant value (7f00000000000000 becomes 0)
   include/linux/bitops.h:274:19: sparse: sparse: cast truncates bits from constant value (1800000000000 becomes 0)
   include/linux/bitops.h:279:37: sparse: sparse: cast truncates bits from constant value (1800000000000 becomes 0)
   include/linux/bitops.h:274:19: sparse: sparse: cast truncates bits from constant value (1f8000000 becomes f8000000)
   include/linux/bitops.h:279:37: sparse: sparse: cast truncates bits from constant value (1f8000000 becomes f8000000)

vim +200 include/linux/bitfield.h

e2192de59e457a Johannes Berg   2023-01-18  120  
e2192de59e457a Johannes Berg   2023-01-18  121  /**
e2192de59e457a Johannes Berg   2023-01-18  122   * FIELD_PREP_CONST() - prepare a constant bitfield element
e2192de59e457a Johannes Berg   2023-01-18  123   * @_mask: shifted mask defining the field's length and position
e2192de59e457a Johannes Berg   2023-01-18  124   * @_val:  value to put in the field
e2192de59e457a Johannes Berg   2023-01-18  125   *
e2192de59e457a Johannes Berg   2023-01-18  126   * FIELD_PREP_CONST() masks and shifts up the value.  The result should
e2192de59e457a Johannes Berg   2023-01-18  127   * be combined with other fields of the bitfield using logical OR.
e2192de59e457a Johannes Berg   2023-01-18  128   *
e2192de59e457a Johannes Berg   2023-01-18  129   * Unlike FIELD_PREP() this is a constant expression and can therefore
e2192de59e457a Johannes Berg   2023-01-18  130   * be used in initializers. Error checking is less comfortable for this
e2192de59e457a Johannes Berg   2023-01-18  131   * version, and non-constant masks cannot be used.
e2192de59e457a Johannes Berg   2023-01-18  132   */
e2192de59e457a Johannes Berg   2023-01-18  133  #define FIELD_PREP_CONST(_mask, _val)					\
e2192de59e457a Johannes Berg   2023-01-18  134  	(								\
e2192de59e457a Johannes Berg   2023-01-18  135  		/* mask must be non-zero */				\
e2192de59e457a Johannes Berg   2023-01-18  136  		BUILD_BUG_ON_ZERO((_mask) == 0) +			\
e2192de59e457a Johannes Berg   2023-01-18  137  		/* check if value fits */				\
e2192de59e457a Johannes Berg   2023-01-18  138  		BUILD_BUG_ON_ZERO(~((_mask) >> __bf_shf(_mask)) & (_val)) + \
e2192de59e457a Johannes Berg   2023-01-18  139  		/* check if mask is contiguous */			\
e2192de59e457a Johannes Berg   2023-01-18  140  		__BF_CHECK_POW2((_mask) + (1ULL << __bf_shf(_mask))) +	\
e2192de59e457a Johannes Berg   2023-01-18  141  		/* and create the value */				\
e2192de59e457a Johannes Berg   2023-01-18  142  		(((typeof(_mask))(_val) << __bf_shf(_mask)) & (_mask))	\
e2192de59e457a Johannes Berg   2023-01-18  143  	)
e2192de59e457a Johannes Berg   2023-01-18  144  
3e9b3112ec74f1 Jakub Kicinski  2016-08-31  145  /**
3e9b3112ec74f1 Jakub Kicinski  2016-08-31  146   * FIELD_GET() - extract a bitfield element
3e9b3112ec74f1 Jakub Kicinski  2016-08-31  147   * @_mask: shifted mask defining the field's length and position
7240767450d6d8 Masahiro Yamada 2017-10-03  148   * @_reg:  value of entire bitfield
3e9b3112ec74f1 Jakub Kicinski  2016-08-31  149   *
3e9b3112ec74f1 Jakub Kicinski  2016-08-31  150   * FIELD_GET() extracts the field specified by @_mask from the
3e9b3112ec74f1 Jakub Kicinski  2016-08-31  151   * bitfield passed in as @_reg by masking and shifting it down.
3e9b3112ec74f1 Jakub Kicinski  2016-08-31  152   */
3e9b3112ec74f1 Jakub Kicinski  2016-08-31  153  #define FIELD_GET(_mask, _reg)						\
3e9b3112ec74f1 Jakub Kicinski  2016-08-31  154  	({								\
3e9b3112ec74f1 Jakub Kicinski  2016-08-31  155  		__BF_FIELD_CHECK(_mask, _reg, 0U, "FIELD_GET: ");	\
3e9b3112ec74f1 Jakub Kicinski  2016-08-31  156  		(typeof(_mask))(((_reg) & (_mask)) >> __bf_shf(_mask));	\
3e9b3112ec74f1 Jakub Kicinski  2016-08-31  157  	})
3e9b3112ec74f1 Jakub Kicinski  2016-08-31  158  
e7d4a95da86e0b Johannes Berg   2018-06-20  159  extern void __compiletime_error("value doesn't fit into mask")
00b0c9b82663ac Al Viro         2017-12-14  160  __field_overflow(void);
00b0c9b82663ac Al Viro         2017-12-14  161  extern void __compiletime_error("bad bitfield mask")
00b0c9b82663ac Al Viro         2017-12-14  162  __bad_mask(void);
00b0c9b82663ac Al Viro         2017-12-14  163  static __always_inline u64 field_multiplier(u64 field)
00b0c9b82663ac Al Viro         2017-12-14  164  {
00b0c9b82663ac Al Viro         2017-12-14  165  	if ((field | (field - 1)) & ((field | (field - 1)) + 1))
00b0c9b82663ac Al Viro         2017-12-14  166  		__bad_mask();
00b0c9b82663ac Al Viro         2017-12-14  167  	return field & -field;
00b0c9b82663ac Al Viro         2017-12-14  168  }
00b0c9b82663ac Al Viro         2017-12-14  169  static __always_inline u64 field_mask(u64 field)
00b0c9b82663ac Al Viro         2017-12-14  170  {
00b0c9b82663ac Al Viro         2017-12-14  171  	return field / field_multiplier(field);
00b0c9b82663ac Al Viro         2017-12-14  172  }
e31a50162feb35 Alex Elder      2020-03-12  173  #define field_max(field)	((typeof(field))field_mask(field))
00b0c9b82663ac Al Viro         2017-12-14  174  #define ____MAKE_OP(type,base,to,from)					\
00b0c9b82663ac Al Viro         2017-12-14  175  static __always_inline __##type type##_encode_bits(base v, base field)	\
00b0c9b82663ac Al Viro         2017-12-14  176  {									\
e7d4a95da86e0b Johannes Berg   2018-06-20  177  	if (__builtin_constant_p(v) && (v & ~field_mask(field)))	\
00b0c9b82663ac Al Viro         2017-12-14  178  		__field_overflow();					\
38e224b77cde3b Ian Rogers      2025-04-30  179  	return to((__##type)((v & field_mask(field)) * field_multiplier(field))); \
00b0c9b82663ac Al Viro         2017-12-14  180  }									\
00b0c9b82663ac Al Viro         2017-12-14  181  static __always_inline __##type type##_replace_bits(__##type old,	\
00b0c9b82663ac Al Viro         2017-12-14  182  					base val, base field)		\
00b0c9b82663ac Al Viro         2017-12-14  183  {									\
00b0c9b82663ac Al Viro         2017-12-14  184  	return (old & ~to(field)) | type##_encode_bits(val, field);	\
00b0c9b82663ac Al Viro         2017-12-14  185  }									\
00b0c9b82663ac Al Viro         2017-12-14  186  static __always_inline void type##p_replace_bits(__##type *p,		\
00b0c9b82663ac Al Viro         2017-12-14  187  					base val, base field)		\
00b0c9b82663ac Al Viro         2017-12-14  188  {									\
00b0c9b82663ac Al Viro         2017-12-14  189  	*p = (*p & ~to(field)) | type##_encode_bits(val, field);	\
00b0c9b82663ac Al Viro         2017-12-14  190  }									\
00b0c9b82663ac Al Viro         2017-12-14  191  static __always_inline base type##_get_bits(__##type v, base field)	\
00b0c9b82663ac Al Viro         2017-12-14  192  {									\
00b0c9b82663ac Al Viro         2017-12-14  193  	return (from(v) & field)/field_multiplier(field);		\
00b0c9b82663ac Al Viro         2017-12-14  194  }
00b0c9b82663ac Al Viro         2017-12-14  195  #define __MAKE_OP(size)							\
00b0c9b82663ac Al Viro         2017-12-14  196  	____MAKE_OP(le##size,u##size,cpu_to_le##size,le##size##_to_cpu)	\
00b0c9b82663ac Al Viro         2017-12-14  197  	____MAKE_OP(be##size,u##size,cpu_to_be##size,be##size##_to_cpu)	\
00b0c9b82663ac Al Viro         2017-12-14  198  	____MAKE_OP(u##size,u##size,,)
37a3862e123826 Johannes Berg   2018-06-20  199  ____MAKE_OP(u8,u8,,)
00b0c9b82663ac Al Viro         2017-12-14 @200  __MAKE_OP(16)
00b0c9b82663ac Al Viro         2017-12-14 @201  __MAKE_OP(32)
00b0c9b82663ac Al Viro         2017-12-14 @202  __MAKE_OP(64)
00b0c9b82663ac Al Viro         2017-12-14  203  #undef __MAKE_OP
00b0c9b82663ac Al Viro         2017-12-14  204  #undef ____MAKE_OP
00b0c9b82663ac Al Viro         2017-12-14  205  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

[PATCH v2 2/5] bitmap: Silence a clang -Wshorten-64-to-32 warning

[Ian Rogers]

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.

Signed-off-by: Ian Rogers <irogers@google.com>
---
 include/linux/bitmap.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/bitmap.h b/include/linux/bitmap.h
index 595217b7a6e7..4395e0a618f4 100644
--- a/include/linux/bitmap.h
+++ b/include/linux/bitmap.h
@@ -442,7 +442,7 @@ static __always_inline
 unsigned int bitmap_weight(const unsigned long *src, unsigned int nbits)
 {
 	if (small_const_nbits(nbits))
-		return hweight_long(*src & BITMAP_LAST_WORD_MASK(nbits));
+		return (int)hweight_long(*src & BITMAP_LAST_WORD_MASK(nbits));
 	return __bitmap_weight(src, nbits);
 }
 
-- 
2.49.0.906.g1f30a19c02-goog

Re: [PATCH v2 2/5] bitmap: Silence a clang -Wshorten-64-to-32 warning

[Yury Norov]

Hi Ian,

On Wed, Apr 30, 2025 at 10:15:31AM -0700, Ian Rogers 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.
> 
> Signed-off-by: Ian Rogers <irogers@google.com>
> ---
>  include/linux/bitmap.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/include/linux/bitmap.h b/include/linux/bitmap.h
> index 595217b7a6e7..4395e0a618f4 100644
> --- a/include/linux/bitmap.h
> +++ b/include/linux/bitmap.h
> @@ -442,7 +442,7 @@ static __always_inline
>  unsigned int bitmap_weight(const unsigned long *src, unsigned int nbits)
>  {
>  	if (small_const_nbits(nbits))
> -		return hweight_long(*src & BITMAP_LAST_WORD_MASK(nbits));
> +		return (int)hweight_long(*src & BITMAP_LAST_WORD_MASK(nbits));

This should return unsigned int, I guess?

Also, most of the functions you touch here have their copies in tools.
Can you please keep them synchronized?

Thanks,
Yury

>  	return __bitmap_weight(src, nbits);
>  }
>  
> -- 
> 2.49.0.906.g1f30a19c02-goog

Re: [PATCH v2 2/5] bitmap: Silence a clang -Wshorten-64-to-32 warning

[Ian Rogers]

On Fri, May 2, 2025 at 9:03 AM Yury Norov <yury.norov@gmail.com> wrote:
>
> Hi Ian,
>
> On Wed, Apr 30, 2025 at 10:15:31AM -0700, Ian Rogers 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.
> >
> > Signed-off-by: Ian Rogers <irogers@google.com>
> > ---
> >  include/linux/bitmap.h | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/include/linux/bitmap.h b/include/linux/bitmap.h
> > index 595217b7a6e7..4395e0a618f4 100644
> > --- a/include/linux/bitmap.h
> > +++ b/include/linux/bitmap.h
> > @@ -442,7 +442,7 @@ static __always_inline
> >  unsigned int bitmap_weight(const unsigned long *src, unsigned int nbits)
> >  {
> >       if (small_const_nbits(nbits))
> > -             return hweight_long(*src & BITMAP_LAST_WORD_MASK(nbits));
> > +             return (int)hweight_long(*src & BITMAP_LAST_WORD_MASK(nbits));
>
> This should return unsigned int, I guess?

Hi Yury, I don't disagree. The issue there is that this could break
printf flags, etc. reliant on the return type. I've tried to keep the
patch minimal in this regard.

> Also, most of the functions you touch here have their copies in tools.
> Can you please keep them synchronized?

Yes, I do most of my work on the perf tool in the tools directory and
these patches come from adding -Wshorten-64-to-32 there due to a bug
found in ARM code that -Wshorten-64-to-32 would have caught:
https://lore.kernel.org/lkml/20250331172759.115604-1-leo.yan@arm.com/
The most recent patch series for tools is:
https://lore.kernel.org/linux-perf-users/20250430175036.184610-1-irogers@google.com/
However, I wanted to get the kernel versions of these headers agreed
before syncing them into the tools directory.

Thanks,
Ian



> Thanks,
> Yury
>
> >       return __bitmap_weight(src, nbits);
> >  }
> >
> > --
> > 2.49.0.906.g1f30a19c02-goog

Re: [PATCH v2 2/5] bitmap: Silence a clang -Wshorten-64-to-32 warning

[Yury Norov]

On Fri, May 02, 2025 at 09:43:12AM -0700, Ian Rogers wrote:
> On Fri, May 2, 2025 at 9:03 AM Yury Norov <yury.norov@gmail.com> wrote:
> >
> > Hi Ian,
> >
> > On Wed, Apr 30, 2025 at 10:15:31AM -0700, Ian Rogers 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.
> > >
> > > Signed-off-by: Ian Rogers <irogers@google.com>
> > > ---
> > >  include/linux/bitmap.h | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/include/linux/bitmap.h b/include/linux/bitmap.h
> > > index 595217b7a6e7..4395e0a618f4 100644
> > > --- a/include/linux/bitmap.h
> > > +++ b/include/linux/bitmap.h
> > > @@ -442,7 +442,7 @@ static __always_inline
> > >  unsigned int bitmap_weight(const unsigned long *src, unsigned int nbits)
> > >  {
> > >       if (small_const_nbits(nbits))
> > > -             return hweight_long(*src & BITMAP_LAST_WORD_MASK(nbits));
> > > +             return (int)hweight_long(*src & BITMAP_LAST_WORD_MASK(nbits));
> >
> > This should return unsigned int, I guess?
> 
> Hi Yury, I don't disagree. The issue there is that this could break
> printf flags, etc. reliant on the return type. I've tried to keep the
> patch minimal in this regard.

Not sure I understand... 

I mean just 
        return (unsigned int)hweight_long(...);

because the bitmap_weight return type is unsigned int. Do I miss
something?
 
> > Also, most of the functions you touch here have their copies in tools.
> > Can you please keep them synchronized?
> 
> Yes, I do most of my work on the perf tool in the tools directory and
> these patches come from adding -Wshorten-64-to-32 there due to a bug
> found in ARM code that -Wshorten-64-to-32 would have caught:
> https://lore.kernel.org/lkml/20250331172759.115604-1-leo.yan@arm.com/
> The most recent patch series for tools is:
> https://lore.kernel.org/linux-perf-users/20250430175036.184610-1-irogers@google.com/
> However, I wanted to get the kernel versions of these headers agreed
> before syncing them into the tools directory.

Yes, I'm in CC for that series, but I didn't find the changes for
bitmap_weight(), fls64() and other functions you touch in this series.
Anyways, it would be logical to sync tools with the mother kernel in
the same series.

Re: [PATCH v2 2/5] bitmap: Silence a clang -Wshorten-64-to-32 warning

[Ian Rogers]

On Fri, May 2, 2025 at 9:55 AM Yury Norov <yury.norov@gmail.com> wrote:
>
> On Fri, May 02, 2025 at 09:43:12AM -0700, Ian Rogers wrote:
> > On Fri, May 2, 2025 at 9:03 AM Yury Norov <yury.norov@gmail.com> wrote:
> > >
> > > Hi Ian,
> > >
> > > On Wed, Apr 30, 2025 at 10:15:31AM -0700, Ian Rogers 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.
> > > >
> > > > Signed-off-by: Ian Rogers <irogers@google.com>
> > > > ---
> > > >  include/linux/bitmap.h | 2 +-
> > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > >
> > > > diff --git a/include/linux/bitmap.h b/include/linux/bitmap.h
> > > > index 595217b7a6e7..4395e0a618f4 100644
> > > > --- a/include/linux/bitmap.h
> > > > +++ b/include/linux/bitmap.h
> > > > @@ -442,7 +442,7 @@ static __always_inline
> > > >  unsigned int bitmap_weight(const unsigned long *src, unsigned int nbits)
> > > >  {
> > > >       if (small_const_nbits(nbits))
> > > > -             return hweight_long(*src & BITMAP_LAST_WORD_MASK(nbits));
> > > > +             return (int)hweight_long(*src & BITMAP_LAST_WORD_MASK(nbits));
> > >
> > > This should return unsigned int, I guess?
> >
> > Hi Yury, I don't disagree. The issue there is that this could break
> > printf flags, etc. reliant on the return type. I've tried to keep the
> > patch minimal in this regard.
>
> Not sure I understand...
>
> I mean just
>         return (unsigned int)hweight_long(...);
>
> because the bitmap_weight return type is unsigned int. Do I miss
> something?

Oh sorry, my misunderstanding.

> > > Also, most of the functions you touch here have their copies in tools.
> > > Can you please keep them synchronized?
> >
> > Yes, I do most of my work on the perf tool in the tools directory and
> > these patches come from adding -Wshorten-64-to-32 there due to a bug
> > found in ARM code that -Wshorten-64-to-32 would have caught:
> > https://lore.kernel.org/lkml/20250331172759.115604-1-leo.yan@arm.com/
> > The most recent patch series for tools is:
> > https://lore.kernel.org/linux-perf-users/20250430175036.184610-1-irogers@google.com/
> > However, I wanted to get the kernel versions of these headers agreed
> > before syncing them into the tools directory.
>
> Yes, I'm in CC for that series, but I didn't find the changes for
> bitmap_weight(), fls64() and other functions you touch in this series.
> Anyways, it would be logical to sync tools with the mother kernel in
> the same series.

Ok, I'll add it. Fwiw, I'm not particularly fond of syncing the files
as it's not clear how to do it and keep the attribution/changes clear.
I've patches to do things like make the tools/include more hermetic,
but they've died a death on LKML:
https://lore.kernel.org/lkml/20201015223119.1712121-1-irogers@google.com/
Tbh, I want to completely change how tools/include works as perf and
other things casually use -I into the tools/include and
tools/include/uapi directories meaning strange things like the types.h
in these things is usually the linux/types.h rather than
uapi/linux/types.h. Amongst other things, the licenses on these files
are not the same. I think we should be building the
tools/include/linux and associated tools/lib files as if they were a
library and installing their headers as is done for libbpf, libsubcmd,
etc. The -I would then reflect the install_headers output path. I'm
less clear on the value of doing this for uapi.

Thanks,
Ian

[PATCH v2 3/5] bitops: Silence a clang -Wshorten-64-to-32 warning

[Ian Rogers]

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.

Signed-off-by: Ian Rogers <irogers@google.com>
---
 include/asm-generic/bitops/fls64.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/asm-generic/bitops/fls64.h b/include/asm-generic/bitops/fls64.h
index 866f2b2304ff..9ad3ff12f454 100644
--- a/include/asm-generic/bitops/fls64.h
+++ b/include/asm-generic/bitops/fls64.h
@@ -21,7 +21,7 @@ static __always_inline int fls64(__u64 x)
 	__u32 h = x >> 32;
 	if (h)
 		return fls(h) + 32;
-	return fls(x);
+	return fls((__u32)x);
 }
 #elif BITS_PER_LONG == 64
 static __always_inline int fls64(__u64 x)
-- 
2.49.0.906.g1f30a19c02-goog

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

[Ian Rogers]

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.

Signed-off-by: Ian Rogers <irogers@google.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;
-- 
2.49.0.906.g1f30a19c02-goog

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

[David Laight]

On Wed, 30 Apr 2025 10:15:33 -0700
Ian Rogers <irogers@google.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.

	David


> 
> Signed-off-by: Ian Rogers <irogers@google.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;

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

[Ian Rogers]

On Thu, May 1, 2025 at 1:07 PM David Laight
<david.laight.linux@gmail.com> wrote:
>
> On Wed, 30 Apr 2025 10:15:33 -0700
> Ian Rogers <irogers@google.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@google.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;
>

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

[David Laight]

On Thu, 1 May 2025 13:15:30 -0700
Ian Rogers <irogers@google.com> wrote:

> On Thu, May 1, 2025 at 1:07 PM David Laight
> <david.laight.linux@gmail.com> wrote:
> >
> > On Wed, 30 Apr 2025 10:15:33 -0700
> > Ian Rogers <irogers@google.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

I'm not a java expert, but I suspect it has 'softer' type conversions
for integers than C casts.
I've been badly bitten by C casts that make code compile when it really
shouldn't, or incorrectly mask off high bits.
There are actually loads of them in the Linux kernel.
As well as all the dubious min_t(u16,...) there are the (__force ...)
where the compiler shouldn't see a cast at all (it is for sparse).

> ```
> 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;

And no one is confused by what the code is doing without the cast.

We live with the 'integer to pointer of differ size' warning,
but even that was only really useful 30 years ago and is well
past its 'best before' date.

	David

> ```
> 
> Thanks,
> Ian
> 
> >         David
> >
> >  
> > >
> > > Signed-off-by: Ian Rogers <irogers@google.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;  
> >  

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

[Ian Rogers]

On Thu, May 1, 2025 at 1:27 PM David Laight
<david.laight.linux@gmail.com> wrote:
>
> On Thu, 1 May 2025 13:15:30 -0700
> Ian Rogers <irogers@google.com> wrote:
>
> > On Thu, May 1, 2025 at 1:07 PM David Laight
> > <david.laight.linux@gmail.com> wrote:
> > >
> > > On Wed, 30 Apr 2025 10:15:33 -0700
> > > Ian Rogers <irogers@google.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
>
> I'm not a java expert, but I suspect it has 'softer' type conversions
> for integers than C casts.

Sorry I don't understand what you're saying. Java certainly has bugs
in this area which is why I've written checkers like:
https://errorprone.info/bugpattern/BadComparable
For code similar to:
```
s32 compare(s64 a, s64 b) { return (s32)(a - b); }
```
where the truncation is going to throw away the sign of the subtract
and is almost certainly a bug. This matches the bugs that are fixed in
this patch series for the perf code, in particular an issue on ARM
that Leo Yan originally provided the fix for:
https://lore.kernel.org/lkml/20250331172759.115604-1-leo.yan@arm.com/

> I've been badly bitten by C casts that make code compile when it really
> shouldn't, or incorrectly mask off high bits.
> There are actually loads of them in the Linux kernel.
> As well as all the dubious min_t(u16,...) there are the (__force ...)
> where the compiler shouldn't see a cast at all (it is for sparse).

Are you arguing for or against checks here? It seems to be about
casts. I'm not getting you.

> > ```
> > 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;
>
> And no one is confused by what the code is doing without the cast.

Someone who saw that `a` was 64-bit may assume from the assignment
that `al` were also 64-bit. The cast is making explicit that you want
to throw away bits after the bottom 32, so I'd disagree.

> We live with the 'integer to pointer of differ size' warning,
> but even that was only really useful 30 years ago and is well
> past its 'best before' date.

You want C to be weakly typed more than it is? Not sure and it seems
we've drifted far from the topic of the patch series.

Thanks,
Ian

>         David
>
> > ```
> >
> > Thanks,
> > Ian
> >
> > >         David
> > >
> > >
> > > >
> > > > Signed-off-by: Ian Rogers <irogers@google.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;
> > >
>

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

[David Laight]

On Thu, 1 May 2025 14:11:59 -0700
Ian Rogers <irogers@google.com> wrote:

....
> Sorry I don't understand what you're saying. Java certainly has bugs
> in this area which is why I've written checkers like:
> https://errorprone.info/bugpattern/BadComparable
> For code similar to:
> ```
> s32 compare(s64 a, s64 b) { return (s32)(a - b); }
> ```
> where the truncation is going to throw away the sign of the subtract
> and is almost certainly a bug. This matches the bugs that are fixed in
> this patch series for the perf code, in particular an issue on ARM
> that Leo Yan originally provided the fix for:
> https://lore.kernel.org/lkml/20250331172759.115604-1-leo.yan@arm.com/

That code is wrong with or without the (s32) cast.
And the explicit cast will hide the compiler warning.

If you want the compiler to find bugs you need to reduce the number
of casts to an absolute minimum and disable/fix the compiler warning
for false positives.

These type based (rather than value domain) warnings are all a PITA.

Another example is the 'signed v unsigned compare' which bleats for:
	int rval = read(... sizeof (foo));
	if (rval < 0)
		return -1;
	if (rval != sizeof (foo))
		// truncated
Whereas a statically_true(rval >= 0) test will pass.

	David

[PATCH v2 5/5] hash.h: Silence a clang -Wshorten-64-to-32 warning

[Ian Rogers]

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.

Signed-off-by: Ian Rogers <irogers@google.com>
---
 include/linux/hash.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/hash.h b/include/linux/hash.h
index 38edaa08f862..ecc8296cb397 100644
--- a/include/linux/hash.h
+++ b/include/linux/hash.h
@@ -75,7 +75,7 @@ static __always_inline u32 hash_64_generic(u64 val, unsigned int bits)
 {
 #if BITS_PER_LONG == 64
 	/* 64x64-bit multiply is efficient on all 64-bit processors */
-	return val * GOLDEN_RATIO_64 >> (64 - bits);
+	return (u32)(val * GOLDEN_RATIO_64 >> (64 - bits));
 #else
 	/* Hash 64 bits using only 32x32-bit multiply. */
 	return hash_32((u32)val ^ __hash_32(val >> 32), bits);
-- 
2.49.0.906.g1f30a19c02-goog