Re: gdb 6.3 C++ breakpoint in constructor/destructor patch

["Thomas Maier" <balagi@justmail.de>]

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

Hello,

ok, second try...

This patch only modifies breakpoint.c
It "expands" the sals returned by decode_line_1().
Means all symtabs are searched for a linetable entries that match the  
required
filename and lineno (returned by decode_line_1()) and then
new sal's are created, and this creates (in some cases, e.g.  
C++ constructors of
gcc 4) more then one breakpoint
at the same line and file, but at different pc's.

The expand is done in a new function called breakpoint_sals_expand().


On a i686 platform with gcc 4.0 i had no problems with this modification  
so far.
I think, it works good enough for me ;) At least till the gdb experts fix  
this
problem themselfs ;)

-Thomas Maier

On Sun, 1 May 2005 17:56:08 -0400, Daniel Jacobowitz <drow@false.org>  
wrote:

> On Sun, May 01, 2005 at 10:53:05PM +0200, Thomas Maier wrote:
>> Ok, hmm, then the code in find_line_symtab() must not return one symtab,
>> which is used to find one or more references to the lineno, it must  
>> return
>> a list of symtabs with references to this lineno. Means, a similar  
>> search
>> over all symtabs which is already done in find_line_symtab().
>> Would that be enough to handle the file:lineno case?
>>
>> (If yes it think a new function like find_multi_line_symtab() is the  
>> best
>> way to do it, then in decode_all_digits() fill in the sals with the  
>> found
>> symtabs + linenos)
>
> It's a representation problem.  I found it much more effective to have
> decode_line_1 only return one symtab_and_line structure, and then
> expand them to multiple PCs later, regardless of whether the symtab
> returned was the only relevant one.  Otherwise, things like "list" will
> get confused.
>


