[PATCH] fix PR-15501
Muhammad Waqas
mwaqas@codesourcery.com
Thu Aug 15 10:32:00 GMT 2013
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
More information about the Gdb-patches
mailing list