Re: [PATCH] use zero as the lower bound for a signed-unsigned range (PR 79327)

[Jakub Jelinek <jakub at redhat dot com>]

On Wed, Feb 01, 2017 at 04:52:35PM -0700, Martin Sebor wrote:
> --- a/gcc/gimple-ssa-sprintf.c
> +++ b/gcc/gimple-ssa-sprintf.c
> @@ -1382,13 +1382,26 @@ format_integer (const directive &dir, tree arg)
>          would reflect the largest possible precision (i.e., INT_MAX).  */
>        res.range.min = format_integer (dir, argmax).range.min;
>        res.range.max = format_integer (dir, argmin).range.max;
> -    }
>  
> -  if (res.range.max < res.range.min)
> -    {
> -      unsigned HOST_WIDE_INT tmp = res.range.max;
> -      res.range.max = res.range.min;
> -      res.range.min = tmp;
> +      if (res.range.max < res.range.min)
> +       {
> +         unsigned HOST_WIDE_INT tmp = res.range.max;
> +         res.range.max = res.range.min;
> +         res.range.min = tmp;

These 3 lines are
	std::swap (res.range.max, res.range.min);
also note the spaces instead of tabs and indentation by 7/9 spaces
instead of 1 tab and 1 tab + 2 spaces.

> +       }
> +
> +      if (!TYPE_UNSIGNED (argtype)
> +         && tree_int_cst_lt (integer_zero_node, argmax)
> +         && tree_int_cst_lt (argmin, integer_zero_node))

And these 2 lines
	  && tree_int_cst_sgn (argmax) > 0
	  && tree_int_cst_sgn (argmin) < 0

> +       {
> +         /* The minimum output for a signed argument in a negative-positive
> +            range is that of zero.  */
> +         unsigned HOST_WIDE_INT
> +           nzero = format_integer (dir, integer_zero_node).range.min;
> +
> +         if (nzero < res.range.min)
> +           res.range.min = nzero;
> +       }
>      }
>  
>    res.range.likely = res.knownrange ? res.range.max : res.range.min;

> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/tree-ssa/pr79327.c
> @@ -0,0 +1,31 @@
> +/* PR tree-optimization/79327 - wrong code at -O2 and -fprintf-return-value
> +   { dg-do run }
> +   { dg-options "-O2 -Wall" }  */
> +
> +volatile int a, b = -1;
> +char buf[64];
> +
> +#define FMT "%+03d%02d"
> +const char* fmt = FMT;

Perhaps make it even
const char *volatile fmt = FMT;
to make sure we don't see through it even with -fwhole-program or LTO?
Don't you need -Wno-format-security or is that not on in -Wall?

> +
> +int main ()
> +{
> +  int c = a;
> +  int d = b;
> +  if (c >= -35791395 && c < 35791394 && d >= -1 && d < __INT_MAX__)
> +    {
> +      /* In the following the range of return values can be computed
> +	 by GCC. */
> +      int n1 = __builtin_sprintf (buf, FMT, c + 1, d + 1);
> +      if (n1 > 7)
> +	__builtin_abort ();
> +
> +      /* Here GCC can't see the format string so the return value
> +	 must be computed by a libc call.  */
> +      int n2 = __builtin_sprintf (buf, fmt, c + 1, d + 1);
> +
> +      if (n1 != n2)
> +        __builtin_abort ();
> +    }
> +  return 0;
> +}


	Jakub