[-- Attachment #2: patch-gdb-6.3-c++-bpconstr --]
[-- Type: application/octet-stream, Size: 7023 bytes --]

diff -cpNr gdb-6.3/gdb/ChangeLog gdb-6.3.1/gdb/ChangeLog
*** gdb-6.3/gdb/ChangeLog	Mon Nov  8 17:21:20 2004
--- gdb-6.3.1/gdb/ChangeLog	Tue May  3 17:11:53 2005
***************
*** 1,3 ****
--- 1,32 ----
+ 2005-04-30  Thomas Maier   <balagi@justmail.de>
+ 
+ 	* breakpoint.c
+ 	  Add better support for breakpoints in C++ constructors
+ 	  and destructors.
+ 	  NOTE: this is only a simple hack.
+ 	  It creates more than one
+ 	  breakpoint at the source at lineno, but each 
+ 	  generated breakpoint will have another address!
+ 	  
+ 	See also: (taken from PROBLEMS file)
+ 	gdb/1091: Constructor breakpoints ignored
+ 	gdb/1193: g++ 3.3 creates multiple constructors: gdb 5.3 can't set breakpoints
+ 
+ 	When gcc 3.x compiles a C++ constructor or C++ destructor, it generates
+ 	2 or 3 different versions of the object code.  These versions have
+ 	unique mangled names (they have to, in order for linking to work), but
+ 	they have identical source code names, which leads to a great deal of
+ 	confusion.  Specifically, if you set a breakpoint in a constructor or a
+ 	destructor, gdb will put a breakpoint in one of the versions, but your
+ 	program may execute the other version.  This makes it impossible to set
+ 	breakpoints reliably in constructors or destructors.
+ 
+ 	gcc 3.x generates these multiple object code functions in order to
+ 	implement virtual base classes.  gcc 2.x generated just one object code
+ 	function with a hidden parameter, but gcc 3.x conforms to a multi-vendor
+ 	ABI for C++ which requires multiple object code functions.
+ 
+ 
  2004-11-08  Andrew Cagney  <cagney@gnu.org>
  
  	GDB 6.3 released.
diff -cpNr gdb-6.3/gdb/breakpoint.c gdb-6.3.1/gdb/breakpoint.c
*** gdb-6.3/gdb/breakpoint.c	Fri Oct  8 19:30:46 2004
--- gdb-6.3.1/gdb/breakpoint.c	Tue May  3 17:16:07 2005
*************** do_captured_parse_breakpoint (struct ui_
*** 5098,5103 ****
--- 5098,5205 ----
    return GDB_RC_OK;
  }
  
+ /* balagi 05/02/2005
+    Try to find other entries in all available symtabs that match
+    filename and line number.
+    This is done only if language == C++ !!!
+    
+    NOTE: sal.pc will be set here if language == C++
+    
+    *SALS is replaced by a new allocated structure, the previous
+    content is released. Dito *ADDR_STRING .
+ */
+ static void
+ breakpoint_sals_expand (struct symtabs_and_lines* sals, char*** addr_string)
+ {
+   /* assume max 128 other entries, should be enough ;) */
+   int maxsal = sals->nelts + 128;
+   int i;
+   struct symtabs_and_lines xsals;
+   xsals.sals = (struct symtab_and_line*)
+   		xmalloc(sizeof(struct symtab_and_line*) * maxsal);
+   xsals.nelts = 0;
+   
+   /* need to rebuild the addr_string's, if requested ! */
+   char** xaddr_string = NULL;
+   if ( addr_string && *addr_string )
+     xaddr_string = (char**)xmalloc(sizeof(char*) * maxsal);
+   
+   /* for each sal */
+   for ( i = 0; i < sals->nelts; i++ )
+   {
+     struct symtab_and_line* sal = &(sals->sals[i]);
+     struct objfile* objfile;
+     struct symtab* st;
+     
+     if ( sal->symtab->language != language_cplus )
+     {
+       /* not C++, simply copy the sal into xsals */
+       if ( xsals.nelts < maxsal )
+       {
+ 	if ( xaddr_string )
+ 	  xaddr_string[xsals.nelts] = ((*addr_string)[i]
+ 	  		? xstrdup((*addr_string)[i]) : NULL);
+       	xsals.sals[xsals.nelts++] = *sal;
+       }
+       continue;
+     }
+     
+     /* search all symtabs to find a match for other entries */
+     ALL_SYMTABS(objfile, st)
+     {
+       struct linetable* l;
+       int k;
+       if (strcmp(sal->symtab->filename, st->filename) != 0)
+         continue;
+       l = LINETABLE(st);
+       for ( k = 0; k < l->nitems; k++ )
+       {
+         int j;
+ 	
+         if ( l->item[k].line != sal->line )
+ 	  continue;
+ 	
+ 	/* avoid duplicates. only match pc, should be
+ 	   good enough.
+ 	*/
+ 	for (j = 0; j < xsals.nelts; j++)
+ 	{
+ 	  if ( xsals.sals[j].pc != 0
+ 	  	&& xsals.sals[j].pc == l->item[k].pc )
+ 	    break;
+ 	}
+ 	if ( j < xsals.nelts )
+ 	  continue;
+ 	
+ 	if ( xsals.nelts >= maxsal )
+ 	  continue;
+ 	if ( xaddr_string )
+ 	  xaddr_string[xsals.nelts] = ((*addr_string)[i]
+ 	  		? xstrdup((*addr_string)[i]) : NULL);
+ 	struct symtab_and_line* sl = &(xsals.sals[xsals.nelts++]);
+ 	init_sal(sl);
+ 	sl->symtab = st;
+ 	sl->line = sal->line;
+ 	sl->pc = l->item[k].pc; /* set pc !! */
+       }
+     }
+   }
+   
+   /* ok, cleanup old structures and copy over new ones */
+   if ( addr_string && *addr_string != NULL )
+   { 
+     for ( i = 0; i < sals->nelts; i++ )
+     {
+       if ( (*addr_string)[i] != NULL )
+         xfree((*addr_string)[i]);
+     }
+     xfree(*addr_string);
+     *addr_string = xaddr_string;
+   }
+   xfree(sals->sals);
+   *sals = xsals;
+ }
+ 
  /* Set a breakpoint according to ARG (function, linenum or *address)
     flag: first bit  : 0 non-temporary, 1 temporary.
     second bit : 0 normal breakpoint, 1 hardware breakpoint. 
*************** break_command_1 (char *arg, int flag, in
*** 5189,5194 ****
--- 5291,5299 ----
  
    if (!pending)
      {
+       /* balagi 05/02/2005  expand (C++) breakpoints... */
+       breakpoint_sals_expand(&sals, &addr_string);
+     
        /* Make sure that all storage allocated to SALS gets freed.  */
        make_cleanup (xfree, sals.sals);
        
*************** do_captured_breakpoint (void *data)
*** 5364,5369 ****
--- 5469,5477 ----
  
    /* Create a chain of things at always need to be cleaned up. */
    old_chain = make_cleanup (null_cleanup, 0);
+   
+   /* balagi 05/02/2005  expand (C++) breakpoints... */
+   breakpoint_sals_expand(&sals, &addr_string);
  
    /* Always have a addr_string array, even if it is empty. */
    make_cleanup (xfree, addr_string);
*************** breakpoint_re_set_one (void *bint)
*** 7206,7211 ****
--- 7314,7350 ----
        s = b->addr_string;
        sals = decode_line_1 (&s, 1, (struct symtab *) NULL, 0, (char ***) NULL,
  		            not_found_ptr);
+       
+       /* balagi 05/02/2005
+          This is really a bad hack. If there are more then one sal
+ 	 expanded by breakpoint_sals_expand(),
+ 	 we must find the matching sal to the given breakpoint.
+ 	 We only can use pc here, can't we?!
+ 	 What if no matching pc to b->loc->address is found??
+ 	 Hmm, don't know....
+       */
+       breakpoint_sals_expand(&sals, NULL);
+       for (i = 0; i < sals.nelts; i++)
+       {
+         /* only for C++ !! */
+         if ( sals.sals[i].symtab->language != language_cplus )
+ 	  continue;
+ 	if ( sals.sals[i].pc == b->loc->address )
+ 	{
+ 	  sals.nelts = 1;
+ 	  if ( i > 0 )
+ 	    sals.sals[0] = sals.sals[i];
+ 	  break;
+ 	}
+       }
+ 
+       /* balagi 05/02/2005
+          Question: is it a good idea, to allow to have multiple
+ 	 sal's here? Can decode_line_1() return more than one
+ 	 sal? If yes, how to match the breakpoint with the
+ 	 multiple sal's ?
+       */
+ 
        for (i = 0; i < sals.nelts; i++)
  	{
  	  resolve_sal_pc (&sals.sals[i]);