public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] pretty-print: Fix up pp_wide_int [PR111329]
@ 2023-09-08 12:16 Jakub Jelinek
  2023-09-11  8:57 ` Richard Sandiford
  0 siblings, 1 reply; 2+ messages in thread
From: Jakub Jelinek @ 2023-09-08 12:16 UTC (permalink / raw)
  To: Richard Sandiford, David Malcolm; +Cc: gcc-patches, Richard Biener

Hi!

The recent pp_wide_int changes for _BitInt support (because not all
wide_ints fit into the small fixed size digit_buffer anymore) apparently
broke
+FAIL: gcc.dg/analyzer/out-of-bounds-diagram-1-debug.c (test for excess errors)
+FAIL: gcc.dg/analyzer/out-of-bounds-diagram-1-debug.c 2 blank line(s) in output
+FAIL: gcc.dg/analyzer/out-of-bounds-diagram-1-debug.c expected multiline pattern lines 17-39
(and I couldn't reproduce that in bisect seed (which is -O0 compiled) and
thought it would be some analyzer diagnostic bug).

The problem is that analyzer uses pp_wide_int with a function call in the
second argument.  Previously, when pp_wide_int macro just did
  print_dec (W, pp_buffer (PP)->digit_buffer, SGN);
  pp_string (PP, pp_buffer (PP)->digit_buffer);
it worked, because the const wide_int_ref & first argument to print_dec
bound to a temporary, which was only destructed at the end of the full
statement after print_dec was called.
But with my changes where I need to first compare the precision of the
const wide_int_ref & to decide whether to use digit_buffer or XALLOCAVEC
something larger, this means that pp_wide_int_ref binds to a temporary
which is destroyed at the end of full statement which is the
  const wide_int_ref &pp_wide_int_ref = (W);
declaration, so then invokes UB accessing a destructed temporary.

The following patch fixes it by rewriting pp_wide_int into an inline
function, so that the end of the full statement is the end of the inline
function call.  As functions using alloca aren't normally inlined, I've
also split that part into a separate out of line function.  Putting that
into pretty-print.cc didn't work, e.g. the gm2 binary doesn't link,
because pretty-print.o is in libcommon.a, but wide-print-print.o which
defines print_dec is not.  So I've put that out of line function into
wide-int-print.cc instead.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

2023-09-08  Jakub Jelinek  <jakub@redhat.com>

	PR middle-end/111329
	* pretty-print.h (pp_wide_int): Rewrite from macro into inline
	function.  For printing values which don't fit into digit_buffer
	use out-of-line function.
	* wide-int-print.h (pp_wide_int_large): Declare.
	* wide-int-print.cc: Include pretty-print.h.
	(pp_wide_int_large): Define.

--- gcc/pretty-print.h.jj	2023-09-06 14:36:53.485246347 +0200
+++ gcc/pretty-print.h	2023-09-08 11:11:21.173649942 +0200
@@ -333,28 +333,6 @@ pp_get_prefix (const pretty_printer *pp)
 #define pp_decimal_int(PP, I)  pp_scalar (PP, "%d", I)
 #define pp_unsigned_wide_integer(PP, I) \
    pp_scalar (PP, HOST_WIDE_INT_PRINT_UNSIGNED, (unsigned HOST_WIDE_INT) I)
-#define pp_wide_int(PP, W, SGN)					\
-  do								\
-    {								\
-      const wide_int_ref &pp_wide_int_ref = (W);		\
-      unsigned int pp_wide_int_prec				\
-	= pp_wide_int_ref.get_precision ();			\
-      if ((pp_wide_int_prec + 3) / 4				\
-	  > sizeof (pp_buffer (PP)->digit_buffer) - 3)		\
-	{							\
-	  char *pp_wide_int_buf					\
-	    = XALLOCAVEC (char, (pp_wide_int_prec + 3) / 4 + 3);\
-	  print_dec (pp_wide_int_ref, pp_wide_int_buf, SGN);	\
-	  pp_string (PP, pp_wide_int_buf);			\
-	}							\
-      else							\
-	{							\
-	  print_dec (pp_wide_int_ref,				\
-		     pp_buffer (PP)->digit_buffer, SGN);	\
-	  pp_string (PP, pp_buffer (PP)->digit_buffer);		\
-	}							\
-    }								\
-  while (0)
 #define pp_vrange(PP, R)					\
   do								\
     {								\
@@ -453,6 +431,19 @@ pp_wide_integer (pretty_printer *pp, HOS
   pp_scalar (pp, HOST_WIDE_INT_PRINT_DEC, i);
 }
 
+inline void
+pp_wide_int (pretty_printer *pp, const wide_int_ref &w, signop sgn)
+{
+  unsigned int prec = w.get_precision ();
+  if (UNLIKELY ((prec + 3) / 4 > sizeof (pp_buffer (pp)->digit_buffer) - 3))
+    pp_wide_int_large (pp, w, sgn);
+  else
+    {
+      print_dec (w, pp_buffer (pp)->digit_buffer, sgn);
+      pp_string (pp, pp_buffer (pp)->digit_buffer);
+    }
+}
+
 template<unsigned int N, typename T>
 void pp_wide_integer (pretty_printer *pp, const poly_int_pod<N, T> &);
 
--- gcc/wide-int-print.h.jj	2023-09-08 11:03:48.320944156 +0200
+++ gcc/wide-int-print.h	2023-09-08 11:11:46.982292282 +0200
@@ -34,5 +34,6 @@ extern void print_decu (const wide_int_r
 extern void print_decu (const wide_int_ref &wi, FILE *file);
 extern void print_hex (const wide_int_ref &wi, char *buf);
 extern void print_hex (const wide_int_ref &wi, FILE *file);
+extern void pp_wide_int_large (pretty_printer *, const wide_int_ref &, signop);
 
 #endif /* WIDE_INT_PRINT_H */
--- gcc/wide-int-print.cc.jj	2023-09-08 11:03:51.543898946 +0200
+++ gcc/wide-int-print.cc	2023-09-08 11:12:11.481952762 +0200
@@ -21,6 +21,7 @@ along with GCC; see the file COPYING3.
 #include "config.h"
 #include "system.h"
 #include "coretypes.h"
+#include "pretty-print.h"
 
 /*
  * public printing routines.
@@ -138,3 +139,14 @@ print_hex (const wide_int_ref &wi, FILE
   fputs (buf, file);
 }
 
+/* Print larger precision wide_int.  Not defined as inline in a header
+   together with pp_wide_int because XALLOCAVEC will make it uninlinable.  */
+
+void
+pp_wide_int_large (pretty_printer *pp, const wide_int_ref &w, signop sgn)
+{
+  unsigned int prec = w.get_precision ();
+  char *buf = XALLOCAVEC (char, (prec + 3) / 4 + 3);
+  print_dec (w, buf, sgn);
+  pp_string (pp, buf);
+}

	Jakub


^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [PATCH] pretty-print: Fix up pp_wide_int [PR111329]
  2023-09-08 12:16 [PATCH] pretty-print: Fix up pp_wide_int [PR111329] Jakub Jelinek
@ 2023-09-11  8:57 ` Richard Sandiford
  0 siblings, 0 replies; 2+ messages in thread
