On 12/01/2016 07:31 PM, Martin Sebor wrote:
On 12/01/2016 02:15 AM, Jakub Jelinek wrote:
On Thu, Dec 01, 2016 at 08:26:47AM +0100, Jakub Jelinek wrote:
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.
Richard noted that you could have a look at CONVERT_EXPR_CODE_P
handling in extract_range_from_unary_expr. I think it is the
|| (vr0.type == VR_RANGE
&& integer_zerop (int_const_binop (RSHIFT_EXPR,
int_const_binop (MINUS_EXPR, vr0.max, vr0.min),
size_int (TYPE_PRECISION (outer_type)))))))
part that is important here for the narrowing conversion.
Thanks, that was a helpful suggestion! Attached is an update that
follows the vrp approach. I assume the infinity stuff is specific
to the VRP pass and not something I need to worry about here.
Martin
PS To your earlier question, argmin and argmax have the same meaning
in the added helper function as in its caller.
gcc-78622.diff
PR middle-end/78622 - [7 Regression] -Wformat-length/-fprintf-return-value
incorrect with overflow/wrapping
gcc/ChangeLog:
PR middle-end/78622
* gimple-ssa-sprintf.c (min_bytes_remaining): Use res.knownrange
rather than res.bounded.
(adjust_range_for_overflow): New function.
(format_integer): Always set res.bounded to true unless either
precision or width is specified and unknown.
Call adjust_range_for_overflow.
(format_directive): Remove vestigial quoting.
(add_bytes): Correct the computation of boundrange used to
decide whether a warning is of a "maybe" or "defnitely" kind.
s/defnitely/definitely/
gcc/testsuite/ChangeLog:
PR middle-end/78622
* gcc.c-torture/execute/pr78622.c: New test.
* gcc.dg/tree-ssa/builtin-sprintf-2.c: Remove "benign" undefined
behavior inadvertently introduced in a previous commit. Tighten
up final checking.
* gcc.dg/tree-ssa/builtin-sprintf-5.c: Rename macros for clarity.
Add test cases.
* gcc.dg/tree-ssa/builtin-sprintf-6.c: Add test cases.
* gcc.dg/tree-ssa/builtin-sprintf-warn-1.c: Same.
* gcc.dg/tree-ssa/builtin-sprintf-warn-6.c: Remove xfails and
add a final optimization check.
* gcc.dg/tree-ssa/builtin-sprintf.c: Add test cases.
diff --git a/gcc/gimple-ssa-sprintf.c b/gcc/gimple-ssa-sprintf.c
index 99a635a..95a8710 100644
--- a/gcc/gimple-ssa-sprintf.c
+++ b/gcc/gimple-ssa-sprintf.c
@@ -795,6 +795,59 @@ 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 to the range of
+ DIRTYPE, false otherwise. */
I wish I'd counted how many times I read that.
+
+static bool
+adjust_range_for_overflow (tree dirtype, tree *argmin, tree *argmax)
+{
+ unsigned argprec = TYPE_PRECISION (TREE_TYPE (*argmin));
+ unsigned dirprec = TYPE_PRECISION (dirtype);
+
+ /* The logic here was inspired/lifted from the CONVERT_EXPR_CODE_P
+ branch in the extract_range_from_unary_expr function in tree-vrp.c.
+ */
Formatting it. If the comment-close won't fit, then line wrap prior to
the last word in the comment.
+
+ if (TREE_CODE (*argmin) == INTEGER_CST
+ && TREE_CODE (*argmax) == INTEGER_CST
+ && (dirprec >= argprec
+ || integer_zerop (int_const_binop (RSHIFT_EXPR,
+ int_const_binop (MINUS_EXPR,
+ *argmax,
+ *argmin),
+ size_int (dirprec)))))
+ {
+ *argmin = force_fit_type (dirtype, wi::to_widest (*argmin), 0, false);
+ *argmax = force_fit_type (dirtype, wi::to_widest (*argmax), 0, false);
+ return false;
+ }
So in this case we're not changing the values, we're just converting
them to a equal or wider type, right? Thus no adjustment (what about a
sign change? Is that an adjustment?) and we return false per the
function's specifications.
What about overflows when either argmin or argmax won't fit into dirtype
without an overflow? What's the right behavior there?
+
+ tree dirmin = TYPE_MIN_VALUE (dirtype);
+ tree dirmax = TYPE_MAX_VALUE (dirtype);
From this point onward argmin/argmax are either not integers or we're
doing a narrowing conversion, right? In both cases the fact that we're
doing a narrowing conversion constrains the range
+
+ if (TYPE_UNSIGNED (dirtype))
+ {
+ *argmin = dirmin;
+ *argmax = dirmax;
+ }
+ else
+ {
+ *argmin = integer_zero_node;
+ *argmax = dirmin;
+ }
Should this be dirmax? Am I missing something here?
Jeff
