This is the mail archive of the
gdb-patches@sources.redhat.com
mailing list for the GDB project.
Re: RFC: The Constructor Breakpoint Issue
- From: David Lecomber <david at streamline-computing dot com>
- To: patches <gdb-patches at sources dot redhat dot com>
- Date: Fri, 15 Oct 2004 14:41:39 +0100
- Subject: Re: RFC: The Constructor Breakpoint Issue
- References: <1097714229.11117.99.camel@localhost.localdomain>
Morning all,
I have reformatted this so it actually compiles - I hope...
Unfortunately this patch busts a lot of test cases (about 30..). I
have discovered that it is being over-zealous in applying breakpoints
and need an expert to suggest solutions.
The first broken example is gdb.base/display.c - there we have a for
loop. So, first line of the for loop (display.c:14) is at PC xxxxxx,
and also at xxxxxx + some no. which presumably is where the increment
part of the for loop occurs. Do we want to get both these addresses for
breakpoints, or not.. because it happens..
I see the intel compiler sets the default is_stmt field to zero, and
never does anything else with it, and the GNU compilers set it to 1.
However, dwarf2read ignores this info anyway.
I cannot see anything in the dwarf2 line info that enables us to
distinguish between the two cases - those where the lines are not really
duplicate, and those that are. I think everyone would find fixing this
issue important..
Opinions??
d.
On Thu, 2004-10-14 at 01:37, David Lecomber wrote:
> Specifically, this one:
> gdb/1091: Constructor breakpoints ignored
> gdb/1193: g++ 3.3 creates multiple constructors: gdb 5.3 can't set
> breakpoints
>
> I see this happening in other situations. For example, Intel's compiler
> seems to produce two versions of each subroutine -- perhaps each is
> optimized for a different architecture. This means that, say, 'break
> test.f:65' will usually put the breakpoint in the wrong one (c.f.
> Murphy's Law).
>
> I have implemented a fix, which works, presently I'm not too proud of
> it, so I'm going to check it through before proposing the patch
> formally..
>
> However, does anyone have any problem with the approach.
>
> In summary, I change the linetable_entry structure to have a pointer,
> 'next', which will be the next linetable_entry in the table which has
> the same source code and line.
>
> find_line_pc then actually modifies the symtab_and_line parameter to get
> the linetable_entry (rather than just pulling the PC from it).
>
> In the breakpoints code, if a symtab_and_line has several entries in the
> chain of entries, it sets a new breakpoint for each one.
>
> This does mean that break test.f:65 will announce each breakpoint
> separately. Also, the first bpoint set is known by the line/source, the
> others can be distinguished by their address.
>
> It cures the breakpoint in the constructor issue, which is probably the
> quickest way for a GNU compiler based check of the fix.
>
> d.
>
>
Index: gdb/breakpoint.c
===================================================================
RCS file: /cvs/src/src/gdb/breakpoint.c,v
retrieving revision 1.183
diff -c -p -r1.183 breakpoint.c
*** gdb/breakpoint.c 8 Oct 2004 17:30:46 -0000 1.183
--- gdb/breakpoint.c 15 Oct 2004 13:24:26 -0000
*************** struct breakpoint *
*** 4061,4073 ****
set_raw_breakpoint (struct symtab_and_line sal, enum bptype bptype)
{
struct breakpoint *b, *b1;
!
b = (struct breakpoint *) xmalloc (sizeof (struct breakpoint));
memset (b, 0, sizeof (*b));
b->loc = allocate_bp_location (b, bptype);
! b->loc->requested_address = sal.pc;
b->loc->address = adjust_breakpoint_address (b->loc->requested_address,
! bptype);
if (sal.symtab == NULL)
b->source_file = NULL;
else
--- 4061,4075 ----
set_raw_breakpoint (struct symtab_and_line sal, enum bptype bptype)
{
struct breakpoint *b, *b1;
! CORE_ADDR pc;
! pc = sal.pc;
!
b = (struct breakpoint *) xmalloc (sizeof (struct breakpoint));
memset (b, 0, sizeof (*b));
b->loc = allocate_bp_location (b, bptype);
! b->loc->requested_address = pc;
b->loc->address = adjust_breakpoint_address (b->loc->requested_address,
! bptype);
if (sal.symtab == NULL)
b->source_file = NULL;
else
*************** set_raw_breakpoint (struct symtab_and_li
*** 4105,4111 ****
b1 = b1->next;
b1->next = b;
}
!
check_duplicates (b);
breakpoints_changed ();
--- 4107,4113 ----
b1 = b1->next;
b1->next = b;
}
!
check_duplicates (b);
breakpoints_changed ();
*************** create_breakpoints (struct symtabs_and_l
*** 4927,4978 ****
int i;
for (i = 0; i < sals.nelts; i++)
{
struct breakpoint *b;
struct symtab_and_line sal = sals.sals[i];
!
if (from_tty)
describe_other_breakpoints (sal.pc, sal.section);
- b = set_raw_breakpoint (sal, type);
- set_breakpoint_count (breakpoint_count + 1);
- b->number = breakpoint_count;
- b->cond = cond[i];
- b->thread = thread;
- if (addr_string[i])
- b->addr_string = addr_string[i];
- else
- /* addr_string has to be used or breakpoint_re_set will delete
- me. */
- b->addr_string = xstrprintf ("*0x%s", paddr (b->loc->address));
- b->cond_string = cond_string[i];
- b->ignore_count = ignore_count;
- b->enable_state = bp_enabled;
- b->disposition = disposition;
- /* If resolving a pending breakpoint, a check must be made to see if
- the user has specified a new condition or commands for the
- breakpoint. A new condition will override any condition that was
- initially specified with the initial breakpoint command. */
- if (pending_bp)
- {
- char *arg;
- if (pending_bp->cond_string)
- {
- arg = pending_bp->cond_string;
- b->cond_string = savestring (arg, strlen (arg));
- b->cond = parse_exp_1 (&arg, block_for_pc (b->loc->address), 0);
- if (*arg)
- error ("Junk at end of pending breakpoint condition expression");
- }
- /* If there are commands associated with the breakpoint, they should
- be copied too. */
- if (pending_bp->commands)
- b->commands = copy_command_lines (pending_bp->commands);
-
- /* We have to copy over the ignore_count and thread as well. */
- b->ignore_count = pending_bp->ignore_count;
- b->thread = pending_bp->thread;
- }
- mention (b);
}
}
}
--- 4929,4995 ----
int i;
for (i = 0; i < sals.nelts; i++)
{
+ char *addr_str;
+ struct linetable_entry *le;
struct breakpoint *b;
struct symtab_and_line sal = sals.sals[i];
! le = sals.sals[i].entry;
if (from_tty)
describe_other_breakpoints (sal.pc, sal.section);
+
+ addr_str = addr_string[i];
+ do {
+ if (le)
+ sal.pc = le->pc;
+
+ b = set_raw_breakpoint (sal, type);
+
+ set_breakpoint_count (breakpoint_count + 1);
+ b->number = breakpoint_count;
+ b->cond = cond[i];
+ b->thread = thread;
+ if (addr_str)
+ {
+ b->addr_string = addr_str;
+ addr_str = NULL;
+ }
+ else
+ /* addr_string has to be used or breakpoint_re_set will delete
+ me. */
+ b->addr_string = xstrprintf ("*0x%s", paddr (b->loc->address));
+ b->cond_string = cond_string[i];
+ b->ignore_count = ignore_count;
+ b->enable_state = bp_enabled;
+ b->disposition = disposition;
+ /* If resolving a pending breakpoint, a check must be made to see if
+ the user has specified a new condition or commands for the
+ breakpoint. A new condition will override any condition that was
+ initially specified with the initial breakpoint command. */
+ if (pending_bp)
+ {
+ char *arg;
+ if (pending_bp->cond_string)
+ {
+ arg = pending_bp->cond_string;
+ b->cond_string = savestring (arg, strlen (arg));
+ b->cond = parse_exp_1 (&arg, block_for_pc (b->loc->address), 0);
+ if (*arg)
+ error ("Junk at end of pending breakpoint condition expression");
+ }
+ /* If there are commands associated with the breakpoint, they should
+ be copied too. */
+ if (pending_bp->commands)
+ b->commands = copy_command_lines (pending_bp->commands);
+
+ /* We have to copy over the ignore_count and thread as well. */
+ b->ignore_count = pending_bp->ignore_count;
+ b->thread = pending_bp->thread;
+ }
+ mention (b);
+ if (le) le = le->next;
+ } while (le);
+
}
}
}
*************** break_at_finish_command_1 (char *arg, in
*** 5617,5630 ****
void
resolve_sal_pc (struct symtab_and_line *sal)
{
- CORE_ADDR pc;
-
if (sal->pc == 0 && sal->symtab != NULL)
{
! if (!find_line_pc (sal->symtab, sal->line, &pc))
error ("No line %d in file \"%s\".",
sal->line, sal->symtab->filename);
! sal->pc = pc;
}
if (sal->section == 0 && sal->symtab != NULL)
--- 5634,5645 ----
void
resolve_sal_pc (struct symtab_and_line *sal)
{
if (sal->pc == 0 && sal->symtab != NULL)
{
! if (!find_line_pc (sal))
error ("No line %d in file \"%s\".",
sal->line, sal->symtab->filename);
!
}
if (sal->section == 0 && sal->symtab != NULL)
Index: gdb/buildsym.c
===================================================================
RCS file: /cvs/src/src/gdb/buildsym.c,v
retrieving revision 1.40
diff -c -p -r1.40 buildsym.c
*** gdb/buildsym.c 11 Sep 2004 10:24:45 -0000 1.40
--- gdb/buildsym.c 15 Oct 2004 13:24:26 -0000
*************** record_line (struct subfile *subfile, in
*** 733,738 ****
--- 733,739 ----
e = subfile->line_vector->item + subfile->line_vector->nitems++;
e->line = line;
e->pc = ADDR_BITS_REMOVE(pc);
+ e->next = 0;
}
/* Needed in order to sort line tables from IBM xcoff files. Sigh! */
Index: gdb/symtab.c
===================================================================
RCS file: /cvs/src/src/gdb/symtab.c,v
retrieving revision 1.140
diff -c -p -r1.140 symtab.c
*** gdb/symtab.c 2 Oct 2004 09:55:15 -0000 1.140
--- gdb/symtab.c 15 Oct 2004 13:24:28 -0000
*************** init_sal (struct symtab_and_line *sal)
*** 700,705 ****
--- 700,706 ----
sal->line = 0;
sal->pc = 0;
sal->end = 0;
+ sal->entry = 0;
}
*************** done:
*** 2307,2326 ****
The source file is specified with a struct symtab. */
int
! find_line_pc (struct symtab *symtab, int line, CORE_ADDR *pc)
{
struct linetable *l;
int ind;
!
! *pc = 0;
if (symtab == 0)
return 0;
! symtab = find_line_symtab (symtab, line, &ind, NULL);
if (symtab != NULL)
{
l = LINETABLE (symtab);
! *pc = l->item[ind].pc;
return 1;
}
else
--- 2308,2329 ----
The source file is specified with a struct symtab. */
int
! find_line_pc (struct symtab_and_line *sal)
{
struct linetable *l;
int ind;
! struct symtab *symtab;
! symtab = sal->symtab;
! sal->pc = 0;
if (symtab == 0)
return 0;
! symtab = find_line_symtab (sal->symtab, sal->line, &ind, NULL);
if (symtab != NULL)
{
l = LINETABLE (symtab);
! sal->pc = l->item[ind].pc;
! sal->entry = &(l->item[ind]);
return 1;
}
else
*************** find_line_pc_range (struct symtab_and_li
*** 2341,2348 ****
struct symtab_and_line found_sal;
startaddr = sal.pc;
! if (startaddr == 0 && !find_line_pc (sal.symtab, sal.line, &startaddr))
! return 0;
/* This whole function is based on address. For example, if line 10 has
two parts, one from 0x100 to 0x200 and one from 0x300 to 0x400, then
--- 2344,2352 ----
struct symtab_and_line found_sal;
startaddr = sal.pc;
! if (startaddr == 0 && !find_line_pc (&sal))
! return 0;
! startaddr = sal.pc;
/* This whole function is based on address. For example, if line 10 has
two parts, one from 0x100 to 0x200 and one from 0x300 to 0x400, then
*************** find_line_common (struct linetable *l, i
*** 2391,2396 ****
--- 2395,2402 ----
if (l == 0)
return -1;
+ *exact_match = 0;
+
len = l->nitems;
for (i = 0; i < len; i++)
{
*************** find_line_common (struct linetable *l, i
*** 2398,2418 ****
if (item->line == lineno)
{
! /* Return the first (lowest address) entry which matches. */
! *exact_match = 1;
! return i;
}
! if (item->line > lineno && (best == 0 || item->line < best))
{
! best = item->line;
! best_index = i;
}
}
-
- /* If we got here, we didn't get an exact match. */
-
- *exact_match = 0;
return best_index;
}
--- 2404,2451 ----
if (item->line == lineno)
{
! if (*exact_match)
! {
! /* create a chain of matching lines: for inlined and
! default arg constructors often line number has several
! PCs.
! */
! l->item[i].next = l->item[best_index].next;
! l->item[best_index].next = &(l->item[i]);
! }
! else
! {
! /* Return the first (lowest address) entry which matches. */
! *exact_match = 1;
! best_index = i;
! best = lineno;
! if (l->item[i].next)
! {
! /* have already found this line and other matches */
! return i;
! }
! }
}
! if (!(*exact_match)
! && item->line > lineno && (best == 0 || item->line <= best))
{
! if (best == item->line)
! {
! if (!l->item[best_index].next)
! {
! /* two matching 'best' lines */
! l->item[i].next = l->item[best_index].next;
! l->item[best_index].next = &(l->item[i]);
! }
! }
! else
! {
! best = item->line;
! best_index = i;
! }
}
}
return best_index;
}
Index: gdb/symtab.h
===================================================================
RCS file: /cvs/src/src/gdb/symtab.h,v
retrieving revision 1.92
diff -c -p -r1.92 symtab.h
*** gdb/symtab.h 10 Jun 2004 20:05:44 -0000 1.92
--- gdb/symtab.h 15 Oct 2004 13:24:28 -0000
*************** struct linetable_entry
*** 708,713 ****
--- 708,714 ----
{
int line;
CORE_ADDR pc;
+ struct linetable_entry *next;
};
/* The order of entries in the linetable is significant. They should
*************** struct symtab_and_line
*** 1198,1203 ****
--- 1199,1206 ----
CORE_ADDR pc;
CORE_ADDR end;
+
+ struct linetable_entry *entry;
};
extern void init_sal (struct symtab_and_line *sal);
*************** extern struct symtab_and_line find_pc_se
*** 1256,1262 ****
/* 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_range (struct symtab_and_line, CORE_ADDR *,
CORE_ADDR *);
--- 1259,1265 ----
/* Given a symtab and line number, return the pc there. */
! extern int find_line_pc (struct symtab_and_line *);
extern int find_line_pc_range (struct symtab_and_line, CORE_ADDR *,
CORE_ADDR *);
Index: gdb/mi/mi-cmd-disas.c
===================================================================
RCS file: /cvs/src/src/gdb/mi/mi-cmd-disas.c,v
retrieving revision 1.20
diff -c -p -r1.20 mi-cmd-disas.c
*** gdb/mi/mi-cmd-disas.c 30 Sep 2002 15:57:26 -0000 1.20
--- gdb/mi/mi-cmd-disas.c 15 Oct 2004 13:24:29 -0000
*************** mi_cmd_disassemble (char *command, char
*** 145,155 ****
if (line_seen && file_seen)
{
s = lookup_symtab (file_string);
if (s == NULL)
error ("mi_cmd_disassemble: Invalid filename.");
! if (!find_line_pc (s, line_num, &start))
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");
}
--- 145,162 ----
if (line_seen && file_seen)
{
+ struct symtab_and_line sal;
+ init_sal(&sal);
s = lookup_symtab (file_string);
if (s == NULL)
error ("mi_cmd_disassemble: Invalid filename.");
!
! sal.symtab = s;
! sal.line = line_num;
!
! if (!find_line_pc (&sal))
error ("mi_cmd_disassemble: Invalid line number");
+ start = sal.pc;
if (find_pc_partial_function (start, NULL, &low, &high) == 0)
error ("mi_cmd_disassemble: No function contains specified address");
}
Index: gdb/tui/tui-layout.c
===================================================================
RCS file: /cvs/src/src/gdb/tui/tui-layout.c,v
retrieving revision 1.18
diff -c -p -r1.18 tui-layout.c
*** gdb/tui/tui-layout.c 13 Mar 2004 14:14:01 -0000 1.18
--- gdb/tui/tui-layout.c 15 Oct 2004 13:24:30 -0000
*************** tui_set_layout_for_display_command (cons
*** 509,514 ****
--- 509,515 ----
static CORE_ADDR
extract_display_start_addr (void)
{
+ struct symtab_and_line s;
enum tui_layout_type cur_layout = tui_current_layout ();
CORE_ADDR addr;
CORE_ADDR pc;
*************** extract_display_start_addr (void)
*** 518,527 ****
{
case SRC_COMMAND:
case SRC_DATA_COMMAND:
! find_line_pc (cursal.symtab,
! TUI_SRC_WIN->detail.source_info.start_line_or_addr.line_no,
! &pc);
! addr = pc;
break;
case DISASSEM_COMMAND:
case SRC_DISASSEM_COMMAND:
--- 519,530 ----
{
case SRC_COMMAND:
case SRC_DATA_COMMAND:
!
! init_sal(&s);
! s.symtab = cursal.symtab;
! s.line = TUI_SRC_WIN->detail.source_info.start_line_or_addr.line_no;
! find_line_pc (&s);
! addr = s.pc;
break;
case DISASSEM_COMMAND:
case SRC_DISASSEM_COMMAND:
Index: gdb/tui/tui-win.c
===================================================================
RCS file: /cvs/src/src/gdb/tui/tui-win.c,v
retrieving revision 1.20
diff -c -p -r1.20 tui-win.c
*** gdb/tui/tui-win.c 26 Jul 2004 14:53:06 -0000 1.20
--- gdb/tui/tui-win.c 15 Oct 2004 13:24:30 -0000
*************** make_visible_with_new_height (struct tui
*** 1313,1319 ****
line.line_no = cursal.line;
else
{
! find_line_pc (s, cursal.line, &line.addr);
}
tui_update_source_window (win_info, s, line, TRUE);
}
--- 1313,1325 ----
line.line_no = cursal.line;
else
{
!
! struct symtab_and_line sal;
! init_sal(&sal);
! sal.symtab = s;
! sal.line = cursal.line;
! find_line_pc (&sal);
! line.addr = sal.pc;
}
tui_update_source_window (win_info, s, line, TRUE);
}
Index: gdb/tui/tui-winsource.c
===================================================================
RCS file: /cvs/src/src/gdb/tui/tui-winsource.c,v
retrieving revision 1.15
diff -c -p -r1.15 tui-winsource.c
*** gdb/tui/tui-winsource.c 16 Feb 2004 21:05:09 -0000 1.15
--- gdb/tui/tui-winsource.c 15 Oct 2004 13:24:30 -0000
*************** void
*** 172,184 ****
tui_update_source_windows_with_line (struct symtab *s, int line)
{
CORE_ADDR pc;
union tui_line_or_address l;
!
switch (tui_current_layout ())
{
case DISASSEM_COMMAND:
case DISASSEM_DATA_COMMAND:
! find_line_pc (s, line, &pc);
tui_update_source_windows_with_addr (pc);
break;
default:
--- 172,191 ----
tui_update_source_windows_with_line (struct symtab *s, int line)
{
CORE_ADDR pc;
+
+ struct symtab_and_line sal;
+
union tui_line_or_address l;
!
! init_sal(&sal);
switch (tui_current_layout ())
{
case DISASSEM_COMMAND:
case DISASSEM_DATA_COMMAND:
! sal.symtab = s;
! sal.line = line;
! find_line_pc (&sal);
! pc = sal.pc;
tui_update_source_windows_with_addr (pc);
break;
default:
*************** tui_update_source_windows_with_line (str
*** 186,192 ****
tui_show_symtab_source (s, l, FALSE);
if (tui_current_layout () == SRC_DISASSEM_COMMAND)
{
! find_line_pc (s, line, &pc);
tui_show_disassem (pc);
}
break;
--- 193,202 ----
tui_show_symtab_source (s, l, FALSE);
if (tui_current_layout () == SRC_DISASSEM_COMMAND)
{
! sal.symtab = s;
! sal.line = line;
! find_line_pc (&sal);
! pc = sal.pc;
tui_show_disassem (pc);
}
break;