This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
disable breakpoints with invalid condition (Re: [Patch] Cannot set pending bp if condition set explicitly)
- From: Pedro Alves <palves at redhat dot com>
- Cc: Marc Khouzam <marc dot khouzam at ericsson dot com>, "'Tom Tromey'" <tromey at redhat dot com>, "'gdb-patches at sourceware dot org'" <gdb-patches at sourceware dot org>
- Date: Fri, 03 Aug 2012 01:44:02 +0100
- Subject: disable breakpoints with invalid condition (Re: [Patch] Cannot set pending bp if condition set explicitly)
- References: <F7CE05678329534C957159168FA70DEC5C24CAEA18@EUSAACMS0703.eamcs.ericsson.se> <87zk6nkghi.fsf@fleche.redhat.com> <F7CE05678329534C957159168FA70DEC5C2500353F@EUSAACMS0703.eamcs.ericsson.se> <501194E0.5040109@redhat.com> <F7CE05678329534C957159168FA70DEC5C24D64A2E@EUSAACMS0703.eamcs.ericsson.se> <5016A5DD.8040502@redhat.com>
On 07/30/2012 04:18 PM, Pedro Alves wrote:
>>> >> This currently works:
>>> >>
>>> >> (gdb) b main if 1 == 1 thread 1
>>> >> Breakpoint 2 at 0x4572bb: file ../../src/gdb/gdb.c, line 29.
>>> >>
>>> >> But this does not:
>>> >>
>>> >> (gdb) condition 2 1 == 1 thread 1
>>> >> Junk at end of expression
>> >
>> > On a side-note, I see that although there is an error printed and
>> > set_breakpoint_condition() is aborted, the bad condition is not
>> > cleaned-up, as shown here:
>> >
>> > (gdb) info b
>> > Num Type Disp Enb Address What
>> > 1 breakpoint keep y 0x0804851d in main at ../../../src/gdb/testsuite/gdb.base/pending.c:26
>> > stop only if k==1 thread 1
>> >
>> > Shouldn't there be some kind of cleanup in set_breakpoint_condition()?
> Indeed. Hmm, that, or disable the location, with a warning. We parse the condition in the
> context of each location. A condition might well make sense and parse okay
> for a location, but not for another (e.g., different locals/parameters). Disabling
> is also what we do if we fail to parse a location later on with "break foo if bar".
>
Here's a patch implementing this idea. Examples (currently, the condition text
is left stale, and the breakpoint becomes uncondition):
(gdb) b main
Breakpoint 2 at 0x4572bb: file ../../src/gdb/gdb.c, line 29.
(gdb) condition 2 foo bar
warning: Breakpoint 2.1 disabled: No symbol "foo" in current context.
(gdb) info breakpoints
Num Type Disp Enb Address What
2 breakpoint keep y <MULTIPLE>
stop only if foo bar
2.1 n 0x00000000004572bb in main at ../../src/gdb/gdb.c:29
(gdb) condition 2
Breakpoint 2 now unconditional.
(gdb) enable 2.1
(gdb) info breakpoints
Num Type Disp Enb Address What
2 breakpoint keep y 0x00000000004572bb in main at ../../src/gdb/gdb.c:29
(gdb) watch args
Hardware watchpoint 3: args
(gdb) condition 3
Breakpoint 3 now unconditional.
(gdb) condition 3 foo bar
warning: Watchpoint 3 disabled: No symbol "foo" in current context.
(gdb) info breakpoints
Num Type Disp Enb Address What
2 breakpoint keep y 0x00000000004572bb in main at ../../src/gdb/gdb.c:29
3 hw watchpoint keep n args
stop only if foo bar
(gdb) condition 3 1 == 1 thread 1
warning: Watchpoint 3 disabled - junk at end of expression: thread 1
(gdb)
There are several little details we get wrong that I haven't addressed.
For instance, in this case, the re-enabling of a watchpoint is prevented:
(gdb) condition 3 asdf
warning: Watchpoint 3 disabled: No symbol "asdf" in current context.
(gdb) info breakpoints 3
Num Type Disp Enb Address What
3 hw watchpoint keep n args
stop only if asdf
breakpoint already hit 1 time
(gdb) enable 3
Cannot enable watchpoint 3: No symbol "asdf" in current context.
While this doesn't:
(gdb) condition 3 1 == 1 thread 1
warning: Watchpoint 3 disabled - junk at end of expression: thread 1
(gdb) info breakpoints
Num Type Disp Enb Address What
2 breakpoint keep y 0x00000000004572bb in main at ../../src/gdb/gdb.c:29
3 hw watchpoint keep n args
stop only if 1 == 1 thread 1
breakpoint already hit 1 time
(gdb) enable 3
(gdb) info breakpoints
Num Type Disp Enb Address What
2 breakpoint keep y 0x00000000004572bb in main at ../../src/gdb/gdb.c:29
3 hw watchpoint keep y args
stop only if 1 == 1 thread 1
breakpoint already hit 1 time
(gdb)
For regular breakpoints, you can always "enable" them, but when you let
the program run, you'll find they are unconditional, and if something causes
a re-set, they get disabled again:
warning: failed to reevaluate condition for breakpoint 4: blah blah.
I'd appreciate hearing opinions on whether this change is a good direction or not.
macscp.exp had a case of an invalid condition being set on a breakpoint, and then
the test assuming the breakpoint would be hit.
2012-08-02 Pedro Alves <palves@redhat.com>
gdb/
* breakpoint.c (set_breakpoint_condition): If condition fails to
parse, disable the watchpoint or breakpoint location, and warn.
gdb/doc/
* gdb.texinfo (condition command): Mention that locations are
disabled when the condition has a problem.
gdb/testsuite/
* gdb.base/condbreak.exp: New tests for invalid conditions with
"condition" command.
* gdb.base/macscp.exp: After setting an invalid condition to a
breakpoint, make the breakpoint unconditional and re-enable it.
---
gdb/testsuite/gdb.base/condbreak.exp | 9 +++++++++
gdb/testsuite/gdb.base/macscp.exp | 7 +++++++
2 files changed, 16 insertions(+)
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 03719d4..b8484a7 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -956,26 +956,59 @@ set_breakpoint_condition (struct breakpoint *b, char *exp,
if (is_watchpoint (b))
{
struct watchpoint *w = (struct watchpoint *) b;
+ volatile struct gdb_exception e;
innermost_block = NULL;
arg = exp;
- w->cond_exp = parse_exp_1 (&arg, 0, 0, 0);
- if (*arg)
- error (_("Junk at end of expression"));
- w->cond_exp_valid_block = innermost_block;
+ TRY_CATCH (e, RETURN_MASK_ERROR)
+ {
+ w->cond_exp = parse_exp_1 (&arg, 0, 0, 0);
+ }
+ if (e.reason < 0)
+ {
+ b->enable_state = bp_disabled;
+ warning (_("Watchpoint %d disabled: %s"),
+ b->number, e.message);
+ }
+ else if (*arg)
+ {
+ b->enable_state = bp_disabled;
+ warning (_("Watchpoint %d disabled - "
+ "junk at end of expression: %s"),
+ b->number, arg);
+ }
+ else
+ w->cond_exp_valid_block = innermost_block;
}
else
{
struct bp_location *loc;
+ int loc_n;
- for (loc = b->loc; loc; loc = loc->next)
+ for (loc = b->loc, loc_n = 1; loc; loc = loc->next, loc_n++)
{
+ volatile struct gdb_exception e;
+
arg = exp;
- loc->cond =
- parse_exp_1 (&arg, loc->address,
- block_for_pc (loc->address), 0);
- if (*arg)
- error (_("Junk at end of expression"));
+
+ TRY_CATCH (e, RETURN_MASK_ERROR)
+ {
+ loc->cond = parse_exp_1 (&arg, loc->address,
+ block_for_pc (loc->address), 0);
+ }
+ if (e.reason < 0)
+ {
+ loc->enabled = 0;
+ warning (_("Breakpoint %d.%d disabled: %s"),
+ b->number, loc_n, e.message);
+ }
+ else if (*arg)
+ {
+ loc->enabled = 0;
+ warning (_("Breakpoint %d.%d disabled - "
+ "junk at end of expression: %s"),
+ b->number, loc_n, arg);
+ }
}
}
}
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index facca8f..8518d2f 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -4487,11 +4487,12 @@ breakpoint @var{bnum} stops your program only if the value of
@code{condition}, @value{GDBN} checks @var{expression} immediately for
syntactic correctness, and to determine whether symbols in it have
referents in the context of your breakpoint. If @var{expression} uses
-symbols not referenced in the context of the breakpoint, @value{GDBN}
-prints an error message:
+symbols not referenced in the context any of the breakpoint's
+locations, @value{GDBN} prints a warning message and disables the
+corresponding location:
@smallexample
-No symbol "foo" in current context.
+warning: Breakpoint 1.1 disabled: No symbol "foo" in current context.
@end smallexample
@noindent
diff --git a/gdb/testsuite/gdb.base/condbreak.exp b/gdb/testsuite/gdb.base/condbreak.exp
index e8e0d84..9d91fab 100644
--- a/gdb/testsuite/gdb.base/condbreak.exp
+++ b/gdb/testsuite/gdb.base/condbreak.exp
@@ -266,3 +266,12 @@ gdb_test "complete cond 1" "cond 1"
gdb_test "set variable \$var = 1"
gdb_test "complete cond \$v" "cond \\\$var"
gdb_test "complete cond 1 values\[0\].a" "cond 1 values.0..a_field"
+
+gdb_test "cond 1 foo bar" ".*" "set invalid condition"
+
+gdb_test "info break 1" \
+ "Num Type\[ \]+Disp Enb Address\[ \]+What.*
+1\[\t \]+breakpoint keep y.* <MULTIPLE>.*
+\[\t \]+stop only if foo bar.*
+1.1\[\t \]+n\[\t \]+.*in main at .*$srcfile:$bp_location6.*" \
+ "location with invalid condition is disabled"
diff --git a/gdb/testsuite/gdb.base/macscp.exp b/gdb/testsuite/gdb.base/macscp.exp
index 1f995d5..ad18245 100644
--- a/gdb/testsuite/gdb.base/macscp.exp
+++ b/gdb/testsuite/gdb.base/macscp.exp
@@ -444,6 +444,13 @@ gdb_test "cond \$bpnum foo == MACRO_TO_EXPAND" \
"No symbol \"MACRO_TO_EXPAND\" in current context\." \
"macro MACRO_TO_EXPAND not in scope at breakpoint"
+gdb_test "cond \$bpnum" \
+ "Breakpoint .* now unconditional\." \
+ "make breakpoint unconditional"
+
+gdb_test_no_output "enable \$bpnum.1" \
+ "re-enable breakpoint"
+
# Note that we choose the condition so that this breakpoint never
# stops.
set l2 [gdb_get_line_number "set second breakpoint here"]