* [PATCH] correct %g handling with unknown arguments in -fprintf-return-value (PR 78696)
@ 2016-12-09 18:08 Martin Sebor
2016-12-12 21:13 ` Jeff Law
0 siblings, 1 reply; 4+ messages in thread
From: Martin Sebor @ 2016-12-09 18:08 UTC (permalink / raw)
To: Gcc Patch List
[-- Attachment #1: Type: text/plain, Size: 705 bytes --]
Bug 78696 points out a bug in the handling of the %g directive in
the gimple-ssa-sprintf pass where precision wasn't being correctly
handled. In the process of developing and testing a fix for it
I noticed a few other subtle problems in the floating point handling
of unknown values. The attached patch corrects them and adds test
cases to better exercise this area.
In the same bug, Jakub also pointed out that the -fprintf-retrun-value
optimization isn't correct in locales where the floating point radix
character (normally the decimal point, '.') is a multibyte character
longer than 1 byte. Since that's unrelated to this report I raised
bug 78703 and will fix it in a subsequent patch.
Martin
[-- Attachment #2: gcc-78696.diff --]
[-- Type: text/x-patch, Size: 35066 bytes --]
PR tree-optimization/78696 - [7 Regression] -fprintf-return-value misoptimizes %.Ng where N is greater than 10
gcc/ChangeLog:
PR tree-optimization/78696
* gimple-ssa-sprintf.c (format_floating): Correct handling of
precision. Use MPFR for %f for greater fidelity. Correct handling
of %g.
(pass_sprintf_length::compute_format_length): Set width and precision
specified by asrerisk to void_node for vararg functions.
(try_substitute_return_value): Adjust dump output.
gcc/testsuite/ChangeLog:
PR tree-optimization/78696
* gcc.dg/tree-ssa/builtin-sprintf-5.c: Add test cases.
* gcc.dg/tree-ssa/builtin-sprintf-warn-7.c: New test.
* gcc.dg/tree-ssa/builtin-sprintf.c (checkv): Add test cases.
diff --git a/gcc/gimple-ssa-sprintf.c b/gcc/gimple-ssa-sprintf.c
index 8de9a1e..3d34342 100644
--- a/gcc/gimple-ssa-sprintf.c
+++ b/gcc/gimple-ssa-sprintf.c
@@ -474,9 +474,11 @@ struct conversion_spec
/* Numeric precision as in "%.32s". */
int precision;
- /* Width specified via the '*' character. */
+ /* Width specified via the '*' character. Need not be INTEGER_CST.
+ For vararg functions set to void_node. */
tree star_width;
- /* Precision specified via the asterisk. */
+ /* Precision specified via the asterisk. Need not be INTEGER_CST.
+ For vararg functions set to void_node. */
tree star_precision;
/* Length modifier. */
@@ -1135,10 +1137,11 @@ format_integer (const conversion_spec &spec, tree arg)
}
/* Return the number of bytes to format using the format specifier
- SPEC the largest value in the real floating TYPE. */
+ SPEC and the precision PREC the largest value in the real floating
+ TYPE. */
static int
-format_floating_max (tree type, char spec, int prec = -1)
+format_floating_max (tree type, char spec, int prec)
{
machine_mode mode = TYPE_MODE (type);
@@ -1189,7 +1192,6 @@ static fmtresult
format_floating (const conversion_spec &spec, int width, int prec)
{
tree type;
- bool ldbl = false;
switch (spec.modifier)
{
@@ -1200,12 +1202,10 @@ format_floating (const conversion_spec &spec, int width, int prec)
case FMT_LEN_L:
type = long_double_type_node;
- ldbl = true;
break;
case FMT_LEN_ll:
type = long_double_type_node;
- ldbl = true;
break;
default:
@@ -1215,19 +1215,20 @@ format_floating (const conversion_spec &spec, int width, int prec)
/* The minimum and maximum number of bytes produced by the directive. */
fmtresult res;
- /* Log10 of of the maximum number of exponent digits for the type. */
- int logexpdigs = 2;
+ /* The result is always bounded (though the range may be all of int). */
+ res.bounded = true;
- if (REAL_MODE_FORMAT (TYPE_MODE (type))->b == 2)
- {
- /* The base in which the exponent is represented should always
- be 2 in GCC. */
-
- const double log10_2 = .30102999566398119521;
+ /* The minimum output as determined by flags. It's always at least 1. */
+ int flagmin = (1 /* for the first digit */
+ + (spec.get_flag ('+') | spec.get_flag (' '))
+ + (prec == 0 && spec.get_flag ('#')));
- /* Compute T_MAX_EXP for base 2. */
- int expdigs = REAL_MODE_FORMAT (TYPE_MODE (type))->emax * log10_2;
- logexpdigs = ilog (expdigs, 10);
+ if (width == INT_MIN || prec == INT_MIN)
+ {
+ /* When either width or precision is specified but unknown
+ the upper bound is the maximum. Otherwise it will be
+ computed for each directive below. */
+ res.range.max = target_int_max ();
}
switch (spec.specifier)
@@ -1235,75 +1236,73 @@ format_floating (const conversion_spec &spec, int width, int prec)
case 'A':
case 'a':
{
- /* The minimum output is "0x.p+0". */
- res.range.min = 6 + (prec > 0 ? prec : 0);
- res.range.max = (width == INT_MIN
- ? HOST_WIDE_INT_MAX
- : format_floating_max (type, 'a', prec));
-
- /* The output of "%a" is fully specified only when precision
- is explicitly specified and width isn't unknown. */
- res.bounded = INT_MIN != width && -1 < prec;
+ /* The minimum output is "0xp+0". */
+ res.range.min = flagmin + 5 + (prec > 0 ? prec + 1 : 0);
+ if (res.range.max == HOST_WIDE_INT_MAX)
+ {
+ /* Compute the upper bound for -TYPE_MAX. */
+ res.range.max = format_floating_max (type, 'a', prec);
+ }
+
break;
}
case 'E':
case 'e':
{
- bool sign = spec.get_flag ('+') || spec.get_flag (' ');
/* The minimum output is "[-+]1.234567e+00" regardless
of the value of the actual argument. */
- res.range.min = (sign
- + 1 /* unit */ + (prec < 0 ? 7 : prec ? prec + 1 : 0)
+ res.range.min = (flagmin
+ + (prec == INT_MIN
+ ? 0 : prec < 0 ? 7 : prec ? prec + 1 : 0)
+ 2 /* e+ */ + 2);
- /* Unless width is uknown the maximum output is the minimum plus
- sign (unless already included), plus the difference between
- the minimum exponent of 2 and the maximum exponent for the type. */
- res.range.max = (width == INT_MIN
- ? HOST_WIDE_INT_M1U
- : res.range.min + !sign + logexpdigs - 2);
-
- /* "%e" is fully specified and the range of bytes is bounded
- unless width is unknown. */
- res.bounded = INT_MIN != width;
+
+ if (res.range.max == HOST_WIDE_INT_MAX)
+ {
+ /* MPFR uses a precision of 16 by default for some reason.
+ Set it to the C default of 6. */
+ res.range.max = format_floating_max (type, 'e',
+ -1 == prec ? 6 : prec);
+ }
break;
}
case 'F':
case 'f':
{
- /* The minimum output is "1.234567" regardless of the value
- of the actual argument. */
- res.range.min = 2 + (prec < 0 ? 6 : prec);
-
- /* Compute the maximum just once. */
- const int f_max[] = {
- format_floating_max (double_type_node, 'f', prec),
- format_floating_max (long_double_type_node, 'f', prec)
- };
- res.range.max = width == INT_MIN ? HOST_WIDE_INT_MAX : f_max [ldbl];
-
- /* "%f" is fully specified and the range of bytes is bounded
- unless width is unknown. */
- res.bounded = INT_MIN != width;
+ /* The lower bound when precision isn't specified is 8 bytes
+ ("1.23456" since precision is taken to be 6). When precision
+ is zero, the lower bound is 1 byte (e.g., "1"). Otherwise,
+ when precision is greater than zero, then the lower bound
+ is 2 plus precision (plus flags). */
+ res.range.min = (flagmin
+ + (prec != INT_MIN) /* for decimal point */
+ + (prec == INT_MIN
+ ? 0 : prec < 0 ? 6 : prec ? prec : -1));
+
+ if (res.range.max == HOST_WIDE_INT_MAX)
+ {
+ /* Compute the upper bound for -TYPE_MAX. */
+ res.range.max = format_floating_max (type, 'f', prec);
+ }
break;
}
+
case 'G':
case 'g':
{
- /* The minimum is the same as for '%F'. */
- res.range.min = 2 + (prec < 0 ? 6 : prec);
-
- /* Compute the maximum just once. */
- const int g_max[] = {
- format_floating_max (double_type_node, 'g', prec),
- format_floating_max (long_double_type_node, 'g', prec)
- };
- res.range.max = width == INT_MIN ? HOST_WIDE_INT_MAX : g_max [ldbl];
+ /* The %g output depends on precision and the exponent of
+ the argument. Since the value of the argument isn't known
+ the lower bound on the range of bytes (not counting flags
+ or width) is 1. */
+ res.range.min = flagmin;
- /* "%g" is fully specified and the range of bytes is bounded
- unless width is unknown. */
- res.bounded = INT_MIN != width;
+ if (res.range.max == HOST_WIDE_INT_MAX)
+ {
+ /* Compute the upper bound for -TYPE_MAX which should be
+ the lesser of %e and %f. */
+ res.range.max = format_floating_max (type, 'g', prec);
+ }
break;
}
@@ -1313,6 +1312,7 @@ format_floating (const conversion_spec &spec, int width, int prec)
if (width > 0)
{
+ /* If width has been specified use it to adjust the range. */
if (res.range.min < (unsigned)width)
res.range.min = width;
if (res.range.max < (unsigned)width)
@@ -1360,11 +1360,7 @@ format_floating (const conversion_spec &spec, tree arg)
if (TREE_CODE (spec.star_precision) == INTEGER_CST)
prec = tree_to_shwi (spec.star_precision);
else
- {
- /* FIXME: Handle non-constant precision. */
- res.range.min = res.range.max = HOST_WIDE_INT_M1U;
- return res;
- }
+ prec = INT_MIN;
}
else if (res.constant && TOUPPER (spec.specifier) != 'A')
{
@@ -1375,11 +1371,6 @@ format_floating (const conversion_spec &spec, tree arg)
if (res.constant)
{
- /* Set up an array to easily iterate over. */
- unsigned HOST_WIDE_INT* const minmax[] = {
- &res.range.min, &res.range.max
- };
-
/* Get the real type format desription for the target. */
const REAL_VALUE_TYPE *rvp = TREE_REAL_CST_PTR (arg);
const real_format *rfmt = REAL_MODE_FORMAT (TYPE_MODE (TREE_TYPE (arg)));
@@ -1418,16 +1409,32 @@ format_floating (const conversion_spec &spec, tree arg)
*pfmt++ = spec.specifier;
*pfmt = '\0';
- for (int i = 0; i != sizeof minmax / sizeof *minmax; ++i)
+ {
+ /* Set up an array to easily iterate over below. */
+ unsigned HOST_WIDE_INT* const minmax[] = {
+ &res.range.min, &res.range.max
+ };
+
+ for (int i = 0; i != sizeof minmax / sizeof *minmax; ++i)
+ {
+ /* Use the MPFR rounding specifier to round down in the first
+ iteration and then up. In most but not all cases this will
+ result in the same number of bytes. */
+ *rndspec = "DU"[i];
+
+ /* Format it and store the result in the corresponding
+ member of the result struct. */
+ *minmax[i] = mpfr_snprintf (NULL, 0, fmtstr, mpfrval);
+ }
+ }
+
+ /* Make sure the minimum is less than the maximum (MPFR rounding
+ in the call to mpfr_snprintf can result in the reverse. */
+ if (res.range.max < res.range.min)
{
- /* Use the MPFR rounding specifier to round down in the first
- iteration and then up. In most but not all cases this will
- result in the same number of bytes. */
- *rndspec = "DU"[i];
-
- /* Format it and store the result in the corresponding
- member of the result struct. */
- *minmax[i] = mpfr_snprintf (NULL, 0, fmtstr, mpfrval);
+ unsigned HOST_WIDE_INT tmp = res.range.min;
+ res.range.min = res.range.max;
+ res.range.max = tmp;
}
/* The range of output is known even if the result isn't bounded. */
@@ -2263,10 +2270,10 @@ pass_sprintf_length::compute_format_length (const call_info &info,
{
/* Similarly to the block above, this could be either a POSIX
positional argument or a width, depending on what follows. */
- if (gimple_call_num_args (info.callstmt) <= argno)
- return false;
-
- spec.star_width = gimple_call_arg (info.callstmt, argno++);
+ if (argno < gimple_call_num_args (info.callstmt))
+ spec.star_width = gimple_call_arg (info.callstmt, argno++);
+ else
+ spec.star_width = void_node;
++pf;
}
@@ -2342,7 +2349,10 @@ pass_sprintf_length::compute_format_length (const call_info &info,
}
else if ('*' == *pf)
{
- spec.star_width = gimple_call_arg (info.callstmt, argno++);
+ if (argno < gimple_call_num_args (info.callstmt))
+ spec.star_width = gimple_call_arg (info.callstmt, argno++);
+ else
+ spec.star_width = void_node;
++pf;
}
else if ('\'' == *pf)
@@ -2370,7 +2380,10 @@ pass_sprintf_length::compute_format_length (const call_info &info,
}
else if ('*' == *pf)
{
- spec.star_precision = gimple_call_arg (info.callstmt, argno++);
+ if (argno < gimple_call_num_args (info.callstmt))
+ spec.star_precision = gimple_call_arg (info.callstmt, argno++);
+ else
+ spec.star_precision = void_node;
++pf;
}
else
@@ -2629,11 +2642,11 @@ try_substitute_return_value (gimple_stmt_iterator *gsi,
fprintf (dump_file,
" %s-bounds return value in range [%lu, %lu]%s.\n",
inbounds,
- (unsigned long)res.number_chars_min,
- (unsigned long)res.number_chars_max, ign);
+ (unsigned long)res.number_chars_min - 1,
+ (unsigned long)res.number_chars_max - 1, ign);
else
fprintf (dump_file, " %s-bounds return value %lu%s.\n",
- inbounds, (unsigned long)res.number_chars, ign);
+ inbounds, (unsigned long)res.number_chars - 1, ign);
}
}
}
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-5.c b/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-5.c
index dbb0dd9..bec6e09 100644
--- a/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-5.c
+++ b/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-5.c
@@ -19,6 +19,7 @@
FAIL (__LINE__)(value); \
} while (0)
+/* Verify that EXPECT == snprintf(0, 0, ...). */
#define T(expect, ...) \
do { \
int n = __builtin_snprintf (0, 0, __VA_ARGS__); \
@@ -108,6 +109,7 @@ void test_arg_multiarg (int i, double d)
T (16, "%*i %s", 12, i, "abc");
}
+/* Verify that EXPECT == vsnprintf(0, 0, ...). */
#define TV(expect, fmt, va) \
do { \
int n = __builtin_vsnprintf (0, 0, fmt, va); \
@@ -117,9 +119,7 @@ void test_arg_multiarg (int i, double d)
void test_va_int (__builtin_va_list va)
{
TV ( 2, "%02hhx", va);
- TV ( 2, "%02.*hhx", va);
TV ( 4, "%04hx", va);
- TV ( 4, "%04.*hx", va);
}
void test_va_multiarg (__builtin_va_list va)
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-7.c b/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-7.c
new file mode 100644
index 0000000..c8c6405
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-7.c
@@ -0,0 +1,276 @@
+/* 78696 - -fprintf-return-value misoptimizes %.Ng where N is greater than 10
+ Test to verify the correctness of ranges of output computed for floating
+ point directives.
+ { dg-do compile }
+ { dg-options "-O2 -Wformat -Wformat-length -ftrack-macro-expansion=0" } */
+
+typedef __builtin_va_list va_list;
+
+char dst[1];
+
+extern void sink (int, void*);
+
+/* Macro to test either width or precision specified by the asterisk
+ (but not both). */
+#define T1(fmt, a) sink (__builtin_sprintf (dst + 1, fmt, a, x), dst)
+
+/* Macro to test both width and precision specified by the asterisk. */
+#define T2(fmt, w, p) sink (__builtin_sprintf (dst + 1, fmt, w, p, x), dst)
+
+/* Macro to test vsprintf with both width and precision specified by
+ the asterisk. */
+#define T(fmt) sink (__builtin_vsprintf (dst + 1, fmt, va), dst)
+
+/* Exercise %a. */
+void test_a (int w, int p, double x)
+{
+ T1 ("%.*a", 0); /* { dg-warning "between 6 and 10 bytes" } */
+ T1 ("%.*a", 1); /* { dg-warning "between 8 and 12 bytes" } */
+ T1 ("%.*a", 2); /* { dg-warning "between 9 and 13 bytes" } */
+ T1 ("%.*a", 99); /* { dg-warning "between 106 and 110 bytes" } */
+ T1 ("%.*a", 199); /* { dg-warning "between 206 and 210 bytes" } */
+ T1 ("%.*a", 1099); /* { dg-warning "between 1106 and 1110 bytes" } */
+
+ T1 ("%*.a", 0); /* { dg-warning "between 6 and 10 bytes" } */
+ T1 ("%*.a", 1); /* { dg-warning "between 6 and 10 bytes" } */
+ T1 ("%*.a", 3); /* { dg-warning "between 6 and 10 bytes" } */
+ T1 ("%*.a", 6); /* { dg-warning "between 6 and 10 bytes" } */
+ T1 ("%*.a", 7); /* { dg-warning "between 7 and 10 bytes" } */
+
+ T1 ("%*.a", w); /* { dg-warning "writing 6 or more bytes" } */
+ T1 ("%*.0a", w); /* { dg-warning "writing 6 or more bytes" } */
+ T1 ("%*.1a", w); /* { dg-warning "writing 8 or more bytes" } */
+ T1 ("%*.2a", w); /* { dg-warning "writing 9 or more bytes" } */
+
+ T1 ("%.*a", p); /* { dg-warning "writing 6 or more bytes" } */
+ T1 ("%1.*a", p); /* { dg-warning "writing 6 or more bytes" } */
+ T1 ("%2.*a", p); /* { dg-warning "writing 6 or more bytes" } */
+ T1 ("%3.*a", p); /* { dg-warning "writing 6 or more bytes" } */
+
+ T2 ("%*.*a", w, p); /* { dg-warning "writing 6 or more bytes" } */
+ T2 ("%*.*a", w, p); /* { dg-warning "writing 6 or more bytes" } */
+ T2 ("%*.*a", w, p); /* { dg-warning "writing 6 or more bytes" } */
+}
+
+/* Exercise %e. */
+void test_e (int w, int p, double x)
+{
+ T1 ("%.*e", 0); /* { dg-warning "between 5 and 7 bytes" } */
+ T1 ("%.*e", 1); /* { dg-warning "between 7 and 9 bytes" } */
+ T1 ("%.*e", 2); /* { dg-warning "between 8 and 10 bytes" } */
+ T1 ("%.*e", 99); /* { dg-warning "between 105 and 107 bytes" } */
+ T1 ("%.*e", 199); /* { dg-warning "between 205 and 207 bytes" } */
+ T1 ("%.*e", 1099); /* { dg-warning "between 1105 and 1107 bytes" } */
+
+ T1 ("%*.e", 0); /* { dg-warning "between 5 and 7 bytes" } */
+ T1 ("%*.e", 1); /* { dg-warning "between 5 and 7 bytes" } */
+ T1 ("%*.e", 1); /* { dg-warning "between 5 and 7 bytes" } */
+ T1 ("%*.e", 3); /* { dg-warning "between 5 and 7 bytes" } */
+ T1 ("%*.e", 6); /* { dg-warning "between 6 and 7 bytes" } */
+ T1 ("%*.e", 7); /* { dg-warning "writing 7 bytes" } */
+
+ T1 ("%*.e", w); /* { dg-warning "writing 5 or more bytes" } */
+ T1 ("%*.0e", w); /* { dg-warning "writing 5 or more bytes" } */
+ T1 ("%*.1e", w); /* { dg-warning "writing 7 or more bytes" } */
+ T1 ("%*.2e", w); /* { dg-warning "writing 8 or more bytes" } */
+
+ T1 ("%.*e", p); /* { dg-warning "writing 5 or more bytes" } */
+ T1 ("%1.*e", p); /* { dg-warning "writing 5 or more bytes" } */
+ T1 ("%2.*e", p); /* { dg-warning "writing 5 or more bytes" } */
+ T1 ("%3.*e", p); /* { dg-warning "writing 5 or more bytes" } */
+
+ T2 ("%*.*e", w, p); /* { dg-warning "writing 5 or more bytes" } */
+ T2 ("%*.*e", w, p); /* { dg-warning "writing 5 or more bytes" } */
+ T2 ("%*.*e", w, p); /* { dg-warning "writing 5 or more bytes" } */
+}
+
+/* Exercise %f. */
+void test_f (int w, int p, double x)
+{
+ T1 ("%.*f", 0); /* { dg-warning "between 1 and 310 bytes" } */
+ T1 ("%.*f", 1); /* { dg-warning "between 3 and 312 bytes" } */
+ T1 ("%.*f", 2); /* { dg-warning "between 4 and 313 bytes" } */
+ T1 ("%.*f", 99); /* { dg-warning "between 101 and 410 bytes" } */
+ T1 ("%.*f", 199); /* { dg-warning "between 201 and 510 bytes" } */
+ T1 ("%.*f", 1099); /* { dg-warning "between 1101 and 1410 bytes" } */
+
+ T2 ("%*.*f", 0, 0); /* { dg-warning "between 1 and 310 bytes" } */
+ T2 ("%*.*f", 1, 0); /* { dg-warning "between 1 and 310 bytes" } */
+ T2 ("%*.*f", 2, 0); /* { dg-warning "between 2 and 310 bytes" } */
+ T2 ("%*.*f", 3, 0); /* { dg-warning "between 3 and 310 bytes" } */
+ T2 ("%*.*f", 310, 0); /* { dg-warning "writing 310 bytes" } */
+ T2 ("%*.*f", 311, 0); /* { dg-warning "writing 311 bytes" } */
+ T2 ("%*.*f", 312, 312); /* { dg-warning "between 314 and 623 bytes" } */
+ T2 ("%*.*f", 312, 313); /* { dg-warning "between 315 and 624 bytes" } */
+
+ T1 ("%*.f", w); /* { dg-warning "writing 1 or more bytes" } */
+ T1 ("%*.0f", w); /* { dg-warning "writing 1 or more bytes" } */
+ T1 ("%*.1f", w); /* { dg-warning "writing 3 or more bytes" } */
+ T1 ("%*.2f", w); /* { dg-warning "writing 4 or more bytes" } */
+
+ T1 ("%.*f", p); /* { dg-warning "writing 1 or more bytes" } */
+ T1 ("%1.*f", p); /* { dg-warning "writing 1 or more bytes" } */
+ T1 ("%2.*f", p); /* { dg-warning "writing 2 or more bytes" } */
+ T1 ("%3.*f", p); /* { dg-warning "writing 3 or more bytes" } */
+
+ T2 ("%*.*f", w, p); /* { dg-warning "writing 1 or more bytes" } */
+ T2 ("%*.*f", w, p); /* { dg-warning "writing 1 or more bytes" } */
+ T2 ("%*.*f", w, p); /* { dg-warning "writing 1 or more bytes" } */
+}
+
+/* Exercise %g. The expected output is the lesser of %e and %f. */
+void test_g (double x)
+{
+ T1 ("%.*g", 0); /* { dg-warning "between 1 and 7 bytes" } */
+ T1 ("%.*g", 1); /* { dg-warning "between 1 and 7 bytes" } */
+ T1 ("%.*g", 2); /* { dg-warning "between 1 and 9 bytes" } */
+ T1 ("%.*g", 99); /* { dg-warning "between 1 and 106 bytes" } */
+ T1 ("%.*g", 199); /* { dg-warning "between 1 and 206 bytes" } */
+ T1 ("%.*g", 1099); /* { dg-warning "between 1 and 310 bytes" } */
+
+ T2 ("%*.*g", 0, 0); /* { dg-warning "between 1 and 7 bytes" } */
+ T2 ("%*.*g", 1, 0); /* { dg-warning "between 1 and 7 bytes" } */
+ T2 ("%*.*g", 2, 0); /* { dg-warning "between 2 and 7 bytes" } */
+ T2 ("%*.*g", 3, 0); /* { dg-warning "between 3 and 7 bytes" } */
+ T2 ("%*.*g", 7, 0); /* { dg-warning "writing 7 bytes" } */
+ T2 ("%*.*g", 310, 0); /* { dg-warning "writing 310 bytes" } */
+ T2 ("%*.*g", 311, 0); /* { dg-warning "writing 311 bytes" } */
+ T2 ("%*.*g", 312, 312); /* { dg-warning "writing 312 bytes" } */
+ T2 ("%*.*g", 312, 313); /* { dg-warning "writing 312 bytes" } */
+ T2 ("%*.*g", 333, 999); /* { dg-warning "writing 333 bytes" } */
+}
+
+/* Exercise %a. */
+void test_a_va (va_list va)
+{
+ T ("%.0a"); /* { dg-warning "between 6 and 10 bytes" } */
+ T ("%.1a"); /* { dg-warning "between 8 and 12 bytes" } */
+ T ("%.2a"); /* { dg-warning "between 9 and 13 bytes" } */
+ T ("%.99a"); /* { dg-warning "between 106 and 110 bytes" } */
+ T ("%.199a"); /* { dg-warning "between 206 and 210 bytes" } */
+ T ("%.1099a"); /* { dg-warning "between 1106 and 1110 bytes" } */
+
+ T ("%0.a"); /* { dg-warning "between 6 and 10 bytes" } */
+ T ("%1.a"); /* { dg-warning "between 6 and 10 bytes" } */
+ T ("%3.a"); /* { dg-warning "between 6 and 10 bytes" } */
+ T ("%6.a"); /* { dg-warning "between 6 and 10 bytes" } */
+ T ("%7.a"); /* { dg-warning "between 7 and 10 bytes" } */
+
+ T ("%*.a"); /* { dg-warning "writing 6 or more bytes" } */
+ T ("%*.0a"); /* { dg-warning "writing 6 or more bytes" } */
+ T ("%*.1a"); /* { dg-warning "writing 8 or more bytes" } */
+ T ("%*.2a"); /* { dg-warning "writing 9 or more bytes" } */
+
+ T ("%.*a"); /* { dg-warning "writing 6 or more bytes" } */
+ T ("%1.*a"); /* { dg-warning "writing 6 or more bytes" } */
+ T ("%2.*a"); /* { dg-warning "writing 6 or more bytes" } */
+ T ("%6.*a"); /* { dg-warning "writing 6 or more bytes" } */
+ T ("%9.*a"); /* { dg-warning "writing 9 or more bytes" } */
+
+ T ("%*.*a"); /* { dg-warning "writing 6 or more bytes" } */
+}
+
+/* Exercise %e. */
+void test_e_va (va_list va)
+{
+ T ("%e"); /* { dg-warning "between 12 and 14 bytes" } */
+ T ("%+e"); /* { dg-warning "between 13 and 14 bytes" } */
+ T ("% e"); /* { dg-warning "between 13 and 14 bytes" } */
+ T ("%#e"); /* { dg-warning "between 12 and 14 bytes" } */
+ T ("%#+e"); /* { dg-warning "between 13 and 14 bytes" } */
+ T ("%# e"); /* { dg-warning "between 13 and 14 bytes" } */
+
+ T ("%.e"); /* { dg-warning "between 5 and 7 bytes" } */
+ T ("%.0e"); /* { dg-warning "between 5 and 7 bytes" } */
+ T ("%.1e"); /* { dg-warning "between 7 and 9 bytes" } */
+ T ("%.2e"); /* { dg-warning "between 8 and 10 bytes" } */
+ T ("%.99e"); /* { dg-warning "between 105 and 107 bytes" } */
+ T ("%.199e"); /* { dg-warning "between 205 and 207 bytes" } */
+ T ("%.1099e"); /* { dg-warning "between 1105 and 1107 bytes" } */
+
+ T ("%0.e"); /* { dg-warning "between 5 and 7 bytes" } */
+ T ("%1.e"); /* { dg-warning "between 5 and 7 bytes" } */
+ T ("%1.e"); /* { dg-warning "between 5 and 7 bytes" } */
+ T ("%3.e"); /* { dg-warning "between 5 and 7 bytes" } */
+ T ("%6.e"); /* { dg-warning "between 6 and 7 bytes" } */
+ T ("%7.e"); /* { dg-warning "writing 7 bytes" } */
+
+ T ("%.*e"); /* { dg-warning "writing 5 or more bytes" } */
+ T ("%1.*e"); /* { dg-warning "writing 5 or more bytes" } */
+ T ("%6.*e"); /* { dg-warning "writing 6 or more bytes" } */
+ T ("%9.*e"); /* { dg-warning "writing 9 or more bytes" } */
+
+ T ("%*.*e"); /* { dg-warning "writing 5 or more bytes" } */
+}
+
+/* Exercise %f. */
+void test_f_va (va_list va)
+{
+ T ("%f"); /* { dg-warning "between 8 and 317 bytes" } */
+ T ("%+f"); /* { dg-warning "between 9 and 317 bytes" } */
+ T ("% f"); /* { dg-warning "between 9 and 317 bytes" } */
+ T ("%#f"); /* { dg-warning "between 8 and 317 bytes" } */
+ T ("%+f"); /* { dg-warning "between 9 and 317 bytes" } */
+ T ("% f"); /* { dg-warning "between 9 and 317 bytes" } */
+ T ("%#+f"); /* { dg-warning "between 9 and 317 bytes" } */
+ T ("%# f"); /* { dg-warning "between 9 and 317 bytes" } */
+
+ T ("%.f"); /* { dg-warning "between 1 and 310 bytes" } */
+ T ("%.0f"); /* { dg-warning "between 1 and 310 bytes" } */
+ T ("%.1f"); /* { dg-warning "between 3 and 312 bytes" } */
+ T ("%.2f"); /* { dg-warning "between 4 and 313 bytes" } */
+ T ("%.99f"); /* { dg-warning "between 101 and 410 bytes" } */
+ T ("%.199f"); /* { dg-warning "between 201 and 510 bytes" } */
+ T ("%.1099f"); /* { dg-warning "between 1101 and 1410 bytes" } */
+
+ T ("%0.0f"); /* { dg-warning "between 1 and 310 bytes" } */
+ T ("%1.0f"); /* { dg-warning "between 1 and 310 bytes" } */
+ T ("%2.0f"); /* { dg-warning "between 2 and 310 bytes" } */
+ T ("%3.0f"); /* { dg-warning "between 3 and 310 bytes" } */
+ T ("%310.0f"); /* { dg-warning "writing 310 bytes" } */
+ T ("%311.0f"); /* { dg-warning "writing 311 bytes" } */
+ T ("%312.312f"); /* { dg-warning "between 314 and 623 bytes" } */
+ T ("%312.313f"); /* { dg-warning "between 315 and 624 bytes" } */
+
+ T ("%.*f"); /* { dg-warning "writing 1 or more bytes" } */
+ T ("%1.*f"); /* { dg-warning "writing 1 or more bytes" } */
+ T ("%3.*f"); /* { dg-warning "writing 3 or more bytes" } */
+
+ T ("%*.*f"); /* { dg-warning "writing 1 or more bytes" } */
+}
+
+/* Exercise %g. The expected output is the lesser of %e and %f. */
+void test_g_va (va_list va)
+{
+ T ("%g"); /* { dg-warning "between 1 and 13 bytes" } */
+ T ("%+g"); /* { dg-warning "between 2 and 13 bytes" } */
+ T ("% g"); /* { dg-warning "between 2 and 13 bytes" } */
+ T ("%#g"); /* { dg-warning "between 1 and 13 bytes" } */
+ T ("%#+g"); /* { dg-warning "between 2 and 13 bytes" } */
+ T ("%# g"); /* { dg-warning "between 2 and 13 bytes" } */
+
+ T ("%.g"); /* { dg-warning "between 1 and 7 bytes" } */
+ T ("%.0g"); /* { dg-warning "between 1 and 7 bytes" } */
+ T ("%.1g"); /* { dg-warning "between 1 and 7 bytes" } */
+ T ("%.2g"); /* { dg-warning "between 1 and 9 bytes" } */
+ T ("%.99g"); /* { dg-warning "between 1 and 106 bytes" } */
+ T ("%.199g"); /* { dg-warning "between 1 and 206 bytes" } */
+ T ("%.1099g"); /* { dg-warning "between 1 and 310 bytes" } */
+
+ T ("%0.0g"); /* { dg-warning "between 1 and 7 bytes" } */
+ T ("%1.0g"); /* { dg-warning "between 1 and 7 bytes" } */
+ T ("%2.0g"); /* { dg-warning "between 2 and 7 bytes" } */
+ T ("%3.0g"); /* { dg-warning "between 3 and 7 bytes" } */
+ T ("%7.0g"); /* { dg-warning "writing 7 bytes" } */
+ T ("%310.0g"); /* { dg-warning "writing 310 bytes" } */
+ T ("%311.0g"); /* { dg-warning "writing 311 bytes" } */
+ T ("%312.312g"); /* { dg-warning "writing 312 bytes" } */
+ T ("%312.313g"); /* { dg-warning "writing 312 bytes" } */
+ T ("%333.999g"); /* { dg-warning "writing 333 bytes" } */
+
+ T ("%.*g"); /* { dg-warning "writing 1 or more bytes" } */
+ T ("%1.*g"); /* { dg-warning "writing 1 or more bytes" } */
+ T ("%4.*g"); /* { dg-warning "writing 4 or more bytes" } */
+
+ T ("%*.*g"); /* { dg-warning "writing 1 or more bytes" } */
+}
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf.c b/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf.c
index 916df79..b3d6128 100644
--- a/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf.c
+++ b/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf.c
@@ -56,9 +56,12 @@ checkv (const char *func, int line, int res, int min, int max,
fail = 1;
}
- else
+ else if (min == max)
__builtin_printf ("PASS: %s:%i: \"%s\" result %i: \"%s\"\n",
func, line, fmt, n, dst);
+ else
+ __builtin_printf ("PASS: %s:%i: \"%s\" result %i in [%i, %i]: \"%s\"\n",
+ func, line, fmt, n, min, max, dst);
}
if (fail)
@@ -75,7 +78,7 @@ check (const char *func, int line, int res, int min, int max,
__builtin_va_end (va);
}
-char buffer[256];
+char buffer[4100];
char* volatile dst = buffer;
char* ptr = buffer;
@@ -368,6 +371,8 @@ test_a_double (double d)
EQL ( 9, 10, "%.2a", 4.0); /* 0x8.00p-1 */
EQL (10, 11, "%.3a", 5.0); /* 0xa.000p-1 */
+ EQL (11, 12, "%.*a", 4, 6.0); /* 0xc.0000p-1 */
+ EQL (12, 13, "%.*a", 5, 7.0); /* 0xe.00000p-1 */
/* d is in [ 0, -DBL_MAX ] */
RNG ( 6, 10, 11, "%.0a", d); /* 0x0p+0 ... -0x2p+1023 */
RNG ( 6, 12, 13, "%.1a", d); /* 0x0p+0 ... -0x2.0p+1023 */
@@ -385,7 +390,7 @@ test_a_long_double (void)
}
static void __attribute__ ((noinline, noclone))
-test_e_double (void)
+test_e_double (double d)
{
EQL (12, 13, "%e", 1.0e0);
EQL (13, 14, "%e", -1.0e0);
@@ -407,10 +412,34 @@ test_e_double (void)
EQL (12, 13, "%e", 1.0e-1);
EQL (12, 13, "%e", 1.0e-12);
EQL (13, 14, "%e", 1.0e-123);
+
+ RNG (12, 14, 15, "%e", d);
+ RNG ( 5, 7, 8, "%.e", d);
+ RNG ( 5, 7, 8, "%.0e", d);
+ RNG ( 7, 9, 10, "%.1e", d);
+ RNG ( 8, 10, 11, "%.2e", d);
+ RNG ( 9, 11, 12, "%.3e", d);
+ RNG (10, 12, 13, "%.4e", d);
+ RNG (11, 13, 14, "%.5e", d);
+ RNG (12, 14, 15, "%.6e", d);
+ RNG (13, 15, 16, "%.7e", d);
+
+ RNG (4006, 4008, 4009, "%.4000e", d);
+
+ RNG ( 5, 7, 8, "%.*e", 0, d);
+ RNG ( 7, 9, 10, "%.*e", 1, d);
+ RNG ( 8, 10, 11, "%.*e", 2, d);
+ RNG ( 9, 11, 12, "%.*e", 3, d);
+ RNG (10, 12, 13, "%.*e", 4, d);
+ RNG (11, 13, 14, "%.*e", 5, d);
+ RNG (12, 14, 15, "%.*e", 6, d);
+ RNG (13, 15, 16, "%.*e", 7, d);
+
+ RNG (4006, 4008, 4009, "%.*e", 4000, d);
}
static void __attribute__ ((noinline, noclone))
-test_e_long_double (void)
+test_e_long_double (long double d)
{
EQL (12, 13, "%Le", 1.0e0L);
EQL (13, 14, "%Le", -1.0e0L);
@@ -443,10 +472,32 @@ test_e_long_double (void)
EQL ( 8, 9, "%.1Le", 1.0e-111L);
EQL (19, 20, "%.12Le", 1.0e-112L);
EQL (20, 21, "%.13Le", 1.0e-113L);
+
+ /* The following correspond to the double results plus 1 for the upper
+ bound accounting for the four-digit exponent. */
+ RNG (12, 15, 16, "%Le", d); /* 0.000000e+00 ... -1.189732e+4932 */
+ RNG ( 5, 8, 9, "%.Le", d);
+ RNG ( 5, 9, 10, "%.0Le", d);
+ RNG ( 7, 10, 11, "%.1Le", d); /* 0.0e+00 ... -1.2e+4932 */
+ RNG ( 8, 11, 12, "%.2Le", d); /* 0.00e+00 ... -1.19e+4932 */
+ RNG ( 9, 12, 13, "%.3Le", d);
+ RNG (10, 13, 14, "%.4Le", d);
+ RNG (11, 14, 15, "%.5Le", d);
+ RNG (12, 15, 16, "%.6Le", d); /* same as plain "%Le" */
+ RNG (13, 16, 17, "%.7Le", d); /* 0.0000000e+00 ... -1.1897315e+4932 */
+
+ RNG ( 5, 9, 10, "%.*Le", 0, d);
+ RNG ( 7, 10, 11, "%.*Le", 1, d);
+ RNG ( 8, 11, 12, "%.*Le", 2, d);
+ RNG ( 9, 12, 13, "%.*Le", 3, d);
+ RNG (10, 13, 14, "%.*Le", 4, d);
+ RNG (11, 14, 15, "%.*Le", 5, d);
+ RNG (12, 15, 16, "%.*Le", 6, d);
+ RNG (13, 16, 17, "%.*Le", 7, d);
}
static void __attribute__ ((noinline, noclone))
-test_f_double (void)
+test_f_double (double d)
{
EQL ( 8, 9, "%f", 0.0e0);
EQL ( 8, 9, "%f", 0.1e0);
@@ -464,6 +515,8 @@ test_f_double (void)
EQL ( 8, 9, "%f", 1.0e-1);
EQL ( 8, 9, "%f", 1.0e-12);
EQL ( 8, 9, "%f", 1.0e-123);
+
+ RNG ( 8, 317, 318, "%f", d);
}
static void __attribute__ ((noinline, noclone))
@@ -488,6 +541,87 @@ test_f_long_double (void)
}
static void __attribute__ ((noinline, noclone))
+test_g_double (double d)
+{
+ /* Numbers exactly representable in binary floating point. */
+ EQL ( 1, 2, "%g", 0.0);
+ EQL ( 3, 4, "%g", 1.0 / 2);
+ EQL ( 4, 5, "%g", 1.0 / 4);
+ EQL ( 5, 6, "%g", 1.0 / 8);
+ EQL ( 6, 7, "%g", 1.0 / 16);
+ EQL ( 7, 8, "%g", 1.0 / 32);
+ EQL ( 8, 9, "%g", 1.0 / 64);
+ EQL ( 9, 10, "%g", 1.0 / 128);
+ EQL ( 10, 11, "%g", 1.0 / 256);
+ EQL ( 10, 11, "%g", 1.0 / 512);
+
+ /* Numbers that are not exactly representable. */
+ RNG ( 3, 8, 9, "%g", 0.1);
+ RNG ( 4, 8, 9, "%g", 0.12);
+ RNG ( 5, 8, 9, "%g", 0.123);
+ RNG ( 6, 8, 9, "%g", 0.1234);
+ RNG ( 7, 8, 9, "%g", 0.12345);
+ RNG ( 8, 8, 9, "%g", 0.123456);
+
+ RNG ( 4, 7, 8, "%g", 0.123e+1);
+ EQL ( 8, 9, "%g", 0.123e+12);
+ RNG ( 9, 12, 13, "%g", 0.123e+134);
+
+ RNG ( 1, 13, 14, "%g", d);
+ RNG ( 1, 7, 8, "%.g", d);
+ RNG ( 1, 7, 8, "%.0g", d);
+ RNG ( 1, 7, 8, "%.1g", d);
+ RNG ( 1, 9, 10, "%.2g", d);
+ RNG ( 1, 10, 11, "%.3g", d);
+ RNG ( 1, 11, 12, "%.4g", d);
+ RNG ( 1, 12, 13, "%.5g", d);
+ RNG ( 1, 13, 14, "%.6g", d);
+ RNG ( 1, 14, 15, "%.7g", d);
+ RNG ( 1, 15, 16, "%.8g", d);
+
+ RNG ( 1,310,311, "%.9999g", d);
+
+ RNG ( 1, 7, 8, "%.*g", 0, d);
+ RNG ( 1, 7, 8, "%.*g", 1, d);
+ RNG ( 1, 9, 10, "%.*g", 2, d);
+ RNG ( 1, 10, 11, "%.*g", 3, d);
+ RNG ( 1, 11, 12, "%.*g", 4, d);
+ RNG ( 1, 12, 13, "%.*g", 5, d);
+ RNG ( 1, 13, 14, "%.*g", 6, d);
+ RNG ( 1, 14, 15, "%.*g", 7, d);
+ RNG ( 1, 15, 16, "%.*g", 8, d);
+ RNG ( 1,310,311, "%.*g", 9999, d);
+}
+
+static void __attribute__ ((noinline, noclone))
+test_g_long_double (void)
+{
+ /* Numbers exactly representable in binary floating point. */
+ EQL ( 1, 2, "%Lg", 0.0L);
+ EQL ( 3, 4, "%Lg", 1.0L / 2);
+ EQL ( 4, 5, "%Lg", 1.0L / 4);
+ EQL ( 5, 6, "%Lg", 1.0L / 8);
+ EQL ( 6, 7, "%Lg", 1.0L / 16);
+ EQL ( 7, 8, "%Lg", 1.0L / 32);
+ EQL ( 8, 9, "%Lg", 1.0L / 64);
+ EQL ( 9, 10, "%Lg", 1.0L / 128);
+ EQL ( 10, 11, "%Lg", 1.0L / 256);
+ EQL ( 10, 11, "%Lg", 1.0L / 512);
+
+ /* Numbers that are not exactly representable. */
+ RNG ( 3, 8, 9, "%Lg", 0.1L);
+ RNG ( 4, 8, 9, "%Lg", 0.12L);
+ RNG ( 5, 8, 9, "%Lg", 0.123L);
+ RNG ( 6, 8, 9, "%Lg", 0.1234L);
+ RNG ( 7, 8, 9, "%Lg", 0.12345L);
+ RNG ( 8, 8, 9, "%Lg", 0.123456L);
+
+ RNG ( 4, 7, 8, "%Lg", 0.123e+1L);
+ EQL ( 8, 9, "%Lg", 0.123e+12L);
+ RNG ( 9, 12, 13, "%Lg", 0.123e+134L);
+}
+
+static void __attribute__ ((noinline, noclone))
test_s (int i)
{
EQL ( 0, 1, "%s", "");
@@ -530,12 +664,14 @@ int main (void)
test_x ('?', 0xdead, 0xdeadbeef);
test_a_double (0.0);
- test_e_double ();
- test_f_double ();
+ test_e_double (0.0);
+ test_f_double (0.0);
+ test_g_double (0.0);
test_a_long_double ();
- test_e_long_double ();
+ test_e_long_double (0.0);
test_f_long_double ();
+ test_g_long_double ();
test_s (0);
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] correct %g handling with unknown arguments in -fprintf-return-value (PR 78696)
2016-12-09 18:08 [PATCH] correct %g handling with unknown arguments in -fprintf-return-value (PR 78696) Martin Sebor
@ 2016-12-12 21:13 ` Jeff Law
2016-12-13 0:06 ` Martin Sebor
0 siblings, 1 reply; 4+ messages in thread
From: Jeff Law @ 2016-12-12 21:13 UTC (permalink / raw)
To: Martin Sebor, Gcc Patch List
On 12/09/2016 11:07 AM, Martin Sebor wrote:
> Bug 78696 points out a bug in the handling of the %g directive in
> the gimple-ssa-sprintf pass where precision wasn't being correctly
> handled. In the process of developing and testing a fix for it
> I noticed a few other subtle problems in the floating point handling
> of unknown values. The attached patch corrects them and adds test
> cases to better exercise this area.
>
> In the same bug, Jakub also pointed out that the -fprintf-retrun-value
> optimization isn't correct in locales where the floating point radix
> character (normally the decimal point, '.') is a multibyte character
> longer than 1 byte. Since that's unrelated to this report I raised
> bug 78703 and will fix it in a subsequent patch.
Definitely address in a subsequent patch.
>
> Martin
>
>
> gcc-78696.diff
>
>
> PR tree-optimization/78696 - [7 Regression] -fprintf-return-value misoptimizes %.Ng where N is greater than 10
>
> gcc/ChangeLog:
>
> PR tree-optimization/78696
> * gimple-ssa-sprintf.c (format_floating): Correct handling of
> precision. Use MPFR for %f for greater fidelity. Correct handling
> of %g.
> (pass_sprintf_length::compute_format_length): Set width and precision
> specified by asrerisk to void_node for vararg functions.
> (try_substitute_return_value): Adjust dump output.
>
> gcc/testsuite/ChangeLog:
>
> PR tree-optimization/78696
> * gcc.dg/tree-ssa/builtin-sprintf-5.c: Add test cases.
> * gcc.dg/tree-ssa/builtin-sprintf-warn-7.c: New test.
> * gcc.dg/tree-ssa/builtin-sprintf.c (checkv): Add test cases.
[ snip ]
> + /* The lower bound when precision isn't specified is 8 bytes
> + ("1.23456" since precision is taken to be 6). When precision
> + is zero, the lower bound is 1 byte (e.g., "1"). Otherwise,
> + when precision is greater than zero, then the lower bound
> + is 2 plus precision (plus flags). */
> + res.range.min = (flagmin
> + + (prec != INT_MIN) /* for decimal point */
> + + (prec == INT_MIN
> + ? 0 : prec < 0 ? 6 : prec ? prec : -1));
Note for the future, nest/chained ternary operators can sometimes just
be hard to visually parse when they're squashed on a single line.
Formatting like this has often been used in the past to help clarify the
intent:
(flagmin
+ (prec != INT_MIN)
+ (prec == INT_MIN ? 0
: prec < 0 ? 6
: prec ? prec
: -1)
If we ignore the flagmin component, I get the following evaluations for
PREC.
PREC RESULT
INTMIN 0
0 0
negative (but not INTMIN) 7
positive prec + 1
That doesn't seem in-line with the comment.
Jeff
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] correct %g handling with unknown arguments in -fprintf-return-value (PR 78696)
2016-12-12 21:13 ` Jeff Law
@ 2016-12-13 0:06 ` Martin Sebor
2016-12-19 22:26 ` Jeff Law
0 siblings, 1 reply; 4+ messages in thread
From: Martin Sebor @ 2016-12-13 0:06 UTC (permalink / raw)
To: Jeff Law, Gcc Patch List
>> + /* The lower bound when precision isn't specified is 8 bytes
>> + ("1.23456" since precision is taken to be 6). When precision
>> + is zero, the lower bound is 1 byte (e.g., "1"). Otherwise,
>> + when precision is greater than zero, then the lower bound
>> + is 2 plus precision (plus flags). */
>> + res.range.min = (flagmin
>> + + (prec != INT_MIN) /* for decimal point */
>> + + (prec == INT_MIN
>> + ? 0 : prec < 0 ? 6 : prec ? prec : -1));
> Note for the future, nest/chained ternary operators can sometimes just
> be hard to visually parse when they're squashed on a single line.
> Formatting like this has often been used in the past to help clarify the
> intent:
>
> (flagmin
> + (prec != INT_MIN)
> + (prec == INT_MIN ? 0
> : prec < 0 ? 6
> : prec ? prec
> : -1)
Okay.
>
> If we ignore the flagmin component, I get the following evaluations for
> PREC.
>
> PREC RESULT
> INTMIN 0
> 0 0
> negative (but not INTMIN) 7
> positive prec + 1
>
> That doesn't seem in-line with the comment.
Sorry, I think I need a hint. Which part doesn't seem in line with
which part of the comment? The numbers you have look correct to me
and I don't see anything wrong with the comment either.
Flagmin is always at least 1 and so RESULT above is 1 when precision
is used and either zero or unknown (because printf ("%.0f", 0) returns
1), and it's 8 when precision is negative because it's taken as if had
been omitted (i.e., it's 6 and printf ("%f", 0) formats "0.000000" and
returns 8), and it's prec + 2 when precision is positive because the 2
accounts for the leading "0." (when argument is zero) and precision is
the number of fractional digits.
Martin
PS While testing this I came across a bug in MPFR. When precision is
large it allocates huge amounts of memory and appears to get stuck in
some sort of a loop (e.g., with sprintf(d, "%.*g", INT_MAX)). I've
tested the diff below to work around it.
diff --git a/gcc/gimple-ssa-sprintf.c b/gcc/gimple-ssa-sprintf.c
index 3d34342..c39b318 100644
--- a/gcc/gimple-ssa-sprintf.c
+++ b/gcc/gimple-ssa-sprintf.c
@@ -1170,8 +1170,27 @@ format_floating_max (tree type, char spec, int prec)
if (-1 < prec)
{
- const char fmt[] = { '%', '.', '*', 'R', spec, '\0' };
- n = mpfr_snprintf (NULL, 0, fmt, prec, x);
+ /* Prevent MPFR from allocating too much memory. */
+ if (TOUPPER (spec) == 'G')
+ {
+ /* For G/g, precision gives the maximum number of significant
+ digits which is bounded by LDBL_MAX_10_EXP, or, for a 128
+ bit IEEE extended precision, 4932. Using twice as much
+ here should be more than sufficient for any format. */
+ int p = 9864 < prec ? 9864 : prec;
+ const char fmt[] = { '%', '.', '*', 'R', spec, '\0' };
+ n = mpfr_snprintf (NULL, 0, fmt, p, x);
+ }
+ else
+ {
+ /* Cap precision at 1KB and add the difference to the MPFR
+ result. */
+ int p = 1024 < prec ? 1024 : prec;
+ const char fmt[] = { '%', '.', '*', 'R', spec, '\0' };
+ n = mpfr_snprintf (NULL, 0, fmt, p, x);
+ if (p < prec)
+ n += prec - p;
+ }
}
else
{
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] correct %g handling with unknown arguments in -fprintf-return-value (PR 78696)
2016-12-13 0:06 ` Martin Sebor
@ 2016-12-19 22:26 ` Jeff Law
0 siblings, 0 replies; 4+ messages in thread
From: Jeff Law @ 2016-12-19 22:26 UTC (permalink / raw)
To: Martin Sebor, Gcc Patch List
On 12/12/2016 05:06 PM, Martin Sebor wrote:
>>> + /* The lower bound when precision isn't specified is 8 bytes
>>> + ("1.23456" since precision is taken to be 6). When precision
>>> + is zero, the lower bound is 1 byte (e.g., "1"). Otherwise,
>>> + when precision is greater than zero, then the lower bound
>>> + is 2 plus precision (plus flags). */
>>> + res.range.min = (flagmin
>>> + + (prec != INT_MIN) /* for decimal point */
>>> + + (prec == INT_MIN
>>> + ? 0 : prec < 0 ? 6 : prec ? prec : -1));
>> Note for the future, nest/chained ternary operators can sometimes just
>> be hard to visually parse when they're squashed on a single line.
>> Formatting like this has often been used in the past to help clarify the
>> intent:
>>
>> (flagmin
>> + (prec != INT_MIN)
>> + (prec == INT_MIN ? 0
>> : prec < 0 ? 6
>> : prec ? prec
>> : -1)
>
> Okay.
>
>>
>> If we ignore the flagmin component, I get the following evaluations for
>> PREC.
>>
>> PREC RESULT
>> INTMIN 0
>> 0 0
>> negative (but not INTMIN) 7
>> positive prec + 1
>>
>> That doesn't seem in-line with the comment.
>
> Sorry, I think I need a hint. Which part doesn't seem in line with
> which part of the comment? The numbers you have look correct to me
> and I don't see anything wrong with the comment either.
>
> Flagmin is always at least 1 and so RESULT above is 1 when precision
> is used and either zero or unknown (because printf ("%.0f", 0) returns
> 1), and it's 8 when precision is negative because it's taken as if had
> been omitted (i.e., it's 6 and printf ("%f", 0) formats "0.000000" and
> returns 8), and it's prec + 2 when precision is positive because the 2
> accounts for the leading "0." (when argument is zero) and precision is
> the number of fractional digits.
So I think it's another case me mis-parsing the comment and conflating
unknown vs unspecified in my mind I took the "plus flags" as applying
to all cases, but re-reading it with the additional context you've
provided, it seems like it actually applies to just the last case.
Try to give the comment a light respin to see if you can clarify. OK
with that update.
Jeff
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2016-12-19 22:21 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-12-09 18:08 [PATCH] correct %g handling with unknown arguments in -fprintf-return-value (PR 78696) Martin Sebor
2016-12-12 21:13 ` Jeff Law
2016-12-13 0:06 ` Martin Sebor
2016-12-19 22:26 ` Jeff Law
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).