From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 23651 invoked by alias); 11 Dec 2008 17:07:43 -0000 Received: (qmail 23595 invoked by uid 22791); 11 Dec 2008 17:07:41 -0000 X-Spam-Check-By: sourceware.org Received: from mx2.redhat.com (HELO mx2.redhat.com) (66.187.237.31) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Thu, 11 Dec 2008 17:07:04 +0000 Received: from int-mx2.corp.redhat.com (int-mx2.corp.redhat.com [172.16.27.26]) by mx2.redhat.com (8.13.8/8.13.8) with ESMTP id mBBH72Pm018961 for ; Thu, 11 Dec 2008 12:07:02 -0500 Received: from ns3.rdu.redhat.com (ns3.rdu.redhat.com [10.11.255.199]) by int-mx2.corp.redhat.com (8.13.1/8.13.1) with ESMTP id mBBH71to010051; Thu, 11 Dec 2008 12:07:01 -0500 Received: from opsy.redhat.com (vpn-12-148.rdu.redhat.com [10.11.12.148]) by ns3.rdu.redhat.com (8.13.8/8.13.8) with ESMTP id mBBH6xFd023503; Thu, 11 Dec 2008 12:07:00 -0500 Received: by opsy.redhat.com (Postfix, from userid 500) id 23951508027; Thu, 11 Dec 2008 10:06:59 -0700 (MST) To: gdb-patches@sourceware.org Subject: RFA: fix PR macros/2564 From: Tom Tromey Reply-To: tromey@redhat.com Date: Thu, 11 Dec 2008 17:07:00 -0000 Message-ID: User-Agent: Gnus/5.11 (Gnus v5.11) Emacs/22.2 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2008-12/txt/msg00205.txt.bz2 This patch fixes PR macros/2564. The bug here is that the lexer calls finished_macro_expansion, freeing the macro expansion text, while a token referring to this memory is still "live". This happens because the parser has not yet reduced the rule using this token. This patch fixes this problem in a simple way: we create an obstack for use during parsing, and simply keep all macro expansions on this obstack for the duration of the parse. While implementing this I noticed that the various macro expansion variables and functions, defined in c-lang.c, are only ever called from c-exp.y. So, I moved them all to c-exp.y and made them static. There is a new test case included. It does not reliably fail on an unpatched gdb. However, if you hack the test suite to run gdb under valgrind, you can clearly see the bug. I verified that the patched gdb is valgrind-clean in this area. Built and regtested on x86-64 (compile farm). Please review. Tom 2008-12-11 Tom Tromey PR macros/2564: * c-exp.y (macro_original_text, expansion_obstack, expression_macro_scope): New globals. (scan_macro_expansion): New function. (scanning_macro_expansion): Likewise. (finished_macro_expansion): Likewise. (scan_macro_cleanup): Likewise. (c_parse): Find macro scope. Initialize obstack. * c-lang.h (scan_macro_expansion, scanning_macro_expansion, finished_macro_expansion, expression_macro_lookup_func, expression_macro_lookup_baton): Remove. * c-lang.c (scan_macro_expansion, scanning_macro_expansion, finished_macro_expansion, expression_macro_lookup_func, expression_macro_lookup_baton): Remove. (macro_original_text, macro_expanded_text, c_preprocess_and_parse): Remove. (c_language_defn, cplus_language_defn, asm_language_defn, minimal_language_defn): Use c_parse. 2008-12-11 Tom Tromey * gdb.base/macscp.exp: Print "address.addr". * gdb.base/macscp1.c (struct outer): New struct. (address): New global. diff --git a/gdb/c-exp.y b/gdb/c-exp.y index 153e2be..219b008 100644 --- a/gdb/c-exp.y +++ b/gdb/c-exp.y @@ -54,6 +54,8 @@ Boston, MA 02110-1301, USA. */ #include "block.h" #include "cp-support.h" #include "dfp.h" +#include "gdb_assert.h" +#include "macroscope.h" #define parse_type builtin_type (parse_gdbarch) @@ -1396,6 +1398,85 @@ static const struct token tokentab2[] = {">=", GEQ, BINOP_END} }; +/* When we find that lexptr (the global var defined in parse.c) is + pointing at a macro invocation, we expand the invocation, and call + scan_macro_expansion to save the old lexptr here and point lexptr + into the expanded text. When we reach the end of that, we call + end_macro_expansion to pop back to the value we saved here. The + macro expansion code promises to return only fully-expanded text, + so we don't need to "push" more than one level. + + This is disgusting, of course. It would be cleaner to do all macro + expansion beforehand, and then hand that to lexptr. But we don't + really know where the expression ends. Remember, in a command like + + (gdb) break *ADDRESS if CONDITION + + we evaluate ADDRESS in the scope of the current frame, but we + evaluate CONDITION in the scope of the breakpoint's location. So + it's simply wrong to try to macro-expand the whole thing at once. */ +static char *macro_original_text; + +/* We save all intermediate macro expansions on this obstack for the + duration of a single parse. The expansion text may sometimes have + to live past the end of the expansion, due to yacc lookahead. + Rather than try to be clever about saving the data for a single + token, we simply keep it all and delete it after parsing has + completed. */ +static struct obstack expansion_obstack; + +static void +scan_macro_expansion (char *expansion) +{ + char *copy; + + /* We'd better not be trying to push the stack twice. */ + gdb_assert (! macro_original_text); + + /* Copy to the obstack, and then free the intermediate + expansion. */ + copy = obstack_copy0 (&expansion_obstack, expansion, strlen (expansion)); + xfree (expansion); + + /* Save the old lexptr value, so we can return to it when we're done + parsing the expanded text. */ + macro_original_text = lexptr; + lexptr = copy; +} + + +static int +scanning_macro_expansion (void) +{ + return macro_original_text != 0; +} + + +static void +finished_macro_expansion (void) +{ + /* There'd better be something to pop back to. */ + gdb_assert (macro_original_text); + + /* Pop back to the original text. */ + lexptr = macro_original_text; + macro_original_text = 0; +} + + +static void +scan_macro_cleanup (void *dummy) +{ + if (macro_original_text) + finished_macro_expansion (); + + obstack_free (&expansion_obstack, NULL); +} + + +/* The scope used for macro expansion. */ +static struct macro_scope *expression_macro_scope; + /* This is set if a NAME token appeared at the very end of the input string, with no whitespace separating the name from the EOF. This is used only when parsing to do field name completion. */ @@ -1431,8 +1512,8 @@ yylex () if (! scanning_macro_expansion ()) { char *expanded = macro_expand_next (&lexptr, - expression_macro_lookup_func, - expression_macro_lookup_baton); + standard_macro_lookup, + expression_macro_scope); if (expanded) scan_macro_expansion (expanded); @@ -1903,11 +1984,36 @@ yylex () int c_parse (void) { + int result; + struct cleanup *back_to = make_cleanup (free_current_contents, + &expression_macro_scope); + + /* Set up the scope for macro expansion. */ + expression_macro_scope = NULL; + + if (expression_context_block) + expression_macro_scope + = sal_macro_scope (find_pc_line (expression_context_pc, 0)); + else + expression_macro_scope = default_macro_scope (); + if (! expression_macro_scope) + expression_macro_scope = user_macro_scope (); + + /* Initialize macro expansion code. */ + obstack_init (&expansion_obstack); + gdb_assert (! macro_original_text); + make_cleanup (scan_macro_cleanup, 0); + + /* Initialize some state used by the lexer. */ last_was_structop = 0; saw_name_at_eof = 0; - return yyparse (); + + result = yyparse (); + do_cleanups (back_to); + return result; } + void yyerror (msg) char *msg; diff --git a/gdb/c-lang.c b/gdb/c-lang.c index 067e429..dc7b059 100644 --- a/gdb/c-lang.c +++ b/gdb/c-lang.c @@ -185,111 +185,6 @@ c_printstr (struct ui_file *stream, const gdb_byte *string, /* Preprocessing and parsing C and C++ expressions. */ -/* When we find that lexptr (the global var defined in parse.c) is - pointing at a macro invocation, we expand the invocation, and call - scan_macro_expansion to save the old lexptr here and point lexptr - into the expanded text. When we reach the end of that, we call - end_macro_expansion to pop back to the value we saved here. The - macro expansion code promises to return only fully-expanded text, - so we don't need to "push" more than one level. - - This is disgusting, of course. It would be cleaner to do all macro - expansion beforehand, and then hand that to lexptr. But we don't - really know where the expression ends. Remember, in a command like - - (gdb) break *ADDRESS if CONDITION - - we evaluate ADDRESS in the scope of the current frame, but we - evaluate CONDITION in the scope of the breakpoint's location. So - it's simply wrong to try to macro-expand the whole thing at once. */ -static char *macro_original_text; -static char *macro_expanded_text; - - -void -scan_macro_expansion (char *expansion) -{ - /* We'd better not be trying to push the stack twice. */ - gdb_assert (! macro_original_text); - gdb_assert (! macro_expanded_text); - - /* Save the old lexptr value, so we can return to it when we're done - parsing the expanded text. */ - macro_original_text = lexptr; - lexptr = expansion; - - /* Save the expanded text, so we can free it when we're finished. */ - macro_expanded_text = expansion; -} - - -int -scanning_macro_expansion (void) -{ - return macro_original_text != 0; -} - - -void -finished_macro_expansion (void) -{ - /* There'd better be something to pop back to, and we better have - saved a pointer to the start of the expanded text. */ - gdb_assert (macro_original_text); - gdb_assert (macro_expanded_text); - - /* Pop back to the original text. */ - lexptr = macro_original_text; - macro_original_text = 0; - - /* Free the expanded text. */ - xfree (macro_expanded_text); - macro_expanded_text = 0; -} - - -static void -scan_macro_cleanup (void *dummy) -{ - if (macro_original_text) - finished_macro_expansion (); -} - - -/* We set these global variables before calling c_parse, to tell it - how it to find macro definitions for the expression at hand. */ -macro_lookup_ftype *expression_macro_lookup_func; -void *expression_macro_lookup_baton; - - -static int -c_preprocess_and_parse (void) -{ - /* Set up a lookup function for the macro expander. */ - struct macro_scope *scope = 0; - struct cleanup *back_to = make_cleanup (free_current_contents, &scope); - - if (expression_context_block) - scope = sal_macro_scope (find_pc_line (expression_context_pc, 0)); - else - scope = default_macro_scope (); - if (! scope) - scope = user_macro_scope (); - - expression_macro_lookup_func = standard_macro_lookup; - expression_macro_lookup_baton = (void *) scope; - - gdb_assert (! macro_original_text); - make_cleanup (scan_macro_cleanup, 0); - - { - int result = c_parse (); - do_cleanups (back_to); - return result; - } -} - - /* Table mapping opcodes into strings for printing operators and precedences of the operators. */ @@ -395,7 +290,7 @@ const struct language_defn c_language_defn = array_row_major, macro_expansion_c, &exp_descriptor_standard, - c_preprocess_and_parse, + c_parse, c_error, null_post_parser, c_printchar, /* Print a character constant */ @@ -513,7 +408,7 @@ const struct language_defn cplus_language_defn = array_row_major, macro_expansion_c, &exp_descriptor_standard, - c_preprocess_and_parse, + c_parse, c_error, null_post_parser, c_printchar, /* Print a character constant */ @@ -550,7 +445,7 @@ const struct language_defn asm_language_defn = array_row_major, macro_expansion_c, &exp_descriptor_standard, - c_preprocess_and_parse, + c_parse, c_error, null_post_parser, c_printchar, /* Print a character constant */ @@ -592,7 +487,7 @@ const struct language_defn minimal_language_defn = array_row_major, macro_expansion_c, &exp_descriptor_standard, - c_preprocess_and_parse, + c_parse, c_error, null_post_parser, c_printchar, /* Print a character constant */ diff --git a/gdb/c-lang.h b/gdb/c-lang.h index cc9abde..a8c95f9 100644 --- a/gdb/c-lang.h +++ b/gdb/c-lang.h @@ -55,13 +55,6 @@ extern void c_printstr (struct ui_file * stream, const gdb_byte *string, int force_ellipses, const struct value_print_options *options); -extern void scan_macro_expansion (char *expansion); -extern int scanning_macro_expansion (void); -extern void finished_macro_expansion (void); - -extern macro_lookup_ftype *expression_macro_lookup_func; -extern void *expression_macro_lookup_baton; - extern void c_language_arch_info (struct gdbarch *gdbarch, struct language_arch_info *lai); diff --git a/gdb/testsuite/gdb.base/macscp.exp b/gdb/testsuite/gdb.base/macscp.exp index 9cb9ef5..f7553a5 100644 --- a/gdb/testsuite/gdb.base/macscp.exp +++ b/gdb/testsuite/gdb.base/macscp.exp @@ -415,6 +415,10 @@ gdb_test "break [gdb_get_line_number "set breakpoint here"]" \ gdb_test "continue" "foo = 0;.*" "continue to macsp_expr" +gdb_test "print address.addr" \ + " = 0" \ + "print address.addr" + gdb_test "print MACRO_TO_EXPAND" \ "No symbol \"MACRO_TO_EXPAND\" in current context\." \ "print expression with macro before define." diff --git a/gdb/testsuite/gdb.base/macscp1.c b/gdb/testsuite/gdb.base/macscp1.c index 40f1217..e754b26 100644 --- a/gdb/testsuite/gdb.base/macscp1.c +++ b/gdb/testsuite/gdb.base/macscp1.c @@ -10,6 +10,20 @@ #define FORTY_EIGHT 48 #undef FORTY_EIGHT +struct outer +{ + struct fields + { + struct fault + { + int addr; + } fault; + } fields; +}; +struct outer address; + +#define addr fields.fault.addr + /* A macro named UNTIL_ is #defined until just before the definition of the function .