public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH 0/5] Make line tables independent of objfile
@ 2023-03-08 15:42 Tom Tromey
  2023-03-08 15:42 ` [PATCH 1/5] Add operator< and operator== to linetable_entry Tom Tromey
                   ` (5 more replies)
  0 siblings, 6 replies; 20+ messages in thread
From: Tom Tromey @ 2023-03-08 15:42 UTC (permalink / raw)
  To: gdb-patches

This is a tiny step toward the long-term goal of "objfile splitting"
-- that is, making symbol tables independent of the objfile.  This
goal, if ever achieved, would allow for sharing of symbols and symbol
tables across inferiors.

The main issue in this splitting is to ensure that runtime offsets
aren't part of symbols.  This series applies this change to line
tables.

Regression tested on x86-64 Fedora 36.

Tom

---
Tom Tromey (5):
      Add operator< and operator== to linetable_entry
      Change linetables to be objfile-independent
      Constify linetables
      Remove extra scopes from objfile_relocate1
      Change linetable_entry::is_stmt to bool

 gdb/buildsym.c            |  30 +++++--------
 gdb/coffread.c            |   1 -
 gdb/dbxread.c             |   9 ++--
 gdb/disasm.c              |  31 +++++++++-----
 gdb/dwarf2/read.c         |   5 +--
 gdb/jit.c                 |  12 +++---
 gdb/linespec.c            |   6 +--
 gdb/mdebugread.c          |  14 ++++---
 gdb/mi/mi-symbol-cmds.c   |   6 ++-
 gdb/objfiles.c            |  92 +++++++++++++++-------------------------
 gdb/python/py-linetable.c |  14 +++----
 gdb/record-btrace.c       |  12 ++++--
 gdb/symmisc.c             |  10 ++---
 gdb/symtab.c              | 104 ++++++++++++++++++++++++++++------------------
 gdb/symtab.h              |  36 +++++++++++++---
 gdb/xcoffread.c           |  19 ++++-----
 16 files changed, 215 insertions(+), 186 deletions(-)
---
base-commit: 447d06696999be437b0e3c1a1f26e050ad91b952
change-id: 20230308-submit-constify-linetable-fb1ad2a5b673

Best regards,
-- 
Tom Tromey <tom@tromey.com>


^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 1/5] Add operator< and operator== to linetable_entry
  2023-03-08 15:42 [PATCH 0/5] Make line tables independent of objfile Tom Tromey
@ 2023-03-08 15:42 ` Tom Tromey
  2023-03-11  3:21   ` Simon Marchi
  2023-03-08 15:42 ` [PATCH 2/5] Change linetables to be objfile-independent Tom Tromey
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 20+ messages in thread
From: Tom Tromey @ 2023-03-08 15:42 UTC (permalink / raw)
  To: gdb-patches

This adds a couple of comparison operators to linetable_entry, and
simplifies both the calls to sort and one other spot that checks for
equality.
---
 gdb/buildsym.c  | 14 +-------------
 gdb/disasm.c    |  2 +-
 gdb/symtab.h    | 13 +++++++++++++
 gdb/xcoffread.c |  4 +---
 4 files changed, 16 insertions(+), 17 deletions(-)

diff --git a/gdb/buildsym.c b/gdb/buildsym.c
index dbc81727cb1..6594da68131 100644
--- a/gdb/buildsym.c
+++ b/gdb/buildsym.c
@@ -892,17 +892,6 @@ buildsym_compunit::end_compunit_symtab_with_blockvector
     {
       if (!subfile->line_vector_entries.empty ())
 	{
-	  const auto lte_is_less_than
-	    = [] (const linetable_entry &ln1,
-		  const linetable_entry &ln2) -> bool
-	      {
-		if (ln1.pc == ln2.pc
-		    && ((ln1.line == 0) != (ln2.line == 0)))
-		  return ln1.line == 0;
-
-		return (ln1.pc < ln2.pc);
-	      };
-
 	  /* Like the pending blocks, the line table may be scrambled in
 	     reordered executables.  Sort it if OBJF_REORDERED is true.  It
 	     is important to preserve the order of lines at the same
@@ -910,8 +899,7 @@ buildsym_compunit::end_compunit_symtab_with_blockvector
 	     relationships, this is why std::stable_sort is used.  */
 	  if (m_objfile->flags & OBJF_REORDERED)
 	    std::stable_sort (subfile->line_vector_entries.begin (),
-			      subfile->line_vector_entries.end (),
-			      lte_is_less_than);
+			      subfile->line_vector_entries.end ());
 	}
 
       /* Allocate a symbol table if necessary.  */
diff --git a/gdb/disasm.c b/gdb/disasm.c
index a0406377acc..49053dcfbad 100644
--- a/gdb/disasm.c
+++ b/gdb/disasm.c
@@ -604,7 +604,7 @@ do_mixed_source_and_assembly_deprecated
 
   for (; i < nlines - 1 && le[i].pc < high; i++)
     {
-      if (le[i].line == le[i + 1].line && le[i].pc == le[i + 1].pc)
+      if (le[i] == le[i + 1])
 	continue;		/* Ignore duplicates.  */
 
       /* Skip any end-of-function markers.  */
diff --git a/gdb/symtab.h b/gdb/symtab.h
index c565bc8eac4..69f0eaa0f88 100644
--- a/gdb/symtab.h
+++ b/gdb/symtab.h
@@ -1547,6 +1547,19 @@ struct rust_vtable_symbol : public symbol
 
 struct linetable_entry
 {
+  bool operator< (const linetable_entry &other) const
+  {
+    if (pc == other.pc
+	&& (line != 0) != (other.line != 0))
+      return line == 0;
+    return pc < other.pc;
+  }
+
+  /* Two entries are equal if they have the same line and PC.  The
+     other members are ignored.  */
+  bool operator== (const linetable_entry &other) const
+  { return line == other.line && pc == other.pc; }
+
   /* The line number for this entry.  */
   int line;
 
diff --git a/gdb/xcoffread.c b/gdb/xcoffread.c
index b74b235ee87..93054bdeb19 100644
--- a/gdb/xcoffread.c
+++ b/gdb/xcoffread.c
@@ -439,9 +439,7 @@ arrange_linetable (std::vector<linetable_entry> &old_linetable)
   if (fentries.empty ())
     return;
 
-  std::sort (fentries.begin (), fentries.end (),
-	     [] (const linetable_entry &lte1, const linetable_entry& lte2)
-	     { return lte1.pc < lte2.pc; });
+  std::sort (fentries.begin (), fentries.end ());
 
   /* Allocate a new line table.  */
   std::vector<linetable_entry> new_linetable;

-- 
2.39.1


^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 2/5] Change linetables to be objfile-independent
  2023-03-08 15:42 [PATCH 0/5] Make line tables independent of objfile Tom Tromey
  2023-03-08 15:42 ` [PATCH 1/5] Add operator< and operator== to linetable_entry Tom Tromey
