public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] gdb/dwarf2: Add support for DW_LNS_set_epilogue_begin in line-table
@ 2023-12-21 10:29 Guinevere Larsen
  2023-12-22 16:34 ` Tom Tromey
  2024-01-10 16:57 ` Simon Marchi
  0 siblings, 2 replies; 8+ messages in thread
From: Guinevere Larsen @ 2023-12-21 10:29 UTC (permalink / raw)
  To: gdb-patches; +Cc: Guinevere Larsen

This commit adds a mechanism for GDB to detect the linetable opcode
DW_LNS_set_epilogue_begin. This opcode is set by compilers to indicate
that a certain instruction marks the point where the frame is destroyed.

While the standard allows for multiple points marked with epilogue_begin
in the same function, for performance reasons, the function that
searches for the epilogue address will only find the last address that
sets this flag for a given block.

This commit also changes amd64_stack_frame_destroyed_p_1 to attempt to
use the epilogue begin directly, and only if an epilogue can't be found
will it attempt heuristics based on the current instruction.

Finally, this commit also changes the dwarf assembler to be able to emit
epilogue-begin instructions, to make it easier to test this patch
---
 gdb/NEWS                                      |   5 +
 gdb/amd64-tdep.c                              |   8 +
 gdb/buildsym.c                                |   1 +
 gdb/buildsym.h                                |   4 +
 gdb/doc/gdb.texinfo                           |  14 +-
 gdb/dwarf2/read.c                             |  14 +-
 gdb/symmisc.c                                 |   4 +-
 gdb/symtab.c                                  |  41 +++++
 gdb/symtab.h                                  |  11 ++
 gdb/testsuite/gdb.dwarf2/dw2-epilogue-begin.c |  51 ++++++
 .../gdb.dwarf2/dw2-epilogue-begin.exp         | 173 ++++++++++++++++++
 gdb/testsuite/gdb.dwarf2/dw2-ranges-base.exp  |   2 +-
 gdb/testsuite/lib/dwarf.exp                   |  13 +-
 13 files changed, 328 insertions(+), 13 deletions(-)
 create mode 100644 gdb/testsuite/gdb.dwarf2/dw2-epilogue-begin.c
 create mode 100644 gdb/testsuite/gdb.dwarf2/dw2-epilogue-begin.exp

diff --git a/gdb/NEWS b/gdb/NEWS
index 534e2e7f364..3d7da83086d 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -16,6 +16,11 @@ disassemble
   command will now give an error.  Previously the 'b' flag would
   always override the 'r' flag.
 
+maintenance info line-table
+  Add an EPILOGUE-BEGIN column to the output of the command.  It indicates
+  if the line is considered the start of the epilgoue, and thus a point at
+  which the frame can be considered destroyed.
+
 * New commands
 
 info missing-debug-handler
diff --git a/gdb/amd64-tdep.c b/gdb/amd64-tdep.c
index 0d2e02a496b..916e3360753 100644
--- a/gdb/amd64-tdep.c
+++ b/gdb/amd64-tdep.c
@@ -2840,6 +2840,14 @@ amd64_stack_frame_destroyed_p_1 (struct gdbarch *gdbarch, CORE_ADDR pc)
 {
   gdb_byte insn;
 
+  std::optional<CORE_ADDR> epilogue = find_epilogue_using_linetable (pc);
+
+  /* PC is pointing at the next instruction to be executed. If it is
+     equal to the epilogue start, it means we're right before it starts,
+     so the stack is still valid.  */
+  if (epilogue)
+    return pc > epilogue;
+
   if (target_read_memory (pc, &insn, 1))
     return 0;   /* Can't read memory at pc.  */
 
diff --git a/gdb/buildsym.c b/gdb/buildsym.c
index 32d60cc22d4..04e2cbdec8e 100644
--- a/gdb/buildsym.c
+++ b/gdb/buildsym.c
@@ -667,6 +667,7 @@ buildsym_compunit::record_line (struct subfile *subfile, int line,
   e.is_stmt = (flags & LEF_IS_STMT) != 0;
   e.set_unrelocated_pc (pc);
   e.prologue_end = (flags & LEF_PROLOGUE_END) != 0;
+  e.epilogue_begin = (flags & LEF_EPILOGUE_BEGIN) != 0;
 }
 
 \f
diff --git a/gdb/buildsym.h b/gdb/buildsym.h
index 98dc8f02874..26a8bff1278 100644
--- a/gdb/buildsym.h
+++ b/gdb/buildsym.h
@@ -131,6 +131,10 @@ enum linetable_entry_flag : unsigned
   /* Indicates this PC is a good location to place a breakpoint at the first
      instruction past a function prologue.  */
   LEF_PROLOGUE_END = 1 << 2,
+
+  /* Indicated that this PC is part of the epilogue of a function, making
+     software watchpoints unreliable.  */
+  LEF_EPILOGUE_BEGIN = 1 << 3,
 };
 DEF_ENUM_FLAGS_TYPE (enum linetable_entry_flag, linetable_entry_flags);
 
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 7a5d357b99b..ce58629acc3 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -20453,25 +20453,27 @@ objfile: /home/gnu/build/a.out ((struct objfile *) 0x6120000e0d40)
 compunit_symtab: simple.cpp ((struct compunit_symtab *) 0x6210000ff450)
 symtab: /home/gnu/src/simple.cpp ((struct symtab *) 0x6210000ff4d0)
 linetable: ((struct linetable *) 0x62100012b760):
