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

Thomas Maier
2005-05-03T17:46:18Z

[-- 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]);