Le 9 mars 2011 à 21:13, Akim Demaille a écrit :
> For the records, these are the patches as installed on 2.5 (including the
> change of address Tys asked me).
Now for master.
The first patch is identical, but the second one (the real one) is slightly
different because of changes in parse-gram.y.
From a4d1bf6a9cdd337fc113c5a3fc7424e7491d6ea5 Mon Sep 17 00:00:00 2001
From: Akim Demaille <[email protected]>
Date: Wed, 9 Mar 2011 21:10:35 +0100
Subject: [PATCH 2/2] 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
---
ChangeLog | 15 +++++++++++++++
THANKS | 2 +-
src/named-ref.c | 6 ++++++
src/named-ref.h | 3 +++
src/parse-gram.y | 29 +++++++++++++++++++++++++----
src/reader.c | 2 +-
src/symlist.c | 3 +--
tests/named-refs.at | 13 +++++++++++++
8 files changed, 65 insertions(+), 8 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index 26a34cf..a3d1202 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,20 @@
2011-03-09 Akim Demaille <[email protected]>
+ 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 <[email protected]>
+
tests: style changes.
* tests/named-refs.at (Redundant words in LHS brackets)
(Unresolved references): here.
diff --git a/THANKS b/THANKS
index 02082c1..f9b1b8a 100644
--- a/THANKS
+++ b/THANKS
@@ -104,7 +104,7 @@ Tom Lane [email protected]
Tom Tromey [email protected]
Tommy Nordgren [email protected]
Troy A. Johnson [email protected]
-Tys Lefering [email protected]
+Tys Lefering [email protected]
Vin Shelton [email protected]
Wayne Green [email protected]
Wolfram Wagner [email protected]
diff --git a/src/named-ref.c b/src/named-ref.c
index 4bf3da1..132c741 100644
--- a/src/named-ref.c
+++ b/src/named-ref.c
@@ -33,6 +33,12 @@ named_ref_new (uniqstr id, location loc)
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)
{
diff --git a/src/named-ref.h b/src/named-ref.h
index 0f96e46..46d9d8d 100644
--- 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 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);
diff --git a/src/parse-gram.y b/src/parse-gram.y
index f79b0e9..514b5d7 100644
--- 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 symbol *current_lhs;
+ static symbol *current_lhs_symbol;
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
@@ -569,8 +587,11 @@ rules_or_grammar_declaration:
;
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:
@@ -581,7 +602,7 @@ rhses.1:
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); }
diff --git a/src/reader.c b/src/reader.c
index 6fc14a3..2289d26 100644
--- 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)
- assign_named_ref(p, lhs_name);
+ assign_named_ref (p, named_ref_copy (lhs_name));
current_rule = grammar_end;
diff --git a/src/symlist.c b/src/symlist.c
index e717c3e..190d007 100644
--- 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;
- if (node->named_ref)
- named_ref_free (node->named_ref);
+ named_ref_free (node->named_ref);
free (node);
}
}
diff --git a/tests/named-refs.at b/tests/named-refs.at
index 8d03518..5f1daa8 100644
--- 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],
[[
--
1.7.4.1
It took me a while to test this patch, because it seems that bootstrapping a
checkout of master on top of branch-2.5 does not work properly. I had to
remove lib/gnulib.mk in order to have bootstrap extract it and run our
conversion program. Otherwise, apparently, bootstrap thinks lib/gnulib.mk is
nice looking as it is, and decides not to install it, and therefore, not to
apply our gnulib_mk_hook.
if test $file = Makefile.am && test "X$gnulib_mk" != XMakefile.am; then
copied=$copied${sep}$gnulib_mk; sep=$nl
remove_intl='/^[^#].*\/intl/s/^/#/;'"s!$bt_regex/!!g"
sed "$remove_intl" $1/$dir/$file |
cmp - $dir/$gnulib_mk > /dev/null || {
echo "$0: Copying $1/$dir/$file to $dir/$gnulib_mk ..." &&
rm -f $dir/$gnulib_mk &&
sed "$remove_intl" $1/$dir/$file >$dir/$gnulib_mk &&
gnulib_mk_hook $dir/$gnulib_mk
}
cmp is not comparing the same thing: one is transformed, the other one is not.
In 2.5 it does not need to be transformed, so it is not, and when moving to
master, where the transformation is needed, it thinks all is done.
We should probably remove gnulib.mk at the beginning of bootstrap.
(and bummer, I just saw wrote rhs where I meant lhs, in the log... bah)
