This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] fix ILP32 bootstrap failures due to -Wformat-length
On 09/21/2016 05:09 PM, Jakub Jelinek wrote:
On Wed, Sep 21, 2016 at 04:39:42PM -0600, Martin Sebor wrote:
diff --git a/gcc/gimple-ssa-sprintf.c b/gcc/gimple-ssa-sprintf.c
index dddb026..652d3fb 100644
--- a/gcc/gimple-ssa-sprintf.c
+++ b/gcc/gimple-ssa-sprintf.c
@@ -210,8 +210,8 @@ struct format_result
static HOST_WIDE_INT
target_int_min ()
{
- static const unsigned HOST_WIDE_INT int_min
- = 1LLU << (sizeof int_min * CHAR_BIT
+ const unsigned HOST_WIDE_INT int_min
+ = 1LLU << (HOST_BITS_PER_WIDE_INT
1LLU should be really HOST_WIDE_INT_1U
- TYPE_PRECISION (integer_type_node) + 1);
Is the shift amount really what you mean?
HOST_BITS_PER_WIDE_INT - TYPE_PRECISION (integer_type_node) + 1
is usually 33 (or 17 or 9 in much rarer cases), so that is
0x200000000ULL. Don't you want instead
= HOST_WIDE_INT_1U << (TYPE_PRECISION (ingeger_type_node) - 1);
so that it will be
0x80000000ULL?
Great catch, thank you! That must have been a copy and paste mistake.
@@ -221,8 +221,8 @@ target_int_min ()
static unsigned HOST_WIDE_INT
target_int_max ()
{
- static const unsigned HOST_WIDE_INT int_max
- = HOST_WIDE_INT_M1U >> (sizeof int_max * CHAR_BIT
+ const unsigned HOST_WIDE_INT int_max
+ = HOST_WIDE_INT_M1U >> (HOST_BITS_PER_WIDE_INT
- TYPE_PRECISION (integer_type_node) + 1);
return int_max;
}
This is expectedly -1ULL >> 33, i.e. 0x7fffffffULL, which looks ok.
Thanks for double checking that. There is a test case for this
in the test suite for the optimization but there wasn't one for
the INT_MIN case. I've added it in the attached patch. I'll
beef up the return range testing some more before re-enabling
the optimization.
Martin
PR target/77676 - powerpc64 and powerpc64le stage2 bootstrap fail
gcc/testsuite/ChangeLog:
2016-09-21 Martin Sebor <msebor@redhat.com>
PR target/77676
* gcc.dg/tree-ssa/builtin-sprintf-2.c: Fix typo.
* gcc.dg/tree-ssa/builtin-sprintf-3.c: New test.
* gcc.dg/tree-ssa/builtin-sprintf-warn-5.c: New test.
gcc/ChangeLog:
2016-09-21 Martin Sebor <msebor@redhat.com>
PR target/77676
* gimple-ssa-sprintf.c (target_int_min, target_int_max): Use
HOST_BITS_PER_WIDE_INT, make a static local variable auto.
(target_int_min): Correct computation.
(format_integer): Use long long as the argument for the ll length
modifier.
(format_floating): Use target_int_max().
(get_string_length): Same.
(format_string): Avoid setting the bounded flag for strings
of unknown length.
(try_substitute_return_value): Avoid setting range info when
the result isn't bounded.
* varasm.c (assemble_name): Increase buffer size.
diff --git a/gcc/gimple-ssa-sprintf.c b/gcc/gimple-ssa-sprintf.c
index dddb026..d1d20ca 100644
--- a/gcc/gimple-ssa-sprintf.c
+++ b/gcc/gimple-ssa-sprintf.c
@@ -210,9 +210,9 @@ struct format_result
static HOST_WIDE_INT
target_int_min ()
{
- static const unsigned HOST_WIDE_INT int_min
- = 1LLU << (sizeof int_min * CHAR_BIT
- - TYPE_PRECISION (integer_type_node) + 1);
+ const unsigned HOST_WIDE_INT int_min
+ = HOST_WIDE_INT_M1U << (TYPE_PRECISION (integer_type_node) - 1);
+
return int_min;
}
@@ -221,8 +221,8 @@ target_int_min ()
static unsigned HOST_WIDE_INT
target_int_max ()
{
- static const unsigned HOST_WIDE_INT int_max
- = HOST_WIDE_INT_M1U >> (sizeof int_max * CHAR_BIT
+ const unsigned HOST_WIDE_INT int_max
+ = HOST_WIDE_INT_M1U >> (HOST_BITS_PER_WIDE_INT
- TYPE_PRECISION (integer_type_node) + 1);
return int_max;
}
@@ -851,7 +851,9 @@ format_integer (const conversion_spec &spec, tree arg)
case FMT_LEN_L:
case FMT_LEN_ll:
- dirtype = sign ? long_integer_type_node : long_unsigned_type_node;
+ dirtype = (sign
+ ? long_long_integer_type_node
+ : long_long_unsigned_type_node);
break;
case FMT_LEN_z:
@@ -1366,7 +1368,7 @@ format_floating (const conversion_spec &spec, tree arg)
*minmax[i] = mpfr_snprintf (NULL, 0, fmtstr, mpfrval);
}
- res.bounded = res.range.min < HOST_WIDE_INT_MAX;
+ res.bounded = res.range.min < target_int_max ();
return res;
}
@@ -1420,7 +1422,7 @@ get_string_length (tree str)
/* Set RES.BOUNDED to true if and only if all strings referenced
by STR are known to be bounded (though not necessarily by their
actual length but perhaps by their maximum possible length). */
- res.bounded = res.range.max < HOST_WIDE_INT_MAX;
+ res.bounded = res.range.max < target_int_max ();
/* Set RES.CONSTANT to false even though that may be overly
conservative in rare cases like: 'x ? a : b' where a and
@@ -1471,6 +1473,10 @@ format_string (const conversion_spec &spec, tree arg)
: 2 == warn_format_length ? 0 <= prec ? prec : 1
: HOST_WIDE_INT_MAX);
+ /* The result is bounded unless overriddden for a non-constant string
+ of an unknown length. */
+ bool bounded = true;
+
if (spec.specifier == 'c')
{
if (spec.modifier == FMT_LEN_l)
@@ -1550,16 +1556,17 @@ format_string (const conversion_spec &spec, tree arg)
if (0 <= prec)
{
if ((unsigned)prec < slen.range.min
- || slen.range.min >= HOST_WIDE_INT_MAX)
+ || slen.range.min >= target_int_max ())
slen.range.min = prec;
if ((unsigned)prec < slen.range.max
- || slen.range.max >= HOST_WIDE_INT_MAX)
+ || slen.range.max >= target_int_max ())
slen.range.max = prec;
}
- else if (slen.range.min >= HOST_WIDE_INT_MAX)
+ else if (slen.range.min >= target_int_max ())
{
slen.range.min = max_bytes_for_unknown_str;
slen.range.max = max_bytes_for_unknown_str;
+ bounded = false;
}
res.range = slen.range;
@@ -1580,7 +1587,8 @@ format_string (const conversion_spec &spec, tree arg)
res.range.max = width;
/* Adjust BOUNDED if width happens to make them equal. */
- if (res.range.min == res.range.max && res.range.min < HOST_WIDE_INT_MAX)
+ if (res.range.min == res.range.max && res.range.min < target_int_max ()
+ && bounded)
res.bounded = true;
return res;
@@ -2389,6 +2397,7 @@ try_substitute_return_value (gimple_stmt_iterator gsi,
unsigned HOST_WIDE_INT maxbytes;
if (lhs
+ && res.bounded
&& ((maxbytes = res.number_chars - 1) <= target_int_max ()
|| (res.number_chars_min - 1 <= target_int_max ()
&& (maxbytes = res.number_chars_max - 1) <= target_int_max ()))
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-2.c b/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-2.c
index f7abfd8..d6a0e6b 100644
--- a/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-2.c
+++ b/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-2.c
@@ -77,7 +77,7 @@ EQL ( 4097, sizeof buf8k, "%.4095f", 1.0);
enum { imax2 = (INT_MAX / 2) * 2 };
EQL (imax2, -1, "%*c%*c", INT_MAX / 2, 'x', INT_MAX / 2, 'y');
-/* Verify that range inforation for calls that overflow the destination
+/* Verify that range information for calls that overflow the destination
isn't available. */
RNG (0, 0, 0, "%hhi", i)
RNG (0, 0, 1, "%hhi", i)
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-3.c b/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-3.c
new file mode 100644
index 0000000..8bde54e
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-3.c
@@ -0,0 +1,40 @@
+/* PR bootstrap/77676 - powerpc64 and powerpc64le stage2 bootstrap fail
+ Test case derived from the one submitted in the bug. It verifies
+ that the sprintf return value (or value range) optimization is not
+ performed for an unknown string. */
+/* { dg-compile } */
+/* { dg-options "-O2 -Wall -Werror -fdump-tree-optimized -fprintf-return-value" } */
+
+#define INT_MAX __INT_MAX__
+#define INT_MIN (-INT_MAX - 1)
+
+extern void string_eq_min_fail ();
+extern void string_eq_max_fail ();
+
+extern void string_lt_0_fail ();
+extern void string_eq_0_fail ();
+extern void string_gt_0_fail ();
+
+void test_string (char *d, const char *s)
+{
+ int n = __builtin_sprintf (d, "%-s", s);
+
+ /* Verify that the return value is NOT assumed NOT to be INT_MIN
+ or INT_MAX. (This is a white box test based on knowinf that
+ the optimization computes its own values of the two constants.) */
+ if (n == INT_MIN) string_eq_min_fail ();
+ if (n == INT_MAX) string_eq_max_fail ();
+
+ /* The return value could be negative when strlen(s) is in excess
+ of 4095 (the maximum number of bytes a single directive is reuired
+ to handle). */
+ if (n < 0) string_lt_0_fail ();
+ if (n == 0) string_eq_0_fail ();
+ if (n > 0) string_gt_0_fail ();
+}
+
+/* { dg-final { scan-tree-dump-times "string_eq_min_fail" 1 "optimized" } } */
+/* { dg-final { scan-tree-dump-times "string_eq_max_fail" 1 "optimized" } } */
+/* { dg-final { scan-tree-dump-times "string_lt_0_fail" 1 "optimized" } } */
+/* { dg-final { scan-tree-dump-times "string_eq_0_fail" 1 "optimized" } } */
+/* { dg-final { scan-tree-dump-times "string_gt_0_fail" 1 "optimized" } } */
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-5.c b/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-5.c
new file mode 100644
index 0000000..f6f60cc
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-5.c
@@ -0,0 +1,28 @@
+/* PR bootstrap/77676 - powerpc64 and powerpc64le stage2 bootstrap fail
+ Test case from comment 6 on the bug. */
+/* { dg-compile } */
+/* { dg-options "-Wall -Werror" } */
+/* { dg-additional-options "-m32" { target { i?86-*-* x86_64-*-* } } } */
+
+struct A
+{
+ const char *a;
+ int b;
+ const char *c;
+};
+
+void bar (char *);
+
+void
+foo (struct A *p)
+{
+ char s[4096];
+ const char *u = p->a;
+ const char *t;
+ while ((t = __builtin_strstr (u, "gcc/")))
+ u = t + 4;
+
+ /* Verfiy the following doesn't emit a warning. */
+ __builtin_sprintf (s, "%s:%i (%s)", u, p->b, p->c);
+ bar (s);
+}
diff --git a/gcc/varasm.c b/gcc/varasm.c
index ba866ce..a5483aa 100644
--- a/gcc/varasm.c
+++ b/gcc/varasm.c
@@ -2545,7 +2545,7 @@ assemble_name (FILE *file, const char *name)
rtx
assemble_static_space (unsigned HOST_WIDE_INT size)
{
- char name[12];
+ char name[16];
const char *namestring;
rtx x;