On 11/25/2016 10:20 AM, Jakub Jelinek wrote:
Hi!
Here is an attempt to fix a couple of bugs in gimple-ssa-sprintf.c.
First of all, it assumes size_t is always the same as uintmax_t, which
is not necessarily the case.
Second, it uses static tree {,u}intmax_type_node; variables for caching
those types, but doesn't register them with GC; but their computation
is quite cheap, so I think it isn't worth wasting a GC root for those,
especially if we compute it only in the very rare case when somebody
uses the j modifier.
Third, the code assumes that ptrdiff_t is the signed type for size_t.
E.g. vms is one port where that isn't true, ptrdiff_t can be 64-bit,
while size_t is 32-bit.
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
It looks fine to me. Thanks for fixing these corner cases!
Martin
PS As the comment above the build_intmax_type_node function
mentions, its body was copied from lto/lto-lang.c. It would be
useful not to have to duplicate the code in the middle-end and
instead provide a shared definition of each of the nodes so that
they could be used everywhere. Ditto for ptrdiff_type_node.
It would make it less likely for patches to break when someone
forgets to bootstrap and test Ada, for instance, or where the
use of the type isn't exercised by bootstrap or the test suite
due to gaps in coverage.
2016-11-25 Jakub Jelinek <ja...@redhat.com>
* gimple-ssa-sprintf.c (build_intmax_type_nodes): Look at
UINTMAX_TYPE rather than SIZE_TYPE. Add gcc_unreachable if
intmax_t couldn't be determined.
(format_integer): Make {,u}intmax_type_node no longer static,
initialize them only when needed. For z and t use
signed_or_unsigned_type_for instead of assuming size_t and
ptrdiff_t have the same precision.
--- gcc/gimple-ssa-sprintf.c.jj 2016-11-25 09:49:47.000000000 +0100
+++ gcc/gimple-ssa-sprintf.c 2016-11-25 10:26:58.763114194 +0100
@@ -733,23 +733,23 @@ format_percent (const conversion_spec &,
}
-/* Ugh. Compute intmax_type_node and uintmax_type_node the same way
- lto/lto-lang.c does it. This should be available in tree.h. */
+/* Compute intmax_type_node and uintmax_type_node similarly to how
+ tree.c builds size_type_node. */
static void
build_intmax_type_nodes (tree *pintmax, tree *puintmax)
{
- if (strcmp (SIZE_TYPE, "unsigned int") == 0)
+ if (strcmp (UINTMAX_TYPE, "unsigned int") == 0)
{
*pintmax = integer_type_node;
*puintmax = unsigned_type_node;
}
- else if (strcmp (SIZE_TYPE, "long unsigned int") == 0)
+ else if (strcmp (UINTMAX_TYPE, "long unsigned int") == 0)
{
*pintmax = long_integer_type_node;
*puintmax = long_unsigned_type_node;
}
- else if (strcmp (SIZE_TYPE, "long long unsigned int") == 0)
+ else if (strcmp (UINTMAX_TYPE, "long long unsigned int") == 0)
{
*pintmax = long_long_integer_type_node;
*puintmax = long_long_unsigned_type_node;
@@ -762,12 +762,14 @@ build_intmax_type_nodes (tree *pintmax,
char name[50];
sprintf (name, "__int%d unsigned", int_n_data[i].bitsize);
- if (strcmp (name, SIZE_TYPE) == 0)
+ if (strcmp (name, UINTMAX_TYPE) == 0)
{
*pintmax = int_n_trees[i].signed_type;
*puintmax = int_n_trees[i].unsigned_type;
+ return;
}
}
+ gcc_unreachable ();
}
}
@@ -851,15 +853,8 @@ format_pointer (const conversion_spec &s
static fmtresult
format_integer (const conversion_spec &spec, tree arg)
{
- /* These are available as macros in the C and C++ front ends but,
- sadly, not here. */
- static tree intmax_type_node;
- static tree uintmax_type_node;
-
- /* Initialize the intmax nodes above the first time through here. */
- if (!intmax_type_node)
- build_intmax_type_nodes (&intmax_type_node, &uintmax_type_node);
-
+ tree intmax_type_node;
+ tree uintmax_type_node;
/* Set WIDTH and PRECISION to either the values in the format
specification or to zero. */
int width = spec.have_width ? spec.width : 0;
@@ -909,19 +904,20 @@ format_integer (const conversion_spec &s
break;
case FMT_LEN_z:
- dirtype = sign ? ptrdiff_type_node : size_type_node;
+ dirtype = signed_or_unsigned_type_for (!sign, size_type_node);
break;
case FMT_LEN_t:
- dirtype = sign ? ptrdiff_type_node : size_type_node;
+ dirtype = signed_or_unsigned_type_for (!sign, ptrdiff_type_node);
break;
case FMT_LEN_j:
+ build_intmax_type_nodes (&intmax_type_node, &uintmax_type_node);
dirtype = sign ? intmax_type_node : uintmax_type_node;
break;
default:
- return fmtresult ();
+ return fmtresult ();
}
/* The type of the argument to the directive, either deduced from
Jakub