-INDEX  LINE   ADDRESS            IS-STMT PROLOGUE-END
+INDEX  LINE   ADDRESS            IS-STMT PROLOGUE-END EPILOGUE-BEGIN
 0      3      0x0000000000401110 Y
-1      4      0x0000000000401114 Y       Y
+1      4      0x0000000000401114 Y       Y            Y
 2      9      0x0000000000401120 Y
 3      10     0x0000000000401124 Y       Y
-4      10     0x0000000000401129
+4      10     0x0000000000401129 Y                    Y
 5      15     0x0000000000401130 Y
 6      16     0x0000000000401134 Y       Y
 7      16     0x0000000000401139
-8      21     0x0000000000401140 Y
+8      21     0x0000000000401140 Y                    Y
 9      22     0x000000000040114f Y       Y
-10     22     0x0000000000401154
+10     22     0x0000000000401154                      Y
 11     END    0x000000000040115a Y
 @end smallexample
 @noindent
 The @samp{IS-STMT} column indicates if the address is a recommended breakpoint
 location to represent a line or a statement.  The @samp{PROLOGUE-END} column
 indicates that a given address is an adequate place to set a breakpoint at the
-first instruction following a function prologue.
+first instruction following a function prologue.  The @samp{EPILOGUE-BEGIN}
+column indicates that a given address marks the point where a block's frame is
+destroyed, making local variables hard or impossible to find.
 
 @kindex set always-read-ctf [on|off]
 @kindex show always-read-ctf
diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index 37cabe52ecc..b203d831663 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -17961,6 +17961,7 @@ class lnp_state_machine
     record_line (false);
     m_discriminator = 0;
     m_flags &= ~LEF_PROLOGUE_END;
+    m_flags &= ~LEF_EPILOGUE_BEGIN;
   }
 
   /* Handle DW_LNE_end_sequence.  */
@@ -17975,6 +17976,11 @@ class lnp_state_machine
     m_flags |= LEF_PROLOGUE_END;
   }
 
+  void handle_set_epilogue_begin ()
+  {
+    m_flags |= LEF_EPILOGUE_BEGIN;
+  }
+
 private:
   /* Advance the line by LINE_DELTA.  */
   void advance_line (int line_delta)
@@ -18064,6 +18070,7 @@ lnp_state_machine::handle_special_opcode (unsigned char op_code)
   record_line (false);
   m_discriminator = 0;
   m_flags &= ~LEF_PROLOGUE_END;
+  m_flags &= ~LEF_EPILOGUE_BEGIN;
 }
 
 void
