Re: [PATCH v2] Fix bogus strncpy source length warning on source bound by constant

[Richard Biener <richard dot guenther at gmail dot com>]

On Tue, Mar 13, 2018 at 7:22 PM, Siddhesh Poyarekar
<siddhesh@sourceware.org> wrote:
> Avoid issuing a bogus warning when the source of strncpy is bound by a
> constant known to be less than the minimum size of the destination.
>
> Changes from v1:
>
> - Use range-info instead of the MIN_EXPR hack
> - Get the minimum size of dst and check for NULL_TREE return
>
> The patch bootstraps successfully and introduces no new regressions in
> the testsuite.
>
> gcc/
>
>         * tree-ssa-strlen.c (handle_builtin_stxncpy): Check bounds of
>         source length if available.
>
> gcc/testsuite/
>
>         * gcc.dg/builtin-stringop-chk-10.c: New test case.
> ---
>  gcc/testsuite/gcc.dg/builtin-stringop-chk-10.c | 17 +++++++++++++++++
>  gcc/tree-ssa-strlen.c                          | 15 +++++++++++++++
>  2 files changed, 32 insertions(+)
>  create mode 100644 gcc/testsuite/gcc.dg/builtin-stringop-chk-10.c
>
> diff --git a/gcc/testsuite/gcc.dg/builtin-stringop-chk-10.c b/gcc/testsuite/gcc.dg/builtin-stringop-chk-10.c
> new file mode 100644
> index 00000000000..13e4bd2f049
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/builtin-stringop-chk-10.c
> @@ -0,0 +1,17 @@
> +/* Bogus -Wstringop-overflow on strncpy when size is based on strlen but is
> +   bound by a constant.
> +   { dg-do compile }
> +   { dg-options "-O2 -Wstringop-overflow" } */
> +
> +char dst[1024];
> +
> +void
> +f1 (const char *src)
> +{
> +  unsigned long limit = 512;
> +  unsigned long len = __builtin_strlen (src);  /* { dg-bogus "length computed here" } */
> +  if (len > limit)
> +    len = limit;
> +
> +  __builtin_strncpy (dst, src, len);   /* { dg-bogus "specified bound depends on the length of the source argument" } */
> +}
> diff --git a/gcc/tree-ssa-strlen.c b/gcc/tree-ssa-strlen.c
> index 72f6a17cd32..265f351ea85 100644
> --- a/gcc/tree-ssa-strlen.c
> +++ b/gcc/tree-ssa-strlen.c
> @@ -2125,6 +2125,21 @@ handle_builtin_stxncpy (built_in_function, gimple_stmt_iterator *gsi)
>        return;
>      }
>
> +  /* Don't bother about the strlen (SRC) in the LEN computation if the range of
> +     values for LEN ends up within dstsize.  */
> +  if (TREE_CODE (len) == SSA_NAME)
> +    {
> +      wide_int min, max;
> +      enum value_range_type vr = get_range_info (len, &min, &max);
> +      tree dstsize = compute_objsize (dst, 3);
> +      if (vr == VR_RANGE && dstsize)
> +       {
> +         tree len_max = wide_int_to_tree (TREE_TYPE (dstsize), max);
> +         if (tree_int_cst_lt (len_max, dstsize))

Instead of building a tree from max you should use

    if (wi::to_widest (max) < wi::to_widest (wi::to_wide (dstsize)))
      return;

given compute_objsize is somewhat confused about the type it returns
a widest_int compare is required.

Note I'm not too familiar with tree-ssa-strlen.c nor this part of the
warning code
so I'll not approve the patch but after fixing that it looks techincally ok.

Richard.

> +           return;
> +       }
> +    }
> +
>    /* Retrieve the strinfo data for the string S that LEN was computed
>       from as some function F of strlen (S) (i.e., LEN need not be equal
>       to strlen(S)).  */
> --
> 2.14.3
>