From a4d1bf6a9cdd337fc113c5a3fc7424e7491d6ea5 Mon Sep 17 00:00:00 2001 From: Akim Demaille 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. * 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 + named references: fix double free. + In `rhs[name]: "a" | "b"', do not free "name" twice. + Reported by Tys Lefering. + + * 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 + 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 address@hidden Tom Tromey address@hidden Tommy Nordgren address@hidden Troy A. Johnson address@hidden -Tys Lefering address@hidden +Tys Lefering address@hidden Vin Shelton address@hidden Wayne Green address@hidden Wolfram Wagner address@hidden 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