@@ -18202,11 +18209,13 @@ lnp_state_machine::record_line (bool end_sequence)
     {
       gdb_printf (gdb_stdlog,
 		  "Processing actual line %u: file %u,"
-		  " address %s, is_stmt %u, prologue_end %u, discrim %u%s\n",
+		  " address %s, is_stmt %u, prologue_end %u,"
+		  " epilogue_begin %u, discrim %u%s\n",
 		  m_line, m_file,
 		  paddress (m_gdbarch, (CORE_ADDR) m_address),
 		  (m_flags & LEF_IS_STMT) != 0,
 		  (m_flags & LEF_PROLOGUE_END) != 0,
+		  (m_flags & LEF_EPILOGUE_BEGIN) != 0,
 		  m_discriminator,
 		  (end_sequence ? "\t(end sequence)" : ""));
     }
@@ -18509,6 +18518,9 @@ dwarf_decode_lines_1 (struct line_header *lh, struct dwarf2_cu *cu,
 	    case DW_LNS_set_prologue_end:
 	      state_machine.handle_set_prologue_end ();
 	      break;
+	    case DW_LNS_set_epilogue_begin:
+	      state_machine.handle_set_epilogue_begin ();
+	      break;
 	    default:
 	      {
 		/* Unknown standard opcode, ignore it.  */
diff --git a/gdb/symmisc.c b/gdb/symmisc.c
index c1a6ab5cd7d..2e500e81071 100644
--- a/gdb/symmisc.c
+++ b/gdb/symmisc.c
@@ -973,13 +973,14 @@ maintenance_print_one_line_table (struct symtab *symtab, void *data)
       /* Leave space for 6 digits of index and line number.  After that the
 	 tables will just not format as well.  */
       struct ui_out *uiout = current_uiout;
-      ui_out_emit_table table_emitter (uiout, 6, -1, "line-table");
+      ui_out_emit_table table_emitter (uiout, 7, -1, "line-table");
       uiout->table_header (6, ui_left, "index", _("INDEX"));
       uiout->table_header (6, ui_left, "line", _("LINE"));
       uiout->table_header (18, ui_left, "rel-address", _("REL-ADDRESS"));
       uiout->table_header (18, ui_left, "unrel-address", _("UNREL-ADDRESS"));
       uiout->table_header (7, ui_left, "is-stmt", _("IS-STMT"));
       uiout->table_header (12, ui_left, "prologue-end", _("PROLOGUE-END"));
+      uiout->table_header (14, ui_left, "epilogue-begin", _("EPILOGUE-BEGIN"));
       uiout->table_body ();
 
       for (int i = 0; i < linetable->nitems; ++i)
@@ -999,6 +1000,7 @@ maintenance_print_one_line_table (struct symtab *symtab, void *data)
 				  CORE_ADDR (item->unrelocated_pc ()));
 	  uiout->field_string ("is-stmt", item->is_stmt ? "Y" : "");
 	  uiout->field_string ("prologue-end", item->prologue_end ? "Y" : "");
+	  uiout->field_string ("epilogue-begin", item->epilogue_begin ? "Y" : "");
 	  uiout->text ("\n");
 	}
     }
diff --git a/gdb/symtab.c b/gdb/symtab.c
index d5d229f9562..7b0559479e1 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -4068,6 +4068,47 @@ skip_prologue_using_sal (struct gdbarch *gdbarch, CORE_ADDR func_addr)
 
 /* See symtab.h.  */
 
