On Tue, 2020-06-02 at 18:12 -0600, Martin Sebor wrote: > The compute_objsize() function started out as a thin wrapper around > compute_builtin_object_size(), but over time developed its own > features to compensate for the other function's limitations (such > as its inability to work with ranges). The interaction of these > features and the limitations has started to become increasingly > problematic as the former function is used in more contexts. > > A complete "fix" for all the problems (as well as some other > limitations) that I'm working on will be more extensive and won't > be appropriate for backports. Until then, the attached patch > cleans up the extensions compute_objsize() has accumulated over > the years to avoid a class of false positives. > > To make the warnings issued based on the results of the function > easier to understand and fix, the patch also adds an informative > message to many instances of -Wstringop-overflow to point to > the object to which the warning refers. This is especially > helpful when the object is referenced by a series of pointer > operations. > > Tested by boostrapping on x86_64-linux and building Binutils/GDB, > Glibc, and the Linux kernel with no new warnings. > > Besides applying it to trunk I'm looking to backport the fix to > GCC 10. > > Martin
> PR middle-end/95353 - spurious -Wstringop-overflow writing to a trailing > array plus offset > PR middle-end/92939 - missing -Wstringop-overflow on negative index from the > end of array > > gcc/ChangeLog: > > PR middle-end/95353 > PR middle-end/92939 > * builtins.c (inform_access): New function. > (check_access): Call it. Add argument. > (addr_decl_size): Remove. > (get_range): New function. > (compute_objsize): New overload. Only use compute_builtin_object_size > with raw memory function. > (check_memop_access): Pass new argument to compute_objsize and > check_access. > (expand_builtin_memchr, expand_builtin_strcat): Same. > (expand_builtin_strcpy, expand_builtin_stpcpy_1): Same. > (expand_builtin_stpncpy, check_strncat_sizes): Same. > (expand_builtin_strncat, expand_builtin_strncpy): Same. > (expand_builtin_memcmp): Same. > * builtins.h (check_nul_terminated_array): Declare extern. > (check_access): Add argument. > (struct access_ref, struct access_data): New structs. > * gimple-ssa-warn-restrict.c (clamp_offset): New helper. > (builtin_access::overlap): Call it. > * tree-object-size.c ((decl_init_size): Declare extern. > (addr_object_size): Correct offset computation. > * tree-object-size.h (decl_init_size): Declare. > * tree-ssa-strlen.c (handle_integral_assign): Remove a call > to maybe_warn_overflow when assigning to an SSA_NAME. > > gcc/testsuite/ChangeLog: > > PR middle-end/95353 > PR middle-end/92939 > * c-c++-common/Wstringop-truncation.c: Remove an xfail. > * gcc.dg/Warray-bounds-46.c: Remove a bogus warning. > * gcc.dg/Wrestrict-9.c: Disable -Wstringop-overflow. > * gcc.dg/Wstringop-overflow-12.c: Remove xfails. > * gcc.dg/Wstringop-overflow-28.c: Same. > * gcc.dg/builtin-stringop-chk-4.c: Same. > * gcc.dg/builtin-stringop-chk-5.c: Same. > * gcc.dg/builtin-stringop-chk-8.c: Same. > * gcc.dg/strlenopt-74.c: Avoid buffer overflow. > * gcc.dg/Wstringop-overflow-33.c: New test. > * gcc.dg/Wstringop-overflow-34.c: New test. > * gcc.dg/Wstringop-overflow-35.c: New test. > * gcc.dg/Wstringop-overflow-36.c: New test. > * gcc.dg/Wstringop-overflow-37.c: New test. > Two nits: > /* Helper to determine and check the sizes of the source and the destination > diff --git a/gcc/tree-object-size.c b/gcc/tree-object-size.c > index 8855065f577..b73a8cb7b1b 100644 > --- a/gcc/tree-object-size.c > +++ b/gcc/tree-object-size.c > @@ -189,7 +189,7 @@ decl_init_size (tree decl, bool min) > || TYPE_SIZE (last_type)) > return size; > > - /* Use TYPE_SIZE_UNIT; DECL_SIZE_UNIT sometimes reflects the size > + /* Use TYPE_SIZE_UNIT; DECL_SIZEgcc.dg/Warray-bounds-46.c_UNIT sometimes > reflects the size Looks like a spurious change. Please fix. > diff --git a/gcc/tree-object-size.h b/gcc/tree-object-size.h > index 871dcdba06a..89d4c46be72 100644 > --- a/gcc/tree-object-size.h > +++ b/gcc/tree-object-size.h > @@ -24,5 +24,6 @@ extern void init_object_sizes (void); > extern void fini_object_sizes (void); > extern bool compute_builtin_object_size (tree, int, unsigned HOST_WIDE_INT *, > tree * = NULL, tree * = NULL); > +tree decl_init_size (tree, bool); I believe convention would be to include the "extern" in the prototype. OK with the nits fixed. jeff