From: Richard Sandiford @ 2023-09-11  8:57 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: David Malcolm, gcc-patches, Richard Biener

Jakub Jelinek <jakub@redhat.com> writes:
> Hi!
>
> The recent pp_wide_int changes for _BitInt support (because not all
> wide_ints fit into the small fixed size digit_buffer anymore) apparently
> broke
> +FAIL: gcc.dg/analyzer/out-of-bounds-diagram-1-debug.c (test for excess errors)
> +FAIL: gcc.dg/analyzer/out-of-bounds-diagram-1-debug.c 2 blank line(s) in output
> +FAIL: gcc.dg/analyzer/out-of-bounds-diagram-1-debug.c expected multiline pattern lines 17-39
> (and I couldn't reproduce that in bisect seed (which is -O0 compiled) and
> thought it would be some analyzer diagnostic bug).
>
> The problem is that analyzer uses pp_wide_int with a function call in the
> second argument.  Previously, when pp_wide_int macro just did
>   print_dec (W, pp_buffer (PP)->digit_buffer, SGN);
>   pp_string (PP, pp_buffer (PP)->digit_buffer);
> it worked, because the const wide_int_ref & first argument to print_dec
> bound to a temporary, which was only destructed at the end of the full
> statement after print_dec was called.
> But with my changes where I need to first compare the precision of the
> const wide_int_ref & to decide whether to use digit_buffer or XALLOCAVEC
> something larger, this means that pp_wide_int_ref binds to a temporary
> which is destroyed at the end of full statement which is the
>   const wide_int_ref &pp_wide_int_ref = (W);
> declaration, so then invokes UB accessing a destructed temporary.
>
> The following patch fixes it by rewriting pp_wide_int into an inline
> function, so that the end of the full statement is the end of the inline
> function call.  As functions using alloca aren't normally inlined, I've
> also split that part into a separate out of line function.  Putting that
> into pretty-print.cc didn't work, e.g. the gm2 binary doesn't link,
> because pretty-print.o is in libcommon.a, but wide-print-print.o which
> defines print_dec is not.  So I've put that out of line function into
> wide-int-print.cc instead.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
>
> 2023-09-08  Jakub Jelinek  <jakub@redhat.com>
>
> 	PR middle-end/111329
> 	* pretty-print.h (pp_wide_int): Rewrite from macro into inline
> 	function.  For printing values which don't fit into digit_buffer
> 	use out-of-line function.
> 	* wide-int-print.h (pp_wide_int_large): Declare.
> 	* wide-int-print.cc: Include pretty-print.h.
> 	(pp_wide_int_large): Define.

