[PATCH 05/08] PR jit/63854: Fix double-initialization within tree-pretty-print.c

[David Malcolm <dmalcolm at redhat dot com>]

Running various JIT testcases under valgrid showed this one-time leak:

8,464 (336 direct, 8,128 indirect) bytes in 1 blocks are definitely lost in loss record 144 of 147
   at 0x4A081D4: calloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
   by 0x5DF4EA0: xcalloc (xmalloc.c:162)
   by 0x5DB0D9C: pretty_printer::pretty_printer(char const*, int) (pretty-print.c:777)
   by 0x54DA67D: __static_initialization_and_destruction_0(int, int) (tree-pretty-print.c:66)
   by 0x54DA6AF: _GLOBAL__sub_I_tree_pretty_print.c (tree-pretty-print.c:3542)
   by 0x309540F2D9: call_init.part.0 (dl-init.c:82)
   by 0x309540F3C2: _dl_init (dl-init.c:34)
   by 0x3095401229: ??? (in /usr/lib64/ld-2.18.so)
   by 0x1: ???
   by 0xFFEFFFFBE: ???
   by 0xFFEFFFFDA: ???

tree-pretty-print.c:66 is:
  static pretty_printer buffer;

pretty-print.c:777 is:
pretty_printer::pretty_printer (const char *p, int l)
  : buffer (new (XCNEW (output_buffer)) output_buffer ()),

This is the ctor of "buffer" (the pretty_printer) running at startup
by the dynamic linker, allocating its "buffer" output_buffer field
and the pair of obstacks it contains.

[Arguably "buffer" is a poor name for a pretty_printer, and these
should be named "pp" or somesuch, though that seems like it should
be a separate patch, which I'll do as a followup].

However, this ctor gets rerun the first time that
maybe_init_pretty_print is called, reallocating the output_buffer and
its obstacks, leaking the old ones:

 static void
 maybe_init_pretty_print (FILE *file)
 {
   if (!initialized)
     {
       new (&buffer) pretty_printer ();
       /* etc */
     }
   /* etc */
 }

This is a one-time leak, so it's worth fixing mostly for the sake of
keeping the valgrind output clean.

Having objects with non-empty ctors/dtors in a statically-allocated
section feels like too much C++ to me: we can't rely on the order in
which they run, and in the past I've been bitten by runtimes that
didn't support them fully (where the compiler worked, but the
linker/startup code didn't).

Hence this patch fixes the leak by rewriting the global "buffer" to
be a pointer, allocating the pp on the heap, eliminating the
before-main static ctor/dtor pair.

gcc/ChangeLog:
	PR jit/63854
	* tree-pretty-print.c: Eliminate include of <new>.
	(buffer): Convert this variable from a pretty_printer to a
	pretty_printer *.
	(initialized): Eliminate this variable in favor of the NULL-ness
	of "buffer".
	(print_generic_decl): Update for "buffer" becoming a pointer.
	(print_generic_stmt): Likewise.
	(print_generic_stmt_indented): Likewise.
	(print_generic_expr): Likewise.
	(maybe_init_pretty_print): Likewise, allocating "buffer" on the
	heap and using its non-NULL-ness to ensure idempotency.
---
 gcc/tree-pretty-print.c | 34 +++++++++++++++-------------------
 1 file changed, 15 insertions(+), 19 deletions(-)

diff --git a/gcc/tree-pretty-print.c b/gcc/tree-pretty-print.c
index 53720de..ddd7521 100644
--- a/gcc/tree-pretty-print.c
+++ b/gcc/tree-pretty-print.c
@@ -48,8 +48,6 @@ along with GCC; see the file COPYING3.  If not see
 #include "wide-int-print.h"
 #include "internal-fn.h"
 
-#include <new>                           // For placement-new.
-
 /* Local functions, macros and variables.  */
 static const char *op_symbol (const_tree);
 static void pretty_print_string (pretty_printer *, const char*);
@@ -63,8 +61,7 @@ static void do_niy (pretty_printer *, const_tree);
 
 #define NIY do_niy (buffer, node)
 
-static pretty_printer buffer;
-static int initialized = 0;
+static pretty_printer *buffer;
 
 /* Try to print something for an unknown tree code.  */
 
@@ -135,8 +132,8 @@ void
 print_generic_decl (FILE *file, tree decl, int flags)
 {
   maybe_init_pretty_print (file);
-  print_declaration (&buffer, decl, 2, flags);
-  pp_write_text_to_stream (&buffer);
+  print_declaration (buffer, decl, 2, flags);
+  pp_write_text_to_stream (buffer);
 }
 
 /* Print tree T, and its successors, on file FILE.  FLAGS specifies details
@@ -146,8 +143,8 @@ void
 print_generic_stmt (FILE *file, tree t, int flags)
 {
   maybe_init_pretty_print (file);
-  dump_generic_node (&buffer, t, 0, flags, true);
-  pp_newline_and_flush (&buffer);
+  dump_generic_node (buffer, t, 0, flags, true);
+  pp_newline_and_flush (buffer);
 }
 
 /* Print tree T, and its successors, on file FILE.  FLAGS specifies details
@@ -162,9 +159,9 @@ print_generic_stmt_indented (FILE *file, tree t, int flags, int indent)
   maybe_init_pretty_print (file);
 
   for (i = 0; i < indent; i++)
-    pp_space (&buffer);
-  dump_generic_node (&buffer, t, indent, flags, true);
-  pp_newline_and_flush (&buffer);
+    pp_space (buffer);
+  dump_generic_node (buffer, t, indent, flags, true);
+  pp_newline_and_flush (buffer);
 }
 
 /* Print a single expression T on file FILE.  FLAGS specifies details to show
@@ -174,8 +171,8 @@ void
 print_generic_expr (FILE *file, tree t, int flags)
 {
   maybe_init_pretty_print (file);
-  dump_generic_node (&buffer, t, 0, flags, false);
-  pp_flush (&buffer);
+  dump_generic_node (buffer, t, 0, flags, false);
+  pp_flush (buffer);
 }
 
 /* Dump the name of a _DECL node and its DECL_UID if TDF_UID is set
@@ -3389,15 +3386,14 @@ pretty_print_string (pretty_printer *buffer, const char *str)
 static void
 maybe_init_pretty_print (FILE *file)
 {
-  if (!initialized)
+  if (!buffer)
     {
-      new (&buffer) pretty_printer ();
-      pp_needs_newline (&buffer) = true;
-      pp_translate_identifiers (&buffer) = false;
-      initialized = 1;
+      buffer = new pretty_printer ();
+      pp_needs_newline (buffer) = true;
+      pp_translate_identifiers (buffer) = false;
     }
 
-  buffer.buffer->stream = file;
+  buffer->buffer->stream = file;
 }
 
 static void
-- 
1.8.5.3