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

[Doug Evans <dje@google.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>"
>>
>