avoid using %lli et al.

The attached patch replaces use of %lli and %llu with
HOST_WIDE_INT_PRINT_DEC and HOST_WIDE_INT_PRINT_UNSIGNED
for portability, and also replaces wide_int::to_shwi() with
offset_int::from().

Although I couldn't come up with a test where the latter change
above would matter, by using obscenely large offset values I did
manage to trigger an assertion in the -Wrestrict pass in place
to try to verify the sanity of the offsets/sizes after they have
been converted from offset_int to HOST_WIDE_INT for printing
(and potentially mangled in the process).  I got rid of
the assertion since it doesn't serve a useful purpose.

Martin

Martin Sebor <msebor@gmail.com> writes:
> The attached patch replaces use of %lli and %llu with
> HOST_WIDE_INT_PRINT_DEC and HOST_WIDE_INT_PRINT_UNSIGNED
> for portability, and also replaces wide_int::to_shwi() with
> offset_int::from().

Thanks for doing this.  I think you need to remove the (long long)
casts too though, so that the format stays in sync with the types.

Richard

On Wed, Dec 20, 2017 at 05:03:23PM -0700, Martin Sebor wrote:
> @@ -1228,24 +1228,30 @@ maybe_diag_overlap (location_t loc, gcall *call, b
>  
>    if (dstref.offrange[0] == dstref.offrange[1]
>        || dstref.offrange[1] > HOST_WIDE_INT_MAX)
> -    sprintf (offstr[0], "%lli", (long long) dstref.offrange[0].to_shwi ());
> +    sprintf (offstr[0], HOST_WIDE_INT_PRINT_DEC,
> +	     (long long) dstref.offrange[0].to_shwi ());

to_shwi () returns a HOST_WIDE_INT, there is no point in casting that to
(long long), and in case long long is different from HOST_WIDE_INT it might
result in warnings or even UB.  Just drop those casts everywhere where
the operand already is SHWI or UHWI.

	Jakub

On 12/20/2017 05:07 PM, Richard Sandiford wrote:
> Martin Sebor <msebor@gmail.com> writes:
>> The attached patch replaces use of %lli and %llu with
>> HOST_WIDE_INT_PRINT_DEC and HOST_WIDE_INT_PRINT_UNSIGNED
>> for portability, and also replaces wide_int::to_shwi() with
>> offset_int::from().
>
> Thanks for doing this.  I think you need to remove the (long long)
> casts too though, so that the format stays in sync with the types.

Doh!  Thanks!  Had I waited for stage 2 bootstrap I suspect I would
have found out when it broke due to -Wformat complaining that
HOST_WIDE_INT_PRINT_DEC (%ld on my system) not matching the long
long argument.

Attached is the patch with the casts removed (still bootstrapping).

Martin
gcc/ChangeLog:

	* gimple-ssa-warn-restrict.c (builtin_memref::builtin_memref): Use
	offset_int::from instead of wide_int::to_shwi.
	(maybe_diag_overlap): Remove assertion.
	Use HOST_WIDE_INT_PRINT_DEC instead of %lli.
	* gimple-ssa-sprintf.c (format_directive): Same.
	(parse_directive): Same.
	(sprintf_dom_walker::compute_format_length): Same.
	(try_substitute_return_value): Same.

gcc/testsuite/ChangeLog:

	* gcc.dg/Wrestrict-3.c: New test.

Index: gcc/testsuite/gcc.dg/Wrestrict-3.c
===================================================================
--- gcc/testsuite/gcc.dg/Wrestrict-3.c	(nonexistent)
+++ gcc/testsuite/gcc.dg/Wrestrict-3.c	(working copy)
@@ -0,0 +1,17 @@
+/* Test to verify that the call below with the out-of-bounds offset
+   doesn't trigger an internal assertion and is diagnosed.
+   { dg-do compile }
+   { dg-options "-O2 -Wrestrict" } */
+
+#define DIFF_MAX   __PTRDIFF_MAX__
+
+void test_no_ice (int *d, __PTRDIFF_TYPE__ i, __SIZE_TYPE__ n)
+{
+  if (i < DIFF_MAX / sizeof *d - 1 || DIFF_MAX / sizeof *d + 2 < i)
+    i = DIFF_MAX / sizeof *d - 1;
+
+  if (n < DIFF_MAX)
+    n = DIFF_MAX / sizeof *d;
+
+  __builtin_strncpy ((char*)(d + i), (char*)d, n);   /* { dg-warning "\\\[-Wrestrict]" } */
+}
Index: gcc/gimple-ssa-warn-restrict.c
===================================================================
--- gcc/gimple-ssa-warn-restrict.c	(revision 255901)
+++ gcc/gimple-ssa-warn-restrict.c	(working copy)
@@ -276,13 +276,13 @@ builtin_memref::builtin_memref (tree expr, tree si
 		  value_range_type rng = get_range_info (offset, &min, &max);
 		  if (rng == VR_RANGE)
 		    {
-		      offrange[0] = min.to_shwi ();
-		      offrange[1] = max.to_shwi ();
+		      offrange[0] = offset_int::from (min, SIGNED);
+		      offrange[1] = offset_int::from (max, SIGNED);
 		    }
 		  else if (rng == VR_ANTI_RANGE)
 		    {
-		      offrange[0] = (max + 1).to_shwi ();
-		      offrange[1] = (min - 1).to_shwi ();
+		      offrange[0] = offset_int::from (max + 1, SIGNED);
+		      offrange[1] = offset_int::from (min - 1, SIGNED);
 		    }
 		  else
 		    {
@@ -1228,25 +1228,31 @@ maybe_diag_overlap (location_t loc, gcall *call, b
 
   if (dstref.offrange[0] == dstref.offrange[1]
       || dstref.offrange[1] > HOST_WIDE_INT_MAX)
-    sprintf (offstr[0], "%lli", (long long) dstref.offrange[0].to_shwi ());
+    sprintf (offstr[0], HOST_WIDE_INT_PRINT_DEC,
+	     dstref.offrange[0].to_shwi ());
   else
-    sprintf (offstr[0], "[%lli, %lli]",
-	     (long long) dstref.offrange[0].to_shwi (),
-	     (long long) dstref.offrange[1].to_shwi ());
+    sprintf (offstr[0],
+	     "[" HOST_WIDE_INT_PRINT_DEC ", " HOST_WIDE_INT_PRINT_DEC "]",
+	     dstref.offrange[0].to_shwi (),
+	     dstref.offrange[1].to_shwi ());
 
   if (srcref.offrange[0] == srcref.offrange[1]
       || srcref.offrange[1] > HOST_WIDE_INT_MAX)
-    sprintf (offstr[1], "%lli", (long long) srcref.offrange[0].to_shwi ());
+    sprintf (offstr[1],
+	     HOST_WIDE_INT_PRINT_DEC,
+	     srcref.offrange[0].to_shwi ());
   else
-    sprintf (offstr[1], "[%lli, %lli]",
-	     (long long) srcref.offrange[0].to_shwi (),
-	     (long long) srcref.offrange[1].to_shwi ());
+    sprintf (offstr[1],
+	     "[" HOST_WIDE_INT_PRINT_DEC ", " HOST_WIDE_INT_PRINT_DEC "]",
+	     srcref.offrange[0].to_shwi (),
+	     srcref.offrange[1].to_shwi ());
 
   if (ovloff[0] == ovloff[1] || !ovloff[1])
-    sprintf (offstr[2], "%lli", (long long) ovloff[0]);
+    sprintf (offstr[2], HOST_WIDE_INT_PRINT_DEC, ovloff[0]);
   else
-    sprintf (offstr[2], "[%lli, %lli]",
-	     (long long) ovloff[0], (long long) ovloff[1]);
+    sprintf (offstr[2],
+	     "[" HOST_WIDE_INT_PRINT_DEC ", " HOST_WIDE_INT_PRINT_DEC "]",
+	     ovloff[0], ovloff[1]);
 
   const offset_int maxobjsize = tree_to_shwi (max_object_size ());
   bool must_overlap = ovlsiz[0] > 0;
@@ -1361,9 +1367,6 @@ maybe_diag_overlap (location_t loc, gcall *call, b
     }
 
   /* Issue "may overlap" diagnostics below.  */
-  gcc_assert (ovlsiz[0] == 0
-	      && ovlsiz[1] > 0
-	      && ovlsiz[1] <= maxobjsize.to_shwi ());
 
   /* Use more concise wording when one of the offsets is unbounded
      to avoid confusing the user with large and mostly meaningless
Index: gcc/gimple-ssa-sprintf.c
===================================================================
--- gcc/gimple-ssa-sprintf.c	(revision 255901)
+++ gcc/gimple-ssa-sprintf.c	(working copy)
@@ -2993,16 +2993,16 @@ format_directive (const sprintf_dom_walker::call_i
 
   if (dump_file && *dir.beg)
     {
-      fprintf (dump_file, "    Result: %lli, %lli, %lli, %lli "
-	       "(%lli, %lli, %lli, %lli)\n",
-	       (long long)fmtres.range.min,
-	       (long long)fmtres.range.likely,
-	       (long long)fmtres.range.max,
-	       (long long)fmtres.range.unlikely,
-	       (long long)res->range.min,
-	       (long long)res->range.likely,
-	       (long long)res->range.max,
-	       (long long)res->range.unlikely);
+      fprintf (dump_file,
+	       "    Result: "
+	       HOST_WIDE_INT_PRINT_DEC ", " HOST_WIDE_INT_PRINT_DEC ", "
+	       HOST_WIDE_INT_PRINT_DEC ", " HOST_WIDE_INT_PRINT_DEC " ("
+	       HOST_WIDE_INT_PRINT_DEC ", " HOST_WIDE_INT_PRINT_DEC ", "
+	       HOST_WIDE_INT_PRINT_DEC ", " HOST_WIDE_INT_PRINT_DEC ")\n",
+	       fmtres.range.min, fmtres.range.likely,
+	       fmtres.range.max, fmtres.range.unlikely,
+	       res->range.min, res->range.likely,
+	       res->range.max, res->range.unlikely);
     }
 
   return true;
@@ -3033,11 +3033,12 @@ parse_directive (sprintf_dom_walker::call_info &in
 
       if (dump_file)
 	{
-	  fprintf (dump_file, "  Directive %u at offset %llu: \"%.*s\", "
-		   "length = %llu\n",
+	  fprintf (dump_file, "  Directive %u at offset "
+		   HOST_WIDE_INT_PRINT_UNSIGNED ": \"%.*s\", "
+		   "length = " HOST_WIDE_INT_PRINT_UNSIGNED "\n",
 		   dir.dirno,
-		   (unsigned long long)(size_t)(dir.beg - info.fmtstr),
-		   (int)dir.len, dir.beg, (unsigned long long)dir.len);
+		   (unsigned HOST_WIDE_INT)(size_t)(dir.beg - info.fmtstr),
+		   (int)dir.len, dir.beg, dir.len);
 	}
 
       return len - !*str;
@@ -3409,25 +3410,34 @@ parse_directive (sprintf_dom_walker::call_info &in
 
   if (dump_file)
     {
-      fprintf (dump_file, "  Directive %u at offset %llu: \"%.*s\"",
-	       dir.dirno, (unsigned long long)(size_t)(dir.beg - info.fmtstr),
+      fprintf (dump_file,
+	       "  Directive %u at offset " HOST_WIDE_INT_PRINT_UNSIGNED
+	       ": \"%.*s\"",
+	       dir.dirno,
+	       (unsigned HOST_WIDE_INT)(size_t)(dir.beg - info.fmtstr),
 	       (int)dir.len, dir.beg);
       if (star_width)
 	{
 	  if (dir.width[0] == dir.width[1])
-	    fprintf (dump_file, ", width = %lli", (long long)dir.width[0]);
+	    fprintf (dump_file, ", width = " HOST_WIDE_INT_PRINT_DEC,
+		     dir.width[0]);
 	  else
-	    fprintf (dump_file, ", width in range [%lli, %lli]",
-		     (long long)dir.width[0], (long long)dir.width[1]);
+	    fprintf (dump_file,
+		     ", width in range [" HOST_WIDE_INT_PRINT_DEC
+		     ", " HOST_WIDE_INT_PRINT_DEC "]",
+		     dir.width[0], dir.width[1]);
 	}
 
       if (star_precision)
 	{
 	  if (dir.prec[0] == dir.prec[1])
-	    fprintf (dump_file, ", precision = %lli", (long long)dir.prec[0]);
+	    fprintf (dump_file, ", precision = " HOST_WIDE_INT_PRINT_DEC,
+		     dir.prec[0]);
 	  else
-	    fprintf (dump_file, ", precision in range [%lli, %lli]",
-		     (long long)dir.prec[0], (long long)dir.prec[1]);
+	    fprintf (dump_file,
+		     ", precision in range [" HOST_WIDE_INT_PRINT_DEC
+		     HOST_WIDE_INT_PRINT_DEC "]",
+		     dir.prec[0], dir.prec[1]);
 	}
       fputc ('\n', dump_file);
     }
@@ -3453,8 +3463,10 @@ sprintf_dom_walker::compute_format_length (call_in
 	       LOCATION_FILE (callloc), LOCATION_LINE (callloc));
       print_generic_expr (dump_file, info.func, dump_flags);
 
-      fprintf (dump_file, ": objsize = %llu, fmtstr = \"%s\"\n",
-	       (unsigned long long)info.objsize, info.fmtstr);
+      fprintf (dump_file,
+	       ": objsize = " HOST_WIDE_INT_PRINT_UNSIGNED
+	       ", fmtstr = \"%s\"\n",
+	       info.objsize, info.fmtstr);
     }
 
   /* Reset the minimum and maximum byte counters.  */
@@ -3680,13 +3692,14 @@ try_substitute_return_value (gimple_stmt_iterator
 	  const char *what = setrange ? "Setting" : "Discarding";
 	  if (retval[0] != retval[1])
 	    fprintf (dump_file,
-		     "  %s %s-bounds return value range [%llu, %llu].\n",
-		     what, inbounds,
-		     (unsigned long long)retval[0],
-		     (unsigned long long)retval[1]);
+		     "  %s %s-bounds return value range ["
+		     HOST_WIDE_INT_PRINT_UNSIGNED ", "
+		     HOST_WIDE_INT_PRINT_UNSIGNED "].\n",
+		     what, inbounds, retval[0], retval[1]);
 	  else
-	    fprintf (dump_file, "  %s %s-bounds return value %llu.\n",
-		     what, inbounds, (unsigned long long)retval[0]);
+	    fprintf (dump_file, "  %s %s-bounds return value "
+		     HOST_WIDE_INT_PRINT_UNSIGNED ".\n",
+		     what, inbounds, retval[0]);
 	}
     }

Ping:

   https://gcc.gnu.org/ml/gcc-patches/2017-12/msg01399.html

On 12/20/2017 05:28 PM, Martin Sebor wrote:
> On 12/20/2017 05:07 PM, Richard Sandiford wrote:
>> Martin Sebor <msebor@gmail.com> writes:
>>> The attached patch replaces use of %lli and %llu with
>>> HOST_WIDE_INT_PRINT_DEC and HOST_WIDE_INT_PRINT_UNSIGNED
>>> for portability, and also replaces wide_int::to_shwi() with
>>> offset_int::from().
>>
>> Thanks for doing this.  I think you need to remove the (long long)
>> casts too though, so that the format stays in sync with the types.
>
> Doh!  Thanks!  Had I waited for stage 2 bootstrap I suspect I would
> have found out when it broke due to -Wformat complaining that
> HOST_WIDE_INT_PRINT_DEC (%ld on my system) not matching the long
> long argument.
>
> Attached is the patch with the casts removed (still bootstrapping).
>
> Martin

On 12/20/2017 05:28 PM, Martin Sebor wrote:
> On 12/20/2017 05:07 PM, Richard Sandiford wrote:
>> Martin Sebor <msebor@gmail.com> writes:
>>> The attached patch replaces use of %lli and %llu with
>>> HOST_WIDE_INT_PRINT_DEC and HOST_WIDE_INT_PRINT_UNSIGNED
>>> for portability, and also replaces wide_int::to_shwi() with
>>> offset_int::from().
>>
>> Thanks for doing this.  I think you need to remove the (long long)
>> casts too though, so that the format stays in sync with the types.
> 
> Doh!  Thanks!  Had I waited for stage 2 bootstrap I suspect I would
> have found out when it broke due to -Wformat complaining that
> HOST_WIDE_INT_PRINT_DEC (%ld on my system) not matching the long
> long argument.
> 
> Attached is the patch with the casts removed (still bootstrapping).
> 
> Martin
> 
> gcc-printf-lli.diff
> 
> 
> gcc/ChangeLog:
> 
> 	* gimple-ssa-warn-restrict.c (builtin_memref::builtin_memref): Use
> 	offset_int::from instead of wide_int::to_shwi.
> 	(maybe_diag_overlap): Remove assertion.
> 	Use HOST_WIDE_INT_PRINT_DEC instead of %lli.
> 	* gimple-ssa-sprintf.c (format_directive): Same.
> 	(parse_directive): Same.
> 	(sprintf_dom_walker::compute_format_length): Same.
> 	(try_substitute_return_value): Same.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* gcc.dg/Wrestrict-3.c: New test.
OK.
jeff

On Tue, Jan 02, 2018 at 04:40:50PM -0700, Jeff Law wrote:
> > Attached is the patch with the casts removed (still bootstrapping).
> > 
> > Martin
> > 
> > gcc-printf-lli.diff
> > 
> > 
> > gcc/ChangeLog:
> > 
> > 	* gimple-ssa-warn-restrict.c (builtin_memref::builtin_memref): Use
> > 	offset_int::from instead of wide_int::to_shwi.
> > 	(maybe_diag_overlap): Remove assertion.
> > 	Use HOST_WIDE_INT_PRINT_DEC instead of %lli.
> > 	* gimple-ssa-sprintf.c (format_directive): Same.
> > 	(parse_directive): Same.
> > 	(sprintf_dom_walker::compute_format_length): Same.
> > 	(try_substitute_return_value): Same.
> > 
> > gcc/testsuite/ChangeLog:
> > 
> > 	* gcc.dg/Wrestrict-3.c: New test.
> OK.

This broke bootstrap on i686-linux, dir.len is size_t, which isn't always
appropriate for HOST_WIDE_INT_PRINT_DEC format.

Fixed thusly, committed to trunk as obvious after i686-linux bootstrap
went past the previous failure point.

2018-01-04  Jakub Jelinek  <jakub@redhat.com>

	* gimple-ssa-sprintf.c (parse_directive): Cast second dir.len to uhwi.

--- gcc/gimple-ssa-sprintf.c.jj	2018-01-03 22:24:36.000000000 +0100
+++ gcc/gimple-ssa-sprintf.c	2018-01-04 00:15:55.950179583 +0100
@@ -3040,7 +3040,7 @@ parse_directive (sprintf_dom_walker::cal
 		   "length = " HOST_WIDE_INT_PRINT_UNSIGNED "\n",
 		   dir.dirno,
 		   (unsigned HOST_WIDE_INT)(size_t)(dir.beg - info.fmtstr),
-		   (int)dir.len, dir.beg, dir.len);
+		   (int)dir.len, dir.beg, (unsigned HOST_WIDE_INT) dir.len);
 	}
 
       return len - !*str;


	Jakub

