[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: Valgrind and Bison memory leaks
|
From: |
Paul Eggert |
|
Subject: |
Re: Valgrind and Bison memory leaks |
|
Date: |
Wed, 08 Nov 2006 12:31:16 -0800 |
|
User-agent: |
Gnus/5.1008 (Gnus v5.10.8) Emacs/21.4 (gnu/linux) |
OK, I reviewed that patch and installed the following
slightly-different one. The main thing was that funny union, which
confused me, and so I replaced it with a char const * value and a char
* storage pointer (which might be null, but can be freed).
"make maintainer-check-valgrind" has a lot of failures still, but I
assume more patches will come later?
2006-11-08 Joel E. Denny <address@hidden>
and Paul Eggert <address@hidden>
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 (<SC_BRACED_CODE>): 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.
Index: src/files.h
===================================================================
RCS file: /cvsroot/bison/bison/src/files.h,v
retrieving revision 1.41
diff -u -r1.41 files.h
--- src/files.h 12 Oct 2006 23:29:52 -0000 1.41
+++ src/files.h 8 Nov 2006 20:27:14 -0000
@@ -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 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 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);
Index: src/files.c
===================================================================
RCS file: /cvsroot/bison/bison/src/files.c,v
retrieving revision 1.97
diff -u -r1.97 files.c
--- src/files.c 12 Oct 2006 23:29:52 -0000 1.97
+++ src/files.c 8 Nov 2006 20:27:14 -0000
@@ -48,10 +48,10 @@
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 @@
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 @@
| 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 @@
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 @@
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 @@
/* 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 @@
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);
}
Index: src/getargs.c
===================================================================
RCS file: /cvsroot/bison/bison/src/getargs.c,v
retrieving revision 1.84
diff -u -r1.84 getargs.c
--- src/getargs.c 1 Nov 2006 06:09:40 -0000 1.84
+++ src/getargs.c 8 Nov 2006 20:27:14 -0000
@@ -98,7 +98,7 @@
*flags = 0;
else
*flags |= value;
- args = strtok (NULL, ",");
+ args = strtok (NULL, ",");
}
}
else
@@ -408,7 +408,7 @@
/* 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 @@
/* 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':
Index: src/main.c
===================================================================
RCS file: /cvsroot/bison/bison/src/main.c,v
retrieving revision 1.90
diff -u -r1.90 main.c
--- src/main.c 12 Oct 2006 23:29:52 -0000 1.90
+++ src/main.c 8 Nov 2006 20:27:14 -0000
@@ -26,6 +26,7 @@
#include <bitset_stats.h>
#include <bitset.h>
#include <configmake.h>
+#include <quotearg.h>
#include <timevar.h>
#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 @@
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)
Index: src/muscle_tab.h
===================================================================
RCS file: /cvsroot/bison/bison/src/muscle_tab.h,v
retrieving revision 1.17
diff -u -r1.17 muscle_tab.h
--- src/muscle_tab.h 8 Oct 2006 11:07:02 -0000 1.17
+++ src/muscle_tab.h 8 Nov 2006 20:27:14 -0000
@@ -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);
Index: src/muscle_tab.c
===================================================================
RCS file: /cvsroot/bison/bison/src/muscle_tab.c,v
retrieving revision 1.39
diff -u -r1.39 muscle_tab.c
--- src/muscle_tab.c 3 Jan 2006 20:19:41 -0000 1.39
+++ src/muscle_tab.c 8 Nov 2006 20:27:14 -0000
@@ -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 @@
| 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 @@
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 @@
`------------------------------------------------------------*/
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 @@
entry = xmalloc (sizeof *entry);
entry->key = key;
hash_insert (muscle_table, entry);
+ entry->storage = NULL;
}
entry->value = value;
}
@@ -138,19 +150,19 @@
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 @@
obstack_1grow (&muscle_obstack, 0);
extension = obstack_finish (&muscle_obstack);
muscle_grow (key, extension, "");
+ obstack_free (&muscle_obstack, extension);
}
Index: src/parse-gram.y
===================================================================
RCS file: /cvsroot/bison/bison/src/parse-gram.y,v
retrieving revision 1.95
diff -u -r1.95 parse-gram.y
--- src/parse-gram.y 21 Oct 2006 10:03:35 -0000 1.95
+++ src/parse-gram.y 8 Nov 2006 20:27:15 -0000
@@ -93,13 +93,13 @@
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 @@
/* braceless is not to be used for rule or symbol actions, as it
calls translate_code. */
-%type <chars> STRING "{...}" "%{...%}" EPILOGUE braceless content content.opt
+%type <chars> STRING "%{...%}" EPILOGUE braceless content content.opt
+%type <code> "{...}"
%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 <uniqstr> TYPE ID ID_COLON
%printer { fprintf (stderr, "<%s>", $$); } TYPE
@@ -267,7 +268,7 @@
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 @@
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"
Index: src/scan-code.h
===================================================================
RCS file: /cvsroot/bison/bison/src/scan-code.h,v
retrieving revision 1.3
diff -u -r1.3 scan-code.h
--- src/scan-code.h 13 Jul 2006 08:12:00 -0000 1.3
+++ src/scan-code.h 8 Nov 2006 20:27:15 -0000
@@ -35,13 +35,13 @@
/* 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_ */
Index: src/scan-code.l
===================================================================
RCS file: /cvsroot/bison/bison/src/scan-code.l,v
retrieving revision 1.13
diff -u -r1.13 scan-code.l
--- src/scan-code.l 15 Sep 2006 16:34:48 -0000 1.13
+++ src/scan-code.l 8 Nov 2006 20:27:15 -0000
@@ -372,8 +372,8 @@
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 @@
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);
}
Index: src/scan-gram.l
===================================================================
RCS file: /cvsroot/bison/bison/src/scan-gram.l,v
retrieving revision 1.106
diff -u -r1.106 scan-gram.l
--- src/scan-gram.l 21 Oct 2006 10:03:35 -0000 1.106
+++ src/scan-gram.l 8 Nov 2006 20:27:15 -0000
@@ -523,7 +523,7 @@
{
STRING_FINISH;
loc->start = code_start;
- val->chars = last_string;
+ val->code = last_string;
BEGIN INITIAL;
return BRACED_CODE;
}
@@ -537,7 +537,7 @@
unexpected_eof (code_start, "}");
STRING_FINISH;
loc->start = code_start;
- val->chars = last_string;
+ val->code = last_string;
BEGIN INITIAL;
return BRACED_CODE;
}
Index: tests/Makefile.am
===================================================================
RCS file: /cvsroot/bison/bison/tests/Makefile.am,v
retrieving revision 1.42
diff -u -r1.42 Makefile.am
--- tests/Makefile.am 6 Mar 2006 08:29:22 -0000 1.42
+++ tests/Makefile.am 8 Nov 2006 20:27:15 -0000
@@ -88,6 +88,7 @@
.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
Index: tests/calc.at
===================================================================
RCS file: /cvsroot/bison/bison/tests/calc.at,v
retrieving revision 1.92
diff -u -r1.92 calc.at
--- tests/calc.at 15 Sep 2006 15:56:26 -0000 1.92
+++ tests/calc.at 8 Nov 2006 20:27:15 -0000
@@ -337,6 +337,7 @@
]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)
Index: tests/testsuite.at
===================================================================
RCS file: /cvsroot/bison/bison/tests/testsuite.at,v
retrieving revision 1.28
diff -u -r1.28 testsuite.at
--- tests/testsuite.at 14 May 2005 06:49:48 -0000 1.28
+++ tests/testsuite.at 8 Nov 2006 20:27:15 -0000
@@ -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([cxx-type.at])
# Regression tests
m4_include([glr-regression.at])
+
+m4_popdef([AT_CHECK])
+m4_popdef([ORIGINAL_AT_CHECK])