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

Reply via email to