On 01/06/2018 03:04 PM, Martin Sebor wrote:
> Bug 83671 - Fix for false positive reported by -Wstringop-overflow
> does not work at -O1, points out that the string length range
> optimization implemented as a solution for bug 83373 doesn't help
> at -O1. The root cause is that the fix was added to the strlen
> pass that doesn't run at -O1.
>
> The string length range computation doesn't depend on the strlen
> pass, and so the range can be set earlier, in gimple-fold, and
> its results made available even at -O1. The attached patch
> changes the gimple_fold_builtin_strlen() function to do that.
>
> While testing the change I came across a number of other simple
> strlen cases that currently aren't handled, some at -O1, others
> at all. I added code to handle some of the simplest of them
> and opened bugs to remind us/myself to get back to the rest in
> the future (pr83693 and pr83702). The significant enhancement
> is handling arrays of arrays with non-constant indices and
> pointers to such things, such as in:
>
> char a[2][7];
>
> void f (int i)
> {
> if (strlen (a[i]) > 6) // eliminated with the patch
> abort ();
> }
>
> Attached is a near-minimal patch to handle PR 83671.
>
> Martin
>
> gcc-83671.diff
>
>
> PR tree-optimization/83671 - Fix for false positive reported by
> -Wstringop-overflow does not work with inlining
>
> gcc/testsuite/ChangeLog:
>
> PR tree-optimization/83671
> * gcc.dg/strlenopt-40.c: New test.
> * gcc.dg/strlenopt-41.c: New test.
>
> gcc/ChangeLog:
>
> PR tree-optimization/83671
> * builtins.c (c_strlen): Unconditionally return zero for the empty
> string.
> Use -Warray-bounds for warnings.
> * gimple-fold.c (get_range_strlen): Handle non-constant lengths
> for non-constant array indices with COMPONENT_REF, arrays of
> arrays, and pointers to arrays.
> (gimple_fold_builtin_strlen): Determine and set length range for
> non-constant character arrays.
>
> @@ -1311,14 +1311,30 @@ get_range_strlen (tree arg, tree length[2], bitmap
> *visited, int type,
[ ... ]
> + else if (TREE_CODE (TREE_OPERAND (op, 0)) == COMPONENT_REF && fuzzy)
> + {
> + tree idx = TREE_OPERAND (op, 1);
> +
> + arg = TREE_OPERAND (op, 0);
> + tree optype = TREE_TYPE (arg);
> + if (tree dom = TYPE_DOMAIN (optype))
> + if (tree bound = TYPE_MAX_VALUE (dom))
> + if (TREE_CODE (bound) == INTEGER_CST
> + && TREE_CODE (idx) == INTEGER_CST
> + && tree_int_cst_lt (bound, idx))
> + return false;
This deserves a comment what are you looking for and why do you return
false when you find it. I think I know the answers, but it'd be clearer
to future readers to spell it out in a comment.
With that comment and removal of the controversial set_range_info, I
think this is OK.
Jeff