On 01/03/2018 04:47 PM, Jakub Jelinek wrote:
> On Tue, Jan 02, 2018 at 04:40:50PM -0700, Jeff Law wrote:
>>> Attached is the patch with the casts removed (still bootstrapping).
>>>
>>> Martin
>>>
>>> gcc-printf-lli.diff
>>>
>>>
>>> gcc/ChangeLog:
>>>
>>> 	* gimple-ssa-warn-restrict.c (builtin_memref::builtin_memref): Use
>>> 	offset_int::from instead of wide_int::to_shwi.
>>> 	(maybe_diag_overlap): Remove assertion.
>>> 	Use HOST_WIDE_INT_PRINT_DEC instead of %lli.
>>> 	* gimple-ssa-sprintf.c (format_directive): Same.
>>> 	(parse_directive): Same.
>>> 	(sprintf_dom_walker::compute_format_length): Same.
>>> 	(try_substitute_return_value): Same.
>>>
>>> gcc/testsuite/ChangeLog:
>>>
>>> 	* gcc.dg/Wrestrict-3.c: New test.
>> OK.
>
> This broke bootstrap on i686-linux, dir.len is size_t, which isn't always
> appropriate for HOST_WIDE_INT_PRINT_DEC format.
>
> Fixed thusly, committed to trunk as obvious after i686-linux bootstrap
> went past the previous failure point.

