Fix an out of bounds array access in find_epilogue_using_linetable

An out of bounds array access in find_epilogue_using_linetable causes random
test failures like these:

FAIL: gdb.base/unwind-on-each-insn-amd64.exp: foo: instruction 6: $fba_value == $fn_fba
FAIL: gdb.base/unwind-on-each-insn-amd64.exp: foo: instruction 6: check frame-id matches
FAIL: gdb.base/unwind-on-each-insn-amd64.exp: foo: instruction 6: bt 2
FAIL: gdb.base/unwind-on-each-insn-amd64.exp: foo: instruction 6: up
FAIL: gdb.base/unwind-on-each-insn-amd64.exp: foo: instruction 6: $sp_value == $::main_sp
FAIL: gdb.base/unwind-on-each-insn-amd64.exp: foo: instruction 6: $fba_value == $::main_fba
FAIL: gdb.base/unwind-on-each-insn-amd64.exp: foo: instruction 6: [string equal $fid $::main_fid]

Here the read happens below the first element of the line
table, and the test failure depends on the value that is
read from there.

It also happens that std::lower_bound returns a pointer exactly at the upper
bound of the line table, also here the read value is undefined, that happens
in this test:

FAIL: gdb.dwarf2/dw2-epilogue-begin.exp: confirm watchpoint doesn't trigger

Fixes: 528b729be1a2 ("gdb/dwarf2: Add support for DW_LNS_set_epilogue_begin in line-table")
Co-Authored-By: Tom de Vries <tdevries@suse.de>
PR symtab/31268
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=31268

diff --git a/gdb/symtab.c b/gdb/symtab.c
index 63b780bb66ed9f4cf445bcfa4b500f43e67babf7..06b32c0388446886f1b5418d0d7ba1d00e998ebb 100644 (file)
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -4157,6 +4157,9 @@ find_epilogue_using_linetable (CORE_ADDR func_addr)
   if (!find_pc_partial_function (func_addr, nullptr, &start_pc, &end_pc))
     return {};
 
+  /* While the standard allows for multiple points marked with epilogue_begin
+     in the same function, for performance reasons, this function will only
+     find the last address that sets this flag for a given block.  */
   const struct symtab_and_line sal = find_pc_line (start_pc, 0);
   if (sal.symtab != nullptr && sal.symtab->language () != language_asm)
     {
@@ -4167,24 +4170,95 @@ find_epilogue_using_linetable (CORE_ADDR func_addr)
        = unrelocated_addr (end_pc - objfile->text_section_offset ());
 
       const linetable *linetable = sal.symtab->linetable ();
-      /* This should find the last linetable entry of the current function.
-        It is probably where the epilogue begins, but since the DWARF 5
-        spec doesn't guarantee it, we iterate backwards through the function
-        until we either find it or are sure that it doesn't exist.  */
+      if (linetable == nullptr || linetable->nitems == 0)
+       {
+         /* Empty line table.  */
+         return {};
+       }
+
+      /* Find the first linetable entry after the current function.  Note that
+        this also may be an end_sequence entry.  */
       auto it = std::lower_bound
        (linetable->item, linetable->item + linetable->nitems, unrel_end,
         [] (const linetable_entry &lte, unrelocated_addr pc)
         {
           return lte.unrelocated_pc () < pc;
         });
+      if (it == linetable->item + linetable->nitems)
+       {
+         /* We couldn't find either:
+            - a linetable entry starting the function after the current
+              function, or
+            - an end_sequence entry that terminates the current function
+              at unrel_end.
+
+            This can happen when the linetable doesn't describe the full
+            extent of the function.  This can be triggered with:
+            - compiler-generated debug info, in the cornercase that the pc
+              with which we call find_pc_line resides in a different file
+              than unrel_end, or
+            - invalid dwarf assembly debug info.
+            In the former case, there's no point in iterating further, simply
+            return "not found".  In the latter case, there's no current
+            incentive to attempt to support this, so handle this
+            conservatively and do the same.  */
+         return {};
+       }
 
-      while (it->unrelocated_pc () >= unrel_start)
-      {
-       if (it->epilogue_begin)
-         return {it->pc (objfile)};
-       it --;
-      }
+      if (unrel_end < it->unrelocated_pc ())
+       {
+         /* We found a line entry that starts past the end of the
+            function.  This can happen if the previous entry straddles
+            two functions, which shouldn't happen with compiler-generated
+            debug info.  Handle the corner case conservatively.  */
+         return {};
+       }
+      gdb_assert (unrel_end == it->unrelocated_pc ());
+
+      /* Move to the last linetable entry of the current function.  */
+      if (it == &linetable->item[0])
+       {
+         /* Doing it-- would introduce undefined behaviour, avoid it by
+            explicitly handling this case.  */
+         return {};
+       }
+      it--;
+      if (it->unrelocated_pc () < unrel_start)
+       {
+         /* Not in the current function.  */
+         return {};
+       }
+      gdb_assert (it->unrelocated_pc () < unrel_end);
+
+      /* We're at the the last linetable entry of the current function.  This
+        is probably where the epilogue begins, but since the DWARF 5 spec
+        doesn't guarantee it, we iterate backwards through the current
+        function until we either find the epilogue beginning, or are sure
+        that it doesn't exist.  */
+      for (; it >= &linetable->item[0]; it--)
+       {
+         if (it->unrelocated_pc () < unrel_start)
+           {
+             /* No longer in the current function.  */
+             break;
+           }
+
+         if (it->epilogue_begin)
+           {
+             /* Found the beginning of the epilogue.  */
+             return {it->pc (objfile)};
+           }
+
+         if (it == &linetable->item[0])
+           {
+             /* No more entries in the current function.
+                Doing it-- would introduce undefined behaviour, avoid it by
+                explicitly handling this case.  */
+             break;
+           }
+       }
     }
+
   return {};
 }