OK, thanks.

Richard

> --- gcc/pretty-print.h.jj	2023-09-06 14:36:53.485246347 +0200
> +++ gcc/pretty-print.h	2023-09-08 11:11:21.173649942 +0200
> @@ -333,28 +333,6 @@ pp_get_prefix (const pretty_printer *pp)
>  #define pp_decimal_int(PP, I)  pp_scalar (PP, "%d", I)
>  #define pp_unsigned_wide_integer(PP, I) \
>     pp_scalar (PP, HOST_WIDE_INT_PRINT_UNSIGNED, (unsigned HOST_WIDE_INT) I)
> -#define pp_wide_int(PP, W, SGN)					\
> -  do								\
> -    {								\
> -      const wide_int_ref &pp_wide_int_ref = (W);		\
> -      unsigned int pp_wide_int_prec				\
> -	= pp_wide_int_ref.get_precision ();			\
> -      if ((pp_wide_int_prec + 3) / 4				\
> -	  > sizeof (pp_buffer (PP)->digit_buffer) - 3)		\
> -	{							\
> -	  char *pp_wide_int_buf					\
> -	    = XALLOCAVEC (char, (pp_wide_int_prec + 3) / 4 + 3);\
> -	  print_dec (pp_wide_int_ref, pp_wide_int_buf, SGN);	\
> -	  pp_string (PP, pp_wide_int_buf);			\
> -	}							\
> -      else							\
> -	{							\
> -	  print_dec (pp_wide_int_ref,				\
> -		     pp_buffer (PP)->digit_buffer, SGN);	\
> -	  pp_string (PP, pp_buffer (PP)->digit_buffer);		\
> -	}							\
> -    }								\
> -  while (0)
>  #define pp_vrange(PP, R)					\
>    do								\
>      {								\
> @@ -453,6 +431,19 @@ pp_wide_integer (pretty_printer *pp, HOS
>    pp_scalar (pp, HOST_WIDE_INT_PRINT_DEC, i);
>  }
>  
> +inline void
> +pp_wide_int (pretty_printer *pp, const wide_int_ref &w, signop sgn)
> +{
> +  unsigned int prec = w.get_precision ();
> +  if (UNLIKELY ((prec + 3) / 4 > sizeof (pp_buffer (pp)->digit_buffer) - 3))
> +    pp_wide_int_large (pp, w, sgn);
> +  else
> +    {
> +      print_dec (w, pp_buffer (pp)->digit_buffer, sgn);
> +      pp_string (pp, pp_buffer (pp)->digit_buffer);
> +    }
> +}
> +
>  template<unsigned int N, typename T>
>  void pp_wide_integer (pretty_printer *pp, const poly_int_pod<N, T> &);
>  
> --- gcc/wide-int-print.h.jj	2023-09-08 11:03:48.320944156 +0200
> +++ gcc/wide-int-print.h	2023-09-08 11:11:46.982292282 +0200
> @@ -34,5 +34,6 @@ extern void print_decu (const wide_int_r
>  extern void print_decu (const wide_int_ref &wi, FILE *file);
>  extern void print_hex (const wide_int_ref &wi, char *buf);
>  extern void print_hex (const wide_int_ref &wi, FILE *file);
> +extern void pp_wide_int_large (pretty_printer *, const wide_int_ref &, signop);
>  
>  #endif /* WIDE_INT_PRINT_H */
> --- gcc/wide-int-print.cc.jj	2023-09-08 11:03:51.543898946 +0200
> +++ gcc/wide-int-print.cc	2023-09-08 11:12:11.481952762 +0200
> @@ -21,6 +21,7 @@ along with GCC; see the file COPYING3.
>  #include "config.h"
>  #include "system.h"
>  #include "coretypes.h"
> +#include "pretty-print.h"
>  
>  /*
>   * public printing routines.
> @@ -138,3 +139,14 @@ print_hex (const wide_int_ref &wi, FILE
>    fputs (buf, file);
>  }
>  
> +/* Print larger precision wide_int.  Not defined as inline in a header
> +   together with pp_wide_int because XALLOCAVEC will make it uninlinable.  */
> +
> +void
> +pp_wide_int_large (pretty_printer *pp, const wide_int_ref &w, signop sgn)
> +{
> +  unsigned int prec = w.get_precision ();
> +  char *buf = XALLOCAVEC (char, (prec + 3) / 4 + 3);
> +  print_dec (w, buf, sgn);
> +  pp_string (pp, buf);
> +}
>
> 	Jakub

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2023-09-11  8:57 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-09-08 12:16 [PATCH] pretty-print: Fix up pp_wide_int [PR111329] Jakub Jelinek
2023-09-11  8:57 ` Richard Sandiford

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).