Thanks.

This is an example where having a solution for bug 78014 would
be helpful.  I.e., a -Wformat checker to help enforce the use
of %zu instead of %u or %lu (or an explicit cast from size_t)
even on targets size_t is unsigned or unsigned long, respectively.

Martin

>
> 2018-01-04  Jakub Jelinek  <jakub@redhat.com>
>
> 	* gimple-ssa-sprintf.c (parse_directive): Cast second dir.len to uhwi.
>
> --- gcc/gimple-ssa-sprintf.c.jj	2018-01-03 22:24:36.000000000 +0100
> +++ gcc/gimple-ssa-sprintf.c	2018-01-04 00:15:55.950179583 +0100
> @@ -3040,7 +3040,7 @@ parse_directive (sprintf_dom_walker::cal
>  		   "length = " HOST_WIDE_INT_PRINT_UNSIGNED "\n",
>  		   dir.dirno,
>  		   (unsigned HOST_WIDE_INT)(size_t)(dir.beg - info.fmtstr),
> -		   (int)dir.len, dir.beg, dir.len);
> +		   (int)dir.len, dir.beg, (unsigned HOST_WIDE_INT) dir.len);
>  	}
>
>        return len - !*str;
>
>
> 	Jakub
>

On Wed, Jan 03, 2018 at 05:00:41PM -0700, Martin Sebor wrote:
> This is an example where having a solution for bug 78014 would
> be helpful.  I.e., a -Wformat checker to help enforce the use

