On 12/13/2017 03:16 PM, Jakub Jelinek wrote:
Hi!
This patch fixes 2 issues in format_floating. One is that when determining
precision, we should consider solely the type *printf* will read the
argument as (i.e. double unless L or ll modifier is used, in which case
long double), not the type of the argument, because the corresponding
argument could have any type, even not floating, or say __float128 etc.
This is fixed in the first 2 hunks.
The last hunk is to treat REAL_CSTs arguments of incompatible types
as unknown argument, we really don't know what say __float128 passed to
%f or double passed to %La will do; that is something diagnosed by -Wformat,
so the patch just treats it as arbitrary value of the type.
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
Thanks for the fix.
For the second part, can you please also add a compile-time test
to verify that the result isn't constrained to the same range as
with a real argument? Checking that the abort below isn't
eliminated would do it for %f:
void f (char *d)
{
int n = __builtin_sprintf (d, "%f", 1.0Q);
if (n < 8 || 13 < n)
__builtin_abort ();
}
A test that has a convenient setup for this is tree-ssa/builtin-
sprintf-2.c in case you want to add to it.
Martin
2017-12-13 Jakub Jelinek <ja...@redhat.com>
PR tree-optimization/83198
* gimple-ssa-sprintf.c (format_floating): Set type solely based on
dir.modifier, regardless of TREE_TYPE (arg). Assume non-REAL_CST
value if arg is a REAL_CST with incompatible type.
* gcc.dg/pr83198.c: New test.
--- gcc/gimple-ssa-sprintf.c.jj 2017-11-03 15:37:04.000000000 +0100
+++ gcc/gimple-ssa-sprintf.c 2017-12-13 13:37:59.289435623 +0100
@@ -1885,6 +1885,8 @@ static fmtresult
format_floating (const directive &dir, tree arg)
{
HOST_WIDE_INT prec[] = { dir.prec[0], dir.prec[1] };
+ tree type = (dir.modifier == FMT_LEN_L || dir.modifier == FMT_LEN_ll
+ ? long_double_type_node : double_type_node);
/* For an indeterminate precision the lower bound must be assumed
to be zero. */
@@ -1892,10 +1894,6 @@ format_floating (const directive &dir, t
{
/* Get the number of fractional decimal digits needed to represent
the argument without a loss of accuracy. */
- tree type = arg ? TREE_TYPE (arg) :
- (dir.modifier == FMT_LEN_L || dir.modifier == FMT_LEN_ll
- ? long_double_type_node : double_type_node);
-
unsigned fmtprec
= REAL_MODE_FORMAT (TYPE_MODE (type))->p;
@@ -1946,7 +1944,9 @@ format_floating (const directive &dir, t
}
}
- if (!arg || TREE_CODE (arg) != REAL_CST)
+ if (!arg
+ || TREE_CODE (arg) != REAL_CST
+ || !useless_type_conversion_p (type, TREE_TYPE (arg)))
return format_floating (dir, prec);
/* The minimum and maximum number of bytes produced by the directive. */
--- gcc/testsuite/gcc.dg/pr83198.c.jj 2017-12-13 13:43:36.056192309 +0100
+++ gcc/testsuite/gcc.dg/pr83198.c 2017-12-13 13:47:11.716474956 +0100
@@ -0,0 +1,18 @@
+/* PR tree-optimization/83198 */
+/* { dg-do compile } */
+/* { dg-options "-Wall -Wno-format" } */
+
+int
+foo (char *d[6], int x)
+{
+ int r = 0;
+ r += __builtin_sprintf (d[0], "%f", x);
+ r += __builtin_sprintf (d[1], "%a", x);
+ r += __builtin_sprintf (d[2], "%f", "foo");
+ r += __builtin_sprintf (d[3], "%a", "bar");
+#ifdef __SIZEOF_FLOAT128__
+ r += __builtin_sprintf (d[4], "%a", 1.0Q);
+ r += __builtin_sprintf (d[5], "%Lf", 1.0Q);
+#endif
+ return r;
+}
Jakub