Re: [PATCH] handle integer overflow/wrapping in printf directives (PR 78622)

[Jakub Jelinek <jakub at redhat dot com>]

On Wed, Nov 30, 2016 at 08:26:04PM -0700, Martin Sebor wrote:
> @@ -795,6 +795,43 @@ get_width_and_precision (const conversion_spec &spec,
>    *pprec = prec;
>  }
>  
> +/* With the range [*ARGMIN, *ARGMAX] of an integer directive's actual
> +   argument, due to the conversion from either *ARGMIN or *ARGMAX to
> +   the type of the directive's formal argument it's possible for both
> +   to result in the same number of bytes or a range of bytes that's
> +   less than the number of bytes that would result from formatting
> +   some other value in the range [*ARGMIN, *ARGMAX].  This can be
> +   determined by checking for the actual argument being in the range
> +   of the type of the directive.  If it isn't it must be assumed to
> +   take on the full range of the directive's type.
> +   Return true when the range has been adjusted, false otherwise.  */
> +
> +static bool
> +adjust_range_for_overflow (tree dirtype, tree *argmin, tree *argmax)
> +{
> +  tree dirmin = TYPE_MIN_VALUE (dirtype);
> +  tree dirmax = TYPE_MAX_VALUE (dirtype);
> +
> +  if (tree_int_cst_lt (*argmin, dirmin)
> +      || tree_int_cst_lt (dirmax, *argmin)
> +      || tree_int_cst_lt (*argmax, dirmin)
> +      || tree_int_cst_lt (dirmax, *argmax))
> +    {
> +      if (TYPE_UNSIGNED (dirtype))
> +	{
> +	  *argmin = dirmin;
> +	  *argmax = dirmax;
> +	}
> +      else
> +	{
> +	  *argmin = integer_zero_node;
> +	  *argmax = dirmin;
> +	}
> +      return true;
> +    }

Isn't this too simplistic?  I mean, if you have say dirtype of signed char
and argmin say 4096 + 32 and argmax say 4096 + 64, (signed char) arg
has range 32, 64, while I think your routine will yield -128, 127 (well,
0 as min and -128 as max as that is what you return for signed type).

Can't you subtract argmax - argmin (best just in wide_int, no need to build
trees), and use what you have just for the case where that number doesn't
fit into the narrower precision, otherwise if argmin - (dirtype) argmin
== argmax - (dirtype) argmax, just use (dirtype) argmin and (dirtype) argmax
as the range, and in case that it crosses a boundary figure if you can do
anything than the above?  Guess all cases of signed/unsigned dirtype and/or
argtype need to be considered.

Also, is argmin and argmax in this case the actual range (what should go
into res.arg{min,max}), or the values with shortest/longest representation?
Wouldn't it be better to always compute the range of values that can be
printed and only later on (after all VR_RANGE and VR_VARYING handling)
transform that into the number with shortest/longest representation in that
range?  Perhaps even using different variable names for the latter would
make things clearer (argshortest, arglongest or whatever).

	Jakub