From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 16358 invoked by alias); 3 Sep 2008 16:58:16 -0000 Received: (qmail 16338 invoked by uid 22791); 3 Sep 2008 16:58:14 -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.31) with ESMTP; Wed, 03 Sep 2008 16:57:39 +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 m83Gv2wt006565 for ; Wed, 3 Sep 2008 12:57:22 -0400 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 m83Guppt012982 for ; Wed, 3 Sep 2008 12:56:51 -0400 Received: from opsy.redhat.com (vpn-10-123.bos.redhat.com [10.16.10.123]) by ns3.rdu.redhat.com (8.13.8/8.13.8) with ESMTP id m83GumvV019615; Wed, 3 Sep 2008 12:56:49 -0400 Received: by opsy.redhat.com (Postfix, from userid 500) id 3E9913781A1; Wed, 3 Sep 2008 10:57:03 -0600 (MDT) To: gdb-patches@sourceware.org Subject: PR gdb/856 From: Tom Tromey Reply-To: tromey@redhat.com X-Attribution: Tom Date: Wed, 03 Sep 2008 16:58:00 -0000 Message-ID: User-Agent: Gnus/5.11 (Gnus v5.11) Emacs/22.1 (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-09/txt/msg00046.txt.bz2 This patch fixes PR gdb/856. This PR is concerned with a particular failure with macro scoping. Basically, parse_exp_1 takes a block argument, but macros are not block-scoped. The PR suggests that parse_exp_1 ought to take a sal instead, so this is what I've implemented. I changed any caller with access to a relevant sal or PC to use that; otherwise I changed the code to use either a sal constructed from the block's location, or an empty sal. The result is an improvement over the current situation, but still not perfect. Perfection here is a pain to achieve... for instance, a watchpoint expression might involve a macro, but macros are scoped in a completely different way from ordinary variables. Built and regtested on x86-64 (compile farm). New test case included. Please review. Tom :ADDPATCH macros: 2008-09-03 Tom Tromey PR gdb/856: * wrapper.h (gdb_parse_exp_1): Update. * wrapper.c (gdb_parse_exp_1): Change 'block' argument to 'sal'. * varobj.c (varobj_create): Update. (varobj_set_value): Likewise. * tracepoint.c (validate_actionline): Update. (encode_actions): Update. * symtab.h (empty_sal): Declare. * symtab.c (empty_sal): New function. * parser-defs.h (expression_context_pc): Remove. (expression_context_sal): Declare. * parse.c (expression_context_pc): Remove. (expression_context_sal): New global. (parse_exp_1): Change 'block' argument to 'sal'. (parse_exp_in_context): Likewise. Set expression_context_sal. (parse_expression): Update. (parse_field_expression): Update. * expression.h (parse_exp_1): Change type. * eval.c (parse_and_eval_address_1): Update. (parse_to_comma_and_eval): Update. * c-lang.c (c_preprocess_and_parse): Use expression_context_sal. * breakpoint.c (condition_command): Update. (update_watchpoint): Update. (create_breakpoint): Update. (find_condition_and_thread): Update. (watch_command_1): Update. (update_breakpoint_locations): Update. * ada-lang.c (ada_parse_catchpoint_condition): Update. 2008-09-03 Tom Tromey * gdb.base/macscp1.c (macscp_expr): Add comment. * gdb.base/macscp.exp: Add test for macro scope. diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c index 2081a4d..06b0e12 100644 --- a/gdb/ada-lang.c +++ b/gdb/ada-lang.c @@ -10352,7 +10352,7 @@ static struct expression * ada_parse_catchpoint_condition (char *cond_string, struct symtab_and_line sal) { - return (parse_exp_1 (&cond_string, block_for_pc (sal.pc), 0)); + return (parse_exp_1 (&cond_string, sal, 0)); } /* Return the symtab_and_line that should be used to insert an exception diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c index 847de00..c29ca65 100644 --- a/gdb/breakpoint.c +++ b/gdb/breakpoint.c @@ -635,7 +635,7 @@ condition_command (char *arg, int from_tty) { arg = p; loc->cond = - parse_exp_1 (&arg, block_for_pc (loc->address), 0); + parse_exp_1 (&arg, find_pc_line (loc->address, 0), 0); if (*arg) error (_("Junk at end of expression")); } @@ -934,7 +934,9 @@ update_watchpoint (struct breakpoint *b, int reparse) b->exp = NULL; } s = b->exp_string; - b->exp = parse_exp_1 (&s, b->exp_valid_block, 0); + b->exp = parse_exp_1 (&s, + find_pc_line (BLOCK_START (b->exp_valid_block), 0), + 0); /* If the meaning of expression itself changed, the old value is no longer relevant. We don't want to report a watchpoint hit to the user when the old value and the new value may actually @@ -1019,7 +1021,8 @@ update_watchpoint (struct breakpoint *b, int reparse) if (b->cond_string != NULL) { char *s = b->cond_string; - b->loc->cond = parse_exp_1 (&s, b->exp_valid_block, 0); + b->loc->cond = parse_exp_1 (&s, + find_pc_line (BLOCK_START (b->exp_valid_block), 0), 0); } } else if (!within_current_scope) @@ -5138,7 +5141,7 @@ create_breakpoint (struct symtabs_and_lines sals, char *addr_string, if (b->cond_string) { char *arg = b->cond_string; - loc->cond = parse_exp_1 (&arg, block_for_pc (loc->address), 0); + loc->cond = parse_exp_1 (&arg, sal, 0); if (*arg) error (_("Garbage %s follows condition"), arg); } @@ -5437,7 +5440,7 @@ find_condition_and_thread (char *tok, CORE_ADDR pc, if (toklen >= 1 && strncmp (tok, "if", toklen) == 0) { tok = cond_start = end_tok + 1; - parse_exp_1 (&tok, block_for_pc (pc), 0); + parse_exp_1 (&tok, find_pc_line (pc, 0), 0); cond_end = tok; *cond_string = savestring (cond_start, cond_end - cond_start); @@ -5943,7 +5946,7 @@ watch_command_1 (char *arg, int accessflag, int from_tty) /* Parse the rest of the arguments. */ innermost_block = NULL; exp_start = arg; - exp = parse_exp_1 (&arg, 0, 0); + exp = parse_exp_1 (&arg, sal, 0); exp_end = arg; exp_valid_block = innermost_block; mark = value_mark (); @@ -5963,7 +5966,7 @@ watch_command_1 (char *arg, int accessflag, int from_tty) if (toklen >= 1 && strncmp (tok, "if", toklen) == 0) { tok = cond_start = end_tok + 1; - cond = parse_exp_1 (&tok, 0, 0); + cond = parse_exp_1 (&tok, sal, 0); cond_end = tok; } if (*tok) @@ -7365,8 +7368,7 @@ update_breakpoint_locations (struct breakpoint *b, s = b->cond_string; TRY_CATCH (e, RETURN_MASK_ERROR) { - new_loc->cond = parse_exp_1 (&s, block_for_pc (sals.sals[i].pc), - 0); + new_loc->cond = parse_exp_1 (&s, sals.sals[i], 0); } if (e.reason < 0) { diff --git a/gdb/c-lang.c b/gdb/c-lang.c index 9ce4bb9..6de792f 100644 --- a/gdb/c-lang.c +++ b/gdb/c-lang.c @@ -268,12 +268,9 @@ c_preprocess_and_parse (void) 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 (); + scope = sal_macro_scope (expression_context_sal); if (! scope) - scope = user_macro_scope (); + scope = default_macro_scope (); expression_macro_lookup_func = standard_macro_lookup; expression_macro_lookup_baton = (void *) scope; diff --git a/gdb/eval.c b/gdb/eval.c index ca36762..0a0d154 100644 --- a/gdb/eval.c +++ b/gdb/eval.c @@ -99,7 +99,7 @@ parse_and_eval_address (char *exp) CORE_ADDR parse_and_eval_address_1 (char **expptr) { - struct expression *expr = parse_exp_1 (expptr, (struct block *) 0, 0); + struct expression *expr = parse_exp_1 (expptr, empty_sal (), 0); CORE_ADDR addr; struct cleanup *old_chain = make_cleanup (free_current_contents, &expr); @@ -144,7 +144,7 @@ parse_and_eval (char *exp) struct value * parse_to_comma_and_eval (char **expp) { - struct expression *expr = parse_exp_1 (expp, (struct block *) 0, 1); + struct expression *expr = parse_exp_1 (expp, empty_sal (), 1); struct value *val; struct cleanup *old_chain = make_cleanup (free_current_contents, &expr); diff --git a/gdb/expression.h b/gdb/expression.h index 084c70e..d5c2b44 100644 --- a/gdb/expression.h +++ b/gdb/expression.h @@ -391,7 +391,7 @@ extern struct expression *parse_expression (char *); extern struct type *parse_field_expression (char *, char **); -extern struct expression *parse_exp_1 (char **, struct block *, int); +extern struct expression *parse_exp_1 (char **, struct symtab_and_line, int); /* For use by parsers; set if we want to parse an expression and attempt to complete a field name. */ diff --git a/gdb/parse.c b/gdb/parse.c index 1d2d501..e0b493d 100644 --- a/gdb/parse.c +++ b/gdb/parse.c @@ -72,7 +72,7 @@ struct expression *expout; int expout_size; int expout_ptr; struct block *expression_context_block; -CORE_ADDR expression_context_pc; +struct symtab_and_line expression_context_sal; struct block *innermost_block; int arglist_len; union type_stack_elt *type_stack; @@ -116,7 +116,8 @@ static int prefixify_expression (struct expression *); static int prefixify_subexp (struct expression *, struct expression *, int, int); -static struct expression *parse_exp_in_context (char **, struct block *, int, +static struct expression *parse_exp_in_context (char **, + struct symtab_and_line, int, int, int *); void _initialize_parse (void); @@ -975,9 +976,9 @@ prefixify_subexp (struct expression *inexpr, If COMMA is nonzero, stop if a comma is reached. */ struct expression * -parse_exp_1 (char **stringptr, struct block *block, int comma) +parse_exp_1 (char **stringptr, struct symtab_and_line sal, int comma) { - return parse_exp_in_context (stringptr, block, comma, 0, NULL); + return parse_exp_in_context (stringptr, sal, comma, 0, NULL); } /* As for parse_exp_1, except that if VOID_CONTEXT_P, then @@ -988,7 +989,7 @@ parse_exp_1 (char **stringptr, struct block *block, int comma) is left untouched. */ static struct expression * -parse_exp_in_context (char **stringptr, struct block *block, int comma, +parse_exp_in_context (char **stringptr, struct symtab_and_line sal, int comma, int void_context_p, int *out_subexp) { volatile struct gdb_exception except; @@ -1010,24 +1011,25 @@ parse_exp_in_context (char **stringptr, struct block *block, int comma, old_chain = make_cleanup (free_funcalls, 0 /*ignore*/); funcall_chain = 0; - expression_context_block = block; - - /* If no context specified, try using the current frame, if any. */ - if (!expression_context_block) - expression_context_block = get_selected_block (&expression_context_pc); + expression_context_sal = sal; + if (sal.pc) + expression_context_block = block_for_pc (sal.pc); else - expression_context_pc = BLOCK_START (expression_context_block); - - /* Fall back to using the current source static context, if any. */ - - if (!expression_context_block) { - struct symtab_and_line cursal = get_current_source_symtab_and_line (); - if (cursal.symtab) - expression_context_block - = BLOCKVECTOR_BLOCK (BLOCKVECTOR (cursal.symtab), STATIC_BLOCK); + /* If no context specified, try using the current frame, if any. */ + CORE_ADDR pc; + expression_context_block = get_selected_block (&pc); if (expression_context_block) - expression_context_pc = BLOCK_START (expression_context_block); + expression_context_sal = find_pc_line (pc, 0); + else + { + /* Fall back to using the current source static context, if any. */ + expression_context_sal = get_current_source_symtab_and_line (); + if (expression_context_sal.symtab) + expression_context_block + = BLOCKVECTOR_BLOCK (BLOCKVECTOR (expression_context_sal.symtab), + STATIC_BLOCK); + } } expout_size = 10; @@ -1088,7 +1090,7 @@ struct expression * parse_expression (char *string) { struct expression *exp; - exp = parse_exp_1 (&string, 0, 0); + exp = parse_exp_1 (&string, empty_sal (), 0); if (*string) error (_("Junk after end of expression.")); return exp; @@ -1110,7 +1112,7 @@ parse_field_expression (char *string, char **name) TRY_CATCH (except, RETURN_MASK_ALL) { in_parse_field = 1; - exp = parse_exp_in_context (&string, 0, 0, 0, &subexp); + exp = parse_exp_in_context (&string, empty_sal (), 0, 0, &subexp); } in_parse_field = 0; if (except.reason < 0 || ! exp) diff --git a/gdb/parser-defs.h b/gdb/parser-defs.h index f49ff9e..65bb4ca 100644 --- a/gdb/parser-defs.h +++ b/gdb/parser-defs.h @@ -37,11 +37,11 @@ extern int expout_ptr; extern struct block *expression_context_block; -/* If expression_context_block is non-zero, then this is the PC within - the block that we want to evaluate expressions at. When debugging - C or C++ code, we use this to find the exact line we're at, and - then look up the macro definitions active at that point. */ -extern CORE_ADDR expression_context_pc; +/* The precise location at which to evaluate the expression. When + debugging C or C++ code, we use this to find the exact line we're + at, and then look up the macro definitions active at that + point. */ +extern struct symtab_and_line expression_context_sal; /* The innermost context required by the stack and register variables we've encountered so far. */ diff --git a/gdb/symtab.c b/gdb/symtab.c index db84f4f..3825c4c 100644 --- a/gdb/symtab.c +++ b/gdb/symtab.c @@ -698,6 +698,15 @@ init_sal (struct symtab_and_line *sal) sal->explicit_pc = 0; sal->explicit_line = 0; } + +/* Return a newly-initialized symtab_and_line. */ +struct symtab_and_line +empty_sal (void) +{ + struct symtab_and_line result; + init_sal (&result); + return result; +} /* Return 1 if the two sections are the same, or if they could diff --git a/gdb/symtab.h b/gdb/symtab.h index 31aed86..0b5aad5 100644 --- a/gdb/symtab.h +++ b/gdb/symtab.h @@ -1202,6 +1202,8 @@ extern int find_line_pc_range (struct symtab_and_line, CORE_ADDR *, extern void resolve_sal_pc (struct symtab_and_line *); +extern struct symtab_and_line empty_sal (void); + /* Given a string, return the line specified by it. For commands like "list" and "breakpoint". */ diff --git a/gdb/testsuite/gdb.base/macscp.exp b/gdb/testsuite/gdb.base/macscp.exp index 3424714..c2f004a 100644 --- a/gdb/testsuite/gdb.base/macscp.exp +++ b/gdb/testsuite/gdb.base/macscp.exp @@ -408,13 +408,23 @@ gdb_test "break [gdb_get_line_number "set breakpoint here"]" \ "Breakpoint.*at.* file .*, line.*" \ "breakpoint macscp_expr" +gdb_test "cond \$bpnum foo == M" \ + "No symbol \"M\" in current context\." \ + "macro M not in scope at breakpoint" + +# Note that we choose the condition so that this breakpoint never +# stops. +gdb_test "break [gdb_get_line_number "set second breakpoint here"] if foo != M" \ + "" \ + "breakpoint macscp_expr using M" + gdb_test "continue" "foo = 0;.*" "continue to macsp_expr" gdb_test "print M" \ "No symbol \"M\" in current context\." \ "print expression with macro before define." -gdb_test "next" "foo = 1;" "next to definition" +gdb_test "next" "foo = 1;.*here.*/" "next to definition" gdb_test "print M" \ " = 0" \ diff --git a/gdb/testsuite/gdb.base/macscp1.c b/gdb/testsuite/gdb.base/macscp1.c index 200ac26..684cf85 100644 --- a/gdb/testsuite/gdb.base/macscp1.c +++ b/gdb/testsuite/gdb.base/macscp1.c @@ -70,7 +70,7 @@ macscp_expr (void) foo = 0; /* set breakpoint here */ #define M foo - foo = 1; + foo = 1; /* set second breakpoint here */ #undef M foo = 2; } diff --git a/gdb/tracepoint.c b/gdb/tracepoint.c index 671a63a..e075521 100644 --- a/gdb/tracepoint.c +++ b/gdb/tracepoint.c @@ -974,7 +974,7 @@ validate_actionline (char **line, struct tracepoint *t) } /* else fall thru, treat p as an expression and parse it! */ } - exp = parse_exp_1 (&p, block_for_pc (t->address), 1); + exp = parse_exp_1 (&p, find_pc_line (t->address, 0), 1); old_chain = make_cleanup (free_current_contents, &exp); if (exp->elts[0].opcode == OP_VAR_VALUE) @@ -1566,7 +1566,7 @@ encode_actions (struct tracepoint *t, char ***tdp_actions, struct agent_reqs areqs; exp = parse_exp_1 (&action_exp, - block_for_pc (t->address), 1); + find_pc_line (t->address, 0), 1); old_chain = make_cleanup (free_current_contents, &exp); switch (exp->elts[0].opcode) diff --git a/gdb/varobj.c b/gdb/varobj.c index 3a0bf5e..1075989 100644 --- a/gdb/varobj.c +++ b/gdb/varobj.c @@ -449,6 +449,7 @@ varobj_create (char *objname, struct frame_info *old_fi = NULL; struct block *block; struct cleanup *old_chain; + struct symtab_and_line sal; /* Fill out a varobj structure for the (root) variable being constructed. */ var = new_root_variable (); @@ -488,7 +489,10 @@ varobj_create (char *objname, innermost_block = NULL; /* Wrap the call to parse expression, so we can return a sensible error. */ - if (!gdb_parse_exp_1 (&p, block, 0, &var->root->exp)) + init_sal (&sal); + if (fi != NULL) + find_frame_sal (fi, &sal); + if (!gdb_parse_exp_1 (&p, sal, 0, &var->root->exp)) { return NULL; } @@ -898,7 +902,7 @@ varobj_set_value (struct varobj *var, char *expression) gdb_assert (varobj_editable_p (var)); input_radix = 10; /* ALWAYS reset to decimal temporarily */ - exp = parse_exp_1 (&s, 0, 0); + exp = parse_exp_1 (&s, empty_sal (), 0); if (!gdb_evaluate_expression (exp, &value)) { /* We cannot proceed without a valid expression. */ diff --git a/gdb/wrapper.c b/gdb/wrapper.c index 1cbfdbc..54f0bef 100644 --- a/gdb/wrapper.c +++ b/gdb/wrapper.c @@ -22,14 +22,14 @@ #include "ui-out.h" int -gdb_parse_exp_1 (char **stringptr, struct block *block, int comma, +gdb_parse_exp_1 (char **stringptr, struct symtab_and_line sal, int comma, struct expression **expression) { volatile struct gdb_exception except; TRY_CATCH (except, RETURN_MASK_ERROR) { - *expression = parse_exp_1 (stringptr, block, comma); + *expression = parse_exp_1 (stringptr, sal, comma); } if (except.reason < 0) diff --git a/gdb/wrapper.h b/gdb/wrapper.h index 670918f..6a44265 100644 --- a/gdb/wrapper.h +++ b/gdb/wrapper.h @@ -23,8 +23,9 @@ struct value; struct expression; struct block; +struct symtab_and_line; -extern int gdb_parse_exp_1 (char **, struct block *, +extern int gdb_parse_exp_1 (char **, struct symtab_and_line, int, struct expression **); extern int gdb_evaluate_expression (struct expression *, struct value **);