From: Tom Tromey <tom@tromey.com>
To: gdb-patches@sourceware.org
Cc: Tom Tromey <tom@tromey.com>
Subject: [PATCH] Fix line table regression
Date: Fri, 17 Mar 2023 07:14:37 -0600 [thread overview]
Message-ID: <20230317131437.1852342-1-tom@tromey.com> (raw)
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.
---
gdb/buildsym-legacy.c | 2 +-
gdb/buildsym-legacy.h | 3 ++-
gdb/buildsym.c | 2 +-
gdb/buildsym.h | 2 +-
gdb/coffread.c | 10 ++++++----
gdb/dbxread.c | 21 ++++++++++++---------
gdb/disasm.c | 24 +++++++++++++++---------
gdb/dwarf2/read.c | 3 ++-
gdb/jit.c | 2 +-
gdb/mdebugread.c | 8 +++++---
gdb/record-btrace.c | 5 +++--
gdb/symmisc.c | 2 +-
gdb/symtab.c | 27 ++++++++++++++++-----------
gdb/symtab.h | 12 +++++++++---
gdb/xcoffread.c | 5 +++--
15 files changed, 78 insertions(+), 50 deletions(-)
diff --git a/gdb/buildsym-legacy.c b/gdb/buildsym-legacy.c
index cb733e7101b..131d24fe9b5 100644
--- a/gdb/buildsym-legacy.c
+++ b/gdb/buildsym-legacy.c
@@ -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
diff --git a/gdb/buildsym-legacy.h b/gdb/buildsym-legacy.h
index 3d705a8beed..664d6320e54 100644
--- a/gdb/buildsym-legacy.h
+++ b/gdb/buildsym-legacy.h
@@ -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,
diff --git a/gdb/buildsym.c b/gdb/buildsym.c
index 56f11dd167f..f000233dafa 100644
--- a/gdb/buildsym.c
+++ b/gdb/buildsym.c
@@ -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;
diff --git a/gdb/buildsym.h b/gdb/buildsym.h
index 42fcd1fdb97..98dc8f02874 100644
--- a/gdb/buildsym.h
+++ b/gdb/buildsym.h
@@ -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 ()
diff --git a/gdb/coffread.c b/gdb/coffread.c
index 4da3799243b..fe1ee506fbe 100644
--- a/gdb/coffread.c
+++ b/gdb/coffread.c
@@ -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;
diff --git a/gdb/dbxread.c b/gdb/dbxread.c
index 1e88121f11c..e5366ccd0d0 100644
--- a/gdb/dbxread.c
+++ b/gdb/dbxread.c
@@ -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:
diff --git a/gdb/disasm.c b/gdb/disasm.c
index 71d3b97258d..03cd4b7ee02 100644
--- a/gdb/disasm.c
+++ b/gdb/disasm.c
@@ -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. */
diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index 138537cfefd..a1c75655ff8 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -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)
{
diff --git a/gdb/jit.c b/gdb/jit.c
index a93813b3f7f..52b65746f11 100644
--- a/gdb/jit.c
+++ b/gdb/jit.c
@@ -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;
}
diff --git a/gdb/mdebugread.c b/gdb/mdebugread.c
index 13dc7899d44..bc466096d61 100644
--- a/gdb/mdebugread.c
+++ b/gdb/mdebugread.c
@@ -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;
}
\f
diff --git a/gdb/record-btrace.c b/gdb/record-btrace.c
index 3e71c6c9a10..2d88e4d20bf 100644
--- a/gdb/record-btrace.c
+++ b/gdb/record-btrace.c
@@ -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);
}
diff --git a/gdb/symmisc.c b/gdb/symmisc.c
index 2f1e4f5b784..54dc570d282 100644
--- a/gdb/symmisc.c
+++ b/gdb/symmisc.c
@@ -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");
diff --git a/gdb/symtab.c b/gdb/symtab.c
index e11f9262a22..8ab1f58affe 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -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 <e, CORE_ADDR pc) -> bool
+ (linetable->item, linetable->item + linetable->nitems, unrel_start,
+ [] (const linetable_entry <e, 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)};
diff --git a/gdb/symtab.h b/gdb/symtab.h
index 005b64b5a08..2fd56ce21bd 100644
--- a/gdb/symtab.h
+++ b/gdb/symtab.h
@@ -1540,6 +1540,12 @@ struct rust_vtable_symbol : public symbol
};
\f
+/* 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
diff --git a/gdb/xcoffread.c b/gdb/xcoffread.c
index 72c0a0db49a..ab763760f43 100644
--- a/gdb/xcoffread.c
+++ b/gdb/xcoffread.c
@@ -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;
}
}
--
2.39.1
next reply other threads:[~2023-03-17 13:14 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-03-17 13:14 Tom Tromey [this message]
2023-03-17 15:21 ` Simon Marchi
2023-03-17 22:17 ` Tom Tromey
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20230317131437.1852342-1-tom@tromey.com \
--to=tom@tromey.com \
--cc=gdb-patches@sourceware.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).