Silence some clang -Wshorten-64-to-32 warnings
Subsystems (3)
cover Silence some clang -Wshorten-64-to-32 warnings
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/[email protected]/
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(-)
Signature · 1 line
2.49.0.504.g3bcea36a83-goog
PATCH 1/5 bitfield: Silence a clang -Wshorten-64-to-32 warning
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.
--- include/linux/bitfield.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
include/linux/bitfield.h +1 -1
@@ -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) \
Signature · 1 line
2.49.0.504.g3bcea36a83-goog
PATCH 2/5 bitmap: Silence a clang -Wshorten-64-to-32 warning
In reply to Ian RogersThe 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.
--- include/linux/bitmap.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
include/linux/bitmap.h +1 -1
@@ -442,7 +442,7 @@ static __always_inlineunsigned 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); }
Signature · 1 line
2.49.0.504.g3bcea36a83-goog
Arnd Bergmann
maintainerRe: [PATCH v1 2/5] bitmap: Silence a clang -Wshorten-64-to-32 warning
Quoted 9 lines · 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.
unsigned int
bitmap_weight(const unsigned long *src, unsigned int nbits){
include/linux/bitmap.h static __always_inline · line 447
Quoted 5 lines
include/linux/bitmap.h +1
-1if (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); }
I don't understand this one. hweight_long() and bitmap_weight() both return unsigned value, so why do you need to cast this to a signed value to avoid a signedness problem?
hweight_long() should never return anything larger than 64ul anyway, which is way outside of the range where it would get sign-extended.
A more logical change to me would be to make hweight_long() and bitmap_weight() have the same return type, either 'unsigned long' or 'unsigned int'.
Arnd
Re: [PATCH v1 2/5] bitmap: Silence a clang -Wshorten-64-to-32 warning
Quoted 21 lines · Arnd Bergmann
Quoted 10 lines · 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.
unsigned int
bitmap_weight(const unsigned long *src, unsigned int nbits){
if (small_const_nbits(nbits))include/linux/bitmap.h static __always_inline · line 447
Quoted 4 lines
include/linux/bitmap.h +1
-1-return hweight_long (*src &BITMAP_LAST_WORD_MASK (nbits)); +return (int )hweight_long (*src &BITMAP_LAST_WORD_MASK (nbits));return __bitmap_weight (src, nbits); }I don't understand this one.
hweight_long()andbitmap_weight()both return unsigned value, so why do you need to cast this to a signed value to avoid a signedness problem?
Quoted 7 lines
hweight_long()should never return anything larger than 64ul anyway, which is way outside of the range where it would get sign-extended.A more logical change to me would be to make
hweight_long()andbitmap_weight()have the same return type, either 'unsigned long' or 'unsigned int'.
I don't disagree but the scope of that change would be much larger. Yury has expressed concern over needing to update printf modifiers. I was aiming for the minimal change that silences clang's -Wshorten-64-to-32 warning.
Thanks,
Ian
Quoted 1 line
Arnd
PATCH 3/5 bitops: Silence a clang -Wshorten-64-to-32 warning
In reply to Ian RogersThe 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.
--- include/asm-generic/bitops/fls64.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
include/asm-generic/bitops/fls64.h +1 -1
@@ -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)
Signature · 1 line
2.49.0.504.g3bcea36a83-goog
Arnd Bergmann
maintainerRe: [PATCH v1 3/5] bitops: Silence a clang -Wshorten-64-to-32 warning
Quoted 5 lines · 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.
The fls64() function only seems to deal with unsigned values, so I don't see how it would change the sign.
include/asm-generic/bitops/fls64.h static __always_inline int fls64(__u64 x) · line 25
Quoted 12 lines
include/asm-generic/bitops/fls64.h +1
-1@@ -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); }
Maybe this would be clearer with an explicit upper_32_bits()/ lower_32_bits() instead of the cast and the shift?
Arnd
Re: [PATCH v1 3/5] bitops: Silence a clang -Wshorten-64-to-32 warning
Quoted 10 lines · Arnd Bergmann
Quoted 5 lines · 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.
The
fls64()function only seems to deal with unsigned values, so I don't see how it would change the sign.
You are right. I was trying to motivate in the message why building with -Wshorten-64-to-32 is a good thing, and in this case we're making an implicit cast explicit.
Quoted 7 lines
Quoted 7 lines
include/asm-generic/bitops/fls64.h +0
-0@@ -21,7 +21,7 @@ static __always_inline int fls64(__u64 x)__u32 h = x >>32 ;
Quoted 8 lines
include/asm-generic/bitops/fls64.h static __always_inline int fls64(__u64 x) · line 25
Quoted 5 lines
include/asm-generic/bitops/fls64.h +1
-1if (h)return fls (h) +32 ; -return fls (x); +return fls ((__u32 )x); }Maybe this would be clearer with an explicit upper_32_bits()/
lower_32_bits()instead of the cast and the shift?
It feels a little overkill to me, but if others prefer it then it is a minor change.
Thanks,
Ian
Quoted 1 line
Arnd
PATCH 4/5 math64: Silence a clang -Wshorten-64-to-32 warning
In reply to Ian RogersThe 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.
--- include/linux/math64.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
include/linux/math64.h +1 -1
@@ -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;
Signature · 1 line
2.49.0.504.g3bcea36a83-goog
PATCH 5/5 hash.h: Silence a clang -Wshorten-64-to-32 warning
In reply to Ian RogersThe 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.
--- include/linux/hash.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
include/linux/hash.h +1 -1
@@ -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);
Signature · 1 line
2.49.0.504.g3bcea36a83-goog