named references: fix double free.

In `rhs[name]: "a" | "b"', do not free "name" twice.
Reported by Tys Lefering.
<http://lists.gnu.org/archive/html/bug-bison/2010-06/msg00002.html>

	* src/named-ref.h, src/named-ref.c (named_ref_copy): New.
	* src/parse-gram.y (current_lhs): Rename as...
	(current_lhs_symbol): this.
	(current_lhs): New function.  Use it to free the current lhs
	named reference.
	* src/reader.c: Bind lhs to a copy of the current named reference.
	* src/symlist.c: Rely on free (0) being valid.
	* tests/named-refs.at: Test this.

(cherry picked from commit 8f462efe923947cc4e72deea5b0fa93a5f88000d)

Conflicts:

	src/parse-gram.y

diff --git a/ChangeLog b/ChangeLog
index 26a34cf93536a768852bdf3035005fa88aed5e08..a3d120215cef52513e96df7388ee038be3d5ff8a 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,18 @@
+2011-03-09  Akim Demaille  <akim@lrde.epita.fr>
+
+       named references: fix double free.
+       In `rhs[name]: "a" | "b"', do not free "name" twice.
+       Reported by Tys Lefering.
+       <http://lists.gnu.org/archive/html/bug-bison/2010-06/msg00002.html>
+       * src/named-ref.h, src/named-ref.c (named_ref_copy): New.
+       * src/parse-gram.y (current_lhs): Rename as...
+       (current_lhs_symbol): this.
+       (current_lhs): New function.  Use it to free the current lhs
+       named reference.
+       * src/reader.c: Bind lhs to a copy of the current named reference.
+       * src/symlist.c: Rely on free (0) being valid.
+       * tests/named-refs.at: Test this.
+

 2011-03-09  Akim Demaille  <akim@lrde.epita.fr>
 
        tests: style changes.
 2011-03-09  Akim Demaille  <akim@lrde.epita.fr>
 
        tests: style changes.

diff --git a/THANKS b/THANKS
index 02082c1c4eea37572bd29adee57dea0a595faa01..f9b1b8a6c8337308ce615bbabbb48772463d613c 100644 (file)
--- a/THANKS
+++ b/THANKS
@@ -104,7 +104,7 @@ Tom Lane                  tgl@sss.pgh.pa.us
 Tom Tromey                tromey@cygnus.com
 Tommy Nordgren            tommy.nordgren@chello.se
 Troy A. Johnson           troyj@ecn.purdue.edu
 Tom Tromey                tromey@cygnus.com
 Tommy Nordgren            tommy.nordgren@chello.se
 Troy A. Johnson           troyj@ecn.purdue.edu

-Tys Lefering              twlevo@xs4all.nl
+Tys Lefering              twlevo@gmail.com

 Vin Shelton               acs@alumni.princeton.edu
 Wayne Green               wayne@infosavvy.com
 Wolfram Wagner            ww@mpi-sb.mpg.de
 Vin Shelton               acs@alumni.princeton.edu
 Wayne Green               wayne@infosavvy.com
 Wolfram Wagner            ww@mpi-sb.mpg.de

diff --git a/src/named-ref.c b/src/named-ref.c
index 4bf3da12e323d7198bcd4ba6b47e5b2528d5e3ed..132c74146d566625a2cb4021797967b1e74c73fc 100644 (file)
--- a/src/named-ref.c
+++ b/src/named-ref.c
@@ -33,6 +33,12 @@ named_ref_new (uniqstr id, location loc)
   return res;
 }
 
   return res;
 }
 

