From 8f462efe923947cc4e72deea5b0fa93a5f88000d Mon Sep 17 00:00:00 2001 From: Akim Demaille Date: Wed, 2 Mar 2011 17:06:58 +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. --- ChangeLog | 15 +++++++++++++++ THANKS | 2 +- src/named-ref.c | 6 ++++++ src/named-ref.h | 3 +++ src/parse-gram.y | 30 ++++++++++++++++++++++++++---- src/reader.c | 2 +- src/symlist.c | 3 +-- tests/named-refs.at | 13 +++++++++++++ 8 files changed, 66 insertions(+), 8 deletions(-) diff --git a/ChangeLog b/ChangeLog index 4bb16a8..448e54a 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 ac6047a..ad6f379 100644 --- a/THANKS +++ b/THANKS @@ -102,7 +102,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 8871f67..8c2e18d 100644 --- a/src/parse-gram.y +++ b/src/parse-gram.y @@ -61,11 +61,30 @@ static void add_param (char const *type, char *decl, location loc); static symbol_class current_class = unknown_sym; static uniqstr current_type = NULL; -static symbol *current_lhs; +static symbol *current_lhs_symbol; static location current_lhs_location; static named_ref *current_lhs_named_ref; static int current_prec = 0; +/** 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 @@ -526,8 +545,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: @@ -538,7 +560,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 9153f21..86cc82c 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 98f45a5..ff414a9 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