Fix line table regression

Simon pointed out a line table regression, and after a couple of false
starts, I was able to reproduce it by hand using his instructions.

The bug is that most of the code in do_mixed_source_and_assembly uses
unrelocated addresses, but one spot does:

  pc = low;

... after the text offset has been removed.

This patch fixes the problem by introducing a new type to represent
unrelocated addresses in the line table.  This prevents this sort of
bug to some degree (it's still possible to manipulate a CORE_ADDR in a
bad way, this is unavoidable).

However, this did let the compiler flag a few spots in that function,
and now it's not possible to compare an unrelocated address from a
line table with an ordinary CORE_ADDR.

Regression tested on x86-64 Fedora 36, though note this setup never
reproduced the bug in the first place.  I also tested it by hand on
the disasm-optim test program.
This commit is contained in:
Tom Tromey 2023-03-16 14:41:31 -06:00
parent 152d9c48a2
commit 48e0f38c30
15 changed files with 78 additions and 50 deletions

View file

@ -205,7 +205,7 @@ finish_block (struct symbol *symbol, struct pending_block *old_blocks,
}
void
record_line (struct subfile *subfile, int line, CORE_ADDR pc)
record_line (struct subfile *subfile, int line, unrelocated_addr pc)
{
gdb_assert (buildsym_compunit != nullptr);
/* Assume every line entry is a statement start, that is a good place to

View file

@ -76,7 +76,8 @@ extern struct context_stack *push_context (int desc, CORE_ADDR valu);
extern struct context_stack pop_context ();
extern void record_line (struct subfile *subfile, int line, CORE_ADDR pc);
extern void record_line (struct subfile *subfile, int line,
unrelocated_addr pc);
extern struct compunit_symtab *start_compunit_symtab (struct objfile *objfile,
const char *name,

View file

@ -629,7 +629,7 @@ buildsym_compunit::pop_subfile ()
void
buildsym_compunit::record_line (struct subfile *subfile, int line,
CORE_ADDR pc, linetable_entry_flags flags)
unrelocated_addr pc, linetable_entry_flags flags)
{
m_have_line_numbers = true;

View file

@ -239,7 +239,7 @@ struct buildsym_compunit
const char *pop_subfile ();
void record_line (struct subfile *subfile, int line, CORE_ADDR pc,
void record_line (struct subfile *subfile, int line, unrelocated_addr pc,
linetable_entry_flags flags);
struct compunit_symtab *get_compunit_symtab ()

View file

@ -1130,9 +1130,10 @@ coff_symtab_read (minimal_symbol_reader &reader,
of the closing '}', and for which we do not have any
other statement-line-number. */
if (fcn_last_line == 1)
record_line (get_current_subfile (), fcn_first_line,
gdbarch_addr_bits_remove (gdbarch,
fcn_first_line_addr));
record_line
(get_current_subfile (), fcn_first_line,
unrelocated_addr (gdbarch_addr_bits_remove (gdbarch,
fcn_first_line_addr)));
else
enter_linenos (fcn_line_ptr, fcn_first_line,
fcn_last_line, objfile);
@ -1460,7 +1461,8 @@ enter_linenos (file_ptr file_offset, int first_line,
CORE_ADDR addr = lptr.l_addr.l_paddr;
record_line (get_current_subfile (),
first_line + L_LNNO32 (&lptr),
gdbarch_addr_bits_remove (gdbarch, addr));
unrelocated_addr (gdbarch_addr_bits_remove (gdbarch,
addr)));
}
else
break;

View file