No, because %zu isn't portable enough for all the hosts we support.

What we could do is define SIZE_T_PRINT_UNSIGNED and similar macros,
to "%zu" on POSIX compliant targets and something else depending on what we
find out during configure (e.g. using templates to find out if size_t
on the host is unsigned {int,long,long long} or something different
(I hope we don't support msp430 as host where size_t is unsigned __int20).

While in theory GCC could handle some PR78014 cases (but only very early
before any folding) for size_t, HOST_WIDE_INT is a define and I don't see
how GCC could argue about portability there.

> of %zu instead of %u or %lu (or an explicit cast from size_t)
> even on targets size_t is unsigned or unsigned long, respectively.

	Jakub

On 01/04/2018 01:30 AM, Jakub Jelinek wrote:
> On Wed, Jan 03, 2018 at 05:00:41PM -0700, Martin Sebor wrote:
>> This is an example where having a solution for bug 78014 would
>> be helpful.  I.e., a -Wformat checker to help enforce the use
>
> No, because %zu isn't portable enough for all the hosts we support.

GCC could mitigate all these portability pitfalls by providing
its own versions of the functions (I suggested gcc_printf the
last time we discussed this subject).  libiberty already does
this for a small subset of the portability problems in some of
the standard functions (including vsprintf), so this would be
a natural extension of that approach.

> What we could do is define SIZE_T_PRINT_UNSIGNED and similar macros,
> to "%zu" on POSIX compliant targets and something else depending on what we
> find out during configure (e.g. using templates to find out if size_t
> on the host is unsigned {int,long,long long} or something different
> (I hope we don't support msp430 as host where size_t is unsigned __int20).

We could do that, but it would be a far inferior solution to
defining our own printf.  Take another look at what using these
awful macros does to readability:

   fprintf (dump_file, "  Directive %u at offset "
            HOST_WIDE_INT_PRINT_UNSIGNED ": \"%.*s\", "
            "length = " HOST_WIDE_INT_PRINT_UNSIGNED "\n",
            dir.dirno,
            (unsigned HOST_WIDE_INT)(size_t)(dir.beg - info.fmtstr),
            (int)dir.len, dir.beg, dir.len);

With GCC's own fprintf, all this ugliness could go away.  In C++ 11
gcc_fprintf could even be a variadic function template that would
completely obviate the need for length modifiers or even conversion
specifiers.  With it, the above could become as simple as:

   gcc_fprintf (dump_file,
                "  Directive %u at offset %u: \"%.*s\", length = %u \n",
                dir.dirno, dir.beg - info.fmtstr,
                dir.len, dir.beg, dir.len);

regardless of the types of the arguments.

Martin

>
> While in theory GCC could handle some PR78014 cases (but only very early
> before any folding) for size_t, HOST_WIDE_INT is a define and I don't see
> how GCC could argue about portability there.
>
>> of %zu instead of %u or %lu (or an explicit cast from size_t)
>> even on targets size_t is unsigned or unsigned long, respectively.
>
> 	Jakub
>

On Thu, Jan 04, 2018 at 08:50:30AM -0700, Martin Sebor wrote:
> On 01/04/2018 01:30 AM, Jakub Jelinek wrote:
> > On Wed, Jan 03, 2018 at 05:00:41PM -0700, Martin Sebor wrote:
> > > This is an example where having a solution for bug 78014 would
> > > be helpful.  I.e., a -Wformat checker to help enforce the use
> > 
> > No, because %zu isn't portable enough for all the hosts we support.
> 
> GCC could mitigate all these portability pitfalls by providing
> its own versions of the functions (I suggested gcc_printf the
> last time we discussed this subject).  libiberty already does
> this for a small subset of the portability problems in some of
> the standard functions (including vsprintf), so this would be
> a natural extension of that approach.

Ugh, no.

	Jakub

On 01/04/2018 08:53 AM, Jakub Jelinek wrote:
> On Thu, Jan 04, 2018 at 08:50:30AM -0700, Martin Sebor wrote:
>> On 01/04/2018 01:30 AM, Jakub Jelinek wrote:
>>> On Wed, Jan 03, 2018 at 05:00:41PM -0700, Martin Sebor wrote:
>>>> This is an example where having a solution for bug 78014 would
>>>> be helpful.  I.e., a -Wformat checker to help enforce the use
>>>
>>> No, because %zu isn't portable enough for all the hosts we support.
>>
>> GCC could mitigate all these portability pitfalls by providing
>> its own versions of the functions (I suggested gcc_printf the
>> last time we discussed this subject).  libiberty already does
>> this for a small subset of the portability problems in some of
>> the standard functions (including vsprintf), so this would be
>> a natural extension of that approach.
>
> Ugh, no.
>
> 	Jakub

I took me a while to process this well-articulated argument
but I think I finally got it.

An alternative to using %zu that I mentioned in my earlier
post is to cast the argument to the type the directive
expects:

 > I.e., a -Wformat checker to help enforce the use ... (or
 > an explicit cast from size_t) even on targets size_t is
 > unsigned or unsigned long, respectively.

If providing interfaces to make portability to non-C99 hosts
easier is not palatable for some unknown reason, then -Wformat
could be enhanced to warn for uses of %zu and other similar
conversions to help prevent introducing them into GCC sources
regardless of the host GCC is being developed on.

The common idea of all of these solutions is to enhance our
tools and APIs to make it easier to avoid regressions like
the one you so kindly (and quickly) fixed for me.  But if
you prefer to spend your time and energy cleaning up the
mistakes of others rather than focus on adding improvements
then I understand if you don't agree with these suggestions.

Martin