* [PATCH] fix PR-15501
@ 2013-08-13 10:03 Muhammad Waqas
2013-08-13 17:37 ` Pedro Alves
0 siblings, 1 reply; 11+ messages in thread
From: Muhammad Waqas @ 2013-08-13 10:03 UTC (permalink / raw)
To: gdb-patches
Hi
GDB enable/disable command does not work correctly as it should be.
http://sourceware.org/bugzilla/show_bug.cgi?id=15501
Addition to Pedro examples.
if we execute following commands these will be executed
without an error.
(gdb) info b
Num Type Disp Enb Address What
1 breakpoint keep y 0x00000000004004b8 in main at 13929.c:13
2 breakpoint keep y 0x00000000004004b8 in main at 13929.c:13
(gdb) disable 1 fooo.1
(gdb) info break
Num Type Disp Enb Address What
1 breakpoint keep y <MULTIPLE>
1.1 n 0x00000000004004b8 in main at 13929.c:13
2 breakpoint keep y 0x00000000004004b8 in main at 13929.c:13
It should disable breakpoint 1 and error on fooo but what gdb did, it disable 1.1
surprisingly.
I am prposing patch for this bug.
Workaround:
Pars args and handle them one by one if it contain period or not and do what it
requires(disable/enable breakpoint or location).
gdb\Changlog
2013-08-13 Muhammad Waqas <mwaqas@codesourcery.com>
PR gdb/15501
* breakpoint.c (enable_command): Handle multiple arguments properly.
(disable_command): Handle multiple arguments properly.
Index: breakpoint.c
===================================================================
RCS file: /cvs/src/src/gdb/breakpoint.c,v
retrieving revision 1.773
diff -u -p -r1.773 breakpoint.c
--- breakpoint.c 24 Jul 2013 19:50:32 -0000 1.773
+++ breakpoint.c 13 Aug 2013 07:55:49 -0000
@@ -14553,25 +14553,35 @@ disable_command (char *args, int from_tt
if (user_breakpoint_p (bpt))
disable_breakpoint (bpt);
}
- else if (strchr (args, '.'))
+ else
{
- struct bp_location *loc = find_location_by_number (args);
- if (loc)
+ char *num = extract_arg (&args);
+
+ while (num)
{
- if (loc->enabled)
+ if (strchr (num, '.'))
{
- loc->enabled = 0;
- mark_breakpoint_location_modified (loc);
+ struct bp_location *loc = find_location_by_number (num);
+
+ if (loc)
+ {
+ if (loc->enabled)
+ {
+ loc->enabled = 0;
+ mark_breakpoint_location_modified (loc);
+ }
+ if (target_supports_enable_disable_tracepoint ()
+ && current_trace_status ()->running && loc->owner
+ && is_tracepoint (loc->owner))
+ target_disable_tracepoint (loc);
+ }
+ update_global_location_list (0);
}
- if (target_supports_enable_disable_tracepoint ()
- && current_trace_status ()->running && loc->owner
- && is_tracepoint (loc->owner))
- target_disable_tracepoint (loc);
+ else
+ map_breakpoint_numbers (num, do_map_disable_breakpoint, NULL);
+ num = extract_arg (&args);
}
- update_global_location_list (0);
}
- else
- map_breakpoint_numbers (args, do_map_disable_breakpoint, NULL);
}
static void
@@ -14677,25 +14687,35 @@ enable_command (char *args, int from_tty
if (user_breakpoint_p (bpt))
enable_breakpoint (bpt);
}
- else if (strchr (args, '.'))
+ else
{
- struct bp_location *loc = find_location_by_number (args);
- if (loc)
+ char *num = extract_arg (&args);
+
+ while (num)
{
- if (!loc->enabled)
+ if (strchr (num, '.'))
{
- loc->enabled = 1;
- mark_breakpoint_location_modified (loc);
+ struct bp_location *loc = find_location_by_number (num);
+
+ if (loc)
+ {
+ if (!loc->enabled)
+ {
+ loc->enabled = 1;
+ mark_breakpoint_location_modified (loc);
+ }
+ if (target_supports_enable_disable_tracepoint ()
+ && current_trace_status ()->running && loc->owner
+ && is_tracepoint (loc->owner))
+ target_enable_tracepoint (loc);
+ }
+ update_global_location_list (1);
}
- if (target_supports_enable_disable_tracepoint ()
- && current_trace_status ()->running && loc->owner
- && is_tracepoint (loc->owner))
- target_enable_tracepoint (loc);
+ else
+ map_breakpoint_numbers (num, do_map_enable_breakpoint, NULL);
+ num = extract_arg (&args);
}
- update_global_location_list (1);
}
- else
- map_breakpoint_numbers (args, do_map_enable_breakpoint, NULL);
}
/* This struct packages up disposition data for application to multiple
testsuite\Changlog
2013-07-13 Muhammad Waqas <mwaqas@codesourccery.com>
PR gdb/15501
* gdb.base/ena-dis-br.exp: Add test to verify
enable\disable commands work correctly with arguments.
Index: ena-dis-br.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.base/ena-dis-br.exp,v
retrieving revision 1.22
diff -u -p -r1.22 ena-dis-br.exp
--- ena-dis-br.exp 27 Jun 2013 18:50:30 -0000 1.22
+++ ena-dis-br.exp 13 Aug 2013 07:57:26 -0000
@@ -301,5 +301,35 @@ gdb_test_multiple "continue 2" "$test" {
}
}
+# Verify that GDB correctly handles the "enable/disable" command with arguments.
+#
+if ![runto_main] then { fail "enable/disable break tests suppressed" }
+
+set b1 0
+set b2 0
+
+gdb_test_multiple "break main" "bp 1" {
+ -re "(Breakpoint )(\[0-9\]+)( at.* file .*$srcfile, line.*)($gdb_prompt $)" {
+ set b1 $expect_out(2,string)
+ pass "breakpoint main 1"
+ }
+}
+
+gdb_test_multiple "break main" "bp 2" {
+ -re "(Breakpoint )(\[0-9\]+)( at.* file .*$srcfile, line.*)($gdb_prompt $)" {
+ set b2 $expect_out(2,string)
+ pass "breakpoint main 2"
+ }
+}
+
+gdb_test_no_output "disable $b1.1 $b2.1" "disable command"
+gdb_test "info break" \
+ "(${b1}.1)(\[^\n\r\]*)( n.*)(${b2}.1)(\[^\n\r\]*)( n.*)" \
+ "disable ${b1}.1 and ${b2}.1"
+
+gdb_test "disable $b1 fooo.1" \
+ "Bad breakpoint number 'fooo'" \
+ "handle multiple args"
+
gdb_exit
return 0
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] fix PR-15501
2013-08-13 10:03 [PATCH] fix PR-15501 Muhammad Waqas
@ 2013-08-13 17:37 ` Pedro Alves
2013-08-15 10:32 ` Muhammad Waqas
0 siblings, 1 reply; 11+ messages in thread
From: Pedro Alves @ 2013-08-13 17:37 UTC (permalink / raw)
To: Muhammad Waqas; +Cc: gdb-patches
On 08/13/2013 11:02 AM, Muhammad Waqas wrote:
> GDB enable/disable command does not work correctly as it should be.
> http://sourceware.org/bugzilla/show_bug.cgi?id=15501
Thanks!
Note it'd be good to assign yourself the PR once
you start working on it, to avoid effort duplication. I
had just suggested this bug to someone else last week;
luckily he hadn't started working on it. :-)
> Addition to Pedro examples.
> if we execute following commands these will be executed
> without an error.
> (gdb) info b
> Num Type Disp Enb Address What
> 1 breakpoint keep y 0x00000000004004b8 in main at 13929.c:13
> 2 breakpoint keep y 0x00000000004004b8 in main at 13929.c:13
> (gdb) disable 1 fooo.1
> (gdb) info break
> Num Type Disp Enb Address What
> 1 breakpoint keep y <MULTIPLE>
> 1.1 n 0x00000000004004b8 in main at 13929.c:13
> 2 breakpoint keep y 0x00000000004004b8 in main at 13929.c:13
>
> It should disable breakpoint 1 and error on fooo but what gdb did, it disable 1.1
> surprisingly.
>
> I am prposing patch for this bug.
>
> Workaround:
> Pars args and handle them one by one if it contain period or not and do what it
> requires(disable/enable breakpoint or location).
>
> gdb\Changlog
>
> 2013-08-13 Muhammad Waqas <mwaqas@codesourcery.com>
>
> PR gdb/15501
> * breakpoint.c (enable_command): Handle multiple arguments properly.
> (disable_command): Handle multiple arguments properly.
"Properly" is subjective, and may change over time. ;-) Say what changed,
like so:
* breakpoint.c (enable_command, disable_command): Iterate over
all specified breakpoint locations.
> testsuite\Changlog
I can't resist saying that backslashes for dir
separators look very alien to me. :-)
> 2013-07-13 Muhammad Waqas <mwaqas@codesourccery.com>
>
> PR gdb/15501
> * gdb.base/ena-dis-br.exp: Add test to verify
> enable\disable commands work correctly with arguments.
Here too. Please use forward slashes. Say:
* gdb.base/ena-dis-br.exp: Add test to verify
enable/disable commands work correctly with
multiple arguments that include multiple locations.
> +set b1 0
> +set b2 0
> +
> +gdb_test_multiple "break main" "bp 1" {
> + -re "(Breakpoint )(\[0-9\]+)( at.* file .*$srcfile, line.*)($gdb_prompt $)" {
> + set b1 $expect_out(2,string)
> + pass "breakpoint main 1"
> + }
> +}
> +
> +gdb_test_multiple "break main" "bp 2" {
> + -re "(Breakpoint )(\[0-9\]+)( at.* file .*$srcfile, line.*)($gdb_prompt $)" {
> + set b2 $expect_out(2,string)
> + pass "breakpoint main 2"
> + }
> +}
Doesn't break_at work for this? It's defined at the top of the file.
> +
> +gdb_test_no_output "disable $b1.1 $b2.1" "disable command"
Write:
gdb_test_no_output "disable $b1.1 $b2.1" "disable \$b1.1 \$b2.1"
> +gdb_test "info break" \
> + "(${b1}.1)(\[^\n\r\]*)( n.*)(${b2}.1)(\[^\n\r\]*)( n.*)" \
> + "disable ${b1}.1 and ${b2}.1"
I think you meant "disabled". Also, this puts the real breakpoint
number in gdb.sum. It's usually better to avoid that, as something
may cause the breakpoint numbers to change, and we'd rather
gdb.sum output was stable(-ish). So, write:
gdb_test "info break" \
"(${b1}.1)(\[^\n\r\]*)( n.*)(${b2}.1)(\[^\n\r\]*)( n.*)" \
"disabled \$b1.1 and \$b2.1"
> +
> +gdb_test "disable $b1 fooo.1" \
> + "Bad breakpoint number 'fooo'" \
> + "handle multiple args"
"handle multiple args" looks like a stale string from some
earlier revision... The other test above was also
about multiple args. Just do:
gdb_test "disable $b1 fooo.1" \
"Bad breakpoint number 'fooo'" \
"disable \$b1 fooo.1"
IMO, the test is incomplete.
- The "enable" command should be tested as well.
- It'd be good to test a mix of breakpoints
and breakpoint locations. E.g., "disable $b3.1 $b4"
- The "info break" tests should ensure that the breakpoints
that were _not_ supposed to be disabled remain enabled (and
vice versa for counterpart "enable" tests. (this suggests
moving the testing code to a procedure that repeats the
same set of tests for either enable or disable).
- This part in the PR:
> In fact, everything after the first location is ignored:
>
> (gdb) disable 2.1 foofoobar
> (gdb) info breakpoints
> Num Type Disp Enb Address What
> 2 breakpoint keep y <MULTIPLE>
> 2.1 n 0x00000000004004cf in main at main.c:5
> 3 breakpoint keep y 0x00000000004004cf in main at main.c:5
> (gdb)
>
> That should warn, just like:
>
> (gdb) disable 2 foofoobar
> warning: bad breakpoint number at or near 'foofoobar'
... is not being tested. I think it should.
Would you like to extend the test a bit and resubmit?
Thanks,
--
Pedro Alves
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] fix PR-15501
2013-08-13 17:37 ` Pedro Alves
@ 2013-08-15 10:32 ` Muhammad Waqas
2013-08-19 15:28 ` Pedro Alves
2013-08-21 22:17 ` Pedro Alves
0 siblings, 2 replies; 11+ messages in thread
From: Muhammad Waqas @ 2013-08-15 10:32 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches
On 08/13/2013 10:37 PM, Pedro Alves wrote:
> On 08/13/2013 11:02 AM, Muhammad Waqas wrote:
>> GDB enable/disable command does not work correctly as it should be.
>> http://sourceware.org/bugzilla/show_bug.cgi?id=15501
> Thanks!
>
> Note it'd be good to assign yourself the PR once
> you start working on it, to avoid effort duplication. I
> had just suggested this bug to someone else last week;
> luckily he hadn't started working on it. :-)
>
ok next time I will be careful.
>> Addition to Pedro examples.
>> if we execute following commands these will be executed
>> without an error.
>> (gdb) info b
>> Num Type Disp Enb Address What
>> 1 breakpoint keep y 0x00000000004004b8 in main at 13929.c:13
>> 2 breakpoint keep y 0x00000000004004b8 in main at 13929.c:13
>> (gdb) disable 1 fooo.1
>> (gdb) info break
>> Num Type Disp Enb Address What
>> 1 breakpoint keep y <MULTIPLE>
>> 1.1 n 0x00000000004004b8 in main at 13929.c:13
>> 2 breakpoint keep y 0x00000000004004b8 in main at 13929.c:13
>>
>> It should disable breakpoint 1 and error on fooo but what gdb did, it disable 1.1
>> surprisingly.
>>
>> I am prposing patch for this bug.
>>
>> Workaround:
>> Pars args and handle them one by one if it contain period or not and do what it
>> requires(disable/enable breakpoint or location).
>>
>> gdb\Changlog
>>
>> 2013-08-13 Muhammad Waqas <mwaqas@codesourcery.com>
>>
>> PR gdb/15501
>> * breakpoint.c (enable_command): Handle multiple arguments properly.
>> (disable_command): Handle multiple arguments properly.
> "Properly" is subjective, and may change over time. ;-) Say what changed,
> like so:
>
> * breakpoint.c (enable_command, disable_command): Iterate over
> all specified breakpoint locations.
>
>> testsuite\Changlog
> I can't resist saying that backslashes for dir
> separators look very alien to me. :-)
>
>> 2013-07-13 Muhammad Waqas <mwaqas@codesourccery.com>
>>
>> PR gdb/15501
>> * gdb.base/ena-dis-br.exp: Add test to verify
>> enable\disable commands work correctly with arguments.
> Here too. Please use forward slashes. Say:
>
> * gdb.base/ena-dis-br.exp: Add test to verify
> enable/disable commands work correctly with
> multiple arguments that include multiple locations.
>
>
>> +set b1 0
>> +set b2 0
>> +
>> +gdb_test_multiple "break main" "bp 1" {
>> + -re "(Breakpoint )(\[0-9\]+)( at.* file .*$srcfile, line.*)($gdb_prompt $)" {
>> + set b1 $expect_out(2,string)
>> + pass "breakpoint main 1"
>> + }
>> +}
>> +
>> +gdb_test_multiple "break main" "bp 2" {
>> + -re "(Breakpoint )(\[0-9\]+)( at.* file .*$srcfile, line.*)($gdb_prompt $)" {
>> + set b2 $expect_out(2,string)
>> + pass "breakpoint main 2"
>> + }
>> +}
> Doesn't break_at work for this? It's defined at the top of the file.
>
>> +
>> +gdb_test_no_output "disable $b1.1 $b2.1" "disable command"
> Write:
>
> gdb_test_no_output "disable $b1.1 $b2.1" "disable \$b1.1 \$b2.1"
>
>> +gdb_test "info break" \
>> + "(${b1}.1)(\[^\n\r\]*)( n.*)(${b2}.1)(\[^\n\r\]*)( n.*)" \
>> + "disable ${b1}.1 and ${b2}.1"
> I think you meant "disabled". Also, this puts the real breakpoint
> number in gdb.sum. It's usually better to avoid that, as something
> may cause the breakpoint numbers to change, and we'd rather
> gdb.sum output was stable(-ish). So, write:
>
> gdb_test "info break" \
> "(${b1}.1)(\[^\n\r\]*)( n.*)(${b2}.1)(\[^\n\r\]*)( n.*)" \
> "disabled \$b1.1 and \$b2.1"
>
>> +
>> +gdb_test "disable $b1 fooo.1" \
>> + "Bad breakpoint number 'fooo'" \
>> + "handle multiple args"
> "handle multiple args" looks like a stale string from some
> earlier revision... The other test above was also
> about multiple args. Just do:
>
> gdb_test "disable $b1 fooo.1" \
> "Bad breakpoint number 'fooo'" \
> "disable \$b1 fooo.1"
>
>
> IMO, the test is incomplete.
>
> - The "enable" command should be tested as well.
> - It'd be good to test a mix of breakpoints
> and breakpoint locations. E.g., "disable $b3.1 $b4"
> - The "info break" tests should ensure that the breakpoints
> that were _not_ supposed to be disabled remain enabled (and
> vice versa for counterpart "enable" tests. (this suggests
> moving the testing code to a procedure that repeats the
> same set of tests for either enable or disable).
> - This part in the PR:
> ok
>> In fact, everything after the first location is ignored:
>>
>> (gdb) disable 2.1 foofoobar
>> (gdb) info breakpoints
>> Num Type Disp Enb Address What
>> 2 breakpoint keep y <MULTIPLE>
>> 2.1 n 0x00000000004004cf in main at main.c:5
>> 3 breakpoint keep y 0x00000000004004cf in main at main.c:5
>> (gdb)
>>
>> That should warn, just like:
>>
>> (gdb) disable 2 foofoobar
>> warning: bad breakpoint number at or near 'foofoobar'
> ... is not being tested. I think it should.
>
> Would you like to extend the test a bit and resubmit?
>
> Thanks,
Thanks for reviewing my patch.
Here are the things that you mention to correct.
gdb/Changlog
2013-08-12 Muhammad Waqas <mwaqas@codesourcery.com>
PR gdb/15501
* breakpoint.c (enable_command, disable_command): Iterate over
all specified breakpoint locations.
testsuite/Changlog
2013-07-12 Muhammad Waqas <mwaqas@codesourccery.com>
PR gdb/15501
* gdb.base/ena-dis-br.exp: Add test to verify
enable/disable commands work correctly with
multiple arguments that include multiple locations.
extended testcase
Index: ena-dis-br.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.base/ena-dis-br.exp,v
retrieving revision 1.22
diff -u -p -r1.22 ena-dis-br.exp
--- ena-dis-br.exp 27 Jun 2013 18:50:30 -0000 1.22
+++ ena-dis-br.exp 15 Aug 2013 09:12:04 -0000
@@ -301,5 +301,88 @@ gdb_test_multiple "continue 2" "$test" {
}
}
+# Verify that GDB correctly handles the "enable/disable" command with arguments that
+# include multiple locations.
+#
+if ![runto_main] then { fail "enable/disable break tests suppressed" }
+
+set b1 0
+set b2 0
+set b3 0
+set b4 0
+
+set b1 [break_at main ""]
+set b2 [break_at main ""]
+set b3 [break_at main ""]
+set b4 [break_at main ""]
+
+# This proc will work correctly If args will be according to below explaned values
+#
+# If "what" = "disable" then
+# "what_res" = "n"
+# "p1" = "pass"
+# "p2" = "fail".
+#
+# If "what" = "enable" then
+# "what_res" = "y"
+# "p1" = "fail"
+# "p2" = "pass".
+
+proc test_ena_dis_br { what what_res p1 p2 } {
+ global b1
+ global b2
+ global b3
+ global b4
+ global gdb_prompt
+
+ gdb_test_no_output "$what $b1.1 $b2.1" "$what \$b1.1 \$b2.1"
+ set test1 "$what \$b1.1 and \$b2.1"
+
+ gdb_test_multiple "info break" "$test1" {
+ -re "(${b1}.1)(\[^\n\r\]*)( n.*)(${b2}.1)(\[^\n\r\]*)( n.*)$gdb_prompt $" {
+ $p1 "$test1"
+ }
+ -re ".*$gdb_prompt $" {
+ $p2 "$test1"
+ }
+ }
+
+ gdb_test "$what $b1 fooo.1" \
+ "Bad breakpoint number 'fooo'" \
+ "$what \$b1 fooo.1"
+
+ gdb_test "info break" \
+ "(${b1})(\[^\n\r]*)( $what_res.*)" \
+ "${what}d \$b1"
+
+ gdb_test_no_output "$what $b4 $b3.1" "$what \$b4 \$b3.1"
+ set test1 "${what}d \$b4 and \$b3.1,remain enabled \$b3"
+
+ gdb_test_multiple "info break" "$test1" {
+ -re "(${b3})(\[^\n\r]*)( y.*)(${b3}.1)(\[^\n\r\]*)( n.*)(${b4})(\[^\n\r\]*)( $what_res.*)$gdb_prompt $" {
+ $p1 "$test1"
+ }
+ -re "(${b3})(\[^\n\r]*)( y.*)(${b4})(\[^\n\r\]*)( $what_res.*)$gdb_prompt $" {
+ $p2 "$test1"
+ }
+ }
+
+ gdb_test "$what $b4.1 fooobaar" \
+ "warning: bad breakpoint number at or near 'fooobaar'" \
+ "$what \$b4.1 fooobar"
+ set test1 "${what}d \$b4.1"
+
+ gdb_test_multiple "info break" "$test1" {
+ -re "(${b4}.1)(\[^\n\r\]*)( n.*)$gdb_prompt $" {
+ $p1 "$test1"
+ }
+ -re ".*$gdb_prompt $" {
+ $p2 "$test1"
+ }
+ }
+}
+
+test_ena_dis_br "disable" "n" "pass" "fail"
+test_ena_dis_br "enable" "y" "fail" "pass"
+
gdb_exit
return 0
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] fix PR-15501
2013-08-15 10:32 ` Muhammad Waqas
@ 2013-08-19 15:28 ` Pedro Alves
2013-08-20 6:45 ` Waqas, Muhammad
2013-08-21 22:17 ` Pedro Alves
1 sibling, 1 reply; 11+ messages in thread
From: Pedro Alves @ 2013-08-19 15:28 UTC (permalink / raw)
To: Muhammad Waqas; +Cc: gdb-patches
Hi!
Unfortunately, I'm having trouble applying your patches. Seems like
they're mangled somehow. I see you're using Thunderbird.
Could you go through:
http://sourceware.org/gdb/wiki/ContributionChecklist#Submitting_patches
and fix it up as necessary (and then check if you can apply
a patch that you privately send yourself?)
Thanks,
--
Pedro Alves
^ permalink raw reply [flat|nested] 11+ messages in thread
* RE: [PATCH] fix PR-15501
2013-08-19 15:28 ` Pedro Alves
@ 2013-08-20 6:45 ` Waqas, Muhammad
0 siblings, 0 replies; 11+ messages in thread
From: Waqas, Muhammad @ 2013-08-20 6:45 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches
[-- Attachment #1: Type: text/plain, Size: 769 bytes --]
Hi!
Sorry for inconvenience. please find attachment for the patch. I'm unable to set
mail editor settings after long try. But attached file will be applied as
I had tested them.
________________________________________
From: Pedro Alves [palves@redhat.com]
Sent: Monday, August 19, 2013 8:28 PM
To: Waqas, Muhammad
Cc: gdb-patches@sourceware.org
Subject: Re: [PATCH] fix PR-15501
Hi!
Unfortunately, I'm having trouble applying your patches. Seems like
they're mangled somehow. I see you're using Thunderbird.
Could you go through:
http://sourceware.org/gdb/wiki/ContributionChecklist#Submitting_patches
and fix it up as necessary (and then check if you can apply
a patch that you privately send yourself?)
Thanks,
--
Pedro Alves
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: breakpoint.patch --]
[-- Type: text/x-patch; name="breakpoint.patch", Size: 3055 bytes --]
Index: breakpoint.c
===================================================================
RCS file: /cvs/src/src/gdb/breakpoint.c,v
retrieving revision 1.773
diff -u -p -r1.773 breakpoint.c
--- breakpoint.c 24 Jul 2013 19:50:32 -0000 1.773
+++ breakpoint.c 20 Aug 2013 05:55:02 -0000
@@ -14553,25 +14553,35 @@ disable_command (char *args, int from_tt
if (user_breakpoint_p (bpt))
disable_breakpoint (bpt);
}
- else if (strchr (args, '.'))
+ else
{
- struct bp_location *loc = find_location_by_number (args);
- if (loc)
+ char *num = extract_arg (&args);
+
+ while (num)
{
- if (loc->enabled)
+ if (strchr (num, '.'))
{
- loc->enabled = 0;
- mark_breakpoint_location_modified (loc);
+ struct bp_location *loc = find_location_by_number (num);
+
+ if (loc)
+ {
+ if (loc->enabled)
+ {
+ loc->enabled = 0;
+ mark_breakpoint_location_modified (loc);
+ }
+ if (target_supports_enable_disable_tracepoint ()
+ && current_trace_status ()->running && loc->owner
+ && is_tracepoint (loc->owner))
+ target_disable_tracepoint (loc);
+ }
+ update_global_location_list (0);
}
- if (target_supports_enable_disable_tracepoint ()
- && current_trace_status ()->running && loc->owner
- && is_tracepoint (loc->owner))
- target_disable_tracepoint (loc);
+ else
+ map_breakpoint_numbers (num, do_map_disable_breakpoint, NULL);
+ num = extract_arg (&args);
}
- update_global_location_list (0);
}
- else
- map_breakpoint_numbers (args, do_map_disable_breakpoint, NULL);
}
static void
@@ -14677,25 +14687,35 @@ enable_command (char *args, int from_tty
if (user_breakpoint_p (bpt))
enable_breakpoint (bpt);
}
- else if (strchr (args, '.'))
+ else
{
- struct bp_location *loc = find_location_by_number (args);
- if (loc)
+ char *num = extract_arg (&args);
+
+ while (num)
{
- if (!loc->enabled)
+ if (strchr (num, '.'))
{
- loc->enabled = 1;
- mark_breakpoint_location_modified (loc);
+ struct bp_location *loc = find_location_by_number (num);
+
+ if (loc)
+ {
+ if (!loc->enabled)
+ {
+ loc->enabled = 1;
+ mark_breakpoint_location_modified (loc);
+ }
+ if (target_supports_enable_disable_tracepoint ()
+ && current_trace_status ()->running && loc->owner
+ && is_tracepoint (loc->owner))
+ target_enable_tracepoint (loc);
+ }
+ update_global_location_list (1);
}
- if (target_supports_enable_disable_tracepoint ()
- && current_trace_status ()->running && loc->owner
- && is_tracepoint (loc->owner))
- target_enable_tracepoint (loc);
+ else
+ map_breakpoint_numbers (num, do_map_enable_breakpoint, NULL);
+ num = extract_arg (&args);
}
- update_global_location_list (1);
}
- else
- map_breakpoint_numbers (args, do_map_enable_breakpoint, NULL);
}
/* This struct packages up disposition data for application to multiple
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: ena-dis-br.patch --]
[-- Type: text/x-patch; name="ena-dis-br.patch", Size: 2639 bytes --]
Index: ena-dis-br.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.base/ena-dis-br.exp,v
retrieving revision 1.22
diff -u -p -r1.22 ena-dis-br.exp
--- ena-dis-br.exp 27 Jun 2013 18:50:30 -0000 1.22
+++ ena-dis-br.exp 15 Aug 2013 09:12:04 -0000
@@ -301,5 +301,88 @@ gdb_test_multiple "continue 2" "$test" {
}
}
+# Verify that GDB correctly handles the "enable/disable" command with arguments.
+#
+if ![runto_main] then { fail "enable/disable break tests suppressed" }
+
+set b1 0
+set b2 0
+set b3 0
+set b4 0
+
+set b1 [break_at main ""]
+set b2 [break_at main ""]
+set b3 [break_at main ""]
+set b4 [break_at main ""]
+
+# This proc will work correctly If args will be according to below explaned values
+#
+# If "what" = "disable" then
+# "what_res" = "n"
+# "p1" = "pass"
+# "p2" = "fail".
+#
+# If "what" = "enable" then
+# "what_res" = "y"
+# "p1" = "fail"
+# "p2" = "pass".
+
+proc test_ena_dis_br { what what_res p1 p2 } {
+ global b1
+ global b2
+ global b3
+ global b4
+ global gdb_prompt
+
+ gdb_test_no_output "$what $b1.1 $b2.1" "$what \$b1.1 \$b2.1"
+ set test1 "$what \$b1.1 and \$b2.1"
+
+ gdb_test_multiple "info break" "$test1" {
+ -re "(${b1}.1)(\[^\n\r\]*)( n.*)(${b2}.1)(\[^\n\r\]*)( n.*)$gdb_prompt $" {
+ $p1 "$test1"
+ }
+ -re ".*$gdb_prompt $" {
+ $p2 "$test1"
+ }
+ }
+
+ gdb_test "$what $b1 fooo.1" \
+ "Bad breakpoint number 'fooo'" \
+ "$what \$b1 fooo.1"
+
+ gdb_test "info break" \
+ "(${b1})(\[^\n\r]*)( $what_res.*)" \
+ "${what}d \$b1"
+
+ gdb_test_no_output "$what $b4 $b3.1" "$what \$b4 \$b3.1"
+ set test1 "${what}d \$b4 and \$b3.1,remain enabled \$b3"
+
+ gdb_test_multiple "info break" "$test1" {
+ -re "(${b3})(\[^\n\r]*)( y.*)(${b3}.1)(\[^\n\r\]*)( n.*)(${b4})(\[^\n\r\]*)( $what_res.*)$gdb_prompt $" {
+ $p1 "$test1"
+ }
+ -re "(${b3})(\[^\n\r]*)( y.*)(${b4})(\[^\n\r\]*)( $what_res.*)$gdb_prompt $" {
+ $p2 "$test1"
+ }
+ }
+
+ gdb_test "$what $b4.1 fooobaar" \
+ "warning: bad breakpoint number at or near 'fooobaar'" \
+ "$what \$b4.1 fooobar"
+ set test1 "${what}d \$b4.1"
+
+ gdb_test_multiple "info break" "$test1" {
+ -re "(${b4}.1)(\[^\n\r\]*)( n.*)$gdb_prompt $" {
+ $p1 "$test1"
+ }
+ -re ".*$gdb_prompt $" {
+ $p2 "$test1"
+ }
+ }
+}
+
+test_ena_dis_br "disable" "n" "pass" "fail"
+test_ena_dis_br "enable" "y" "fail" "pass"
+
gdb_exit
return 0
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] fix PR-15501
2013-08-15 10:32 ` Muhammad Waqas
2013-08-19 15:28 ` Pedro Alves
@ 2013-08-21 22:17 ` Pedro Alves
2013-08-22 9:36 ` Waqas, Muhammad
1 sibling, 1 reply; 11+ messages in thread
From: Pedro Alves @ 2013-08-21 22:17 UTC (permalink / raw)
To: Muhammad Waqas; +Cc: gdb-patches
On 08/15/2013 11:32 AM, Muhammad Waqas wrote:
> On 08/13/2013 10:37 PM, Pedro Alves wrote:
>> > On 08/13/2013 11:02 AM, Muhammad Waqas wrote:
>>> >> GDB enable/disable command does not work correctly as it should be.
>>> >> http://sourceware.org/bugzilla/show_bug.cgi?id=15501
>> > Thanks!
>> >
>> > Note it'd be good to assign yourself the PR once
>> > you start working on it, to avoid effort duplication. I
>> > had just suggested this bug to someone else last week;
>> > luckily he hadn't started working on it. :-)
>> >
> ok next time I will be careful.
>
>>> >> Addition to Pedro examples.
>>> >> if we execute following commands these will be executed
>>> >> without an error.
>>> >> (gdb) info b
>>> >> Num Type Disp Enb Address What
>>> >> 1 breakpoint keep y 0x00000000004004b8 in main at 13929.c:13
>>> >> 2 breakpoint keep y 0x00000000004004b8 in main at 13929.c:13
>>> >> (gdb) disable 1 fooo.1
>>> >> (gdb) info break
>>> >> Num Type Disp Enb Address What
>>> >> 1 breakpoint keep y <MULTIPLE>
>>> >> 1.1 n 0x00000000004004b8 in main at 13929.c:13
>>> >> 2 breakpoint keep y 0x00000000004004b8 in main at 13929.c:13
>>> >>
>>> >> It should disable breakpoint 1 and error on fooo but what gdb did, it disable 1.1
>>> >> surprisingly.
>>> >>
>>> >> I am prposing patch for this bug.
>>> >>
>>> >> Workaround:
>>> >> Pars args and handle them one by one if it contain period or not and do what it
>>> >> requires(disable/enable breakpoint or location).
>>> >>
>>> >> gdb\Changlog
>>> >>
>>> >> 2013-08-13 Muhammad Waqas <mwaqas@codesourcery.com>
>>> >>
>>> >> PR gdb/15501
>>> >> * breakpoint.c (enable_command): Handle multiple arguments properly.
>>> >> (disable_command): Handle multiple arguments properly.
>> > "Properly" is subjective, and may change over time. ;-) Say what changed,
>> > like so:
>> >
>> > * breakpoint.c (enable_command, disable_command): Iterate over
>> > all specified breakpoint locations.
>> >
>>> >> testsuite\Changlog
>> > I can't resist saying that backslashes for dir
>> > separators look very alien to me. :-)
>> >
>>> >> 2013-07-13 Muhammad Waqas <mwaqas@codesourccery.com>
>>> >>
>>> >> PR gdb/15501
>>> >> * gdb.base/ena-dis-br.exp: Add test to verify
>>> >> enable\disable commands work correctly with arguments.
>> > Here too. Please use forward slashes. Say:
>> >
>> > * gdb.base/ena-dis-br.exp: Add test to verify
>> > enable/disable commands work correctly with
>> > multiple arguments that include multiple locations.
>> >
>> >
>>> >> +set b1 0
>>> >> +set b2 0
>>> >> +
>>> >> +gdb_test_multiple "break main" "bp 1" {
>>> >> + -re "(Breakpoint )(\[0-9\]+)( at.* file .*$srcfile, line.*)($gdb_prompt $)" {
>>> >> + set b1 $expect_out(2,string)
>>> >> + pass "breakpoint main 1"
>>> >> + }
>>> >> +}
>>> >> +
>>> >> +gdb_test_multiple "break main" "bp 2" {
>>> >> + -re "(Breakpoint )(\[0-9\]+)( at.* file .*$srcfile, line.*)($gdb_prompt $)" {
>>> >> + set b2 $expect_out(2,string)
>>> >> + pass "breakpoint main 2"
>>> >> + }
>>> >> +}
>> > Doesn't break_at work for this? It's defined at the top of the file.
>> >
>>> >> +
>>> >> +gdb_test_no_output "disable $b1.1 $b2.1" "disable command"
>> > Write:
>> >
>> > gdb_test_no_output "disable $b1.1 $b2.1" "disable \$b1.1 \$b2.1"
>> >
>>> >> +gdb_test "info break" \
>>> >> + "(${b1}.1)(\[^\n\r\]*)( n.*)(${b2}.1)(\[^\n\r\]*)( n.*)" \
>>> >> + "disable ${b1}.1 and ${b2}.1"
>> > I think you meant "disabled". Also, this puts the real breakpoint
>> > number in gdb.sum. It's usually better to avoid that, as something
>> > may cause the breakpoint numbers to change, and we'd rather
>> > gdb.sum output was stable(-ish). So, write:
>> >
>> > gdb_test "info break" \
>> > "(${b1}.1)(\[^\n\r\]*)( n.*)(${b2}.1)(\[^\n\r\]*)( n.*)" \
>> > "disabled \$b1.1 and \$b2.1"
>> >
>>> >> +
>>> >> +gdb_test "disable $b1 fooo.1" \
>>> >> + "Bad breakpoint number 'fooo'" \
>>> >> + "handle multiple args"
>> > "handle multiple args" looks like a stale string from some
>> > earlier revision... The other test above was also
>> > about multiple args. Just do:
>> >
>> > gdb_test "disable $b1 fooo.1" \
>> > "Bad breakpoint number 'fooo'" \
>> > "disable \$b1 fooo.1"
>> >
>> >
>> > IMO, the test is incomplete.
>> >
>> > - The "enable" command should be tested as well.
>> > - It'd be good to test a mix of breakpoints
>> > and breakpoint locations. E.g., "disable $b3.1 $b4"
>> > - The "info break" tests should ensure that the breakpoints
>> > that were _not_ supposed to be disabled remain enabled (and
>> > vice versa for counterpart "enable" tests. (this suggests
>> > moving the testing code to a procedure that repeats the
>> > same set of tests for either enable or disable).
>> > - This part in the PR:
>> > ok
>>> >> In fact, everything after the first location is ignored:
>>> >>
>>> >> (gdb) disable 2.1 foofoobar
>>> >> (gdb) info breakpoints
>>> >> Num Type Disp Enb Address What
>>> >> 2 breakpoint keep y <MULTIPLE>
>>> >> 2.1 n 0x00000000004004cf in main at main.c:5
>>> >> 3 breakpoint keep y 0x00000000004004cf in main at main.c:5
>>> >> (gdb)
>>> >>
>>> >> That should warn, just like:
>>> >>
>>> >> (gdb) disable 2 foofoobar
>>> >> warning: bad breakpoint number at or near 'foofoobar'
>> > ... is not being tested. I think it should.
>> >
>> > Would you like to extend the test a bit and resubmit?
>> >
>> > Thanks,
> Thanks for reviewing my patch.
>
> Here are the things that you mention to correct.
>
>
>
> gdb/Changlog
>
> 2013-08-12 Muhammad Waqas <mwaqas@codesourcery.com>
>
> PR gdb/15501
> * breakpoint.c (enable_command, disable_command): Iterate over
> all specified breakpoint locations.
>
> testsuite/Changlog
>
> 2013-07-12 Muhammad Waqas <mwaqas@codesourccery.com>
>
> PR gdb/15501
> * gdb.base/ena-dis-br.exp: Add test to verify
> enable/disable commands work correctly with
> multiple arguments that include multiple locations.
>
> extended testcase
>
> Index: ena-dis-br.exp
> ===================================================================
> RCS file: /cvs/src/src/gdb/testsuite/gdb.base/ena-dis-br.exp,v
> retrieving revision 1.22
> diff -u -p -r1.22 ena-dis-br.exp
> --- ena-dis-br.exp 27 Jun 2013 18:50:30 -0000 1.22
> +++ ena-dis-br.exp 15 Aug 2013 09:12:04 -0000
> @@ -301,5 +301,88 @@ gdb_test_multiple "continue 2" "$test" {
> }
> }
>
> +# Verify that GDB correctly handles the "enable/disable" command with arguments that
> +# include multiple locations.
Line too long.
> +#
> +if ![runto_main] then { fail "enable/disable break tests suppressed" }
> +
> +set b1 0
> +set b2 0
> +set b3 0
> +set b4 0
> +
> +set b1 [break_at main ""]
> +set b2 [break_at main ""]
> +set b3 [break_at main ""]
> +set b4 [break_at main ""]
> +
> +# This proc will work correctly If args will be according to below explaned values
# This proc will work correctly If args will be according to below explaned values
- "will work correctly" is unfortunately not a useful
indication of what the function actually does.
- "If" is uppercased for no reason.
- Typo "explaned".
- Missing period.
> +#
> +# If "what" = "disable" then
> +# "what_res" = "n"
> +# "p1" = "pass"
> +# "p2" = "fail".
> +#
> +# If "what" = "enable" then
> +# "what_res" = "y"
> +# "p1" = "fail"
> +# "p2" = "pass".
> +
Should e.g., say that WHAT is a command, etc.
> +proc test_ena_dis_br { what what_res p1 p2 } {
> + global b1
> + global b2
> + global b3
> + global b4
> + global gdb_prompt
> +
> + gdb_test_no_output "$what $b1.1 $b2.1" "$what \$b1.1 \$b2.1"
> + set test1 "$what \$b1.1 and \$b2.1"
Should be "${what}d" here too, right?
Please add some comments in the body of the function,
with a minimal explanation of what the multiple testing steps
are doing. It'll make it much easier to follow. As is,
it's hard to grok. Things like, "Now disable foo. bar
should remain enabled.", etc.
> +
> + gdb_test_multiple "info break" "$test1" {
> + -re "(${b1}.1)(\[^\n\r\]*)( n.*)(${b2}.1)(\[^\n\r\]*)( n.*)$gdb_prompt $" {
> + $p1 "$test1"
> + }
> + -re ".*$gdb_prompt $" {
> + $p2 "$test1"
> + }
> + }
> +
> + gdb_test "$what $b1 fooo.1" \
> + "Bad breakpoint number 'fooo'" \
> + "$what \$b1 fooo.1"
> +
> + gdb_test "info break" \
> + "(${b1})(\[^\n\r]*)( $what_res.*)" \
> + "${what}d \$b1"
> +
> + gdb_test_no_output "$what $b4 $b3.1" "$what \$b4 \$b3.1"
> + set test1 "${what}d \$b4 and \$b3.1,remain enabled \$b3"
Suggest:
set test1 "${what}d \$b4 and \$b3.1, \$b3 remains enabled"
Actually, shouldn't "enabled" here be the opposite of "${what}d" ?
Here's what we now get:
PASS: gdb.base/ena-dis-br.exp: disabled $b4 and $b3.1,remain enabled $b3
...
PASS: gdb.base/ena-dis-br.exp: enabled $b4 and $b3.1,remain enabled $b3
> +
> + gdb_test_multiple "info break" "$test1" {
> + -re "(${b3})(\[^\n\r]*)( y.*)(${b3}.1)(\[^\n\r\]*)( n.*)(${b4})(\[^\n\r\]*)( $what_res.*)$gdb_prompt $" {
> + $p1 "$test1"
> + }
> + -re "(${b3})(\[^\n\r]*)( y.*)(${b4})(\[^\n\r\]*)( $what_res.*)$gdb_prompt $" {
> + $p2 "$test1"
> + }
> + }
> +
> + gdb_test "$what $b4.1 fooobaar" \
> + "warning: bad breakpoint number at or near 'fooobaar'" \
> + "$what \$b4.1 fooobar"
> + set test1 "${what}d \$b4.1"
> +
> + gdb_test_multiple "info break" "$test1" {
> + -re "(${b4}.1)(\[^\n\r\]*)( n.*)$gdb_prompt $" {
> + $p1 "$test1"
> + }
> + -re ".*$gdb_prompt $" {
> + $p2 "$test1"
> + }
> + }
> +}
> +
> +test_ena_dis_br "disable" "n" "pass" "fail"
> +test_ena_dis_br "enable" "y" "fail" "pass"
> +
> gdb_exit
> return 0
This patch adds a series of trailing whitespace. Please fix
that up.
--
Pedro Alves
^ permalink raw reply [flat|nested] 11+ messages in thread
* RE: [PATCH] fix PR-15501
2013-08-21 22:17 ` Pedro Alves
@ 2013-08-22 9:36 ` Waqas, Muhammad
2013-08-22 12:02 ` Pedro Alves
0 siblings, 1 reply; 11+ messages in thread
From: Waqas, Muhammad @ 2013-08-22 9:36 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches
[-- Attachment #1: Type: text/plain, Size: 18116 bytes --]
Thanks for reviewing patch.
________________________________________
From: Pedro Alves [palves@redhat.com]
Sent: Thursday, August 22, 2013 3:17 AM
To: Waqas, Muhammad
Cc: gdb-patches@sourceware.org
Subject: Re: [PATCH] fix PR-15501
On 08/15/2013 11:32 AM, Muhammad Waqas wrote:
> On 08/13/2013 10:37 PM, Pedro Alves wrote:
>> > On 08/13/2013 11:02 AM, Muhammad Waqas wrote:
>>> >> GDB enable/disable command does not work correctly as it should be.
>>> >> http://sourceware.org/bugzilla/show_bug.cgi?id=15501
>> > Thanks!
>> >
>> > Note it'd be good to assign yourself the PR once
>> > you start working on it, to avoid effort duplication. I
>> > had just suggested this bug to someone else last week;
>> > luckily he hadn't started working on it. :-)
>> >
> ok next time I will be careful.
>
>>> >> Addition to Pedro examples.
>>> >> if we execute following commands these will be executed
>>> >> without an error.
>>> >> (gdb) info b
>>> >> Num Type Disp Enb Address What
>>> >> 1 breakpoint keep y 0x00000000004004b8 in main at 13929.c:13
>>> >> 2 breakpoint keep y 0x00000000004004b8 in main at 13929.c:13
>>> >> (gdb) disable 1 fooo.1
>>> >> (gdb) info break
>>> >> Num Type Disp Enb Address What
>>> >> 1 breakpoint keep y <MULTIPLE>
>>> >> 1.1 n 0x00000000004004b8 in main at 13929.c:13
>>> >> 2 breakpoint keep y 0x00000000004004b8 in main at 13929.c:13
>>> >>
>>> >> It should disable breakpoint 1 and error on fooo but what gdb did, it disable 1.1
>>> >> surprisingly.
>>> >>
>>> >> I am prposing patch for this bug.
>>> >>
>>> >> Workaround:
>>> >> Pars args and handle them one by one if it contain period or not and do what it
>>> >> requires(disable/enable breakpoint or location).
>>> >>
>>> >> gdb\Changlog
>>> >>
>>> >> 2013-08-13 Muhammad Waqas <mwaqas@codesourcery.com>
>>> >>
>>> >> PR gdb/15501
>>> >> * breakpoint.c (enable_command): Handle multiple arguments properly.
>>> >> (disable_command): Handle multiple arguments properly.
>> > "Properly" is subjective, and may change over time. ;-) Say what changed,
>> > like so:
>> >
>> > * breakpoint.c (enable_command, disable_command): Iterate over
>> > all specified breakpoint locations.
>> >
>>> >> testsuite\Changlog
>> > I can't resist saying that backslashes for dir
>> > separators look very alien to me. :-)
>> >
>>> >> 2013-07-13 Muhammad Waqas <mwaqas@codesourccery.com>
>>> >>
>>> >> PR gdb/15501
>>> >> * gdb.base/ena-dis-br.exp: Add test to verify
>>> >> enable\disable commands work correctly with arguments.
>> > Here too. Please use forward slashes. Say:
>> >
>> > * gdb.base/ena-dis-br.exp: Add test to verify
>> > enable/disable commands work correctly with
>> > multiple arguments that include multiple locations.
>> >
>> >
>>> >> +set b1 0
>>> >> +set b2 0
>>> >> +
>>> >> +gdb_test_multiple "break main" "bp 1" {
>>> >> + -re "(Breakpoint )(\[0-9\]+)( at.* file .*$srcfile, line.*)($gdb_prompt $)" {
>>> >> + set b1 $expect_out(2,string)
>>> >> + pass "breakpoint main 1"
>>> >> + }
>>> >> +}
>>> >> +
>>> >> +gdb_test_multiple "break main" "bp 2" {
>>> >> + -re "(Breakpoint )(\[0-9\]+)( at.* file .*$srcfile, line.*)($gdb_prompt $)" {
>>> >> + set b2 $expect_out(2,string)
>>> >> + pass "breakpoint main 2"
>>> >> + }
>>> >> +}
>> > Doesn't break_at work for this? It's defined at the top of the file.
>> >
>>> >> +
>>> >> +gdb_test_no_output "disable $b1.1 $b2.1" "disable command"
>> > Write:
>> >
>> > gdb_test_no_output "disable $b1.1 $b2.1" "disable \$b1.1 \$b2.1"
>> >
>>> >> +gdb_test "info break" \
>>> >> + "(${b1}.1)(\[^\n\r\]*)( n.*)(${b2}.1)(\[^\n\r\]*)( n.*)" \
>>> >> + "disable ${b1}.1 and ${b2}.1"
>> > I think you meant "disabled". Also, this puts the real breakpoint
>> > number in gdb.sum. It's usually better to avoid that, as something
>> > may cause the breakpoint numbers to change, and we'd rather
>> > gdb.sum output was stable(-ish). So, write:
>> >
>> > gdb_test "info break" \
>> > "(${b1}.1)(\[^\n\r\]*)( n.*)(${b2}.1)(\[^\n\r\]*)( n.*)" \
>> > "disabled \$b1.1 and \$b2.1"
>> >
>>> >> +
>>> >> +gdb_test "disable $b1 fooo.1" \
>>> >> + "Bad breakpoint number 'fooo'" \
>>> >> + "handle multiple args"
>> > "handle multiple args" looks like a stale string from some
>> > earlier revision... The other test above was also
>> > about multiple args. Just do:
>> >
>> > gdb_test "disable $b1 fooo.1" \
>> > "Bad breakpoint number 'fooo'" \
>> > "disable \$b1 fooo.1"
>> >
>> >
>> > IMO, the test is incomplete.
>> >
>> > - The "enable" command should be tested as well.
>> > - It'd be good to test a mix of breakpoints
>> > and breakpoint locations. E.g., "disable $b3.1 $b4"
>> > - The "info break" tests should ensure that the breakpoints
>> > that were _not_ supposed to be disabled remain enabled (and
>> > vice versa for counterpart "enable" tests. (this suggests
>> > moving the testing code to a procedure that repeats the
>> > same set of tests for either enable or disable).
>> > - This part in the PR:
>> > ok
>>> >> In fact, everything after the first location is ignored:
>>> >>
>>> >> (gdb) disable 2.1 foofoobar
>>> >> (gdb) info breakpoints
>>> >> Num Type Disp Enb Address What
>>> >> 2 breakpoint keep y <MULTIPLE>
>>> >> 2.1 n 0x00000000004004cf in main at main.c:5
>>> >> 3 breakpoint keep y 0x00000000004004cf in main at main.c:5
>>> >> (gdb)
>>> >>
>>> >> That should warn, just like:
>>> >>
>>> >> (gdb) disable 2 foofoobar
>>> >> warning: bad breakpoint number at or near 'foofoobar'
>> > ... is not being tested. I think it should.
>> >
>> > Would you like to extend the test a bit and resubmit?
>> >
>> > Thanks,
> Thanks for reviewing my patch.
>
> Here are the things that you mention to correct.
>
>
>
> gdb/Changlog
>
> 2013-08-12 Muhammad Waqas <mwaqas@codesourcery.com>
>
> PR gdb/15501
> * breakpoint.c (enable_command, disable_command): Iterate over
> all specified breakpoint locations.
>
> testsuite/Changlog
>
> 2013-07-12 Muhammad Waqas <mwaqas@codesourccery.com>
>
> PR gdb/15501
> * gdb.base/ena-dis-br.exp: Add test to verify
> enable/disable commands work correctly with
> multiple arguments that include multiple locations.
>
> extended testcase
>
> Index: ena-dis-br.exp
> ===================================================================
> RCS file: /cvs/src/src/gdb/testsuite/gdb.base/ena-dis-br.exp,v
> retrieving revision 1.22
> diff -u -p -r1.22 ena-dis-br.exp
> --- ena-dis-br.exp 27 Jun 2013 18:50:30 -0000 1.22
> +++ ena-dis-br.exp 15 Aug 2013 09:12:04 -0000
> @@ -301,5 +301,88 @@ gdb_test_multiple "continue 2" "$test" {
> }
> }
>
> +# Verify that GDB correctly handles the "enable/disable" command with arguments that
> +# include multiple locations.
Line too long.
fixed
> +#
> +if ![runto_main] then { fail "enable/disable break tests suppressed" }
> +
> +set b1 0
> +set b2 0
> +set b3 0
> +set b4 0
> +
> +set b1 [break_at main ""]
> +set b2 [break_at main ""]
> +set b3 [break_at main ""]
> +set b4 [break_at main ""]
> +
> +# This proc will work correctly If args will be according to below explaned values
# This proc will work correctly If args will be according to below explaned values
- "will work correctly" is unfortunately not a useful
indication of what the function actually does.
- "If" is uppercased for no reason.
- Typo "explaned".
- Missing period.
fixed
> +#
> +# If "what" = "disable" then
> +# "what_res" = "n"
> +# "p1" = "pass"
> +# "p2" = "fail".
> +#
> +# If "what" = "enable" then
> +# "what_res" = "y"
> +# "p1" = "fail"
> +# "p2" = "pass".
> +
Should e.g., say that WHAT is a command, etc.
fixed
> +proc test_ena_dis_br { what what_res p1 p2 } {
> + global b1
> + global b2
> + global b3
> + global b4
> + global gdb_prompt
> +
> + gdb_test_no_output "$what $b1.1 $b2.1" "$what \$b1.1 \$b2.1"
> + set test1 "$what \$b1.1 and \$b2.1"
Should be "${what}d" here too, right?
fixed
Please add some comments in the body of the function,
with a minimal explanation of what the multiple testing steps
are doing. It'll make it much easier to follow. As is,
it's hard to grok. Things like, "Now disable foo. bar
should remain enabled.", etc.
fixed
> +
> + gdb_test_multiple "info break" "$test1" {
> + -re "(${b1}.1)(\[^\n\r\]*)( n.*)(${b2}.1)(\[^\n\r\]*)( n.*)$gdb_prompt $" {
> + $p1 "$test1"
> + }
> + -re ".*$gdb_prompt $" {
> + $p2 "$test1"
> + }
> + }
> +
> + gdb_test "$what $b1 fooo.1" \
> + "Bad breakpoint number 'fooo'" \
> + "$what \$b1 fooo.1"
> +
> + gdb_test "info break" \
> + "(${b1})(\[^\n\r]*)( $what_res.*)" \
> + "${what}d \$b1"
> +
> + gdb_test_no_output "$what $b4 $b3.1" "$what \$b4 \$b3.1"
> + set test1 "${what}d \$b4 and \$b3.1,remain enabled \$b3"
Suggest:
set test1 "${what}d \$b4 and \$b3.1, \$b3 remains enabled"
Actually, shouldn't "enabled" here be the opposite of "${what}d" ?
Here's what we now get:
PASS: gdb.base/ena-dis-br.exp: disabled $b4 and $b3.1,remain enabled $b3
...
PASS: gdb.base/ena-dis-br.exp: enabled $b4 and $b3.1,remain enabled $b3
Yes it should be and fixed it.
> +
> + gdb_test_multiple "info break" "$test1" {
> + -re "(${b3})(\[^\n\r]*)( y.*)(${b3}.1)(\[^\n\r\]*)( n.*)(${b4})(\[^\n\r\]*)( $what_res.*)$gdb_prompt $" {
> + $p1 "$test1"
> + }
> + -re "(${b3})(\[^\n\r]*)( y.*)(${b4})(\[^\n\r\]*)( $what_res.*)$gdb_prompt $" {
> + $p2 "$test1"
> + }
> + }
> +
> + gdb_test "$what $b4.1 fooobaar" \
> + "warning: bad breakpoint number at or near 'fooobaar'" \
> + "$what \$b4.1 fooobar"
> + set test1 "${what}d \$b4.1"
> +
> + gdb_test_multiple "info break" "$test1" {
> + -re "(${b4}.1)(\[^\n\r\]*)( n.*)$gdb_prompt $" {
> + $p1 "$test1"
> + }
> + -re ".*$gdb_prompt $" {
> + $p2 "$test1"
> + }
> + }
> +}
> +
> +test_ena_dis_br "disable" "n" "pass" "fail"
> +test_ena_dis_br "enable" "y" "fail" "pass"
> +
> gdb_exit
> return 0
This patch adds a series of trailing whitespace. Please fix
that up.
fixed
--
Pedro Alves
Please also find this patch in attachment.
gdb/ChangeLog
2013-08-12 Muhammad Waqas <mwaqas@codesourcery.com>
PR gdb/15501
* breakpoint.c (enable_command, disable_command): Iterate over
all specified breakpoint locations.
Index: breakpoint.c
===================================================================
RCS file: /cvs/src/src/gdb/breakpoint.c,v
retrieving revision 1.773
diff -u -p -r1.773 breakpoint.c
--- breakpoint.c 24 Jul 2013 19:50:32 -0000 1.773
+++ breakpoint.c 22 Aug 2013 06:37:27 -0000
@@ -14553,25 +14553,35 @@ disable_command (char *args, int from_tt
if (user_breakpoint_p (bpt))
disable_breakpoint (bpt);
}
- else if (strchr (args, '.'))
+ else
{
- struct bp_location *loc = find_location_by_number (args);
- if (loc)
+ char *num = extract_arg (&args);
+
+ while (num)
{
- if (loc->enabled)
+ if (strchr (num, '.'))
{
- loc->enabled = 0;
- mark_breakpoint_location_modified (loc);
+ struct bp_location *loc = find_location_by_number (num);
+
+ if (loc)
+ {
+ if (loc->enabled)
+ {
+ loc->enabled = 0;
+ mark_breakpoint_location_modified (loc);
+ }
+ if (target_supports_enable_disable_tracepoint ()
+ && current_trace_status ()->running && loc->owner
+ && is_tracepoint (loc->owner))
+ target_disable_tracepoint (loc);
+ }
+ update_global_location_list (0);
}
- if (target_supports_enable_disable_tracepoint ()
- && current_trace_status ()->running && loc->owner
- && is_tracepoint (loc->owner))
- target_disable_tracepoint (loc);
+ else
+ map_breakpoint_numbers (num, do_map_disable_breakpoint, NULL);
+ num = extract_arg (&args);
}
- update_global_location_list (0);
}
- else
- map_breakpoint_numbers (args, do_map_disable_breakpoint, NULL);
}
static void
@@ -14677,25 +14687,35 @@ enable_command (char *args, int from_tty
if (user_breakpoint_p (bpt))
enable_breakpoint (bpt);
}
- else if (strchr (args, '.'))
+ else
{
- struct bp_location *loc = find_location_by_number (args);
- if (loc)
+ char *num = extract_arg (&args);
+
+ while (num)
{
- if (!loc->enabled)
+ if (strchr (num, '.'))
{
- loc->enabled = 1;
- mark_breakpoint_location_modified (loc);
+ struct bp_location *loc = find_location_by_number (num);
+
+ if (loc)
+ {
+ if (!loc->enabled)
+ {
+ loc->enabled = 1;
+ mark_breakpoint_location_modified (loc);
+ }
+ if (target_supports_enable_disable_tracepoint ()
+ && current_trace_status ()->running && loc->owner
+ && is_tracepoint (loc->owner))
+ target_enable_tracepoint (loc);
+ }
+ update_global_location_list (1);
}
- if (target_supports_enable_disable_tracepoint ()
- && current_trace_status ()->running && loc->owner
- && is_tracepoint (loc->owner))
- target_enable_tracepoint (loc);
+ else
+ map_breakpoint_numbers (num, do_map_enable_breakpoint, NULL);
+ num = extract_arg (&args);
}
- update_global_location_list (1);
}
- else
- map_breakpoint_numbers (args, do_map_enable_breakpoint, NULL);
}
/* This struct packages up disposition data for application to multiple
testsuite/ChangLog
2013-07-12 Muhammad Waqas <mwaqas@codesourccery.com>
PR gdb/15501
* gdb.base/ena-dis-br.exp: Add test to verify
enable/disable commands work correctly with
multiple arguments that include multiple locations.
Index: ena-dis-br.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.base/ena-dis-br.exp,v
retrieving revision 1.22
diff -u -p -r1.22 ena-dis-br.exp
--- ena-dis-br.exp 27 Jun 2013 18:50:30 -0000 1.22
+++ ena-dis-br.exp 22 Aug 2013 07:55:03 -0000
@@ -301,5 +301,116 @@ gdb_test_multiple "continue 2" "$test" {
}
}
+# Verify that GDB correctly handles the "enable/disable" command
+# with arguments, that include multiple locations.
+#
+if ![runto_main] then { fail "enable/disable break tests suppressed" }
+
+set b1 0
+set b2 0
+set b3 0
+set b4 0
+
+set b1 [break_at main ""]
+set b2 [break_at main ""]
+set b3 [break_at main ""]
+set b4 [break_at main ""]
+
+# This proc will be able to perform test for disable/enable
+# commads on multiple locations and breakpoints,
+# if args will be according to below explained values.
+# Here arg are
+# "what" is command (disable/enable),
+# "what_res" is for breakpoints should be enabled or not,
+# "p1/p2" are proc(pass/fail) but must be opposite.
+#
+# Here arg's values
+# If "what" = "disable" then
+# "what_res" = "n"
+# "p1" = "pass"
+# "p2" = "fail".
+#
+# If "what" = "enable" then
+# "what_res" = "y"
+# "p1" = "fail"
+# "p2" = "pass".
+
+proc test_ena_dis_br { what what_res p1 p2 } {
+ global b1
+ global b2
+ global b3
+ global b4
+ global gdb_prompt
+
+ # Now enable/disable $b.1 $b2.1.
+ gdb_test_no_output "$what $b1.1 $b2.1" "$what \$b1.1 \$b2.1"
+ set test1 "${what}d \$b1.1 and \$b2.1"
+
+ # Now $b1.1 and $b2.1 should be enabled/disabled
+ gdb_test_multiple "info break" "$test1" {
+ -re "(${b1}.1)(\[^\n\r\]*)( n.*)(${b2}.1)(\[^\n\r\]*)( n.*)$gdb_prompt $" {
+ $p1 "$test1"
+ }
+ -re ".*$gdb_prompt $" {
+ $p2 "$test1"
+ }
+ }
+
+ # Now enable/disbale $b1 fooo.1, it should give error on fooo.
+ gdb_test "$what $b1 fooo.1" \
+ "Bad breakpoint number 'fooo'" \
+ "$what \$b1 fooo.1"
+
+ # $b1 should be enabled/disabled.
+ gdb_test "info break" \
+ "(${b1})(\[^\n\r]*)( $what_res.*)" \
+ "${what}d \$b1"
+
+ # Here "oppos" is commmand that should be opposite of "what".
+ set oppos "enable"
+ set oppos_res "y"
+
+ if { $what == "enable" } {
+ set oppos "disable"
+ set oppos_res "n"
+
+ }
+
+ gdb_test_no_output "$oppos $b3" "$oppos \$b3"
+ gdb_test_no_output "$what $b4 $b3.1" "$what \$b4 \$b3.1"
+ set test1 "${what}d \$b4 and \$b3.1,remain ${oppos}d \$b3"
+
+ # Now $b4 $b3.1 should be enabled/disabled and
+ # $b3 should remain enabled/disabled
+ gdb_test_multiple "info break" "$test1" {
+ -re "(${b3})(\[^\n\r]*)( $oppos_res.*)(${b3}.1)(\[^\n\r\]*)( n.*)(${b4})(\[^\n\r\]*)( $what_res.*)$gdb_prompt $" {
+ $p1 "$test1"
+ }
+ -re "(${b3})(\[^\n\r]*)( $oppos_res.*)(${b4})(\[^\n\r\]*)( $what_res.*)$gdb_prompt $" {
+ $p2 "$test1"
+ }
+ }
+
+ # Now enable/disable $b4.1 fooobaar and
+ # it should give warning on fooobaar.
+ gdb_test "$what $b4.1 fooobaar" \
+ "warning: bad breakpoint number at or near 'fooobaar'" \
+ "$what \$b4.1 fooobar"
+ set test1 "${what}d \$b4.1"
+
+ # $b4.1 should be enabled/disbaled
+ gdb_test_multiple "info break" "$test1" {
+ -re "(${b4}.1)(\[^\n\r\]*)( n.*)$gdb_prompt $" {
+ $p1 "$test1"
+ }
+ -re ".*$gdb_prompt $" {
+ $p2 "$test1"
+ }
+ }
+}
+
+test_ena_dis_br "disable" "n" "pass" "fail"
+test_ena_dis_br "enable" "y" "fail" "pass"
+
gdb_exit
return 0
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: ena-dis-br.exp.patch --]
[-- Type: text/x-patch; name="ena-dis-br.exp.patch", Size: 3608 bytes --]
Index: ena-dis-br.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.base/ena-dis-br.exp,v
retrieving revision 1.22
diff -u -p -r1.22 ena-dis-br.exp
--- ena-dis-br.exp 27 Jun 2013 18:50:30 -0000 1.22
+++ ena-dis-br.exp 22 Aug 2013 07:55:03 -0000
@@ -301,5 +301,116 @@ gdb_test_multiple "continue 2" "$test" {
}
}
+# Verify that GDB correctly handles the "enable/disable" command
+# with arguments, that include multiple locations.
+#
+if ![runto_main] then { fail "enable/disable break tests suppressed" }
+
+set b1 0
+set b2 0
+set b3 0
+set b4 0
+
+set b1 [break_at main ""]
+set b2 [break_at main ""]
+set b3 [break_at main ""]
+set b4 [break_at main ""]
+
+# This proc will be able to perform test for disable/enable
+# commads on multiple locations and breakpoints,
+# if args will be according to below explained values.
+# Here arg are
+# "what" is command (disable/enable),
+# "what_res" is for breakpoints should be enabled or not,
+# "p1/p2" are proc(pass/fail) but must be opposite.
+#
+# Here arg's values
+# If "what" = "disable" then
+# "what_res" = "n"
+# "p1" = "pass"
+# "p2" = "fail".
+#
+# If "what" = "enable" then
+# "what_res" = "y"
+# "p1" = "fail"
+# "p2" = "pass".
+
+proc test_ena_dis_br { what what_res p1 p2 } {
+ global b1
+ global b2
+ global b3
+ global b4
+ global gdb_prompt
+
+ # Now enable/disable $b.1 $b2.1.
+ gdb_test_no_output "$what $b1.1 $b2.1" "$what \$b1.1 \$b2.1"
+ set test1 "${what}d \$b1.1 and \$b2.1"
+
+ # Now $b1.1 and $b2.1 should be enabled/disabled
+ gdb_test_multiple "info break" "$test1" {
+ -re "(${b1}.1)(\[^\n\r\]*)( n.*)(${b2}.1)(\[^\n\r\]*)( n.*)$gdb_prompt $" {
+ $p1 "$test1"
+ }
+ -re ".*$gdb_prompt $" {
+ $p2 "$test1"
+ }
+ }
+
+ # Now enable/disbale $b1 fooo.1, it should give error on fooo.
+ gdb_test "$what $b1 fooo.1" \
+ "Bad breakpoint number 'fooo'" \
+ "$what \$b1 fooo.1"
+
+ # $b1 should be enabled/disabled.
+ gdb_test "info break" \
+ "(${b1})(\[^\n\r]*)( $what_res.*)" \
+ "${what}d \$b1"
+
+ # Here "oppos" is commmand that should be opposite of "what".
+ set oppos "enable"
+ set oppos_res "y"
+
+ if { $what == "enable" } {
+ set oppos "disable"
+ set oppos_res "n"
+
+ }
+
+ gdb_test_no_output "$oppos $b3" "$oppos \$b3"
+ gdb_test_no_output "$what $b4 $b3.1" "$what \$b4 \$b3.1"
+ set test1 "${what}d \$b4 and \$b3.1,remain ${oppos}d \$b3"
+
+ # Now $b4 $b3.1 should be enabled/disabled and
+ # $b3 should remain enabled/disabled
+ gdb_test_multiple "info break" "$test1" {
+ -re "(${b3})(\[^\n\r]*)( $oppos_res.*)(${b3}.1)(\[^\n\r\]*)( n.*)(${b4})(\[^\n\r\]*)( $what_res.*)$gdb_prompt $" {
+ $p1 "$test1"
+ }
+ -re "(${b3})(\[^\n\r]*)( $oppos_res.*)(${b4})(\[^\n\r\]*)( $what_res.*)$gdb_prompt $" {
+ $p2 "$test1"
+ }
+ }
+
+ # Now enable/disable $b4.1 fooobaar and
+ # it should give warning on fooobaar.
+ gdb_test "$what $b4.1 fooobaar" \
+ "warning: bad breakpoint number at or near 'fooobaar'" \
+ "$what \$b4.1 fooobar"
+ set test1 "${what}d \$b4.1"
+
+ # $b4.1 should be enabled/disbaled
+ gdb_test_multiple "info break" "$test1" {
+ -re "(${b4}.1)(\[^\n\r\]*)( n.*)$gdb_prompt $" {
+ $p1 "$test1"
+ }
+ -re ".*$gdb_prompt $" {
+ $p2 "$test1"
+ }
+ }
+}
+
+test_ena_dis_br "disable" "n" "pass" "fail"
+test_ena_dis_br "enable" "y" "fail" "pass"
+
gdb_exit
return 0
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: breakpoint.c.patch --]
[-- Type: text/x-patch; name="breakpoint.c.patch", Size: 3055 bytes --]
Index: breakpoint.c
===================================================================
RCS file: /cvs/src/src/gdb/breakpoint.c,v
retrieving revision 1.773
diff -u -p -r1.773 breakpoint.c
--- breakpoint.c 24 Jul 2013 19:50:32 -0000 1.773
+++ breakpoint.c 22 Aug 2013 06:37:27 -0000
@@ -14553,25 +14553,35 @@ disable_command (char *args, int from_tt
if (user_breakpoint_p (bpt))
disable_breakpoint (bpt);
}
- else if (strchr (args, '.'))
+ else
{
- struct bp_location *loc = find_location_by_number (args);
- if (loc)
+ char *num = extract_arg (&args);
+
+ while (num)
{
- if (loc->enabled)
+ if (strchr (num, '.'))
{
- loc->enabled = 0;
- mark_breakpoint_location_modified (loc);
+ struct bp_location *loc = find_location_by_number (num);
+
+ if (loc)
+ {
+ if (loc->enabled)
+ {
+ loc->enabled = 0;
+ mark_breakpoint_location_modified (loc);
+ }
+ if (target_supports_enable_disable_tracepoint ()
+ && current_trace_status ()->running && loc->owner
+ && is_tracepoint (loc->owner))
+ target_disable_tracepoint (loc);
+ }
+ update_global_location_list (0);
}
- if (target_supports_enable_disable_tracepoint ()
- && current_trace_status ()->running && loc->owner
- && is_tracepoint (loc->owner))
- target_disable_tracepoint (loc);
+ else
+ map_breakpoint_numbers (num, do_map_disable_breakpoint, NULL);
+ num = extract_arg (&args);
}
- update_global_location_list (0);
}
- else
- map_breakpoint_numbers (args, do_map_disable_breakpoint, NULL);
}
static void
@@ -14677,25 +14687,35 @@ enable_command (char *args, int from_tty
if (user_breakpoint_p (bpt))
enable_breakpoint (bpt);
}
- else if (strchr (args, '.'))
+ else
{
- struct bp_location *loc = find_location_by_number (args);
- if (loc)
+ char *num = extract_arg (&args);
+
+ while (num)
{
- if (!loc->enabled)
+ if (strchr (num, '.'))
{
- loc->enabled = 1;
- mark_breakpoint_location_modified (loc);
+ struct bp_location *loc = find_location_by_number (num);
+
+ if (loc)
+ {
+ if (!loc->enabled)
+ {
+ loc->enabled = 1;
+ mark_breakpoint_location_modified (loc);
+ }
+ if (target_supports_enable_disable_tracepoint ()
+ && current_trace_status ()->running && loc->owner
+ && is_tracepoint (loc->owner))
+ target_enable_tracepoint (loc);
+ }
+ update_global_location_list (1);
}
- if (target_supports_enable_disable_tracepoint ()
- && current_trace_status ()->running && loc->owner
- && is_tracepoint (loc->owner))
- target_enable_tracepoint (loc);
+ else
+ map_breakpoint_numbers (num, do_map_enable_breakpoint, NULL);
+ num = extract_arg (&args);
}
- update_global_location_list (1);
}
- else
- map_breakpoint_numbers (args, do_map_enable_breakpoint, NULL);
}
/* This struct packages up disposition data for application to multiple
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] fix PR-15501
2013-08-22 9:36 ` Waqas, Muhammad
@ 2013-08-22 12:02 ` Pedro Alves
2013-08-22 13:13 ` Waqas, Muhammad
0 siblings, 1 reply; 11+ messages in thread
From: Pedro Alves @ 2013-08-22 12:02 UTC (permalink / raw)
To: Waqas, Muhammad; +Cc: gdb-patches
On 08/22/2013 10:36 AM, Waqas, Muhammad wrote:
> +# This proc will be able to perform test for disable/enable
> +# commads on multiple locations and breakpoints,
> +# if args will be according to below explained values.
> +# Here arg are
> +# "what" is command (disable/enable),
> +# "what_res" is for breakpoints should be enabled or not,
> +# "p1/p2" are proc(pass/fail) but must be opposite.
> +#
> +# Here arg's values
> +# If "what" = "disable" then
> +# "what_res" = "n"
> +# "p1" = "pass"
> +# "p2" = "fail".
> +#
> +# If "what" = "enable" then
> +# "what_res" = "y"
> +# "p1" = "fail"
> +# "p2" = "pass".
Thanks. I suggest this edit then:
# Perform tests for disable/enable commands on multiple
# locations and breakpoints.
#
# WHAT - the command to test (disable/enable),
# WHAT_RES - whether breakpoints are expected to end
# up enabled or disabled.
# P1/P2 - proc to call (pass/fail). Must be
# opposites.
#
# Furthermore, arguments must follow these rules:
#
# If "what" = "disable" then
# "what_res" = "n"
# "p1" = "pass"
# "p2" = "fail".
#
# If "what" = "enable" then
# "what_res" = "y"
# "p1" = "fail"
# "p2" = "pass".
#
> +
> +proc test_ena_dis_br { what what_res p1 p2 } {
> + global b1
> + global b2
> + global b3
> + global b4
> + global gdb_prompt
> +
> + # Now enable/disable $b.1 $b2.1.
> + gdb_test_no_output "$what $b1.1 $b2.1" "$what \$b1.1 \$b2.1"
> + set test1 "${what}d \$b1.1 and \$b2.1"
> +
> + # Now $b1.1 and $b2.1 should be enabled/disabled
> + gdb_test_multiple "info break" "$test1" {
> + -re "(${b1}.1)(\[^\n\r\]*)( n.*)(${b2}.1)(\[^\n\r\]*)( n.*)$gdb_prompt $" {
> + $p1 "$test1"
> + }
> + -re ".*$gdb_prompt $" {
> + $p2 "$test1"
> + }
> + }
> +
> + # Now enable/disbale $b1 fooo.1, it should give error on fooo.
Typo: disbale.
> + # Now enable/disbale $b1 fooo.1, it should give error on fooo.
> + gdb_test "$what $b1 fooo.1" \
> + "Bad breakpoint number 'fooo'" \
> + "$what \$b1 fooo.1"
> +
> + # $b1 should be enabled/disabled.
> + gdb_test "info break" \
> + "(${b1})(\[^\n\r]*)( $what_res.*)" \
> + "${what}d \$b1"
> +
> + # Here "oppos" is commmand that should be opposite of "what".
> + set oppos "enable"
> + set oppos_res "y"
> +
> + if { $what == "enable" } {
> + set oppos "disable"
> + set oppos_res "n"
Indentation is wrong here. If we're doing this, then
one has to wonder why does the routine expect all
these arguments:
# If "what" = "disable" then
# "what_res" = "n"
# "p1" = "pass"
# "p2" = "fail".
#
# If "what" = "enable" then
# "what_res" = "y"
# "p1" = "fail"
# "p2" = "pass".
... instead of accepting only "what", and computing
the rest from that itself.
> +
Spurious newline.
> + }
> +
> + gdb_test_no_output "$oppos $b3" "$oppos \$b3"
> + gdb_test_no_output "$what $b4 $b3.1" "$what \$b4 \$b3.1"
> + set test1 "${what}d \$b4 and \$b3.1,remain ${oppos}d \$b3"
> +
> + # Now $b4 $b3.1 should be enabled/disabled and
> + # $b3 should remain enabled/disabled
enabled/disabled ... enabled/disabled
That's confusing, and says practically nothing... They're
all either enabled or disabled. I suggest adjusting all
these comments to be written in terms of "enabled",
with the disabled state written on the right, in parens.
So that'd be:
# Now $b4 $b3.1 should be enabled(disabled) and
# $b3 should remain disabled(enabled).
So, when reading this, one now clearly understands
that $b3 should end up in the opposite state of$b4 $b3.1
(it that's what should happen).
> + gdb_test_multiple "info break" "$test1" {
> + -re "(${b3})(\[^\n\r]*)( $oppos_res.*)(${b3}.1)(\[^\n\r\]*)( n.*)(${b4})(\[^\n\r\]*)( $what_res.*)$gdb_prompt $" {
> + $p1 "$test1"
> + }
> + -re "(${b3})(\[^\n\r]*)( $oppos_res.*)(${b4})(\[^\n\r\]*)( $what_res.*)$gdb_prompt $" {
> + $p2 "$test1"
> + }
> + }
> +
> + # Now enable/disable $b4.1 fooobaar and
> + # it should give warning on fooobaar.
> + gdb_test "$what $b4.1 fooobaar" \
> + "warning: bad breakpoint number at or near 'fooobaar'" \
> + "$what \$b4.1 fooobar"
> + set test1 "${what}d \$b4.1"
> +
> + # $b4.1 should be enabled/disbaled
Typo. Please remember to always self review for typos. I
think I'm noticing them at all iterations.
> + gdb_test_multiple "info break" "$test1" {
> + -re "(${b4}.1)(\[^\n\r\]*)( n.*)$gdb_prompt $" {
> + $p1 "$test1"
> + }
> + -re ".*$gdb_prompt $" {
> + $p2 "$test1"
> + }
> + }
> +}
> +
> +test_ena_dis_br "disable" "n" "pass" "fail"
> +test_ena_dis_br "enable" "y" "fail" "pass"
> +
> gdb_exit
> return 0
Thanks,
--
Pedro Alves
^ permalink raw reply [flat|nested] 11+ messages in thread
* RE: [PATCH] fix PR-15501
2013-08-22 12:02 ` Pedro Alves
@ 2013-08-22 13:13 ` Waqas, Muhammad
2013-08-22 13:41 ` Pedro Alves
0 siblings, 1 reply; 11+ messages in thread
From: Waqas, Muhammad @ 2013-08-22 13:13 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches
[-- Attachment #1: Type: text/plain, Size: 9564 bytes --]
Thanks. Now carefully remove all typo error.
________________________________________
From: Pedro Alves [palves@redhat.com]
Sent: Thursday, August 22, 2013 5:02 PM
To: Waqas, Muhammad
Cc: gdb-patches@sourceware.org
Subject: Re: [PATCH] fix PR-15501
On 08/22/2013 10:36 AM, Waqas, Muhammad wrote:
> +# This proc will be able to perform test for disable/enable
> +# commads on multiple locations and breakpoints,
> +# if args will be according to below explained values.
> +# Here arg are
> +# "what" is command (disable/enable),
> +# "what_res" is for breakpoints should be enabled or not,
> +# "p1/p2" are proc(pass/fail) but must be opposite.
> +#
> +# Here arg's values
> +# If "what" = "disable" then
> +# "what_res" = "n"
> +# "p1" = "pass"
> +# "p2" = "fail".
> +#
> +# If "what" = "enable" then
> +# "what_res" = "y"
> +# "p1" = "fail"
> +# "p2" = "pass".
Thanks. I suggest this edit then:
# Perform tests for disable/enable commands on multiple
# locations and breakpoints.
#
# WHAT - the command to test (disable/enable),
# WHAT_RES - whether breakpoints are expected to end
# up enabled or disabled.
# P1/P2 - proc to call (pass/fail). Must be
# opposites.
#
# Furthermore, arguments must follow these rules:
#
# If "what" = "disable" then
# "what_res" = "n"
# "p1" = "pass"
# "p2" = "fail".
#
# If "what" = "enable" then
# "what_res" = "y"
# "p1" = "fail"
# "p2" = "pass".
#
> +
> +proc test_ena_dis_br { what what_res p1 p2 } {
> + global b1
> + global b2
> + global b3
> + global b4
> + global gdb_prompt
> +
> + # Now enable/disable $b.1 $b2.1.
> + gdb_test_no_output "$what $b1.1 $b2.1" "$what \$b1.1 \$b2.1"
> + set test1 "${what}d \$b1.1 and \$b2.1"
> +
> + # Now $b1.1 and $b2.1 should be enabled/disabled
> + gdb_test_multiple "info break" "$test1" {
> + -re "(${b1}.1)(\[^\n\r\]*)( n.*)(${b2}.1)(\[^\n\r\]*)( n.*)$gdb_prompt $" {
> + $p1 "$test1"
> + }
> + -re ".*$gdb_prompt $" {
> + $p2 "$test1"
> + }
> + }
> +
> + # Now enable/disbale $b1 fooo.1, it should give error on fooo.
Typo: disbale.
> + # Now enable/disbale $b1 fooo.1, it should give error on fooo.
> + gdb_test "$what $b1 fooo.1" \
> + "Bad breakpoint number 'fooo'" \
> + "$what \$b1 fooo.1"
> +
> + # $b1 should be enabled/disabled.
> + gdb_test "info break" \
> + "(${b1})(\[^\n\r]*)( $what_res.*)" \
> + "${what}d \$b1"
> +
> + # Here "oppos" is commmand that should be opposite of "what".
> + set oppos "enable"
> + set oppos_res "y"
> +
> + if { $what == "enable" } {
> + set oppos "disable"
> + set oppos_res "n"
Indentation is wrong here. If we're doing this, then
one has to wonder why does the routine expect all
these arguments:
# If "what" = "disable" then
# "what_res" = "n"
# "p1" = "pass"
# "p2" = "fail".
#
# If "what" = "enable" then
# "what_res" = "y"
# "p1" = "fail"
# "p2" = "pass".
... instead of accepting only "what", and computing
the rest from that itself.
This is good to only receive "what" and computing
the rest from that itself. I fixed this.
> +
Spurious newline.
> + }
> +
> + gdb_test_no_output "$oppos $b3" "$oppos \$b3"
> + gdb_test_no_output "$what $b4 $b3.1" "$what \$b4 \$b3.1"
> + set test1 "${what}d \$b4 and \$b3.1,remain ${oppos}d \$b3"
> +
> + # Now $b4 $b3.1 should be enabled/disabled and
> + # $b3 should remain enabled/disabled
enabled/disabled ... enabled/disabled
That's confusing, and says practically nothing... They're
all either enabled or disabled. I suggest adjusting all
these comments to be written in terms of "enabled",
with the disabled state written on the right, in parens.
So that'd be:
# Now $b4 $b3.1 should be enabled(disabled) and
# $b3 should remain disabled(enabled).
So, when reading this, one now clearly understands
that $b3 should end up in the opposite state of$b4 $b3.1
(it that's what should happen).
fixed
> + gdb_test_multiple "info break" "$test1" {
> + -re "(${b3})(\[^\n\r]*)( $oppos_res.*)(${b3}.1)(\[^\n\r\]*)( n.*)(${b4})(\[^\n\r\]*)( $what_res.*)$gdb_prompt $" {
> + $p1 "$test1"
> + }
> + -re "(${b3})(\[^\n\r]*)( $oppos_res.*)(${b4})(\[^\n\r\]*)( $what_res.*)$gdb_prompt $" {
> + $p2 "$test1"
> + }
> + }
> +
> + # Now enable/disable $b4.1 fooobaar and
> + # it should give warning on fooobaar.
> + gdb_test "$what $b4.1 fooobaar" \
> + "warning: bad breakpoint number at or near 'fooobaar'" \
> + "$what \$b4.1 fooobar"
> + set test1 "${what}d \$b4.1"
> +
> + # $b4.1 should be enabled/disbaled
Typo. Please remember to always self review for typos. I
think I'm noticing them at all iterations.
I will be careful now on.
> + gdb_test_multiple "info break" "$test1" {
> + -re "(${b4}.1)(\[^\n\r\]*)( n.*)$gdb_prompt $" {
> + $p1 "$test1"
> + }
> + -re ".*$gdb_prompt $" {
> + $p2 "$test1"
> + }
> + }
> +}
> +
> +test_ena_dis_br "disable" "n" "pass" "fail"
> +test_ena_dis_br "enable" "y" "fail" "pass"
> +
> gdb_exit
> return 0
Thanks,
--
Pedro Alves
Also find the attachment for same patch.
gdb/ChangeLog
2013-08-12 Muhammad Waqas <mwaqas@codesourcery.com>
PR gdb/15501
* breakpoint.c (enable_command, disable_command): Iterate over
all specified breakpoint locations.
testsuite/ChangeLog
2013-07-12 Muhammad Waqas <mwaqas@codesourccery.com>
PR gdb/15501
* gdb.base/ena-dis-br.exp: Add test to verify
enable/disable commands work correctly with
multiple arguments that include multiple locations.
Index: ena-dis-br.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.base/ena-dis-br.exp,v
retrieving revision 1.22
diff -u -p -r1.22 ena-dis-br.exp
--- ena-dis-br.exp 27 Jun 2013 18:50:30 -0000 1.22
+++ ena-dis-br.exp 22 Aug 2013 13:00:14 -0000
@@ -301,5 +301,114 @@ gdb_test_multiple "continue 2" "$test" {
}
}
+# Verify that GDB correctly handles the "enable/disable" command
+# with arguments, that include multiple locations.
+#
+if ![runto_main] then { fail "enable/disable break tests suppressed" }
+
+set b1 0
+set b2 0
+set b3 0
+set b4 0
+set b1 [break_at main ""]
+set b2 [break_at main ""]
+set b3 [break_at main ""]
+set b4 [break_at main ""]
+
+# Perform tests for disable/enable commands on multiple
+# locations and breakpoints.
+#
+# WHAT - the command to test (disable/enable).
+#
+proc test_ena_dis_br { what } {
+ global b1
+ global b2
+ global b3
+ global b4
+ global gdb_prompt
+
+ # OPPOS - the command opposite to WHAT.
+ # WHAT_RES - whether breakpoints are expected to end
+ # up enabled or disabled.
+ # OPPOS_RES- same as WHAT_RES but opposite.
+ # P1/P2 - proc to call (pass/fail). Must be
+ # opposites.
+ # Set variable values for disable command.
+ set oppos "enable"
+ set oppos_res "y"
+ set what_res "n"
+ set p1 "pass"
+ set p2 "fail"
+
+ if { "$what" == "enable" } {
+ # Set variable values for enable command.
+ set oppos "disable"
+ set oppos_res "n"
+ set what_res "y"
+ set p1 "fail"
+ set p2 "pass"
+
+ }
+
+ # Now enable(disable) $b.1 $b2.1.
+ gdb_test_no_output "$what $b1.1 $b2.1" "$what \$b1.1 \$b2.1"
+ set test1 "${what}d \$b1.1 and \$b2.1"
+
+ # Now $b1.1 and $b2.1 should be enabled(disabled)
+ gdb_test_multiple "info break" "$test1" {
+ -re "(${b1}.1)(\[^\n\r\]*)( n.*)(${b2}.1)(\[^\n\r\]*)( n.*)$gdb_prompt $" {
+ $p1 "$test1"
+ }
+ -re ".*$gdb_prompt $" {
+ $p2 "$test1"
+ }
+ }
+
+ # Now enable(disable) $b1 fooo.1, it should give error on fooo.
+ gdb_test "$what $b1 fooo.1" \
+ "Bad breakpoint number 'fooo'" \
+ "$what \$b1 fooo.1"
+
+ # $b1 should be enabled(disabled).
+ gdb_test "info break" \
+ "(${b1})(\[^\n\r]*)( $what_res.*)" \
+ "${what}d \$b1"
+
+ gdb_test_no_output "$oppos $b3" "$oppos \$b3"
+ gdb_test_no_output "$what $b4 $b3.1" "$what \$b4 \$b3.1"
+ set test1 "${what}d \$b4 and \$b3.1,remain ${oppos}d \$b3"
+
+ # Now $b4 $b3.1 should be enabled(disabled) and
+ # $b3 should remain disabled(enabled)
+ gdb_test_multiple "info break" "$test1" {
+ -re "(${b3})(\[^\n\r]*)( $oppos_res.*)(${b3}.1)(\[^\n\r\]*)( n.*)(${b4})(\[^\n\r\]*)( $what_res.*)$gdb_prompt $" {
+ $p1 "$test1"
+ }
+ -re "(${b3})(\[^\n\r]*)( $oppos_res.*)(${b4})(\[^\n\r\]*)( $what_res.*)$gdb_prompt $" {
+ $p2 "$test1"
+ }
+ }
+
+ # Now enable(disable) $b4.1 fooobaar and
+ # it should give warning on fooobaar.
+ gdb_test "$what $b4.1 fooobaar" \
+ "warning: bad breakpoint number at or near 'fooobaar'" \
+ "$what \$b4.1 fooobar"
+ set test1 "${what}d \$b4.1"
+
+ # $b4.1 should be enabled(disabled)
+ gdb_test_multiple "info break" "$test1" {
+ -re "(${b4}.1)(\[^\n\r\]*)( n.*)$gdb_prompt $" {
+ $p1 "$test1"
+ }
+ -re ".*$gdb_prompt $" {
+ $p2 "$test1"
+ }
+ }
+}
+
+test_ena_dis_br "disable"
+test_ena_dis_br "enable"
+
gdb_exit
return 0
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: ena-dis-br.exp.patch --]
[-- Type: text/x-patch; name="ena-dis-br.exp.patch", Size: 3590 bytes --]
Index: ena-dis-br.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.base/ena-dis-br.exp,v
retrieving revision 1.22
diff -u -p -r1.22 ena-dis-br.exp
--- ena-dis-br.exp 27 Jun 2013 18:50:30 -0000 1.22
+++ ena-dis-br.exp 22 Aug 2013 13:00:14 -0000
@@ -301,5 +301,114 @@ gdb_test_multiple "continue 2" "$test" {
}
}
+# Verify that GDB correctly handles the "enable/disable" command
+# with arguments, that include multiple locations.
+#
+if ![runto_main] then { fail "enable/disable break tests suppressed" }
+
+set b1 0
+set b2 0
+set b3 0
+set b4 0
+set b1 [break_at main ""]
+set b2 [break_at main ""]
+set b3 [break_at main ""]
+set b4 [break_at main ""]
+
+# Perform tests for disable/enable commands on multiple
+# locations and breakpoints.
+#
+# WHAT - the command to test (disable/enable).
+#
+proc test_ena_dis_br { what } {
+ global b1
+ global b2
+ global b3
+ global b4
+ global gdb_prompt
+
+ # OPPOS - the command opposite to WHAT.
+ # WHAT_RES - whether breakpoints are expected to end
+ # up enabled or disabled.
+ # OPPOS_RES- same as WHAT_RES but opposite.
+ # P1/P2 - proc to call (pass/fail). Must be
+ # opposites.
+ # Set variable values for disable command.
+ set oppos "enable"
+ set oppos_res "y"
+ set what_res "n"
+ set p1 "pass"
+ set p2 "fail"
+
+ if { "$what" == "enable" } {
+ # Set varibale values for enable command.
+ set oppos "disable"
+ set oppos_res "n"
+ set what_res "y"
+ set p1 "fail"
+ set p2 "pass"
+
+ }
+
+ # Now enable(disable) $b.1 $b2.1.
+ gdb_test_no_output "$what $b1.1 $b2.1" "$what \$b1.1 \$b2.1"
+ set test1 "${what}d \$b1.1 and \$b2.1"
+
+ # Now $b1.1 and $b2.1 should be enabled(disabled)
+ gdb_test_multiple "info break" "$test1" {
+ -re "(${b1}.1)(\[^\n\r\]*)( n.*)(${b2}.1)(\[^\n\r\]*)( n.*)$gdb_prompt $" {
+ $p1 "$test1"
+ }
+ -re ".*$gdb_prompt $" {
+ $p2 "$test1"
+ }
+ }
+
+ # Now enable(disable) $b1 fooo.1, it should give error on fooo.
+ gdb_test "$what $b1 fooo.1" \
+ "Bad breakpoint number 'fooo'" \
+ "$what \$b1 fooo.1"
+
+ # $b1 should be enabled(disabled).
+ gdb_test "info break" \
+ "(${b1})(\[^\n\r]*)( $what_res.*)" \
+ "${what}d \$b1"
+
+ gdb_test_no_output "$oppos $b3" "$oppos \$b3"
+ gdb_test_no_output "$what $b4 $b3.1" "$what \$b4 \$b3.1"
+ set test1 "${what}d \$b4 and \$b3.1,remain ${oppos}d \$b3"
+
+ # Now $b4 $b3.1 should be enabled(disabled) and
+ # $b3 should remain disabled(enabled)
+ gdb_test_multiple "info break" "$test1" {
+ -re "(${b3})(\[^\n\r]*)( $oppos_res.*)(${b3}.1)(\[^\n\r\]*)( n.*)(${b4})(\[^\n\r\]*)( $what_res.*)$gdb_prompt $" {
+ $p1 "$test1"
+ }
+ -re "(${b3})(\[^\n\r]*)( $oppos_res.*)(${b4})(\[^\n\r\]*)( $what_res.*)$gdb_prompt $" {
+ $p2 "$test1"
+ }
+ }
+
+ # Now enable(disable) $b4.1 fooobaar and
+ # it should give warning on fooobaar.
+ gdb_test "$what $b4.1 fooobaar" \
+ "warning: bad breakpoint number at or near 'fooobaar'" \
+ "$what \$b4.1 fooobar"
+ set test1 "${what}d \$b4.1"
+
+ # $b4.1 should be enabled(disabled)
+ gdb_test_multiple "info break" "$test1" {
+ -re "(${b4}.1)(\[^\n\r\]*)( n.*)$gdb_prompt $" {
+ $p1 "$test1"
+ }
+ -re ".*$gdb_prompt $" {
+ $p2 "$test1"
+ }
+ }
+}
+
+test_ena_dis_br "disable"
+test_ena_dis_br "enable"
+
gdb_exit
return 0
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] fix PR-15501
2013-08-22 13:13 ` Waqas, Muhammad
@ 2013-08-22 13:41 ` Pedro Alves
2013-08-23 7:27 ` Muhammad Waqas
0 siblings, 1 reply; 11+ messages in thread
From: Pedro Alves @ 2013-08-22 13:41 UTC (permalink / raw)
To: Waqas, Muhammad; +Cc: gdb-patches
On 08/22/2013 02:13 PM, Waqas, Muhammad wrote:
> +# Perform tests for disable/enable commands on multiple
> +# locations and breakpoints.
> +#
> +# WHAT - the command to test (disable/enable).
That extra whitespace after WHAT was there before to align
with the other arguments. There are no other arguments now,
so write:
# WHAT - the command to test (disable/enable).
> + if { "$what" == "enable" } {
> + # Set variable values for enable command.
> + set oppos "disable"
> + set oppos_res "n"
> + set what_res "y"
> + set p1 "fail"
> + set p2 "pass"
> +
Spurious extra line.
> + }
> +
> + # Now enable(disable) $b.1 $b2.1.
> + gdb_test_no_output "$what $b1.1 $b2.1" "$what \$b1.1 \$b2.1"
> + set test1 "${what}d \$b1.1 and \$b2.1"
> +
> + # Now $b1.1 and $b2.1 should be enabled(disabled)
Missing period. (Most other sentences had it.)
> +
> + # Now $b4 $b3.1 should be enabled(disabled) and
> + # $b3 should remain disabled(enabled)
Ditto.
OK with those changes. Thanks!
As the final patch will have changes, please commit, and
still repost it.
--
Pedro Alves
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] fix PR-15501
2013-08-22 13:41 ` Pedro Alves
@ 2013-08-23 7:27 ` Muhammad Waqas
0 siblings, 0 replies; 11+ messages in thread
From: Muhammad Waqas @ 2013-08-23 7:27 UTC (permalink / raw)
To: Pedro Alves; +Cc: Waqas, Muhammad, gdb-patches
Thanks.
Below is the committed patch.
gdb/ChangeLog
2013-08-12 Muhammad Waqas <mwaqas@codesourcery.com>
PR gdb/15501
* breakpoint.c (enable_command, disable_command): Iterate over
all specified breakpoint locations.
Index: breakpoint.c
===================================================================
RCS file: /cvs/src/src/gdb/breakpoint.c,v
retrieving revision 1.773
diff -u -p -r1.773 breakpoint.c
--- breakpoint.c 24 Jul 2013 19:50:32 -0000 1.773
+++ breakpoint.c 22 Aug 2013 06:37:27 -0000
@@ -14553,25 +14553,35 @@ disable_command (char *args, int from_tt
if (user_breakpoint_p (bpt))
disable_breakpoint (bpt);
}
- else if (strchr (args, '.'))
+ else
{
- struct bp_location *loc = find_location_by_number (args);
- if (loc)
+ char *num = extract_arg (&args);
+
+ while (num)
{
- if (loc->enabled)
+ if (strchr (num, '.'))
{
- loc->enabled = 0;
- mark_breakpoint_location_modified (loc);
+ struct bp_location *loc = find_location_by_number (num);
+
+ if (loc)
+ {
+ if (loc->enabled)
+ {
+ loc->enabled = 0;
+ mark_breakpoint_location_modified (loc);
+ }
+ if (target_supports_enable_disable_tracepoint ()
+ && current_trace_status ()->running && loc->owner
+ && is_tracepoint (loc->owner))
+ target_disable_tracepoint (loc);
+ }
+ update_global_location_list (0);
}
- if (target_supports_enable_disable_tracepoint ()
- && current_trace_status ()->running && loc->owner
- && is_tracepoint (loc->owner))
- target_disable_tracepoint (loc);
+ else
+ map_breakpoint_numbers (num, do_map_disable_breakpoint, NULL);
+ num = extract_arg (&args);
}
- update_global_location_list (0);
}
- else
- map_breakpoint_numbers (args, do_map_disable_breakpoint, NULL);
}
static void
@@ -14677,25 +14687,35 @@ enable_command (char *args, int from_tty
if (user_breakpoint_p (bpt))
enable_breakpoint (bpt);
}
- else if (strchr (args, '.'))
+ else
{
- struct bp_location *loc = find_location_by_number (args);
- if (loc)
+ char *num = extract_arg (&args);
+
+ while (num)
{
- if (!loc->enabled)
+ if (strchr (num, '.'))
{
- loc->enabled = 1;
- mark_breakpoint_location_modified (loc);
+ struct bp_location *loc = find_location_by_number (num);
+
+ if (loc)
+ {
+ if (!loc->enabled)
+ {
+ loc->enabled = 1;
+ mark_breakpoint_location_modified (loc);
+ }
+ if (target_supports_enable_disable_tracepoint ()
+ && current_trace_status ()->running && loc->owner
+ && is_tracepoint (loc->owner))
+ target_enable_tracepoint (loc);
+ }
+ update_global_location_list (1);
}
- if (target_supports_enable_disable_tracepoint ()
- && current_trace_status ()->running && loc->owner
- && is_tracepoint (loc->owner))
- target_enable_tracepoint (loc);
+ else
+ map_breakpoint_numbers (num, do_map_enable_breakpoint, NULL);
+ num = extract_arg (&args);
}
- update_global_location_list (1);
}
- else
- map_breakpoint_numbers (args, do_map_enable_breakpoint, NULL);
}
gdb/testsuite/ChangeLog
2013-07-12 Muhammad Waqas <mwaqas@codesourccery.com>
PR gdb/15501
* gdb.base/ena-dis-br.exp: Add test to verify
enable/disable commands work correctly with
multiple arguments that include multiple locations.
Index: ena-dis-br.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.base/ena-dis-br.exp,v
retrieving revision 1.22
diff -u -p -r1.22 ena-dis-br.exp
--- ena-dis-br.exp 27 Jun 2013 18:50:30 -0000 1.22
+++ ena-dis-br.exp 23 Aug 2013 05:37:54 -0000
@@ -301,5 +301,113 @@ gdb_test_multiple "continue 2" "$test" {
}
}
+# Verify that GDB correctly handles the "enable/disable" command
+# with arguments, that include multiple locations.
+#
+if ![runto_main] then { fail "enable/disable break tests suppressed" }
+
+set b1 0
+set b2 0
+set b3 0
+set b4 0
+set b1 [break_at main ""]
+set b2 [break_at main ""]
+set b3 [break_at main ""]
+set b4 [break_at main ""]
+
+# Perform tests for disable/enable commands on multiple
+# locations and breakpoints.
+#
+# WHAT - the command to test (disable/enable).
+#
+proc test_ena_dis_br { what } {
+ global b1
+ global b2
+ global b3
+ global b4
+ global gdb_prompt
+
+ # OPPOS - the command opposite to WHAT.
+ # WHAT_RES - whether breakpoints are expected to end
+ # up enabled or disabled.
+ # OPPOS_RES- same as WHAT_RES but opposite.
+ # P1/P2 - proc to call (pass/fail). Must be
+ # opposites.
+ # Set variable values for disable command.
+ set oppos "enable"
+ set oppos_res "y"
+ set what_res "n"
+ set p1 "pass"
+ set p2 "fail"
+
+ if { "$what" == "enable" } {
+ # Set variable values for enable command.
+ set oppos "disable"
+ set oppos_res "n"
+ set what_res "y"
+ set p1 "fail"
+ set p2 "pass"
+ }
+
+ # Now enable(disable) $b.1 $b2.1.
+ gdb_test_no_output "$what $b1.1 $b2.1" "$what \$b1.1 \$b2.1"
+ set test1 "${what}d \$b1.1 and \$b2.1"
+
+ # Now $b1.1 and $b2.1 should be enabled(disabled).
+ gdb_test_multiple "info break" "$test1" {
+ -re "(${b1}.1)(\[^\n\r\]*)( n.*)(${b2}.1)(\[^\n\r\]*)(
n.*)$gdb_prompt $" {
+ $p1 "$test1"
+ }
+ -re ".*$gdb_prompt $" {
+ $p2 "$test1"
+ }
+ }
+
+ # Now enable(disable) $b1 fooo.1, it should give error on fooo.
+ gdb_test "$what $b1 fooo.1" \
+ "Bad breakpoint number 'fooo'" \
+ "$what \$b1 fooo.1"
+
+ # $b1 should be enabled(disabled).
+ gdb_test "info break" \
+ "(${b1})(\[^\n\r]*)( $what_res.*)" \
+ "${what}d \$b1"
+
+ gdb_test_no_output "$oppos $b3" "$oppos \$b3"
+ gdb_test_no_output "$what $b4 $b3.1" "$what \$b4 \$b3.1"
+ set test1 "${what}d \$b4 and \$b3.1,remain ${oppos}d \$b3"
+
+ # Now $b4 $b3.1 should be enabled(disabled) and
+ # $b3 should remain disabled(enabled).
+ gdb_test_multiple "info break" "$test1" {
+ -re "(${b3})(\[^\n\r]*)( $oppos_res.*)(${b3}.1)(\[^\n\r\]*)(
n.*)(${b4})(\[^\n\r\]*)( $what_res.*)$gdb_prompt $" {
+ $p1 "$test1"
+ }
+ -re "(${b3})(\[^\n\r]*)( $oppos_res.*)(${b4})(\[^\n\r\]*)(
$what_res.*)$gdb_prompt $" {
+ $p2 "$test1"
+ }
+ }
+
+ # Now enable(disable) $b4.1 fooobaar and
+ # it should give warning on fooobaar.
+ gdb_test "$what $b4.1 fooobaar" \
+ "warning: bad breakpoint number at or near 'fooobaar'" \
+ "$what \$b4.1 fooobar"
+ set test1 "${what}d \$b4.1"
+
+ # $b4.1 should be enabled(disabled).
+ gdb_test_multiple "info break" "$test1" {
+ -re "(${b4}.1)(\[^\n\r\]*)( n.*)$gdb_prompt $" {
+ $p1 "$test1"
+ }
+ -re ".*$gdb_prompt $" {
+ $p2 "$test1"
+ }
+ }
+}
+
+test_ena_dis_br "disable"
+test_ena_dis_br "enable"
+
gdb_exit
return 0
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2013-08-23 7:27 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-08-13 10:03 [PATCH] fix PR-15501 Muhammad Waqas
2013-08-13 17:37 ` Pedro Alves
2013-08-15 10:32 ` Muhammad Waqas
2013-08-19 15:28 ` Pedro Alves
2013-08-20 6:45 ` Waqas, Muhammad
2013-08-21 22:17 ` Pedro Alves
2013-08-22 9:36 ` Waqas, Muhammad
2013-08-22 12:02 ` Pedro Alves
2013-08-22 13:13 ` Waqas, Muhammad
2013-08-22 13:41 ` Pedro Alves
2013-08-23 7:27 ` Muhammad Waqas
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).