Re: [RFA]: Handle multi-line versions of multi-breakpoints

[Doug Evans <dje at google dot com>]

Ping.
[This doesn't seem to be a controversial patch, and there is
definitely a bug here that needs fixing.]

On Fri, Mar 13, 2009 at 9:47 AM, Doug Evans <dje@google.com> wrote:
> Ping.
>
> On Sun, Mar 1, 2009 at 11:53 PM, Doug Evans <dje@google.com> wrote:
>> Hi.
>>
>> "multi-breakpoints" (multiple breakpoints for the same source line)
>> don't work for constructors and inline functions where the source expression
>> spans several lines (at least for the new testcases included in this patch).
>>
>> This patch fixes it by, if there's no exact match,
>> rescanning the symtabs for all matches of the best match.
>> Included are updated testcases to exercise the bug.
>> The bug isn't exposed by templates, but I updated mb-templates.{cc,exp}
>> anyway (if only to confirm things work there too).
>>
>> Ok to check in?
>>
>> 2009-03-01 ?Doug Evans ?<dje@google.com>
>>
>> ? ? ? ?* symtab.c (expand_line_sal): If we don't find an exact match,
>> ? ? ? ?rescan all symtabs again to find all copies of the best match.
>>
>> ? ? ? ?* gdb.cp/mb-ctor.exp: Add multi-line source statement test.
>> ? ? ? ?* gdb.cp/mb-ctor.cc: Ditto.
>> ? ? ? ?* gdb.cp/mb-inline.exp: Add multi-line source statement test.
>> ? ? ? ?* gdb.cp/mb-inline.h (multi_line_foo): New function.
>> ? ? ? ?* gdb.cp/mb-inline1.cc: Call it.
>> ? ? ? ?* gdb.cp/mb-inline2.cc: Ditto.
>> ? ? ? ?* gdb.cp/mb-templates.exp: Add multi-line source statement test.
>> ? ? ? ?* gdb.cp/mb-templates.cc (multi_line_foo): New template.
>>
>> Index: symtab.c
>> ===================================================================
>> RCS file: /cvs/src/src/gdb/symtab.c,v
>> retrieving revision 1.204
>> diff -u -p -r1.204 symtab.c
>> --- symtab.c ? ?2 Mar 2009 06:33:24 -0000 ? ? ? 1.204
>> +++ symtab.c ? ?2 Mar 2009 06:39:17 -0000
>> @@ -4481,7 +4481,6 @@ expand_line_sal (struct symtab_and_line
>> ? else
>> ? ? {
>> ? ? ? struct linetable_entry *best_item = 0;
>> - ? ? ?struct symtab *best_symtab = 0;
>> ? ? ? int exact = 0;
>>
>> ? ? ? lineno = sal.line;
>> @@ -4533,13 +4532,41 @@ expand_line_sal (struct symtab_and_line
>> ? ? ? ? ? ? ? ? ? ? ? ? ? && (best_item == NULL || item->line < best_item->line))
>> ? ? ? ? ? ? ? ? ? ?{
>> ? ? ? ? ? ? ? ? ? ? ?best_item = item;
>> - ? ? ? ? ? ? ? ? ? ? best_symtab = symtab;
>> ? ? ? ? ? ? ? ? ? ?}
>> ? ? ? ? ? ? ? ?}
>> ? ? ? ? ? ?}
>> ? ? ? ?}
>> +
>> ? ? ? if (!exact && best_item)
>> - ? ? ? append_expanded_sal (&ret, best_symtab, lineno, best_item->pc);
>> + ? ? ? {
>> + ? ? ? ? /* We found a "good enough" match.
>> + ? ? ? ? ? ?Rescan to find all such matches.
>> + ? ? ? ? ? ?This is to handle constructors, templates, inline-functions
>> + ? ? ? ? ? ?that have multiple breakpoints for one line number. ?*/
>> +
>> + ? ? ? ? ALL_SYMTABS (objfile, symtab)
>> + ? ? ? ? ? {
>> + ? ? ? ? ? ? if (strcmp (sal.symtab->filename,
>> + ? ? ? ? ? ? ? ? ? ? ? ? symtab->filename) == 0)
>> + ? ? ? ? ? ? ? {
>> + ? ? ? ? ? ? ? ? struct linetable *l;
>> + ? ? ? ? ? ? ? ? int len;
>> + ? ? ? ? ? ? ? ? l = LINETABLE (symtab);
>> + ? ? ? ? ? ? ? ? if (!l)
>> + ? ? ? ? ? ? ? ? ? continue;
>> + ? ? ? ? ? ? ? ? len = l->nitems;
>> +
>> + ? ? ? ? ? ? ? ? for (j = 0; j < len; j++)
>> + ? ? ? ? ? ? ? ? ? {
>> + ? ? ? ? ? ? ? ? ? ? struct linetable_entry *item = &(l->item[j]);
>> +
>> + ? ? ? ? ? ? ? ? ? ? if (item->line == best_item->line)
>> + ? ? ? ? ? ? ? ? ? ? ? append_expanded_sal (&ret, symtab, item->line,
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?item->pc);
>> + ? ? ? ? ? ? ? ? ? }
>> + ? ? ? ? ? ? ? }
>> + ? ? ? ? ? }
>> + ? ? ? }
>> ? ? }
>>
>> ? /* For optimized code, compiler can scatter one source line accross
>> Index: testsuite/gdb.cp/mb-ctor.cc
>> ===================================================================
>> RCS file: /cvs/src/src/gdb/testsuite/gdb.cp/mb-ctor.cc,v
>> retrieving revision 1.1
>> diff -u -p -r1.1 mb-ctor.cc
>> --- testsuite/gdb.cp/mb-ctor.cc 24 Sep 2007 07:40:32 -0000 ? ? ?1.1
>> +++ testsuite/gdb.cp/mb-ctor.cc 2 Mar 2009 06:39:17 -0000
>> @@ -28,11 +28,19 @@ public:
>> ? ~Derived();
>> ?private:
>> ? int i;
>> + ?int i2;
>> ?};
>>
>> ?Derived::Derived(int i) : Base(i)
>> ?{
>> ? this->i = i;
>> + ?/* The next statement is spread over two lines on purpose to exercise
>> + ? ? a bug where breakpoints set on all but the last line of a statement
>> + ? ? would not get multiple breakpoints.
>> + ? ? The second line's text for gdb_get_line_number is a subset of the
>> + ? ? first line so that we don't care which line gdb prints when it stops. ?*/
>> + ?this->i2 = // set breakpoint here
>> + ? ?i; // breakpoint here
>> ?}
>>
>> ?Derived::~Derived()
>> Index: testsuite/gdb.cp/mb-ctor.exp
>> ===================================================================
>> RCS file: /cvs/src/src/gdb/testsuite/gdb.cp/mb-ctor.exp,v
>> retrieving revision 1.5
>> diff -u -p -r1.5 mb-ctor.exp
>> --- testsuite/gdb.cp/mb-ctor.exp ? ? ? ?3 Jan 2009 05:58:04 -0000 ? ? ? 1.5
>> +++ testsuite/gdb.cp/mb-ctor.exp ? ? ? ?2 Mar 2009 06:39:17 -0000
>> @@ -43,6 +43,11 @@ gdb_start
>> ?gdb_reinitialize_dir $srcdir/$subdir
>> ?gdb_load ${binfile}
>>
>> +if ![runto_main] then {
>> + ? ?perror "couldn't run to breakpoint"
>> + ? ?continue
>> +}
>> +
>> ?# Set a breakpoint with multiple locations
>> ?# and a condition.
>>
>> @@ -50,34 +55,31 @@ gdb_test "break 'Derived::Derived(int)'"
>> ? ? "Breakpoint.*at.* file .*$srcfile, line.*\\(2 locations\\).*" \
>> ? ? "set-breakpoint at ctor"
>>
>> +gdb_breakpoint [gdb_get_line_number "set breakpoint here"]
>> +
>> ?gdb_test "break 'Derived::~Derived()'" \
>> ? ? "Breakpoint.*at.* file .*$srcfile, line.*\\(2 locations\\).*" \
>> ? ? "set-breakpoint at dtor"
>>
>> -gdb_run_cmd
>> -gdb_expect {
>> - ? ?-re "Breakpoint \[0-9\]+,.*Derived.*i=7.*$gdb_prompt $" {
>> - ? ? ? pass "run to breakpoint"
>> - ? ?}
>> - ? ?-re "$gdb_prompt $" {
>> - ? ? ? fail "run to breakpoint"
>> - ? ?}
>> - ? ?timeout {
>> - ? ? ? fail "run to breakpoint (timeout)"
>> - ? ?}
>> -}
>> +gdb_test "continue" \
>> + ? ?".*Breakpoint.*Derived.*i=7.*" \
>> + ? ?"run to breakpoint 1 v1"
>> +
>> +gdb_continue_to_breakpoint "set breakpoint here" ".* breakpoint here"
>>
>> ?gdb_test "continue" \
>> ? ? ".*Breakpoint.*Derived.*i=15.*" \
>> - ? ?"run to breakpoint 2"
>> + ? ?"run to breakpoint 1 v2"
>> +
>> +gdb_continue_to_breakpoint "set breakpoint here" ".* breakpoint here"
>>
>> ?gdb_test "continue" \
>> ? ? ".*Breakpoint.*~Derived.*" \
>> - ? ?"run to breakpoint 3"
>> + ? ?"run to breakpoint 3 v1"
>>
>> ?gdb_test "continue" \
>> ? ? ".*Breakpoint.*~Derived.*" \
>> - ? ?"run to breakpoint 4"
>> + ? ?"run to breakpoint 3 v2"
>>
>> ?gdb_test "continue" \
>> ? ? ".*exited normally.*" \
>> Index: testsuite/gdb.cp/mb-inline.exp
>> ===================================================================
>> RCS file: /cvs/src/src/gdb/testsuite/gdb.cp/mb-inline.exp,v
>> retrieving revision 1.2
>> diff -u -p -r1.2 mb-inline.exp
>> --- testsuite/gdb.cp/mb-inline.exp ? ? ?3 Jan 2009 05:58:04 -0000 ? ? ? 1.2
>> +++ testsuite/gdb.cp/mb-inline.exp ? ? ?2 Mar 2009 06:39:17 -0000
>> @@ -106,3 +106,25 @@ gdb_expect {
>> ?gdb_test "continue" \
>> ? ? ".*Program exited normally.*" \
>> ? ? "continue with disabled breakpoint 1.2"
>> +
>> +# Make sure we can set a breakpoint on a source statement that spans
>> +# multiple lines.
>> +
>> +delete_breakpoints
>> +
>> +set bp_location [gdb_get_line_number "set multi-line breakpoint here" $hdrfile]
>> +
>> +if { ![runto_main] } {
>> + ? ?fail "Can't run to main for multi_line_foo tests."
>> + ? ?return 0
>> +}
>> +
>> +gdb_test "break $hdrfile:$bp_location" \
>> + ? ?"Breakpoint.*at.* file .*$hdrfile, line.*\\(2 locations\\).*" \
>> + ? ?"set multi_line_foo breakpoint"
>> +gdb_test "continue" \
>> + ? ?".*Breakpoint.*multi_line_foo \\(i=0\\).*" \
>> + ? ?"run to multi_line_foo breakpoint 4 afn"
>> +gdb_test "continue" \
>> + ? ?".*Breakpoint.*multi_line_foo \\(i=1\\).*" \
>> + ? ?"run to multi_line_foo breakpoint 4 bfn"
>> Index: testsuite/gdb.cp/mb-inline.h
>> ===================================================================
>> RCS file: /cvs/src/src/gdb/testsuite/gdb.cp/mb-inline.h,v
>> retrieving revision 1.2
>> diff -u -p -r1.2 mb-inline.h
>> --- testsuite/gdb.cp/mb-inline.h ? ? ? ?3 Jan 2009 05:58:04 -0000 ? ? ? 1.2
>> +++ testsuite/gdb.cp/mb-inline.h ? ? ? ?2 Mar 2009 06:39:17 -0000
>> @@ -26,5 +26,12 @@ foo (int i)
>> ? return i; // set breakpoint here
>> ?}
>>
>> +static int
>> +multi_line_foo (int i)
>> +{
>> + ?return // set multi-line breakpoint here
>> + ? ?i;
>> +}
>> +
>> ?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
>> retrieving revision 1.2
>> diff -u -p -r1.2 mb-inline1.cc
>> --- testsuite/gdb.cp/mb-inline1.cc ? ? ?3 Jan 2009 05:58:04 -0000 ? ? ? 1.2
>> +++ testsuite/gdb.cp/mb-inline1.cc ? ? ?2 Mar 2009 06:39:17 -0000
>> @@ -23,7 +23,7 @@
>> ?int
>> ?afn ()
>> ?{
>> - ?return foo (0);
>> + ?return foo (0) + multi_line_foo (0);
>> ?}
>>
>> ?int
>> Index: testsuite/gdb.cp/mb-inline2.cc
>> ===================================================================
>> RCS file: /cvs/src/src/gdb/testsuite/gdb.cp/mb-inline2.cc,v
>> retrieving revision 1.2
>> diff -u -p -r1.2 mb-inline2.cc
>> --- testsuite/gdb.cp/mb-inline2.cc ? ? ?3 Jan 2009 05:58:04 -0000 ? ? ? 1.2
>> +++ testsuite/gdb.cp/mb-inline2.cc ? ? ?2 Mar 2009 06:39:17 -0000
>> @@ -21,5 +21,5 @@
>> ?int
>> ?bfn ()
>> ?{
>> - ?return foo (1);
>> + ?return foo (1) + multi_line_foo (1);
>> ?}
>> Index: testsuite/gdb.cp/mb-templates.cc
>> ===================================================================
>> RCS file: /cvs/src/src/gdb/testsuite/gdb.cp/mb-templates.cc,v
>> retrieving revision 1.1
>> diff -u -p -r1.1 mb-templates.cc
>> --- testsuite/gdb.cp/mb-templates.cc ? ?24 Sep 2007 07:40:32 -0000 ? ? ?1.1
>> +++ testsuite/gdb.cp/mb-templates.cc ? ?2 Mar 2009 06:39:17 -0000
>> @@ -8,6 +8,13 @@ void foo(T i)
>> ? std::cout << "hi\n"; // set breakpoint here
>> ?}
>>
>> +template<class T>
>> +void multi_line_foo(T i)
>> +{
>> + ?std::cout // set multi-line breakpoint here
>> + ? ?<< "hi\n";
>> +}
>> +
>> ?int main()
>> ?{
>> ? ? foo<int>(0);
>> @@ -16,4 +23,9 @@ int main()
>> ? ? foo<double>(1);
>> ? ? foo<int>(2);
>> ? ? foo<double>(2);
>> +
>> + ? ?multi_line_foo<int>(0);
>> + ? ?multi_line_foo<double>(0);
>> +
>> + ? ?return 0;
>> ?}
>> Index: testsuite/gdb.cp/mb-templates.exp
>> ===================================================================
>> RCS file: /cvs/src/src/gdb/testsuite/gdb.cp/mb-templates.exp,v
>> retrieving revision 1.5
>> diff -u -p -r1.5 mb-templates.exp
>> --- testsuite/gdb.cp/mb-templates.exp ? 3 Jan 2009 05:58:04 -0000 ? ? ? 1.5
>> +++ testsuite/gdb.cp/mb-templates.exp ? 2 Mar 2009 06:39:17 -0000
>> @@ -165,3 +165,25 @@ gdb_test "continue" \
>> ? ? ".*Breakpoint.*foo<int> \\(i=1\\).*" \
>> ? ? "instantiation: run to breakpoint 2"
>>
>> +
>> +# Make sure we can set a breakpoint on a source statement that spans
>> +# multiple lines.
>> +
>> +delete_breakpoints
>> +
>> +set bp_location [gdb_get_line_number "set multi-line breakpoint here"]
>> +
>> +if { ![runto_main] } {
>> + ? ?fail "Can't run to main for multi_line_foo tests."
>> + ? ?return 0
>> +}
>> +
>> +gdb_test "break $srcfile:$bp_location" \
>> + ? ?"Breakpoint.*at.* file .*$srcfile, line.*\\(2 locations\\).*" \
>> + ? ?"set multi_line_foo breakpoint"
>> +gdb_test "continue" \
>> + ? ?".*Breakpoint.*multi_line_foo<int> \\(i=0\\).*" \
>> + ? ?"run to multi_line_foo breakpoint 2 <int>"
>> +gdb_test "continue" \
>> + ? ?".*Breakpoint.*multi_line_foo<double> \\(i=0\\).*" \
>> + ? ?"run to multi_line_foo breakpoint 2 <double>"
>>
>