Fix memory leak of pretty_printer prefixes

We were rather sloppy about handling the ownership of prefixes for
pretty_printer, and this lead to a memory leak for any time a
diagnostic_show_locus call emits multiple line spans.

This showed up in "make selftest-valgrind" as:

3,976 bytes in 28 blocks are definitely lost in loss record 632 of 669
   at 0x4A0645D: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
   by 0x1F08227: xmalloc (xmalloc.c:147)
   by 0x1F083E6: xvasprintf (xvasprintf.c:58)
   by 0x1E7EC7D: build_message_string(char const*, ...) (diagnostic.c:78)
   by 0x1E7F438: diagnostic_get_location_text(diagnostic_context*, expanded_location) (diagnostic.c:328)
   by 0x1E7FD54: default_diagnostic_start_span_fn(diagnostic_context*, expanded_location) (diagnostic.c:626)
   by 0x1EB3508: selftest::test_diagnostic_context::start_span_cb(diagnostic_context*, expanded_location) (selftest-diagnostic.c:57)
   by 0x1E89215: diagnostic_show_locus(diagnostic_context*, rich_location*, diagnostic_t) (diagnostic-show-locus.c:1992)
   by 0x1E8ECAD: selftest::test_fixit_insert_containing_newline_2(selftest::line_table_case const&) (diagnostic-show-locus.c:3044)
   by 0x1EB0606: selftest::for_each_line_table_case(void (*)(selftest::line_table_case const&)) (input.c:3525)
   by 0x1E8F3F5: selftest::diagnostic_show_locus_c_tests() (diagnostic-show-locus.c:3164)
   by 0x1E010BF: selftest::run_tests() (selftest-run-tests.c:88)

4,004 bytes in 28 blocks are definitely lost in loss record 633 of 669
   at 0x4A0645D: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
   by 0x1F08227: xmalloc (xmalloc.c:147)
   by 0x1F083E6: xvasprintf (xvasprintf.c:58)
   by 0x1E7EC7D: build_message_string(char const*, ...) (diagnostic.c:78)
   by 0x1E7F438: diagnostic_get_location_text(diagnostic_context*, expanded_location) (diagnostic.c:328)
   by 0x1E7FD54: default_diagnostic_start_span_fn(diagnostic_context*, expanded_location) (diagnostic.c:626)
   by 0x1EB3508: selftest::test_diagnostic_context::start_span_cb(diagnostic_context*, expanded_location) (selftest-diagnostic.c:57)
   by 0x1E89215: diagnostic_show_locus(diagnostic_context*, rich_location*, diagnostic_t) (diagnostic-show-locus.c:1992)
   by 0x1E8B373: selftest::test_diagnostic_show_locus_fixit_lines(selftest::line_table_case const&) (diagnostic-show-locus.c:2500)
   by 0x1EB0606: selftest::for_each_line_table_case(void (*)(selftest::line_table_case const&)) (input.c:3525)
   by 0x1E8F3B9: selftest::diagnostic_show_locus_c_tests() (diagnostic-show-locus.c:3159)
   by 0x1E010BF: selftest::run_tests() (selftest-run-tests.c:88)

This patch fixes the leaks by ensuring that the pretty_printer "owns"
the prefix if it's non-NULL, freeing it in the dtor and in pp_set_prefix.

gcc/cp/ChangeLog:
	Backport of r263275 from trunk.
	2018-08-02  David Malcolm  <dmalcolm@redhat.com>

	* error.c (cxx_print_error_function): Duplicate "file" before
	passing it to pp_set_prefix.
	(cp_print_error_function): Use pp_take_prefix when saving the
	existing prefix.

gcc/ChangeLog:
	Backport of r263275 from trunk.
	2018-08-02  David Malcolm  <dmalcolm@redhat.com>

	* diagnostic-show-locus.c (diagnostic_show_locus): Use
	pp_take_prefix when saving the existing prefix.
	* diagnostic.c (diagnostic_append_note): Likewise.
	* langhooks.c (lhd_print_error_function): Likewise.
	* pretty-print.c (pp_set_prefix): Drop the "const" from "prefix"
	param's type.  Free the existing prefix.
	(pp_take_prefix): New function.
	(pretty_printer::pretty_printer): Drop the prefix parameter.
	Rename the length parameter to match the comment.
	(pretty_printer::~pretty_printer): Free the prefix.
	* pretty-print.h (pretty_printer::pretty_printer): Drop the prefix
	parameter.
	(struct pretty_printer): Drop the "const" from "prefix" field's
	type and clarify memory management.
	(pp_set_prefix): Drop the "const" from the 2nd param.
	(pp_take_prefix): New decl.



git-svn-id: https://gcc.gnu.org/svn/gcc/branches/gcc-8-branch@268910 138bc75d-0d04-0410-961f-82ee72b054a4

8 files changed, 73 insertions, 20 deletions

diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index 6a188fa2317..5992288f81f 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,25 @@
+2019-02-14  David Malcolm  <dmalcolm@redhat.com>
+
+	Backport of r263275 from trunk.
+	2018-08-02  David Malcolm  <dmalcolm@redhat.com>
+
+	* diagnostic-show-locus.c (diagnostic_show_locus): Use
+	pp_take_prefix when saving the existing prefix.
+	* diagnostic.c (diagnostic_append_note): Likewise.
+	* langhooks.c (lhd_print_error_function): Likewise.
+	* pretty-print.c (pp_set_prefix): Drop the "const" from "prefix"
+	param's type.  Free the existing prefix.
+	(pp_take_prefix): New function.
+	(pretty_printer::pretty_printer): Drop the prefix parameter.
+	Rename the length parameter to match the comment.
+	(pretty_printer::~pretty_printer): Free the prefix.
+	* pretty-print.h (pretty_printer::pretty_printer): Drop the prefix
+	parameter.
+	(struct pretty_printer): Drop the "const" from "prefix" field's
+	type and clarify memory management.
+	(pp_set_prefix): Drop the "const" from the 2nd param.
+	(pp_take_prefix): New decl.
+
 2019-02-14  Segher Boessenkool  <segher@kernel.crashing.org>
 
 	Backport from trunk
diff --git a/gcc/cp/error.c b/gcc/cp/error.c
index 95b8b84f34a..f7895dee64a 100644
--- a/gcc/cp/error.c
+++ b/gcc/cp/error.c
@@ -3286,8 +3286,13 @@ void
 cxx_print_error_function (diagnostic_context *context, const char *file,
 			  diagnostic_info *diagnostic)
 {
+  char *prefix;
+  if (file)
+    prefix = xstrdup (file);
+  else
+    prefix = NULL;
   lhd_print_error_function (context, file, diagnostic);
-  pp_set_prefix (context->printer, file);
+  pp_set_prefix (context->printer, prefix);
   maybe_print_instantiation_context (context);
 }
 
