This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
PR gdb/856
- From: Tom Tromey <tromey at redhat dot com>
- To: gdb-patches at sourceware dot org
- Date: Wed, 03 Sep 2008 10:57:03 -0600
- Subject: PR gdb/856
- Reply-to: tromey at redhat dot com
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 <tromey@redhat.com>
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 <tromey@redhat.com>
* 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 **);