This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH] fix PR-15501
- From: Pedro Alves <palves at redhat dot com>
- To: Muhammad Waqas <mwaqas at codesourcery dot com>
- Cc: gdb-patches at sourceware dot org
- Date: Tue, 13 Aug 2013 18:37:47 +0100
- Subject: Re: [PATCH] fix PR-15501
- References: <520A0453 dot 4070309 at codesourcery dot com>
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