@ -2453,9 +2453,10 @@ process_one_symbol (int type, int desc, CORE_ADDR valu, const char *name,
{
CORE_ADDR addr = last_function_start + valu;
record_line (get_current_subfile (), 0,
gdbarch_addr_bits_remove (gdbarch, addr)
- objfile->text_section_offset ());
record_line
(get_current_subfile (), 0,
unrelocated_addr (gdbarch_addr_bits_remove (gdbarch, addr)
- objfile->text_section_offset ()));
}
within_function = 0;
@ -2662,15 +2663,17 @@ process_one_symbol (int type, int desc, CORE_ADDR valu, const char *name,
CORE_ADDR addr = processing_gcc_compilation == 2 ?
last_function_start : valu;
record_line (get_current_subfile (), desc,
gdbarch_addr_bits_remove (gdbarch, addr)
- objfile->text_section_offset ());
record_line
(get_current_subfile (), desc,
unrelocated_addr (gdbarch_addr_bits_remove (gdbarch, addr)
- objfile->text_section_offset ()));
sline_found_in_function = 1;
}
else
record_line (get_current_subfile (), desc,
gdbarch_addr_bits_remove (gdbarch, valu)
- objfile->text_section_offset ());
record_line
(get_current_subfile (), desc,
unrelocated_addr (gdbarch_addr_bits_remove (gdbarch, valu)
- objfile->text_section_offset ()));
break;
case N_BCOMM:

View file

@ -594,8 +594,11 @@ do_mixed_source_and_assembly_deprecated
alloca (nlines * sizeof (struct deprecated_dis_line_entry));
struct objfile *objfile = symtab->compunit ()->objfile ();
low -= objfile->text_section_offset ();
high -= objfile->text_section_offset ();
unrelocated_addr unrel_low
= unrelocated_addr (low - objfile->text_section_offset ());
unrelocated_addr unrel_high
= unrelocated_addr (high - objfile->text_section_offset ());
/* Copy linetable entries for this function into our data
structure, creating end_pc's and setting out_of_order as
@ -603,11 +606,11 @@ do_mixed_source_and_assembly_deprecated
/* First, skip all the preceding functions. */
for (i = 0; i < nlines - 1 && le[i].raw_pc () < low; i++);
for (i = 0; i < nlines - 1 && le[i].raw_pc () < unrel_low; i++);
/* Now, copy all entries before the end of this function. */
for (; i < nlines - 1 && le[i].raw_pc () < high; i++)
for (; i < nlines - 1 && le[i].raw_pc () < unrel_high; i++)
{
if (le[i] == le[i + 1])
continue; /* Ignore duplicates. */
@ -627,7 +630,7 @@ do_mixed_source_and_assembly_deprecated
/* If we're on the last line, and it's part of the function,
then we need to get the end pc in a special way. */
if (i == nlines - 1 && le[i].raw_pc () < high)
if (i == nlines - 1 && le[i].raw_pc () < unrel_high)
{
mle[newlines].line = le[i].line;
mle[newlines].start_pc = le[i].pc (objfile);
@ -739,8 +742,11 @@ do_mixed_source_and_assembly (struct gdbarch *gdbarch,
htab_up dis_line_table (allocate_dis_line_table ());
struct objfile *objfile = main_symtab->compunit ()->objfile ();
low -= objfile->text_section_offset ();
high -= objfile->text_section_offset ();
unrelocated_addr unrel_low
= unrelocated_addr (low - objfile->text_section_offset ());
unrelocated_addr unrel_high
= unrelocated_addr (high - objfile->text_section_offset ());
pc = low;
@ -755,10 +761,10 @@ do_mixed_source_and_assembly (struct gdbarch *gdbarch,
first_le = NULL;
/* Skip all the preceding functions. */
for (i = 0; i < nlines && le[i].raw_pc () < low; i++)
for (i = 0; i < nlines && le[i].raw_pc () < unrel_low; i++)
continue;
if (i < nlines && le[i].raw_pc () < high)
if (i < nlines && le[i].raw_pc () < unrel_high)
first_le = &le[i];
/* Add lines for every pc value. */

View file

@ -18209,7 +18209,8 @@ dwarf_record_line_1 (struct gdbarch *gdbarch, struct subfile *subfile,
linetable_entry_flags flags,
struct dwarf2_cu *cu)
{
CORE_ADDR addr = gdbarch_addr_bits_remove (gdbarch, address);
unrelocated_addr addr
= unrelocated_addr (gdbarch_addr_bits_remove (gdbarch, address));
if (dwarf_line_debug)
{

View file

@ -491,7 +491,7 @@ jit_symtab_line_mapping_add_impl (struct gdb_symbol_callbacks *cb,
stab->linetable->nitems = nlines;
for (i = 0; i < nlines; i++)
{
stab->linetable->item[i].set_raw_pc ((CORE_ADDR) map[i].pc);
stab->linetable->item[i].set_raw_pc (unrelocated_addr (map[i].pc));
stab->linetable->item[i].line = map[i].line;
stab->linetable->item[i].is_stmt = true;
}

View file

@ -3982,8 +3982,10 @@ mdebug_expand_psymtab (legacy_psymtab *pst, struct objfile *objfile)
else
{
/* Handle encoded stab line number. */
record_line (get_current_subfile (), sh.index,
gdbarch_addr_bits_remove (gdbarch, valu));
record_line
(get_current_subfile (), sh.index,
unrelocated_addr (gdbarch_addr_bits_remove (gdbarch,
valu)));
}
}
else if (sh.st == stProc || sh.st == stStaticProc
@ -4513,7 +4515,7 @@ add_line (struct linetable *lt, int lineno, CORE_ADDR adr, int last)
return lineno;
lt->item[lt->nitems].line = lineno;
lt->item[lt->nitems++].set_raw_pc (adr << 2);
lt->item[lt->nitems++].set_raw_pc (unrelocated_addr (adr << 2));
return lineno;
}

View file

@ -724,7 +724,8 @@ btrace_find_line_range (CORE_ADDR pc)
return btrace_mk_line_range (symtab, 0, 0);
struct objfile *objfile = symtab->compunit ()->objfile ();
pc -= objfile->text_section_offset ();
unrelocated_addr unrel_pc
= unrelocated_addr (pc - objfile->text_section_offset ());
range = btrace_mk_line_range (symtab, 0, 0);
for (i = 0; i < nlines - 1; i++)
@ -737,7 +738,7 @@ btrace_find_line_range (CORE_ADDR pc)
possibly adding more line numbers to the range. At the time this
change was made I was unsure how to test this so chose to go with
maintaining the existing experience. */
if ((lines[i].raw_pc () == pc) && (lines[i].line != 0)
if (lines[i].raw_pc () == unrel_pc && lines[i].line != 0
&& lines[i].is_stmt)
range = btrace_line_range_add (range, lines[i].line);
}

View file

@ -996,7 +996,7 @@ maintenance_print_one_line_table (struct symtab *symtab, void *data)
else
uiout->field_string ("line", _("END"));
uiout->field_core_addr ("address", objfile->arch (),
item->raw_pc ());
CORE_ADDR (item->raw_pc ()));
uiout->field_string ("is-stmt", item->is_stmt ? "Y" : "");
uiout->field_string ("prologue-end", item->prologue_end ? "Y" : "");
uiout->text ("\n");

View file

@ -332,7 +332,7 @@ search_domain_name (enum search_domain e)
CORE_ADDR
linetable_entry::pc (const struct objfile *objfile) const
{
return m_pc + objfile->text_section_offset ();
return CORE_ADDR (m_pc) + objfile->text_section_offset ();
}
/* See symtab.h. */
@ -3158,17 +3158,18 @@ find_pc_sect_line (CORE_ADDR pc, struct obj_section *section, int notcurrent)
&& (!alt || item->raw_pc () < alt->raw_pc ()))
alt = item;
auto pc_compare = [](const CORE_ADDR & comp_pc,
const struct linetable_entry & lhs)->bool
auto pc_compare = [] (const unrelocated_addr &comp_pc,
const struct linetable_entry & lhs)
{
return comp_pc < lhs.raw_pc ();
};
const linetable_entry *first = item;
const linetable_entry *last = item + len;
item = std::upper_bound (first, last,
pc - objfile->text_section_offset (),
pc_compare);
item = (std::upper_bound
(first, last,
unrelocated_addr (pc - objfile->text_section_offset ()),
pc_compare));
if (item != first)
prev = item - 1; /* Found a matching item. */
@ -3689,18 +3690,22 @@ skip_prologue_using_linetable (CORE_ADDR func_addr)
const linetable *linetable = prologue_sal.symtab->linetable ();
struct objfile *objfile = prologue_sal.symtab->compunit ()->objfile ();
start_pc -= objfile->text_section_offset ();
end_pc -= objfile->text_section_offset ();
unrelocated_addr unrel_start
= unrelocated_addr (start_pc - objfile->text_section_offset ());
unrelocated_addr unrel_end
= unrelocated_addr (end_pc - objfile->text_section_offset ());
auto it = std::lower_bound
(linetable->item, linetable->item + linetable->nitems, start_pc,
[] (const linetable_entry &lte, CORE_ADDR pc) -> bool
(linetable->item, linetable->item + linetable->nitems, unrel_start,
[] (const linetable_entry &lte, unrelocated_addr pc)
{
return lte.raw_pc () < pc;
});
for (;
it < linetable->item + linetable->nitems && it->raw_pc () <= end_pc;
(it < linetable->item + linetable->nitems
&& it->raw_pc () <= unrel_end);
it++)
if (it->prologue_end)
return {it->pc (objfile)};

View file

@ -1540,6 +1540,12 @@ struct rust_vtable_symbol : public symbol
};
/* Like a CORE_ADDR, but not directly convertible. This is used to
represent an unrelocated CORE_ADDR. DEFINE_OFFSET_TYPE is not used
here because there's no need to add or subtract values of this
type. */
enum class unrelocated_addr : CORE_ADDR { };
/* Each item represents a line-->pc (or the reverse) mapping. This is
somewhat more wasteful of space than one might wish, but since only
the files which are actually debugged are read in to core, we don't
@ -1548,11 +1554,11 @@ struct rust_vtable_symbol : public symbol
struct linetable_entry
{
/* Set the (unrelocated) PC for this entry. */
void set_raw_pc (CORE_ADDR pc)
void set_raw_pc (unrelocated_addr pc)
{ m_pc = pc; }
/* Return the unrelocated PC for this entry. */
CORE_ADDR raw_pc () const
unrelocated_addr raw_pc () const
{ return m_pc; }
/* Return the relocated PC for this entry. */
@ -1582,7 +1588,7 @@ struct linetable_entry
bool prologue_end : 1;
/* The address for this entry. */
CORE_ADDR m_pc;
unrelocated_addr m_pc;
};
/* The order of entries in the linetable is significant. They should

View file

@ -795,11 +795,12 @@ enter_line_range (struct subfile *subfile, unsigned beginoffset,
if (int_lnno.l_lnno == 0)
{
*firstLine = read_symbol_lineno (int_lnno.l_addr.l_symndx);
record_line (subfile, 0, record_addr);
record_line (subfile, 0, unrelocated_addr (record_addr));
--(*firstLine);
}
else
record_line (subfile, *firstLine + int_lnno.l_lnno, record_addr);
record_line (subfile, *firstLine + int_lnno.l_lnno,
unrelocated_addr (record_addr));
curoffset += linesz;
}
}