address remaining -Wtautological-constant-out-of-range-compare
Subsystems (15)
cover address remaining -Wtautological-constant-out-of-range-compare
From: Arnd Bergmann <[email protected]>
The warning option was introduced a few years ago but left disabled by default. All of the actual bugs that this has found have been fixed in the meantime, and this series should address the remaining false-positives, as tested on arm/arm64/x86 randconfigs as well as allmodconfig builds for all architectures supported by clang.
Please apply the patches individually to subsystem maintainer trees.
Arnd
Arnd Bergmann (9):
dm integrity: fix out-of-range warning
libceph: avoid clang out-of-range warning
rbd: avoid out-of-range warning
kcov: avoid clang out-of-range warning
ipv4: tcp_output: avoid warning about NET_ADD_STATS
nilfs2: fix out-of-range warning
infiniband: uverbs: avoid out-of-range warnings
mlx5: stop warning for 64KB pages
kbuild: enable tautological-constant-out-of-range-compare
drivers/block/rbd.c | 2 +- drivers/infiniband/core/uverbs_ioctl.c | 4 ++-- drivers/md/dm-integrity.c | 2 +- drivers/net/ethernet/mellanox/mlx5/core/en/xsk/setup.c | 6 ++++-- fs/ceph/snap.c | 2 +- fs/nilfs2/ioctl.c | 2 +- kernel/kcov.c | 3 ++- net/ceph/osdmap.c | 4 ++-- net/ipv4/tcp_output.c | 2 +- scripts/Makefile.extrawarn | 1 - 10 files changed, 15 insertions(+), 13 deletions(-)
Signature · 41 lines
2.39.2 Cc: Ilya Dryomov <[email protected]> Cc: Dongsheng Yang <[email protected]> Cc: Jens Axboe <[email protected]> Cc: Jason Gunthorpe <[email protected]> Cc: Leon Romanovsky <[email protected]> Cc: Alasdair Kergon <[email protected]> Cc: Mike Snitzer <[email protected]> Cc: Mikulas Patocka <[email protected]> Cc: [email protected] Cc: Saeed Mahameed <[email protected]> Cc: "David S. Miller" <[email protected]> Cc: Eric Dumazet <[email protected]> Cc: Jakub Kicinski <[email protected]> Cc: Paolo Abeni <[email protected]> Cc: Xiubo Li <[email protected]> Cc: Jeff Layton <[email protected]> Cc: Ryusuke Konishi <[email protected]> Cc: Dmitry Vyukov <[email protected]> Cc: Andrey Konovalov <[email protected]> Cc: David Ahern <[email protected]> Cc: Masahiro Yamada <[email protected]> Cc: Nathan Chancellor <[email protected]> Cc: Nicolas Schier <[email protected]> Cc: Nick Desaulniers <[email protected]> Cc: Bill Wendling <[email protected]> Cc: Justin Stitt <[email protected]> Cc: Kees Cook <[email protected]> Cc: "Gustavo A. R. Silva" <[email protected]> Cc: Arnd Bergmann <[email protected]> Cc: Tariq Toukan <[email protected]> Cc: [email protected] Cc: [email protected] Cc: [email protected] Cc: [email protected] Cc: [email protected] Cc: [email protected] Cc: [email protected] Cc: [email protected] Cc: [email protected]
PATCH 1/9 dm integrity: fix out-of-range warning
From: Arnd Bergmann <[email protected]>
Depending on the value of CONFIG_HZ, clang compares about a pointless comparison:
drivers/md/dm-integrity.c:4085:12: error: result of comparison of constant 42949672950 with expression of type 'unsigned int' is always false [-Werror,-Wtautological-constant-out-of-range-compare]
if (val >= (uint64_t)UINT_MAX * 1000 / HZ) {As the check remains useful for other configurations, shut up the warning by adding a second type cast to uint64_t.
--- drivers/md/dm-integrity.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/md/dm-integrity.c +1 -1
@@ -4221,7 +4221,7 @@ static int dm_integrity_ctr(struct dm_target *ti, unsigned int argc, char **argv }else if (sscanf (opt_string,"sectors_per_bit:%llu%c" , &llval, &dummy) ==1 ) { log2_sectors_per_bitmap_bit = !llval ?0 :__ilog2_u64 (llval); }else if (sscanf (opt_string,"bitmap_flush_interval:%u%c" , &val, &dummy) ==1 ) { -if (val >= (uint64_t )UINT_MAX *1000 /HZ ) { +if ((uint64_t )val >= (uint64_t )UINT_MAX *1000 /HZ ) { r = -EINVAL ; ti->error ="Invalid bitmap_flush_interval argument" ; goto bad;
Signature · 1 line
2.39.2
Re: [PATCH 1/9] dm integrity: fix out-of-range warning
Quoted 5 lines · Arnd Bergmann
From: Arnd Bergmann <[email protected]>
Depending on the value of
CONFIG_HZ, clang compares about a pointless comparison:
Quoted 8 lines
drivers/md/dm-integrity.c:4085:12: error: result of comparison of constant 42949672950 with expression of type 'unsigned int' is always false [-Werror,-Wtautological-constant-out-of-range-compare]if (val >= (uint64_t)UINT_MAX * 1000 / HZ) {As the check remains useful for other configurations, shut up the warning by adding a second type cast to
uint64_t.Fixes 468dfca38b1a ("dm integrity: add a bitmap mode")Signed-off-by Arnd Bergmann <[email protected]>
Quoted 20 lines
--- drivers/md/dm-integrity.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)drivers/md/dm-integrity.c +1
-1@@ -4221,7 +4221,7 @@ static int dm_integrity_ctr(struct dm_target *ti, unsigned int argc, char **argv }else if (sscanf (opt_string,"sectors_per_bit:%llu%c" , &llval, &dummy) ==1 ) { log2_sectors_per_bitmap_bit = !llval ?0 :__ilog2_u64 (llval); }else if (sscanf (opt_string,"bitmap_flush_interval:%u%c" , &val, &dummy) ==1 ) { -if (val >= (uint64_t )UINT_MAX *1000 /HZ ) { +if ((uint64_t )val >= (uint64_t )UINT_MAX *1000 /HZ ) { r = -EINVAL ; ti->error ="Invalid bitmap_flush_interval argument" ; goto bad;Signature · 1 line
2.39.2
Re: [PATCH 1/9] dm integrity: fix out-of-range warning
In reply to Arnd BergmannQuoted 11 lines · Arnd Bergmann
From: Arnd Bergmann <[email protected]>
Depending on the value of
CONFIG_HZ, clang compares about a pointless comparison:
drivers/md/dm-integrity.c:4085:12: error: result of comparison of constant 42949672950 with expression of type 'unsigned int' is always false [-Werror,-Wtautological-constant-out-of-range-compare]if (val >= (uint64_t)UINT_MAX * 1000 / HZ) {As the check remains useful for other configurations, shut up the warning by adding a second type cast to
uint64_t.
Yeah, nice.
Quoted 3 lines
Fixes 468dfca38b1a ("dm integrity: add a bitmap mode")Signed-off-by Arnd Bergmann <[email protected]>
Quoted 20 lines
--- drivers/md/dm-integrity.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)drivers/md/dm-integrity.c +1
-2@@ -4221,7 +4221,7 @@ static int dm_integrity_ctr(struct dm_target *ti, unsigned int argc, char **argv }else if (sscanf (opt_string,"sectors_per_bit:%llu%c" , &llval, &dummy) ==1 ) { log2_sectors_per_bitmap_bit = !llval ?0 :__ilog2_u64 (llval); }else if (sscanf (opt_string,"bitmap_flush_interval:%u%c" , &val, &dummy) ==1 ) { -if (val >= (uint64_t )UINT_MAX *1000 /HZ ) { +if ((uint64_t )val >= (uint64_t )UINT_MAX *1000 /HZ ) { r = -EINVAL ; ti->error ="Invalid bitmap_flush_interval argument" ; goto bad; --2.39.2
PATCH 2/9 libceph: avoid clang out-of-range warning
In reply to Arnd BergmannFrom: Arnd Bergmann <[email protected]>
clang-14 points out that on 64-bit architectures, a u32 is never larger than constant based on SIZE_MAX:
net/ceph/osdmap.c:1425:10: error: result of comparison of constant 4611686018427387891 with expression of type 'u32' (aka 'unsigned int') is always false [-Werror,-Wtautological-constant-out-of-range-compare]
if (len > (SIZE_MAX - sizeof(*pg)) / sizeof(u32))
~~~ ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
net/ceph/osdmap.c:1608:10: error: result of comparison of constant 2305843009213693945 with expression of type 'u32' (aka 'unsigned int') is always false [-Werror,-Wtautological-constant-out-of-range-compare]
if (len > (SIZE_MAX - sizeof(*pg)) / (2 * sizeof(u32)))
~~~ ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~The code is correct anyway, so just shut up that warning.
--- fs/ceph/snap.c | 2 +- net/ceph/osdmap.c | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-)
fs/ceph/snap.c +1 -1
@@ -374,7 +374,7 @@ static int build_snap_context(struct ceph_mds_client *mdsc,/* alloc new snap context */ err = -ENOMEM ; -if (num > (SIZE_MAX -sizeof (*snapc)) /sizeof (u64)) +if ((size_t )num > (SIZE_MAX -sizeof (*snapc)) /sizeof (u64)) goto fail; snapc =ceph_create_snap_context (num,GFP_NOFS );if (!snapc)
net/ceph/osdmap.c +2 -2
@@ -1438,7 +1438,7 @@ static struct ceph_pg_mapping *__decode_pg_temp(void **p, void *end,ceph_decode_32_safe (p, end, len, e_inval);if (len ==0 && incremental)return NULL ;/* new_pg_temp: [] to remove */ -if (len > (SIZE_MAX -sizeof (*pg)) /sizeof (u32)) +if ((size_t )len > (SIZE_MAX -sizeof (*pg)) /sizeof (u32))return ERR_PTR (-EINVAL );ceph_decode_need (p, end, len *sizeof (u32), e_inval);
@@ -1621,7 +1621,7 @@ static struct ceph_pg_mapping *__decode_pg_upmap_items(void **p, void *end,u32 len, i;ceph_decode_32_safe (p, end, len, e_inval); -if (len > (SIZE_MAX -sizeof (*pg)) / (2 *sizeof (u32))) +if ((size_t )len > (SIZE_MAX -sizeof (*pg)) / (2 *sizeof (u32)))return ERR_PTR (-EINVAL );ceph_decode_need (p, end,2 * len *sizeof (u32), e_inval);
Signature · 1 line
2.39.2
Re: [PATCH 2/9] libceph: avoid clang out-of-range warning
Quoted 6 lines · Arnd Bergmann
From: Arnd Bergmann <[email protected]>
clang-14 points out that on 64-bit architectures, a u32 is never larger than constant based on
SIZE_MAX:
Quoted 8 lines
net/ceph/osdmap.c:1425:10: error: result of comparison of constant 4611686018427387891 with expression of type 'u32' (aka 'unsigned int') is always false [-Werror,-Wtautological-constant-out-of-range-compare]if (len > (SIZE_MAX - sizeof(*pg)) / sizeof(u32)) ~~~ ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ net/ceph/osdmap.c:1608:10: error: result of comparison of constant 2305843009213693945 with expression of type 'u32' (aka 'unsigned int') is always false [-Werror,-Wtautological-constant-out-of-range-compare] if (len > (SIZE_MAX - sizeof(*pg)) / (2 * sizeof(u32))) ~~~ ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~The code is correct anyway, so just shut up that warning.
OK.
Quoted 3 lines
Fixes 6f428df47dae ("libceph: pg_upmap[_items] infrastructure")Signed-off-by Arnd Bergmann <[email protected]>
Quoted 43 lines
--- fs/ceph/snap.c | 2 +- net/ceph/osdmap.c | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-)fs/ceph/snap.c +1
-1@@ -374,7 +374,7 @@ static int build_snap_context(struct ceph_mds_client *mdsc,/* alloc new snap context */ err = -ENOMEM ; -if (num > (SIZE_MAX -sizeof (*snapc)) /sizeof (u64)) +if ((size_t )num > (SIZE_MAX -sizeof (*snapc)) /sizeof (u64)) goto fail; snapc =ceph_create_snap_context (num,GFP_NOFS );if (!snapc)net/ceph/osdmap.c +2
-3@@ -1438,7 +1438,7 @@ static struct ceph_pg_mapping *__decode_pg_temp(void **p, void *end,ceph_decode_32_safe (p, end, len, e_inval);if (len ==0 && incremental)return NULL ;/* new_pg_temp: [] to remove */ -if (len > (SIZE_MAX -sizeof (*pg)) /sizeof (u32)) +if ((size_t )len > (SIZE_MAX -sizeof (*pg)) /sizeof (u32))return ERR_PTR (-EINVAL );ceph_decode_need (p, end, len *sizeof (u32), e_inval);@@ -1621,7 +1621,7 @@ static struct ceph_pg_mapping *__decode_pg_upmap_items(void **p, void *end,u32 len, i;ceph_decode_32_safe (p, end, len, e_inval); -if (len > (SIZE_MAX -sizeof (*pg)) / (2 *sizeof (u32))) +if ((size_t )len > (SIZE_MAX -sizeof (*pg)) / (2 *sizeof (u32)))return ERR_PTR (-EINVAL );ceph_decode_need (p, end,2 * len *sizeof (u32), e_inval); --2.39.2
Re: [PATCH 2/9] libceph: avoid clang out-of-range warning
In reply to Arnd BergmannQuoted 48 lines · Arnd Bergmann
From: Arnd Bergmann <[email protected]>
clang-14 points out that on 64-bit architectures, a u32 is never larger than constant based on
SIZE_MAX:
net/ceph/osdmap.c:1425:10: error: result of comparison of constant 4611686018427387891 with expression of type 'u32' (aka 'unsigned int') is always false [-Werror,-Wtautological-constant-out-of-range-compare]if (len > (SIZE_MAX - sizeof(*pg)) / sizeof(u32)) ~~~ ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ net/ceph/osdmap.c:1608:10: error: result of comparison of constant 2305843009213693945 with expression of type 'u32' (aka 'unsigned int') is always false [-Werror,-Wtautological-constant-out-of-range-compare] if (len > (SIZE_MAX - sizeof(*pg)) / (2 * sizeof(u32))) ~~~ ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~The code is correct anyway, so just shut up that warning.
Fixes 6f428df47dae ("libceph: pg_upmap[_items] infrastructure")Signed-off-by Arnd Bergmann <[email protected]>--- fs/ceph/snap.c | 2 +- net/ceph/osdmap.c | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-)fs/ceph/snap.c +1
-1@@ -374,7 +374,7 @@ static int build_snap_context(struct ceph_mds_client *mdsc,/* alloc new snap context */ err = -ENOMEM ; -if (num > (SIZE_MAX -sizeof (*snapc)) /sizeof (u64)) +if ((size_t )num > (SIZE_MAX -sizeof (*snapc)) /sizeof (u64)) goto fail; snapc =ceph_create_snap_context (num,GFP_NOFS );if (!snapc)net/ceph/osdmap.c +1
-1@@ -1438,7 +1438,7 @@ static struct ceph_pg_mapping *__decode_pg_temp(void **p, void *end,ceph_decode_32_safe (p, end, len, e_inval);if (len ==0 && incremental)return NULL ;/* new_pg_temp: [] to remove */ -if (len > (SIZE_MAX -sizeof (*pg)) /sizeof (u32)) +if ((size_t )len > (SIZE_MAX -sizeof (*pg)) /sizeof (u32))return ERR_PTR (-EINVAL );ceph_decode_need (p, end, len *sizeof (u32), e_inval);
net/ceph/osdmap.c static struct ceph_pg_mapping *__decode_pg_upmap_items(void **p, void *end, · line 1627
Quoted 8 lines
net/ceph/osdmap.c +1
-1@@ -1621,7 +1621,7 @@ static struct ceph_pg_mapping *__decode_pg_upmap_items(void **p, void *end,u32 len, i;ceph_decode_32_safe (p, end, len, e_inval); -if (len > (SIZE_MAX -sizeof (*pg)) / (2 *sizeof (u32))) +if ((size_t )len > (SIZE_MAX -sizeof (*pg)) / (2 *sizeof (u32)))return ERR_PTR (-EINVAL );ceph_decode_need (p, end,2 * len *sizeof (u32), e_inval);
Thanks
- Xiubo
PATCH 3/9 rbd: avoid out-of-range warning
In reply to Arnd BergmannFrom: Arnd Bergmann <[email protected]>
clang-14 points out that the range check is always true on 64-bit architectures since a u32 is not greater than the allowed size:
drivers/block/rbd.c:6079:17: error: result of comparison of constant 2305843009213693948 with expression of type 'u32' (aka 'unsigned int') is always false [-Werror,-Wtautological-constant-out-of-range-compare]
if (snap_count > (SIZE_MAX - sizeof (struct ceph_snap_context))
~~~~~~~~~~ ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
This is harmless, so just change the type of the temporary to size_t to shut up that warning.
--- drivers/block/rbd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/block/rbd.c +1 -1
@@ -6062,7 +6062,7 @@ static int rbd_dev_v2_snap_context(struct rbd_device *rbd_dev,void *p;void *end;u64 seq; -u32 snap_count; +size_t snap_count;struct ceph_snap_context *snapc;u32 i;
Signature · 1 line
2.39.2
Re: [PATCH 3/9] rbd: avoid out-of-range warning
Quoted 6 lines · Arnd Bergmann
From: Arnd Bergmann <[email protected]>
clang-14 points out that the range check is always true on 64-bit architectures since a u32 is not greater than the allowed size:
drivers/block/rbd.c:6079:17: error: result of comparison of constant 2305843009213693948 with expression of type 'u32' (aka 'unsigned int') is always false [-Werror,-Wtautological-constant-out-of-range-compare]
w
Quoted 4 lines
~~~~~~~~~~ ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~This is harmless, so just change the type of the temporary to
size_tto shut up that warning.
This fixes the warning, but then the now size_t value is passed to ceph_decode_32_safe(), which implies a different type conversion. That too is not harmful, but...
Could we just cast the value in the comparison instead?
if ((size_t)snap_count > (SIZE_MAX - sizeof (struct ceph_snap_context))
You could drop the space between sizeof and ( while you're at it (I always used the space back then).
-Alex
Quoted 20 lines
Fixes bb23e37acb2a ("rbd: refactor rbd_header_from_disk()")Signed-off-by Arnd Bergmann <[email protected]>--- drivers/block/rbd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)drivers/block/rbd.c +1
-1@@ -6062,7 +6062,7 @@ static int rbd_dev_v2_snap_context(struct rbd_device *rbd_dev,void *p;void *end;u64 seq; -u32 snap_count; +size_t snap_count;struct ceph_snap_context *snapc;u32 i;
Re: [PATCH 3/9] rbd: avoid out-of-range warning
Quoted 17 lines · Alex Elder
Quoted 6 lines · Arnd Bergmann
From: Arnd Bergmann <[email protected]>
clang-14 points out that the range check is always true on 64-bit architectures since a u32 is not greater than the allowed size:
drivers/block/rbd.c:6079:17: error: result of comparison of constant 2305843009213693948 with expression of type 'u32' (aka 'unsigned int') is always false [-Werror,-Wtautological-constant-out-of-range-compare]w
Quoted 4 lines
~~~~~~~~~~ ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
This is harmless, so just change the type of the temporary to
size_tto shut up that warning.This fixes the warning, but then the now
size_tvalue is passed toceph_decode_32_safe(), which implies a different type conversion. That too is not harmful, but...
Quoted 7 lines
Could we just cast the value in the comparison instead?
if ((size_t)snap_count > (
SIZE_MAX- sizeof (structceph_snap_context))You could drop the space between sizeof and ( while you're at it (I always used the space back then).
Agree.
- Xiubo
Quoted 24 lines
-Alex
Quoted 20 lines
Fixes bb23e37acb2a ("rbd: refactor rbd_header_from_disk()")Signed-off-by Arnd Bergmann <[email protected]>---
drivers/block/rbd.c| 2 +-1 file changed, 1 insertion(+), 1 deletion(-)drivers/block/rbd.c +1
-1@@ -6062,7 +6062,7 @@ static int rbd_dev_v2_snap_context(structrbd_device *rbd_dev, void *p; void *end; u64 seq; - u32 snap_count; + size_t snap_count; struct ceph_snap_context *snapc; u32 i;
PATCH 4/9 kcov: avoid clang out-of-range warning
In reply to Arnd BergmannFrom: Arnd Bergmann <[email protected]>
The area_size is never larger than the maximum on 64-bit architectutes:
kernel/kcov.c:634:29: error: result of comparison of constant 1152921504606846975 with expression of type '__u32' (aka 'unsigned int') is always false [-Werror,-Wtautological-constant-out-of-range-compare]
if (remote_arg->area_size > LONG_MAX / sizeof(unsigned long))
~~~~~~~~~~~~~~~~~~~~~ ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~The compiler can correctly optimize the check away and the code appears correct to me, so just add a cast to avoid the warning.
--- kernel/kcov.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
kernel/kcov.c +2 -1
@@ -627,7 +627,8 @@ static int kcov_ioctl_locked(struct kcov *kcov, unsigned int cmd, mode =kcov_get_mode (remote_arg->trace_mode);if (mode <0 )return mode; -if (remote_arg->area_size >LONG_MAX /sizeof (unsigned long )) +if ((unsigned long )remote_arg->area_size > +LONG_MAX /sizeof (unsigned long ))return -EINVAL ; kcov->mode = mode; t->kcov = kcov;
Signature · 1 line
2.39.2
Re: [PATCH 4/9] kcov: avoid clang out-of-range warning
Quoted 5 lines · Arnd Bergmann
From: Arnd Bergmann <[email protected]>
The
area_sizeis never larger than the maximum on 64-bit architectutes:
Quoted 8 lines
kernel/kcov.c:634:29: error: result of comparison of constant 1152921504606846975 with expression of type '__u32' (aka 'unsigned int') is always false [-Werror,-Wtautological-constant-out-of-range-compare]if (remote_arg->area_size > LONG_MAX / sizeof(unsigned long)) ~~~~~~~~~~~~~~~~~~~~~ ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~The compiler can correctly optimize the check away and the code appears correct to me, so just add a cast to avoid the warning.
Signed-off-by Arnd Bergmann <[email protected]>
Quoted 21 lines
--- kernel/kcov.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)kernel/kcov.c +2
-2@@ -627,7 +627,8 @@ static int kcov_ioctl_locked(struct kcov *kcov, unsigned int cmd, mode =kcov_get_mode (remote_arg->trace_mode);if (mode <0 )return mode; -if (remote_arg->area_size >LONG_MAX /sizeof (unsigned long )) +if ((unsigned long )remote_arg->area_size > +LONG_MAX /sizeof (unsigned long ))return -EINVAL ; kcov->mode = mode; t->kcov = kcov; --2.39.2
PATCH 5/9 ipv4: tcp_output: avoid warning about NET_ADD_STATS
In reply to Arnd BergmannFrom: Arnd Bergmann <[email protected]>
Clang warns about a range check in percpu_add_op() being impossible to hit for an u8 variable:
net/ipv4/tcp_output.c:188:3: error: result of comparison of constant -1 with expression of type 'u8' (aka 'unsigned char') is always false [-Werror,-Wtautological-constant-out-of-range-compare]
NET_ADD_STATS(sock_net(sk), LINUX_MIB_TCPACKCOMPRESSED,
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/net/ip.h:291:41: note: expanded from macro 'NET_ADD_STATS'
#define NET_ADD_STATS(net, field, adnd) SNMP_ADD_STATS((net)->mib.net_statistics, field, adnd)
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/net/snmp.h:143:4: note: expanded from macro 'SNMP_ADD_STATS'
this_cpu_add(mib->mibs[field], addend)
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/percpu-defs.h:509:33: note: expanded from macro 'this_cpu_add'
#define this_cpu_add(pcp, val) __pcpu_size_call(this_cpu_add_, pcp, val)
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~note: (skipping 1 expansions in backtrace; use -fmacro-backtrace-limit=0 to see all) <scratch space>:187:1: note: expanded from herethis_cpu_add_8
^arch/x86/include/asm/percpu.h:326:35: note: expanded from macro 'this_cpu_add_8'
#define this_cpu_add_8(pcp, val) percpu_add_op(8, volatile, (pcp), val)
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
arch/x86/include/asm/percpu.h:127:31: note: expanded from macro 'percpu_add_op'
((val) == 1 || (val) == -1)) ? \
~~~~~ ^ ~~Avoid this warning with a cast to a signed 'int'.
--- net/ipv4/tcp_output.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
net/ipv4/tcp_output.c +1 -1
@@ -183,7 +183,7 @@ static inline void tcp_event_ack_sent(struct sock *sk, u32 rcv_nxt)if (unlikely (tp->compressed_ack)) {NET_ADD_STATS (sock_net (sk),LINUX_MIB_TCPACKCOMPRESSED , - tp->compressed_ack); + (int )tp->compressed_ack); tp->compressed_ack =0 ;if (hrtimer_try_to_cancel (&tp->compressed_ack_timer) ==1 )__sock_put (sk);
Signature · 1 line
2.39.2
Re: [PATCH 5/9] ipv4: tcp_output: avoid warning about NET_ADD_STATS
Quoted 23 lines · Arnd Bergmann
From: Arnd Bergmann <[email protected]>
Clang warns about a range check in
percpu_add_op()being impossible to hit for an u8 variable:
net/ipv4/tcp_output.c:188:3: error: result of comparison of constant -1 with expression of type 'u8' (aka 'unsigned char') is always false [-Werror,-Wtautological-constant-out-of-range-compare]NET_ADD_STATS(sock_net(sk), LINUX_MIB_TCPACKCOMPRESSED, ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ include/net/ip.h:291:41: note: expanded from macro 'NET_ADD_STATS' #define NET_ADD_STATS(net, field, adnd) SNMP_ADD_STATS((net)->mib.net_statistics, field, adnd) ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ include/net/snmp.h:143:4: note: expanded from macro 'SNMP_ADD_STATS' this_cpu_add(mib->mibs[field], addend) ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ include/linux/percpu-defs.h:509:33: note: expanded from macro 'this_cpu_add' #define this_cpu_add(pcp, val) __pcpu_size_call(this_cpu_add_, pcp, val) ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~note: (skipping 1 expansions in backtrace; use -fmacro-backtrace-limit=0 to see all) <scratch space>:187:1: note: expanded from here
this_cpu_add_8
^arch/x86/include/asm/percpu.h:326:35: note: expanded from macro 'this_cpu_add_8'
Quoted 6 lines
#define this_cpu_add_8(pcp, val) percpu_add_op(8, volatile, (pcp), val)
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
arch/x86/include/asm/percpu.h:127:31: note: expanded from macro 'percpu_add_op'
((val) == 1 || (val) == -1)) ? \
~~~~~ ^ ~~This seems like a bug in the macro or the compiler, because val is not a constant ?
__builtin_constant_p(val) should return false ???
+#define percpu_add_op(size, qual, var, val) \
+do { \
+ const int pao_ID__ = (__builtin_constant_p(val) && \
+ ((val) == 1 || (val) == -1)) ? \
+ (int)(val) : 0; \
+ if (0) { \
+ typeof(var) pao_tmp__; \
+ pao_tmp__ = (val); \
+ (void)pao_tmp__; \
+ } \
+ if (pao_ID__ == 1) \
+ percpu_unary_op(size, qual, "inc", var); \
+ else if (pao_ID__ == -1) \
+ percpu_unary_op(size, qual, "dec", var); \
+ else \
+ percpu_to_op(size, qual, "add", var, val); \+} while (0)
+
Quoted 23 lines
Avoid this warning with a cast to a signed 'int'.
Signed-off-by Arnd Bergmann <[email protected]>--- net/ipv4/tcp_output.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)net/ipv4/tcp_output.c +1
-2@@ -183,7 +183,7 @@ static inline void tcp_event_ack_sent(struct sock *sk, u32 rcv_nxt)if (unlikely (tp->compressed_ack)) {NET_ADD_STATS (sock_net (sk),LINUX_MIB_TCPACKCOMPRESSED , - tp->compressed_ack); + (int )tp->compressed_ack); tp->compressed_ack =0 ;if (hrtimer_try_to_cancel (&tp->compressed_ack_timer) ==1 )__sock_put (sk); --2.39.2
Re: [PATCH 5/9] ipv4: tcp_output: avoid warning about NET_ADD_STATS
Quoted 1 line · Eric Dumazet
On Thu, Mar 28, 2024 at 3:31 PM Arnd Bergmann <[email protected]> wrote:
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Quoted 10 lines
Quoted 4 lines
arch/x86/include/asm/percpu.h:127:31: note: expanded from macro 'percpu_add_op'((val) == 1 || (val) == -1)) ? \ ~~~~~ ^ ~~This seems like a bug in the macro or the compiler, because val is not a constant ?
__builtin_constant_p(val)should return false ???
Review
Quoted 5 lines
(quoted diff) +5
-0+#define percpu_add_op(size, qual, var, val) \ +do { \ + const int pao_ID__ = (__builtin_constant_p(val) && \ + ((val) == 1 || (val) == -1)) ? \ + (int)(val) : 0; \
It looks like gcc does the same thing, with the broader and still disabled -Wtype-limits, see: https://godbolt.org/z/3EPTGx68n
As far as I can tell, it does not matter that the comparison against -1 is never actually evaluated, since the warning is already printed before it simplifies the condition.
This is the only such warning I got from percpu, but I guess we could also add the cast inside of the macro, such as
arch/x86/include/asm/percpu.h +7 -5
@@ -181,12 +181,14 @@ do { \*/ #define percpu_add_op (size, qual, var, val) \ do { \ - const int pao_ID__ = (__builtin_constant_p(val) && \ - ((val) == 1 || (val) == -1)) ? \ - (int)(val) : 0; \ + __auto_type __val = (val); \ + const int pao_ID__ = (__builtin_constant_p(__val) && \ + ((__val) == (typeof(__val))1 || \ + (__val) == (typeof(__val))-1)) ? \ + (int)(__val) : 0; \ if (0) { \ typeof(var) pao_tmp__; \ - pao_tmp__ = (val); \ + pao_tmp__ = (__val); \ (void)pao_tmp__; \ } \ if (pao_ID__ == 1) \
@@ -194,7 +196,7 @@ do { \else if (pao_ID__ ==-1 ) \percpu_unary_op (size, qual,"dec" , var); \else \ -percpu_to_op (size, qual,"add" , var, val); \ +percpu_to_op (size, qual,"add" , var, __val); \ }while (0 ) #define percpu_from_op(size, qual, op, _var) \
I added a temporary variable there to avoid expanding the argument too many times.
Arnd
PATCH 6/9 nilfs2: fix out-of-range warning
In reply to Arnd BergmannFrom: Arnd Bergmann <[email protected]>
clang-14 points out that v_size is always smaller than a 64KB page size if that is configured by the CPU architecture:
fs/nilfs2/ioctl.c:63:19: error: result of comparison of constant 65536 with expression of type '__u16' (aka 'unsigned short') is always false [-Werror,-Wtautological-constant-out-of-range-compare]
if (argv->v_size > PAGE_SIZE)
~~~~~~~~~~~~ ^ ~~~~~~~~~This is ok, so just shut up that warning with a cast.
--- fs/nilfs2/ioctl.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
fs/nilfs2/ioctl.c +1 -1
@@ -60,7 +60,7 @@ static int nilfs_ioctl_wrap_copy(struct the_nilfs *nilfs,if (argv->v_nmembs ==0 )return 0 ; -if (argv->v_size >PAGE_SIZE ) +if ((size_t )argv->v_size >PAGE_SIZE )return -EINVAL ;/*
Signature · 1 line
2.39.2
Re: [PATCH 6/9] nilfs2: fix out-of-range warning
Quoted 5 lines · Arnd Bergmann
From: Arnd Bergmann <[email protected]>
clang-14 points out that
v_sizeis always smaller than a 64KB page size if that is configured by the CPU architecture:
Quoted 7 lines
fs/nilfs2/ioctl.c:63:19: error: result of comparison of constant 65536 with expression of type '__u16' (aka 'unsigned short') is always false [-Werror,-Wtautological-constant-out-of-range-compare] if (argv->v_size>PAGE_SIZE)
~~~~~~~~~~~~ ^ ~~~~~~~~~This is ok, so just shut up that warning with a cast.
nit:
It's not a warning, but actually a compile error, right?
(no idea why they make that an error btw. Warning would be perfectly fine)
Quoted 2 lines
Signed-off-by Arnd Bergmann <[email protected]>
Should / could that be backported to stable kernels in case people start building those with clang-14?
Regards,
P.
Quoted 18 lines
---
fs/nilfs2/ioctl.c | 2 +-1 file changed, 1 insertion(+), 1 deletion(-)fs/nilfs2/ioctl.c +0
-0@@ -60,7 +60,7 @@ static int nilfs_ioctl_wrap_copy(struct the_nilfs *nilfs, if (argv->v_nmembs == 0) return 0;
- if (argv->v_size>PAGE_SIZE)
+ if ((size_t)argv->v_size >PAGE_SIZE)
return-EINVAL;
/*
Re: [PATCH 6/9] nilfs2: fix out-of-range warning
Quoted 6 lines · Philipp Stanner
Quoted 2 lines · Arnd Bergmann
This is ok, so just shut up that warning with a cast.
nit:
It's not a warning, but actually a compile error, right?
I build with CONFIG_WERROR=y, which turns all warnings into errors. It's just a warning without that, and is currently only enabled when building with 'make W=1', though the point of my series is to have it always enabled.
Quoted 8 lines
(no idea why they make that an error btw. Warning would be perfectly fine)
Quoted 2 lines
Signed-off-by Arnd Bergmann <[email protected]>Should / could that be backported to stable kernels in case people start building those with clang-14?
It's clearly harmless and could be backported, but it is not needed either since older kernels will keep the option as part of W=1, not the default.
Arnd
Re: [PATCH 6/9] nilfs2: fix out-of-range warning
In reply to Arnd BergmannQuoted 5 lines · Arnd Bergmann
From: Arnd Bergmann <[email protected]>
clang-14 points out that
v_sizeis always smaller than a 64KB page size if that is configured by the CPU architecture:
Is this only with clang-14?
Quoted 8 lines
fs/nilfs2/ioctl.c:63:19: error: result of comparison of constant 65536 with expression of type '__u16' (aka 'unsigned short') is always false [-Werror,-Wtautological-constant-out-of-range-compare]if (argv->v_size > PAGE_SIZE) ~~~~~~~~~~~~ ^ ~~~~~~~~~This is ok, so just shut up that warning with a cast.
Signed-off-by Arnd Bergmann <[email protected]>
My question out of curiosity aside,
Quoted 20 lines
--- fs/nilfs2/ioctl.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)fs/nilfs2/ioctl.c +1
-2@@ -60,7 +60,7 @@ static int nilfs_ioctl_wrap_copy(struct the_nilfs *nilfs,if (argv->v_nmembs ==0 )return 0 ; -if (argv->v_size >PAGE_SIZE ) +if ((size_t )argv->v_size >PAGE_SIZE )return -EINVAL ;/* -- 2.39.2
Re: [PATCH 6/9] nilfs2: fix out-of-range warning
Quoted 8 lines · Justin Stitt
Quoted 5 lines · Arnd Bergmann
From: Arnd Bergmann <[email protected]>
clang-14 points out that
v_sizeis always smaller than a 64KB page size if that is configured by the CPU architecture:Is this only with clang-14?
No, it should be every version since then. I checked my randconfig tree and found that I originally committed this patch in September 2021 when I was first testing prerelease clang-14 builds. I sent a couple of fixes for this class of warnings then, but for some reason not this one.
Arnd
Re: [PATCH 6/9] nilfs2: fix out-of-range warning
In reply to Arnd BergmannQuoted 31 lines · Arnd Bergmann
From: Arnd Bergmann <[email protected]>
clang-14 points out that
v_sizeis always smaller than a 64KB page size if that is configured by the CPU architecture:
fs/nilfs2/ioctl.c:63:19: error: result of comparison of constant 65536 with expression of type '__u16' (aka 'unsigned short') is always false [-Werror,-Wtautological-constant-out-of-range-compare]if (argv->v_size > PAGE_SIZE) ~~~~~~~~~~~~ ^ ~~~~~~~~~This is ok, so just shut up that warning with a cast.
Signed-off-by Arnd Bergmann <[email protected]>--- fs/nilfs2/ioctl.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)fs/nilfs2/ioctl.c +1
-2@@ -60,7 +60,7 @@ static int nilfs_ioctl_wrap_copy(struct the_nilfs *nilfs,if (argv->v_nmembs ==0 )return 0 ; -if (argv->v_size >PAGE_SIZE ) +if ((size_t )argv->v_size >PAGE_SIZE )return -EINVAL ;/* --
Quoted 2 lines
2.39.2
Andrew, could you please apply this to the -mm tree along with the following tags and Justin's reviewed-by tag?
Thank you, Arnd.
I didn't notice this warning existed depending on the environment.
Ryusuke Konishi
Christian Brauner
AppliedRe: (subset) [PATCH 6/9] nilfs2: fix out-of-range warning
In reply to Arnd BergmannQuoted 8 lines · Arnd Bergmann
clang-14 points out that
v_sizeis always smaller than a 64KB page size if that is configured by the CPU architecture:
fs/nilfs2/ioctl.c:63:19: error: result of comparison of constant 65536 with expression of type '__u16' (aka 'unsigned short') is always false [-Werror,-Wtautological-constant-out-of-range-compare]if (argv->v_size > PAGE_SIZE) ~~~~~~~~~~~~ ^ ~~~~~~~~~[...]
Applied to the vfs.misc branch of the vfs/vfs.git tree. Patches in the vfs.misc branch should appear in linux-next soon.
Please report any outstanding bugs that were missed during review in a new review to the original patch series allowing us to drop it.
It's encouraged to provide Acked-bys and Reviewed-bys even though the patch has now been applied. If possible patch trailers will be updated.
Note that commit hashes shown below are subject to change due to rebase, trailer updates or similar. If in doubt, please check the listed branch.
tree: https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git branch: vfs.misc
[6/9] nilfs2: fix out-of-range warning
https://git.kernel.org/vfs/vfs/c/d20180d5dd10
PATCH 7/9 infiniband: uverbs: avoid out-of-range warnings
In reply to Arnd BergmannFrom: Arnd Bergmann <[email protected]>
clang warns for comparisons that are always true, which is the case for these two page size checks on architectures with 64KB pages:
drivers/infiniband/core/uverbs_ioctl.c:90:39: error: result of comparison of constant 65536 with expression of type 'u16' (aka 'unsigned short') is always false [-Werror,-Wtautological-constant-out-of-range-compare]
WARN_ON_ONCE(method_elm->bundle_size > PAGE_SIZE);
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~
include/asm-generic/bug.h:104:25: note: expanded from macro 'WARN_ON_ONCE'
int __ret_warn_on = !!(condition); \
^~~~~~~~~
drivers/infiniband/core/uverbs_ioctl.c:621:17: error: result of comparison of constant 65536 with expression of type '__u16' (aka 'unsigned short') is always false [-Werror,-Wtautological-constant-out-of-range-compare]
if (hdr.length > PAGE_SIZE ||
~~~~~~~~~~ ^ ~~~~~~~~~Add a cast to u32 in both cases, so it never warns about this.
--- drivers/infiniband/core/uverbs_ioctl.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/infiniband/core/uverbs_ioctl.c +2 -2
@@ -90,7 +90,7 @@ void uapi_compute_bundle_size(struct uverbs_api_ioctl_method *method_elm,ALIGN (bundle_size +256 ,sizeof (*pbundle->internal_buffer));/* Do not want order-2 allocations for this. */ -WARN_ON_ONCE (method_elm->bundle_size >PAGE_SIZE ); +WARN_ON_ONCE ((u32 )method_elm->bundle_size >PAGE_SIZE ); }/**
@@ -636,7 +636,7 @@ long ib_uverbs_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)if (err)return -EFAULT ; -if (hdr.length >PAGE_SIZE || +if ((u32 )hdr.length >PAGE_SIZE || hdr.length !=struct_size (&hdr, attrs, hdr.num_attrs))return -EINVAL ;
Signature · 1 line
2.39.2
Re: [PATCH 7/9] infiniband: uverbs: avoid out-of-range warnings
Quoted 3 lines · Arnd Bergmann
Add a cast to u32 in both cases, so it never warns about this.
Signed-off-by Arnd Bergmann <[email protected]>
Quoted 29 lines
--- drivers/infiniband/core/uverbs_ioctl.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)drivers/infiniband/core/uverbs_ioctl.c +2
-3@@ -90,7 +90,7 @@ void uapi_compute_bundle_size(struct uverbs_api_ioctl_method *method_elm,ALIGN (bundle_size +256 ,sizeof (*pbundle->internal_buffer));/* Do not want order-2 allocations for this. */ -WARN_ON_ONCE (method_elm->bundle_size >PAGE_SIZE ); +WARN_ON_ONCE ((u32 )method_elm->bundle_size >PAGE_SIZE ); }/** @@ -636,7 +636,7 @@ long ib_uverbs_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)if (err)return -EFAULT ; -if (hdr.length >PAGE_SIZE || +if ((u32 )hdr.length >PAGE_SIZE || hdr.length !=struct_size (&hdr, attrs, hdr.num_attrs))return -EINVAL ; --2.39.2
Re: [PATCH 7/9] infiniband: uverbs: avoid out-of-range warnings
In reply to Arnd BergmannQuoted 15 lines · Arnd Bergmann
From: Arnd Bergmann <[email protected]>
clang warns for comparisons that are always true, which is the case for these two page size checks on architectures with 64KB pages:
drivers/infiniband/core/uverbs_ioctl.c:90:39: error: result of comparison of constant 65536 with expression of type 'u16' (aka 'unsigned short') is always false [-Werror,-Wtautological-constant-out-of-range-compare]WARN_ON_ONCE(method_elm->bundle_size > PAGE_SIZE); ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~ include/asm-generic/bug.h:104:25: note: expanded from macro 'WARN_ON_ONCE' int __ret_warn_on = !!(condition); \ ^~~~~~~~~ drivers/infiniband/core/uverbs_ioctl.c:621:17: error: result of comparison of constant 65536 with expression of type '__u16' (aka 'unsigned short') is always false [-Werror,-Wtautological-constant-out-of-range-compare] if (hdr.length > PAGE_SIZE || ~~~~~~~~~~ ^ ~~~~~~~~~
Quoted 1 line
Add a cast to u32 in both cases, so it never warns about this.
But doesn't that hurt the codegen?
Jason
Re: [PATCH 7/9] infiniband: uverbs: avoid out-of-range warnings
Quoted 16 lines · Jason Gunthorpe
Quoted 15 lines · Arnd Bergmann
From: Arnd Bergmann <[email protected]>
clang warns for comparisons that are always true, which is the case for these two page size checks on architectures with 64KB pages:
drivers/infiniband/core/uverbs_ioctl.c:90:39: error: result of comparison of constant 65536 with expression of type 'u16' (aka 'unsigned short') is always false [-Werror,-Wtautological-constant-out-of-range-compare]WARN_ON_ONCE(method_elm->bundle_size > PAGE_SIZE); ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~ include/asm-generic/bug.h:104:25: note: expanded from macro 'WARN_ON_ONCE' int __ret_warn_on = !!(condition); \ ^~~~~~~~~ drivers/infiniband/core/uverbs_ioctl.c:621:17: error: result of comparison of constant 65536 with expression of type '__u16' (aka 'unsigned short') is always false [-Werror,-Wtautological-constant-out-of-range-compare] if (hdr.length > PAGE_SIZE || ~~~~~~~~~~ ^ ~~~~~~~~~
Quoted 3 lines
Quoted 1 line
Add a cast to u32 in both cases, so it never warns about this.
But doesn't that hurt the codegen?
I just double-checked in the compiler explorer to confirm that this works as I expected: both gcc and clang are still able to optimize out the comparison for 64K pages, but clang no longer complains after my change that this is an obvious case.
I also see that gcc still produces a -Wtype-limits warning, but that likely has to stay disabled because it produces too much output elsewhere and I don't see an easy way to shut it up.
Arnd
PATCH 8/9 mlx5: stop warning for 64KB pages
In reply to Arnd BergmannFrom: Arnd Bergmann <[email protected]>
When building with 64KB pages, clang points out that xsk->chunk_size can never be PAGE_SIZE:
drivers/net/ethernet/mellanox/mlx5/core/en/xsk/setup.c:19:22: error: result of comparison of constant 65536 with expression of type 'u16' (aka 'unsigned short') is always false [-Werror,-Wtautological-constant-out-of-range-compare]
if (xsk->chunk_size > PAGE_SIZE ||
~~~~~~~~~~~~~~~ ^ ~~~~~~~~~In older versions of this code, using PAGE_SIZE was the only possibility, so this would have never worked on 64KB page kernels, but the patch apparently did not address this case completely.
As Maxim Mikityanskiy suggested, 64KB chunks are really not all that useful, so just shut up the warning by adding a cast.
--- drivers/net/ethernet/mellanox/mlx5/core/en/xsk/setup.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
drivers/net/ethernet/mellanox/mlx5/core/en/xsk/setup.c +4 -2
@@ -28,8 +28,10 @@ bool mlx5e_validate_xsk_param(struct mlx5e_params *params, struct mlx5e_xsk_param *xsk, struct mlx5_core_dev *mdev) { -/* AF_XDP doesn't support frames larger than PAGE_SIZE. */ -if (xsk->chunk_size >PAGE_SIZE || xsk->chunk_size <MLX5E_MIN_XSK_CHUNK_SIZE ) { +/* AF_XDP doesn't support frames larger than PAGE_SIZE, +* and xsk->chunk_size is limited to 65535 bytes. +*/ +if ((size_t )xsk->chunk_size >PAGE_SIZE || xsk->chunk_size <MLX5E_MIN_XSK_CHUNK_SIZE ) {mlx5_core_err (mdev,"XSK chunk size %u out of bounds [%u, %lu]\n" , xsk->chunk_size,MLX5E_MIN_XSK_CHUNK_SIZE ,PAGE_SIZE );return false;
Signature · 1 line
2.39.2
Re: [PATCH 8/9] mlx5: stop warning for 64KB pages
Quoted 29 lines · Arnd Bergmann
From: Arnd Bergmann <[email protected]>
When building with 64KB pages, clang points out that
xsk->chunk_sizecan never bePAGE_SIZE:
drivers/net/ethernet/mellanox/mlx5/core/en/xsk/setup.c:19:22: error: result of comparison of constant 65536 with expression of type 'u16' (aka 'unsigned short') is always false [-Werror,-Wtautological-constant-out-of-range-compare]if (xsk->chunk_size > PAGE_SIZE || ~~~~~~~~~~~~~~~ ^ ~~~~~~~~~In older versions of this code, using
PAGE_SIZEwas the only possibility, so this would have never worked on 64KB page kernels, but the patch apparently did not address this case completely.As Maxim Mikityanskiy suggested, 64KB chunks are really not all that useful, so just shut up the warning by adding a cast.
Fixes 282c0c798f8e ("net/mlx5e: Allow XSK frames smaller than a page")Signed-off-by Arnd Bergmann <[email protected]>--- drivers/net/ethernet/mellanox/mlx5/core/en/xsk/setup.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)drivers/net/ethernet/mellanox/mlx5/core/en/xsk/setup.c +0
-0
drivers/net/ethernet/mellanox/mlx5/core/en/xsk/setup.c bool mlx5e_validate_xsk_param(struct mlx5e_params *params, · line 34
Quoted 9 lines
drivers/net/ethernet/mellanox/mlx5/core/en/xsk/setup.c +4
-2@@ -28,8 +28,10 @@ bool mlx5e_validate_xsk_param(struct mlx5e_params *params, struct mlx5e_xsk_param *xsk, struct mlx5_core_dev *mdev) { -/* AF_XDP doesn't support frames larger than PAGE_SIZE. */ -if (xsk->chunk_size >PAGE_SIZE || xsk->chunk_size <MLX5E_MIN_XSK_CHUNK_SIZE ) { +/* AF_XDP doesn't support frames larger than PAGE_SIZE, +* and xsk->chunk_size is limited to 65535 bytes. +*/ +if ((size_t )xsk->chunk_size >PAGE_SIZE || xsk->chunk_size <MLX5E_MIN_XSK_CHUNK_SIZE ) {
Quoted 6 lines
drivers/net/ethernet/mellanox/mlx5/core/en/xsk/setup.c +0
-0mlx5_core_err (mdev,"XSK chunk size %u out of bounds [%u, %lu]\n" , xsk->chunk_size,MLX5E_MIN_XSK_CHUNK_SIZE ,PAGE_SIZE );return false;Signature · 1 line
2.39.2
Re: [PATCH 8/9] mlx5: stop warning for 64KB pages
In reply to Arnd BergmannQuoted 5 lines · Arnd Bergmann
From: Arnd Bergmann <[email protected]>
When building with 64KB pages, clang points out that
xsk->chunk_sizecan never bePAGE_SIZE:
This is under W=1 right? Otherwise this is a mighty annoying warning.
Quoted 9 lines
drivers/net/ethernet/mellanox/mlx5/core/en/xsk/setup.c:19:22: error: result of comparison of constant 65536 with expression of type 'u16' (aka 'unsigned short') is always false [-Werror,-Wtautological-constant-out-of-range-compare]if (xsk->chunk_size > PAGE_SIZE || ~~~~~~~~~~~~~~~ ^ ~~~~~~~~~In older versions of this code, using
PAGE_SIZEwas the only possibility, so this would have never worked on 64KB page kernels, but the patch apparently did not address this case completely.
Quoted 7 lines
As Maxim Mikityanskiy suggested, 64KB chunks are really not all that useful, so just shut up the warning by adding a cast.
Fixes 282c0c798f8e ("net/mlx5e: Allow XSK frames smaller than a page")Signed-off-by Arnd Bergmann <[email protected]>
Quoted 24 lines
--- drivers/net/ethernet/mellanox/mlx5/core/en/xsk/setup.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)drivers/net/ethernet/mellanox/mlx5/core/en/xsk/setup.c +4
-3@@ -28,8 +28,10 @@ bool mlx5e_validate_xsk_param(struct mlx5e_params *params, struct mlx5e_xsk_param *xsk, struct mlx5_core_dev *mdev) { -/* AF_XDP doesn't support frames larger than PAGE_SIZE. */ -if (xsk->chunk_size >PAGE_SIZE || xsk->chunk_size <MLX5E_MIN_XSK_CHUNK_SIZE ) { +/* AF_XDP doesn't support frames larger than PAGE_SIZE, +* and xsk->chunk_size is limited to 65535 bytes. +*/ +if ((size_t )xsk->chunk_size >PAGE_SIZE || xsk->chunk_size <MLX5E_MIN_XSK_CHUNK_SIZE ) {mlx5_core_err (mdev,"XSK chunk size %u out of bounds [%u, %lu]\n" , xsk->chunk_size,MLX5E_MIN_XSK_CHUNK_SIZE ,PAGE_SIZE );return false; --2.39.2
Re: [PATCH 8/9] mlx5: stop warning for 64KB pages
Quoted 8 lines · Justin Stitt
Quoted 5 lines · Arnd Bergmann
From: Arnd Bergmann <[email protected]>
When building with 64KB pages, clang points out that
xsk->chunk_sizecan never bePAGE_SIZE:This is under W=1 right? Otherwise this is a mighty annoying warning.
At the moment yes. I'm fairly sure that I've covered all the common cases with thousands of randconfig builds, so we should be able to make it the default when this series is fully merged.
Arnd
Re: [PATCH 8/9] mlx5: stop warning for 64KB pages
In reply to Arnd BergmannQuoted 13 lines · Arnd Bergmann
From: Arnd Bergmann <[email protected]>
When building with 64KB pages, clang points out that
xsk->chunk_sizecan never bePAGE_SIZE:
drivers/net/ethernet/mellanox/mlx5/core/en/xsk/setup.c:19:22: error: result of comparison of constant 65536 with expression of type 'u16' (aka 'unsigned short') is always false [-Werror,-Wtautological-constant-out-of-range-compare]if (xsk->chunk_size > PAGE_SIZE || ~~~~~~~~~~~~~~~ ^ ~~~~~~~~~In older versions of this code, using
PAGE_SIZEwas the only possibility, so this would have never worked on 64KB page kernels, but the patch apparently did not address this case completely.
Quoted 7 lines
As Maxim Mikityanskiy suggested, 64KB chunks are really not all that useful, so just shut up the warning by adding a cast.
Fixes 282c0c798f8e ("net/mlx5e: Allow XSK frames smaller than a page")Signed-off-by Arnd Bergmann <[email protected]>
Thanks for your patch.
PATCH 9/9 kbuild: enable tautological-constant-out-of-range-compare
In reply to Arnd BergmannFrom: Arnd Bergmann <[email protected]>
All the previously reported warnings from this option have been addressed, so the option can now be left default-enabled, rather than disabled without W=1. There are not too many actual bugs found by this, but it can help detect a silly mistake earlier, and it's usually trivial to work around the false-positives.
--- scripts/Makefile.extrawarn | 1 - 1 file changed, 1 deletion(-)
scripts/Makefile.extrawarn +0 -1
@@ -120,7 +120,6 @@ KBUILD_CFLAGS += -Wformat-insufficient-args endif endif KBUILD_CFLAGS += $(call cc-disable-warning, pointer-to-enum-cast) -KBUILD_CFLAGS += -Wno-tautological-constant-out-of-range-compare KBUILD_CFLAGS += $(call cc-disable-warning, unaligned-access) KBUILD_CFLAGS += -Wno-enum-compare-conditional KBUILD_CFLAGS += -Wno-enum-enum-conversion
Signature · 1 line
2.39.2
This series was applied to netdev/net-next.git (main) by Jakub Kicinski <[email protected]>:
Hello:
This series was applied to netdev/net-next.git (main) by Jakub Kicinski <[email protected]>:
Quoted 9 lines · On Thu, 28 Mar 2024 15:30:38 +0100 you wrote:
From: Arnd Bergmann <[email protected]>
The warning option was introduced a few years ago but left disabled by default. All of the actual bugs that this has found have been fixed in the meantime, and this series should address the remaining false-positives, as tested on arm/arm64/x86 randconfigs as well as allmodconfig builds for all architectures supported by clang.
[...]
Here is the summary with links:
- [2/9] libceph: avoid clang out-of-range warning (no matching commit)
- [5/9] ipv4: tcp_output: avoid warning about NET_ADD_STATS (no matching commit)
- [8/9] mlx5: stop warning for 64KB pages
https://git.kernel.org/netdev/net-next/c/a5535e533694
You are awesome, thank you!
Signature · 2 lines
Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/patchwork/pwbot.html