| Message ID | 20240201090434.1762340-1-elver@google.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [-mm,v2] stackdepot: fix -Wstringop-overflow warning | expand |
On Thu, Feb 1, 2024 at 10:04 AM Marco Elver <elver@google.com> wrote: > > Since 113a61863ecb ("Makefile: Enable -Wstringop-overflow globally") > string overflow checking is enabled by default. Within stackdepot, the > compiler (GCC 13.2.0) assumes that a multiplication overflow may be > possible and flex_array_size() can return SIZE_MAX (4294967295 on > 32-bit), resulting in this warning: > > In function 'depot_alloc_stack', > inlined from 'stack_depot_save_flags' at lib/stackdepot.c:688:4: > arch/x86/include/asm/string_32.h:150:25: error: '__builtin_memcpy' specified bound 4294967295 exceeds maximum object size 2147483647 [-Werror=stringop-overflow=] > 150 | #define memcpy(t, f, n) __builtin_memcpy(t, f, n) > | ^~~~~~~~~~~~~~~~~~~~~~~~~ > lib/stackdepot.c:459:9: note: in expansion of macro 'memcpy' > 459 | memcpy(stack->entries, entries, flex_array_size(stack, entries, nr_entries)); > | ^~~~~~ > cc1: all warnings being treated as errors > > This is due to depot_alloc_stack() accepting an 'int nr_entries' which > could be negative without deeper analysis of callers. > > The call to depot_alloc_stack() from stack_depot_save_flags(), however, > only passes in its nr_entries which is unsigned int. Fix the warning by > switching depot_alloc_stack()'s nr_entries to also be unsigned. > > Link: https://lore.kernel.org/all/20240201135747.18eca98e@canb.auug.org.au/ > Fixes: d869d3fb362c ("stackdepot: use variable size records for non-evictable entries") > Reported-by: Stephen Rothwell <sfr@canb.auug.org.au> > Signed-off-by: Marco Elver <elver@google.com> > --- > v2: > * Just switch 'nr_entries' to unsigned int which is already the case > elsewhere. > --- > lib/stackdepot.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/lib/stackdepot.c b/lib/stackdepot.c > index 8f3b2c84ec2d..4a7055a63d9f 100644 > --- a/lib/stackdepot.c > +++ b/lib/stackdepot.c > @@ -420,7 +420,7 @@ static inline size_t depot_stack_record_size(struct stack_record *s, unsigned in > > /* Allocates a new stack in a stack depot pool. */ > static struct stack_record * > -depot_alloc_stack(unsigned long *entries, int nr_entries, u32 hash, depot_flags_t flags, void **prealloc) > +depot_alloc_stack(unsigned long *entries, unsigned int nr_entries, u32 hash, depot_flags_t flags, void **prealloc) > { > struct stack_record *stack = NULL; > size_t record_size; > -- > 2.43.0.429.g432eaa2c6b-goog > Reviewed-by: Andrey Konovalov <andreyknvl@gmail.com>
Hi all, On Thu, 1 Feb 2024 10:04:30 +0100 Marco Elver <elver@google.com> wrote: > > Since 113a61863ecb ("Makefile: Enable -Wstringop-overflow globally") > string overflow checking is enabled by default. Within stackdepot, the > compiler (GCC 13.2.0) assumes that a multiplication overflow may be > possible and flex_array_size() can return SIZE_MAX (4294967295 on > 32-bit), resulting in this warning: > > In function 'depot_alloc_stack', > inlined from 'stack_depot_save_flags' at lib/stackdepot.c:688:4: > arch/x86/include/asm/string_32.h:150:25: error: '__builtin_memcpy' specified bound 4294967295 exceeds maximum object size 2147483647 [-Werror=stringop-overflow=] > 150 | #define memcpy(t, f, n) __builtin_memcpy(t, f, n) > | ^~~~~~~~~~~~~~~~~~~~~~~~~ > lib/stackdepot.c:459:9: note: in expansion of macro 'memcpy' > 459 | memcpy(stack->entries, entries, flex_array_size(stack, entries, nr_entries)); > | ^~~~~~ > cc1: all warnings being treated as errors > > This is due to depot_alloc_stack() accepting an 'int nr_entries' which > could be negative without deeper analysis of callers. > > The call to depot_alloc_stack() from stack_depot_save_flags(), however, > only passes in its nr_entries which is unsigned int. Fix the warning by > switching depot_alloc_stack()'s nr_entries to also be unsigned. > > Link: https://lore.kernel.org/all/20240201135747.18eca98e@canb.auug.org.au/ > Fixes: d869d3fb362c ("stackdepot: use variable size records for non-evictable entries") > Reported-by: Stephen Rothwell <sfr@canb.auug.org.au> > Signed-off-by: Marco Elver <elver@google.com> > --- > v2: > * Just switch 'nr_entries' to unsigned int which is already the case > elsewhere. > --- > lib/stackdepot.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/lib/stackdepot.c b/lib/stackdepot.c > index 8f3b2c84ec2d..4a7055a63d9f 100644 > --- a/lib/stackdepot.c > +++ b/lib/stackdepot.c > @@ -420,7 +420,7 @@ static inline size_t depot_stack_record_size(struct stack_record *s, unsigned in > > /* Allocates a new stack in a stack depot pool. */ > static struct stack_record * > -depot_alloc_stack(unsigned long *entries, int nr_entries, u32 hash, depot_flags_t flags, void **prealloc) > +depot_alloc_stack(unsigned long *entries, unsigned int nr_entries, u32 hash, depot_flags_t flags, void **prealloc) > { > struct stack_record *stack = NULL; > size_t record_size; > -- > 2.43.0.429.g432eaa2c6b-goog > I have applied this patch to the merge of the mm tree today.
diff --git a/lib/stackdepot.c b/lib/stackdepot.c index 8f3b2c84ec2d..4a7055a63d9f 100644 --- a/lib/stackdepot.c +++ b/lib/stackdepot.c @@ -420,7 +420,7 @@ static inline size_t depot_stack_record_size(struct stack_record *s, unsigned in /* Allocates a new stack in a stack depot pool. */ static struct stack_record * -depot_alloc_stack(unsigned long *entries, int nr_entries, u32 hash, depot_flags_t flags, void **prealloc) +depot_alloc_stack(unsigned long *entries, unsigned int nr_entries, u32 hash, depot_flags_t flags, void **prealloc) { struct stack_record *stack = NULL; size_t record_size;
Since 113a61863ecb ("Makefile: Enable -Wstringop-overflow globally") string overflow checking is enabled by default. Within stackdepot, the compiler (GCC 13.2.0) assumes that a multiplication overflow may be possible and flex_array_size() can return SIZE_MAX (4294967295 on 32-bit), resulting in this warning: In function 'depot_alloc_stack', inlined from 'stack_depot_save_flags' at lib/stackdepot.c:688:4: arch/x86/include/asm/string_32.h:150:25: error: '__builtin_memcpy' specified bound 4294967295 exceeds maximum object size 2147483647 [-Werror=stringop-overflow=] 150 | #define memcpy(t, f, n) __builtin_memcpy(t, f, n) | ^~~~~~~~~~~~~~~~~~~~~~~~~ lib/stackdepot.c:459:9: note: in expansion of macro 'memcpy' 459 | memcpy(stack->entries, entries, flex_array_size(stack, entries, nr_entries)); | ^~~~~~ cc1: all warnings being treated as errors This is due to depot_alloc_stack() accepting an 'int nr_entries' which could be negative without deeper analysis of callers. The call to depot_alloc_stack() from stack_depot_save_flags(), however, only passes in its nr_entries which is unsigned int. Fix the warning by switching depot_alloc_stack()'s nr_entries to also be unsigned. Link: https://lore.kernel.org/all/20240201135747.18eca98e@canb.auug.org.au/ Fixes: d869d3fb362c ("stackdepot: use variable size records for non-evictable entries") Reported-by: Stephen Rothwell <sfr@canb.auug.org.au> Signed-off-by: Marco Elver <elver@google.com> --- v2: * Just switch 'nr_entries' to unsigned int which is already the case elsewhere. --- lib/stackdepot.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)