This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH] Failure to stop at duplicate breakpoints
Pedro,
Thanks for taking the time to review my patch. I've addressed the issues you raised, and
On 25/09/2012 5:15 PM, Pedro Alves wrote:
>
>> --- /dev/null
>> +++ b/gdb/testsuite/gdb.base/duplicate-bp.c
>
> Please add the standard copyright header boilerplate. Even if the
> contents of the file don't have that much copyrightable content
> now, it's better to always add a header as a rule, than to
> police later changes to files and worry about having to add
> a header then.
Done.
>> +
>> +if { [prepare_for_testing duplicate-bp.exp "duplicate-bp" {duplicate-bp.c} {debug nowarnings}] } {
>
> Could you make this use standard_testfile? Like:
>
> standard_testfile
>
> if { [prepare_for_testing ${testfile}.exp ${testfile} $srcfile {debug nowarnings}] } {
Done, well it turns out "prepare_for_testing ${testfile}.exp ${testfile}" is enough.
>
>
>> + return -1
>> +}
>> +set srcfile "duplicate-bp.c"
>
> This won't be needed then.
>
> While at it, is "nowarnings" really necessary?
Removed.
>
>
>> +
>> +# Setup for the test, create COUNT breakpoints at the function BREAKPT.
>> +proc test_setup { count } {
>> + upvar srcfile srcfile
>
> It is more idiomatic in our testsuite to use "global srcfile".
Fixed.
>
> There are many duplicate test output messages in the .sum file:
>
They should all be unique now.
>
> Also, please get rid of the trailing whitespace the patch is adding:
>
Done.
Again, thanks for taking the time to review this patch.
Cheers,
Andrew
gdb/ChangeLog
2012-09-25 Andrew Burgess <aburgess@broadcom.com>
* breakpoint.c (update_global_location_list): Ignore previous
duplicate status of a breakpoint when starting a new scan for
duplicate breakpoints.
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index b841bcd..f771d06 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -12463,7 +12463,7 @@ update_global_location_list (int should_insert)
struct bp_location **loc_first_p;
b = loc->owner;
- if (!should_be_inserted (loc)
+ if (!unduplicated_should_be_inserted (loc)
|| !breakpoint_address_is_meaningful (b)
/* Don't detect duplicate for tracepoint locations because they are
never duplicated. See the comments in field `duplicate' of
gdb/testsuite/ChangeLog
2012-09-25 Andrew Burgess <aburgess@broadcom.com>
* gdb.base/duplicate-bp.c: New file.
* gdb.base/duplicate-bp.exp: New file.
diff --git a/gdb/testsuite/gdb.base/duplicate-bp.c b/gdb/testsuite/gdb.base/duplicate-bp.c
new file mode 100644
index 0000000..662a7c4
--- /dev/null
+++ b/gdb/testsuite/gdb.base/duplicate-bp.c
@@ -0,0 +1,40 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+ Copyright 2012 Free Software Foundation, Inc.
+
+ This program is free software; you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation; either version 3 of the License, or
+ (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program. If not, see <http://www.gnu.org/licenses/>. */
+
+void
+spacer ()
+{
+ /* Nothing. */
+}
+
+void
+breakpt ()
+{
+ /* Nothing. */
+}
+
+int
+main ()
+{
+ spacer ();
+
+ breakpt ();
+
+ spacer ();
+
+ return 0;
+}
diff --git a/gdb/testsuite/gdb.base/duplicate-bp.exp b/gdb/testsuite/gdb.base/duplicate-bp.exp
new file mode 100644
index 0000000..01279fb
--- /dev/null
+++ b/gdb/testsuite/gdb.base/duplicate-bp.exp
@@ -0,0 +1,156 @@
+# Copyright 2012 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program. If not, see <http://www.gnu.org/licenses/>.
+
+standard_testfile
+
+if { [prepare_for_testing ${testfile}.exp ${testfile}] } {
+ return -1
+}
+
+# Setup for the test, create COUNT breakpoints at the function BREAKPT.
+proc test_setup { count } {
+ global srcfile
+
+ clean_restart duplicate-bp
+
+ if ![runto_main] then {
+ fail "can't run to main"
+ return 0
+ }
+
+ for {set i 1} {$i <= $count} {incr i} {
+ gdb_breakpoint "breakpt"
+ gdb_test_no_output "set \$bp_num_${i} = \$bpnum"
+ }
+
+ gdb_test "step" \
+ "spacer \\(\\) at .*$srcfile:\[0-9\]+.*" \
+ "step to place breakpoints"
+
+ return 1
+}
+
+
+# Test 1: Create two breakpoints at BREAKPT. Delete #1 and expect to stop
+# at #2.
+with_test_prefix "del_1_stop_2" {
+ test_setup 2
+
+ gdb_test_no_output {delete $bp_num_1}
+
+ gdb_test continue "Continuing\\..*Breakpoint \[0-9\]+, breakpt \\(\\) at .*$srcfile:\[0-9\]+.*" \
+ "delete #1, stop at #2"
+}
+
+# Test 2: Create two breakpoints at BREAKPT. Delete #2 and expect to stop
+# at #1.
+with_test_prefix "del_2_stop_1" {
+ test_setup 2
+
+ gdb_test_no_output {delete $bp_num_2}
+
+ gdb_test continue "Continuing\\..*Breakpoint \[0-9\]+, breakpt \\(\\) at .*$srcfile:\[0-9\]+.*" \
+ "delete #2, stop at #1"
+}
+
+# Test 3: Create three breakpoints at BREAKPT. Disable #1, delete #2,
+# expect to stop at #3.
+with_test_prefix "dis_1_del_2_stop_3" {
+ test_setup 3
+
+ gdb_test_no_output {disable $bp_num_1}
+
+ gdb_test "step" ".*"
+
+ gdb_test_no_output {delete $bp_num_2}
+
+ gdb_test continue "Continuing\\..*Breakpoint \[0-9\]+, breakpt \\(\\) at .*$srcfile:\[0-9\]+.*" \
+ "disable #1, delete #2, stop at #3"
+}
+
+# Test 4: Create three breakpoints at BREAKPT. Disable #2, delete #1,
+# expect to stop at #3.
+with_test_prefix "dis_2_del_1_stop_3" {
+ test_setup 3
+
+ gdb_test_no_output {disable $bp_num_2}
+
+ gdb_test "step" ".*"
+
+ gdb_test_no_output {delete $bp_num_1}
+
+ gdb_test continue "Continuing\\..*Breakpoint \[0-9\]+, breakpt \\(\\) at .*$srcfile:\[0-9\]+.*" \
+ "disable #2, delete #1, stop at #3"
+}
+
+# Test 5: Create three breakpoints at BREAKPT. Disable #1, delete #3,
+# expect to stop at #2.
+with_test_prefix "dis_1_del_3_stop_1" {
+ test_setup 3
+
+ gdb_test_no_output {disable $bp_num_1}
+
+ gdb_test "step" ".*"
+
+ gdb_test_no_output {delete $bp_num_3}
+
+ gdb_test continue "Continuing\\..*Breakpoint \[0-9\]+, breakpt \\(\\) at .*$srcfile:\[0-9\]+.*" \
+ "disable #1, delete #3, stop at #2"
+}
+
+# Test 6: Create three breakpoints at BREAKPT. Disable #3, delete #1,
+# expect to stop at #2
+with_test_prefix "dis_3_del_1_stop_2" {
+ test_setup 3
+
+ gdb_test_no_output {disable $bp_num_3}
+
+ gdb_test "step" ".*"
+
+ gdb_test_no_output {delete $bp_num_1}
+
+ gdb_test continue "Continuing\\..*Breakpoint \[0-9\]+, breakpt \\(\\) at .*$srcfile:\[0-9\]+.*" \
+ "disable #3, delete #1, stop at #2"
+}
+
+# Test 7: Create three breakpoints at BREAKPT. Disable #2, delete #3,
+# expect to stop at #1.
+with_test_prefix "dis_2_del_3_stop_1" {
+ test_setup 3
+
+ gdb_test_no_output {disable $bp_num_2}
+
+ gdb_test "step" ".*"
+
+ gdb_test_no_output {delete $bp_num_3}
+
+ gdb_test continue "Continuing\\..*Breakpoint \[0-9\]+, breakpt \\(\\) at .*$srcfile:\[0-9\]+.*" \
+ "disable #2, delete #3, stop at #1"
+}
+
+# Test 8: Create three breakpoints at BREAKPT. Disable #3, delete #2,
+# expect to stop at #1.
+with_test_prefix "dis_3_del_2_stop_1" {
+ test_setup 3
+
+ gdb_test_no_output {disable $bp_num_3}
+
+ gdb_test "step" ".*"
+
+ gdb_test_no_output {delete $bp_num_2}
+
+ gdb_test continue "Continuing\\..*Breakpoint \[0-9\]+, breakpt \\(\\) at .*$srcfile:\[0-9\]+.*" \
+ "disable #3, delete #2, stop at #1"
+}