Fix line table regression
authorTom Tromey <tom@tromey.com>
Thu, 16 Mar 2023 20:41:31 +0000 (14:41 -0600)
committerTom Tromey <tom@tromey.com>
Fri, 17 Mar 2023 22:17:43 +0000 (16:17 -0600)
Simon pointed out a line table regression, and after a couple of false
starts, I was able to reproduce it by hand using his instructions.

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

  pc = low;

... after the text offset has been removed.

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

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

Regression tested on x86-64 Fedora 36, though note this setup never
reproduced the bug in the first place.  I also tested it by hand on
the disasm-optim test program.

15 files changed:
gdb/buildsym-legacy.c
gdb/buildsym-legacy.h
gdb/buildsym.c
gdb/buildsym.h
gdb/coffread.c
gdb/dbxread.c
gdb/disasm.c
gdb/dwarf2/read.c
gdb/jit.c
gdb/mdebugread.c
gdb/record-btrace.c
gdb/symmisc.c
gdb/symtab.c
gdb/symtab.h
gdb/xcoffread.c

index cb733e7101b45777b334ca199678d2e5c7a3903d..131d24fe9b541d43f0fed3011b990b16ab1f707d 100644 (file)
@@ -205,7 +205,7 @@ finish_block (struct symbol *symbol, struct pending_block *old_blocks,
 }
 
 void
-record_line (struct subfile *subfile, int line, CORE_ADDR pc)
+record_line (struct subfile *subfile, int line, unrelocated_addr pc)
 {
   gdb_assert (buildsym_compunit != nullptr);
   /* Assume every line entry is a statement start, that is a good place to
index 3d705a8beed8c76b6bfad3b609f41d51d8c72fd2..664d6320e540d9aa55c642b5dac506f2fbcf5449 100644 (file)
@@ -76,7 +76,8 @@ extern struct context_stack *push_context (int desc, CORE_ADDR valu);
 
 extern struct context_stack pop_context ();
 
-extern void record_line (struct subfile *subfile, int line, CORE_ADDR pc);
+extern void record_line (struct subfile *subfile, int line,
+                        unrelocated_addr pc);
 
 extern struct compunit_symtab *start_compunit_symtab (struct objfile *objfile,
                                                      const char *name,
index 56f11dd167fbab63bcc39e306915d8b6e55966bb..f000233dafac9d5796de7633620a21842253f6ed 100644 (file)
@@ -629,7 +629,7 @@ buildsym_compunit::pop_subfile ()
 
 void
 buildsym_compunit::record_line (struct subfile *subfile, int line,
-                               CORE_ADDR pc, linetable_entry_flags flags)
+                               unrelocated_addr pc, linetable_entry_flags flags)
 {
   m_have_line_numbers = true;
 
index 42fcd1fdb97b326b05b1f14537702c7f52f8b896..98dc8f028746d5e7dbbbea89accbc8841459dc0d 100644 (file)
@@ -239,7 +239,7 @@ struct buildsym_compunit
 
   const char *pop_subfile ();
 
-  void record_line (struct subfile *subfile, int line, CORE_ADDR pc,
+  void record_line (struct subfile *subfile, int line, unrelocated_addr pc,
                    linetable_entry_flags flags);
 
   struct compunit_symtab *get_compunit_symtab ()
index 4da3799243b5aeb89bf65819b922bfe71fb71fb2..fe1ee506fbe6f0f979724e5e23dca5436b6a5b39 100644 (file)
@@ -1130,9 +1130,10 @@ coff_symtab_read (minimal_symbol_reader &reader,
                 of the closing '}', and for which we do not have any
                 other statement-line-number.  */
              if (fcn_last_line == 1)
-               record_line (get_current_subfile (), fcn_first_line,
-                            gdbarch_addr_bits_remove (gdbarch,
-                                                      fcn_first_line_addr));
+               record_line
+                 (get_current_subfile (), fcn_first_line,
+                  unrelocated_addr (gdbarch_addr_bits_remove (gdbarch,
+                                                              fcn_first_line_addr)));
              else
                enter_linenos (fcn_line_ptr, fcn_first_line,
                               fcn_last_line, objfile);
@@ -1460,7 +1461,8 @@ enter_linenos (file_ptr file_offset, int first_line,
          CORE_ADDR addr = lptr.l_addr.l_paddr;
          record_line (get_current_subfile (),
                       first_line + L_LNNO32 (&lptr),
-                      gdbarch_addr_bits_remove (gdbarch, addr));
+                      unrelocated_addr (gdbarch_addr_bits_remove (gdbarch,
+                                                                  addr)));
        }
       else
        break;
