This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: Breakpoints in constructors
On Wednesday 04 Jan 2006 1:10 am, Daniel Jacobowitz wrote:
> On Tue, Jan 03, 2006 at 06:35:45PM +0530, Amit Kale wrote:
> > Hi All,
> >
> > I've rebased the breakpoints in constructors patch to current cvs. It's
> > attached for reviews and a submission into mainline gdb after any
> > modifications/improvements as per gdb gurus' suggestions.
> >
> > I ran gdb testsuite with this patch and found several failures which were
> > absent in a cvs-built gdb. They are mainly related to "advance" command.
> > I am looking at the failures and trying to fix them. I'll send an update
> > if I can fix any of them.
> >
> > I'll very much appreciate any feedback for this.
>
> Hi Amit, and sorry for not getting back to you about this last time you
> posted it on gdb@.
>
> The short version is that I believe this is roughly the right approach,
> but not quite. I posted a work-in-progress patch some time ago that
> takes a slightly different approach:
>
> Date: Sun, 13 Mar 2005 19:38:24 -0500
> From: Daniel Jacobowitz <drow@false.org>
> Subject: RFC: First stab at breakpoints in multiple locations
>
> You might want to take a look at that thread, if you haven't already,
> to see what I mean. The basic difference is that instead of "break
> Foo::Foo" setting multiple breakpoints, it would set only one
> breakpoint, but that breakpoint would be associated with multiple PC
> values.
It's a large patch, though I like your approach. Multiple breakpoints
appearing when only one was specified might confuse a user.
>
> I can't really tell from your patch what cases you do handle or don't;
> do breakpoints on constructors by name work? How about by line number?
It handles breakpoints specified by line numbers. It doesn't work for
constructors by name (e.g. MyClass::MyClass). The original patch I had
written for gdb-5.3.* worked for names also. Seems like some part of the
rebase went wrong. I'll look into that.
gdb testsuite showed several failures. One of them was "advance" command
didn't work. I fixed that. I have attached the same patch with this fix in
it.
I was not able to fix other failures though. The major one being a breakpoint
on a statement in main causes three breakpoints to appear. It's because this
statement is a call to a C++ inlined function. The expansion of inline
function dodges the check in my patch for multiple breakpoints appearing for
the same C statement when the statement is spread across multiple
instructions interlaced with instructions from surrounding statements. I
checked for function name, which would be the inline function name. That's
why my check fails. Sigh, I wanted to send this patch again only after fixing
this problem, but don't have any good ideas on how to do that. I'll very much
appreciate if anyone has ideas about fixing this.
Thanks.
-Amit
Index: gdb-cvs/gdb/breakpoint.c
===================================================================
--- gdb-cvs.orig/gdb/breakpoint.c 2006-01-03 11:24:29.000000000 +0530
+++ gdb-cvs/gdb/breakpoint.c 2006-01-04 16:20:06.000000000 +0530
@@ -4518,7 +4518,8 @@
make_cleanup (xfree, canonical[0]);
}
- resolve_sal_pc (&sals.sals[0]);
+ /* solib hook doesn't have multiple instances */
+ resolve_sal_pc (&sals, NULL, NULL);
/* Remove the canonical strings from the cleanup, they are needed below. */
if (canonical != (char **) NULL)
@@ -5075,34 +5076,39 @@
static void
breakpoint_sals_to_pc (struct symtabs_and_lines *sals,
- char *address)
+ char *address, char ***addr_string)
{
- int i;
+ int i, j;
for (i = 0; i < sals->nelts; i++)
{
- resolve_sal_pc (&sals->sals[i]);
+ j = i;
+ resolve_sal_pc (sals, &i, addr_string);
/* It's possible for the PC to be nonzero, but still an illegal
- value on some targets.
-
- For example, on HP-UX if you start gdb, and before running the
- inferior you try to set a breakpoint on a shared library function
- "foo" where the inferior doesn't call "foo" directly but does
- pass its address to another function call, then we do find a
- minimal symbol for the "foo", but it's address is invalid.
- (Appears to be an index into a table that the loader sets up
- when the inferior is run.)
+ value on some targets.
- Give the target a chance to bless sals.sals[i].pc before we
- try to make a breakpoint for it. */
+ For example, on HP-UX if you start gdb, and before running the
+ inferior you try to set a breakpoint on a shared library function
+ "foo" where the inferior doesn't call "foo" directly but does
+ pass its address to another function call, then we do find a
+ minimal symbol for the "foo", but it's address is invalid.
+ (Appears to be an index into a table that the loader sets up
+ when the inferior is run.)
+
+ Give the target a chance to bless sals.sals[j].pc before we
+ try to make a breakpoint for it. */
#ifdef DEPRECATED_PC_REQUIRES_RUN_BEFORE_USE
- if (DEPRECATED_PC_REQUIRES_RUN_BEFORE_USE (sals->sals[i].pc))
+ while (j <= i)
{
- if (address == NULL)
- error (_("Cannot break without a running program."));
- else
- error (_("Cannot break on %s without a running program."),
- address);
+ if (DEPRECATED_PC_REQUIRES_RUN_BEFORE_USE (sals->sals[j].pc))
+ {
+ if (address == NULL)
+ error (_("Cannot break without a running program."));
+ else
+ error (_("Cannot break on %s without a running program."),
+ address);
+ }
+ j++;
}
#endif
}
@@ -5210,6 +5216,12 @@
return GDB_RC_FAIL;
}
+ /* Resolve all line numbers to PC's and verify that the addresses
+ are ok for the target. This function call can result in an increase in
+ the number of nelts */
+ if (!pending)
+ breakpoint_sals_to_pc (&sals, addr_start, &addr_string);
+
/* Create a chain of things that always need to be cleaned up. */
old_chain = make_cleanup (null_cleanup, 0);
@@ -5245,11 +5257,6 @@
make_cleanup (xfree, addr_string[i]);
}
- /* Resolve all line numbers to PC's and verify that the addresses
- are ok for the target. */
- if (!pending)
- breakpoint_sals_to_pc (&sals, addr_start);
-
/* Verify that condition can be parsed, before setting any
breakpoints. Allocate a separate condition expression for each
breakpoint. */
@@ -5386,6 +5393,11 @@
if (!sals.nelts)
return GDB_RC_NONE;
+ /* Resolve all line numbers to PC's and verify that the addresses
+ are ok for the target. This function call can result in an increase in
+ the number of nelts */
+ breakpoint_sals_to_pc (&sals, args->address, &addr_string);
+
/* Create a chain of things at always need to be cleaned up. */
old_chain = make_cleanup (null_cleanup, 0);
@@ -5424,9 +5436,6 @@
if (*address_end != '\0')
error (_("Garbage %s following breakpoint address"), address_end);
- /* Resolve all line numbers to PC's. */
- breakpoint_sals_to_pc (&sals, args->address);
-
/* Verify that conditions can be parsed, before setting any
breakpoints. */
for (i = 0; i < sals.nelts; i++)
@@ -5473,17 +5482,30 @@
error_message, RETURN_MASK_ALL);
}
-
-/* Helper function for break_command_1 and disassemble_command. */
-
+/* Helper function for break_command_1 and disassemble_command.
+ If index is NULL doesn't extend sals. Assumes that there is only one sals
+ entry. Otherwise extends sals if more than one
+ entry is found for given line number.
+ */
+
void
-resolve_sal_pc (struct symtab_and_line *sal)
+resolve_sal_pc (struct symtabs_and_lines *sals, int *index, char
+ ***addr_string)
{
CORE_ADDR pc;
+ struct symtab_and_line *sal;
+ int ind = 0;
+ int i;
+ CORE_ADDR origpc;
+ if (index)
+ sal = &sals->sals[*index];
+ else
+ sal = &sals->sals[0];
+
if (sal->pc == 0 && sal->symtab != NULL)
{
- if (!find_line_pc (sal->symtab, sal->line, &pc))
+ if ((ind = find_line_pc (sal->symtab, sal->line, &pc, ind)) == 0)
error (_("No line %d in file \"%s\"."),
sal->line, sal->symtab->filename);
sal->pc = pc;
@@ -5521,6 +5543,54 @@
}
}
}
+ if (index && sal->pc)
+ {
+ struct symbol *psym;
+ struct symbol *nsym;
+ struct block *bl;
+ origpc = sal->pc;
+ bl = block_for_pc(sal->pc);
+ psym = bl ? block_function(bl): NULL;
+ while ((ind = find_line_pc (sal->symtab, sal->line, &pc, ind)) != 0)
+ {
+ /* Insert this entry into sals if it isn't the duplicate of the
+ * first entry and if it doesn't appear in the same function as the
+ * previous entry */
+ bl = block_for_pc(pc);
+ nsym = bl ? block_function(bl): NULL;
+ if (pc != origpc && psym != nsym)
+ {
+ psym = nsym;
+ sals->sals = xrealloc(sals->sals, sizeof (struct symtab_and_line) *
+ (sals->nelts + 1));
+ if (addr_string)
+ *addr_string = xrealloc(*addr_string, sizeof (char **) *
+ (sals->nelts + 1));
+
+ for (i = sals->nelts; i > *index; i--)
+ {
+ sals->sals[i] = sals->sals[i - 1];
+ if (addr_string)
+ (*addr_string)[i] = (*addr_string)[i - 1];
+ }
+ sals->nelts++;
+ (*index)++;
+ sals->sals[*index].pc = pc;
+ if (addr_string)
+ {
+ if ((*addr_string)[*index - 1])
+ {
+ (*addr_string)[*index] =
+ savestring((*addr_string)[*index - 1],
+ strlen((*addr_string)[*index - 1]));
+ }
+ else
+ (*addr_string)[*index] = NULL;
+ }
+ sal = &sals->sals[*index];
+ }
+ }
+ }
}
void
@@ -5948,13 +6018,16 @@
if (sals.nelts != 1)
error (_("Couldn't get information on specified line."));
- sal = sals.sals[0];
- xfree (sals.sals); /* malloc'd, so freed */
-
if (*arg)
- error (_("Junk at end of arguments."));
+ {
+ xfree (sals.sals); /* malloc'd, so freed */
+ error ("Junk at end of arguments.");
+ }
+
+ resolve_sal_pc (&sals, NULL, NULL);
+ sal = sals.sals[0];
- resolve_sal_pc (&sal);
+ xfree (sals.sals); /* malloc'd, so freed */
if (anywhere)
/* If the user told us to continue until a specified location,
@@ -7056,7 +7129,36 @@
not_found_ptr);
for (i = 0; i < sals.nelts; i++)
{
- resolve_sal_pc (&sals.sals[i]);
+ int j = i;
+ int thisbpt = i;
+ struct breakpoint *bpt;
+
+ resolve_sal_pc (&sals, &i, NULL);
+
+ /* If there are multiple breakpoints for this address. We need to
+ * find out which one of them corresponds to this breakpoint. If any
+ * of them correspond to some other breakpoints, they'll be handled
+ * later when we try to insert them. */
+
+ while (j <= i)
+ {
+ ALL_BREAKPOINTS(bpt)
+ if (bpt->loc->address == sals.sals[j].pc)
+ break;
+ if (!bpt)
+ {
+ if (j < i)
+ /* There is no breakpoint corresponding to this
+ * address. We'll have to create one. FIXME: can't
+ * handle that for multiple breakpoints yet */
+ error("New breakpoint found while reserting "
+ "breakpoints");
+ }
+ else
+ if (bpt == b)
+ thisbpt = j;
+ j++;
+ }
/* Reparse conditions, they might contain references to the
old symtab. */
@@ -7070,33 +7172,33 @@
to parse_exp_1. */
b->cond = NULL;
}
- b->cond = parse_exp_1 (&s, block_for_pc (sals.sals[i].pc), 0);
+ b->cond = parse_exp_1 (&s, block_for_pc (sals.sals[thisbpt].pc), 0);
}
/* We need to re-set the breakpoint if the address changes... */
- if (b->loc->address != sals.sals[i].pc
+ if (b->loc->address != sals.sals[thisbpt].pc
/* ...or new and old breakpoints both have source files, and
the source file name or the line number changes... */
|| (b->source_file != NULL
- && sals.sals[i].symtab != NULL
- && (strcmp (b->source_file, sals.sals[i].symtab->filename) != 0
- || b->line_number != sals.sals[i].line)
+ && sals.sals[thisbpt].symtab != NULL
+ && (strcmp (b->source_file, sals.sals[thisbpt].symtab->filename) != 0
+ || b->line_number != sals.sals[thisbpt].line)
)
/* ...or we switch between having a source file and not having
one. */
- || ((b->source_file == NULL) != (sals.sals[i].symtab == NULL))
+ || ((b->source_file == NULL) != (sals.sals[thisbpt].symtab == NULL))
)
{
if (b->source_file != NULL)
xfree (b->source_file);
- if (sals.sals[i].symtab == NULL)
+ if (sals.sals[thisbpt].symtab == NULL)
b->source_file = NULL;
else
b->source_file =
- savestring (sals.sals[i].symtab->filename,
- strlen (sals.sals[i].symtab->filename));
- b->line_number = sals.sals[i].line;
- b->loc->requested_address = sals.sals[i].pc;
+ savestring (sals.sals[thisbpt].symtab->filename,
+ strlen (sals.sals[thisbpt].symtab->filename));
+ b->line_number = sals.sals[thisbpt].line;
+ b->loc->requested_address = sals.sals[thisbpt].pc;
b->loc->address
= adjust_breakpoint_address (b->loc->requested_address,
b->type);
@@ -7111,13 +7213,14 @@
rather than once for every breakpoint. */
breakpoints_changed ();
}
- b->loc->section = sals.sals[i].section;
+ b->loc->section = sals.sals[thisbpt].section;
b->enable_state = save_enable; /* Restore it, this worked. */
/* Now that this is re-enabled, check_duplicates
can be used. */
check_duplicates (b);
+ i = j;
}
xfree (sals.sals);
Index: gdb-cvs/gdb/dwarf2read.c
===================================================================
--- gdb-cvs.orig/gdb/dwarf2read.c 2006-01-03 11:24:30.000000000 +0530
+++ gdb-cvs/gdb/dwarf2read.c 2006-01-03 11:27:31.000000000 +0530
@@ -3515,6 +3515,11 @@
else
return;
+ /* Check for artificial methods. */
+ attr = dwarf2_attr (die, DW_AT_artificial, cu);
+ if (attr && DW_UNSND (attr) != 0)
+ return;
+
/* Get the mangled name. */
physname = dwarf2_linkage_name (die, cu);
@@ -3608,11 +3613,6 @@
}
}
- /* Check for artificial methods. */
- attr = dwarf2_attr (die, DW_AT_artificial, cu);
- if (attr && DW_UNSND (attr) != 0)
- fnp->is_artificial = 1;
-
/* Get index in virtual function table if it is a virtual member function. */
attr = dwarf2_attr (die, DW_AT_vtable_elem_location, cu);
if (attr)
Index: gdb-cvs/gdb/infcmd.c
===================================================================
--- gdb-cvs.orig/gdb/infcmd.c 2006-01-03 11:24:31.000000000 +0530
+++ gdb-cvs/gdb/infcmd.c 2006-01-03 11:50:44.000000000 +0530
@@ -878,12 +878,15 @@
}
sal = sals.sals[0];
- xfree (sals.sals);
if (sal.symtab == 0 && sal.pc == 0)
- error (_("No source file has been specified."));
+ {
+ xfree (sals.sals);
+ error ("No source file has been specified.");
+ }
- resolve_sal_pc (&sal); /* May error out */
+ resolve_sal_pc (&sals, NULL, NULL); /* May error out */
+ xfree (sals.sals);
/* See if we are trying to jump to another function. */
fn = get_frame_function (get_current_frame ());
Index: gdb-cvs/gdb/symtab.c
===================================================================
--- gdb-cvs.orig/gdb/symtab.c 2006-01-03 11:24:36.000000000 +0530
+++ gdb-cvs/gdb/symtab.c 2006-01-03 11:27:31.000000000 +0530
@@ -73,7 +73,7 @@
static void output_source_filename (const char *, int *);
-static int find_line_common (struct linetable *, int, int *);
+static int find_line_common (struct linetable *, int, int *, int, int);
/* This one is used by linespec.c */
@@ -2231,18 +2231,34 @@
find_line_symtab (struct symtab *symtab, int line, int *index, int *exact_match)
{
int exact;
+ int find_exact = 0;
/* BEST_INDEX and BEST_LINETABLE identify the smallest linenumber > LINE
so far seen. */
int best_index;
+ int start_index = 0;
struct linetable *best_linetable;
struct symtab *best_symtab;
/* First try looking it up in the given symtab. */
best_linetable = LINETABLE (symtab);
+
+ /* Check whether we are continuing a previous search */
+ if (best_linetable && index && *index)
+ {
+ if (*index >= best_linetable->nitems)
+ return NULL;
+ if (best_linetable->item[(*index) - 1].line != line)
+ /* We are continuing an approximate search. Continue with the line
+ * numbers that exactly match previous hit */
+ line = best_linetable->item[(*index) - 1].line;
+ find_exact = 1;
+ start_index = *index;
+ }
best_symtab = symtab;
- best_index = find_line_common (best_linetable, line, &exact);
+ best_index = find_line_common (best_linetable, line, &exact, find_exact,
+ start_index);
if (best_index < 0 || !exact)
{
/* Didn't find an exact match. So we better keep looking for
@@ -2273,7 +2289,7 @@
if (strcmp (symtab->filename, s->filename) != 0)
continue;
l = LINETABLE (s);
- ind = find_line_common (l, line, &exact);
+ ind = find_line_common (l, line, &exact, find_exact, start_index);
if (ind >= 0)
{
if (exact)
@@ -2305,15 +2321,16 @@
return best_symtab;
}
-/* Set the PC value for a given source file and line number and return true.
- Returns zero for invalid line number (and sets the PC to 0).
- The source file is specified with a struct symtab. */
+/* Set the PC value for a given source file and line number and return index
+ into the line table for the symbol table;
+ Returns 0 for if doesn't find an entry (and sets the PC to 0).
+ The source file is specified with a struct symtab.
+ Begins the search at index ind */
int
-find_line_pc (struct symtab *symtab, int line, CORE_ADDR *pc)
+find_line_pc (struct symtab *symtab, int line, CORE_ADDR *pc, int ind)
{
struct linetable *l;
- int ind;
*pc = 0;
if (symtab == 0)
@@ -2324,7 +2341,7 @@
{
l = LINETABLE (symtab);
*pc = l->item[ind].pc;
- return 1;
+ return ind + 1;
}
else
return 0;
@@ -2344,7 +2361,7 @@
struct symtab_and_line found_sal;
startaddr = sal.pc;
- if (startaddr == 0 && !find_line_pc (sal.symtab, sal.line, &startaddr))
+ if (startaddr == 0 && !find_line_pc (sal.symtab, sal.line, &startaddr, 0))
return 0;
/* This whole function is based on address. For example, if line 10 has
@@ -2373,11 +2390,13 @@
table for the pc of the nearest line whose number is >= the specified one.
Return -1 if none is found. The value is >= 0 if it is an index.
- Set *EXACT_MATCH nonzero if the value returned is an exact match. */
+ Set *EXACT_MATCH nonzero if the value returned is an exact match.
+ If find_exact is non-zero, it forces an exact match
+ start_index is the index to start searching from */
static int
find_line_common (struct linetable *l, int lineno,
- int *exact_match)
+ int *exact_match, int find_exact, int start_index)
{
int i;
int len;
@@ -2395,7 +2414,7 @@
return -1;
len = l->nitems;
- for (i = 0; i < len; i++)
+ for (i = start_index; i < len; i++)
{
struct linetable_entry *item = &(l->item[i]);
@@ -2405,6 +2424,8 @@
*exact_match = 1;
return i;
}
+ if (find_exact)
+ continue;
if (item->line > lineno && (best == 0 || item->line < best))
{
Index: gdb-cvs/gdb/symtab.h
===================================================================
--- gdb-cvs.orig/gdb/symtab.h 2006-01-03 11:24:36.000000000 +0530
+++ gdb-cvs/gdb/symtab.h 2006-01-03 11:27:31.000000000 +0530
@@ -1256,12 +1256,12 @@
/* Given a symtab and line number, return the pc there. */
-extern int find_line_pc (struct symtab *, int, CORE_ADDR *);
+extern int find_line_pc (struct symtab *, int, CORE_ADDR *, int);
extern int find_line_pc_range (struct symtab_and_line, CORE_ADDR *,
CORE_ADDR *);
-extern void resolve_sal_pc (struct symtab_and_line *);
+extern void resolve_sal_pc (struct symtabs_and_lines *, int *, char ***);
/* Given a string, return the line specified by it. For commands like "list"
and "breakpoint". */
Index: gdb-cvs/gdb/tracepoint.c
===================================================================
--- gdb-cvs.orig/gdb/tracepoint.c 2006-01-03 11:24:36.000000000 +0530
+++ gdb-cvs/gdb/tracepoint.c 2006-01-03 11:27:31.000000000 +0530
@@ -390,6 +390,7 @@
struct tracepoint *t;
char *addr_start = 0, *addr_end = 0;
int i;
+ int j;
if (!arg || !*arg)
error (_("trace command requires an argument"));
@@ -406,7 +407,11 @@
/* Resolve all line numbers to PC's */
for (i = 0; i < sals.nelts; i++)
- resolve_sal_pc (&sals.sals[i]);
+ {
+ /* Let resolve_sal_pc insert sals if required */
+ j = i;
+ resolve_sal_pc (&sals, &j, NULL);
+ }
/* Now set all the tracepoints. */
for (i = 0; i < sals.nelts; i++)
@@ -2398,7 +2403,7 @@
return; /* presumably decode_line_1 has already warned */
/* Resolve line numbers to PC */
- resolve_sal_pc (&sals.sals[0]);
+ resolve_sal_pc (&sals, NULL, NULL);
block = block_for_pc (sals.sals[0].pc);
while (block != 0)
Index: gdb-cvs/gdb/mi/mi-cmd-disas.c
===================================================================
--- gdb-cvs.orig/gdb/mi/mi-cmd-disas.c 2006-01-03 11:24:40.000000000 +0530
+++ gdb-cvs/gdb/mi/mi-cmd-disas.c 2006-01-03 11:52:30.000000000 +0530
@@ -148,7 +148,7 @@
s = lookup_symtab (file_string);
if (s == NULL)
error (_("mi_cmd_disassemble: Invalid filename."));
- if (!find_line_pc (s, line_num, &start))
+ if (!find_line_pc (s, line_num, &start, 0))
error (_("mi_cmd_disassemble: Invalid line number"));
if (find_pc_partial_function (start, NULL, &low, &high) == 0)
error (_("mi_cmd_disassemble: No function contains specified address"));
Index: gdb-cvs/gdb/tui/tui-layout.c
===================================================================
--- gdb-cvs.orig/gdb/tui/tui-layout.c 2006-01-03 11:24:52.000000000 +0530
+++ gdb-cvs/gdb/tui/tui-layout.c 2006-01-03 11:53:15.000000000 +0530
@@ -520,7 +520,7 @@
case SRC_DATA_COMMAND:
find_line_pc (cursal.symtab,
TUI_SRC_WIN->detail.source_info.start_line_or_addr.u.line_no,
- &pc);
+ &pc, 0);
addr = pc;
break;
case DISASSEM_COMMAND:
Index: gdb-cvs/gdb/tui/tui-win.c
===================================================================
--- gdb-cvs.orig/gdb/tui/tui-win.c 2006-01-03 11:24:52.000000000 +0530
+++ gdb-cvs/gdb/tui/tui-win.c 2006-01-03 11:53:31.000000000 +0530
@@ -1343,7 +1343,7 @@
else
{
line.loa = LOA_ADDRESS;
- find_line_pc (s, cursal.line, &line.u.addr);
+ find_line_pc (s, cursal.line, &line.u.addr, 0);
}
tui_update_source_window (win_info, s, line, TRUE);
}
Index: gdb-cvs/gdb/tui/tui-winsource.c
===================================================================
--- gdb-cvs.orig/gdb/tui/tui-winsource.c 2006-01-03 11:24:52.000000000 +0530
+++ gdb-cvs/gdb/tui/tui-winsource.c 2006-01-03 11:27:31.000000000 +0530
@@ -180,7 +180,7 @@
{
case DISASSEM_COMMAND:
case DISASSEM_DATA_COMMAND:
- find_line_pc (s, line, &pc);
+ find_line_pc (s, line, &pc, 0);
tui_update_source_windows_with_addr (pc);
break;
default:
@@ -189,7 +189,7 @@
tui_show_symtab_source (s, l, FALSE);
if (tui_current_layout () == SRC_DISASSEM_COMMAND)
{
- find_line_pc (s, line, &pc);
+ find_line_pc (s, line, &pc, 0);
tui_show_disassem (pc);
}
break;