Commit 653fee19 by David Malcolm Committed by David Malcolm

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:
	* 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:
	* 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.

From-SVN: r263275
parent 74f9986e
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.
2018-08-02 Aaron Sawdey <acsawdey@linux.ibm.com> 2018-08-02 Aaron Sawdey <acsawdey@linux.ibm.com>
* config/rs6000/rs6000-string.c (select_block_compare_mode): Move test * config/rs6000/rs6000-string.c (select_block_compare_mode): Move test
......
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.
2018-08-01 Martin Sebor <msebor@redhat.com> 2018-08-01 Martin Sebor <msebor@redhat.com>
PR tree-optimization/86650 PR tree-optimization/86650
......
...@@ -3287,8 +3287,13 @@ void ...@@ -3287,8 +3287,13 @@ void
cxx_print_error_function (diagnostic_context *context, const char *file, cxx_print_error_function (diagnostic_context *context, const char *file,
diagnostic_info *diagnostic) diagnostic_info *diagnostic)
{ {
char *prefix;
if (file)
prefix = xstrdup (file);
else
prefix = NULL;
lhd_print_error_function (context, file, diagnostic); lhd_print_error_function (context, file, diagnostic);
pp_set_prefix (context->printer, file); pp_set_prefix (context->printer, prefix);
maybe_print_instantiation_context (context); maybe_print_instantiation_context (context);
} }
...@@ -3316,7 +3321,7 @@ cp_print_error_function (diagnostic_context *context, ...@@ -3316,7 +3321,7 @@ cp_print_error_function (diagnostic_context *context,
return; return;
if (diagnostic_last_function_changed (context, diagnostic)) 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)); const char *file = LOCATION_FILE (diagnostic_location (diagnostic));
tree abstract_origin = diagnostic_abstract_origin (diagnostic); tree abstract_origin = diagnostic_abstract_origin (diagnostic);
char *new_prefix = (file && abstract_origin == NULL) char *new_prefix = (file && abstract_origin == NULL)
......
...@@ -1978,7 +1978,7 @@ diagnostic_show_locus (diagnostic_context * context, ...@@ -1978,7 +1978,7 @@ diagnostic_show_locus (diagnostic_context * context,
context->last_location = loc; 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); pp_set_prefix (context->printer, NULL);
layout layout (context, richloc, diagnostic_kind); layout layout (context, richloc, diagnostic_kind);
......
...@@ -1063,7 +1063,6 @@ diagnostic_append_note (diagnostic_context *context, ...@@ -1063,7 +1063,6 @@ diagnostic_append_note (diagnostic_context *context,
{ {
diagnostic_info diagnostic; diagnostic_info diagnostic;
va_list ap; va_list ap;
const char *saved_prefix;
rich_location richloc (line_table, location); rich_location richloc (line_table, location);
va_start (ap, gmsgid); va_start (ap, gmsgid);
...@@ -1073,7 +1072,7 @@ diagnostic_append_note (diagnostic_context *context, ...@@ -1073,7 +1072,7 @@ diagnostic_append_note (diagnostic_context *context,
va_end (ap); va_end (ap);
return; return;
} }
saved_prefix = pp_get_prefix (context->printer); char *saved_prefix = pp_take_prefix (context->printer);
pp_set_prefix (context->printer, pp_set_prefix (context->printer,
diagnostic_build_prefix (context, &diagnostic)); diagnostic_build_prefix (context, &diagnostic));
pp_format (context->printer, &diagnostic.message); pp_format (context->printer, &diagnostic.message);
......
...@@ -369,7 +369,7 @@ lhd_print_error_function (diagnostic_context *context, const char *file, ...@@ -369,7 +369,7 @@ lhd_print_error_function (diagnostic_context *context, const char *file,
{ {
if (diagnostic_last_function_changed (context, diagnostic)) 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); tree abstract_origin = diagnostic_abstract_origin (diagnostic);
char *new_prefix = (file && abstract_origin == NULL) char *new_prefix = (file && abstract_origin == NULL)
? file_name_as_prefix (context, file) : NULL; ? file_name_as_prefix (context, file) : NULL;
......
...@@ -1482,23 +1482,38 @@ pp_clear_output_area (pretty_printer *pp) ...@@ -1482,23 +1482,38 @@ pp_clear_output_area (pretty_printer *pp)
pp_buffer (pp)->line_length = 0; 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 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->prefix = prefix;
pp_set_real_maximum_length (pp); pp_set_real_maximum_length (pp);
pp->emitted_prefix = false; pp->emitted_prefix = false;
pp_indentation (pp) = 0; 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. */ /* Free PRETTY-PRINTER's prefix, a previously malloc()'d string. */
void void
pp_destroy_prefix (pretty_printer *pp) pp_destroy_prefix (pretty_printer *pp)
{ {
if (pp->prefix != NULL) if (pp->prefix != NULL)
{ {
free (CONST_CAST (char *, pp->prefix)); free (pp->prefix);
pp->prefix = NULL; pp->prefix = NULL;
} }
} }
...@@ -1535,10 +1550,9 @@ pp_emit_prefix (pretty_printer *pp) ...@@ -1535,10 +1550,9 @@ pp_emit_prefix (pretty_printer *pp)
} }
} }
/* Construct a PRETTY-PRINTER with PREFIX and of MAXIMUM_LENGTH /* Construct a PRETTY-PRINTER of MAXIMUM_LENGTH characters per line. */
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 ()), : buffer (new (XCNEW (output_buffer)) output_buffer ()),
prefix (), prefix (),
padding (pp_none), padding (pp_none),
...@@ -1552,10 +1566,10 @@ pretty_printer::pretty_printer (const char *p, int l) ...@@ -1552,10 +1566,10 @@ pretty_printer::pretty_printer (const char *p, int l)
translate_identifiers (true), translate_identifiers (true),
show_color () show_color ()
{ {
pp_line_cutoff (this) = l; pp_line_cutoff (this) = maximum_length;
/* By default, we emit prefixes once per message. */ /* By default, we emit prefixes once per message. */
pp_prefixing_rule (this) = DIAGNOSTICS_SHOW_PREFIX_ONCE; pp_prefixing_rule (this) = DIAGNOSTICS_SHOW_PREFIX_ONCE;
pp_set_prefix (this, p); pp_set_prefix (this, NULL);
} }
pretty_printer::~pretty_printer () pretty_printer::~pretty_printer ()
...@@ -1564,6 +1578,7 @@ pretty_printer::~pretty_printer () ...@@ -1564,6 +1578,7 @@ pretty_printer::~pretty_printer ()
delete m_format_postprocessor; delete m_format_postprocessor;
buffer->~output_buffer (); buffer->~output_buffer ();
XDELETE (buffer); XDELETE (buffer);
free (prefix);
} }
/* Append a string delimited by START and END to the output area of /* Append a string delimited by START and END to the output area of
......
...@@ -215,17 +215,18 @@ class format_postprocessor ...@@ -215,17 +215,18 @@ class format_postprocessor
and add additional fields they need. */ and add additional fields they need. */
struct pretty_printer struct pretty_printer
{ {
// Default construct a pretty printer with specified prefix /* Default construct a pretty printer with specified
// and a maximum line length cut off limit. maximum line length cut off limit. */
explicit pretty_printer (const char* = NULL, int = 0); explicit pretty_printer (int = 0);
virtual ~pretty_printer (); virtual ~pretty_printer ();
/* Where we print external representation of ENTITY. */ /* Where we print external representation of ENTITY. */
output_buffer *buffer; output_buffer *buffer;
/* The prefix for each new line. */ /* The prefix for each new line. If non-NULL, this is "owned" by the
const char *prefix; pretty_printer, and will eventually be free-ed. */
char *prefix;
/* Where to put whitespace around the entity being formatted. */ /* Where to put whitespace around the entity being formatted. */
pp_padding padding; pp_padding padding;
...@@ -338,7 +339,8 @@ pp_get_prefix (const pretty_printer *pp) { return pp->prefix; } ...@@ -338,7 +339,8 @@ pp_get_prefix (const pretty_printer *pp) { return pp->prefix; }
#define pp_buffer(PP) (PP)->buffer #define pp_buffer(PP) (PP)->buffer
extern void pp_set_line_maximum_length (pretty_printer *, int); 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 void pp_destroy_prefix (pretty_printer *);
extern int pp_remaining_character_count_for_line (pretty_printer *); extern int pp_remaining_character_count_for_line (pretty_printer *);
extern void pp_clear_output_area (pretty_printer *); extern void pp_clear_output_area (pretty_printer *);
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment