Re: RFA: patch to fix multi-breakpoint enable/disable handling of inline functions

Douglas Evans
2007-10-22T21:23:00Z

[-- Attachment #1: Type: text/plain, Size: 2657 bytes --]

Ping.

I've split the patch into two parts, separating out the tweaks to
mb-templates.exp,mb-ctor.exp (I'll send this in a separate message).
I also fixed a few typos. The revised patch is attached.

Ok to check in?

On 10/15/07, Doug Evans <dje@google.com> wrote:
> Consider:
>
> dje@ruffy:~/gnu/sourceware/head/obj/gdb/testsuite/gdb.cp$ ~/gnu/sourceware/head/rel/bin/gdb mb-inline
> GNU gdb 6.7.50-20071009-cvs
> Copyright (C) 2007 Free Software Foundation, Inc.
> License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
> This is free software: you are free to change and redistribute it.
> There is NO WARRANTY, to the extent permitted by law. Type "show copying"
> and "show warranty" for details.
> This GDB was configured as "i386-linux"...
> Using host libthread_db library "/lib/tls/i686/cmov/libthread_db.so.1".
> (gdb) b mb-inline.h:6
> Breakpoint 1 at 0x80483e9: file ../../../src/gdb/testsuite/gdb.cp/mb-inline.h, line 6. (2 locations)
> (gdb) i b
> Num Type Disp Enb Address What
> 1 breakpoint keep y <MULTIPLE>
> 1.1 y 0x080483e9 in foo
> at ../../../src/gdb/testsuite/gdb.cp/mb-inline.h:6
> 1.2 y 0x0804843d in foo
> at ../../../src/gdb/testsuite/gdb.cp/mb-inline.h:6
> (gdb) dis 1.2
> (gdb) i b
> Num Type Disp Enb Address What
> 1 breakpoint keep y <MULTIPLE>
> 1.1 y 0x080483e9 in foo
> at ../../../src/gdb/testsuite/gdb.cp/mb-inline.h:6
> 1.2 n 0x0804843d in foo
> at ../../../src/gdb/testsuite/gdb.cp/mb-inline.h:6
> (gdb) r
> Starting program: /home/dje/gnu/sourceware/head/obj/gdb/testsuite/gdb.cp/mb-inline
>
> Breakpoint 1, foo (i=1) at ../../../src/gdb/testsuite/gdb.cp/mb-inline.h:6
> 6 return i; // set breakpoint here
> (gdb) i b
> Num Type Disp Enb Address What
> 1 breakpoint keep y <MULTIPLE>
> breakpoint already hit 1 time
> 1.1 n 0x080483e9 in foo
> at ../../../src/gdb/testsuite/gdb.cp/mb-inline.h:6
> 1.2 y 0x0804843d in foo
> at ../../../src/gdb/testsuite/gdb.cp/mb-inline.h:6
> (gdb)
>
> Note that 1.1 is now disabled and 1.2 is enabled.
>
> Here's a patch.
> There is a heuristic involved in knowing when to not compare function names.
> I've tried to come up with something reasonable.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: gdb-071022-mb-inline-2a.patch --]
[-- Type: text/x-patch; name=gdb-071022-mb-inline-2a.patch, Size: 10156 bytes --]

Consider:

dje@ruffy:~/gnu/sourceware/head/obj/gdb/testsuite/gdb.cp$ ~/gnu/sourceware/head/rel/bin/gdb mb-inline
GNU gdb 6.7.50-20071009-cvs
Copyright (C) 2007 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law. Type "show copying"
and "show warranty" for details.
This GDB was configured as "i386-linux"...
Using host libthread_db library "/lib/tls/i686/cmov/libthread_db.so.1".
(gdb) b mb-inline.h:6
Breakpoint 1 at 0x80483e9: file ../../../src/gdb/testsuite/gdb.cp/mb-inline.h, line 6. (2 locations)
(gdb) i b
Num Type Disp Enb Address What
1 breakpoint keep y <MULTIPLE>
1.1 y 0x080483e9 in foo
at ../../../src/gdb/testsuite/gdb.cp/mb-inline.h:6
1.2 y 0x0804843d in foo
at ../../../src/gdb/testsuite/gdb.cp/mb-inline.h:6
(gdb) dis 1.2
(gdb) i b
Num Type Disp Enb Address What
1 breakpoint keep y <MULTIPLE>
1.1 y 0x080483e9 in foo
at ../../../src/gdb/testsuite/gdb.cp/mb-inline.h:6
1.2 n 0x0804843d in foo
at ../../../src/gdb/testsuite/gdb.cp/mb-inline.h:6
(gdb) r
Starting program: /home/dje/gnu/sourceware/head/obj/gdb/testsuite/gdb.cp/mb-inline

Breakpoint 1, foo (i=1) at ../../../src/gdb/testsuite/gdb.cp/mb-inline.h:6
6 return i; // set breakpoint here
(gdb) i b
Num Type Disp Enb Address What
1 breakpoint keep y <MULTIPLE>
breakpoint already hit 1 time
1.1 n 0x080483e9 in foo
at ../../../src/gdb/testsuite/gdb.cp/mb-inline.h:6
1.2 y 0x0804843d in foo
at ../../../src/gdb/testsuite/gdb.cp/mb-inline.h:6
(gdb)

Note that 1.1 is now disabled and 1.2 is enabled.

Here's a patch.
There is a heuristic involved in knowing when to not compare function names.
I've tried to come up with something reasonable.

2007-10-15 Doug Evans <dje@google.com>

* breakpoint.c (ambiguous_names_p): New fn.
(update_breakpoint_locations): When restoring bp enable status, don't
compare function names if all functions have same name.

* gdb.cp/mb-inline.exp: New.
* gdb.cp/mb-inline.h: New.
* gdb.cp/mb-inline1.cc: New.
* gdb.cp/mb-inline2.cc: New.

Index: breakpoint.c
===================================================================
RCS file: /cvs/src/src/gdb/breakpoint.c,v
retrieving revision 1.274
diff -u -u -p -r1.274 breakpoint.c
--- ./breakpoint.c 12 Oct 2007 16:11:11 -0000 1.274
+++ ./breakpoint.c 15 Oct 2007 23:47:54 -0000
@@ -7496,6 +7496,37 @@ all_locations_are_pending (struct bp_loc
return 1;
}

+/* Return non-zero if multiple fns in list LOC have the same name.
+ Null names are ignored.
+ This returns zero if there's <= one named function, there's no ambiguity.
+ ??? This tests for ambiguity with the first named function, it doesn't
+ handle the case of multiple ambiguities. This can be fixed at the cost of
+ some complexity in the caller, but it's unknown if this is a practical
+ issue. */
+
+static int
+ambiguous_names_p (struct bp_location *loc)
+{
+ struct bp_location *l;
+ const char *name = NULL;
+
+ for (l = loc; l != NULL; l = l->next)
+ {
+ /* Allow for some names to be NULL, ignore them. */
+ if (l->function_name == NULL)
+ continue;
+ if (name == NULL)
+ {
+ name = l->function_name;
+ continue;
+ }
+ if (strcmp (name, l->function_name) == 0)
+ return 1;
+ }
+
+ return 0;
+}
+
static void
update_breakpoint_locations (struct breakpoint *b,
struct symtabs_and_lines sals)
@@ -7558,18 +7589,37 @@ update_breakpoint_locations (struct brea
/* If possible, carry over 'disable' status from existing breakpoints. */
{
struct bp_location *e = existing_locations;
+ /* If there are multiple breakpoints with the same function name,
+ e.g. for inline functions, comparing function names won't work.
+ Instead compare pc addresses; this is just a heuristic as things
+ may have moved, but in practice it gives the correct answer
+ often enough until a better solution is found. */
+ int have_ambiguous_names = ambiguous_names_p (b->loc);
+
for (; e; e = e->next)
{
if (!e->enabled && e->function_name)
{
struct bp_location *l = b->loc;
- for (; l; l = l->next)
- if (l->function_name
- && strcmp (e->function_name, l->function_name) == 0)
- {
- l->enabled = 0;
- break;
- }
+ if (have_ambiguous_names)
+ {
+ for (; l; l = l->next)
+ if (e->address == l->address)
+ {
+ l->enabled = 0;
+ break;
+ }
+ }
+ else
+ {
+ for (; l; l = l->next)
+ if (l->function_name
+ && strcmp (e->function_name, l->function_name) == 0)
+ {
+ l->enabled = 0;
+ break;
+ }
+ }
}
}
}
Index: testsuite/gdb.cp/mb-inline.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.cp/mb-inline.exp,v
diff -u -p -N mb-inline.exp
--- .//dev/null 1969-12-31 16:00:00.000000000 -0800
+++ ./testsuite/gdb.cp/mb-inline.exp 2007-10-15 16:44:36.858766000 -0700
@@ -0,0 +1,108 @@
+# Copyright 2007 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/>.
+
+# This file is part of the gdb testsuite.
+
+# This test verifies that setting breakpoint on line in inline
+# function will fire in all instantiations of that function.
+
+if $tracelevel then {
+ strace $tracelevel
+}
+
+if { [skip_cplus_tests] } { continue }
+
+set prms_id 0
+set bug_id 0
+
+set testfile "mb-inline"
+set hdrfile "${testfile}.h"
+set srcfile1 "${testfile}1.cc"
+set objfile1 "${testfile}1.o"
+set srcfile2 "${testfile}2.cc"
+set objfile2 "${testfile}2.o"
+set binfile "${objdir}/${subdir}/${testfile}"
+
+if { [gdb_compile "$srcdir/$subdir/$srcfile1" "$objdir/$subdir/$objfile1" object {debug c++}] != "" } {
+ untested rtti.exp
+ return -1
+}
+
+if { [gdb_compile "$srcdir/$subdir/$srcfile2" "$objdir/$subdir/$objfile2" object {debug c++}] != "" } {
+ untested rtti.exp
+ return -1
+}
+
+if { [gdb_compile "$objdir/$subdir/$objfile1 $objdir/$subdir/$objfile2" "${binfile}" executable {debug c++}] != "" } {
+ untested rtti.exp
+ return -1
+}
+
+if [get_compiler_info ${binfile} "c++"] {
+ return -1
+}
+
+gdb_exit
+gdb_start
+gdb_reinitialize_dir $srcdir/$subdir
+gdb_load ${binfile}
+
+set bp_location [gdb_get_line_number "set breakpoint here" $hdrfile]
+
+# Set a breakpoint with multiple locations.
+
+gdb_test "break $hdrfile:$bp_location" \
+ "Breakpoint.*at.* file .*$hdrfile, line.*\\(2 locations\\).*" \
+ "set breakpoint"
+
+gdb_run_cmd
+gdb_expect {
+ -re "Breakpoint \[0-9\]+,.*foo \\(i=0\\).*$gdb_prompt $" {
+ pass "run to breakpoint"
+ }
+ -re "$gdb_prompt $" {
+ fail "run to breakpoint"
+ }
+ timeout {
+ fail "run to breakpoint (timeout)"
+ }
+}
+
+gdb_test "continue" \
+ ".*Breakpoint.*foo \\(i=1\\).*" \
+ "run to breakpoint 2"
+
+# Try disabling a single location. We also test
+# that at least in simple cases, the enable/disable
+# state of locations survive "run".
+# Early bug would disable 1.1 and enable 1.2 when program is run.
+gdb_test "disable 1.2" "" "disabling location: disable"
+
+gdb_run_cmd
+gdb_expect {
+ -re "Breakpoint \[0-9\]+,.*foo \\(i=0\\).*$gdb_prompt $" {
+ pass "disabling location: run to breakpoint"
+ }
+ -re "$gdb_prompt $" {
+ fail "disabling location: run to breakpoint"
+ }
+ timeout {
+ fail "disabling location: run to breakpoint (timeout)"
+ }
+}
+
+gdb_test "continue" \
+ ".*Program exited normally.*" \
+ "continue with disabled breakpoint 1.2"
Index: testsuite/gdb.cp/mb-inline.h
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.cp/mb-inline.h,v
diff -u -p -N mb-inline.h
--- .//dev/null 1969-12-31 16:00:00.000000000 -0800
+++ ./testsuite/gdb.cp/mb-inline.h 2007-10-15 16:40:42.908114000 -0700
@@ -0,0 +1,10 @@
+// Test gdb support for setting file:line breakpoints on inline fns.
+
+static inline int
+foo (int i)
+{
+ return i; // set breakpoint here
+}
+
+extern int afn ();
+extern int bfn ();
Index: testsuite/gdb.cp/mb-inline1.cc
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.cp/mb-inline1.cc,v
diff -u -p -N mb-inline1.cc
--- .//dev/null 1969-12-31 16:00:00.000000000 -0800
+++ ./testsuite/gdb.cp/mb-inline1.cc 2007-10-15 16:42:02.644024000 -0700
@@ -0,0 +1,17 @@
+// Test gdb support for setting file:line breakpoints on inline fns.
+
+#include "mb-inline.h"
+
+int
+afn ()
+{
+ return foo (0);
+}
+
+int
+main ()
+{
+ int a = afn ();
+ int b = bfn ();
+ return a * b;
+}
Index: testsuite/gdb.cp/mb-inline2.cc
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.cp/mb-inline2.cc,v
diff -u -p -N mb-inline2.cc
--- .//dev/null 1969-12-31 16:00:00.000000000 -0800
+++ ./testsuite/gdb.cp/mb-inline2.cc 2007-10-15 16:42:06.315615000 -0700
@@ -0,0 +1,7 @@
+#include "mb-inline.h"
+
+int
+bfn ()
+{
+ return foo (1);
+}