[PATCH] Fix a couple of issues in gimple-ssa-sprintf.c

Richard Biener rguenther@suse.de
Mon Nov 28 09:28:00 GMT 2016


On Fri, 25 Nov 2016, 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?

Ok.

Richard.

> 2016-11-25  Jakub Jelinek  <jakub@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
> 
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)



More information about the Gcc-patches mailing list