index 1e88121f11c26badb6c50b93f12dd501774bc39c..e5366ccd0d09be103f983c04e066e621128b9c79 100644 (file)
@@ -2453,9 +2453,10 @@ process_one_symbol (int type, int desc, CORE_ADDR valu, const char *name,
            {
              CORE_ADDR addr = last_function_start + valu;
 
-             record_line (get_current_subfile (), 0,
-                          gdbarch_addr_bits_remove (gdbarch, addr)
-                          - objfile->text_section_offset ());
+             record_line
+               (get_current_subfile (), 0,
+                unrelocated_addr (gdbarch_addr_bits_remove (gdbarch, addr)
+                                  - objfile->text_section_offset ()));
            }
 
          within_function = 0;
@@ -2662,15 +2663,17 @@ process_one_symbol (int type, int desc, CORE_ADDR valu, const char *name,
          CORE_ADDR addr = processing_gcc_compilation == 2 ?
                           last_function_start : valu;
 
-         record_line (get_current_subfile (), desc,
-                      gdbarch_addr_bits_remove (gdbarch, addr)
-                      - objfile->text_section_offset ());
+         record_line
+           (get_current_subfile (), desc,
+            unrelocated_addr (gdbarch_addr_bits_remove (gdbarch, addr)
+                              - objfile->text_section_offset ()));
          sline_found_in_function = 1;
        }
       else
-       record_line (get_current_subfile (), desc,
-                    gdbarch_addr_bits_remove (gdbarch, valu)
-                    - objfile->text_section_offset ());
+       record_line
+         (get_current_subfile (), desc,
+          unrelocated_addr (gdbarch_addr_bits_remove (gdbarch, valu)
+                            - objfile->text_section_offset ()));
       break;
 
     case N_BCOMM:
index 71d3b97258d32d384d927bc63673bca62d6ca990..03cd4b7ee025744ab31ad8f7c85236818c00836f 100644 (file)
@@ -594,8 +594,11 @@ do_mixed_source_and_assembly_deprecated
     alloca (nlines * sizeof (struct deprecated_dis_line_entry));
 
   struct objfile *objfile = symtab->compunit ()->objfile ();
-  low -= objfile->text_section_offset ();
-  high -= objfile->text_section_offset ();
+
+  unrelocated_addr unrel_low
+    = unrelocated_addr (low - objfile->text_section_offset ());
+  unrelocated_addr unrel_high
+    = unrelocated_addr (high - objfile->text_section_offset ());
 
   /* Copy linetable entries for this function into our data
      structure, creating end_pc's and setting out_of_order as
@@ -603,11 +606,11 @@ do_mixed_source_and_assembly_deprecated
 
   /* First, skip all the preceding functions.  */
 
-  for (i = 0; i < nlines - 1 && le[i].raw_pc () < low; i++);
+  for (i = 0; i < nlines - 1 && le[i].raw_pc () < unrel_low; i++);
 
   /* Now, copy all entries before the end of this function.  */
 
-  for (; i < nlines - 1 && le[i].raw_pc () < high; i++)
+  for (; i < nlines - 1 && le[i].raw_pc () < unrel_high; i++)
     {
       if (le[i] == le[i + 1])
        continue;               /* Ignore duplicates.  */
@@ -627,7 +630,7 @@ do_mixed_source_and_assembly_deprecated
   /* If we're on the last line, and it's part of the function,
      then we need to get the end pc in a special way.  */
 
-  if (i == nlines - 1 && le[i].raw_pc () < high)
+  if (i == nlines - 1 && le[i].raw_pc () < unrel_high)
     {
       mle[newlines].line = le[i].line;
       mle[newlines].start_pc = le[i].pc (objfile);
@@ -739,8 +742,11 @@ do_mixed_source_and_assembly (struct gdbarch *gdbarch,
   htab_up dis_line_table (allocate_dis_line_table ());
 
   struct objfile *objfile = main_symtab->compunit ()->objfile ();
-  low -= objfile->text_section_offset ();
-  high -= objfile->text_section_offset ();
+
+  unrelocated_addr unrel_low
+    = unrelocated_addr (low - objfile->text_section_offset ());
+  unrelocated_addr unrel_high
+    = unrelocated_addr (high - objfile->text_section_offset ());
 
   pc = low;
 
@@ -755,10 +761,10 @@ do_mixed_source_and_assembly (struct gdbarch *gdbarch,
   first_le = NULL;
 
   /* Skip all the preceding functions.  */
-  for (i = 0; i < nlines && le[i].raw_pc () < low; i++)
+  for (i = 0; i < nlines && le[i].raw_pc () < unrel_low; i++)
     continue;
 
-  if (i < nlines && le[i].raw_pc () < high)
+  if (i < nlines && le[i].raw_pc () < unrel_high)
     first_le = &le[i];
 
   /* Add lines for every pc value.  */
index 138537cfefd128ad1ed75461e72b12b96c265a3e..a1c75655ff8f13fea7095b658f5e7764b4d6145a 100644 (file)
@@ -18209,7 +18209,8 @@ dwarf_record_line_1 (struct gdbarch *gdbarch, struct subfile *subfile,
                     linetable_entry_flags flags,
                     struct dwarf2_cu *cu)
 {
-  CORE_ADDR addr = gdbarch_addr_bits_remove (gdbarch, address);
+  unrelocated_addr addr
+    = unrelocated_addr (gdbarch_addr_bits_remove (gdbarch, address));
 
   if (dwarf_line_debug)
     {
index a93813b3f7f2a77ae86bff636e7e45d2c956e72b..52b65746f112c3db0e3972273b73bc35ec666860 100644 (file)
--- a/gdb/jit.c
+++ b/gdb/jit.c
@@ -491,7 +491,7 @@ jit_symtab_line_mapping_add_impl (struct gdb_symbol_callbacks *cb,
   stab->linetable->nitems = nlines;
   for (i = 0; i < nlines; i++)
     {
-      stab->linetable->item[i].set_raw_pc ((CORE_ADDR) map[i].pc);
+      stab->linetable->item[i].set_raw_pc (unrelocated_addr (map[i].pc));
       stab->linetable->item[i].line = map[i].line;
       stab->linetable->item[i].is_stmt = true;
     }
index 13dc7899d446a265bb07b994cffdbff42768fd87..bc466096d610a6cce0d4e21ff43b59e455427325 100644 (file)
@@ -3982,8 +3982,10 @@ mdebug_expand_psymtab (legacy_psymtab *pst, struct objfile *objfile)
              else
                {
                  /* Handle encoded stab line number.  */
-                 record_line (get_current_subfile (), sh.index,
-                              gdbarch_addr_bits_remove (gdbarch, valu));
+                 record_line
+                   (get_current_subfile (), sh.index,
+                    unrelocated_addr (gdbarch_addr_bits_remove (gdbarch,
+                                                                valu)));
                }
            }
          else if (sh.st == stProc || sh.st == stStaticProc
@@ -4513,7 +4515,7 @@ add_line (struct linetable *lt, int lineno, CORE_ADDR adr, int last)
     return lineno;
 
   lt->item[lt->nitems].line = lineno;
-  lt->item[lt->nitems++].set_raw_pc (adr << 2);
+  lt->item[lt->nitems++].set_raw_pc (unrelocated_addr (adr << 2));
   return lineno;
 }
 \f
index 3e71c6c9a10d4e9ac5227e4f5b597bdec2e55acb..2d88e4d20bf6fb61bdbdfcaacd93fc55fc57941a 100644 (file)
@@ -724,7 +724,8 @@ btrace_find_line_range (CORE_ADDR pc)
     return btrace_mk_line_range (symtab, 0, 0);
 
   struct objfile *objfile = symtab->compunit ()->objfile ();
-  pc -= objfile->text_section_offset ();
+  unrelocated_addr unrel_pc
+    = unrelocated_addr (pc - objfile->text_section_offset ());
 
   range = btrace_mk_line_range (symtab, 0, 0);
   for (i = 0; i < nlines - 1; i++)
@@ -737,7 +738,7 @@ btrace_find_line_range (CORE_ADDR pc)
         possibly adding more line numbers to the range.  At the time this
         change was made I was unsure how to test this so chose to go with
         maintaining the existing experience.  */
-      if ((lines[i].raw_pc () == pc) && (lines[i].line != 0)
+      if (lines[i].raw_pc () == unrel_pc && lines[i].line != 0
          && lines[i].is_stmt)
        range = btrace_line_range_add (range, lines[i].line);
     }
index 2f1e4f5b784c1f5f98a46167df305d1b8c70fc59..54dc570d282f9405a8b31792c84cc5fb8e448153 100644 (file)
@@ -996,7 +996,7 @@ maintenance_print_one_line_table (struct symtab *symtab, void *data)
          else
            uiout->field_string ("line", _("END"));
          uiout->field_core_addr ("address", objfile->arch (),
-                                 item->raw_pc ());
+                                 CORE_ADDR (item->raw_pc ()));
          uiout->field_string ("is-stmt", item->is_stmt ? "Y" : "");
          uiout->field_string ("prologue-end", item->prologue_end ? "Y" : "");
          uiout->text ("\n");
index e11f9262a22301dd460934f6c8a7b66bc5bd30b9..8ab1f58affec61995dceffe5c0b6d2e616afd271 100644 (file)
@@ -332,7 +332,7 @@ search_domain_name (enum search_domain e)
 CORE_ADDR
 linetable_entry::pc (const struct objfile *objfile) const
 {
-  return m_pc + objfile->text_section_offset ();
+  return CORE_ADDR (m_pc) + objfile->text_section_offset ();
 }
 
 /* See symtab.h.  */
@@ -3158,17 +3158,18 @@ find_pc_sect_line (CORE_ADDR pc, struct obj_section *section, int notcurrent)
          && (!alt || item->raw_pc () < alt->raw_pc ()))
        alt = item;
 
-      auto pc_compare = [](const CORE_ADDR & comp_pc,
-                          const struct linetable_entry & lhs)->bool
+      auto pc_compare = [] (const unrelocated_addr &comp_pc,
+                           const struct linetable_entry & lhs)
       {
        return comp_pc < lhs.raw_pc ();
       };
 
       const linetable_entry *first = item;
       const linetable_entry *last = item + len;
-      item = std::upper_bound (first, last,
-                              pc - objfile->text_section_offset (),
-                              pc_compare);
+      item = (std::upper_bound
+             (first, last,
+              unrelocated_addr (pc - objfile->text_section_offset ()),
+              pc_compare));
       if (item != first)
        prev = item - 1;                /* Found a matching item.  */
 
@@ -3689,18 +3690,22 @@ skip_prologue_using_linetable (CORE_ADDR func_addr)
       const linetable *linetable = prologue_sal.symtab->linetable ();
 
       struct objfile *objfile = prologue_sal.symtab->compunit ()->objfile ();
-      start_pc -= objfile->text_section_offset ();
-      end_pc -= objfile->text_section_offset ();
+
+      unrelocated_addr unrel_start
+       = unrelocated_addr (start_pc - objfile->text_section_offset ());
+      unrelocated_addr unrel_end
+       = unrelocated_addr (end_pc - objfile->text_section_offset ());
 
       auto it = std::lower_bound
-       (linetable->item, linetable->item + linetable->nitems, start_pc,
-        [] (const linetable_entry &lte, CORE_ADDR pc) -> bool
+       (linetable->item, linetable->item + linetable->nitems, unrel_start,
+        [] (const linetable_entry &lte, unrelocated_addr pc)
         {
           return lte.raw_pc () < pc;
         });
 
       for (;
-          it < linetable->item + linetable->nitems && it->raw_pc () <= end_pc;
+          (it < linetable->item + linetable->nitems
+           && it->raw_pc () <= unrel_end);
           it++)
        if (it->prologue_end)
          return {it->pc (objfile)};
index 005b64b5a0845b9ff503357d8c63a5b0105e003f..2fd56ce21bd4e89f8d202f59a64c2b5fd1320c0d 100644 (file)
@@ -1540,6 +1540,12 @@ struct rust_vtable_symbol : public symbol
 };
 
 \f
+/* Like a CORE_ADDR, but not directly convertible.  This is used to
+   represent an unrelocated CORE_ADDR.  DEFINE_OFFSET_TYPE is not used
+   here because there's no need to add or subtract values of this
+   type.  */
+enum class unrelocated_addr : CORE_ADDR { };
+
 /* Each item represents a line-->pc (or the reverse) mapping.  This is
    somewhat more wasteful of space than one might wish, but since only
    the files which are actually debugged are read in to core, we don't
@@ -1548,11 +1554,11 @@ struct rust_vtable_symbol : public symbol
 struct linetable_entry
 {
   /* Set the (unrelocated) PC for this entry.  */
-  void set_raw_pc (CORE_ADDR pc)
+  void set_raw_pc (unrelocated_addr pc)
   { m_pc = pc; }
 
   /* Return the unrelocated PC for this entry.  */
-  CORE_ADDR raw_pc () const
+  unrelocated_addr raw_pc () const
   { return m_pc; }
 
   /* Return the relocated PC for this entry.  */
@@ -1582,7 +1588,7 @@ struct linetable_entry
   bool prologue_end : 1;
 
   /* The address for this entry.  */
-  CORE_ADDR m_pc;
+  unrelocated_addr m_pc;
 };
 
 /* The order of entries in the linetable is significant.  They should
index 72c0a0db49acf2e62bd27cabe4bcf86656aec526..ab763760f433b8b57e7c9b17f1818a1b082c3694 100644 (file)
@@ -795,11 +795,12 @@ enter_line_range (struct subfile *subfile, unsigned beginoffset,
       if (int_lnno.l_lnno == 0)
        {
          *firstLine = read_symbol_lineno (int_lnno.l_addr.l_symndx);
-         record_line (subfile, 0, record_addr);
+         record_line (subfile, 0, unrelocated_addr (record_addr));
          --(*firstLine);
        }
       else
-       record_line (subfile, *firstLine + int_lnno.l_lnno, record_addr);
+       record_line (subfile, *firstLine + int_lnno.l_lnno,
+                    unrelocated_addr (record_addr));
       curoffset += linesz;
     }
 }