From eb095650011f68858e877e3590f6901bc79ba668 Mon Sep 17 00:00:00 2001 From: Paul Eggert Date: Wed, 8 Nov 2006 20:28:57 +0000 Subject: [PATCH] Don't let Bison leak memory except when it complains. * src/files.h (parser_file_name, spec_verbose_file, spec_graph_file): (spec_defines_file, dir_prefix): Now char *, not const char *, since they are freed. * src/files.c: Likewise. (all_but_ext, all_but_tab_ext, src_extension, header_extension): Likewise. (tr): Now operates in-place. All uses changed. (compute_exts_from_gf, compute_exts_from_src): Don't leak temporary values. (compute_file_name_parts, compute_output_file_names): Don't store read-only data in variables that will be freed. (compute_output_file_names): Free all_but_ext, all_but_tab_ext, src_extension, and header_extension. (output_file_names_free): New public function to free spec_verbose_file, spec_graph_file, spec_defines_file, parser_file_name, and dir_prefix. * src/getargs.c (getargs): Don't store read-only data in variables that will be freed. * src/main.c (main): Invoke output_file_names_free, code_scanner_free (which previously existed but was unused), and quotearg_free. * src/muscle_tab.h (muscle_insert): value arg is now a `char const *'. * src/muscle_tab.c: Likewise. (muscle_entry): Make the value char const *, and add a new storage member that is char * and can be freed. (muscle_entry_free): New private function. (muscle_init): Use it instead of free. (muscle_insert, muscle_grow): Update and use new storage member. (muscle_code_grow): Free the string passed to muscle_grow since it's not needed anymore. * src/parse-gram.y (%union): Make `chars' member a `char const *', and add a new `char *code' member. ("{...}"): Declare semantic type as code. * src/scan-code.h (translate_rule_action): (translate_symbol_action, translate_code, translate_action): Return `char const *' rather than `char *' since external code should not free these strings. * src/scan-code.l: Likewise. * src/scan-gram.l (): Use val->code for BRACED_CODE, which is "{...}" in the parser. * tests/Makefile.am (maintainer-check-valgrind): Set VALGRIND_OPTS='--leak-check=full --show-reacheable=yes' before invoking Valgrind. * tests/calc.at (_AT_DATA_CALC_Y): fclose the FILE* so Valgrind doesn't complain. * tests/testsuite.at (AT_CHECK): Redefine so that running Bison and expecting a non-zero exit status sets --leak-check=summary and --show-reachable=no for Valgrind. Bison unabashedly leaks memory in this case, and we don't want to hear about it. --- ChangeLog | 53 ++++++++++++++++++++++++++++++ src/files.c | 80 ++++++++++++++++++++++++++-------------------- src/files.h | 11 ++++--- src/getargs.c | 6 ++-- src/main.c | 5 +++ src/muscle_tab.c | 29 ++++++++++++----- src/muscle_tab.h | 6 ++-- src/parse-gram.y | 15 +++++---- src/scan-code.h | 6 ++-- src/scan-code.l | 14 ++++---- src/scan-gram.l | 4 +-- tests/Makefile.am | 1 + tests/calc.at | 1 + tests/testsuite.at | 18 +++++++++-- 14 files changed, 174 insertions(+), 75 deletions(-) diff --git a/ChangeLog b/ChangeLog index 1a7d9090..451c9d00 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,56 @@ +2006-11-08 Joel E. Denny + and Paul Eggert + + Don't let Bison leak memory except when it complains. + * src/files.h (parser_file_name, spec_verbose_file, spec_graph_file): + (spec_defines_file, dir_prefix): Now char *, not const char *, + since they are freed. + * src/files.c: Likewise. + (all_but_ext, all_but_tab_ext, src_extension, header_extension): + Likewise. + (tr): Now operates in-place. All uses changed. + (compute_exts_from_gf, compute_exts_from_src): Don't leak temporary + values. + (compute_file_name_parts, compute_output_file_names): Don't store + read-only data in variables that will be freed. + (compute_output_file_names): Free all_but_ext, all_but_tab_ext, + src_extension, and header_extension. + (output_file_names_free): New public function to free + spec_verbose_file, spec_graph_file, spec_defines_file, + parser_file_name, and dir_prefix. + * src/getargs.c (getargs): Don't store read-only data in variables that + will be freed. + * src/main.c (main): Invoke output_file_names_free, code_scanner_free + (which previously existed but was unused), and quotearg_free. + * src/muscle_tab.h (muscle_insert): value arg is now a `char const *'. + * src/muscle_tab.c: Likewise. + (muscle_entry): Make the value char const *, + and add a new storage member that is char * and can be freed. + (muscle_entry_free): New private function. + (muscle_init): Use it instead of free. + (muscle_insert, muscle_grow): Update and use new storage member. + (muscle_code_grow): Free the string passed to muscle_grow + since it's not needed anymore. + * src/parse-gram.y (%union): Make `chars' member a `char const *', and + add a new `char *code' member. + ("{...}"): Declare semantic type as code. + * src/scan-code.h (translate_rule_action): + (translate_symbol_action, translate_code, translate_action): Return + `char const *' rather than `char *' since external code should not free + these strings. + * src/scan-code.l: Likewise. + * src/scan-gram.l (): Use val->code for BRACED_CODE, + which is "{...}" in the parser. + * tests/Makefile.am (maintainer-check-valgrind): Set + VALGRIND_OPTS='--leak-check=full --show-reacheable=yes' before invoking + Valgrind. + * tests/calc.at (_AT_DATA_CALC_Y): fclose the FILE* so Valgrind doesn't + complain. + * tests/testsuite.at (AT_CHECK): Redefine so that running Bison and + expecting a non-zero exit status sets --leak-check=summary and + --show-reachable=no for Valgrind. Bison unabashedly leaks memory in + this case, and we don't want to hear about it. + 2006-11-08 Paul Eggert * bootstrap (runtime-po/Makevars): Derive from po/Makevars diff --git a/src/files.c b/src/files.c index ecb31738..951a7acc 100644 --- a/src/files.c +++ b/src/files.c @@ -48,10 +48,10 @@ struct obstack post_prologue_obstack; char const *spec_outfile = NULL; /* for -o. */ char const *spec_file_prefix = NULL; /* for -b. */ char const *spec_name_prefix = NULL; /* for -p. */ -char const *spec_verbose_file = NULL; /* for --verbose. */ -char const *spec_graph_file = NULL; /* for -g. */ -char const *spec_defines_file = NULL; /* for --defines. */ -char const *parser_file_name; +char *spec_verbose_file = NULL; /* for --verbose. */ +char *spec_graph_file = NULL; /* for -g. */ +char *spec_defines_file = NULL; /* for --defines. */ +char *parser_file_name; uniqstr grammar_file = NULL; uniqstr current_file = NULL; @@ -72,14 +72,14 @@ uniqstr current_file = NULL; empty string (meaning the current directory); otherwise it is `dir/'. */ -static char const *all_but_ext; -static char const *all_but_tab_ext; -char const *dir_prefix; +static char *all_but_ext; +static char *all_but_tab_ext; +char *dir_prefix; /* C source file extension (the parser source). */ -static char const *src_extension = NULL; +static char *src_extension = NULL; /* Header file extension (if option ``-d'' is specified). */ -static char const *header_extension = NULL; +static char *header_extension = NULL; /*-----------------------------------------------------------------. | Return a newly allocated string composed of the concatenation of | @@ -136,31 +136,25 @@ xfclose (FILE *ptr) | Compute ALL_BUT_EXT, ALL_BUT_TAB_EXT and output files extensions. | `------------------------------------------------------------------*/ -/* Replace all characters FROM by TO in the string IN. - and returns a new allocated string. */ +/* In the string S, replace all characters FROM by TO. */ static char * -tr (const char *in, char from, char to) +tr (char *s, char from, char to) { - char *temp; - char *out = xmalloc (strlen (in) + 1); - - for (temp = out; *in; in++, out++) - if (*in == from) - *out = to; - else - *out = *in; - *out = 0; - return (temp); + for (; *s; s++) + if (*s == from) + *s = to; } /* Compute extensions from the grammar file extension. */ static void compute_exts_from_gf (const char *ext) { - src_extension = tr (ext, 'y', 'c'); - src_extension = tr (src_extension, 'Y', 'C'); - header_extension = tr (ext, 'y', 'h'); - header_extension = tr (header_extension, 'Y', 'H'); + src_extension = xstrdup (ext); + header_extension = xstrdup (ext); + tr (src_extension, 'y', 'c'); + tr (src_extension, 'Y', 'C'); + tr (header_extension, 'y', 'h'); + tr (header_extension, 'Y', 'H'); } /* Compute extensions from the given c source file extension. */ @@ -171,8 +165,9 @@ compute_exts_from_src (const char *ext) so the extenions must be computed unconditionally from the file name given by this option. */ src_extension = xstrdup (ext); - header_extension = tr (ext, 'c', 'h'); - header_extension = tr (header_extension, 'C', 'H'); + header_extension = xstrdup (ext); + tr (header_extension, 'c', 'h'); + tr (header_extension, 'C', 'H'); } @@ -270,14 +265,14 @@ compute_file_name_parts (void) else if (yacc_flag) { /* If --yacc, then the output is `y.tab.c'. */ - dir_prefix = ""; - all_but_tab_ext = "y"; + dir_prefix = xstrdup (""); + all_but_tab_ext = xstrdup ("y"); } else { /* Otherwise, ALL_BUT_TAB_EXT is computed from the input grammar: `foo/bar.yy' => `bar'. */ - dir_prefix = ""; + dir_prefix = xstrdup (""); all_but_tab_ext = xstrndup (base, (strlen (base) - (ext ? strlen (ext) : 0))); } @@ -306,12 +301,14 @@ compute_output_file_names (void) /* If not yet done. */ if (!src_extension) - src_extension = ".c"; + src_extension = xstrdup (".c"); if (!header_extension) - header_extension = ".h"; + header_extension = xstrdup (".h"); name[names++] = parser_file_name = - spec_outfile ? spec_outfile : concat2 (all_but_ext, src_extension); + (spec_outfile + ? xstrdup (spec_outfile) + : concat2 (all_but_ext, src_extension)); if (defines_flag) { @@ -337,4 +334,19 @@ compute_output_file_names (void) for (i = 0; i < j; i++) if (strcmp (name[i], name[j]) == 0) warn (_("conflicting outputs to file %s"), quote (name[i])); + + free (all_but_ext); + free (all_but_tab_ext); + free (src_extension); + free (header_extension); +} + +void +output_file_names_free (void) +{ + free (spec_verbose_file); + free (spec_graph_file); + free (spec_defines_file); + free (parser_file_name); + free (dir_prefix); } diff --git a/src/files.h b/src/files.h index e8df14e7..e702db65 100644 --- a/src/files.h +++ b/src/files.h @@ -29,7 +29,7 @@ extern char const *spec_outfile; /* File name for the parser (i.e., the one above, or its default.) */ -extern char const *parser_file_name; +extern char *parser_file_name; /* Symbol prefix specified with -p, or 0 if no -p. */ extern const char *spec_name_prefix; @@ -38,16 +38,16 @@ extern const char *spec_name_prefix; extern char const *spec_file_prefix; /* --verbose. */ -extern char const *spec_verbose_file; +extern char *spec_verbose_file; /* File name specified for the output graph. */ -extern char const *spec_graph_file; +extern char *spec_graph_file; /* File name specified with --defines. */ -extern char const *spec_defines_file; +extern char *spec_defines_file; /* Directory prefix of output file names. */ -extern char const *dir_prefix; +extern char *dir_prefix; /* If semantic parser, output a .h file that defines YYSTYPE... */ @@ -63,6 +63,7 @@ extern uniqstr grammar_file; extern uniqstr current_file; void compute_output_file_names (void); +void output_file_names_free (void); FILE *xfopen (const char *name, const char *mode); void xfclose (FILE *ptr); diff --git a/src/getargs.c b/src/getargs.c index 177255e7..9a2f5307 100644 --- a/src/getargs.c +++ b/src/getargs.c @@ -98,7 +98,7 @@ flags_argmatch (const char *option, *flags = 0; else *flags |= value; - args = strtok (NULL, ","); + args = strtok (NULL, ","); } } else @@ -408,7 +408,7 @@ getargs (int argc, char *argv[]) /* Here, the -g and --graph=FILE options are differentiated. */ graph_flag = true; if (optarg) - spec_graph_file = AS_FILE_NAME (optarg); + spec_graph_file = xstrdup (AS_FILE_NAME (optarg)); break; case 'h': @@ -426,7 +426,7 @@ getargs (int argc, char *argv[]) /* Here, the -d and --defines options are differentiated. */ defines_flag = true; if (optarg) - spec_defines_file = AS_FILE_NAME (optarg); + spec_defines_file = xstrdup (AS_FILE_NAME (optarg)); break; case 'k': diff --git a/src/main.c b/src/main.c index a6fafc25..a2056c82 100644 --- a/src/main.c +++ b/src/main.c @@ -26,6 +26,7 @@ #include #include #include +#include #include #include "LR0.h" @@ -43,6 +44,7 @@ #include "print_graph.h" #include "reader.h" #include "reduce.h" +#include "scan-code.h" #include "scan-gram.h" #include "symtab.h" #include "tables.h" @@ -168,12 +170,15 @@ main (int argc, char *argv[]) reduce_free (); conflicts_free (); grammar_free (); + output_file_names_free (); /* The scanner memory cannot be released right after parsing, as it contains things such as user actions, prologue, epilogue etc. */ gram_scanner_free (); muscle_free (); uniqstrs_free (); + code_scanner_free (); + quotearg_free (); timevar_pop (TV_FREE); if (trace_flag & trace_bitsets) diff --git a/src/muscle_tab.c b/src/muscle_tab.c index 2d71085e..6ceb71f3 100644 --- a/src/muscle_tab.c +++ b/src/muscle_tab.c @@ -1,6 +1,6 @@ /* Muscle table manager for Bison. - Copyright (C) 2001, 2002, 2003, 2004, 2005 Free Software + Copyright (C) 2001, 2002, 2003, 2004, 2005, 2006 Free Software Foundation, Inc. This file is part of Bison, the GNU Compiler Compiler. @@ -30,10 +30,13 @@ #include "muscle_tab.h" #include "getargs.h" +/* A key-value pair, along with storage that can be reclaimed when + this pair is no longer needed. */ typedef struct { - const char *key; - char *value; + char const *key; + char const *value; + char *storage; } muscle_entry; /* An obstack used to create some entries. */ @@ -64,6 +67,14 @@ hash_muscle (const void *x, size_t tablesize) | Also set up the MUSCLE_OBSTACK. | `-----------------------------------------------------------------*/ +static void +muscle_entry_free (void *entry) +{ + muscle_entry *mentry = entry; + free (mentry->storage); + free (mentry); +} + void muscle_init (void) { @@ -71,7 +82,7 @@ muscle_init (void) obstack_init (&muscle_obstack); muscle_table = hash_initialize (HT_INITIAL_CAPACITY, NULL, hash_muscle, - hash_compare_muscles, free); + hash_compare_muscles, muscle_entry_free); /* Version and input file. */ MUSCLE_INSERT_STRING ("version", VERSION); @@ -98,7 +109,7 @@ muscle_free (void) `------------------------------------------------------------*/ void -muscle_insert (const char *key, char *value) +muscle_insert (char const *key, char const *value) { muscle_entry probe; muscle_entry *entry; @@ -112,6 +123,7 @@ muscle_insert (const char *key, char *value) entry = xmalloc (sizeof *entry); entry->key = key; hash_insert (muscle_table, entry); + entry->storage = NULL; } entry->value = value; } @@ -138,19 +150,19 @@ muscle_grow (const char *key, const char *val, const char *separator) entry = xmalloc (sizeof *entry); entry->key = key; hash_insert (muscle_table, entry); - entry->value = xstrdup (val); + entry->value = entry->storage = xstrdup (val); } else { /* Grow the current value. */ char *new_val; obstack_sgrow (&muscle_obstack, entry->value); - free (entry->value); + free (entry->storage); obstack_sgrow (&muscle_obstack, separator); obstack_sgrow (&muscle_obstack, val); obstack_1grow (&muscle_obstack, 0); new_val = obstack_finish (&muscle_obstack); - entry->value = xstrdup (new_val); + entry->value = entry->storage = xstrdup (new_val); obstack_free (&muscle_obstack, new_val); } } @@ -173,6 +185,7 @@ muscle_code_grow (const char *key, const char *val, location loc) obstack_1grow (&muscle_obstack, 0); extension = obstack_finish (&muscle_obstack); muscle_grow (key, extension, ""); + obstack_free (&muscle_obstack, extension); } diff --git a/src/muscle_tab.h b/src/muscle_tab.h index cffdf1bd..32989bc7 100644 --- a/src/muscle_tab.h +++ b/src/muscle_tab.h @@ -1,5 +1,5 @@ /* Muscle table manager for Bison, - Copyright (C) 2001, 2002, 2003 Free Software Foundation, Inc. + Copyright (C) 2001, 2002, 2003, 2006 Free Software Foundation, Inc. This file is part of Bison, the GNU Compiler Compiler. @@ -24,8 +24,8 @@ # include "location.h" void muscle_init (void); -void muscle_insert (const char *key, char *value); -char *muscle_find (const char *key); +void muscle_insert (char const *key, char const *value); +char *muscle_find (char const *key); void muscle_free (void); diff --git a/src/parse-gram.y b/src/parse-gram.y index 04265596..522540ac 100644 --- a/src/parse-gram.y +++ b/src/parse-gram.y @@ -93,13 +93,13 @@ static int current_prec = 0; boundary_set (&@$.end, current_file, 1, 1); } -/* Only NUMBERS have a value. */ %union { symbol *symbol; symbol_list *list; int integer; - char *chars; + char const *chars; + char *code; assoc assoc; uniqstr uniqstr; unsigned char character; @@ -183,11 +183,12 @@ static int current_prec = 0; /* braceless is not to be used for rule or symbol actions, as it calls translate_code. */ -%type STRING "{...}" "%{...%}" EPILOGUE braceless content content.opt +%type STRING "%{...%}" EPILOGUE braceless content content.opt +%type "{...}" %printer { fputs (quotearg_style (c_quoting_style, $$), stderr); } - STRING + STRING %printer { fprintf (stderr, "{\n%s\n}", $$); } - braceless content content.opt "{...}" "%{...%}" EPILOGUE + braceless content content.opt "{...}" "%{...%}" EPILOGUE %type TYPE ID ID_COLON %printer { fprintf (stderr, "<%s>", $$); } TYPE @@ -267,7 +268,7 @@ grammar_declaration: symbol_list *list; const char *action = translate_symbol_action ($2, @2); for (list = $3; list; list = list->next) - symbol_list_destructor_set (list, action, @2); + symbol_list_destructor_set (list, action, @2); symbol_list_free ($3); } | "%printer" "{...}" generic_symlist @@ -275,7 +276,7 @@ grammar_declaration: symbol_list *list; const char *action = translate_symbol_action ($2, @2); for (list = $3; list; list = list->next) - symbol_list_printer_set (list, action, @2); + symbol_list_printer_set (list, action, @2); symbol_list_free ($3); } | "%default-prec" diff --git a/src/scan-code.h b/src/scan-code.h index 43073220..f2d79720 100644 --- a/src/scan-code.h +++ b/src/scan-code.h @@ -35,13 +35,13 @@ void code_scanner_free (void); /* The action of the rule R contains $$, $1 etc. referring to the values of the rule R. */ -char *translate_rule_action (symbol_list *r); +char const *translate_rule_action (symbol_list *r); /* The action A refers to $$ and @$ only, referring to a symbol. */ -char *translate_symbol_action (const char *a, location l); +char const *translate_symbol_action (char const *a, location l); /* The action contains no special escapes, just protect M4 special symbols. */ -char *translate_code (const char *a, location l); +char const *translate_code (char const *a, location l); #endif /* !SCAN_CODE_H_ */ diff --git a/src/scan-code.l b/src/scan-code.l index d41c6e46..6b33e015 100644 --- a/src/scan-code.l +++ b/src/scan-code.l @@ -372,8 +372,8 @@ handle_action_at (symbol_list *rule, char *text, location at_loc) translation is for \a rule, in the context \a sc_context (SC_RULE_ACTION, SC_SYMBOL_ACTION, INITIAL). */ -static char * -translate_action (int sc_context, symbol_list *rule, const char *a, location l) +static char const * +translate_action (int sc_context, symbol_list *rule, char const *a, location l) { char *res; static bool initialized = false; @@ -394,21 +394,21 @@ translate_action (int sc_context, symbol_list *rule, const char *a, location l) return res; } -char * +char const * translate_rule_action (symbol_list *rule) { return translate_action (SC_RULE_ACTION, rule, rule->action, rule->action_location); } -char * -translate_symbol_action (const char *a, location l) +char const * +translate_symbol_action (char const *a, location l) { return translate_action (SC_SYMBOL_ACTION, NULL, a, l); } -char * -translate_code (const char *a, location l) +char const * +translate_code (char const *a, location l) { return translate_action (INITIAL, NULL, a, l); } diff --git a/src/scan-gram.l b/src/scan-gram.l index 5e595cf6..6fa9f220 100644 --- a/src/scan-gram.l +++ b/src/scan-gram.l @@ -523,7 +523,7 @@ splice (\\[ \f\t\v]*\n)* { STRING_FINISH; loc->start = code_start; - val->chars = last_string; + val->code = last_string; BEGIN INITIAL; return BRACED_CODE; } @@ -537,7 +537,7 @@ splice (\\[ \f\t\v]*\n)* unexpected_eof (code_start, "}"); STRING_FINISH; loc->start = code_start; - val->chars = last_string; + val->code = last_string; BEGIN INITIAL; return BRACED_CODE; } diff --git a/tests/Makefile.am b/tests/Makefile.am index 7cf188aa..b5f5260e 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -88,6 +88,7 @@ maintainer-check-posix: $(TESTSUITE) .PHONY: maintainer-check-valgrind maintainer-check-valgrind: $(TESTSUITE) test -z '$(VALGRIND)' || \ + VALGRIND_OPTS='--leak-check=full --show-reachable=yes' \ $(TESTSUITE) PREBISON='$(VALGRIND) -q' PREPARSER='$(VALGRIND) -q' .PHONY: maintainer-check diff --git a/tests/calc.at b/tests/calc.at index a3a76feb..2cc68031 100644 --- a/tests/calc.at +++ b/tests/calc.at @@ -337,6 +337,7 @@ main (int argc, const char **argv) ]AT_SKEL_CC_IF([], [m4_bmatch([$4], [%debug], [ yydebug = 1;])])[ status = yyparse (]AT_PARAM_IF([&result, &count])[); + fclose (input); if (global_result != result) abort (); if (global_count != count) diff --git a/tests/testsuite.at b/tests/testsuite.at index 10bb3d89..bd5e1169 100644 --- a/tests/testsuite.at +++ b/tests/testsuite.at @@ -1,7 +1,7 @@ -# Process this file with autom4te to create testsuite. -*- Autotest -*- +# Test suite for GNU Bison. -*- Autotest -*- -# Test suite for GNU Bison. -# Copyright (C) 2000, 2001, 2002, 2003, 2004 Free Software Foundation, Inc. +# Copyright (C) 2000, 2001, 2002, 2003, 2004, 2006 Free Software +# Foundation, Inc. # This program is free software; you can redistribute it and/or modify # it under the terms of the GNU General Public License as published by @@ -19,6 +19,15 @@ # 02110-1301, USA. +# Bison often leaks memory when its exit status is non-zero, so set +# --leak-check=summary for Valgrind in that case. +m4_pushdef([ORIGINAL_AT_CHECK], m4_defn([AT_CHECK])) +m4_pushdef([AT_CHECK], +[ORIGINAL_AT_CHECK( + m4_if(m4_quote(m4_substr(m4_quote($1), 0, 5)), [bison], + m4_if([$2], [0], [], + [[VALGRIND_OPTS="$VALGRIND_OPTS --leak-check=summary --show-reachable=no"; export VALGRIND_OPTS; ]]))$@)]) + # Testing resistance to user bugs. m4_include([input.at]) @@ -64,3 +73,6 @@ m4_include([c++.at]) m4_include([cxx-type.at]) # Regression tests m4_include([glr-regression.at]) + +m4_popdef([AT_CHECK]) +m4_popdef([ORIGINAL_AT_CHECK]) -- 2.47.2