+std::optional<CORE_ADDR>
+find_epilogue_using_linetable (CORE_ADDR func_addr)
+{
+  CORE_ADDR start_pc, end_pc;
+
+  if (!find_pc_partial_function (func_addr, nullptr, &start_pc, &end_pc))
+    return {};
+
+  const struct symtab_and_line sal = find_pc_line (start_pc, 0);
+  if (sal.symtab != nullptr && sal.symtab->language () != language_asm)
+    {
+      struct objfile *objfile = sal.symtab->compunit ()->objfile ();
+      unrelocated_addr unrel_start
+	= unrelocated_addr (start_pc - objfile->text_section_offset ());
+      unrelocated_addr unrel_end
+	= 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.  */
+      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;
+	 });
+
+      while (it->unrelocated_pc () >= unrel_start)
+      {
+	if (it->epilogue_begin)
+	  return {it->pc (objfile)};
+	it --;
+      }
+    }
+  return {};
+}
+
+/* See symtab.h.  */
+
 symbol *
 find_function_alias_target (bound_minimal_symbol msymbol)
 {
diff --git a/gdb/symtab.h b/gdb/symtab.h
index e9406e8a9f9..32e89930a30 100644
--- a/gdb/symtab.h
+++ b/gdb/symtab.h
@@ -1630,6 +1630,9 @@ struct linetable_entry
      function prologue.  */
   bool prologue_end : 1;
 
+  /* True if this location marks the start of the epilogue.  */
+  bool epilogue_begin : 1;
+
 private:
 
   /* The address for this entry.  */
@@ -2926,4 +2929,12 @@ extern void info_sources_worker (struct ui_out *uiout,
 				 bool group_by_objfile,
 				 const info_sources_filter &filter);
 
+/* This function returns the address at which the function epilogue begins,
+   according to the linetable.
+
+   Returns an empty optional if EPILOGUE_BEGIN is never set in the
+   linetable.  */
+
+std::optional<CORE_ADDR> find_epilogue_using_linetable (CORE_ADDR func_addr);
+
 #endif /* !defined(SYMTAB_H) */
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-epilogue-begin.c b/gdb/testsuite/gdb.dwarf2/dw2-epilogue-begin.c
new file mode 100644
index 00000000000..b760099b9cb
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/dw2-epilogue-begin.c
@@ -0,0 +1,51 @@
+/* Copyright 2023 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+void
+__attribute__((used))
+trivial (void)
+{
+  asm ("trivial_label: .global trivial_label");		/* trivial function */
+}
+
+char global;
+
+void
+watch (void)
+{							/* watch start */
+  asm ("watch_label: .global watch_label");
+  asm ("mov $0x0, %rax");
+  int local = 0;					/* watch prologue */
+
+  asm ("watch_start: .global watch_start");
+  asm ("mov $0x1, %rax");
+  local = 1;						/* watch assign */
+  asm ("watch_reassign: .global watch_reassign");
+  asm ("mov $0x2, %rax");
+  local = 2;						/* watch reassign */
+  asm ("watch_end: .global watch_end");			/* watch end */
+}
+
+int
+main (void)
+{							/* main prologue */
+  asm ("main_label: .global main_label");
+  global = 0;
+  asm ("main_fun_call: .global main_fun_call");
+  watch ();						/* main function call */
+  asm ("main_epilogue: .global main_epilogue");
+  global = 10;
+  return 0;						/* main end */
+}
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-epilogue-begin.exp b/gdb/testsuite/gdb.dwarf2/dw2-epilogue-begin.exp
new file mode 100644
index 00000000000..0109f3e7d79
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/dw2-epilogue-begin.exp
@@ -0,0 +1,173 @@
+# Copyright 2022-2023 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Check that GDB can honor the epilogue_begin flag the compiler can place
+# in the line-table data.
+# We test 2 things: 1. that a software watchpoint triggered in an epilogue
+# is correctly ignored
+# 2. that GDB can mark the same line as both prologue and epilogue
+
+load_lib dwarf.exp
+
+# This test can only be run on targets which support DWARF-2 and use gas.
+require dwarf2_support
+# restricted to x86 to make it simpler to follow a variable
+require is_x86_64_m64_target
+
+standard_testfile .c .S
+
+set trivial_line [gdb_get_line_number "trivial function"]
+set main_prologue [gdb_get_line_number "main prologue"]
+set main_epilogue [gdb_get_line_number "main end"]
+set watch_start_line [gdb_get_line_number "watch start"]
+
+set asm_file [standard_output_file $srcfile2]
+
+# The producer will be set to clang because at the time of writing
+# we only care about epilogues if the producer is clang.  When the
+# producer is GCC, variables use CFA locations, so watchpoints can
+# continue working even on epilogues.
+Dwarf::assemble $asm_file {
+    global srcdir subdir srcfile srcfile2
+    global trivial_line main_prologue main_epilogue watch_start_line
+    declare_labels lines_label
+
+    get_func_info main
+    get_func_info trivial
+    get_func_info watch
+
+    cu {} {
+	compile_unit {
+	    {language @DW_LANG_C}
+	    {name dw2-prologue-end.c}
+	    {stmt_list ${lines_label} DW_FORM_sec_offset}
+	    {producer "clang version 17.0.1"}
+	} {
+	    declare_labels char_label
+
+	    char_label: base_type {
+		{name char}
+		{encoding @DW_ATE_signed}
+		{byte_size 1 DW_FORM_sdata}
+	    }
+
+	    subprogram {
+		{external 1 flag}
+		{name trivial}
+		{low_pc $trivial_start addr}
+		{high_pc "$trivial_start + $trivial_len" addr}
+	    }
+	    subprogram {
+		{external 1 flag}
+		{name watch}
+		{low_pc $watch_start addr}
+		{high_pc "$watch_start + $watch_len" addr}
+	    } {
+		DW_TAG_variable {
+		    {name local}
+		    {type :$char_label}
+		    {DW_AT_location {DW_OP_reg0} SPECIAL_expr}
+		}
+	    }
+	    subprogram {
+		{external 1 flag}
+		{name main}
+		{low_pc $main_start addr}
+		{high_pc "$main_start + $main_len" addr}
+	    }
+	}
+    }
+
+    lines {version 5} lines_label {
+	set diridx [include_dir "${srcdir}/${subdir}"]
+	file_name "$srcfile" $diridx
+
+	program {
+	    DW_LNS_set_file $diridx
+	    DW_LNE_set_address $trivial_start
+	    line $trivial_line
+	    DW_LNS_set_prologue_end
+	    DW_LNS_set_epilogue_begin
+	    DW_LNS_copy
+
+	    DW_LNE_set_address watch
+	    line $watch_start_line
+	    DW_LNS_copy
+
+	    DW_LNE_set_address watch_start
+	    line [gdb_get_line_number "watch assign"]
+	    DW_LNS_set_prologue_end
+	    DW_LNS_copy
+
+	    DW_LNE_set_address watch_reassign
+	    line [gdb_get_line_number "watch reassign"]
+	    DW_LNS_set_epilogue_begin
+	    DW_LNS_copy
+
+	    DW_LNE_set_address watch_end
+	    line [gdb_get_line_number "watch end"]
+	    DW_LNS_copy
+
+	    DW_LNE_set_address $main_start
+	    line $main_prologue
+	    DW_LNS_set_prologue_end
+	    DW_LNS_copy
+
+	    DW_LNE_set_address main_fun_call
+	    line [gdb_get_line_number "main function call"]
+	    DW_LNS_copy
+
+	    DW_LNE_set_address main_epilogue
+	    line $main_epilogue
+	    DW_LNS_set_epilogue_begin
+	    DW_LNS_copy
+
+	    DW_LNE_end_sequence
+	}
+    }
+}
+
+if { [prepare_for_testing "failed to prepare" ${testfile} \
+	  [list $srcfile $asm_file] {nodebug}] } {
+    return -1
+}
+
+if ![runto_main] {
+    return -1
+}
+
+# Moving to the scope with a local variable.
+gdb_breakpoint $watch_start_line
+gdb_continue_to_breakpoint "continuing to function" ".*"
+gdb_test "next" "local = 2.*" "stepping to epilogue"
+
+# Forcing software watchpoints because hardware ones don't care if we
+# are in the epilogue or not.
+gdb_test_no_output "set can-use-hw-watchpoints 0"
+
+# Test that the software watchpoint will not trigger in this case
+gdb_test "watch local" "\[W|w\]atchpoint .: local" "set watchpoint"
+gdb_test "continue" ".*\[W|w\]atchpoint . deleted.*" \
+    "confirm watchpoint doesn't trigger"
+
+# First we test that the trivial function has a line with both a prologue
+# and an epilogue. Do this by finding a line that has 3 Y columns
+set sep "\[ \t\]"
+set hex_number "0x\[0-9a-f\]+"
+gdb_test_multiple "maint info line-table" "test epilogue in linetable" -lbl {
+    -re "\[0-9\]$sep+$trivial_line$sep+$hex_number$sep+$hex_number$sep+Y$sep+Y$sep+Y" {
+	pass $gdb_test_name
+    }
+}
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-ranges-base.exp b/gdb/testsuite/gdb.dwarf2/dw2-ranges-base.exp
index ee274ee128c..170b0a1bd47 100644
--- a/gdb/testsuite/gdb.dwarf2/dw2-ranges-base.exp
+++ b/gdb/testsuite/gdb.dwarf2/dw2-ranges-base.exp
@@ -174,7 +174,7 @@ gdb_test_multiple "maint info line-table gdb.dwarf2/dw2-ranges-base.c" \
 	-re ".*linetable: \\(\\(struct linetable \\*\\) 0x0\\):\r\nNo line table.\r\n" {
 	    exp_continue
 	}
-	-re ".*linetable: \\(\\(struct linetable \\*\\) $hex\\):\r\nINDEX\[ \t\]+LINE\[ \t\]+REL-ADDRESS\[ \t\]+UNREL-ADDRESS\[ \t\]+IS-STMT\[ \t\]PROLOGUE-END *\r\n" {
+	-re ".*linetable: \\(\\(struct linetable \\*\\) $hex\\):\r\nINDEX\[ \t\]+LINE\[ \t\]+REL-ADDRESS\[ \t\]+UNREL-ADDRESS\[ \t\]+IS-STMT\[ \t\]PROLOGUE-END\[ \t\]EPILOGUE-BEGIN *\r\n" {
 	    exp_continue
 	}
     }
diff --git a/gdb/testsuite/lib/dwarf.exp b/gdb/testsuite/lib/dwarf.exp
index a9b5be859a8..934cc47d297 100644
--- a/gdb/testsuite/lib/dwarf.exp
+++ b/gdb/testsuite/lib/dwarf.exp
@@ -2387,13 +2387,13 @@ namespace eval Dwarf {
 	_op .byte $_default_is_stmt "default_is_stmt"
 	_op .byte 1 "line_base"
 	_op .byte 1 "line_range"
-	_op .byte 11 "opcode_base"
+	_op .byte 12 "opcode_base"
 
 	# The standard_opcode_lengths table.  The number of arguments
-	# for each of the standard opcodes.  Generating 10 entries here
-	# matches the use of 11 in the opcode_base above.  These 10
+	# for each of the standard opcodes.  Generating 11 entries here
+	# matches the use of 12 in the opcode_base above.  These 10
 	# entries match the 9 standard opcodes for DWARF2 plus
-	# DW_LNS_prologue_end from DWARF3.
+	# DW_LNS_prologue_end and DW_LNS_epilogue_begin from DWARF3.
 	_op .byte 0 "standard opcode 1"
 	_op .byte 1 "standard opcode 2"
 	_op .byte 1 "standard opcode 3"
@@ -2404,6 +2404,7 @@ namespace eval Dwarf {
 	_op .byte 0 "standard opcode 8"
 	_op .byte 1 "standard opcode 9"
 	_op .byte 0 "standard opcode 10"
+	_op .byte 0 "standard opcode 11"
 
 	# Add a directory entry to the line table header's directory table.
 	#
@@ -2601,6 +2602,10 @@ namespace eval Dwarf {
 		_op .byte 0x0a
 	    }
 
+	    proc DW_LNS_set_epilogue_begin {} {
+		_op .byte 0x0b
+	    }
+
 	    proc DW_LNS_advance_pc {offset} {
 		_op .byte 2
 		_op .uleb128 ${offset}
-- 
2.42.0


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

* Re: [PATCH] gdb/dwarf2: Add support for DW_LNS_set_epilogue_begin in line-table
  2023-12-21 10:29 [PATCH] gdb/dwarf2: Add support for DW_LNS_set_epilogue_begin in line-table Guinevere Larsen
@ 2023-12-22 16:34 ` Tom Tromey
  2023-12-22 16:56   ` Guinevere Larsen
  2024-01-02  9:24   ` Guinevere Larsen
  2024-01-10 16:57 ` Simon Marchi
  1 sibling, 2 replies; 8+ messages in thread
From: Tom Tromey @ 2023-12-22 16:34 UTC (permalink / raw)
  To: Guinevere Larsen; +Cc: gdb-patches

>>>>> "Guinevere" == Guinevere Larsen <blarsen@redhat.com> writes:

Guinevere> This commit adds a mechanism for GDB to detect the linetable opcode
Guinevere> DW_LNS_set_epilogue_begin. This opcode is set by compilers to indicate
Guinevere> that a certain instruction marks the point where the frame is destroyed.

Thanks for doing this.  It looks good to me.
Approved-By: Tom Tromey <tom@tromey.com>

Tom

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

* Re: [PATCH] gdb/dwarf2: Add support for DW_LNS_set_epilogue_begin in line-table
  2023-12-22 16:34 ` Tom Tromey
@ 2023-12-22 16:56   ` Guinevere Larsen
  2024-01-02  9:24   ` Guinevere Larsen
  1 sibling, 0 replies; 8+ messages in thread
From: Guinevere Larsen @ 2023-12-22 16:56 UTC (permalink / raw)
  To: Tom Tromey; +Cc: gdb-patches

On 22/12/2023 17:34, Tom Tromey wrote:
>>>>>> "Guinevere" == Guinevere Larsen <blarsen@redhat.com> writes:
> Guinevere> This commit adds a mechanism for GDB to detect the linetable opcode
> Guinevere> DW_LNS_set_epilogue_begin. This opcode is set by compilers to indicate
> Guinevere> that a certain instruction marks the point where the frame is destroyed.
>
> Thanks for doing this.  It looks good to me.
> Approved-By: Tom Tromey <tom@tromey.com>
>
> Tom
>
Thanks for approving!

I will follow Carl Love's rule of not pushing something right before 
leaving for vacation, and only do it once I'm back from the holidays, 
though :)

-- 
Cheers,
Guinevere Larsen
She/Her/Hers


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

* Re: [PATCH] gdb/dwarf2: Add support for DW_LNS_set_epilogue_begin in line-table
  2023-12-22 16:34 ` Tom Tromey
  2023-12-22 16:56   ` Guinevere Larsen
@ 2024-01-02  9:24   ` Guinevere Larsen
  1 sibling, 0 replies; 8+ messages in thread
From: Guinevere Larsen @ 2024-01-02  9:24 UTC (permalink / raw)
  To: Tom Tromey; +Cc: gdb-patches

On 22/12/2023 17:34, Tom Tromey wrote:
>>>>>> "Guinevere" == Guinevere Larsen <blarsen@redhat.com> writes:
> Guinevere> This commit adds a mechanism for GDB to detect the linetable opcode
> Guinevere> DW_LNS_set_epilogue_begin. This opcode is set by compilers to indicate
> Guinevere> that a certain instruction marks the point where the frame is destroyed.
>
> Thanks for doing this.  It looks good to me.
> Approved-By: Tom Tromey <tom@tromey.com>
>
> Tom
>
I'm back from vacations! I've pushed this patch

-- 
Cheers,
Guinevere Larsen
She/Her/Hers


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

* Re: [PATCH] gdb/dwarf2: Add support for DW_LNS_set_epilogue_begin in line-table
  2023-12-21 10:29 [PATCH] gdb/dwarf2: Add support for DW_LNS_set_epilogue_begin in line-table Guinevere Larsen
  2023-12-22 16:34 ` Tom Tromey
@ 2024-01-10 16:57 ` Simon Marchi
  2024-01-10 17:01   ` Guinevere Larsen
  1 sibling, 1 reply; 8+ messages in thread
From: Simon Marchi @ 2024-01-10 16:57 UTC (permalink / raw)
  To: Guinevere Larsen, gdb-patches

On 12/21/23 05:29, Guinevere Larsen wrote:
> This commit adds a mechanism for GDB to detect the linetable opcode
> DW_LNS_set_epilogue_begin. This opcode is set by compilers to indicate
> that a certain instruction marks the point where the frame is destroyed.
> 
> While the standard allows for multiple points marked with epilogue_begin
> in the same function, for performance reasons, the function that
> searches for the epilogue address will only find the last address that
> sets this flag for a given block.
> 
> This commit also changes amd64_stack_frame_destroyed_p_1 to attempt to
> use the epilogue begin directly, and only if an epilogue can't be found
> will it attempt heuristics based on the current instruction.
> 
> Finally, this commit also changes the dwarf assembler to be able to emit
> epilogue-begin instructions, to make it easier to test this patch

Starting with this commit, I see:

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]

Do you see it?

Simon

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

* Re: [PATCH] gdb/dwarf2: Add support for DW_LNS_set_epilogue_begin in line-table
  2024-01-10 16:57 ` Simon Marchi
@ 2024-01-10 17:01   ` Guinevere Larsen
  2024-01-10 17:59     ` Tom Tromey
  0 siblings, 1 reply; 8+ messages in thread
From: Guinevere Larsen @ 2024-01-10 17:01 UTC (permalink / raw)
  To: Simon Marchi, gdb-patches

On 10/01/2024 17:57, Simon Marchi wrote:
> On 12/21/23 05:29, Guinevere Larsen wrote:
>> This commit adds a mechanism for GDB to detect the linetable opcode
>> DW_LNS_set_epilogue_begin. This opcode is set by compilers to indicate
>> that a certain instruction marks the point where the frame is destroyed.
>>
>> While the standard allows for multiple points marked with epilogue_begin
>> in the same function, for performance reasons, the function that
>> searches for the epilogue address will only find the last address that
>> sets this flag for a given block.
>>
>> This commit also changes amd64_stack_frame_destroyed_p_1 to attempt to
>> use the epilogue begin directly, and only if an epilogue can't be found
>> will it attempt heuristics based on the current instruction.
>>
>> Finally, this commit also changes the dwarf assembler to be able to emit
>> epilogue-begin instructions, to make it easier to test this patch
> Starting with this commit, I see:
>
> 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]
>
> Do you see it?
>
Hi Simon,

I don't see anything wrong with this test locally, I get 179 expected 
passes. Could it be some difference in config?

-- 
Cheers,
Guinevere Larsen
She/Her/Hers


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

* Re: [PATCH] gdb/dwarf2: Add support for DW_LNS_set_epilogue_begin in line-table
  2024-01-10 17:01   ` Guinevere Larsen
@ 2024-01-10 17:59     ` Tom Tromey
  2024-01-11 17:02       ` Tom Tromey
  0 siblings, 1 reply; 8+ messages in thread
From: Tom Tromey @ 2024-01-10 17:59 UTC (permalink / raw)
  To: Guinevere Larsen; +Cc: Simon Marchi, gdb-patches

>>>>> "Guinevere" == Guinevere Larsen <blarsen@redhat.com> writes:

Guinevere> I don't see anything wrong with this test locally, I get 179 expected
Guinevere> passes. Could it be some difference in config?

FWIW it works for me on x86-64 Fedora 38, using the system gcc.
I was wondering if there could be a compiler difference but I see the
test uses a .s file, so maybe not.

Tom

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

* Re: [PATCH] gdb/dwarf2: Add support for DW_LNS_set_epilogue_begin in line-table
  2024-01-10 17:59     ` Tom Tromey
@ 2024-01-11 17:02       ` Tom Tromey
  0 siblings, 0 replies; 8+ messages in thread
From: Tom Tromey @ 2024-01-11 17:02 UTC (permalink / raw)
  To: Tom Tromey; +Cc: Guinevere Larsen, Simon Marchi, gdb-patches

Guinevere> I don't see anything wrong with this test locally, I get 179 expected
Guinevere> passes. Could it be some difference in config?

Tom> FWIW it works for me on x86-64 Fedora 38, using the system gcc.
Tom> I was wondering if there could be a compiler difference but I see the
Tom> test uses a .s file, so maybe not.

I found out today that it fails in my ASAN build but not my regular build.
I didn't see any ASAN warnings in the log, though.

Tom

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

end of thread, other threads:[~2024-01-11 17:02 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-12-21 10:29 [PATCH] gdb/dwarf2: Add support for DW_LNS_set_epilogue_begin in line-table Guinevere Larsen
2023-12-22 16:34 ` Tom Tromey
2023-12-22 16:56   ` Guinevere Larsen
2024-01-02  9:24   ` Guinevere Larsen
2024-01-10 16:57 ` Simon Marchi
2024-01-10 17:01   ` Guinevere Larsen
2024-01-10 17:59     ` Tom Tromey
2024-01-11 17:02       ` Tom Tromey

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