@@ -3315,7 +3320,7 @@ cp_print_error_function (diagnostic_context *context,
     return;
   if (diagnostic_last_function_changed (context, diagnostic))
     {
-      const char *old_prefix = context->printer->prefix;
+      char *old_prefix = pp_take_prefix (context->printer);
       const char *file = LOCATION_FILE (diagnostic_location (diagnostic));
       tree abstract_origin = diagnostic_abstract_origin (diagnostic);
       char *new_prefix = (file && abstract_origin == NULL)
diff --git a/gcc/diagnostic-show-locus.c b/gcc/diagnostic-show-locus.c
index bdf608a08bc..9c567bde68d 100644
--- a/gcc/diagnostic-show-locus.c
+++ b/gcc/diagnostic-show-locus.c
@@ -2000,7 +2000,7 @@ diagnostic_show_locus (diagnostic_context * context,
 
   context->last_location = loc;
 
-  const char *saved_prefix = pp_get_prefix (context->printer);
+  char *saved_prefix = pp_take_prefix (context->printer);
   pp_set_prefix (context->printer, NULL);
 
   layout layout (context, richloc, diagnostic_kind);
diff --git a/gcc/diagnostic.c b/gcc/diagnostic.c
index e22c17bc02c..c61e0c4572a 100644
--- a/gcc/diagnostic.c
+++ b/gcc/diagnostic.c
@@ -1063,7 +1063,6 @@ diagnostic_append_note (diagnostic_context *context,
 {
   diagnostic_info diagnostic;
   va_list ap;
-  const char *saved_prefix;
   rich_location richloc (line_table, location);
 
   va_start (ap, gmsgid);
@@ -1073,7 +1072,7 @@ diagnostic_append_note (diagnostic_context *context,
       va_end (ap);
       return;
     }
-  saved_prefix = pp_get_prefix (context->printer);
+  char *saved_prefix = pp_take_prefix (context->printer);
   pp_set_prefix (context->printer,
                  diagnostic_build_prefix (context, &diagnostic));
   pp_format (context->printer, &diagnostic.message);
diff --git a/gcc/langhooks.c b/gcc/langhooks.c
index 3cd790105b8..b24c87a8a88 100644
--- a/gcc/langhooks.c
+++ b/gcc/langhooks.c
@@ -368,7 +368,7 @@ lhd_print_error_function (diagnostic_context *context, const char *file,
 {
   if (diagnostic_last_function_changed (context, diagnostic))
     {
-      const char *old_prefix = context->printer->prefix;
+      char *old_prefix = pp_take_prefix (context->printer);
       tree abstract_origin = diagnostic_abstract_origin (diagnostic);
       char *new_prefix = (file && abstract_origin == NULL)
 			 ? file_name_as_prefix (context, file) : NULL;
diff --git a/gcc/pretty-print.c b/gcc/pretty-print.c
index da0781ea77c..6243aed3dac 100644
--- a/gcc/pretty-print.c
+++ b/gcc/pretty-print.c
@@ -1491,23 +1491,38 @@ pp_clear_output_area (pretty_printer *pp)
   pp_buffer (pp)->line_length = 0;
 }
 
-/* Set PREFIX for PRETTY-PRINTER.  */
+/* Set PREFIX for PRETTY-PRINTER, taking ownership of PREFIX, which
+   will eventually be free-ed.  */
+
 void
-pp_set_prefix (pretty_printer *pp, const char *prefix)
+pp_set_prefix (pretty_printer *pp, char *prefix)
 {
+  free (pp->prefix);
   pp->prefix = prefix;
   pp_set_real_maximum_length (pp);
   pp->emitted_prefix = false;
   pp_indentation (pp) = 0;
 }
 
+/* Take ownership of PP's prefix, setting it to NULL.
+   This allows clients to save, overide, and then restore an existing
+   prefix, without it being free-ed.  */
+
+char *
+pp_take_prefix (pretty_printer *pp)
+{
+  char *result = pp->prefix;
+  pp->prefix = NULL;
+  return result;
+}
+
 /* Free PRETTY-PRINTER's prefix, a previously malloc()'d string.  */
 void
 pp_destroy_prefix (pretty_printer *pp)
 {
   if (pp->prefix != NULL)
     {
-      free (CONST_CAST (char *, pp->prefix));
+      free (pp->prefix);
       pp->prefix = NULL;
     }
 }
@@ -1544,10 +1559,9 @@ pp_emit_prefix (pretty_printer *pp)
     }
 }
 
-/* Construct a PRETTY-PRINTER with PREFIX and of MAXIMUM_LENGTH
-   characters per line.  */
+/* Construct a PRETTY-PRINTER of MAXIMUM_LENGTH characters per line.  */
 
-pretty_printer::pretty_printer (const char *p, int l)
+pretty_printer::pretty_printer (int maximum_length)
   : buffer (new (XCNEW (output_buffer)) output_buffer ()),
     prefix (),
     padding (pp_none),
@@ -1561,10 +1575,10 @@ pretty_printer::pretty_printer (const char *p, int l)
     translate_identifiers (true),
     show_color ()
 {
-  pp_line_cutoff (this) = l;
+  pp_line_cutoff (this) = maximum_length;
   /* By default, we emit prefixes once per message.  */
   pp_prefixing_rule (this) = DIAGNOSTICS_SHOW_PREFIX_ONCE;
-  pp_set_prefix (this, p);
+  pp_set_prefix (this, NULL);
 }
 
 pretty_printer::~pretty_printer ()
@@ -1573,6 +1587,7 @@ pretty_printer::~pretty_printer ()
     delete m_format_postprocessor;
   buffer->~output_buffer ();
   XDELETE (buffer);
+  free (prefix);
 }
 
 /* Append a string delimited by START and END to the output area of
diff --git a/gcc/pretty-print.h b/gcc/pretty-print.h
index 56abe03e46b..0d67e308050 100644
--- a/gcc/pretty-print.h
+++ b/gcc/pretty-print.h
@@ -215,17 +215,18 @@ class format_postprocessor
    and add additional fields they need.  */
 struct pretty_printer
 {
-  // Default construct a pretty printer with specified prefix
-  // and a maximum line length cut off limit.
-  explicit pretty_printer (const char* = NULL, int = 0);
+  /* Default construct a pretty printer with specified
+     maximum line length cut off limit.  */
+  explicit pretty_printer (int = 0);
 
   virtual ~pretty_printer ();
 
   /* Where we print external representation of ENTITY.  */
   output_buffer *buffer;
 
-  /* The prefix for each new line.  */
-  const char *prefix;
+  /* The prefix for each new line.  If non-NULL, this is "owned" by the
+     pretty_printer, and will eventually be free-ed.  */
+  char *prefix;
 
   /* Where to put whitespace around the entity being formatted.  */
   pp_padding padding;
@@ -338,7 +339,8 @@ pp_get_prefix (const pretty_printer *pp) { return pp->prefix; }
 #define pp_buffer(PP) (PP)->buffer
 
 extern void pp_set_line_maximum_length (pretty_printer *, int);
-extern void pp_set_prefix (pretty_printer *, const char *);
+extern void pp_set_prefix (pretty_printer *, char *);
+extern char *pp_take_prefix (pretty_printer *);
 extern void pp_destroy_prefix (pretty_printer *);
 extern int pp_remaining_character_count_for_line (pretty_printer *);
 extern void pp_clear_output_area (pretty_printer *);
diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
index 84e11ce4b17..729364e1690 100644
--- a/gcc/testsuite/ChangeLog
+++ b/gcc/testsuite/ChangeLog
@@ -1,5 +1,15 @@
 2019-02-14  David Malcolm  <dmalcolm@redhat.com>
 
+	Backport of r263275 from trunk.
+	2018-08-02  David Malcolm  <dmalcolm@redhat.com>
+
+	* error.c (cxx_print_error_function): Duplicate "file" before
+	passing it to pp_set_prefix.
+	(cp_print_error_function): Use pp_take_prefix when saving the
+	existing prefix.
+
+2019-02-14  David Malcolm  <dmalcolm@redhat.com>
+
 	Backport of r262199 from trunk.
 	2018-06-27  David Malcolm  <dmalcolm@redhat.com>