From: Tom Tromey Date: Thu, 16 Mar 2023 20:41:31 +0000 (-0600) Subject: Fix line table regression X-Git-Url: https://git.libre-soc.org/?a=commitdiff_plain;h=48e0f38c30a153855e1adc9dc76614f3f88d686a;p=binutils-gdb.git 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. --- 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; } 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 }; +/* 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; } }