@ 2023-03-08 15:42 ` Tom Tromey
  2023-03-11  3:48   ` Simon Marchi
  2023-03-08 15:42 ` [PATCH 3/5] Constify linetables Tom Tromey
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 20+ messages in thread
From: Tom Tromey @ 2023-03-08 15:42 UTC (permalink / raw)
  To: gdb-patches

This changes linetables to not add the text offset to the addresses
they contain.  I did this in a few steps, necessarily combined
together in one patch: I renamed the 'pc' member to 'm_pc', added the
appropriate accessors, and then recompiled.  Then I fixed all the
errors.  Where possible I generally chose to use the raw_pc accessor,
as it is less expensive.

Note that this patch discounts the possibility that the text section
offset might cause wraparound in the addresses in the line table.
However, this was already discounted -- in particular,
objfile_relocate1 did not re-sort the table in this scenario.  (There
was a bug open about this, but as far as I can tell this has never
happened, it's not even clear what inspired that bug.)
---
 gdb/buildsym.c            |  4 +--
 gdb/coffread.c            |  1 -
 gdb/dbxread.c             |  9 ++++---
 gdb/disasm.c              | 27 +++++++++++++-------
 gdb/dwarf2/read.c         |  5 ++--
 gdb/jit.c                 |  2 +-
 gdb/mdebugread.c          |  9 +++----
 gdb/mi/mi-symbol-cmds.c   |  6 +++--
 gdb/objfiles.c            | 16 ------------
 gdb/python/py-linetable.c |  3 ++-
 gdb/record-btrace.c       |  6 ++++-
 gdb/symmisc.c             |  4 +--
 gdb/symtab.c              | 64 +++++++++++++++++++++++++++++++----------------
 gdb/symtab.h              | 19 +++++++++++---
 gdb/xcoffread.c           | 11 ++++----
 15 files changed, 110 insertions(+), 76 deletions(-)

diff --git a/gdb/buildsym.c b/gdb/buildsym.c
index 6594da68131..33c51beed57 100644
--- a/gdb/buildsym.c
+++ b/gdb/buildsym.c
@@ -655,7 +655,7 @@ buildsym_compunit::record_line (struct subfile *subfile, int line,
 	  linetable_entry *last = &subfile->line_vector_entries.back ();
 	  last_line = last->line;
 
-	  if (last->pc != pc)
+	  if (last->raw_pc () != pc)
 	    break;
 
 	  subfile->line_vector_entries.pop_back ();
@@ -670,7 +670,7 @@ buildsym_compunit::record_line (struct subfile *subfile, int line,
   linetable_entry &e = subfile->line_vector_entries.back ();
   e.line = line;
   e.is_stmt = (flags & LEF_IS_STMT) != 0;
-  e.pc = pc;
+  e.set_pc (pc);
   e.prologue_end = (flags & LEF_PROLOGUE_END) != 0;
 }
 
diff --git a/gdb/coffread.c b/gdb/coffread.c
index 45542bc2432..81df2579619 100644
--- a/gdb/coffread.c
+++ b/gdb/coffread.c
@@ -1463,7 +1463,6 @@ enter_linenos (file_ptr file_offset, int first_line,
       if (L_LNNO32 (&lptr) && L_LNNO32 (&lptr) <= last_line)
 	{
 	  CORE_ADDR addr = lptr.l_addr.l_paddr;
-	  addr += objfile->text_section_offset ();
 	  record_line (get_current_subfile (),
 		       first_line + L_LNNO32 (&lptr),
 		       gdbarch_addr_bits_remove (gdbarch, addr));
diff --git a/gdb/dbxread.c b/gdb/dbxread.c
index 02c7e3118ad..1e88121f11c 100644
--- a/gdb/dbxread.c
+++ b/gdb/dbxread.c
@@ -2454,7 +2454,8 @@ process_one_symbol (int type, int desc, CORE_ADDR valu, const char *name,
 	      CORE_ADDR addr = last_function_start + valu;
 
 	      record_line (get_current_subfile (), 0,
-			   gdbarch_addr_bits_remove (gdbarch, addr));
+			   gdbarch_addr_bits_remove (gdbarch, addr)
+			   - objfile->text_section_offset ());
 	    }
 
 	  within_function = 0;
@@ -2662,12 +2663,14 @@ process_one_symbol (int type, int desc, CORE_ADDR valu, const char *name,
 			   last_function_start : valu;
 
 	  record_line (get_current_subfile (), desc,
-		       gdbarch_addr_bits_remove (gdbarch, addr));
+		       gdbarch_addr_bits_remove (gdbarch, addr)
+		       - objfile->text_section_offset ());
 	  sline_found_in_function = 1;
 	}
       else
 	record_line (get_current_subfile (), desc,
-		     gdbarch_addr_bits_remove (gdbarch, valu));
+		     gdbarch_addr_bits_remove (gdbarch, valu)
+		     - objfile->text_section_offset ());
       break;
 
     case N_BCOMM:
diff --git a/gdb/disasm.c b/gdb/disasm.c
index 49053dcfbad..2acde04ffe9 100644
--- a/gdb/disasm.c
+++ b/gdb/disasm.c
@@ -32,6 +32,7 @@
 #include "gdbsupport/gdb_optional.h"
 #include "valprint.h"
 #include "cli/cli-style.h"
+#include "objfiles.h"
 
 /* Disassemble functions.
    FIXME: We should get rid of all the duplicate code in gdb that does
@@ -592,17 +593,21 @@ do_mixed_source_and_assembly_deprecated
   mle = (struct deprecated_dis_line_entry *)
     alloca (nlines * sizeof (struct deprecated_dis_line_entry));
 
+  struct objfile *objfile = symtab->compunit ()->objfile ();
+  low -= objfile->text_section_offset ();
+  high -= objfile->text_section_offset ();
+
   /* Copy linetable entries for this function into our data
      structure, creating end_pc's and setting out_of_order as
      appropriate.  */
 
   /* 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].raw_pc () < 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].raw_pc () < high; i++)
     {
       if (le[i] == le[i + 1])
 	continue;		/* Ignore duplicates.  */
@@ -614,19 +619,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 = le[i].pc (objfile);
+      mle[newlines].end_pc = le[i + 1].pc (objfile);
       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].raw_pc () < 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 = le[i].pc (objfile);
+      sal = find_pc_line (le[i].pc (objfile), 0);
       mle[newlines].end_pc = sal.end;
       newlines++;
     }
@@ -733,6 +738,10 @@ do_mixed_source_and_assembly (struct gdbarch *gdbarch,
 
   htab_up dis_line_table (allocate_dis_line_table ());
 
+  struct objfile *objfile = main_symtab->compunit ()->objfile ();
+  low -= objfile->text_section_offset ();
+  high -= objfile->text_section_offset ();
+
   pc = low;
 
   /* The prologue may be empty, but there may still be a line number entry
@@ -746,10 +755,10 @@ 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++)
+  for (i = 0; i < nlines && le[i].raw_pc () < low; i++)
     continue;
 
-  if (i < nlines && le[i].pc < high)
+  if (i < nlines && le[i].raw_pc () < high)
     first_le = &le[i];
 
   /* Add lines for every pc value.  */
diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index a9758c305b3..e1e2b4a805d 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -17980,10 +17980,9 @@ class lnp_state_machine
   }
 
   /* Handle DW_LNE_set_address.  */
-  void handle_set_address (CORE_ADDR baseaddr, CORE_ADDR address)
+  void handle_set_address (CORE_ADDR address)
   {
     m_op_index = 0;
-    address += baseaddr;
     m_address = gdbarch_adjust_dwarf2_line (m_gdbarch, address, false);
   }
 
@@ -18458,7 +18457,7 @@ dwarf_decode_lines_1 (struct line_header *lh, struct dwarf2_cu *cu,
 
 		    state_machine.check_line_address (cu, line_ptr,
 						      lowpc - baseaddr, address);
-		    state_machine.handle_set_address (baseaddr, address);
+		    state_machine.handle_set_address (address);
 		  }
 		  break;
 		case DW_LNE_define_file:
diff --git a/gdb/jit.c b/gdb/jit.c
index 7f4b9f0fd37..f9f2c92553c 100644
--- a/gdb/jit.c
+++ b/gdb/jit.c
@@ -491,7 +491,7 @@ jit_symtab_line_mapping_add_impl (struct gdb_symbol_callbacks *cb,
   stab->linetable->nitems = nlines;
   for (i = 0; i < nlines; i++)
     {
-      stab->linetable->item[i].pc = (CORE_ADDR) map[i].pc;
+      stab->linetable->item[i].set_pc ((CORE_ADDR) map[i].pc);
       stab->linetable->item[i].line = map[i].line;
       stab->linetable->item[i].is_stmt = 1;
     }
diff --git a/gdb/mdebugread.c b/gdb/mdebugread.c
index 793795c903a..85dceb54fe7 100644
--- a/gdb/mdebugread.c
+++ b/gdb/mdebugread.c
@@ -2152,7 +2152,7 @@ parse_external (EXTR *es, int bigend, const section_offsets &section_offsets,
 
 static void
 parse_lines (FDR *fh, PDR *pr, struct linetable *lt, int maxlines,
-	     CORE_ADDR textlow, CORE_ADDR lowest_pdr_addr)
+	     CORE_ADDR lowest_pdr_addr)
 {
   unsigned char *base;
   int j, k;
@@ -2183,7 +2183,7 @@ parse_lines (FDR *fh, PDR *pr, struct linetable *lt, int maxlines,
 	halt = base + fh->cbLine;
       base += pr->cbLineOffset;
 
-      adr = textlow + pr->adr - lowest_pdr_addr;
+      adr = pr->adr - lowest_pdr_addr;
 
       l = adr >> 2;		/* in words */
       for (lineno = pr->lnLow; base < halt;)
@@ -4011,7 +4011,6 @@ mdebug_expand_psymtab (legacy_psymtab *pst, struct objfile *objfile)
 	      else
 		{
 		  /* Handle encoded stab line number.  */
-		  valu += section_offsets[SECT_OFF_TEXT (objfile)];
 		  record_line (get_current_subfile (), sh.index,
 			       gdbarch_addr_bits_remove (gdbarch, valu));
 		}
@@ -4163,7 +4162,7 @@ mdebug_expand_psymtab (legacy_psymtab *pst, struct objfile *objfile)
 		}
 
 	      parse_lines (fh, pr_block.data (), lines, maxlines,
-			   pst->text_low (objfile), lowest_pdr_addr);
+			   lowest_pdr_addr);
 	      if (lines->nitems < fh->cline)
 		lines = shrink_linetable (lines);
 
@@ -4540,7 +4539,7 @@ add_line (struct linetable *lt, int lineno, CORE_ADDR adr, int last)
     return lineno;
 
   lt->item[lt->nitems].line = lineno;
-  lt->item[lt->nitems++].pc = adr << 2;
+  lt->item[lt->nitems++].set_pc (adr << 2);
   return lineno;
 }
 \f
diff --git a/gdb/mi/mi-symbol-cmds.c b/gdb/mi/mi-symbol-cmds.c
index 16947dad51b..08e6e15e14d 100644
--- a/gdb/mi/mi-symbol-cmds.c
+++ b/gdb/mi/mi-symbol-cmds.c
@@ -50,14 +50,16 @@ mi_cmd_symbol_list_lines (const char *command, char **argv, int argc)
      already sorted by increasing values in the symbol table, so no
      need to perform any other sorting.  */
 
-  gdbarch = s->compunit ()->objfile ()->arch ();
+  struct objfile *objfile = s->compunit ()->objfile ();
+  gdbarch = objfile->arch ();
 
   ui_out_emit_list list_emitter (uiout, "lines");
   if (s->linetable () != NULL && s->linetable ()->nitems > 0)
     for (i = 0; i < s->linetable ()->nitems; i++)
       {
 	ui_out_emit_tuple tuple_emitter (uiout, NULL);
-	uiout->field_core_addr ("pc", gdbarch, s->linetable ()->item[i].pc);
+	uiout->field_core_addr ("pc", gdbarch,
+				s->linetable ()->item[i].pc (objfile));
 	uiout->field_signed ("line", s->linetable ()->item[i].line);
       }
 }
diff --git a/gdb/objfiles.c b/gdb/objfiles.c
index 8e7be8e2e25..2c8f5d14444 100644
--- a/gdb/objfiles.c
+++ b/gdb/objfiles.c
@@ -616,22 +616,6 @@ objfile_relocate1 (struct objfile *objfile,
 
   /* OK, get all the symtabs.  */
   {
-    for (compunit_symtab *cust : objfile->compunits ())
-      {
-	for (symtab *s : cust->filetabs ())
-	  {
-	    struct linetable *l;
-
-	    /* First the line table.  */
-	    l = s->linetable ();
-	    if (l)
-	      {
-		for (int i = 0; i < l->nitems; ++i)
-		  l->item[i].pc += delta[SECT_OFF_TEXT (objfile)];
-	      }
-	  }
-      }
-
     for (compunit_symtab *cust : objfile->compunits ())
       {
 	struct blockvector *bv = cust->blockvector ();
diff --git a/gdb/python/py-linetable.c b/gdb/python/py-linetable.c
index 7e5a6536fde..e42bcc2c792 100644
--- a/gdb/python/py-linetable.c
+++ b/gdb/python/py-linetable.c
@@ -422,7 +422,8 @@ ltpy_iternext (PyObject *self)
       item = &(symtab->linetable ()->item[iter_obj->current_index]);
     }
 
-  obj = build_linetable_entry (item->line, item->pc);
+  struct objfile *objfile = symtab->compunit ()->objfile ();
+  obj = build_linetable_entry (item->line, item->pc (objfile));
   iter_obj->current_index++;
 
   return obj;
diff --git a/gdb/record-btrace.c b/gdb/record-btrace.c
index 61de8491bb9..55fe25e84ae 100644
--- a/gdb/record-btrace.c
+++ b/gdb/record-btrace.c
@@ -44,6 +44,7 @@
 #include "cli/cli-style.h"
 #include "async-event.h"
 #include <forward_list>
+#include "objfiles.h"
 
 static const target_info record_btrace_target_info = {
   "record-btrace",
@@ -722,6 +723,9 @@ btrace_find_line_range (CORE_ADDR pc)
   if (nlines <= 0)
     return btrace_mk_line_range (symtab, 0, 0);
 
+  struct objfile *objfile = symtab->compunit ()->objfile ();
+  pc -= objfile->text_section_offset ();
+
   range = btrace_mk_line_range (symtab, 0, 0);
   for (i = 0; i < nlines - 1; i++)
     {
@@ -733,7 +737,7 @@ btrace_find_line_range (CORE_ADDR pc)
 	 possibly adding more line numbers to the range.  At the time this
 	 change was made I was unsure how to test this so chose to go with
 	 maintaining the existing experience.  */
-      if ((lines[i].pc == pc) && (lines[i].line != 0)
+      if ((lines[i].raw_pc () == pc) && (lines[i].line != 0)
 	  && (lines[i].is_stmt == 1))
 	range = btrace_line_range_add (range, lines[i].line);
     }
diff --git a/gdb/symmisc.c b/gdb/symmisc.c
index 6ff06ccaa93..d1458784e5c 100644
--- a/gdb/symmisc.c
+++ b/gdb/symmisc.c
@@ -263,7 +263,7 @@ dump_symtab_1 (struct symtab *symtab, struct ui_file *outfile)
       for (int i = 0; i < len; i++)
 	{
 	  gdb_printf (outfile, " line %d at ", l->item[i].line);
-	  gdb_puts (paddress (gdbarch, l->item[i].pc), outfile);
+	  gdb_puts (paddress (gdbarch, l->item[i].raw_pc ()), outfile);
 	  if (l->item[i].is_stmt)
 	    gdb_printf (outfile, "\t(stmt)");
 	  gdb_printf (outfile, "\n");
@@ -996,7 +996,7 @@ maintenance_print_one_line_table (struct symtab *symtab, void *data)
 	  else
 	    uiout->field_string ("line", _("END"));
 	  uiout->field_core_addr ("address", objfile->arch (),
-				  item->pc);
+				  item->raw_pc ());
 	  uiout->field_string ("is-stmt", item->is_stmt ? "Y" : "");
 	  uiout->field_string ("prologue-end", item->prologue_end ? "Y" : "");
 	  uiout->text ("\n");
diff --git a/gdb/symtab.c b/gdb/symtab.c
index f26dd2cb187..df7d3b2a398 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -329,6 +329,14 @@ search_domain_name (enum search_domain e)
 
 /* See symtab.h.  */
 
+CORE_ADDR
+linetable_entry::pc (const struct objfile *objfile) const
+{
+  return m_pc + objfile->text_section_offset ();
+}
+
+/* See symtab.h.  */
+
 call_site *
 compunit_symtab::find_call_site (CORE_ADDR pc) const
 {
@@ -3126,6 +3134,7 @@ find_pc_sect_line (CORE_ADDR pc, struct obj_section *section, int notcurrent)
     }
 
   bv = cust->blockvector ();
+  struct objfile *objfile = cust->objfile ();
 
   /* Look at all the symtabs that share this blockvector.
      They all have the same apriori range, that we found was right;
@@ -3152,18 +3161,21 @@ 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))
+      if (item->pc (objfile) > pc
+	  && (!alt || item->raw_pc () < alt->raw_pc ()))
 	alt = item;
 
       auto pc_compare = [](const CORE_ADDR & comp_pc,
 			   const struct linetable_entry & lhs)->bool
       {
-	return comp_pc < lhs.pc;
+	return comp_pc < lhs.raw_pc ();
       };
 
       struct linetable_entry *first = item;
       struct linetable_entry *last = item + len;
-      item = std::upper_bound (first, last, pc, pc_compare);
+      item = std::upper_bound (first, last,
+			       pc - objfile->text_section_offset (),
+			       pc_compare);
       if (item != first)
 	prev = item - 1;		/* Found a matching item.  */
 
@@ -3177,7 +3189,7 @@ 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->raw_pc () > best->raw_pc ()))
 	{
 	  best = prev;
 	  best_symtab = iter_s;
@@ -3192,7 +3204,7 @@ find_pc_sect_line (CORE_ADDR pc, struct obj_section *section, int notcurrent)
 	  if (!best->is_stmt)
 	    {
 	      struct linetable_entry *tmp = best;
-	      while (tmp > first && (tmp - 1)->pc == tmp->pc
+	      while (tmp > first && (tmp - 1)->raw_pc () == tmp->raw_pc ()
 		     && (tmp - 1)->line != 0 && !tmp->is_stmt)
 		--tmp;
 	      if (tmp->is_stmt)
@@ -3200,16 +3212,16 @@ find_pc_sect_line (CORE_ADDR pc, struct obj_section *section, int notcurrent)
 	    }
 
 	  /* Discard BEST_END if it's before the PC of the current BEST.  */
-	  if (best_end <= best->pc)
+	  if (best_end <= best->pc (objfile))
 	    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->raw_pc () > best->raw_pc ()
+	  && (best_end == 0 || best_end > item->pc (objfile)))
+	best_end = item->pc (objfile);
     }
 
   if (!best_symtab)
@@ -3232,11 +3244,11 @@ find_pc_sect_line (CORE_ADDR pc, struct obj_section *section, int notcurrent)
       val.is_stmt = best->is_stmt;
       val.symtab = best_symtab;
       val.line = best->line;
-      val.pc = best->pc;
-      if (best_end && (!alt || best_end < alt->pc))
+      val.pc = best->pc (objfile);
+      if (best_end && (!alt || best_end < alt->pc (objfile)))
 	val.end = best_end;
       else if (alt)
-	val.end = alt->pc;
+	val.end = alt->pc (objfile);
       else
 	val.end = bv->global_block ()->end ();
     }
@@ -3387,6 +3399,7 @@ find_pcs_for_symtab_line (struct symtab *symtab, int line,
 {
   int start = 0;
   std::vector<CORE_ADDR> result;
+  struct objfile *objfile = symtab->compunit ()->objfile ();
 
   /* First, collect all the PCs that are at this line.  */
   while (1)
@@ -3410,7 +3423,7 @@ find_pcs_for_symtab_line (struct symtab *symtab, int line,
 	  break;
 	}
 
-      result.push_back (symtab->linetable ()->item[idx].pc);
+      result.push_back (symtab->linetable ()->item[idx].pc (objfile));
       start = idx + 1;
     }
 
@@ -3436,7 +3449,7 @@ find_line_pc (struct symtab *symtab, int line, CORE_ADDR *pc)
   if (symtab != NULL)
     {
       l = symtab->linetable ();
-      *pc = l->item[ind].pc;
+      *pc = l->item[ind].pc (symtab->compunit ()->objfile ());
       return true;
     }
   else
@@ -3640,6 +3653,8 @@ skip_prologue_using_lineinfo (CORE_ADDR func_addr, struct symtab *symtab)
   if (!find_pc_partial_function (func_addr, NULL, &func_start, &func_end))
     return func_addr;
 
+  struct objfile *objfile = symtab->compunit ()->objfile ();
+
   /* Linetable entries are ordered by PC values, see the commentary in
      symtab.h where `struct linetable' is defined.  Thus, the first
      entry whose PC is in the range [FUNC_START..FUNC_END[ is the
@@ -3647,12 +3662,13 @@ skip_prologue_using_lineinfo (CORE_ADDR func_addr, struct symtab *symtab)
   for (i = 0; i < l->nitems; i++)
     {
       struct linetable_entry *item = &(l->item[i]);
+      CORE_ADDR item_pc = item->pc (objfile);
 
       /* 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_pc && item_pc < func_end)
+	return item_pc;
     }
 
   return func_addr;
@@ -3679,18 +3695,22 @@ skip_prologue_using_linetable (CORE_ADDR func_addr)
     {
       struct linetable *linetable = prologue_sal.symtab->linetable ();
 
+      struct objfile *objfile = prologue_sal.symtab->compunit ()->objfile ();
+      start_pc -= objfile->text_section_offset ();
+      end_pc -= objfile->text_section_offset ();
+
       auto it = std::lower_bound
 	(linetable->item, linetable->item + linetable->nitems, start_pc,
 	 [] (const linetable_entry &lte, CORE_ADDR pc) -> bool
 	 {
-	   return lte.pc < pc;
+	   return lte.raw_pc () < pc;
 	 });
 
       for (;
-	   it < linetable->item + linetable->nitems && it->pc <= end_pc;
+	   it < linetable->item + linetable->nitems && it->raw_pc () <= end_pc;
 	   it++)
 	if (it->prologue_end)
-	  return {it->pc};
+	  return {it->pc (objfile)};
     }
 
   return {};
@@ -3924,18 +3944,20 @@ skip_prologue_using_sal (struct gdbarch *gdbarch, CORE_ADDR func_addr)
 	 do this.  */
       if (prologue_sal.symtab->language () != language_asm)
 	{
+	  struct objfile *objfile
+	    = prologue_sal.symtab->compunit ()->objfile ();
 	  struct linetable *linetable = prologue_sal.symtab->linetable ();
 	  int idx = 0;
 
 	  /* Skip any earlier lines, and any end-of-sequence marker
 	     from a previous function.  */
-	  while (linetable->item[idx].pc != prologue_sal.pc
+	  while (linetable->item[idx].pc (objfile) != 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].pc (objfile) == start_pc)
 	    return start_pc;
 	}
 
diff --git a/gdb/symtab.h b/gdb/symtab.h
index 69f0eaa0f88..84d12617f31 100644
--- a/gdb/symtab.h
+++ b/gdb/symtab.h
@@ -1547,18 +1547,29 @@ struct rust_vtable_symbol : public symbol
 
 struct linetable_entry
 {
+  /* Set the (unrelocated) PC for this entry.  */
+  void set_pc (CORE_ADDR pc)
+  { m_pc = pc; }
+
+  /* Return the unrelocated PC for this entry.  */
+  CORE_ADDR raw_pc () const
+  { return m_pc; }
+
+  /* Return the relocated PC for this entry.  */
+  CORE_ADDR pc (const struct objfile *objfile) const;
+
   bool operator< (const linetable_entry &other) const
   {
-    if (pc == other.pc
+    if (m_pc == other.m_pc
 	&& (line != 0) != (other.line != 0))
       return line == 0;
-    return pc < other.pc;
+    return m_pc < other.m_pc;
   }
 
   /* Two entries are equal if they have the same line and PC.  The
      other members are ignored.  */
   bool operator== (const linetable_entry &other) const
-  { return line == other.line && pc == other.pc; }
+  { return line == other.line && m_pc == other.m_pc; }
 
   /* The line number for this entry.  */
   int line;
@@ -1571,7 +1582,7 @@ struct linetable_entry
   bool prologue_end : 1;
 
   /* The address for this entry.  */
-  CORE_ADDR pc;
+  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 93054bdeb19..45578311705 100644
--- a/gdb/xcoffread.c
+++ b/gdb/xcoffread.c
@@ -432,7 +432,7 @@ arrange_linetable (std::vector<linetable_entry> &old_linetable)
 	  linetable_entry &e = fentries.back ();
 	  e.line = ii;
 	  e.is_stmt = 1;
-	  e.pc = old_linetable[ii].pc;
+	  e.set_pc (old_linetable[ii].raw_pc ());
 	}
     }
 
@@ -457,7 +457,7 @@ arrange_linetable (std::vector<linetable_entry> &old_linetable)
 	 extra line to cover the function prologue.  */
       int jj = entry.line;
       if (jj + 1 < old_linetable.size ()
-	  && old_linetable[jj].pc != old_linetable[jj + 1].pc)
+	  && old_linetable[jj].raw_pc () != old_linetable[jj + 1].raw_pc ())
 	{
 	  new_linetable.push_back (old_linetable[jj]);
 	  new_linetable.back ().line = old_linetable[jj + 1].line;
@@ -790,15 +790,16 @@ enter_line_range (struct subfile *subfile, unsigned beginoffset,
       if (addr < startaddr || (endaddr && addr >= endaddr))
 	return;
 
+      CORE_ADDR record_addr = (gdbarch_addr_bits_remove (gdbarch, addr)
+			       - objfile->text_section_offset ());
       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, record_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, record_addr);
       curoffset += linesz;
     }
 }

-- 
2.39.1


^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 3/5] Constify linetables
  2023-03-08 15:42 [PATCH 0/5] Make line tables independent of objfile Tom Tromey
  2023-03-08 15:42 ` [PATCH 1/5] Add operator< and operator== to linetable_entry Tom Tromey
  2023-03-08 15:42 ` [PATCH 2/5] Change linetables to be objfile-independent Tom Tromey
