On Sun, Mar 11, 2018 at 4:22 PM, Siddhesh Poyarekar
<siddh...@sourceware.org> wrote:
> Avoid issuing a bogus warning when the source of strncpy is bound by a
> constant and is known to be less than the size of the destination.
>
> Testsuite run is underway (not complete yet, but no new errors so far)
> and a bootstrap is also underway, I'll report status once they're both
> done.
>
> gcc/
>
> * 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 | 24 ++++++++++++++++++++++++
> 2 files changed, 41 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..49a31a551f5 100644
> --- a/gcc/tree-ssa-strlen.c
> +++ b/gcc/tree-ssa-strlen.c
> @@ -2125,6 +2125,30 @@ handle_builtin_stxncpy (built_in_function,
> gimple_stmt_iterator *gsi)
> return;
> }
>
> + /* When LEN is MIN_EXPR of strlen and a constant, then the copy is bound by
> + that constant. If the destination size is also constant then compare
> with
> + it to avoid a bogus warning. */
> + if (TREE_CODE (len) == SSA_NAME)
> + {
> + gimple *def_stmt = SSA_NAME_DEF_STMT (len);
> +
> + if (is_gimple_assign (def_stmt)
> + && gimple_assign_rhs_code (def_stmt) == MIN_EXPR)
> + {
> + /* RHS1 is the strlen, so check if RHS2 and DSTSIZE are constant.
> */
> + tree rhs2 = gimple_assign_rhs2 (def_stmt);
> + tree dstsize = compute_objsize (dst, 1);
> +
> + if (TREE_CODE (rhs2) == INTEGER_CST
> + && TREE_CODE (dstsize) == INTEGER_CST
> + && int_cst_value (rhs2) < int_cst_value (dstsize))
Please use tree_int_cst_lt (rhs1, dstsize) instead.
> + {
> + gimple_set_no_warning (stmt, true);
Why this? There's only a single bit -- where do we warn from if you
don't do this here?
I also wonder why this code doesn't use range-info given the
INTEGER_CST constraints
range-info should tell us more than just handling MIN_EXPR?
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
>
