* [RFA 0/2] Make line tables independent of progspace @ 2018-03-21 17:18 Tom Tromey 2018-03-21 17:18 ` [RFA 2/2] Constify the line table Tom Tromey 2018-03-21 17:18 ` [RFA 1/2] Make line tables independent of progspace Tom Tromey 0 siblings, 2 replies; 18+ messages in thread From: Tom Tromey @ 2018-03-21 17:18 UTC (permalink / raw) To: gdb-patches This is a series I originally sent in 2013 (I think) but didn't check in. I've rebased it and made a few changes. The first patch is the real change. Please read it carefully, and read the text as well -- the correct course here wasn't entirely clear to me, and perhaps something else should be done. Regression tested by the buildbot. Though, as is often the case, it's hard to be sure that everything relevant is really tested. Tom ^ permalink raw reply [flat|nested] 18+ messages in thread
* [RFA 2/2] Constify the line table 2018-03-21 17:18 [RFA 0/2] Make line tables independent of progspace Tom Tromey @ 2018-03-21 17:18 ` Tom Tromey 2018-03-21 17:18 ` [RFA 1/2] Make line tables independent of progspace Tom Tromey 1 sibling, 0 replies; 18+ messages in thread From: Tom Tromey @ 2018-03-21 17:18 UTC (permalink / raw) To: gdb-patches; +Cc: Tom Tromey From: Tom Tromey <tromey@redhat.com> Now that linetables aren't relocated, we can make them immutable in the symbol table. This patch applies "const" there and then fixes up the fallout. gdb/ChangeLog 2018-03-21 Tom Tromey <tom@tromey.com> * record-btrace.c (btrace_find_line_range): Constify. * buildsym.c (end_symtab_from_static_block): Update. * disasm.c (do_mixed_source_and_assembly, gdb_disassembly): Update. * jit.c (finalize_symtab): Update and rearrange for constification. * linespec.c (create_sals_line_offset): Update. (decode_digits_ordinary): Make "best_entry" const. * mdebugread.c (psymtab_to_symtab_1): Update. (new_symtab): Add "lines" parameter. * mi/mi-symbol-cmds.c (mi_cmd_symbol_list_lines): Update. * python/py-linetable.c (ltpy_get_pcs_for_line, ltpy_has_line) (ltpy_get_all_source_lines, ltpy_iternext): Update. * symmisc.c (dump_symtab_1, maintenance_print_one_line_table): Update. * symtab.c (find_pc_sect_line, find_line_symtab) (find_pcs_for_symtab_line, find_line_common) (skip_prologue_using_lineinfo, skip_prologue_using_sal): Update. * symtab.h (struct symtab) <linetable>: Now const. (find_pcs_for_symtab_line): Update. --- gdb/ChangeLog | 23 +++++++++++++++++++++++ gdb/buildsym.c | 6 +++--- gdb/disasm.c | 2 +- gdb/jit.c | 8 ++++---- gdb/linespec.c | 6 +++--- gdb/mdebugread.c | 22 +++++++++------------- gdb/mi/mi-symbol-cmds.c | 2 +- gdb/python/py-linetable.c | 9 +++++---- gdb/record-btrace.c | 4 ++-- gdb/symmisc.c | 6 +++--- gdb/symtab.c | 39 ++++++++++++++++++++------------------- gdb/symtab.h | 5 +++-- 12 files changed, 77 insertions(+), 55 deletions(-) diff --git a/gdb/buildsym.c b/gdb/buildsym.c index 99a92a8e31..a4945aa7aa 100644 --- a/gdb/buildsym.c +++ b/gdb/buildsym.c @@ -1373,10 +1373,10 @@ end_symtab_with_blockvector (struct block *static_block, if (subfile->line_vector) { /* Reallocate the line table on the symbol obstack. */ - SYMTAB_LINETABLE (symtab) = (struct linetable *) + linetable *new_table = (struct linetable *) obstack_alloc (&objfile->objfile_obstack, linetablesize); - memcpy (SYMTAB_LINETABLE (symtab), subfile->line_vector, - linetablesize); + memcpy (new_table, subfile->line_vector, linetablesize); + SYMTAB_LINETABLE (symtab) = new_table; } else { diff --git a/gdb/disasm.c b/gdb/disasm.c index ec5b8a9b81..3f9d6f8b55 100644 --- a/gdb/disasm.c +++ b/gdb/disasm.c @@ -343,7 +343,7 @@ do_mixed_source_and_assembly_deprecated { int newlines = 0; int nlines; - struct linetable_entry *le; + const struct linetable_entry *le; struct deprecated_dis_line_entry *mle; struct symtab_and_line sal; int i; diff --git a/gdb/jit.c b/gdb/jit.c index e5c64cc791..c68ee167a0 100644 --- a/gdb/jit.c +++ b/gdb/jit.c @@ -672,10 +672,10 @@ finalize_symtab (struct gdb_symtab *stab, struct objfile *objfile) size_t size = ((stab->linetable->nitems - 1) * sizeof (struct linetable_entry) + sizeof (struct linetable)); - SYMTAB_LINETABLE (COMPUNIT_FILETABS (cust)) - = (struct linetable *) obstack_alloc (&objfile->objfile_obstack, size); - memcpy (SYMTAB_LINETABLE (COMPUNIT_FILETABS (cust)), stab->linetable, - size); + struct linetable *new_table = + (struct linetable *) obstack_alloc (&objfile->objfile_obstack, size); + memcpy (new_table, stab->linetable, size); + SYMTAB_LINETABLE (COMPUNIT_FILETABS (cust)) = new_table; } blockvector_size = (sizeof (struct blockvector) diff --git a/gdb/linespec.c b/gdb/linespec.c index 1236b3f475..9f94f80de5 100644 --- a/gdb/linespec.c +++ b/gdb/linespec.c @@ -389,7 +389,7 @@ static std::vector<symtab_and_line> decode_digits_ordinary (struct linespec_state *self, linespec_p ls, int line, - linetable_entry **best_entry); + const linetable_entry **best_entry); static std::vector<symtab_and_line> decode_digits_list_mode (struct linespec_state *self, @@ -2192,7 +2192,7 @@ create_sals_line_offset (struct linespec_state *self, values = decode_digits_list_mode (self, ls, val); else { - struct linetable_entry *best_entry = NULL; + const struct linetable_entry *best_entry = NULL; int i, j; std::vector<symtab_and_line> intermediate_results @@ -4235,7 +4235,7 @@ static std::vector<symtab_and_line> decode_digits_ordinary (struct linespec_state *self, linespec_p ls, int line, - struct linetable_entry **best_entry) + const struct linetable_entry **best_entry) { int ix; struct symtab *elt; diff --git a/gdb/mdebugread.c b/gdb/mdebugread.c index c9b29bcc0f..cd53582baf 100644 --- a/gdb/mdebugread.c +++ b/gdb/mdebugread.c @@ -236,7 +236,8 @@ enum block_type { FUNCTION_BLOCK, NON_FUNCTION_BLOCK }; static struct block *new_block (enum block_type, enum language); -static struct compunit_symtab *new_symtab (const char *, int, struct objfile *); +static struct compunit_symtab *new_symtab (const char *, int, struct objfile *, + struct linetable **); static struct linetable *new_linetable (int); @@ -2177,9 +2178,6 @@ parse_external (EXTR *es, int bigend, struct section_offsets *section_offsets, numbers can go back and forth, apparently we can live with that and do not need to reorder our linetables. */ -static void parse_lines (FDR *, PDR *, struct linetable *, int, - struct partial_symtab *, CORE_ADDR); - static void parse_lines (FDR *fh, PDR *pr, struct linetable *lt, int maxlines, struct partial_symtab *pst, CORE_ADDR lowest_pdr_addr) @@ -3919,7 +3917,6 @@ psymtab_to_symtab_1 (struct objfile *objfile, int i; struct compunit_symtab *cust = NULL; FDR *fh; - struct linetable *lines; CORE_ADDR lowest_pdr_addr = 0; int last_symtab_ended = 0; struct section_offsets *section_offsets = objfile->section_offsets; @@ -4159,16 +4156,17 @@ psymtab_to_symtab_1 (struct objfile *objfile, int maxlines, size; EXTR *ext_ptr; + struct linetable *lines; if (fh == 0) { maxlines = 0; - cust = new_symtab ("unknown", 0, objfile); + cust = new_symtab ("unknown", 0, objfile, &lines); } else { maxlines = 2 * fh->cline; - cust = new_symtab (pst->filename, maxlines, objfile); + cust = new_symtab (pst->filename, maxlines, objfile, &lines); /* The proper language was already determined when building the psymtab, use it. */ @@ -4177,8 +4175,6 @@ psymtab_to_symtab_1 (struct objfile *objfile, psymtab_language = COMPUNIT_FILETABS (cust)->language; - lines = SYMTAB_LINETABLE (COMPUNIT_FILETABS (cust)); - /* Get a new lexical context. */ push_parse_stack (); @@ -4712,17 +4708,17 @@ sort_blocks (struct symtab *s) linenumbers MAXLINES we'll put in it. */ static struct compunit_symtab * -new_symtab (const char *name, int maxlines, struct objfile *objfile) +new_symtab (const char *name, int maxlines, struct objfile *objfile, + struct linetable **lines) { struct compunit_symtab *cust = allocate_compunit_symtab (objfile, name); - struct symtab *symtab; struct blockvector *bv; enum language lang; add_compunit_symtab_to_objfile (cust); - symtab = allocate_symtab (cust, name); + allocate_symtab (cust, name); - SYMTAB_LINETABLE (symtab) = new_linetable (maxlines); + *lines = new_linetable (maxlines); lang = compunit_language (cust); /* All symtabs must have at least two blocks. */ diff --git a/gdb/mi/mi-symbol-cmds.c b/gdb/mi/mi-symbol-cmds.c index 8f3b28e8f8..f42014a2ae 100644 --- a/gdb/mi/mi-symbol-cmds.c +++ b/gdb/mi/mi-symbol-cmds.c @@ -34,7 +34,7 @@ mi_cmd_symbol_list_lines (const char *command, char **argv, int argc) struct symtab *s; int i; struct ui_out *uiout = current_uiout; - struct linetable *linetable; + const struct linetable *linetable; if (argc != 1) error (_("-symbol-list-lines: Usage: SOURCE_FILENAME")); diff --git a/gdb/python/py-linetable.c b/gdb/python/py-linetable.c index 5f91804d60..aa4a5982e8 100644 --- a/gdb/python/py-linetable.c +++ b/gdb/python/py-linetable.c @@ -158,7 +158,7 @@ ltpy_get_pcs_for_line (PyObject *self, PyObject *args) { struct symtab *symtab; gdb_py_longest py_line; - struct linetable_entry *best_entry = NULL; + const struct linetable_entry *best_entry = NULL; std::vector<CORE_ADDR> pcs; LTPY_REQUIRE_VALID (self, symtab); @@ -204,7 +204,8 @@ ltpy_has_line (PyObject *self, PyObject *args) for (index = 0; index < SYMTAB_LINETABLE (symtab)->nitems; index++) { - struct linetable_entry *item = &(SYMTAB_LINETABLE (symtab)->item[index]); + const struct linetable_entry *item + = &(SYMTAB_LINETABLE (symtab)->item[index]); if (item->line == py_line) Py_RETURN_TRUE; } @@ -222,7 +223,7 @@ ltpy_get_all_source_lines (PyObject *self, PyObject *args) { struct symtab *symtab; Py_ssize_t index; - struct linetable_entry *item; + const struct linetable_entry *item; LTPY_REQUIRE_VALID (self, symtab); @@ -397,7 +398,7 @@ ltpy_iternext (PyObject *self) ltpy_iterator_object *iter_obj = (ltpy_iterator_object *) self; struct symtab *symtab; PyObject *obj; - struct linetable_entry *item; + const struct linetable_entry *item; LTPY_REQUIRE_VALID (iter_obj->source, symtab); diff --git a/gdb/record-btrace.c b/gdb/record-btrace.c index 0b447292f6..d2ecfea51a 100644 --- a/gdb/record-btrace.c +++ b/gdb/record-btrace.c @@ -576,8 +576,8 @@ static struct btrace_line_range btrace_find_line_range (CORE_ADDR pc) { struct btrace_line_range range; - struct linetable_entry *lines; - struct linetable *ltable; + const struct linetable_entry *lines; + const struct linetable *ltable; struct symtab *symtab; int nlines, i; diff --git a/gdb/symmisc.c b/gdb/symmisc.c index 89506ebfc0..794dc6b41a 100644 --- a/gdb/symmisc.c +++ b/gdb/symmisc.c @@ -276,7 +276,7 @@ dump_symtab_1 (struct symtab *symtab, struct ui_file *outfile) int i; struct dict_iterator iter; int len; - struct linetable *l; + const struct linetable *l; const struct blockvector *bv; struct symbol *sym; struct block *b; @@ -986,7 +986,7 @@ block_depth (struct block *block) static int maintenance_print_one_line_table (struct symtab *symtab, void *data) { - struct linetable *linetable; + const struct linetable *linetable; struct objfile *objfile; objfile = symtab->compunit_symtab->objfile; @@ -1017,7 +1017,7 @@ maintenance_print_one_line_table (struct symtab *symtab, void *data) for (i = 0; i < linetable->nitems; ++i) { - struct linetable_entry *item; + const struct linetable_entry *item; item = &linetable->item [i]; printf_filtered (_("%-6d %6d %s\n"), i, item->line, diff --git a/gdb/symtab.c b/gdb/symtab.c index f57ecafa42..8550f096a7 100644 --- a/gdb/symtab.c +++ b/gdb/symtab.c @@ -72,7 +72,7 @@ static void rbreak_command (const char *, int); -static int find_line_common (struct linetable *, int, int *, int); +static int find_line_common (const struct linetable *, int, int *, int); static struct block_symbol lookup_symbol_aux (const char *name, @@ -3051,16 +3051,16 @@ find_pc_sect_line (CORE_ADDR pc, struct obj_section *section, int notcurrent) { struct compunit_symtab *cust; struct symtab *iter_s; - struct linetable *l; + const struct linetable *l; int len; int i; - struct linetable_entry *item; + const struct linetable_entry *item; const struct blockvector *bv; struct bound_minimal_symbol msymbol; /* Info on best line seen so far, and where it starts, and its file. */ - struct linetable_entry *best = NULL; + const struct linetable_entry *best = NULL; CORE_ADDR best_end = 0; struct symtab *best_symtab = 0; @@ -3069,12 +3069,12 @@ find_pc_sect_line (CORE_ADDR pc, struct obj_section *section, int notcurrent) If we don't find a line whose range contains PC, we will use a line one less than this, with a range from the start of that file to the first line's pc. */ - struct linetable_entry *alt = NULL; + const struct linetable_entry *alt = NULL; struct symtab *alt_symtab = NULL; /* Info on best line seen in this file. */ - struct linetable_entry *prev; + const struct linetable_entry *prev; /* If this pc is not from the current frame, it is the address of the end of a call instruction. @@ -3224,8 +3224,8 @@ find_pc_sect_line (CORE_ADDR pc, struct obj_section *section, int notcurrent) return pc < LINETABLE_ENTRY_ADDRESS (iter_s, lhs); }; - struct linetable_entry *first = item; - struct linetable_entry *last = item + len; + const struct linetable_entry *first = item; + const struct linetable_entry *last = item + len; item = std::upper_bound (first, last, pc, pc_compare); if (item != first) prev = item - 1; /* Found a matching item. */ @@ -3342,7 +3342,7 @@ find_line_symtab (struct symtab *symtab, int line, so far seen. */ int best_index; - struct linetable *best_linetable; + const struct linetable *best_linetable; struct symtab *best_symtab; /* First try looking it up in the given symtab. */ @@ -3381,7 +3381,7 @@ find_line_symtab (struct symtab *symtab, int line, ALL_FILETABS (objfile, cu, s) { - struct linetable *l; + const struct linetable *l; int ind; if (FILENAME_CMP (symtab->filename, s->filename) != 0) @@ -3428,11 +3428,11 @@ done: std::vector<CORE_ADDR> find_pcs_for_symtab_line (struct symtab *symtab, int line, - struct linetable_entry **best_item) + const struct linetable_entry **best_item) { int start = 0; std::vector<CORE_ADDR> result; - struct linetable *linetable = SYMTAB_LINETABLE (symtab); + const struct linetable *linetable = SYMTAB_LINETABLE (symtab); /* First, collect all the PCs that are at this line. */ while (1) @@ -3446,7 +3446,7 @@ find_pcs_for_symtab_line (struct symtab *symtab, int line, if (!was_exact) { - struct linetable_entry *item = &linetable->item[idx]; + const struct linetable_entry *item = &linetable->item[idx]; if (*best_item == NULL || item->line < (*best_item)->line) *best_item = item; @@ -3470,7 +3470,7 @@ find_pcs_for_symtab_line (struct symtab *symtab, int line, int find_line_pc (struct symtab *symtab, int line, CORE_ADDR *pc) { - struct linetable *l; + const struct linetable *l; int ind; *pc = 0; @@ -3535,7 +3535,7 @@ find_line_pc_range (struct symtab_and_line sal, CORE_ADDR *startptr, Set *EXACT_MATCH nonzero if the value returned is an exact match. */ static int -find_line_common (struct linetable *l, int lineno, +find_line_common (const struct linetable *l, int lineno, int *exact_match, int start) { int i; @@ -3558,7 +3558,7 @@ find_line_common (struct linetable *l, int lineno, len = l->nitems; for (i = start; i < len; i++) { - struct linetable_entry *item = &(l->item[i]); + const struct linetable_entry *item = &(l->item[i]); if (item->line == lineno) { @@ -3646,7 +3646,7 @@ static CORE_ADDR skip_prologue_using_lineinfo (CORE_ADDR func_addr, struct symtab *symtab) { CORE_ADDR func_start, func_end; - struct linetable *l; + const struct linetable *l; int i; /* Give up if this symbol has no lineinfo table. */ @@ -3665,7 +3665,7 @@ skip_prologue_using_lineinfo (CORE_ADDR func_addr, struct symtab *symtab) address we are looking for. */ for (i = 0; i < l->nitems; i++) { - struct linetable_entry *item = &(l->item[i]); + const struct linetable_entry *item = &(l->item[i]); /* Don't use line numbers of zero, they mark special entries in the table. See the commentary on symtab.h before the @@ -3889,7 +3889,8 @@ skip_prologue_using_sal (struct gdbarch *gdbarch, CORE_ADDR func_addr) do this. */ if (prologue_sal.symtab->language != language_asm) { - struct linetable *linetable = SYMTAB_LINETABLE (prologue_sal.symtab); + const struct linetable *linetable + = SYMTAB_LINETABLE (prologue_sal.symtab); int idx = 0; /* Skip any earlier lines, and any end-of-sequence marker diff --git a/gdb/symtab.h b/gdb/symtab.h index 2b3d8018a7..33e0ba4c09 100644 --- a/gdb/symtab.h +++ b/gdb/symtab.h @@ -1316,7 +1316,7 @@ struct symtab /* Table mapping core addresses to line numbers for this file. Can be NULL if none. Never shared between different symtabs. */ - struct linetable *linetable; + const struct linetable *linetable; /* Name of this source file. This pointer is never NULL. */ @@ -2032,7 +2032,8 @@ void iterate_over_symtabs (const char *name, std::vector<CORE_ADDR> find_pcs_for_symtab_line - (struct symtab *symtab, int line, struct linetable_entry **best_entry); + (struct symtab *symtab, int line, + const struct linetable_entry **best_entry); /* Prototype for callbacks for LA_ITERATE_OVER_SYMBOLS. The callback is called once per matching symbol SYM. The callback should return -- 2.13.6 ^ permalink raw reply [flat|nested] 18+ messages in thread
* [RFA 1/2] Make line tables independent of progspace 2018-03-21 17:18 [RFA 0/2] Make line tables independent of progspace Tom Tromey 2018-03-21 17:18 ` [RFA 2/2] Constify the line table Tom Tromey @ 2018-03-21 17:18 ` Tom Tromey 2018-03-24 3:40 ` Simon Marchi 1 sibling, 1 reply; 18+ messages in thread From: Tom Tromey @ 2018-03-21 17:18 UTC (permalink / raw) To: gdb-patches; +Cc: Tom Tromey From: Tom Tromey <tromey@redhat.com> This changes line tables to be independent of the program space. Unlike the corresponding series for minimal symbols, this is a single patch -- because line tables are used much less frequently than symbols, it seemed ok to have one patch for the whole conversion. This patch first renames the "pc" field of the line table. This made it simple to find all users of the field. It adds accessors and setters for the new field. All the setters of the field are updated not to offset the PC by the section offset. I think I've done this properly but I can't test all the spots, unfortunately. This patch also cleans up xcoffread.c:compare_lte to make it more obviously correct. It was difficult while writing this patch to decide what to do about gdbarch_addr_bits_remove (and to a lesser extent, gdbarch_adjust_dwarf2_line). Previously, all the symbol readers called this during reading, with the relocated address. However, there was an assumption later, in objfile_relocate1, that this did not need to be re-done after relocation. So, one option would have been to call this on the relocated address, and then "unrelocate" it (this is what I did for gdbarch_adjust_dwarf2_line). Instead, though, in this patch I chose to call gdbarch_addr_bits_remove at the point where the PC is computed. I'm still not completely certain that this is the best approach -- please consider this. While this patch is a step forward, it is still using the backlink from the symtab to the objfile. So, more work will be needed in this area before line tables can be moved to the per-BFD storage. gdb/ChangeLog 2018-03-21 Tom Tromey <tom@tromey.com> Obsoletes PR symtab/8077: * record-btrace.c (btrace_find_line_range): Update. * dwarf2read.c (lnp_state_machine::handle_set_address): Remove baseaddr argument. (dwarf_record_line_1): Don't call gdbarch_addr_bits_remove. * buildsym.c (record_line, compare_line_numbers): Update. * coffread.c (enter_linenos): Don't add section offset when creating line table. (coff_symtab_read): Don't call gdbarch_addr_bits_remove. * dbxread.c (process_one_symbol): Remove section offset when creating line table. * disasm.c (do_mixed_source_and_assembly): Update. * dwarf2read.c (dwarf_decode_lines_1): Don't add section offset when creating line table. * jit.c (jit_symtab_line_mapping_add_impl): Update. * mdebugread.c (parse_lines, psymtab_to_symtab_1): Don't add section offset when creating line table. (add_line): Update. * mi/mi-symbol-cmds.c (mi_cmd_symbol_list_lines): Update. * objfiles.c (objfile_relocate1): Don't relocate line tables. * python/py-linetable.c (ltpy_iternext): Update. * symmisc.c (dump_symtab_1): Update. * symtab.c (find_pc_sect_line, find_pcs_for_symtab_line) (skip_prologue_using_lineinfo, skip_prologue_using_sal): Update. * symtab.h (struct linetable_entry) <m_pc>: Rename from "pc". (SET_LINETABLE_ENTRY_ADDRESS, LINETABLE_ENTRY_RAW_ADDRESS) (LINETABLE_ENTRY_ADDRESS): New macros. * xcoffread.c (compare_lte): Update and rewrite. (arrange_linetable): Update. (enter_line_range): Don't add section offset when creating line table. --- gdb/ChangeLog | 34 +++++++++++++++++++++++++ gdb/buildsym.c | 9 ++++--- gdb/coffread.c | 7 ++---- gdb/dbxread.c | 26 +++++++++++-------- gdb/disasm.c | 30 +++++++++++++--------- gdb/dwarf2read.c | 9 ++++--- gdb/jit.c | 5 +++- gdb/mdebugread.c | 10 +++----- gdb/mi/mi-symbol-cmds.c | 11 ++++++--- gdb/objfiles.c | 17 ------------- gdb/python/py-linetable.c | 4 ++- gdb/record-btrace.c | 4 ++- gdb/symmisc.c | 8 ++++-- gdb/symtab.c | 63 ++++++++++++++++++++++++++++++----------------- gdb/symtab.h | 16 +++++++++++- gdb/xcoffread.c | 28 ++++++++++++++------- 16 files changed, 183 insertions(+), 98 deletions(-) diff --git a/gdb/buildsym.c b/gdb/buildsym.c index 5d38cb250f..99a92a8e31 100644 --- a/gdb/buildsym.c +++ b/gdb/buildsym.c @@ -947,7 +947,8 @@ record_line (struct subfile *subfile, int line, CORE_ADDR pc) if (line == 0 && subfile->line_vector->nitems > 0) { e = subfile->line_vector->item + subfile->line_vector->nitems - 1; - while (subfile->line_vector->nitems > 0 && e->pc == pc) + while (subfile->line_vector->nitems > 0 + && LINETABLE_ENTRY_RAW_ADDRESS (*e) == pc) { e--; subfile->line_vector->nitems--; @@ -956,7 +957,7 @@ record_line (struct subfile *subfile, int line, CORE_ADDR pc) e = subfile->line_vector->item + subfile->line_vector->nitems++; e->line = line; - e->pc = pc; + SET_LINETABLE_ENTRY_ADDRESS (*e, pc); } /* Needed in order to sort line tables from IBM xcoff files. Sigh! */ @@ -969,10 +970,10 @@ compare_line_numbers (const void *ln1p, const void *ln2p) /* Note: this code does not assume that CORE_ADDRs can fit in ints. Please keep it that way. */ - if (ln1->pc < ln2->pc) + if (LINETABLE_ENTRY_RAW_ADDRESS (*ln1) < LINETABLE_ENTRY_RAW_ADDRESS (*ln2)) return -1; - if (ln1->pc > ln2->pc) + if (LINETABLE_ENTRY_RAW_ADDRESS (*ln1) > LINETABLE_ENTRY_RAW_ADDRESS (*ln2)) return 1; /* If pc equal, sort by line. I'm not sure whether this is optimum diff --git a/gdb/coffread.c b/gdb/coffread.c index cad3e7e2f1..8408d07bde 100644 --- a/gdb/coffread.c +++ b/gdb/coffread.c @@ -1130,8 +1130,7 @@ coff_symtab_read (minimal_symbol_reader &reader, other statement-line-number. */ if (fcn_last_line == 1) record_line (current_subfile, fcn_first_line, - gdbarch_addr_bits_remove (gdbarch, - fcn_first_line_addr)); + fcn_first_line_addr); else enter_linenos (fcn_line_ptr, fcn_first_line, fcn_last_line, objfile); @@ -1506,11 +1505,9 @@ enter_linenos (long file_offset, int first_line, if (L_LNNO32 (&lptr) && L_LNNO32 (&lptr) <= last_line) { CORE_ADDR addr = lptr.l_addr.l_paddr; - addr += ANOFFSET (objfile->section_offsets, - SECT_OFF_TEXT (objfile)); record_line (current_subfile, first_line + L_LNNO32 (&lptr), - gdbarch_addr_bits_remove (gdbarch, addr)); + addr); } else break; diff --git a/gdb/dbxread.c b/gdb/dbxread.c index bdf4fb9c79..ff6f9ec20f 100644 --- a/gdb/dbxread.c +++ b/gdb/dbxread.c @@ -2562,10 +2562,10 @@ process_one_symbol (int type, int desc, CORE_ADDR valu, const char *name, but no N_SLINE stabs. */ if (sline_found_in_function) { - CORE_ADDR addr = last_function_start + valu; - - record_line (current_subfile, 0, - gdbarch_addr_bits_remove (gdbarch, addr)); + CORE_ADDR addr = (last_function_start + valu + - ANOFFSET (section_offsets, + SECT_OFF_TEXT (objfile))); + record_line (current_subfile, 0, addr); } within_function = 0; @@ -2767,16 +2767,22 @@ process_one_symbol (int type, int desc, CORE_ADDR valu, const char *name, if (within_function && sline_found_in_function == 0) { - CORE_ADDR addr = processing_gcc_compilation == 2 ? - last_function_start : valu; + CORE_ADDR addr = ((processing_gcc_compilation == 2 ? + last_function_start : valu) + - ANOFFSET (section_offsets, + SECT_OFF_TEXT (objfile))); - record_line (current_subfile, desc, - gdbarch_addr_bits_remove (gdbarch, addr)); + record_line (current_subfile, desc, addr); sline_found_in_function = 1; } else - record_line (current_subfile, desc, - gdbarch_addr_bits_remove (gdbarch, valu)); + { + CORE_ADDR addr = (valu + - ANOFFSET (section_offsets, + SECT_OFF_TEXT (objfile))); + + record_line (current_subfile, desc, addr); + } break; case N_BCOMM: diff --git a/gdb/disasm.c b/gdb/disasm.c index 833341a169..ec5b8a9b81 100644 --- a/gdb/disasm.c +++ b/gdb/disasm.c @@ -30,6 +30,7 @@ #include "safe-ctype.h" #include <algorithm> #include "common/gdb_optional.h" +#include "objfiles.h" /* Disassemble functions. FIXME: We should get rid of all the duplicate code in gdb that does @@ -368,13 +369,18 @@ do_mixed_source_and_assembly_deprecated /* First, skip all the preceding functions. */ - for (i = 0; i < nlines - 1 && le[i].pc < low; i++); + for (i = 0; + i < nlines - 1 && LINETABLE_ENTRY_ADDRESS (symtab, le[i]) < low; + i++) + ; /* Now, copy all entries before the end of this function. */ - for (; i < nlines - 1 && le[i].pc < high; i++) + for (; i < nlines - 1 && LINETABLE_ENTRY_ADDRESS (symtab, le[i]) < high; i++) { - if (le[i].line == le[i + 1].line && le[i].pc == le[i + 1].pc) + if (le[i].line == le[i + 1].line + && (LINETABLE_ENTRY_RAW_ADDRESS (le[i]) + == LINETABLE_ENTRY_RAW_ADDRESS (le[i + 1]))) continue; /* Ignore duplicates. */ /* Skip any end-of-function markers. */ @@ -384,19 +390,19 @@ do_mixed_source_and_assembly_deprecated mle[newlines].line = le[i].line; if (le[i].line > le[i + 1].line) out_of_order = 1; - mle[newlines].start_pc = le[i].pc; - mle[newlines].end_pc = le[i + 1].pc; + mle[newlines].start_pc = LINETABLE_ENTRY_ADDRESS (symtab, le[i]); + mle[newlines].end_pc = LINETABLE_ENTRY_ADDRESS (symtab, le[i + 1]); newlines++; } /* 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].pc < high) + if (i == nlines - 1 && LINETABLE_ENTRY_ADDRESS (symtab, le[i]) < high) { mle[newlines].line = le[i].line; - mle[newlines].start_pc = le[i].pc; - sal = find_pc_line (le[i].pc, 0); + mle[newlines].start_pc = LINETABLE_ENTRY_ADDRESS (symtab, le[i]); + sal = find_pc_line (LINETABLE_ENTRY_ADDRESS (symtab, le[i]), 0); mle[newlines].end_pc = sal.end; newlines++; } @@ -517,10 +523,12 @@ do_mixed_source_and_assembly (struct gdbarch *gdbarch, first_le = NULL; /* Skip all the preceding functions. */ - for (i = 0; i < nlines && le[i].pc < low; i++) - continue; + for (i = 0; + i < nlines - 1 && LINETABLE_ENTRY_ADDRESS (main_symtab, le[i]) < low; + i++) + ; - if (i < nlines && le[i].pc < high) + if (i < nlines && LINETABLE_ENTRY_ADDRESS (main_symtab, le[i]) < high) first_le = &le[i]; /* Add lines for every pc value. */ diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c index 6100438049..eb7a53c331 100644 --- a/gdb/dwarf2read.c +++ b/gdb/dwarf2read.c @@ -20860,8 +20860,11 @@ public: void handle_set_address (CORE_ADDR baseaddr, CORE_ADDR address) { m_op_index = 0; + /* Pass the relocated address to the gdbarch for modification, but + then only store the relative address. */ address += baseaddr; - m_address = gdbarch_adjust_dwarf2_line (m_gdbarch, address, false); + m_address = (gdbarch_adjust_dwarf2_line (m_gdbarch, address, false) + - baseaddr); } /* Handle DW_LNS_advance_pc. */ @@ -21087,8 +21090,6 @@ dwarf_record_line_1 (struct gdbarch *gdbarch, struct subfile *subfile, unsigned int line, CORE_ADDR address, record_line_ftype p_record_line) { - CORE_ADDR addr = gdbarch_addr_bits_remove (gdbarch, address); - if (dwarf_line_debug) { fprintf_unfiltered (gdb_stdlog, @@ -21097,7 +21098,7 @@ dwarf_record_line_1 (struct gdbarch *gdbarch, struct subfile *subfile, paddress (gdbarch, address)); } - (*p_record_line) (subfile, line, addr); + (*p_record_line) (subfile, line, address); } /* Subroutine of dwarf_decode_lines_1 to simplify it. diff --git a/gdb/jit.c b/gdb/jit.c index 2f23c058a0..e5c64cc791 100644 --- a/gdb/jit.c +++ b/gdb/jit.c @@ -622,7 +622,10 @@ 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].pc = (CORE_ADDR) map[i].pc; + /* Use the absolute address here, as the offsets will all be + zero. */ + SET_LINETABLE_ENTRY_ADDRESS (stab->linetable->item[i], + (CORE_ADDR) map[i].pc); stab->linetable->item[i].line = map[i].line; } } diff --git a/gdb/mdebugread.c b/gdb/mdebugread.c index c0bce55148..c9b29bcc0f 100644 --- a/gdb/mdebugread.c +++ b/gdb/mdebugread.c @@ -2213,7 +2213,7 @@ parse_lines (FDR *fh, PDR *pr, struct linetable *lt, int maxlines, halt = base + fh->cbLine; base += pr->cbLineOffset; - adr = pst->textlow + pr->adr - lowest_pdr_addr; + adr = pr->adr - lowest_pdr_addr; l = adr >> 2; /* in words */ for (lineno = pr->lnLow; base < halt;) @@ -4090,10 +4090,7 @@ psymtab_to_symtab_1 (struct objfile *objfile, else { /* Handle encoded stab line number. */ - valu += ANOFFSET (section_offsets, - SECT_OFF_TEXT (objfile)); - record_line (current_subfile, sh.index, - gdbarch_addr_bits_remove (gdbarch, valu)); + record_line (current_subfile, sh.index, valu); } } else if (sh.st == stProc || sh.st == stStaticProc @@ -4635,7 +4632,8 @@ add_line (struct linetable *lt, int lineno, CORE_ADDR adr, int last) return lineno; lt->item[lt->nitems].line = lineno; - lt->item[lt->nitems++].pc = adr << 2; + SET_LINETABLE_ENTRY_ADDRESS (lt->item[lt->nitems], adr << 2); + ++lt->nitems; return lineno; } \f diff --git a/gdb/mi/mi-symbol-cmds.c b/gdb/mi/mi-symbol-cmds.c index 223b8c3ce1..8f3b28e8f8 100644 --- a/gdb/mi/mi-symbol-cmds.c +++ b/gdb/mi/mi-symbol-cmds.c @@ -34,6 +34,7 @@ mi_cmd_symbol_list_lines (const char *command, char **argv, int argc) struct symtab *s; int i; struct ui_out *uiout = current_uiout; + struct linetable *linetable; if (argc != 1) error (_("-symbol-list-lines: Usage: SOURCE_FILENAME")); @@ -51,11 +52,13 @@ mi_cmd_symbol_list_lines (const char *command, char **argv, int argc) gdbarch = get_objfile_arch (SYMTAB_OBJFILE (s)); ui_out_emit_list list_emitter (uiout, "lines"); - if (SYMTAB_LINETABLE (s) != NULL && SYMTAB_LINETABLE (s)->nitems > 0) - for (i = 0; i < SYMTAB_LINETABLE (s)->nitems; i++) + linetable = SYMTAB_LINETABLE (s); + if (linetable != NULL && linetable->nitems > 0) + for (i = 0; i < linetable->nitems; i++) { ui_out_emit_tuple tuple_emitter (uiout, NULL); - uiout->field_core_addr ("pc", gdbarch, SYMTAB_LINETABLE (s)->item[i].pc); - uiout->field_int ("line", SYMTAB_LINETABLE (s)->item[i].line); + uiout->field_core_addr ("pc", gdbarch, + LINETABLE_ENTRY_ADDRESS (s, linetable->item[i])); + uiout->field_int ("line", linetable->item[i].line); } } diff --git a/gdb/objfiles.c b/gdb/objfiles.c index 98e81c48c4..0c1d3ffb57 100644 --- a/gdb/objfiles.c +++ b/gdb/objfiles.c @@ -799,23 +799,6 @@ objfile_relocate1 (struct objfile *objfile, /* OK, get all the symtabs. */ { struct compunit_symtab *cust; - struct symtab *s; - - ALL_OBJFILE_FILETABS (objfile, cust, s) - { - struct linetable *l; - int i; - - /* First the line table. */ - l = SYMTAB_LINETABLE (s); - if (l) - { - for (i = 0; i < l->nitems; ++i) - l->item[i].pc += ANOFFSET (delta, - COMPUNIT_BLOCK_LINE_SECTION - (cust)); - } - } ALL_OBJFILE_COMPUNITS (objfile, cust) { diff --git a/gdb/python/py-linetable.c b/gdb/python/py-linetable.c index c4e80d02a5..5f91804d60 100644 --- a/gdb/python/py-linetable.c +++ b/gdb/python/py-linetable.c @@ -20,6 +20,7 @@ #include "defs.h" #include "python-internal.h" #include "py-ref.h" +#include "objfiles.h" typedef struct { PyObject_HEAD @@ -423,7 +424,8 @@ ltpy_iternext (PyObject *self) item = &(SYMTAB_LINETABLE (symtab)->item[iter_obj->current_index]); } - obj = build_linetable_entry (item->line, item->pc); + obj = build_linetable_entry (item->line, + LINETABLE_ENTRY_ADDRESS (symtab, *item)); iter_obj->current_index++; return obj; diff --git a/gdb/record-btrace.c b/gdb/record-btrace.c index 7264b8ed2b..0b447292f6 100644 --- a/gdb/record-btrace.c +++ b/gdb/record-btrace.c @@ -40,6 +40,7 @@ #include "inf-loop.h" #include "vec.h" #include <algorithm> +#include "objfiles.h" /* The target_ops of record-btrace. */ static struct target_ops record_btrace_ops; @@ -596,7 +597,8 @@ btrace_find_line_range (CORE_ADDR pc) range = btrace_mk_line_range (symtab, 0, 0); for (i = 0; i < nlines - 1; i++) { - if ((lines[i].pc == pc) && (lines[i].line != 0)) + if (LINETABLE_ENTRY_ADDRESS (symtab, lines[i]) == pc + && (lines[i].line != 0)) range = btrace_line_range_add (range, lines[i].line); } diff --git a/gdb/symmisc.c b/gdb/symmisc.c index 9adde044cd..89506ebfc0 100644 --- a/gdb/symmisc.c +++ b/gdb/symmisc.c @@ -303,7 +303,10 @@ dump_symtab_1 (struct symtab *symtab, struct ui_file *outfile) for (i = 0; i < len; i++) { fprintf_filtered (outfile, " line %d at ", l->item[i].line); - fputs_filtered (paddress (gdbarch, l->item[i].pc), outfile); + fputs_filtered (paddress (gdbarch, + LINETABLE_ENTRY_ADDRESS (symtab, + l->item[i])), + outfile); fprintf_filtered (outfile, "\n"); } } @@ -1018,7 +1021,8 @@ maintenance_print_one_line_table (struct symtab *symtab, void *data) item = &linetable->item [i]; printf_filtered (_("%-6d %6d %s\n"), i, item->line, - core_addr_to_string (item->pc)); + core_addr_to_string (LINETABLE_ENTRY_ADDRESS + (symtab, *item))); } } diff --git a/gdb/symtab.c b/gdb/symtab.c index 2b1f9558ab..f57ecafa42 100644 --- a/gdb/symtab.c +++ b/gdb/symtab.c @@ -3070,6 +3070,7 @@ find_pc_sect_line (CORE_ADDR pc, struct obj_section *section, int notcurrent) we will use a line one less than this, with a range from the start of that file to the first line's pc. */ struct linetable_entry *alt = NULL; + struct symtab *alt_symtab = NULL; /* Info on best line seen in this file. */ @@ -3209,13 +3210,18 @@ find_pc_sect_line (CORE_ADDR pc, struct obj_section *section, int notcurrent) /* Is this file's first line closer than the first lines of other files? If so, record this file, and its first line, as best alternate. */ - if (item->pc > pc && (!alt || item->pc < alt->pc)) - alt = item; + if (LINETABLE_ENTRY_ADDRESS (iter_s, *item) > pc + && (!alt || (LINETABLE_ENTRY_ADDRESS (iter_s, *item) + < LINETABLE_ENTRY_ADDRESS (alt_symtab, *alt)))) + { + alt = item; + alt_symtab = iter_s; + } - auto pc_compare = [](const CORE_ADDR & pc, - const struct linetable_entry & lhs)->bool + auto pc_compare = [=](const CORE_ADDR & pc, + const struct linetable_entry & lhs)->bool { - return pc < lhs.pc; + return pc < LINETABLE_ENTRY_ADDRESS (iter_s, lhs); }; struct linetable_entry *first = item; @@ -3234,22 +3240,28 @@ find_pc_sect_line (CORE_ADDR pc, struct obj_section *section, int notcurrent) save prev if it represents the end of a function (i.e. line number 0) instead of a real line. */ - if (prev && prev->line && (!best || prev->pc > best->pc)) + if (prev + && prev->line + && (!best || (LINETABLE_ENTRY_ADDRESS (iter_s, *prev) + > LINETABLE_ENTRY_ADDRESS (best_symtab, *best)))) { best = prev; best_symtab = iter_s; /* Discard BEST_END if it's before the PC of the current BEST. */ - if (best_end <= best->pc) + if (best_end <= LINETABLE_ENTRY_ADDRESS (best_symtab, *best)) best_end = 0; } /* If another line (denoted by ITEM) is in the linetable and its PC is after BEST's PC, but before the current BEST_END, then use ITEM's PC as the new best_end. */ - if (best && item < last && item->pc > best->pc - && (best_end == 0 || best_end > item->pc)) - best_end = item->pc; + if (best && item < last + && (LINETABLE_ENTRY_ADDRESS (iter_s, *item) + > LINETABLE_ENTRY_ADDRESS (best_symtab, *best)) + && (best_end == 0 + || best_end > LINETABLE_ENTRY_ADDRESS (iter_s, *item))) + best_end = LINETABLE_ENTRY_ADDRESS (iter_s, *item); } if (!best_symtab) @@ -3271,11 +3283,12 @@ find_pc_sect_line (CORE_ADDR pc, struct obj_section *section, int notcurrent) { val.symtab = best_symtab; val.line = best->line; - val.pc = best->pc; - if (best_end && (!alt || best_end < alt->pc)) + val.pc = LINETABLE_ENTRY_ADDRESS (best_symtab, *best); + if (best_end && (!alt || best_end < LINETABLE_ENTRY_ADDRESS (alt_symtab, + *alt))) val.end = best_end; else if (alt) - val.end = alt->pc; + val.end = LINETABLE_ENTRY_ADDRESS (alt_symtab, *alt); else val.end = BLOCK_END (BLOCKVECTOR_BLOCK (bv, GLOBAL_BLOCK)); } @@ -3419,6 +3432,7 @@ find_pcs_for_symtab_line (struct symtab *symtab, int line, { int start = 0; std::vector<CORE_ADDR> result; + struct linetable *linetable = SYMTAB_LINETABLE (symtab); /* First, collect all the PCs that are at this line. */ while (1) @@ -3426,14 +3440,13 @@ find_pcs_for_symtab_line (struct symtab *symtab, int line, int was_exact; int idx; - idx = find_line_common (SYMTAB_LINETABLE (symtab), line, &was_exact, - start); + idx = find_line_common (linetable, line, &was_exact, start); if (idx < 0) break; if (!was_exact) { - struct linetable_entry *item = &SYMTAB_LINETABLE (symtab)->item[idx]; + struct linetable_entry *item = &linetable->item[idx]; if (*best_item == NULL || item->line < (*best_item)->line) *best_item = item; @@ -3441,7 +3454,8 @@ find_pcs_for_symtab_line (struct symtab *symtab, int line, break; } - result.push_back (SYMTAB_LINETABLE (symtab)->item[idx].pc); + result.push_back (LINETABLE_ENTRY_ADDRESS (symtab, + linetable->item[idx])); start = idx + 1; } @@ -3467,7 +3481,7 @@ find_line_pc (struct symtab *symtab, int line, CORE_ADDR *pc) if (symtab != NULL) { l = SYMTAB_LINETABLE (symtab); - *pc = l->item[ind].pc; + *pc = LINETABLE_ENTRY_ADDRESS (symtab, l->item[ind]); return 1; } else @@ -3656,8 +3670,10 @@ skip_prologue_using_lineinfo (CORE_ADDR func_addr, struct symtab *symtab) /* Don't use line numbers of zero, they mark special entries in the table. See the commentary on symtab.h before the definition of struct linetable. */ - if (item->line > 0 && func_start <= item->pc && item->pc < func_end) - return item->pc; + if (item->line > 0 + && func_start <= LINETABLE_ENTRY_ADDRESS (symtab, *item) + && LINETABLE_ENTRY_ADDRESS (symtab, *item) < func_end) + return LINETABLE_ENTRY_ADDRESS (symtab, *item); } return func_addr; @@ -3878,13 +3894,16 @@ skip_prologue_using_sal (struct gdbarch *gdbarch, CORE_ADDR func_addr) /* Skip any earlier lines, and any end-of-sequence marker from a previous function. */ - while (linetable->item[idx].pc != prologue_sal.pc + while ((LINETABLE_ENTRY_ADDRESS (prologue_sal.symtab, + linetable->item[idx]) + != prologue_sal.pc) || linetable->item[idx].line == 0) idx++; if (idx+1 < linetable->nitems && linetable->item[idx+1].line != 0 - && linetable->item[idx+1].pc == start_pc) + && LINETABLE_ENTRY_ADDRESS (prologue_sal.symtab, + linetable->item[idx+1]) == start_pc) return start_pc; } diff --git a/gdb/symtab.h b/gdb/symtab.h index f9d52e7697..2b3d8018a7 100644 --- a/gdb/symtab.h +++ b/gdb/symtab.h @@ -1228,9 +1228,23 @@ struct rust_vtable_symbol : public symbol struct linetable_entry { int line; - CORE_ADDR pc; + + /* Note that the PC as stored is unrelocated. The appropriate + offset must be applied before it can be used. */ + CORE_ADDR m_pc; }; +#define SET_LINETABLE_ENTRY_ADDRESS(ENTRY, PC) \ + ((ENTRY).m_pc = (PC)) +#define LINETABLE_ENTRY_RAW_ADDRESS(ENTRY) \ + ((ENTRY).m_pc + 0) +#define LINETABLE_ENTRY_ADDRESS(SYMTAB, ENTRY) \ + gdbarch_addr_bits_remove \ + (get_objfile_arch (SYMTAB_OBJFILE (SYMTAB)), \ + ((ENTRY).m_pc \ + + ANOFFSET (SYMTAB_OBJFILE (SYMTAB)->section_offsets, \ + SYMTAB_COMPUNIT (SYMTAB)->block_line_section))) + /* The order of entries in the linetable is significant. They should be sorted by increasing values of the pc field. If there is more than one entry for a given pc, then I'm not sure what should happen (and diff --git a/gdb/xcoffread.c b/gdb/xcoffread.c index 8c707aa8fe..8a07af402d 100644 --- a/gdb/xcoffread.c +++ b/gdb/xcoffread.c @@ -420,7 +420,11 @@ compare_lte (const void *lte1p, const void *lte2p) struct linetable_entry *lte1 = (struct linetable_entry *) lte1p; struct linetable_entry *lte2 = (struct linetable_entry *) lte2p; - return lte1->pc - lte2->pc; + if (LINETABLE_ENTRY_RAW_ADDRESS (*lte1) < LINETABLE_ENTRY_RAW_ADDRESS (*lte2)) + return -1; + if (LINETABLE_ENTRY_RAW_ADDRESS (*lte1) > LINETABLE_ENTRY_RAW_ADDRESS (*lte2)) + return 1; + return 0; } /* Given a line table with function entries are marked, arrange its @@ -458,13 +462,16 @@ arrange_linetable (struct linetable *oldLineTb) fentry_size * sizeof (struct linetable_entry)); } fentry[function_count].line = ii; - fentry[function_count].pc = oldLineTb->item[ii].pc; + SET_LINETABLE_ENTRY_ADDRESS + (fentry[function_count], + LINETABLE_ENTRY_RAW_ADDRESS (oldLineTb->item[ii])); ++function_count; /* If the function was compiled with XLC, we may have to add an extra line entry later. Reserve space for that. */ if (ii + 1 < oldLineTb->nitems - && oldLineTb->item[ii].pc != oldLineTb->item[ii + 1].pc) + && (LINETABLE_ENTRY_RAW_ADDRESS (oldLineTb->item[ii]) + != LINETABLE_ENTRY_RAW_ADDRESS (oldLineTb->item[ii + 1]))) extra_lines++; } } @@ -501,7 +508,8 @@ arrange_linetable (struct linetable *oldLineTb) extra line to cover the function prologue. */ jj = fentry[ii].line; if (jj + 1 < oldLineTb->nitems - && oldLineTb->item[jj].pc != oldLineTb->item[jj + 1].pc) + && (LINETABLE_ENTRY_RAW_ADDRESS (oldLineTb->item[jj]) + != LINETABLE_ENTRY_RAW_ADDRESS (oldLineTb->item[jj + 1]))) { newLineTb->item[newline] = oldLineTb->item[jj]; newLineTb->item[newline].line = oldLineTb->item[jj + 1].line; @@ -871,6 +879,8 @@ enter_line_range (struct subfile *subfile, unsigned beginoffset, while (curoffset <= limit_offset) { + CORE_ADDR relocated; + bfd_seek (abfd, curoffset, SEEK_SET); bfd_bread (ext_lnno, linesz, abfd); bfd_coff_swap_lineno_in (abfd, ext_lnno, &int_lnno); @@ -879,20 +889,20 @@ enter_line_range (struct subfile *subfile, unsigned beginoffset, addr = (int_lnno.l_lnno ? int_lnno.l_addr.l_paddr : read_symbol_nvalue (int_lnno.l_addr.l_symndx)); - addr += ANOFFSET (objfile->section_offsets, SECT_OFF_TEXT (objfile)); - if (addr < startaddr || (endaddr && addr >= endaddr)) + relocated = (addr + ANOFFSET (objfile->section_offsets, + SECT_OFF_TEXT (objfile))); + if (relocated < startaddr || (endaddr && relocated >= endaddr)) return; if (int_lnno.l_lnno == 0) { *firstLine = read_symbol_lineno (int_lnno.l_addr.l_symndx); - record_line (subfile, 0, gdbarch_addr_bits_remove (gdbarch, addr)); + record_line (subfile, 0, addr); --(*firstLine); } else - record_line (subfile, *firstLine + int_lnno.l_lnno, - gdbarch_addr_bits_remove (gdbarch, addr)); + record_line (subfile, *firstLine + int_lnno.l_lnno, addr); curoffset += linesz; } } -- 2.13.6 ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [RFA 1/2] Make line tables independent of progspace 2018-03-21 17:18 ` [RFA 1/2] Make line tables independent of progspace Tom Tromey @ 2018-03-24 3:40 ` Simon Marchi 2018-03-27 4:16 ` Tom Tromey 0 siblings, 1 reply; 18+ messages in thread From: Simon Marchi @ 2018-03-24 3:40 UTC (permalink / raw) To: Tom Tromey, gdb-patches; +Cc: Tom Tromey Hi Tom, I haven't looked at in details, but I just have one comment for now: On 2018-03-21 01:18 PM, Tom Tromey wrote: > diff --git a/gdb/symtab.h b/gdb/symtab.h > index f9d52e7697..2b3d8018a7 100644 > --- a/gdb/symtab.h > +++ b/gdb/symtab.h > @@ -1228,9 +1228,23 @@ struct rust_vtable_symbol : public symbol > struct linetable_entry > { > int line; > - CORE_ADDR pc; > + > + /* Note that the PC as stored is unrelocated. The appropriate > + offset must be applied before it can be used. */ > + CORE_ADDR m_pc; > }; > > +#define SET_LINETABLE_ENTRY_ADDRESS(ENTRY, PC) \ > + ((ENTRY).m_pc = (PC)) > +#define LINETABLE_ENTRY_RAW_ADDRESS(ENTRY) \ > + ((ENTRY).m_pc + 0) > +#define LINETABLE_ENTRY_ADDRESS(SYMTAB, ENTRY) \ > + gdbarch_addr_bits_remove \ > + (get_objfile_arch (SYMTAB_OBJFILE (SYMTAB)), \ > + ((ENTRY).m_pc \ > + + ANOFFSET (SYMTAB_OBJFILE (SYMTAB)->section_offsets, \ > + SYMTAB_COMPUNIT (SYMTAB)->block_line_section))) Is it possible to make these functions instead of macros? They could even be made into methods of struct linetable_entry without making the structure non-pod. I think it would also make the code more readable: LINETABLE_ENTRY_RAW_ADDRESS (e) SET_LINETABLE_ENTRY_ADDRESS (e, pc); LINETABLE_ENTRY_ADDRESS (s, e) vs e.raw_address () e.set_address (pc) e.address (s) I think it makes a good difference in terms of readability in those big ifs: if (LINETABLE_ENTRY_ADDRESS (iter_s, *item) > pc && (!alt || (LINETABLE_ENTRY_ADDRESS (iter_s, *item) < LINETABLE_ENTRY_ADDRESS (alt_symtab, *alt)))) Simon ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [RFA 1/2] Make line tables independent of progspace 2018-03-24 3:40 ` Simon Marchi @ 2018-03-27 4:16 ` Tom Tromey 2018-03-27 4:53 ` Tom Tromey 0 siblings, 1 reply; 18+ messages in thread From: Tom Tromey @ 2018-03-27 4:16 UTC (permalink / raw) To: Simon Marchi; +Cc: Tom Tromey, gdb-patches, Tom Tromey >>>>> "Simon" == Simon Marchi <simark@simark.ca> writes: >> +#define SET_LINETABLE_ENTRY_ADDRESS(ENTRY, PC) \ >> + ((ENTRY).m_pc = (PC)) >> +#define LINETABLE_ENTRY_RAW_ADDRESS(ENTRY) \ >> + ((ENTRY).m_pc + 0) >> +#define LINETABLE_ENTRY_ADDRESS(SYMTAB, ENTRY) \ >> + gdbarch_addr_bits_remove \ >> + (get_objfile_arch (SYMTAB_OBJFILE (SYMTAB)), \ >> + ((ENTRY).m_pc \ >> + + ANOFFSET (SYMTAB_OBJFILE (SYMTAB)->section_offsets, \ >> + SYMTAB_COMPUNIT (SYMTAB)->block_line_section))) Simon> Is it possible to make these functions instead of macros? They Simon> could even be made into methods of struct linetable_entry without Simon> making the structure non-pod. I did this and it does seem nicer. The call sites are shorter now and fewer of them have to be line-wrapped. Simon> I think it makes a good difference in terms of readability in those big ifs: Yes, good call. I've run the new patch through the buildbot. I'll resubmit it soon, I just have to update the ChangeLog and rewrite the commit message. Tom ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [RFA 1/2] Make line tables independent of progspace 2018-03-27 4:16 ` Tom Tromey @ 2018-03-27 4:53 ` Tom Tromey 2018-03-27 20:22 ` Simon Marchi 2018-03-28 3:34 ` Simon Marchi 0 siblings, 2 replies; 18+ messages in thread From: Tom Tromey @ 2018-03-27 4:53 UTC (permalink / raw) To: Tom Tromey; +Cc: Simon Marchi, gdb-patches Tom> I've run the new patch through the buildbot. I'll resubmit it soon, I Tom> just have to update the ChangeLog and rewrite the commit message. Here it is. Let me reiterate that this probably requires careful review as the buildbot may not test some of these paths. Tom commit 79caf09e07239e97ea9237d80a1da134b4a9cf87 Author: Tom Tromey <tom@tromey.com> Date: Wed Mar 21 15:11:09 2018 -0600 Make line tables independent of progspace This changes line tables to be independent of the program space. Unlike the corresponding series for minimal symbols, this is a single patch -- because line tables are used much less frequently than symbols, it seemed ok to have one patch for the whole conversion. This patch first renames the fields of the line table, making them private. This made it simple to find all users of the fields. It adds accessors and setters. All the setters of the field are updated not to offset the PC by the section offset. I think I've done this properly but I can't test all the spots, unfortunately. This patch also cleans up xcoffread.c:compare_lte to make it more obviously correct. It was difficult while writing this patch to decide what to do about gdbarch_addr_bits_remove (and to a lesser extent, gdbarch_adjust_dwarf2_line). Previously, all the symbol readers called this during reading, with the relocated address. However, there was an assumption later, in objfile_relocate1, that this did not need to be re-done after relocation. So, one option would have been to call this on the relocated address, and then "unrelocate" it (this is what I did for gdbarch_adjust_dwarf2_line). Instead, though, in this patch I chose to call gdbarch_addr_bits_remove at the point where the PC is computed. I'm still not completely certain that this is the best approach -- please consider this. While this patch is a step forward, it is still using the backlink from the symtab to the objfile. So, more work will be needed in this area before line tables can be moved to the per-BFD storage. gdb/ChangeLog 2018-03-21 Tom Tromey <tom@tromey.com> Obsoletes PR symtab/8077: * record-btrace.c (btrace_find_line_range): Update. * dwarf2read.c (lnp_state_machine::handle_set_address): Remove baseaddr argument. (dwarf_record_line_1): Don't call gdbarch_addr_bits_remove. * buildsym.c (record_line, compare_line_numbers): Update. * coffread.c (enter_linenos): Don't add section offset when creating line table. (coff_symtab_read): Don't call gdbarch_addr_bits_remove. * dbxread.c (process_one_symbol): Remove section offset when creating line table. * disasm.c (do_mixed_source_and_assembly): Update. * dwarf2read.c (dwarf_decode_lines_1): Don't add section offset when creating line table. * jit.c (jit_symtab_line_mapping_add_impl): Update. * mdebugread.c (parse_lines, psymtab_to_symtab_1): Don't add section offset when creating line table. (add_line): Update. * mi/mi-symbol-cmds.c (mi_cmd_symbol_list_lines): Update. * objfiles.c (objfile_relocate1): Don't relocate line tables. * python/py-linetable.c (ltpy_iternext): Update. * symmisc.c (dump_symtab_1): Update. * symtab.c (find_pc_sect_line, find_pcs_for_symtab_line) (skip_prologue_using_lineinfo, skip_prologue_using_sal): Update. * symtab.h (struct linetable_entry) <m_pc>: Rename from "pc". <m_line>: Rename from "line". <set, raw_address, address, line>: New methods. * xcoffread.c (compare_lte): Update and rewrite. (arrange_linetable): Update. (enter_line_range): Don't add section offset when creating line table. diff --git a/gdb/ChangeLog b/gdb/ChangeLog index 64130c5392..1170c62673 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,37 @@ +2018-03-21 Tom Tromey <tom@tromey.com> + + Obsoletes PR symtab/8077: + * record-btrace.c (btrace_find_line_range): Update. + * dwarf2read.c (lnp_state_machine::handle_set_address): Remove + baseaddr argument. + (dwarf_record_line_1): Don't call gdbarch_addr_bits_remove. + * buildsym.c (record_line, compare_line_numbers): Update. + * coffread.c (enter_linenos): Don't add section offset when + creating line table. + (coff_symtab_read): Don't call gdbarch_addr_bits_remove. + * dbxread.c (process_one_symbol): Remove section offset when + creating line table. + * disasm.c (do_mixed_source_and_assembly): Update. + * dwarf2read.c (dwarf_decode_lines_1): Don't add section offset + when creating line table. + * jit.c (jit_symtab_line_mapping_add_impl): Update. + * mdebugread.c (parse_lines, psymtab_to_symtab_1): Don't add + section offset when creating line table. + (add_line): Update. + * mi/mi-symbol-cmds.c (mi_cmd_symbol_list_lines): Update. + * objfiles.c (objfile_relocate1): Don't relocate line tables. + * python/py-linetable.c (ltpy_iternext): Update. + * symmisc.c (dump_symtab_1): Update. + * symtab.c (find_pc_sect_line, find_pcs_for_symtab_line) + (skip_prologue_using_lineinfo, skip_prologue_using_sal): Update. + * symtab.h (struct linetable_entry) <m_pc>: Rename from "pc". + <m_line>: Rename from "line". + <set, raw_address, address, line>: New methods. + * xcoffread.c (compare_lte): Update and rewrite. + (arrange_linetable): Update. + (enter_line_range): Don't add section offset when creating line + table. + 2018-03-26 Tom Tromey <tom@tromey.com> * stack.c (backtrace_command_1): Remove verbose code. diff --git a/gdb/buildsym.c b/gdb/buildsym.c index 5d38cb250f..f2c08aa530 100644 --- a/gdb/buildsym.c +++ b/gdb/buildsym.c @@ -947,7 +947,8 @@ record_line (struct subfile *subfile, int line, CORE_ADDR pc) if (line == 0 && subfile->line_vector->nitems > 0) { e = subfile->line_vector->item + subfile->line_vector->nitems - 1; - while (subfile->line_vector->nitems > 0 && e->pc == pc) + while (subfile->line_vector->nitems > 0 + && e->raw_address () == pc) { e--; subfile->line_vector->nitems--; @@ -955,8 +956,7 @@ record_line (struct subfile *subfile, int line, CORE_ADDR pc) } e = subfile->line_vector->item + subfile->line_vector->nitems++; - e->line = line; - e->pc = pc; + e->set (line, pc); } /* Needed in order to sort line tables from IBM xcoff files. Sigh! */ @@ -969,15 +969,15 @@ compare_line_numbers (const void *ln1p, const void *ln2p) /* Note: this code does not assume that CORE_ADDRs can fit in ints. Please keep it that way. */ - if (ln1->pc < ln2->pc) + if (ln1->raw_address () < ln2->raw_address ()) return -1; - if (ln1->pc > ln2->pc) + if (ln1->raw_address () > ln2->raw_address ()) return 1; /* If pc equal, sort by line. I'm not sure whether this is optimum behavior (see comment at struct linetable in symtab.h). */ - return ln1->line - ln2->line; + return ln1->line () - ln2->line (); } \f /* See buildsym.h. */ diff --git a/gdb/coffread.c b/gdb/coffread.c index cad3e7e2f1..8408d07bde 100644 --- a/gdb/coffread.c +++ b/gdb/coffread.c @@ -1130,8 +1130,7 @@ coff_symtab_read (minimal_symbol_reader &reader, other statement-line-number. */ if (fcn_last_line == 1) record_line (current_subfile, fcn_first_line, - gdbarch_addr_bits_remove (gdbarch, - fcn_first_line_addr)); + fcn_first_line_addr); else enter_linenos (fcn_line_ptr, fcn_first_line, fcn_last_line, objfile); @@ -1506,11 +1505,9 @@ enter_linenos (long file_offset, int first_line, if (L_LNNO32 (&lptr) && L_LNNO32 (&lptr) <= last_line) { CORE_ADDR addr = lptr.l_addr.l_paddr; - addr += ANOFFSET (objfile->section_offsets, - SECT_OFF_TEXT (objfile)); record_line (current_subfile, first_line + L_LNNO32 (&lptr), - gdbarch_addr_bits_remove (gdbarch, addr)); + addr); } else break; diff --git a/gdb/dbxread.c b/gdb/dbxread.c index bdf4fb9c79..ff6f9ec20f 100644 --- a/gdb/dbxread.c +++ b/gdb/dbxread.c @@ -2562,10 +2562,10 @@ process_one_symbol (int type, int desc, CORE_ADDR valu, const char *name, but no N_SLINE stabs. */ if (sline_found_in_function) { - CORE_ADDR addr = last_function_start + valu; - - record_line (current_subfile, 0, - gdbarch_addr_bits_remove (gdbarch, addr)); + CORE_ADDR addr = (last_function_start + valu + - ANOFFSET (section_offsets, + SECT_OFF_TEXT (objfile))); + record_line (current_subfile, 0, addr); } within_function = 0; @@ -2767,16 +2767,22 @@ process_one_symbol (int type, int desc, CORE_ADDR valu, const char *name, if (within_function && sline_found_in_function == 0) { - CORE_ADDR addr = processing_gcc_compilation == 2 ? - last_function_start : valu; + CORE_ADDR addr = ((processing_gcc_compilation == 2 ? + last_function_start : valu) + - ANOFFSET (section_offsets, + SECT_OFF_TEXT (objfile))); - record_line (current_subfile, desc, - gdbarch_addr_bits_remove (gdbarch, addr)); + record_line (current_subfile, desc, addr); sline_found_in_function = 1; } else - record_line (current_subfile, desc, - gdbarch_addr_bits_remove (gdbarch, valu)); + { + CORE_ADDR addr = (valu + - ANOFFSET (section_offsets, + SECT_OFF_TEXT (objfile))); + + record_line (current_subfile, desc, addr); + } break; case N_BCOMM: diff --git a/gdb/disasm.c b/gdb/disasm.c index 833341a169..506152d214 100644 --- a/gdb/disasm.c +++ b/gdb/disasm.c @@ -30,6 +30,7 @@ #include "safe-ctype.h" #include <algorithm> #include "common/gdb_optional.h" +#include "objfiles.h" /* Disassemble functions. FIXME: We should get rid of all the duplicate code in gdb that does @@ -368,35 +369,39 @@ do_mixed_source_and_assembly_deprecated /* First, skip all the preceding functions. */ - for (i = 0; i < nlines - 1 && le[i].pc < low; i++); + for (i = 0; + i < nlines - 1 && le[i].address (symtab) < low; + i++) + ; /* Now, copy all entries before the end of this function. */ - for (; i < nlines - 1 && le[i].pc < high; i++) + for (; i < nlines - 1 && le[i].address (symtab) < high; i++) { - if (le[i].line == le[i + 1].line && le[i].pc == le[i + 1].pc) + if (le[i].line () == le[i + 1].line () + && le[i].raw_address () == le[i + 1].raw_address ()) continue; /* Ignore duplicates. */ /* Skip any end-of-function markers. */ - if (le[i].line == 0) + if (le[i].line () == 0) continue; - mle[newlines].line = le[i].line; - if (le[i].line > le[i + 1].line) + mle[newlines].line = le[i].line (); + if (le[i].line () > le[i + 1].line ()) out_of_order = 1; - mle[newlines].start_pc = le[i].pc; - mle[newlines].end_pc = le[i + 1].pc; + mle[newlines].start_pc = le[i].address (symtab); + mle[newlines].end_pc = le[i + 1].address (symtab); newlines++; } /* 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].pc < high) + if (i == nlines - 1 && le[i].address (symtab) < high) { - mle[newlines].line = le[i].line; - mle[newlines].start_pc = le[i].pc; - sal = find_pc_line (le[i].pc, 0); + mle[newlines].line = le[i].line (); + mle[newlines].start_pc = le[i].address (symtab); + sal = find_pc_line (le[i].address (symtab), 0); mle[newlines].end_pc = sal.end; newlines++; } @@ -517,10 +522,12 @@ do_mixed_source_and_assembly (struct gdbarch *gdbarch, first_le = NULL; /* Skip all the preceding functions. */ - for (i = 0; i < nlines && le[i].pc < low; i++) - continue; + for (i = 0; + i < nlines - 1 && le[i].address (main_symtab) < low; + i++) + ; - if (i < nlines && le[i].pc < high) + if (i < nlines && le[i].address (main_symtab) < high) first_le = &le[i]; /* Add lines for every pc value. */ @@ -589,9 +596,9 @@ do_mixed_source_and_assembly (struct gdbarch *gdbarch, lines. */ if (last_line == 0 && first_le != NULL - && first_le->line < sal.line) + && first_le->line () < sal.line) { - start_preceding_line_to_display = first_le->line; + start_preceding_line_to_display = first_le->line (); end_preceding_line_to_display = sal.line; } } diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c index 211364c428..ab6e687952 100644 --- a/gdb/dwarf2read.c +++ b/gdb/dwarf2read.c @@ -20859,8 +20859,11 @@ public: void handle_set_address (CORE_ADDR baseaddr, CORE_ADDR address) { m_op_index = 0; + /* Pass the relocated address to the gdbarch for modification, but + then only store the relative address. */ address += baseaddr; - m_address = gdbarch_adjust_dwarf2_line (m_gdbarch, address, false); + m_address = (gdbarch_adjust_dwarf2_line (m_gdbarch, address, false) + - baseaddr); } /* Handle DW_LNS_advance_pc. */ @@ -21086,8 +21089,6 @@ dwarf_record_line_1 (struct gdbarch *gdbarch, struct subfile *subfile, unsigned int line, CORE_ADDR address, record_line_ftype p_record_line) { - CORE_ADDR addr = gdbarch_addr_bits_remove (gdbarch, address); - if (dwarf_line_debug) { fprintf_unfiltered (gdb_stdlog, @@ -21096,7 +21097,7 @@ dwarf_record_line_1 (struct gdbarch *gdbarch, struct subfile *subfile, paddress (gdbarch, address)); } - (*p_record_line) (subfile, line, addr); + (*p_record_line) (subfile, line, address); } /* Subroutine of dwarf_decode_lines_1 to simplify it. diff --git a/gdb/jit.c b/gdb/jit.c index 2f23c058a0..eb4579c96b 100644 --- a/gdb/jit.c +++ b/gdb/jit.c @@ -622,8 +622,9 @@ 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].pc = (CORE_ADDR) map[i].pc; - stab->linetable->item[i].line = map[i].line; + /* Use the absolute address here, as the offsets will all be + zero. */ + stab->linetable->item[i].set (map[i].line, (CORE_ADDR) map[i].pc); } } diff --git a/gdb/linespec.c b/gdb/linespec.c index 1236b3f475..d9ad798200 100644 --- a/gdb/linespec.c +++ b/gdb/linespec.c @@ -2199,7 +2199,7 @@ create_sals_line_offset (struct linespec_state *self, = decode_digits_ordinary (self, ls, val.line, &best_entry); if (intermediate_results.empty () && best_entry != NULL) intermediate_results = decode_digits_ordinary (self, ls, - best_entry->line, + best_entry->line (), &best_entry); /* For optimized code, the compiler can scatter one source line diff --git a/gdb/mdebugread.c b/gdb/mdebugread.c index c0bce55148..a12e459cc6 100644 --- a/gdb/mdebugread.c +++ b/gdb/mdebugread.c @@ -2213,7 +2213,7 @@ parse_lines (FDR *fh, PDR *pr, struct linetable *lt, int maxlines, halt = base + fh->cbLine; base += pr->cbLineOffset; - adr = pst->textlow + pr->adr - lowest_pdr_addr; + adr = pr->adr - lowest_pdr_addr; l = adr >> 2; /* in words */ for (lineno = pr->lnLow; base < halt;) @@ -4090,10 +4090,7 @@ psymtab_to_symtab_1 (struct objfile *objfile, else { /* Handle encoded stab line number. */ - valu += ANOFFSET (section_offsets, - SECT_OFF_TEXT (objfile)); - record_line (current_subfile, sh.index, - gdbarch_addr_bits_remove (gdbarch, valu)); + record_line (current_subfile, sh.index, valu); } } else if (sh.st == stProc || sh.st == stStaticProc @@ -4634,8 +4631,8 @@ add_line (struct linetable *lt, int lineno, CORE_ADDR adr, int last) if (last == lineno) /* Skip continuation lines. */ return lineno; - lt->item[lt->nitems].line = lineno; - lt->item[lt->nitems++].pc = adr << 2; + lt->item[lt->nitems].set (lineno, adr << 2); + ++lt->nitems; return lineno; } \f diff --git a/gdb/mi/mi-symbol-cmds.c b/gdb/mi/mi-symbol-cmds.c index 223b8c3ce1..ca0fc379a4 100644 --- a/gdb/mi/mi-symbol-cmds.c +++ b/gdb/mi/mi-symbol-cmds.c @@ -34,6 +34,7 @@ mi_cmd_symbol_list_lines (const char *command, char **argv, int argc) struct symtab *s; int i; struct ui_out *uiout = current_uiout; + struct linetable *linetable; if (argc != 1) error (_("-symbol-list-lines: Usage: SOURCE_FILENAME")); @@ -51,11 +52,12 @@ mi_cmd_symbol_list_lines (const char *command, char **argv, int argc) gdbarch = get_objfile_arch (SYMTAB_OBJFILE (s)); ui_out_emit_list list_emitter (uiout, "lines"); - if (SYMTAB_LINETABLE (s) != NULL && SYMTAB_LINETABLE (s)->nitems > 0) - for (i = 0; i < SYMTAB_LINETABLE (s)->nitems; i++) + linetable = SYMTAB_LINETABLE (s); + if (linetable != NULL && linetable->nitems > 0) + for (i = 0; i < linetable->nitems; i++) { ui_out_emit_tuple tuple_emitter (uiout, NULL); - uiout->field_core_addr ("pc", gdbarch, SYMTAB_LINETABLE (s)->item[i].pc); - uiout->field_int ("line", SYMTAB_LINETABLE (s)->item[i].line); + uiout->field_core_addr ("pc", gdbarch, linetable->item[i].address (s)); + uiout->field_int ("line", linetable->item[i].line ()); } } diff --git a/gdb/objfiles.c b/gdb/objfiles.c index 98e81c48c4..0c1d3ffb57 100644 --- a/gdb/objfiles.c +++ b/gdb/objfiles.c @@ -799,23 +799,6 @@ objfile_relocate1 (struct objfile *objfile, /* OK, get all the symtabs. */ { struct compunit_symtab *cust; - struct symtab *s; - - ALL_OBJFILE_FILETABS (objfile, cust, s) - { - struct linetable *l; - int i; - - /* First the line table. */ - l = SYMTAB_LINETABLE (s); - if (l) - { - for (i = 0; i < l->nitems; ++i) - l->item[i].pc += ANOFFSET (delta, - COMPUNIT_BLOCK_LINE_SECTION - (cust)); - } - } ALL_OBJFILE_COMPUNITS (objfile, cust) { diff --git a/gdb/python/py-linetable.c b/gdb/python/py-linetable.c index c4e80d02a5..c17aac31e8 100644 --- a/gdb/python/py-linetable.c +++ b/gdb/python/py-linetable.c @@ -20,6 +20,7 @@ #include "defs.h" #include "python-internal.h" #include "py-ref.h" +#include "objfiles.h" typedef struct { PyObject_HEAD @@ -204,7 +205,7 @@ ltpy_has_line (PyObject *self, PyObject *args) for (index = 0; index < SYMTAB_LINETABLE (symtab)->nitems; index++) { struct linetable_entry *item = &(SYMTAB_LINETABLE (symtab)->item[index]); - if (item->line == py_line) + if (item->line () == py_line) Py_RETURN_TRUE; } @@ -242,9 +243,9 @@ ltpy_get_all_source_lines (PyObject *self, PyObject *args) /* 0 is used to signify end of line table information. Do not include in the source set. */ - if (item->line > 0) + if (item->line () > 0) { - gdbpy_ref<> line (gdb_py_object_from_longest (item->line)); + gdbpy_ref<> line (gdb_py_object_from_longest (item->line ())); if (line == NULL) return NULL; @@ -410,7 +411,7 @@ ltpy_iternext (PyObject *self) /* Skip over internal entries such as 0. 0 signifies the end of line table data and is not useful to the API user. */ - while (item->line < 1) + while (item->line () < 1) { iter_obj->current_index++; @@ -423,7 +424,7 @@ ltpy_iternext (PyObject *self) item = &(SYMTAB_LINETABLE (symtab)->item[iter_obj->current_index]); } - obj = build_linetable_entry (item->line, item->pc); + obj = build_linetable_entry (item->line (), item->address (symtab)); iter_obj->current_index++; return obj; diff --git a/gdb/record-btrace.c b/gdb/record-btrace.c index 7264b8ed2b..d09a4ad8cb 100644 --- a/gdb/record-btrace.c +++ b/gdb/record-btrace.c @@ -40,6 +40,7 @@ #include "inf-loop.h" #include "vec.h" #include <algorithm> +#include "objfiles.h" /* The target_ops of record-btrace. */ static struct target_ops record_btrace_ops; @@ -596,8 +597,8 @@ btrace_find_line_range (CORE_ADDR pc) range = btrace_mk_line_range (symtab, 0, 0); for (i = 0; i < nlines - 1; i++) { - if ((lines[i].pc == pc) && (lines[i].line != 0)) - range = btrace_line_range_add (range, lines[i].line); + if (lines[i].address (symtab) == pc && lines[i].line () != 0) + range = btrace_line_range_add (range, lines[i].line ()); } return range; diff --git a/gdb/symmisc.c b/gdb/symmisc.c index 9adde044cd..05348b880f 100644 --- a/gdb/symmisc.c +++ b/gdb/symmisc.c @@ -302,8 +302,9 @@ dump_symtab_1 (struct symtab *symtab, struct ui_file *outfile) len = l->nitems; for (i = 0; i < len; i++) { - fprintf_filtered (outfile, " line %d at ", l->item[i].line); - fputs_filtered (paddress (gdbarch, l->item[i].pc), outfile); + fprintf_filtered (outfile, " line %d at ", l->item[i].line ()); + fputs_filtered (paddress (gdbarch, l->item[i].address (symtab)), + outfile); fprintf_filtered (outfile, "\n"); } } @@ -1017,8 +1018,8 @@ maintenance_print_one_line_table (struct symtab *symtab, void *data) struct linetable_entry *item; item = &linetable->item [i]; - printf_filtered (_("%-6d %6d %s\n"), i, item->line, - core_addr_to_string (item->pc)); + printf_filtered (_("%-6d %6d %s\n"), i, item->line (), + core_addr_to_string (item->address (symtab))); } } diff --git a/gdb/symtab.c b/gdb/symtab.c index 2b1f9558ab..4baf57c755 100644 --- a/gdb/symtab.c +++ b/gdb/symtab.c @@ -287,6 +287,15 @@ search_domain_name (enum search_domain e) } } +CORE_ADDR +linetable_entry::address (struct symtab *symtab) const +{ + return gdbarch_addr_bits_remove + (get_objfile_arch (SYMTAB_OBJFILE (symtab)), + m_pc + ANOFFSET (SYMTAB_OBJFILE (symtab)->section_offsets, + SYMTAB_COMPUNIT (symtab)->block_line_section)); +} + /* See symtab.h. */ struct symtab * @@ -3070,6 +3079,7 @@ find_pc_sect_line (CORE_ADDR pc, struct obj_section *section, int notcurrent) we will use a line one less than this, with a range from the start of that file to the first line's pc. */ struct linetable_entry *alt = NULL; + struct symtab *alt_symtab = NULL; /* Info on best line seen in this file. */ @@ -3209,13 +3219,17 @@ find_pc_sect_line (CORE_ADDR pc, struct obj_section *section, int notcurrent) /* Is this file's first line closer than the first lines of other files? If so, record this file, and its first line, as best alternate. */ - if (item->pc > pc && (!alt || item->pc < alt->pc)) - alt = item; + if (item->address (iter_s) > pc + && (!alt || (item->address (iter_s) < alt->address (alt_symtab)))) + { + alt = item; + alt_symtab = iter_s; + } - auto pc_compare = [](const CORE_ADDR & pc, - const struct linetable_entry & lhs)->bool + auto pc_compare = [=](const CORE_ADDR & pc, + const struct linetable_entry & lhs)->bool { - return pc < lhs.pc; + return pc < lhs.address (iter_s); }; struct linetable_entry *first = item; @@ -3234,22 +3248,25 @@ find_pc_sect_line (CORE_ADDR pc, struct obj_section *section, int notcurrent) save prev if it represents the end of a function (i.e. line number 0) instead of a real line. */ - if (prev && prev->line && (!best || prev->pc > best->pc)) + if (prev + && prev->line () + && (!best || (prev->address (iter_s) > best->address (best_symtab)))) { best = prev; best_symtab = iter_s; /* Discard BEST_END if it's before the PC of the current BEST. */ - if (best_end <= best->pc) + if (best_end <= best->address (best_symtab)) best_end = 0; } /* If another line (denoted by ITEM) is in the linetable and its PC is after BEST's PC, but before the current BEST_END, then use ITEM's PC as the new best_end. */ - if (best && item < last && item->pc > best->pc - && (best_end == 0 || best_end > item->pc)) - best_end = item->pc; + if (best && item < last + && (item->address (iter_s) > best->address (best_symtab)) + && (best_end == 0 || best_end > item->address (iter_s))) + best_end = item->address (iter_s); } if (!best_symtab) @@ -3260,7 +3277,7 @@ find_pc_sect_line (CORE_ADDR pc, struct obj_section *section, int notcurrent) don't make some up. */ val.pc = pc; } - else if (best->line == 0) + else if (best->line () == 0) { /* If our best fit is in a range of PC's for which no line number info is available (line number is zero) then we didn't @@ -3270,12 +3287,12 @@ find_pc_sect_line (CORE_ADDR pc, struct obj_section *section, int notcurrent) else { val.symtab = best_symtab; - val.line = best->line; - val.pc = best->pc; - if (best_end && (!alt || best_end < alt->pc)) + val.line = best->line (); + val.pc = best->address (best_symtab); + if (best_end && (!alt || best_end < alt->address (alt_symtab))) val.end = best_end; else if (alt) - val.end = alt->pc; + val.end = alt->address (alt_symtab); else val.end = BLOCK_END (BLOCKVECTOR_BLOCK (bv, GLOBAL_BLOCK)); } @@ -3355,7 +3372,7 @@ find_line_symtab (struct symtab *symtab, int line, struct symtab *s; if (best_index >= 0) - best = best_linetable->item[best_index].line; + best = best_linetable->item[best_index].line (); else best = 0; @@ -3387,9 +3404,9 @@ find_line_symtab (struct symtab *symtab, int line, best_symtab = s; goto done; } - if (best == 0 || l->item[ind].line < best) + if (best == 0 || l->item[ind].line () < best) { - best = l->item[ind].line; + best = l->item[ind].line (); best_index = ind; best_linetable = l; best_symtab = s; @@ -3419,6 +3436,7 @@ find_pcs_for_symtab_line (struct symtab *symtab, int line, { int start = 0; std::vector<CORE_ADDR> result; + struct linetable *linetable = SYMTAB_LINETABLE (symtab); /* First, collect all the PCs that are at this line. */ while (1) @@ -3426,22 +3444,21 @@ find_pcs_for_symtab_line (struct symtab *symtab, int line, int was_exact; int idx; - idx = find_line_common (SYMTAB_LINETABLE (symtab), line, &was_exact, - start); + idx = find_line_common (linetable, line, &was_exact, start); if (idx < 0) break; if (!was_exact) { - struct linetable_entry *item = &SYMTAB_LINETABLE (symtab)->item[idx]; + struct linetable_entry *item = &linetable->item[idx]; - if (*best_item == NULL || item->line < (*best_item)->line) + if (*best_item == NULL || item->line () < (*best_item)->line ()) *best_item = item; break; } - result.push_back (SYMTAB_LINETABLE (symtab)->item[idx].pc); + result.push_back (linetable->item[idx].address (symtab)); start = idx + 1; } @@ -3467,7 +3484,7 @@ find_line_pc (struct symtab *symtab, int line, CORE_ADDR *pc) if (symtab != NULL) { l = SYMTAB_LINETABLE (symtab); - *pc = l->item[ind].pc; + *pc = l->item[ind].address (symtab); return 1; } else @@ -3546,16 +3563,16 @@ find_line_common (struct linetable *l, int lineno, { struct linetable_entry *item = &(l->item[i]); - if (item->line == lineno) + 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)) + if (item->line () > lineno && (best == 0 || item->line () < best)) { - best = item->line; + best = item->line (); best_index = i; } } @@ -3656,8 +3673,10 @@ skip_prologue_using_lineinfo (CORE_ADDR func_addr, struct symtab *symtab) /* Don't use line numbers of zero, they mark special entries in the table. See the commentary on symtab.h before the definition of struct linetable. */ - if (item->line > 0 && func_start <= item->pc && item->pc < func_end) - return item->pc; + if (item->line () > 0 + && func_start <= item->address (symtab) + && item->address (symtab) < func_end) + return item->address (symtab); } return func_addr; @@ -3878,13 +3897,15 @@ skip_prologue_using_sal (struct gdbarch *gdbarch, CORE_ADDR func_addr) /* Skip any earlier lines, and any end-of-sequence marker from a previous function. */ - while (linetable->item[idx].pc != prologue_sal.pc - || linetable->item[idx].line == 0) + while ((linetable->item[idx].address (prologue_sal.symtab) + != prologue_sal.pc) + || linetable->item[idx].line () == 0) idx++; if (idx+1 < linetable->nitems - && linetable->item[idx+1].line != 0 - && linetable->item[idx+1].pc == start_pc) + && linetable->item[idx+1].line () != 0 + && (linetable->item[idx+1].address (prologue_sal.symtab) + == start_pc)) return start_pc; } diff --git a/gdb/symtab.h b/gdb/symtab.h index f9d52e7697..ebb21abbd9 100644 --- a/gdb/symtab.h +++ b/gdb/symtab.h @@ -29,6 +29,7 @@ #include "common/function-view.h" #include "common/gdb_optional.h" #include "completer.h" +#include "gdbarch.h" /* Opaque declarations. */ struct ui_file; @@ -1227,8 +1228,45 @@ struct rust_vtable_symbol : public symbol struct linetable_entry { - int line; - CORE_ADDR pc; + /* Set the members of this object. */ + void set (int line_, CORE_ADDR pc) + { + m_line = line_; + m_pc = pc; + } + + /* Set the members of this object from another linetable_entry. */ + void set (const linetable_entry &other) + { + m_line = other.m_line; + m_pc = other.m_pc; + } + + /* Return the raw address. */ + CORE_ADDR raw_address () const + { + return m_pc; + } + + /* Return the relocated address, using the offsets provided by + SYMTAB. */ + CORE_ADDR address (struct symtab *symtab) const; + + /* Return the line number. */ + int line () const + { + return m_line; + } + +private: + + int m_line; + + /* Note that the PC as stored is unrelocated. The appropriate + offset must be applied before it can be used; see the "address" + method. */ + CORE_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 8c707aa8fe..880d87436e 100644 --- a/gdb/xcoffread.c +++ b/gdb/xcoffread.c @@ -420,7 +420,11 @@ compare_lte (const void *lte1p, const void *lte2p) struct linetable_entry *lte1 = (struct linetable_entry *) lte1p; struct linetable_entry *lte2 = (struct linetable_entry *) lte2p; - return lte1->pc - lte2->pc; + if (lte1->raw_address () < lte2->raw_address ()) + return -1; + if (lte1->raw_address () > lte2->raw_address ()) + return -1; + return 0; } /* Given a line table with function entries are marked, arrange its @@ -448,7 +452,7 @@ arrange_linetable (struct linetable *oldLineTb) for (function_count = 0, ii = 0; ii < oldLineTb->nitems; ++ii) { - if (oldLineTb->item[ii].line == 0) + if (oldLineTb->item[ii].line () == 0) { /* Function entry found. */ if (function_count >= fentry_size) { /* Make sure you have room. */ @@ -457,14 +461,15 @@ arrange_linetable (struct linetable *oldLineTb) xrealloc (fentry, fentry_size * sizeof (struct linetable_entry)); } - fentry[function_count].line = ii; - fentry[function_count].pc = oldLineTb->item[ii].pc; + fentry[function_count].set (ii, + oldLineTb->item[ii].raw_address ()); ++function_count; /* If the function was compiled with XLC, we may have to add an extra line entry later. Reserve space for that. */ if (ii + 1 < oldLineTb->nitems - && oldLineTb->item[ii].pc != oldLineTb->item[ii + 1].pc) + && (oldLineTb->item[ii].raw_address () + != oldLineTb->item[ii + 1].raw_address ())) extra_lines++; } } @@ -488,10 +493,11 @@ arrange_linetable (struct linetable *oldLineTb) a function begin. */ newline = 0; - if (oldLineTb->item[0].line != 0) + if (oldLineTb->item[0].line () != 0) for (newline = 0; - newline < oldLineTb->nitems && oldLineTb->item[newline].line; ++newline) - newLineTb->item[newline] = oldLineTb->item[newline]; + newline < oldLineTb->nitems && oldLineTb->item[newline].line (); + ++newline) + newLineTb->item[newline].set (oldLineTb->item[newline]); /* Now copy function lines one by one. */ @@ -499,19 +505,20 @@ arrange_linetable (struct linetable *oldLineTb) { /* If the function was compiled with XLC, we may have to add an extra line to cover the function prologue. */ - jj = fentry[ii].line; + jj = fentry[ii].line (); if (jj + 1 < oldLineTb->nitems - && oldLineTb->item[jj].pc != oldLineTb->item[jj + 1].pc) + && (oldLineTb->item[jj].raw_address () + != oldLineTb->item[jj + 1].raw_address ())) { - newLineTb->item[newline] = oldLineTb->item[jj]; - newLineTb->item[newline].line = oldLineTb->item[jj + 1].line; + newLineTb->item[newline].set (oldLineTb->item[jj].raw_address (), + oldLineTb->item[jj + 1].line ()); newline++; } - for (jj = fentry[ii].line + 1; - jj < oldLineTb->nitems && oldLineTb->item[jj].line != 0; + for (jj = fentry[ii].line () + 1; + jj < oldLineTb->nitems && oldLineTb->item[jj].line () != 0; ++jj, ++newline) - newLineTb->item[newline] = oldLineTb->item[jj]; + newLineTb->item[newline].set (oldLineTb->item[jj]); } xfree (fentry); /* The number of items in the line table must include these @@ -871,6 +878,8 @@ enter_line_range (struct subfile *subfile, unsigned beginoffset, while (curoffset <= limit_offset) { + CORE_ADDR relocated; + bfd_seek (abfd, curoffset, SEEK_SET); bfd_bread (ext_lnno, linesz, abfd); bfd_coff_swap_lineno_in (abfd, ext_lnno, &int_lnno); @@ -879,20 +888,20 @@ enter_line_range (struct subfile *subfile, unsigned beginoffset, addr = (int_lnno.l_lnno ? int_lnno.l_addr.l_paddr : read_symbol_nvalue (int_lnno.l_addr.l_symndx)); - addr += ANOFFSET (objfile->section_offsets, SECT_OFF_TEXT (objfile)); - if (addr < startaddr || (endaddr && addr >= endaddr)) + relocated = (addr + ANOFFSET (objfile->section_offsets, + SECT_OFF_TEXT (objfile))); + if (relocated < startaddr || (endaddr && relocated >= endaddr)) return; if (int_lnno.l_lnno == 0) { *firstLine = read_symbol_lineno (int_lnno.l_addr.l_symndx); - record_line (subfile, 0, gdbarch_addr_bits_remove (gdbarch, addr)); + record_line (subfile, 0, addr); --(*firstLine); } else - record_line (subfile, *firstLine + int_lnno.l_lnno, - gdbarch_addr_bits_remove (gdbarch, addr)); + record_line (subfile, *firstLine + int_lnno.l_lnno, addr); curoffset += linesz; } } ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [RFA 1/2] Make line tables independent of progspace 2018-03-27 4:53 ` Tom Tromey @ 2018-03-27 20:22 ` Simon Marchi 2018-03-28 4:53 ` Tom Tromey 2018-03-28 3:34 ` Simon Marchi 1 sibling, 1 reply; 18+ messages in thread From: Simon Marchi @ 2018-03-27 20:22 UTC (permalink / raw) To: Tom Tromey; +Cc: Simon Marchi, gdb-patches On 2018-03-27 12:53 AM, Tom Tromey wrote: > Tom> I've run the new patch through the buildbot. I'll resubmit it soon, I > Tom> just have to update the ChangeLog and rewrite the commit message. > > Here it is. Let me reiterate that this probably requires careful review > as the buildbot may not test some of these paths. > > Tom Hi Tom, I had a first quick look, enough to familiarize myself with the problem. Conceptually, the approach looks ok to me, but I'd like to have a closer look later (unless someone else reviews it in the mean time). Here are two small comments I noted. > @@ -517,10 +522,12 @@ do_mixed_source_and_assembly (struct gdbarch *gdbarch, > first_le = NULL; > > /* Skip all the preceding functions. */ > - for (i = 0; i < nlines && le[i].pc < low; i++) > - continue; > + for (i = 0; > + i < nlines - 1 && le[i].address (main_symtab) < low; I don't understand why "i < nlines" becomes "i < nlines - 1" here. > diff --git a/gdb/xcoffread.c b/gdb/xcoffread.c > index 8c707aa8fe..880d87436e 100644 > --- a/gdb/xcoffread.c > +++ b/gdb/xcoffread.c > @@ -420,7 +420,11 @@ compare_lte (const void *lte1p, const void *lte2p) > struct linetable_entry *lte1 = (struct linetable_entry *) lte1p; > struct linetable_entry *lte2 = (struct linetable_entry *) lte2p; > > - return lte1->pc - lte2->pc; > + if (lte1->raw_address () < lte2->raw_address ()) > + return -1; > + if (lte1->raw_address () > lte2->raw_address ()) > + return -1; > + return 0; > } Both branches return -1 here. I don't mind the "left - right", I see that often. Especially when you want to sort by multiple fields, it's short and clear (IMO) to do if (left->foo != right->foo) return left->foo - right->foo; return left->bar - right->bar; Simon ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [RFA 1/2] Make line tables independent of progspace 2018-03-27 20:22 ` Simon Marchi @ 2018-03-28 4:53 ` Tom Tromey 2018-03-28 12:30 ` Simon Marchi 0 siblings, 1 reply; 18+ messages in thread From: Tom Tromey @ 2018-03-28 4:53 UTC (permalink / raw) To: Simon Marchi; +Cc: Tom Tromey, Simon Marchi, gdb-patches >>>>> "Simon" == Simon Marchi <simon.marchi@ericsson.com> writes: Simon> Here are two small comments I noted. Thank you. >> @@ -517,10 +522,12 @@ do_mixed_source_and_assembly (struct gdbarch *gdbarch, >> first_le = NULL; >> >> /* Skip all the preceding functions. */ >> - for (i = 0; i < nlines && le[i].pc < low; i++) >> - continue; >> + for (i = 0; >> + i < nlines - 1 && le[i].address (main_symtab) < low; Simon> I don't understand why "i < nlines" becomes "i < nlines - 1" here. Bad change on my part. >> + if (lte1->raw_address () < lte2->raw_address ()) >> + return -1; >> + if (lte1->raw_address () > lte2->raw_address ()) >> + return -1; >> + return 0; Simon> Both branches return -1 here. Ugh, sorry. This got introduced during the changes to use member functions. Simon> I don't mind the "left - right", I see that often. Especially when you Simon> want to sort by multiple fields, it's short and clear (IMO) to do In this particular case I think it is a bit weird, because the addresses are unsigned, so this is relying on the implicit cast to make the value negative. But also, because the addresses are CORE_ADDR and the return type is int, it seems like underflow is possible as well (in theory, I suppose I wouldn't expect it in practice). So, I thought rather than being tricky here, it was more obviously correct to just write the longer form. Which would be true if it didn't introduce bugs. Tom ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [RFA 1/2] Make line tables independent of progspace 2018-03-28 4:53 ` Tom Tromey @ 2018-03-28 12:30 ` Simon Marchi 0 siblings, 0 replies; 18+ messages in thread From: Simon Marchi @ 2018-03-28 12:30 UTC (permalink / raw) To: Tom Tromey, Simon Marchi; +Cc: gdb-patches On 2018-03-28 12:53 AM, Tom Tromey wrote: > Simon> Both branches return -1 here. > > Ugh, sorry. This got introduced during the changes to use member > functions. > > Simon> I don't mind the "left - right", I see that often. Especially when you > Simon> want to sort by multiple fields, it's short and clear (IMO) to do > > In this particular case I think it is a bit weird, because the addresses > are unsigned, so this is relying on the implicit cast to make the value > negative. But also, because the addresses are CORE_ADDR and the return > type is int, it seems like underflow is possible as well (in theory, I > suppose I wouldn't expect it in practice). > > So, I thought rather than being tricky here, it was more obviously > correct to just write the longer form. Which would be true if it didn't > introduce bugs. Ah, I thought that subtracting two unsigned yielded a signed result, like subtracting two pointers gives a ptrdiff_t result (which is signed). But that doesn't seem true. So I'm fine with changing it then. ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [RFA 1/2] Make line tables independent of progspace 2018-03-27 4:53 ` Tom Tromey 2018-03-27 20:22 ` Simon Marchi @ 2018-03-28 3:34 ` Simon Marchi 2018-03-28 5:02 ` Tom Tromey 2018-03-28 19:33 ` Maciej W. Rozycki 1 sibling, 2 replies; 18+ messages in thread From: Simon Marchi @ 2018-03-28 3:34 UTC (permalink / raw) To: Tom Tromey; +Cc: gdb-patches, macro On 2018-03-27 12:53 AM, Tom Tromey wrote: > Tom> I've run the new patch through the buildbot. I'll resubmit it soon, I > Tom> just have to update the ChangeLog and rewrite the commit message. > > Here it is. Let me reiterate that this probably requires careful review > as the buildbot may not test some of these paths. Hi again, I took another look. For the old debug info formats, it's really hard to tell if the code is right, since we don't have automated tests or even a manual way to test, for most of them. But I don't think that should hinder improvements to the formats that are actually used today. I think the changes you did are a good "best effort". If we judge that the old formats are not useful anymore and have most likely bit-rotten with time, we should think about removing them eventually. Anyway, I just have some small comments here, otherwise the patch LGTM. Maybe it would be a good idea to get the opinion of Maciej (in CC) for the addr_bits_remove part? > @@ -3209,13 +3219,17 @@ find_pc_sect_line (CORE_ADDR pc, struct obj_section *section, int notcurrent) > > /* Is this file's first line closer than the first lines of other files? > If so, record this file, and its first line, as best alternate. */ > - if (item->pc > pc && (!alt || item->pc < alt->pc)) > - alt = item; > + if (item->address (iter_s) > pc > + && (!alt || (item->address (iter_s) < alt->address (alt_symtab)))) > + { > + alt = item; > + alt_symtab = iter_s; > + } > > - auto pc_compare = [](const CORE_ADDR & pc, > - const struct linetable_entry & lhs)->bool > + auto pc_compare = [=](const CORE_ADDR & pc, > + const struct linetable_entry & lhs)->bool > { > - return pc < lhs.pc; > + return pc < lhs.address (iter_s); > }; Since we know this will be called many times and address() is substantially more costly than just reading a CORE_ADDR field, maybe it would be good to save it to a variable before and use that in the lambda. I was also thinking the same thing for the various other calls to address() in this function, since sometimes the same value is recomputed multiple times. For example, "item->address (iter_s)" in if (best && item < last && (item->address (iter_s) > best->address (best_symtab)) && (best_end == 0 || best_end > item->address (iter_s))) best_end = item->address (iter_s); > @@ -1227,8 +1228,45 @@ struct rust_vtable_symbol : public symbol > > struct linetable_entry > { > - int line; > - CORE_ADDR pc; > + /* Set the members of this object. */ > + void set (int line_, CORE_ADDR pc) > + { > + m_line = line_; > + m_pc = pc; > + } > + > + /* Set the members of this object from another linetable_entry. */ > + void set (const linetable_entry &other) > + { > + m_line = other.m_line; > + m_pc = other.m_pc; > + } This really looks like an assignment operator (the default one would be enough). Is there a reason to have this set overload? Either way works, I'm just curious. > @@ -499,19 +505,20 @@ arrange_linetable (struct linetable *oldLineTb) > { > /* If the function was compiled with XLC, we may have to add an > extra line to cover the function prologue. */ > - jj = fentry[ii].line; > + jj = fentry[ii].line (); > if (jj + 1 < oldLineTb->nitems > - && oldLineTb->item[jj].pc != oldLineTb->item[jj + 1].pc) > + && (oldLineTb->item[jj].raw_address () > + != oldLineTb->item[jj + 1].raw_address ())) > { > - newLineTb->item[newline] = oldLineTb->item[jj]; > - newLineTb->item[newline].line = oldLineTb->item[jj + 1].line; > + newLineTb->item[newline].set (oldLineTb->item[jj].raw_address (), > + oldLineTb->item[jj + 1].line ()); The address and the line are inverted here. Thanks, Simon ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [RFA 1/2] Make line tables independent of progspace 2018-03-28 3:34 ` Simon Marchi @ 2018-03-28 5:02 ` Tom Tromey 2018-03-28 12:32 ` Simon Marchi 2018-03-28 19:33 ` Maciej W. Rozycki 1 sibling, 1 reply; 18+ messages in thread From: Tom Tromey @ 2018-03-28 5:02 UTC (permalink / raw) To: Simon Marchi; +Cc: Tom Tromey, gdb-patches, macro >>>>> "Simon" == Simon Marchi <simark@simark.ca> writes: >> + auto pc_compare = [=](const CORE_ADDR & pc, >> + const struct linetable_entry & lhs)->bool >> { >> - return pc < lhs.pc; >> + return pc < lhs.address (iter_s); >> }; Simon> Since we know this will be called many times and address() is substantially Simon> more costly than just reading a CORE_ADDR field, maybe it would be good to Simon> save it to a variable before and use that in the lambda. I am not sure this would work, because LHS is what changes here -- std::upper_bound is searching through the line table looking for a match. One idea might be to "unrelocate" PC to do the search. This would make the search more efficient. I will try this. Simon> I was also thinking Simon> the same thing for the various other calls to address() in this function, since Simon> sometimes the same value is recomputed multiple times. I suppose most work could be done on the unrelocated form, with the address computation happening only at the end. >> + /* Set the members of this object from another linetable_entry. */ >> + void set (const linetable_entry &other) >> + { >> + m_line = other.m_line; >> + m_pc = other.m_pc; >> + } Simon> This really looks like an assignment operator (the default one would be Simon> enough). Is there a reason to have this set overload? Either way works, Simon> I'm just curious. I just did it to keep the names consistent, but I will change it to use the default operator=. >> - newLineTb->item[newline] = oldLineTb->item[jj]; >> - newLineTb->item[newline].line = oldLineTb->item[jj + 1].line; >> + newLineTb->item[newline].set (oldLineTb->item[jj].raw_address (), >> + oldLineTb->item[jj + 1].line ()); Simon> The address and the line are inverted here. Bleah. Thanks again. Tom ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [RFA 1/2] Make line tables independent of progspace 2018-03-28 5:02 ` Tom Tromey @ 2018-03-28 12:32 ` Simon Marchi 0 siblings, 0 replies; 18+ messages in thread From: Simon Marchi @ 2018-03-28 12:32 UTC (permalink / raw) To: Tom Tromey; +Cc: gdb-patches, macro On 2018-03-28 01:02 AM, Tom Tromey wrote: >>>>>> "Simon" == Simon Marchi <simark@simark.ca> writes: > >>> + auto pc_compare = [=](const CORE_ADDR & pc, >>> + const struct linetable_entry & lhs)->bool >>> { >>> - return pc < lhs.pc; >>> + return pc < lhs.address (iter_s); >>> }; > > Simon> Since we know this will be called many times and address() is substantially > Simon> more costly than just reading a CORE_ADDR field, maybe it would be good to > Simon> save it to a variable before and use that in the lambda. > > I am not sure this would work, because LHS is what changes here -- > std::upper_bound is searching through the line table looking for a > match. > > One idea might be to "unrelocate" PC to do the search. This would make > the search more efficient. I will try this. Ah sorry, you are right. Simon ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [RFA 1/2] Make line tables independent of progspace 2018-03-28 3:34 ` Simon Marchi 2018-03-28 5:02 ` Tom Tromey @ 2018-03-28 19:33 ` Maciej W. Rozycki 2018-03-29 15:04 ` Simon Marchi 1 sibling, 1 reply; 18+ messages in thread From: Maciej W. Rozycki @ 2018-03-28 19:33 UTC (permalink / raw) To: Simon Marchi; +Cc: Tom Tromey, gdb-patches On Tue, 27 Mar 2018, Simon Marchi wrote: > Anyway, I just have some small comments here, otherwise the patch LGTM. Maybe > it would be a good idea to get the opinion of Maciej (in CC) for the > addr_bits_remove part? It's not clear to me offhand what the impact on processing that involves `addr_bits_remove' the change proposed will have. So what specifically would you like me to comment on here? As to the MIPS handler it seems ill-conceived to me. On 64-bit MIPS hardware 32-bit addresses are architecturally sign-extended, so assuming that the comment in `mips_addr_bits_remove' is correct any workaround for inconsistent handling should have been made the other way round, that is by sign-extending rather than zero-extending addresses. This hack was introduced with: commit 96431497ff2c780978d95f5fda7148ba732dcae0 Author: Mark Alexander <marka@cygnus> Date: Wed Nov 27 03:40:02 1996 +0000 and then the default for whether to mask was changed to off with: commit 4014092b582281f6db1c43f618d3722561fb01e6 Author: Andrew Cagney <cagney@redhat.com> Date: Tue Jul 11 09:25:22 2000 +0000 I'm tempted to remove the hack altogether as nowadays we should be getting the sign-extension right for 32-bit addresses everywhere. I know of a single case of hardware, the Sony R5900 processor (used in the PS2 video game console), which is 64-bit but has addressing limited to 32 bits and, contrary to what the MIPS architecture requires, does not sign-extend the PC correctly. This is only observable with the link register (typically $ra) and the JAL/JALR/BGEZAL/BGEZALL/BLTZAL/BLTZALL instructions that copy the value of the PC there. And then only in bare metal debugging, as bit #31 is only set for kernel mode addresses. I don't think any bare metal debugging facility is available for that processor though, and if one turns up, then we can think how to deal with that. FWIW, Maciej ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [RFA 1/2] Make line tables independent of progspace 2018-03-28 19:33 ` Maciej W. Rozycki @ 2018-03-29 15:04 ` Simon Marchi 2018-03-29 21:07 ` Maciej W. Rozycki 0 siblings, 1 reply; 18+ messages in thread From: Simon Marchi @ 2018-03-29 15:04 UTC (permalink / raw) To: Maciej W. Rozycki, Simon Marchi; +Cc: Tom Tromey, gdb-patches On 2018-03-28 03:27 PM, Maciej W. Rozycki wrote: > It's not clear to me offhand what the impact on processing that involves > `addr_bits_remove' the change proposed will have. So what specifically > would you like me to comment on here? Err sorry I wasn't very clear. Currently, for addresses coming from the line table, gdbarch_addr_bits_remove is called on the relocated address in dwarf_record_line_1, before that address is put in the line table data structure. In Tom's patch, since we store unrelocated addresses in the line table, relocation happens "just in time" when reading the address from the table. gdbarch_addr_bits_remove is called at this time: CORE_ADDR linetable_entry::address (struct symtab *symtab) const { return gdbarch_addr_bits_remove (get_objfile_arch (SYMTAB_OBJFILE (symtab)), m_pc + ANOFFSET (SYMTAB_OBJFILE (symtab)->section_offsets, SYMTAB_COMPUNIT (symtab)->block_line_section)); } I just wanted to confirm it was still fine with mips' implementation of addr_bits_remove. But now I realize that other arches implement this callback too, not only mips. I think I originally meant to ask about gdbarch_adjust_dwarf2_line, which is only implemented by mips. Since adjust_dwarf2_line used to be passed the relocated address, Tom included a "hack" where he relocates the address, passes it through gdbarch_adjust_dwarf2_line, and unrelocates it: /* Handle DW_LNE_set_address. */ void handle_set_address (CORE_ADDR baseaddr, CORE_ADDR address) { m_op_index = 0; /* Pass the relocated address to the gdbarch for modification, but then only store the relative address. */ address += baseaddr; m_address = (gdbarch_adjust_dwarf2_line (m_gdbarch, address, false) - baseaddr); } I think right now this is necessary, because mips_adjust_dwarf2_line does a minimal symbol lookup, which are relocated. So if we passed an unrelocated address, I guess it wouldn't find the symbol. I just wanted to make sure there wasn't anything you saw that is obviously wrong with this approach. I think that eventually we'll be able to pass an unrelocated address to adjust_dwarf2_line, but in the mean time we probably need this hack. Simon ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [RFA 1/2] Make line tables independent of progspace 2018-03-29 15:04 ` Simon Marchi @ 2018-03-29 21:07 ` Maciej W. Rozycki 2018-04-26 21:30 ` Tom Tromey 0 siblings, 1 reply; 18+ messages in thread From: Maciej W. Rozycki @ 2018-03-29 21:07 UTC (permalink / raw) To: Simon Marchi; +Cc: Simon Marchi, Tom Tromey, Yao Qi, gdb-patches On Thu, 29 Mar 2018, Simon Marchi wrote: > Since adjust_dwarf2_line used to be passed the relocated address, Tom included a > "hack" where he relocates the address, passes it through gdbarch_adjust_dwarf2_line, > and unrelocates it: > > /* Handle DW_LNE_set_address. */ > void handle_set_address (CORE_ADDR baseaddr, CORE_ADDR address) > { > m_op_index = 0; > /* Pass the relocated address to the gdbarch for modification, but > then only store the relative address. */ > address += baseaddr; > m_address = (gdbarch_adjust_dwarf2_line (m_gdbarch, address, false) > - baseaddr); > } > > I think right now this is necessary, because mips_adjust_dwarf2_line does a > minimal symbol lookup, which are relocated. So if we passed an unrelocated > address, I guess it wouldn't find the symbol. Yeah, I guess it's all right, the difference will ever be either 1 or 0. I would pass it through MIPS16 and/or microMIPS regression testing to be sure. We have the Good Friday public holiday tomorrow here in the UK and then Easter Monday, so I won't be able to schedule a test run until sometime next week though. > I just wanted to make sure there wasn't anything you saw that is obviously > wrong with this approach. I think that eventually we'll be able to pass an > unrelocated address to adjust_dwarf2_line, but in the mean time we probably > need this hack. This piece and the associated infrastructure is scheduled to go once Yao has sorted the issue with function descriptions and his proposed change: <https://sourceware.org/ml/gdb-patches/2016-10/msg00430.html>, when we'll be able to reduce the amount of hackery required to handle the broken debug record generation for compressed MIPS code (where the ISA bit is set in line records, but not in any other debug records). At that point we'll have to have a hook to remove the ISA bit from line entries instead though; perhaps that'll be the very `gdbarch_addr_bits_remove'. Maciej ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [RFA 1/2] Make line tables independent of progspace 2018-03-29 21:07 ` Maciej W. Rozycki @ 2018-04-26 21:30 ` Tom Tromey 2018-05-03 23:17 ` Maciej W. Rozycki 0 siblings, 1 reply; 18+ messages in thread From: Tom Tromey @ 2018-04-26 21:30 UTC (permalink / raw) To: Maciej W. Rozycki Cc: Simon Marchi, Simon Marchi, Tom Tromey, Yao Qi, gdb-patches >>>>> "Maciej" == Maciej W Rozycki <macro@mips.com> writes: >> Since adjust_dwarf2_line used to be passed the relocated address, Tom included a >> "hack" where he relocates the address, passes it through gdbarch_adjust_dwarf2_line, >> and unrelocates it: [...] Maciej> Yeah, I guess it's all right, the difference will ever be either 1 or 0. Maciej> I would pass it through MIPS16 and/or microMIPS regression testing to be Maciej> sure. We have the Good Friday public holiday tomorrow here in the UK and Maciej> then Easter Monday, so I won't be able to schedule a test run until Maciej> sometime next week though. Did you do this? Either way, my understanding of this thread was that I needn't change anything about adjust_dwarf2_line yet, and that I can go forward with the line table patches. So, I am going to send a new version. If that's in error please let me know. Tom ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [RFA 1/2] Make line tables independent of progspace 2018-04-26 21:30 ` Tom Tromey @ 2018-05-03 23:17 ` Maciej W. Rozycki 2018-05-14 23:54 ` Maciej W. Rozycki 0 siblings, 1 reply; 18+ messages in thread From: Maciej W. Rozycki @ 2018-05-03 23:17 UTC (permalink / raw) To: Tom Tromey; +Cc: Simon Marchi, Simon Marchi, Yao Qi, gdb-patches Hi Tom, > >>>>> "Maciej" == Maciej W Rozycki <macro@mips.com> writes: > > >> Since adjust_dwarf2_line used to be passed the relocated address, Tom included a > >> "hack" where he relocates the address, passes it through gdbarch_adjust_dwarf2_line, > >> and unrelocates it: > [...] > Maciej> Yeah, I guess it's all right, the difference will ever be either 1 or 0. > > Maciej> I would pass it through MIPS16 and/or microMIPS regression testing to be > Maciej> sure. We have the Good Friday public holiday tomorrow here in the UK and > Maciej> then Easter Monday, so I won't be able to schedule a test run until > Maciej> sometime next week though. > > Did you do this? I didn't, sorry, recent weeks have been rather hectic here and I was taken away to do other stuff. I'll try to schedule regression-testing over the next few days. Maciej ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [RFA 1/2] Make line tables independent of progspace 2018-05-03 23:17 ` Maciej W. Rozycki @ 2018-05-14 23:54 ` Maciej W. Rozycki 0 siblings, 0 replies; 18+ messages in thread From: Maciej W. Rozycki @ 2018-05-14 23:54 UTC (permalink / raw) To: Tom Tromey; +Cc: Simon Marchi, Simon Marchi, Yao Qi, gdb-patches Hi Tom, > > Did you do this? > > I didn't, sorry, recent weeks have been rather hectic here and I was > taken away to do other stuff. I'll try to schedule regression-testing > over the next few days. I have now completed MIPS16 testing with no issues observed using v3 of your change. I have troubles chasing a microMIPS board I could use for testing right now, so I think this will have to suffice. Again, apologies for taking so long. Maciej ^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2018-05-14 17:59 UTC | newest] Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-03-21 17:18 [RFA 0/2] Make line tables independent of progspace Tom Tromey 2018-03-21 17:18 ` [RFA 2/2] Constify the line table Tom Tromey 2018-03-21 17:18 ` [RFA 1/2] Make line tables independent of progspace Tom Tromey 2018-03-24 3:40 ` Simon Marchi 2018-03-27 4:16 ` Tom Tromey 2018-03-27 4:53 ` Tom Tromey 2018-03-27 20:22 ` Simon Marchi 2018-03-28 4:53 ` Tom Tromey 2018-03-28 12:30 ` Simon Marchi 2018-03-28 3:34 ` Simon Marchi 2018-03-28 5:02 ` Tom Tromey 2018-03-28 12:32 ` Simon Marchi 2018-03-28 19:33 ` Maciej W. Rozycki 2018-03-29 15:04 ` Simon Marchi 2018-03-29 21:07 ` Maciej W. Rozycki 2018-04-26 21:30 ` Tom Tromey 2018-05-03 23:17 ` Maciej W. Rozycki 2018-05-14 23:54 ` Maciej W. Rozycki
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).