@ 2023-03-08 15:42 ` Tom Tromey
  2023-03-08 15:42 ` [PATCH 4/5] Remove extra scopes from objfile_relocate1 Tom Tromey
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 20+ messages in thread
From: Tom Tromey @ 2023-03-08 15:42 UTC (permalink / raw)
  To: gdb-patches

Linetables no longer change after they are created.  This patch
applies const to them.

Note there is one hack to cast away const in mdebugread.c.  This code
allocates a linetable using 'malloc', then later copies it to the
obstack.  While this could be cleaned up, I chose not to do so because
I have no way of testing it.
---
 gdb/buildsym.c            | 12 +++++++-----
 gdb/disasm.c              |  2 +-
 gdb/jit.c                 |  8 +++++---
 gdb/linespec.c            |  6 +++---
 gdb/mdebugread.c          |  5 ++++-
 gdb/python/py-linetable.c | 11 +++++------
 gdb/record-btrace.c       |  4 ++--
 gdb/symmisc.c             |  6 +++---
 gdb/symtab.c              | 40 ++++++++++++++++++++--------------------
 gdb/symtab.h              |  8 ++++----
 10 files changed, 54 insertions(+), 48 deletions(-)

diff --git a/gdb/buildsym.c b/gdb/buildsym.c
index 33c51beed57..392b446943c 100644
--- a/gdb/buildsym.c
+++ b/gdb/buildsym.c
@@ -918,13 +918,15 @@ buildsym_compunit::end_compunit_symtab_with_blockvector
 	  size_t entry_array_size = n_entries * sizeof (struct linetable_entry);
 	  int linetablesize = sizeof (struct linetable) + entry_array_size;
 