+named_ref *
+named_ref_copy (const named_ref *r)
+{
+  return named_ref_new (r->id, r->loc);
+}
+

 void
 named_ref_free (named_ref *r)
 {
 void
 named_ref_free (named_ref *r)
 {

diff --git a/src/named-ref.h b/src/named-ref.h
index 0f96e463ea848fc3343d495ad50a4270e34efd9b..46d9d8d42af8a5d4251c4fd739fbbec14a694cdf 100644 (file)
--- a/src/named-ref.h
+++ b/src/named-ref.h
@@ -37,6 +37,9 @@ typedef struct named_ref
 /* Allocate a named reference object. */
 named_ref *named_ref_new (uniqstr id, location loc);
 
 /* Allocate a named reference object. */
 named_ref *named_ref_new (uniqstr id, location loc);
 

+/* Allocate and return a copy.  */
+named_ref *named_ref_copy (const named_ref *r);
+

 /* Free a named reference object. */
 void named_ref_free (named_ref *r);
 
 /* Free a named reference object. */
 void named_ref_free (named_ref *r);
 

diff --git a/src/parse-gram.y b/src/parse-gram.y
index f79b0e928c50b1e83adb6e4c66d62399a1f7f3d8..514b5d79952bdcb4f0ebffbf6dee70007205522e 100644 (file)
--- a/src/parse-gram.y
+++ b/src/parse-gram.y
@@ -56,10 +56,28 @@ static char const *char_name (char);
   static int current_prec = 0;
   static location current_lhs_location;
   static named_ref *current_lhs_named_ref;
   static int current_prec = 0;
   static location current_lhs_location;
   static named_ref *current_lhs_named_ref;

-  static symbol *current_lhs;
+  static symbol *current_lhs_symbol;

   static symbol_class current_class = unknown_sym;
   static uniqstr current_type = NULL;
 
   static symbol_class current_class = unknown_sym;
   static uniqstr current_type = NULL;
 

+  /** Set the new current left-hand side symbol, possibly common
+   * to several right-hand side parts of rule.
+   */
+  static
+  void
+  current_lhs(symbol *sym, location loc, named_ref *ref)
+  {
+    current_lhs_symbol = sym;
+    current_lhs_location = loc;
+    /* In order to simplify memory management, named references for lhs
+       are always assigned by deep copy into the current symbol_list
+       node.  This is because a single named-ref in the grammar may
+       result in several uses when the user factors lhs between several
+       rules using "|".  Therefore free the parser's original copy.  */
+    free (current_lhs_named_ref);
+    current_lhs_named_ref = ref;
+  }
+

   #define YYTYPE_INT16 int_fast16_t
   #define YYTYPE_INT8 int_fast8_t
   #define YYTYPE_UINT16 uint_fast16_t
   #define YYTYPE_INT16 int_fast16_t
   #define YYTYPE_INT8 int_fast8_t
   #define YYTYPE_UINT16 uint_fast16_t


@@ -569,8 +587,11 @@ rules_or_grammar_declaration:
 ;
 
 rules:
 ;
 
 rules:

-  id_colon named_ref.opt { current_lhs = $1; current_lhs_location = @1;
-    current_lhs_named_ref = $2; } rhses.1
+  id_colon named_ref.opt { current_lhs ($1, @1, $2); } rhses.1
+  {
+    /* Free the current lhs. */
+    current_lhs (0, @1, 0);
+  }

 ;
 
 rhses.1:
 ;
 
 rhses.1:


@@ -581,7 +602,7 @@ rhses.1:
 
 rhs:
   /* Nothing.  */
 
 rhs:
   /* Nothing.  */

-    { grammar_current_rule_begin (current_lhs, current_lhs_location,
+    { grammar_current_rule_begin (current_lhs_symbol, current_lhs_location,

                                  current_lhs_named_ref); }
 | rhs symbol named_ref.opt
     { grammar_current_rule_symbol_append ($2, @2, $3); }
                                  current_lhs_named_ref); }
 | rhs symbol named_ref.opt
     { grammar_current_rule_symbol_append ($2, @2, $3); }

diff --git a/src/reader.c b/src/reader.c
index 6fc14a3eb9f6ba3cff06dff95546927b57a5fafe..2289d26e80c66ee23db8048c996dbd14c60f0741 100644 (file)
--- a/src/reader.c
+++ b/src/reader.c
@@ -231,7 +231,7 @@ grammar_current_rule_begin (symbol *lhs, location loc,
 
   p = grammar_symbol_append (lhs, loc);
   if (lhs_name)
 
   p = grammar_symbol_append (lhs, loc);
   if (lhs_name)

-    assign_named_ref(p, lhs_name);
+    assign_named_ref (p, named_ref_copy (lhs_name));

 
   current_rule = grammar_end;
 
 
   current_rule = grammar_end;
 

diff --git a/src/symlist.c b/src/symlist.c
index e717c3e1a4775774aa2e47b4005557ffb1f175c4..190d0073a4feb6c94cc873dfbc61e440baeeed77 100644 (file)
--- a/src/symlist.c
+++ b/src/symlist.c
@@ -151,8 +151,7 @@ symbol_list_free (symbol_list *list)
   for (node = list; node; node = next)
     {
       next = node->next;
   for (node = list; node; node = next)
     {
       next = node->next;

-      if (node->named_ref)
-        named_ref_free (node->named_ref);
+      named_ref_free (node->named_ref);

       free (node);
     }
 }
       free (node);
     }
 }

diff --git a/tests/named-refs.at b/tests/named-refs.at
index 8d03518961cb66a676c0a9418d682f7e3fe970c3..5f1daa802abfee2261535c763a55f637ac58ac06 100644 (file)
--- a/tests/named-refs.at
+++ b/tests/named-refs.at
@@ -472,6 +472,19 @@ AT_CLEANUP
 
 #######################################################################
 
 
 #######################################################################
 

+# Bison used to free twice the named ref for "a", since a single copy
+# was used in two rules.
+AT_SETUP([Factored LHS])
+AT_DATA_GRAMMAR([test.y],
+[[
+%%
+start[a]: "foo" | "bar";
+]])
+AT_BISON_CHECK([-o test.c test.y])
+AT_CLEANUP
+
+#######################################################################
+

 AT_SETUP([Unresolved references])
 AT_DATA_GRAMMAR([test.y],
 [[
 AT_SETUP([Unresolved references])
 AT_DATA_GRAMMAR([test.y],
 [[