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

[Jeff Law]

On 11/25/14 18:39, David Malcolm wrote:

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.

OK.

Jeff