-	  symtab->set_linetable
-	    (XOBNEWVAR (&m_objfile->objfile_obstack, struct linetable,
-			linetablesize));
+	  struct linetable *new_table
+	    = XOBNEWVAR (&m_objfile->objfile_obstack, struct linetable,
+			 linetablesize);
 
-	  symtab->linetable ()->nitems = n_entries;
-	  memcpy (symtab->linetable ()->item,
+	  new_table->nitems = n_entries;
+	  memcpy (new_table->item,
 		  subfile->line_vector_entries.data (), entry_array_size);
+
+	  symtab->set_linetable (new_table);
 	}
       else
 	symtab->set_linetable (nullptr);
diff --git a/gdb/disasm.c b/gdb/disasm.c
index 2acde04ffe9..71d3b97258d 100644
--- a/gdb/disasm.c
+++ b/gdb/disasm.c
@@ -573,7 +573,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 f9f2c92553c..b27df8a6e17 100644
--- a/gdb/jit.c
+++ b/gdb/jit.c
@@ -543,9 +543,11 @@ finalize_symtab (struct gdb_symtab *stab, struct objfile *objfile)
       size_t size = ((stab->linetable->nitems - 1)
 		     * sizeof (struct linetable_entry)
 		     + sizeof (struct linetable));
-      filetab->set_linetable ((struct linetable *)
-			      obstack_alloc (&objfile->objfile_obstack, size));
-      memcpy (filetab->linetable (), stab->linetable.get (), size);
+      struct linetable *new_table
+	= (struct linetable *) obstack_alloc (&objfile->objfile_obstack,
+					      size);
+      memcpy (new_table, stab->linetable.get (), size);
+      filetab->set_linetable (new_table);
     }
 
   blockvector_size = (sizeof (struct blockvector)
diff --git a/gdb/linespec.c b/gdb/linespec.c
index 36f2ef46a7c..7d969f37fbf 100644
--- a/gdb/linespec.c
+++ b/gdb/linespec.c
@@ -403,7 +403,7 @@ static std::vector<symtab_and_line> decode_digits_ordinary
   (struct linespec_state *self,
    linespec *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,
@@ -2079,7 +2079,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 linetable_entry *best_entry = NULL;
       int i, j;
 
       std::vector<symtab_and_line> intermediate_results
@@ -4026,7 +4026,7 @@ static std::vector<symtab_and_line>
 decode_digits_ordinary (struct linespec_state *self,
 			linespec *ls,
 			int line,
-			struct linetable_entry **best_entry)
+			const linetable_entry **best_entry)
 {
   std::vector<symtab_and_line> sals;
   for (const auto &elt : ls->file_symtabs)
diff --git a/gdb/mdebugread.c b/gdb/mdebugread.c
index 85dceb54fe7..d2234f18a1f 100644
--- a/gdb/mdebugread.c
+++ b/gdb/mdebugread.c
@@ -4094,7 +4094,10 @@ mdebug_expand_psymtab (legacy_psymtab *pst, struct objfile *objfile)
 
       psymtab_language = cust->primary_filetab ()->language ();
 
-      lines = cust->primary_filetab ()->linetable ();
+      /* This code allocates the line table on the heap and then later
+	 copies it to the obstack.  So, while casting away const here
+	 is ugly, it's not incorrect.  */
+      lines = const_cast<linetable *> (cust->primary_filetab ()->linetable ());
 
       /* Get a new lexical context.  */
 
diff --git a/gdb/python/py-linetable.c b/gdb/python/py-linetable.c
index e42bcc2c792..6e89c43e65f 100644
--- a/gdb/python/py-linetable.c
+++ b/gdb/python/py-linetable.c
@@ -156,7 +156,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 linetable_entry *best_entry = nullptr;
   std::vector<CORE_ADDR> pcs;
 
   LTPY_REQUIRE_VALID (self, symtab);
@@ -201,7 +201,7 @@ ltpy_has_line (PyObject *self, PyObject *args)
 
   for (index = 0; index < symtab->linetable ()->nitems; index++)
     {
-      struct linetable_entry *item = &(symtab->linetable ()->item[index]);
+      const linetable_entry *item = &(symtab->linetable ()->item[index]);
       if (item->line == py_line)
 	  Py_RETURN_TRUE;
     }
@@ -219,7 +219,6 @@ ltpy_get_all_source_lines (PyObject *self, PyObject *args)
 {
   struct symtab *symtab;
   Py_ssize_t index;
-  struct linetable_entry *item;
 
   LTPY_REQUIRE_VALID (self, symtab);
 
@@ -236,7 +235,7 @@ ltpy_get_all_source_lines (PyObject *self, PyObject *args)
 
   for (index = 0; index < symtab->linetable ()->nitems; index++)
     {
-      item = &(symtab->linetable ()->item[index]);
+      const linetable_entry *item = &(symtab->linetable ()->item[index]);
 
       /* 0 is used to signify end of line table information.  Do not
 	 include in the source set. */
@@ -395,7 +394,6 @@ ltpy_iternext (PyObject *self)
   ltpy_iterator_object *iter_obj = (ltpy_iterator_object *) self;
   struct symtab *symtab;
   PyObject *obj;
-  struct linetable_entry *item;
 
   LTPY_REQUIRE_VALID (iter_obj->source, symtab);
 
@@ -405,7 +403,8 @@ ltpy_iternext (PyObject *self)
       return NULL;
     }
 
-  item = &(symtab->linetable ()->item[iter_obj->current_index]);
+  const linetable_entry *item
+    = &(symtab->linetable ()->item[iter_obj->current_index]);
 
   /* Skip over internal entries such as 0.  0 signifies the end of
      line table data and is not useful to the API user.  */
diff --git a/gdb/record-btrace.c b/gdb/record-btrace.c
index 55fe25e84ae..5b6e33f78fe 100644
--- a/gdb/record-btrace.c
+++ b/gdb/record-btrace.c
@@ -705,8 +705,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 linetable_entry *lines;
+  const linetable *ltable;
   struct symtab *symtab;
   int nlines, i;
 
diff --git a/gdb/symmisc.c b/gdb/symmisc.c
index d1458784e5c..c24d26554d3 100644
--- a/gdb/symmisc.c
+++ b/gdb/symmisc.c
@@ -237,7 +237,7 @@ dump_symtab_1 (struct symtab *symtab, struct ui_file *outfile)
   struct objfile *objfile = symtab->compunit ()->objfile ();
   struct gdbarch *gdbarch = objfile->arch ();
   struct mdict_iterator miter;
-  struct linetable *l;
+  const struct linetable *l;
   struct symbol *sym;
   int depth;
 
@@ -948,7 +948,7 @@ block_depth (const 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 ()->objfile ();
@@ -986,7 +986,7 @@ maintenance_print_one_line_table (struct symtab *symtab, void *data)
 
       for (int i = 0; i < linetable->nitems; ++i)
 	{
-	  struct linetable_entry *item;
+	  const linetable_entry *item;
 
 	  item = &linetable->item [i];
 	  ui_out_emit_tuple tuple_emitter (uiout, nullptr);
diff --git a/gdb/symtab.c b/gdb/symtab.c
index df7d3b2a398..df7dc69fc07 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -78,7 +78,7 @@
 
 static void rbreak_command (const char *, int);
 
-static int find_line_common (struct linetable *, int, int *, int);
+static int find_line_common (const linetable *, int, int *, int);
 
 static struct block_symbol
   lookup_symbol_aux (const char *name,
@@ -2994,15 +2994,15 @@ struct symtab_and_line
 find_pc_sect_line (CORE_ADDR pc, struct obj_section *section, int notcurrent)
 {
   struct compunit_symtab *cust;
-  struct linetable *l;
+  const linetable *l;
   int len;
-  struct linetable_entry *item;
+  const 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 linetable_entry *best = NULL;
   CORE_ADDR best_end = 0;
   struct symtab *best_symtab = 0;
 
@@ -3011,11 +3011,11 @@ 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 linetable_entry *alt = NULL;
 
   /* Info on best line seen in this file.  */
 
-  struct linetable_entry *prev;
+  const linetable_entry *prev;
 
   /* If this pc is not from the current frame,
      it is the address of the end of a call instruction.
@@ -3171,8 +3171,8 @@ find_pc_sect_line (CORE_ADDR pc, struct obj_section *section, int notcurrent)
 	return comp_pc < lhs.raw_pc ();
       };
 
-      struct linetable_entry *first = item;
-      struct linetable_entry *last = item + len;
+      const linetable_entry *first = item;
+      const linetable_entry *last = item + len;
       item = std::upper_bound (first, last,
 			       pc - objfile->text_section_offset (),
 			       pc_compare);
@@ -3203,7 +3203,7 @@ find_pc_sect_line (CORE_ADDR pc, struct obj_section *section, int notcurrent)
 	     pretty cheap.  */
 	  if (!best->is_stmt)
 	    {
-	      struct linetable_entry *tmp = best;
+	      const linetable_entry *tmp = best;
 	      while (tmp > first && (tmp - 1)->raw_pc () == tmp->raw_pc ()
 		     && (tmp - 1)->line != 0 && !tmp->is_stmt)
 		--tmp;
@@ -3311,7 +3311,7 @@ find_line_symtab (struct symtab *sym_tab, 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.  */
@@ -3346,7 +3346,7 @@ find_line_symtab (struct symtab *sym_tab, int line,
 	    {
 	      for (symtab *s : cu->filetabs ())
 		{
-		  struct linetable *l;
+		  const struct linetable *l;
 		  int ind;
 
 		  if (FILENAME_CMP (sym_tab->filename, s->filename) != 0)
@@ -3395,7 +3395,7 @@ find_line_symtab (struct symtab *sym_tab, int line,
 
 std::vector<CORE_ADDR>
 find_pcs_for_symtab_line (struct symtab *symtab, int line,
-			  struct linetable_entry **best_item)
+			  const linetable_entry **best_item)
 {
   int start = 0;
   std::vector<CORE_ADDR> result;
@@ -3414,7 +3414,7 @@ find_pcs_for_symtab_line (struct symtab *symtab, int line,
 
       if (!was_exact)
 	{
-	  struct linetable_entry *item = &symtab->linetable ()->item[idx];
+	  const linetable_entry *item = &symtab->linetable ()->item[idx];
 
 	  if (*best_item == NULL
 	      || (item->line < (*best_item)->line && item->is_stmt))
@@ -3438,7 +3438,7 @@ find_pcs_for_symtab_line (struct symtab *symtab, int line,
 bool
 find_line_pc (struct symtab *symtab, int line, CORE_ADDR *pc)
 {
-  struct linetable *l;
+  const struct linetable *l;
   int ind;
 
   *pc = 0;
@@ -3503,7 +3503,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 linetable *l, int lineno,
 		  int *exact_match, int start)
 {
   int i;
@@ -3526,7 +3526,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 linetable_entry *item = &(l->item[i]);
 
       /* Ignore non-statements.  */
       if (!item->is_stmt)
@@ -3640,7 +3640,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.  */
@@ -3661,7 +3661,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 linetable_entry *item = &(l->item[i]);
       CORE_ADDR item_pc = item->pc (objfile);
 
       /* Don't use line numbers of zero, they mark special entries in
@@ -3693,7 +3693,7 @@ skip_prologue_using_linetable (CORE_ADDR func_addr)
   if (prologue_sal.symtab != nullptr
       && prologue_sal.symtab->language () != language_asm)
     {
-      struct linetable *linetable = prologue_sal.symtab->linetable ();
+      const linetable *linetable = prologue_sal.symtab->linetable ();
 
       struct objfile *objfile = prologue_sal.symtab->compunit ()->objfile ();
       start_pc -= objfile->text_section_offset ();
@@ -3946,7 +3946,7 @@ skip_prologue_using_sal (struct gdbarch *gdbarch, CORE_ADDR func_addr)
 	{
 	  struct objfile *objfile
 	    = prologue_sal.symtab->compunit ()->objfile ();
-	  struct linetable *linetable = prologue_sal.symtab->linetable ();
+	  const linetable *linetable = prologue_sal.symtab->linetable ();
 	  int idx = 0;
 
 	  /* Skip any earlier lines, and any end-of-sequence marker
diff --git a/gdb/symtab.h b/gdb/symtab.h
index 84d12617f31..654da14e9a8 100644
--- a/gdb/symtab.h
+++ b/gdb/symtab.h
@@ -1635,12 +1635,12 @@ struct symtab
     m_compunit = compunit;
   }
 
-  struct linetable *linetable () const
+  const struct linetable *linetable () const
   {
     return m_linetable;
   }
 
-  void set_linetable (struct linetable *linetable)
+  void set_linetable (const struct linetable *linetable)
   {
     m_linetable = linetable;
   }
@@ -1667,7 +1667,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 *m_linetable;
+  const struct linetable *m_linetable;
 
   /* Name of this source file, in a form appropriate to print to the user.
 
@@ -2698,7 +2698,7 @@ 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 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.39.1


^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 4/5] Remove extra scopes from objfile_relocate1
  2023-03-08 15:42 [PATCH 0/5] Make line tables independent of objfile Tom Tromey
                   ` (2 preceding siblings ...)
  2023-03-08 15:42 ` [PATCH 3/5] Constify linetables Tom Tromey
@ 2023-03-08 15:42 ` Tom Tromey
  2023-03-08 15:42 ` [PATCH 5/5] Change linetable_entry::is_stmt to bool Tom Tromey
  2023-03-11  3:51 ` [PATCH 0/5] Make line tables independent of objfile Simon Marchi
  5 siblings, 0 replies; 20+ messages in thread
From: Tom Tromey @ 2023-03-08 15:42 UTC (permalink / raw)
  To: gdb-patches

objfile_relocate1 introduces new scopes that aren't necessary.  I
noticed this while working on an earlier patch in this series.  This
patch removes these.
---
 gdb/objfiles.c | 76 +++++++++++++++++++++++++---------------------------------
 1 file changed, 33 insertions(+), 43 deletions(-)

diff --git a/gdb/objfiles.c b/gdb/objfiles.c
index 2c8f5d14444..21ec8c7ad2e 100644
--- a/gdb/objfiles.c
+++ b/gdb/objfiles.c
@@ -615,53 +615,43 @@ objfile_relocate1 (struct objfile *objfile,
     return 0;
 
   /* OK, get all the symtabs.  */
-  {
-    for (compunit_symtab *cust : objfile->compunits ())
-      {
-	struct blockvector *bv = cust->blockvector ();
-	int block_line_section = SECT_OFF_TEXT (objfile);
-
-	if (bv->map () != nullptr)
-	  bv->map ()->relocate (delta[block_line_section]);
-
-	for (block *b : bv->blocks ())
-	  {
-	    struct symbol *sym;
-	    struct mdict_iterator miter;
-
-	    b->set_start (b->start () + delta[block_line_section]);
-	    b->set_end (b->end () + delta[block_line_section]);
-
-	    for (blockrange &r : b->ranges ())
-	      {
-		r.set_start (r.start () + delta[block_line_section]);
-		r.set_end (r.end () + delta[block_line_section]);
-	      }
-
-	    /* We only want to iterate over the local symbols, not any
-	       symbols in included symtabs.  */
-	    ALL_DICT_SYMBOLS (b->multidict (), miter, sym)
-	      {
-		relocate_one_symbol (sym, objfile, delta);
-	      }
-	  }
-      }
-  }
+  for (compunit_symtab *cust : objfile->compunits ())
+    {
+      struct blockvector *bv = cust->blockvector ();
+      int block_line_section = SECT_OFF_TEXT (objfile);
 
-  /* Relocate isolated symbols.  */
-  {
-    struct symbol *iter;
+      if (bv->map () != nullptr)
+	bv->map ()->relocate (delta[block_line_section]);
 
-    for (iter = objfile->template_symbols; iter; iter = iter->hash_next)
-      relocate_one_symbol (iter, objfile, delta);
-  }
+      for (block *b : bv->blocks ())
+	{
+	  struct symbol *sym;
+	  struct mdict_iterator miter;
 
-  {
-    int i;
+	  b->set_start (b->start () + delta[block_line_section]);
+	  b->set_end (b->end () + delta[block_line_section]);
 
-    for (i = 0; i < objfile->section_offsets.size (); ++i)
-      objfile->section_offsets[i] = new_offsets[i];
-  }
+	  for (blockrange &r : b->ranges ())
+	    {
+	      r.set_start (r.start () + delta[block_line_section]);
+	      r.set_end (r.end () + delta[block_line_section]);
+	    }
+
+	  /* We only want to iterate over the local symbols, not any
+	     symbols in included symtabs.  */
+	  ALL_DICT_SYMBOLS (b->multidict (), miter, sym)
+	    {
+	      relocate_one_symbol (sym, objfile, delta);
+	    }
+	}
+    }
+
+  /* Relocate isolated symbols.  */
+  for (symbol *iter = objfile->template_symbols; iter; iter = iter->hash_next)
+    relocate_one_symbol (iter, objfile, delta);
+
+  for (int i = 0; i < objfile->section_offsets.size (); ++i)
+    objfile->section_offsets[i] = new_offsets[i];
 
   /* Rebuild section map next time we need it.  */
   get_objfile_pspace_data (objfile->pspace)->section_map_dirty = 1;

-- 
2.39.1


^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 5/5] Change linetable_entry::is_stmt to bool
  2023-03-08 15:42 [PATCH 0/5] Make line tables independent of objfile Tom Tromey
                   ` (3 preceding siblings ...)
  2023-03-08 15:42 ` [PATCH 4/5] Remove extra scopes from objfile_relocate1 Tom Tromey
@ 2023-03-08 15:42 ` Tom Tromey
  2023-03-11  3:51 ` [PATCH 0/5] Make line tables independent of objfile Simon Marchi
  5 siblings, 0 replies; 20+ messages in thread
From: Tom Tromey @ 2023-03-08 15:42 UTC (permalink / raw)
  To: gdb-patches

This changes linetable_entry::is_stmt to type bool, rather than
unsigned.
---
 gdb/jit.c           | 2 +-
 gdb/record-btrace.c | 2 +-
 gdb/symtab.h        | 2 +-
 gdb/xcoffread.c     | 4 ++--
 4 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/gdb/jit.c b/gdb/jit.c
index b27df8a6e17..22ce7542c98 100644
--- a/gdb/jit.c
+++ b/gdb/jit.c
@@ -493,7 +493,7 @@ jit_symtab_line_mapping_add_impl (struct gdb_symbol_callbacks *cb,
     {
       stab->linetable->item[i].set_pc ((CORE_ADDR) map[i].pc);
       stab->linetable->item[i].line = map[i].line;
-      stab->linetable->item[i].is_stmt = 1;
+      stab->linetable->item[i].is_stmt = true;
     }
 }
 
diff --git a/gdb/record-btrace.c b/gdb/record-btrace.c
index 5b6e33f78fe..3e71c6c9a10 100644
--- a/gdb/record-btrace.c
+++ b/gdb/record-btrace.c
@@ -738,7 +738,7 @@ btrace_find_line_range (CORE_ADDR pc)
 	 change was made I was unsure how to test this so chose to go with
 	 maintaining the existing experience.  */
       if ((lines[i].raw_pc () == pc) && (lines[i].line != 0)
-	  && (lines[i].is_stmt == 1))
+	  && lines[i].is_stmt)
 	range = btrace_line_range_add (range, lines[i].line);
     }
 
diff --git a/gdb/symtab.h b/gdb/symtab.h
index 654da14e9a8..5c1b0611f9a 100644
--- a/gdb/symtab.h
+++ b/gdb/symtab.h
@@ -1575,7 +1575,7 @@ struct linetable_entry
   int line;
 
   /* True if this PC is a good location to place a breakpoint for LINE.  */
-  unsigned is_stmt : 1;
+  bool is_stmt : 1;
 
   /* True if this location is a good location to place a breakpoint after a
      function prologue.  */
diff --git a/gdb/xcoffread.c b/gdb/xcoffread.c
index 45578311705..9a7606015e2 100644
--- a/gdb/xcoffread.c
+++ b/gdb/xcoffread.c
@@ -422,7 +422,7 @@ arrange_linetable (std::vector<linetable_entry> &old_linetable)
 
   for (int ii = 0; ii < old_linetable.size (); ++ii)
     {
-      if (old_linetable[ii].is_stmt == 0)
+      if (!old_linetable[ii].is_stmt)
 	continue;
 
       if (old_linetable[ii].line == 0)
@@ -431,7 +431,7 @@ arrange_linetable (std::vector<linetable_entry> &old_linetable)
 	  fentries.emplace_back ();
 	  linetable_entry &e = fentries.back ();
 	  e.line = ii;
-	  e.is_stmt = 1;
+	  e.is_stmt = true;
 	  e.set_pc (old_linetable[ii].raw_pc ());
 	}
     }

-- 
2.39.1


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 1/5] Add operator< and operator== to linetable_entry
  2023-03-08 15:42 ` [PATCH 1/5] Add operator< and operator== to linetable_entry Tom Tromey
@ 2023-03-11  3:21   ` Simon Marchi
  0 siblings, 0 replies; 20+ messages in thread
From: Simon Marchi @ 2023-03-11  3:21 UTC (permalink / raw)
  To: Tom Tromey, gdb-patches

> diff --git a/gdb/symtab.h b/gdb/symtab.h
> index c565bc8eac4..69f0eaa0f88 100644
> --- a/gdb/symtab.h
> +++ b/gdb/symtab.h
> @@ -1547,6 +1547,19 @@ struct rust_vtable_symbol : public symbol
>  
>  struct linetable_entry
>  {
> +  bool operator< (const linetable_entry &other) const
> +  {
> +    if (pc == other.pc
> +	&& (line != 0) != (other.line != 0))
> +      return line == 0;
> +    return pc < other.pc;
> +  }
The way this is written looks a bit complicated to me, but this is
pre-existing code, so it's fine.

Simon


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 2/5] Change linetables to be objfile-independent
  2023-03-08 15:42 ` [PATCH 2/5] Change linetables to be objfile-independent Tom Tromey
@ 2023-03-11  3:48   ` Simon Marchi
  2023-03-11 15:36     ` Tom Tromey
  0 siblings, 1 reply; 20+ messages in thread
From: Simon Marchi @ 2023-03-11  3:48 UTC (permalink / raw)
  To: Tom Tromey, gdb-patches

> diff --git a/gdb/objfiles.c b/gdb/objfiles.c
> index 8e7be8e2e25..2c8f5d14444 100644
> --- a/gdb/objfiles.c
> +++ b/gdb/objfiles.c
> @@ -616,22 +616,6 @@ objfile_relocate1 (struct objfile *objfile,
>  
>    /* OK, get all the symtabs.  */
>    {
> -    for (compunit_symtab *cust : objfile->compunits ())
> -      {
> -	for (symtab *s : cust->filetabs ())
> -	  {
> -	    struct linetable *l;
> -
> -	    /* First the line table.  */
> -	    l = s->linetable ();
> -	    if (l)
> -	      {
> -		for (int i = 0; i < l->nitems; ++i)
> -		  l->item[i].pc += delta[SECT_OFF_TEXT (objfile)];
> -	      }
> -	  }
> -      }
> -
It's nice to see the objfile_relocate1 function shrinking, it's like a
progress meter of your project :).

> diff --git a/gdb/symmisc.c b/gdb/symmisc.c
> index 6ff06ccaa93..d1458784e5c 100644
> --- a/gdb/symmisc.c
> +++ b/gdb/symmisc.c
> @@ -263,7 +263,7 @@ dump_symtab_1 (struct symtab *symtab, struct ui_file *outfile)
>        for (int i = 0; i < len; i++)
>  	{
>  	  gdb_printf (outfile, " line %d at ", l->item[i].line);
> -	  gdb_puts (paddress (gdbarch, l->item[i].pc), outfile);
> +	  gdb_puts (paddress (gdbarch, l->item[i].raw_pc ()), outfile);

Here, do we want to see the raw of the relocated PC?  Maybe both, if
different?  The one you want depends if you are trying to match this
information against addresses from the ELF file (unrelocated) or against
addresses from the inferior (relocated).

> diff --git a/gdb/symtab.h b/gdb/symtab.h
> index 69f0eaa0f88..84d12617f31 100644
> --- a/gdb/symtab.h
> +++ b/gdb/symtab.h
> @@ -1547,18 +1547,29 @@ struct rust_vtable_symbol : public symbol
>  
>  struct linetable_entry
>  {
> +  /* Set the (unrelocated) PC for this entry.  */
> +  void set_pc (CORE_ADDR pc)
> +  { m_pc = pc; }

For symmetry with the getter, should this one be named "set_raw_pc"?

Simon


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 0/5] Make line tables independent of objfile
  2023-03-08 15:42 [PATCH 0/5] Make line tables independent of objfile Tom Tromey
                   ` (4 preceding siblings ...)
  2023-03-08 15:42 ` [PATCH 5/5] Change linetable_entry::is_stmt to bool Tom Tromey
@ 2023-03-11  3:51 ` Simon Marchi
  5 siblings, 0 replies; 20+ messages in thread
From: Simon Marchi @ 2023-03-11  3:51 UTC (permalink / raw)
  To: Tom Tromey, gdb-patches



On 3/8/23 10:42, Tom Tromey wrote:
> This is a tiny step toward the long-term goal of "objfile splitting"
> -- that is, making symbol tables independent of the objfile.  This
> goal, if ever achieved, would allow for sharing of symbols and symbol
> tables across inferiors.
> 
> The main issue in this splitting is to ensure that runtime offsets
> aren't part of symbols.  This series applies this change to line
> tables.
> 
> Regression tested on x86-64 Fedora 36.
> 
> Tom

I sent minor comments on patch 2, but otherwise:

Approved-By: Simon Marchi <simon.marchi@efficios.com>

Simon

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 2/5] Change linetables to be objfile-independent
  2023-03-11  3:48   ` Simon Marchi
@ 2023-03-11 15:36     ` Tom Tromey
  2023-03-11 19:13       ` Simon Marchi
  0 siblings, 1 reply; 20+ messages in thread
From: Tom Tromey @ 2023-03-11 15:36 UTC (permalink / raw)
  To: Simon Marchi; +Cc: Tom Tromey, gdb-patches

Simon> It's nice to see the objfile_relocate1 function shrinking, it's
Simon> like a progress meter of your project :).

Yeah... though this one is a refresh of some ancient patches.
The other remaining parts are super hard, I've made several failed
attempts already :(

>> gdb_printf (outfile, " line %d at ", l->item[i].line);
>> -	  gdb_puts (paddress (gdbarch, l->item[i].pc), outfile);
>> +	  gdb_puts (paddress (gdbarch, l->item[i].raw_pc ()), outfile);

Simon> Here, do we want to see the raw of the relocated PC?  Maybe both, if
Simon> different?  The one you want depends if you are trying to match this
Simon> information against addresses from the ELF file (unrelocated) or against
Simon> addresses from the inferior (relocated).

Now that you mention it, I see that minsym printing relocates the
address, so I'll change this one to match.  That way it will also match
blocks.

>> +  /* Set the (unrelocated) PC for this entry.  */
>> +  void set_pc (CORE_ADDR pc)
>> +  { m_pc = pc; }

Simon> For symmetry with the getter, should this one be named "set_raw_pc"?

I'll change it.

Tom

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 2/5] Change linetables to be objfile-independent
  2023-03-11 15:36     ` Tom Tromey
@ 2023-03-11 19:13       ` Simon Marchi
  2023-03-12  3:24         ` Tom Tromey
  0 siblings, 1 reply; 20+ messages in thread
From: Simon Marchi @ 2023-03-11 19:13 UTC (permalink / raw)
  To: Tom Tromey; +Cc: gdb-patches



On 3/11/23 10:36, Tom Tromey wrote:
> Simon> It's nice to see the objfile_relocate1 function shrinking, it's
> Simon> like a progress meter of your project :).
> 
> Yeah... though this one is a refresh of some ancient patches.
> The other remaining parts are super hard, I've made several failed
> attempts already :(
> 
>>> gdb_printf (outfile, " line %d at ", l->item[i].line);
>>> -	  gdb_puts (paddress (gdbarch, l->item[i].pc), outfile);
>>> +	  gdb_puts (paddress (gdbarch, l->item[i].raw_pc ()), outfile);
> 
> Simon> Here, do we want to see the raw of the relocated PC?  Maybe both, if
> Simon> different?  The one you want depends if you are trying to match this
> Simon> information against addresses from the ELF file (unrelocated) or against
> Simon> addresses from the inferior (relocated).
> 
> Now that you mention it, I see that minsym printing relocates the
> address, so I'll change this one to match.  That way it will also match
> blocks.
> 
>>> +  /* Set the (unrelocated) PC for this entry.  */
>>> +  void set_pc (CORE_ADDR pc)
>>> +  { m_pc = pc; }
> 
> Simon> For symmetry with the getter, should this one be named "set_raw_pc"?
> 
> I'll change it.

Hi Tom,

I see these regressions:

FAIL: gdb.base/async.exp: stepi&
FAIL: gdb.base/async.exp: nexti&
FAIL: gdb.base/async.exp: finish&
FAIL: gdb.base/consecutive.exp: stopped at bp, 2nd instr (missing hex prefix)
FAIL: gdb.base/disasm-optim.exp: Disassemble main with source (pattern 2)
FAIL: gdb.dwarf2/dw2-disasm-over-non-stmt.exp: opt=s: disassemble /s main
FAIL: gdb.dwarf2/dw2-disasm-over-non-stmt.exp: opt=s: check_disassembly_results 16 line_label_1 (no disassembly for 16)
FAIL: gdb.dwarf2/dw2-disasm-over-non-stmt.exp: opt=s: check_disassembly_results 16 line_label_2 (no disassembly for 16)
FAIL: gdb.dwarf2/dw2-disasm-over-non-stmt.exp: opt=s: check_disassembly_results 17 line_label_3 (no disassembly for 17)
FAIL: gdb.dwarf2/dw2-disasm-over-non-stmt.exp: opt=s: check_disassembly_results 17 line_label_4 (no disassembly for 17)
FAIL: gdb.dwarf2/dw2-disasm-over-non-stmt.exp: opt=s: check_disassembly_results 17 line_label_5 (no disassembly for 17)
FAIL: gdb.dwarf2/dw2-disasm-over-non-stmt.exp: opt=s: check_disassembly_results 18 line_label_6 (no disassembly for 18)
FAIL: gdb.reverse/consecutive-precsave.exp: stopped at bp, 2nd instr (missing hex prefix)
FAIL: gdb.reverse/consecutive-reverse.exp: stopped at bp, 2nd instr (missing hex prefix)

Simon

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 2/5] Change linetables to be objfile-independent
  2023-03-11 19:13       ` Simon Marchi
@ 2023-03-12  3:24         ` Tom Tromey
  2023-03-12 13:40           ` Simon Marchi
  0 siblings, 1 reply; 20+ messages in thread
From: Tom Tromey @ 2023-03-12  3:24 UTC (permalink / raw)
  To: Simon Marchi; +Cc: Tom Tromey, gdb-patches

Simon> FAIL: gdb.base/async.exp: stepi&
Simon> FAIL: gdb.base/async.exp: nexti&
Simon> FAIL: gdb.base/async.exp: finish&
Simon> FAIL: gdb.base/consecutive.exp: stopped at bp, 2nd instr (missing hex prefix)
Simon> FAIL: gdb.base/disasm-optim.exp: Disassemble main with source (pattern 2)
Simon> FAIL: gdb.dwarf2/dw2-disasm-over-non-stmt.exp: opt=s: disassemble /s main
Simon> FAIL: gdb.dwarf2/dw2-disasm-over-non-stmt.exp: opt=s: check_disassembly_results 16 line_label_1 (no disassembly for 16)
Simon> FAIL: gdb.dwarf2/dw2-disasm-over-non-stmt.exp: opt=s: check_disassembly_results 16 line_label_2 (no disassembly for 16)
Simon> FAIL: gdb.dwarf2/dw2-disasm-over-non-stmt.exp: opt=s: check_disassembly_results 17 line_label_3 (no disassembly for 17)
Simon> FAIL: gdb.dwarf2/dw2-disasm-over-non-stmt.exp: opt=s: check_disassembly_results 17 line_label_4 (no disassembly for 17)
Simon> FAIL: gdb.dwarf2/dw2-disasm-over-non-stmt.exp: opt=s: check_disassembly_results 17 line_label_5 (no disassembly for 17)
Simon> FAIL: gdb.dwarf2/dw2-disasm-over-non-stmt.exp: opt=s: check_disassembly_results 18 line_label_6 (no disassembly for 18)
Simon> FAIL: gdb.reverse/consecutive-precsave.exp: stopped at bp, 2nd instr (missing hex prefix)
Simon> FAIL: gdb.reverse/consecutive-reverse.exp: stopped at bp, 2nd instr (missing hex prefix)

I tried these with git master and they all work fine for me.  I wonder
what the difference could be.  Could you run one of them perhaps & send
me the gdb.log?  Maybe that would give a clue.

Tom

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 2/5] Change linetables to be objfile-independent
  2023-03-12  3:24         ` Tom Tromey
@ 2023-03-12 13:40           ` Simon Marchi
  2023-03-16 14:02             ` Tom Tromey
  0 siblings, 1 reply; 20+ messages in thread
From: Simon Marchi @ 2023-03-12 13:40 UTC (permalink / raw)
  To: Tom Tromey; +Cc: gdb-patches



On 3/11/23 22:24, Tom Tromey wrote:
> Simon> FAIL: gdb.base/async.exp: stepi&
> Simon> FAIL: gdb.base/async.exp: nexti&
> Simon> FAIL: gdb.base/async.exp: finish&
> Simon> FAIL: gdb.base/consecutive.exp: stopped at bp, 2nd instr (missing hex prefix)
> Simon> FAIL: gdb.base/disasm-optim.exp: Disassemble main with source (pattern 2)
> Simon> FAIL: gdb.dwarf2/dw2-disasm-over-non-stmt.exp: opt=s: disassemble /s main
> Simon> FAIL: gdb.dwarf2/dw2-disasm-over-non-stmt.exp: opt=s: check_disassembly_results 16 line_label_1 (no disassembly for 16)
> Simon> FAIL: gdb.dwarf2/dw2-disasm-over-non-stmt.exp: opt=s: check_disassembly_results 16 line_label_2 (no disassembly for 16)
> Simon> FAIL: gdb.dwarf2/dw2-disasm-over-non-stmt.exp: opt=s: check_disassembly_results 17 line_label_3 (no disassembly for 17)
> Simon> FAIL: gdb.dwarf2/dw2-disasm-over-non-stmt.exp: opt=s: check_disassembly_results 17 line_label_4 (no disassembly for 17)
> Simon> FAIL: gdb.dwarf2/dw2-disasm-over-non-stmt.exp: opt=s: check_disassembly_results 17 line_label_5 (no disassembly for 17)
> Simon> FAIL: gdb.dwarf2/dw2-disasm-over-non-stmt.exp: opt=s: check_disassembly_results 18 line_label_6 (no disassembly for 18)
> Simon> FAIL: gdb.reverse/consecutive-precsave.exp: stopped at bp, 2nd instr (missing hex prefix)
> Simon> FAIL: gdb.reverse/consecutive-reverse.exp: stopped at bp, 2nd instr (missing hex prefix)
> 
> I tried these with git master and they all work fine for me.  I wonder
> what the difference could be.  Could you run one of them perhaps & send
> me the gdb.log?  Maybe that would give a clue.

This for instance:


 68 disassemble /s main^M
 69 Dump of assembler code for function main:^M
 70 gdb_expect_list pattern: /disasm-optim\.c:^M
 71 24/
 72 Cannot access memory at address 0x1130^M
 73 (gdb) FAIL: gdb.base/disasm-optim.exp: Disassemble main with source (pattern 2)

Looks like we're trying to read from the inferior at an unrelocated
address.  From some previous bug, I recall that you were using a system
with no-PIE as a default, so that could be why you don't see it.

Simon

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 2/5] Change linetables to be objfile-independent
  2023-03-12 13:40           ` Simon Marchi
@ 2023-03-16 14:02             ` Tom Tromey
  2023-03-16 14:45               ` Simon Marchi
  0 siblings, 1 reply; 20+ messages in thread
From: Tom Tromey @ 2023-03-16 14:02 UTC (permalink / raw)
  To: Simon Marchi via Gdb-patches; +Cc: Tom Tromey, Simon Marchi

>>>>> "Simon" == Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> writes:

Simon> Looks like we're trying to read from the inferior at an unrelocated
Simon> address.  From some previous bug, I recall that you were using a system
Simon> with no-PIE as a default, so that could be why you don't see it.

I tried building this executable with -fPIE and it still seems to work
for me :(

Tom

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 2/5] Change linetables to be objfile-independent
  2023-03-16 14:02             ` Tom Tromey
@ 2023-03-16 14:45               ` Simon Marchi
  2023-03-16 23:43                 ` Tom Tromey
  0 siblings, 1 reply; 20+ messages in thread
From: Simon Marchi @ 2023-03-16 14:45 UTC (permalink / raw)
  To: Tom Tromey, Simon Marchi via Gdb-patches

On 3/16/23 10:02, Tom Tromey wrote:
>>>>>> "Simon" == Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> writes:
> 
> Simon> Looks like we're trying to read from the inferior at an unrelocated
> Simon> address.  From some previous bug, I recall that you were using a system
> Simon> with no-PIE as a default, so that could be why you don't see it.
> 
> I tried building this executable with -fPIE and it still seems to work
> for me :(

Ok, I'll look into it then.

I can reproduce it like hand like this:

    $ gcc -fno-stack-protector -c \
        -o /home/smarchi/build/binutils-gdb/gdb/testsuite/outputs/gdb.base/disasm-optim/disasm-optim0.o \
        /home/smarchi/src/binutils-gdb/gdb/testsuite/gdb.base/disasm-optim.S \
        -fPIE
    $ gcc /home/smarchi/build/binutils-gdb/gdb/testsuite/outputs/gdb.base/disasm-optim/disasm-optim0.o \
        -pie -o disasm-optim

    $ gdb ./gdb -nx -q --data-directory=data-directory disasm-optim
    Reading symbols from disasm-optim...
    (gdb) start

    This GDB supports auto-downloading debuginfo from the following URLs:
      <https://debuginfod.archlinux.org>
    Enable debuginfod for this session? (y or [n]) n
    Debuginfod has been disabled.
    To make this setting permanent, add 'set debuginfod enabled off' to .gdbinit.
    Temporary breakpoint 1 at 0x1120: file disasm-optim.c, line 25.
    Starting program: /home/smarchi/build/binutils-gdb/gdb/disasm-optim
    [Thread debugging using libthread_db enabled]
    Using host libthread_db library "/usr/lib/../lib/libthread_db.so.1".

    Temporary breakpoint 1, main () at disasm-optim.c:25
    25      disasm-optim.c: No such file or directory.
    (gdb) disassemble main
    Dump of assembler code for function main:
    => 0x0000555555555120 <+0>:     mov    0x2ef2(%rip),%eax        # 0x555555558018 <y>
       0x0000555555555126 <+6>:     test   %eax,%eax
       0x0000555555555128 <+8>:     js     0x555555555140 <main+32>
       0x000055555555512a <+10>:    lea    0xa(%rax),%edx
       0x000055555555512d <+13>:    test   %eax,%eax
       0x000055555555512f <+15>:    mov    $0x1,%eax
       0x0000555555555134 <+20>:    cmovne %edx,%eax
       0x0000555555555137 <+23>:    mov    %eax,0x2ed7(%rip)        # 0x555555558014 <x>
       0x000055555555513d <+29>:    xor    %eax,%eax
       0x000055555555513f <+31>:    ret
       0x0000555555555140 <+32>:    add    %eax,%eax
       0x0000555555555142 <+34>:    jmp    0x555555555137 <main+23>
    End of assembler dump.
    (gdb) disassemble /s main
    Dump of assembler code for function main:
    Cannot access memory at address 0x1120

If I link with -no-pie, then it works.

Simon

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 2/5] Change linetables to be objfile-independent
  2023-03-16 14:45               ` Simon Marchi
@ 2023-03-16 23:43                 ` Tom Tromey
  2023-03-17  2:45                   ` Simon Marchi
  0 siblings, 1 reply; 20+ messages in thread
From: Tom Tromey @ 2023-03-16 23:43 UTC (permalink / raw)
  To: Simon Marchi via Gdb-patches; +Cc: Tom Tromey, Simon Marchi

Simon> I can reproduce it like hand like this:
[...]

I think I have a fix for it.  I'll send it tomorrow.

Tom

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 2/5] Change linetables to be objfile-independent
  2023-03-16 23:43                 ` Tom Tromey
@ 2023-03-17  2:45                   ` Simon Marchi
  2023-03-17 13:09                     ` Tom Tromey
  0 siblings, 1 reply; 20+ messages in thread
From: Simon Marchi @ 2023-03-17  2:45 UTC (permalink / raw)
  To: Tom Tromey, Simon Marchi via Gdb-patches



On 3/16/23 19:43, Tom Tromey wrote:
> Simon> I can reproduce it like hand like this:
> [...]
> 
> I think I have a fix for it.  I'll send it tomorrow.
> 
> Tom

Ok, I worked on that today and also have some patches.  One fixes
do_mixed_source_and_assembly, and the other makes "maint info
line-table" show relocated addresses again, it fixes a test that relied
on that (I guess we could also change the test).

If you want to take a peek:

https://review.lttng.org/c/binutils-gdb/+/9665

Simon

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 2/5] Change linetables to be objfile-independent
  2023-03-17  2:45                   ` Simon Marchi
@ 2023-03-17 13:09                     ` Tom Tromey
  2023-03-17 22:21                       ` Tom Tromey
  0 siblings, 1 reply; 20+ messages in thread
From: Tom Tromey @ 2023-03-17 13:09 UTC (permalink / raw)
  To: Simon Marchi; +Cc: Tom Tromey, Simon Marchi via Gdb-patches

Simon> https://review.lttng.org/c/binutils-gdb/+/9665

Lots of patches there!

#1 Renaming is fine but the "_raw" name was chosen to match psymtabs.
psymtabs are essentially obsolete now (depending on the state of the
stabs removal thread) but as long as they are around the naming should
match.

#2 Making a new header seems great to me.

#3 Ditto

#4 For this one I think my fix is better, it introduces a new type and
prevents this sort of bug.

#5 Looks good

Tom

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 2/5] Change linetables to be objfile-independent
  2023-03-17 13:09                     ` Tom Tromey
@ 2023-03-17 22:21                       ` Tom Tromey
  2023-03-20 14:57                         ` Simon Marchi
  0 siblings, 1 reply; 20+ messages in thread
From: Tom Tromey @ 2023-03-17 22:21 UTC (permalink / raw)
  To: Tom Tromey; +Cc: Simon Marchi, Simon Marchi via Gdb-patches

Tom> #1 Renaming is fine but the "_raw" name was chosen to match psymtabs.

Well, I have this partly wrong, sorry about that.

psymbols themselves use the other name:

  CORE_ADDR unrelocated_address () const

but psymtabs use:

  CORE_ADDR raw_text_low () const
  CORE_ADDR raw_text_high () const

and so do minsyms:

  CORE_ADDR value_raw_address () const

I think one set or the other should be renamed.  Which one do you
prefer?  I could go either way.  Anyway let me know and I'll rename
stuff.

Tom

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 2/5] Change linetables to be objfile-independent
  2023-03-17 22:21                       ` Tom Tromey
@ 2023-03-20 14:57                         ` Simon Marchi
  0 siblings, 0 replies; 20+ messages in thread
From: Simon Marchi @ 2023-03-20 14:57 UTC (permalink / raw)
  To: Tom Tromey; +Cc: Simon Marchi via Gdb-patches

On 3/17/23 18:21, Tom Tromey wrote:
> Tom> #1 Renaming is fine but the "_raw" name was chosen to match psymtabs.
> 
> Well, I have this partly wrong, sorry about that.
> 
> psymbols themselves use the other name:
> 
>   CORE_ADDR unrelocated_address () const
> 
> but psymtabs use:
> 
>   CORE_ADDR raw_text_low () const
>   CORE_ADDR raw_text_high () const
> 
> and so do minsyms:
> 
>   CORE_ADDR value_raw_address () const
> 
> I think one set or the other should be renamed.  Which one do you
> prefer?  I could go either way.  Anyway let me know and I'll rename
> stuff.

I prefer "unrelocated", since it's a bit more specific and precise than
"raw". "raw" suggests that it's a value before some kind of processing,
but it doesn't tell which processing that it.  With "unrelocated", you
know it's before relocation.

Simon

^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2023-03-20 14:57 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-03-08 15:42 [PATCH 0/5] Make line tables independent of objfile Tom Tromey
2023-03-08 15:42 ` [PATCH 1/5] Add operator< and operator== to linetable_entry Tom Tromey
2023-03-11  3:21   ` Simon Marchi
2023-03-08 15:42 ` [PATCH 2/5] Change linetables to be objfile-independent Tom Tromey
2023-03-11  3:48   ` Simon Marchi
2023-03-11 15:36     ` Tom Tromey
2023-03-11 19:13       ` Simon Marchi
2023-03-12  3:24         ` Tom Tromey
2023-03-12 13:40           ` Simon Marchi
2023-03-16 14:02             ` Tom Tromey
2023-03-16 14:45               ` Simon Marchi
2023-03-16 23:43                 ` Tom Tromey
2023-03-17  2:45                   ` Simon Marchi
2023-03-17 13:09                     ` Tom Tromey
2023-03-17 22:21                       ` Tom Tromey
2023-03-20 14:57                         ` Simon Marchi
2023-03-08 15:42 ` [PATCH 3/5] Constify linetables Tom Tromey
2023-03-08 15:42 ` [PATCH 4/5] Remove extra scopes from objfile_relocate1 Tom Tromey
2023-03-08 15:42 ` [PATCH 5/5] Change linetable_entry::is_stmt to bool Tom Tromey
2023-03-11  3:51 ` [PATCH 0/5] Make line tables independent of objfile Simon Marchi

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).