[PATCH v4 0/4] GDB support for DW_AT_trampoline

[PATCH v4 0/4] GDB support for DW_AT_trampoline

[Abdul Basit Ijaz]

From: "Ijaz, Abdul B" <abdul.b.ijaz@intel.com>

Hi,

please find attached a series of patches that attempts to enable GDB to
handle the DW_AT_trampoline attribute on functions and inlined
functions.

DW_AT_trampoline can be emitted by the compiler for functions that are
compiler generated trampolines (e.g. wrapping other function calls).
In doing so, the compiler can specify a target as the value of
DW_AT_trampoline which the trampoline is wrapping.

This series enables GDB to recognize the DW_AT_trampoline and store the
target for a given function.  Patch 3 adapts GDB's stepping behavior
when dealing with trampolines and attempts to - by default - hide these
trampolines from the user.  When about to step into a trampoline,
instead, GDB will try and step through the trampoline and directly
towards the target.  Patch 4 in V2 now skip trampoline frames
in the stack for printing or finish command.  A new setting has been
introduced to turn off this modified stepping behavior or printing of stack
for trampolines.

The motivation for these patches comes from ifx which emits the
attribute for some of its compiler generated functions.  As I do not
know of any other compiler (especially gcc/gfortran) emitting
DW_AT_trampoline, I added a gdb.dwarf2 test to this series in order to
test the trampoline handling within GDB.

Changes since V3:
  * Fixed reverse stepping for trampolines.
  * Use mangled name in case it is available for the trampoline targets.
  * Added comments, reorder or rename variables as per the feedback in V3.

V3 series of patches can be found here:
https://sourceware.org/pipermail/gdb-patches/2023-July/200793.html

No regression seen on testing x64/native-extended-gdbserver(-m32)/x32/
native-gdbserver test configurations for these changes using Ubuntu 22.

Abdul Basit Ijaz (1):
  gdb: Skip trampoline frames in the stack for printing or finish
    command.

Abdul Basit Ijaz (1):
  gdb: Skip trampoline frames in the stack for printing or finish
    command.

Nils-Christian Kempke (3):
  gdb, dwarf: add support for DW_AT_trampoline in DWARF reader
  gdb/symtab: add lookup for trampoline functions
  gdb/infrun: handle stepping through functions with DW_AT_trampoline

 gdb/NEWS                                      |  15 ++
 gdb/doc/gdb.texinfo                           |  47 ++++
 gdb/dwarf2/read.c                             |  49 +++-
 gdb/gdbtypes.c                                |  34 ++-
 gdb/gdbtypes.h                                | 113 +++++++-
 gdb/infcmd.c                                  |  13 +
 gdb/infrun.c                                  |  86 +++++-
 gdb/infrun.h                                  |  10 +
 gdb/stack.c                                   |   8 +
 gdb/symtab.c                                  |  80 ++++++
 gdb/symtab.h                                  |  19 ++
 .../gdb.dwarf2/dw2-function-trampolines.c     |  80 ++++++
 .../gdb.dwarf2/dw2-function-trampolines.exp   | 245 ++++++++++++++++++
 gdb/testsuite/gdb.fortran/func-trampoline.exp |  69 +++++
 gdb/testsuite/gdb.fortran/func-trampoline.f90 |  39 +++
 .../gdb.fortran/mixed-lang-stack.exp          |  10 +-
 16 files changed, 903 insertions(+), 14 deletions(-)
 create mode 100644 gdb/testsuite/gdb.dwarf2/dw2-function-trampolines.c
 create mode 100644 gdb/testsuite/gdb.dwarf2/dw2-function-trampolines.exp
 create mode 100644 gdb/testsuite/gdb.fortran/func-trampoline.exp
 create mode 100644 gdb/testsuite/gdb.fortran/func-trampoline.f90

-- 
2.34.1

Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva  
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928

[PATCH v4 1/4] gdb, dwarf: add support for DW_AT_trampoline in DWARF reader

[Abdul Basit Ijaz]

From: Nils-Christian Kempke <nils-christian.kempke@intel.com>

DW_AT_trampoline can be used to describe compiler generated functions
that serve some intermediary purpose on making a call to another
function.  A compiler can emit this tag in order to help a debugger hide
the trampolines from a user.

The attribute is only applicable to DW_TAG_subroutine and
DW_TAG_inlined_subroutine tags.  It contains information about the
trampoline target either as a reference to its DIE, as its address or
its name.  DW_AT_trampoline can also be a flag indicating that the DIE
is a trampoline or not without specifying the target (e.g. if it is
unknown).

This patch adds support to GDB for reading the DW_AT_trampoline
attribute.  It stores the attribute and its value in the type_specific
part of a GDB type.  This patch is implemented in preparation of the
following patches, which will add a mechanism to hide DW_AT_trampoline
subroutines from the user.

2023-08-01 Nils-Christian Kempke <nils-christian.kempke@intel.com>
---
 gdb/dwarf2/read.c |  49 +++++++++++++++++++-
 gdb/gdbtypes.c    |  34 +++++++++++++-
 gdb/gdbtypes.h    | 113 ++++++++++++++++++++++++++++++++++++++++++++--
 3 files changed, 189 insertions(+), 7 deletions(-)

diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index b1f1c1d7257..56407c618d2 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -14657,6 +14657,53 @@ read_subroutine_type (struct die_info *die, struct dwarf2_cu *cu)
   if (prototyped_function_p (die, cu))
     ftype->set_is_prototyped (true);
 
+  /* If this is a trampoline function store it and its target here.  */
+  attr = dwarf2_attr (die, DW_AT_trampoline, cu);
+  if (attr != nullptr)
+    {
+      TYPE_FUNC_FLAGS (ftype) |= FUNC_TYPE_TRAMPOLINE;
+      TYPE_TRAMPOLINE_TARGET (ftype)
+	= (trampoline_target *) TYPE_ZALLOC (ftype,
+					     sizeof (trampoline_target));
+
+      /* A DW_AT_trampoline can be either an address, a flag, a reference or a
+	 string.  */
+      if (attr->form_is_string ())
+	TYPE_TRAMPOLINE_TARGET (ftype)->set_target_name
+	  (attr->as_string ());
+      else if (attr->form_is_ref ())
+	{
+	  die_info *target_die;
+	  dwarf2_cu *target_cu = cu;
+
+	  target_die = follow_die_ref (die, attr, &target_cu);
+	  const char *target_name;
+	  /* Prefer the mangled name; otherwise compute the demangled one.  */
+	  target_name = dw2_linkage_name (target_die, target_cu);
+	  if (target_name == NULL)
+	    target_name = dwarf2_physname (NULL, target_die, target_cu);
+	  if (target_name == nullptr)
+	    {
+	      complaint (_("DW_AT_trampoline target DIE has no name for"
+			 "referencing DIE %s [in module %s]"),
+			 sect_offset_str (die->sect_off),
+			 objfile_name (objfile));
+	    }
+	  TYPE_TRAMPOLINE_TARGET (ftype)->set_target_name (target_name);
+	}
+      else if (attr->form_is_unsigned ())
+	TYPE_TRAMPOLINE_TARGET (ftype)->set_target_flag (attr->as_boolean ());
+      else
+	{
+	  unrelocated_addr target_addr_reloc = attr->as_address ();
+	  CORE_ADDR target_addr
+	    = cu->per_objfile->relocate (target_addr_reloc);
+	  target_addr = gdbarch_adjust_dwarf2_addr (objfile->arch (),
+						    target_addr);
+	  TYPE_TRAMPOLINE_TARGET (ftype)->set_target_physaddr (target_addr);
+	}
+    }
+
   /* Store the calling convention in the type if it's available in
      the subroutine die.  Otherwise set the calling convention to
      the default value DW_CC_normal.  */
@@ -14674,7 +14721,7 @@ read_subroutine_type (struct die_info *die, struct dwarf2_cu *cu)
      if the DWARF producer set that information.  */
   attr = dwarf2_attr (die, DW_AT_noreturn, cu);
   if (attr && attr->as_boolean ())
-    TYPE_NO_RETURN (ftype) = 1;
+    TYPE_FUNC_FLAGS (ftype) |= FUNC_TYPE_NO_RETURN;
 
   /* We need to add the subroutine type to the die immediately so
      we don't infinitely recurse when dealing with parameters
diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c
index c5272979cb9..6d25cdc944e 100644
--- a/gdb/gdbtypes.c
+++ b/gdb/gdbtypes.c
@@ -5091,6 +5091,33 @@ print_fixed_point_type_info (struct type *type, int spaces)
 	      type->fixed_point_scaling_factor ().str ().c_str ());
 }
 
+/* Print the contents of the TYPE's self_trampoline_target, assuming that its
+   type-specific kind is TYPE_SPECIFIC_FUNC and is_trampoline is not 0.  */
+static void
+print_trampoline_target_info (struct type *type, int spaces)
+{
+  switch (TYPE_TRAMPOLINE_TARGET (type)->target_kind ())
+    {
+    case TRAMPOLINE_TARGET_PHYSADDR:
+      gdb_printf ("%*starget physaddr: 0x%s\n", spaces + 2, "",
+		  print_core_address (type->arch_owner (),
+				      TYPE_TRAMPOLINE_TARGET (type)
+				      ->target_physaddr ()));
+      break;
+    case TRAMPOLINE_TARGET_NAME:
+      gdb_printf ("%*starget name: %s\n", spaces + 2, "",
+		  TYPE_TRAMPOLINE_TARGET (type)->target_name ());
+      break;
+    case TRAMPOLINE_TARGET_FLAG:
+      gdb_printf ("%*starget flag: %d\n", spaces + 2, "",
+		  TYPE_TRAMPOLINE_TARGET (type)->target_flag ());
+      break;
+    default:
+      gdb_assert_not_reached ("unhandled trampoline target kind");
+      break;
+    }
+}
+
 static struct obstack dont_print_type_obstack;
 
 /* Print the dynamic_prop PROP.  */
@@ -5417,6 +5444,10 @@ recursive_dump_type (struct type *type, int spaces)
 	gdb_printf ("%*scalling_convention %d\n", spaces, "",
 		    TYPE_CALLING_CONVENTION (type));
 	/* tail_call_list is not printed.  */
+	gdb_printf ("%*sfunc_type_flags %u\n", spaces, "",
+		    (unsigned int) TYPE_FUNC_FLAGS (type));
+	if (TYPE_IS_TRAMPOLINE (type))
+	  print_trampoline_target_info (type, spaces);
 	break;
 
       case TYPE_SPECIFIC_SELF_TYPE:
@@ -5633,8 +5664,9 @@ copy_type_recursive (struct type *type, htab_t copied_types)
     case TYPE_SPECIFIC_FUNC:
       INIT_FUNC_SPECIFIC (new_type);
       TYPE_CALLING_CONVENTION (new_type) = TYPE_CALLING_CONVENTION (type);
-      TYPE_NO_RETURN (new_type) = TYPE_NO_RETURN (type);
+      TYPE_FUNC_FLAGS (new_type) = TYPE_FUNC_FLAGS (type);
       TYPE_TAIL_CALL_LIST (new_type) = NULL;
+      TYPE_TRAMPOLINE_TARGET (new_type) = TYPE_TRAMPOLINE_TARGET (type);
       break;
     case TYPE_SPECIFIC_FLOATFORMAT:
       TYPE_FLOATFORMAT (new_type) = TYPE_FLOATFORMAT (type);
diff --git a/gdb/gdbtypes.h b/gdb/gdbtypes.h
index aedaf53cd5d..310d00d0230 100644
--- a/gdb/gdbtypes.h
+++ b/gdb/gdbtypes.h
@@ -106,6 +106,21 @@ enum type_instance_flag_value : unsigned
 
 DEF_ENUM_FLAGS_TYPE (enum type_instance_flag_value, type_instance_flags);
 
+/* * Define flags for function types.  */
+enum func_type_flag_value : unsigned
+{
+  /* * Flag indicates, whether this function normally returns to its
+     caller.  It is set from the DW_AT_noreturn attribute if set on
+     the DW_TAG_subprogram.  */
+  FUNC_TYPE_NO_RETURN = (1 << 0),
+
+  /* * Flag is used for functions marked with DW_AT_trampoline.  These
+     are compiler generated wrappers that should be hidden from the user.  */
+  FUNC_TYPE_TRAMPOLINE = (1 << 1)
+};
+
+DEF_ENUM_FLAGS_TYPE (enum func_type_flag_value, func_type_flags);
+
 /* * Not textual.  By default, GDB treats all single byte integers as
    characters (or elements of strings) unless this flag is set.  */
 
@@ -1730,11 +1745,9 @@ struct func_type
 
     ENUM_BITFIELD (dwarf_calling_convention) calling_convention : 8;
 
-    /* * Whether this function normally returns to its caller.  It is
-       set from the DW_AT_noreturn attribute if set on the
-       DW_TAG_subprogram.  */
+    /* * For storing function types defined in eunm func_type_flag_value.  */
 
-    unsigned int is_noreturn : 1;
+    func_type_flags flags;
 
     /* * Only those DW_TAG_call_site's in this function that have
        DW_AT_call_tail_call set are linked in this list.  Function
@@ -1749,6 +1762,87 @@ struct func_type
        contains the method.  */
 
     struct type *self_type;
+
+    struct trampoline_target *self_trampoline_target;
+  };
+
+/* The kind of location held by this call site target.  */
+enum trampoline_target_kind
+  {
+    /* An address.  */
+    TRAMPOLINE_TARGET_PHYSADDR,
+    /* A (mangled) name.  */
+    TRAMPOLINE_TARGET_NAME,
+    /* An flag (target is unknown).  */
+    TRAMPOLINE_TARGET_FLAG,
+  };
+
+/* Target information for trampoline functions.  */
+
+struct trampoline_target
+  {
+    /* Returns what kind of target the trampoline points to.  */
+    trampoline_target_kind target_kind () const
+    {
+      return m_target_kind;
+    }
+
+    /* Set the address for trampoline target.  */
+    void set_target_physaddr (CORE_ADDR physaddr)
+    {
+      m_target_kind = TRAMPOLINE_TARGET_PHYSADDR;
+      m_trampoline_target.physaddr = physaddr;
+    }
+
+    /* Return the address of the trampoline target.  */
+    CORE_ADDR target_physaddr () const
+    {
+      gdb_assert (m_target_kind == TRAMPOLINE_TARGET_PHYSADDR);
+      return m_trampoline_target.physaddr;
+    }
+
+    /* Set the name of the trampoline target.  */
+    void set_target_name (const char *name)
+    {
+      m_target_kind = TRAMPOLINE_TARGET_NAME;
+      m_trampoline_target.name = name;
+    }
+
+    /* Return the name of the trampoline target.  */
+    const char *target_name () const
+    {
+      gdb_assert (m_target_kind == TRAMPOLINE_TARGET_NAME);
+      return m_trampoline_target.name;
+    }
+
+    /* Set the flag for trampoline target subroutine.  */
+    void set_target_flag (bool flag)
+    {
+      m_target_kind = TRAMPOLINE_TARGET_FLAG;
+      m_trampoline_target.flag = flag;
+    }
+
+    /* Return the flag for trampoline target.  */
+    bool target_flag () const
+    {
+      gdb_assert (m_target_kind == TRAMPOLINE_TARGET_FLAG);
+      return m_trampoline_target.flag;
+    }
+
+  private:
+
+    union
+    {
+      /* Address.  */
+      CORE_ADDR physaddr;
+      /* Mangled name.  */
+      const char *name;
+      /* Flag.  */
+      bool flag;
+    } m_trampoline_target;
+
+    /* * Discriminant for union m_trampoline_target.  */
+    ENUM_BITFIELD (trampoline_target_kind) m_target_kind : 2;
   };
 
 /* The type-specific info for TYPE_CODE_FIXED_POINT types.  */
@@ -1891,7 +1985,16 @@ extern void set_type_vptr_basetype (struct type *, struct type *);
 #define TYPE_GNAT_SPECIFIC(thistype) TYPE_MAIN_TYPE(thistype)->type_specific.gnat_stuff
 #define TYPE_DESCRIPTIVE_TYPE(thistype) TYPE_GNAT_SPECIFIC(thistype)->descriptive_type
 #define TYPE_CALLING_CONVENTION(thistype) TYPE_MAIN_TYPE(thistype)->type_specific.func_stuff->calling_convention
-#define TYPE_NO_RETURN(thistype) TYPE_MAIN_TYPE(thistype)->type_specific.func_stuff->is_noreturn
+#define TYPE_FUNC_FLAGS(thistype) \
+  TYPE_MAIN_TYPE(thistype)->type_specific.func_stuff->flags
+#define TYPE_NO_RETURN(thistype) \
+  (TYPE_MAIN_TYPE(thistype)->type_specific.func_stuff->flags \
+   & FUNC_TYPE_NO_RETURN)
+#define TYPE_IS_TRAMPOLINE(thistype) \
+  (TYPE_MAIN_TYPE(thistype)->type_specific.func_stuff->flags \
+   & FUNC_TYPE_TRAMPOLINE)
+#define TYPE_TRAMPOLINE_TARGET(thistype) \
+  TYPE_MAIN_TYPE(thistype)->type_specific.func_stuff->self_trampoline_target
 #define TYPE_TAIL_CALL_LIST(thistype) TYPE_MAIN_TYPE(thistype)->type_specific.func_stuff->tail_call_list
 #define TYPE_BASECLASS(thistype,index) ((thistype)->field (index).type ())
 #define TYPE_N_BASECLASSES(thistype) TYPE_CPLUS_SPECIFIC(thistype)->n_baseclasses
-- 
2.34.1

Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva  
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928

[PATCH v4 2/4] gdb/symtab: add lookup for trampoline functions

[Abdul Basit Ijaz]

From: Nils-Christian Kempke <nils-christian.kempke@intel.com>

In order to query information about the DW_AT_trampoline tag for
subroutines and inlined subroutines, two function were added to symtab.

First, a routine for querying whether the given pc belongs to a block
that is associated with a function (maybe inlined) marked
DW_AT_trampoline.

Second, a routine for querying a trampoline function's target.
Subroutines and inlined subroutines marked with DW_AT_trampoline usually
contain information about the target subroutine they are 'wrapping'/
passing control to.

2023-08-01 Nils-Christian Kempke <nils-christian.kempke@intel.com>
---
 gdb/symtab.c | 68 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 gdb/symtab.h | 14 +++++++++++
 2 files changed, 82 insertions(+)

diff --git a/gdb/symtab.c b/gdb/symtab.c
index 0117a2a59d7..745fd7d5d25 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -73,6 +73,7 @@
 #include "gdbsupport/gdb_string_view.h"
 #include "gdbsupport/pathstuff.h"
 #include "gdbsupport/common-utils.h"
+#include "gdbsupport/symbol.h"
 
 /* Forward declarations for local functions.  */
 
@@ -4051,6 +4052,73 @@ find_function_alias_target (bound_minimal_symbol msymbol)
   return NULL;
 }
 
+/* See symtab.h.  */
+
+bool
+in_trampoline_function (CORE_ADDR pc)
+{
+  /* Find the innermost function containing pc.  This might be an inlined
+     function.  */
+  symbol *sym = find_pc_sect_containing_function (pc,
+						  find_pc_mapped_section (pc));
+  return sym != nullptr && TYPE_IS_TRAMPOLINE (sym->type ());
+}
+
+/* See symtab.h.  */
+
+CORE_ADDR
+find_function_trampoline_target (CORE_ADDR pc)
+{
+  /* Find the innermost function containing pc.  This might be an inlined
+     function.  */
+  symbol *sym = find_pc_sect_containing_function (pc,
+						  find_pc_mapped_section (pc));
+  CORE_ADDR target_address = 0;
+
+  if (sym != nullptr && TYPE_IS_TRAMPOLINE (sym->type ()))
+    {
+      trampoline_target *trampoline = TYPE_TRAMPOLINE_TARGET (sym->type ());
+
+      /* DW_AT_trampoline can be given as an address, name, or flag here (die
+	 references have been resolved as names at this point.  In the case
+	 where DW_AT_trampoline contains a flag we do not know the target
+	 address and return 0.  */
+      if (trampoline->target_kind () == TRAMPOLINE_TARGET_NAME)
+	{
+	  /* Handle both the mangled and demangled PHYSNAME.  */
+	  const char *physname = trampoline->target_name ();
+
+	  /* First, check whether there exists a symbol matching the
+	     physname.  If we cannot find one also check for minimal
+	     symbols.  */
+	  const block *blk = block_for_pc (pc);
+	  struct block_symbol bs = lookup_symbol (physname, blk, VAR_DOMAIN, 0);
+	  if (bs.symbol != nullptr)
+	    {
+	      const struct block *block = bs.symbol->value_block ();
+	      gdb_assert (block != nullptr);
+	      target_address = block->start ();
+	    }
+	  else
+	    {
+	      if (find_minimal_symbol_address (physname, &target_address,
+					       nullptr) != 0)
+		target_address = 0;
+	    }
+	}
+      else if (trampoline->target_kind () == TRAMPOLINE_TARGET_PHYSADDR)
+	{
+	  /* If the function symbol containing this trampoline target has
+	     been relocated we assume the target_address also needs relocation.
+	     If it has not been relocated the offset should be zero.  */
+	  target_address
+	    = (trampoline->target_physaddr ()
+	       + sym->objfile ()->section_offsets[sym->section_index ()]);
+	}
+    }
+
+  return target_address;
+}
 \f
 /* If P is of the form "operator[ \t]+..." where `...' is
    some legitimate operator text, return a pointer to the
diff --git a/gdb/symtab.h b/gdb/symtab.h
index ee4729b14cd..fc2d82e6567 100644
--- a/gdb/symtab.h
+++ b/gdb/symtab.h
@@ -2312,6 +2312,20 @@ extern const struct gnu_ifunc_fns *gnu_ifunc_fns_p;
 
 extern CORE_ADDR find_solib_trampoline_target (frame_info_ptr, CORE_ADDR);
 
+/* Return whether or not the current pc is within a block that belongs to a
+   function that is marked as a trampoline by the compiler.  */
+
+extern bool in_trampoline_function (CORE_ADDR pc);
+
+/* Find the target of a trampoline function marked via the DW_AT_trampoline
+   attribute and return its address.  Returns 0 if the pc is not contained
+   in a trampoline function (inlined or not).  If DW_AT_trampoline
+   is given as a flag, the target is unknown and the function will still return
+   0.  One has to additionally query in_trampoline_function to cover this
+   case.  */
+
+extern CORE_ADDR find_function_trampoline_target (CORE_ADDR pc);
+
 struct symtab_and_line
 {
   /* The program space of this sal.  */
-- 
2.34.1

Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva  
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928

[PATCH v4 3/4] gdb/infrun: handle stepping through functions with DW_AT_trampoline

[Abdul Basit Ijaz]

From: Nils-Christian Kempke <nils-christian.kempke@intel.com>

This patch makes infrun continue stepping into and through trampoline
functions marked via the DW_AT_trampoline in DWARF.  The attribute can
be emitted by the compiler for certain subroutines/inlined subroutines
that are compiler generated and should be hidden from a user.

Mainly, infrun is modified in 3 ways.

First, GDB will now attempt to step through trampoline functions.
Whenever we issued a step command that would make GDB step into a
function that is marked trampoline, GDB will try to step directly towards
the trampoline's 'target' instead and, e.g., not stop at the first
instruction of the trampoline.
The target can be specified by the compiler by the value of
DW_AT_trampoline if its form is either an address, a name, or a DIE
reference.  DW_AT_trampoline is also allowed to be specified as a flag
(containing true or false), in which case the target is assumed to be
unknown.  If GDB successfully finds a target, so if the value of
DW_AT_trampoline was not a flag and could be resolved successfully, GDB
steps directly towards the target and through the trampoline, hiding
the trampoline from the user.  If GDB cannot, however deduce a target,
most likely because the DW_AT_trampoline was given as a flag or because
of broken debug info, it will instead continue inside execution in the
trampoline function until it reaches an instruction that is not
associated with a trampoline function, which is usually the target
function.  It will then stop and give control back to the user.
It should be noted, that there might be the cases, where trampolines
call functions other than the target before the actual target call.  If,
in such a situation, GDB fails to resolve the target, it would resume
execution until stepping into this other function call, and hand back
control to the user, without actually having reached the target.  A
second step would have to be issued by the user to arrive a the target
(by resuming in the trampoline and then until leaving it a second time).
As this is a rather pathological case and no real instance of this is
known, I think the current behavior here is good enough and seems to be
the best GDB can do in such a situation.

Secondly, as trampoline functions normally do not have any real source
code correlation, it is likely that they mostly appear without line
info.
Normally, GDB would skip completely over a function call to a function
that has no source line information, so we would never get to the
aforementioned stepping through a trampoline and target resolution.  To
remedy this, for debug info trampolines, GDB now attempts to step through
them regardless of them having source line information or not.  So
issuing a step at a function call wrapped by a trampoline without source
line information will no longer skip the whole function call, but now
step through the trampoline and attempt to resolve the trampoline target
as described above (so usually, a single step at the call site will step
through the trampoline and towards the target, even if the trampoline had
not source line info).

Last, in all other cases when GDB is about to stop at a location that is
included in a trampoline region (e.g. after a step from the target back
into the trampoline) GDB will instead continue until the trampoline
region is left again and only then give control back to the user.  This
change serves the purpose of allowing stepping back from a target call
through the trampoline without the user noticing the artificial function
call inbetween call site and target.

Together, these changes attempt to hide the trampoline function from the
user while stepping.  Additionally, the skip-trampoline-functions option
has been introduced in infrun.  It is set by default, and, when turned
off, GDB will return to its 'normal' stepping behavior and ignore any
possible DW_AT_trampoline.

As currently only ifx emits the DW_AT_trampoline tag, a test has been
added to gdb.dwarf2 that artificially creates a set of trampoline
functions.

2023-08-01 Nils-Christian Kempke <nils-christian.kempke@intel.com>
---
 gdb/NEWS                                      |  15 ++
 gdb/doc/gdb.texinfo                           |  36 +++
 gdb/infrun.c                                  |  86 +++++-
 gdb/infrun.h                                  |   4 +
 .../gdb.dwarf2/dw2-function-trampolines.c     |  80 ++++++
 .../gdb.dwarf2/dw2-function-trampolines.exp   | 245 ++++++++++++++++++
 6 files changed, 462 insertions(+), 4 deletions(-)
 create mode 100644 gdb/testsuite/gdb.dwarf2/dw2-function-trampolines.c
 create mode 100644 gdb/testsuite/gdb.dwarf2/dw2-function-trampolines.exp

diff --git a/gdb/NEWS b/gdb/NEWS
index 3f414a54fb7..c8cb4410f39 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -594,6 +594,21 @@ platform.
 
 * New commands
 
+set skip-trampoline-functions on|off
+show skip-trampoline-functions
+  This setting is 'on' by default.  When 'on' it controls whether GDB's
+  stepping behavior will recognize function calls that have been marked as
+  trampolines in the debug info.  It improves stepping behavior in that it
+  steps through trampoline code and hides it from the user.  GDB can now step
+  through trampolines that are correctly marked as such in the compiler's
+  debug info.  If the target of a trampoline is unknown, GDB will continue
+  until the trampoline section is left again and only then hand control back
+  to the user.  GDB does this even if the trampoline has no associated line
+  info.  If this is turned off, GDB will step into trampolines if there is
+  line table information for them or step over the trampoline calls if there
+  is no line table information.  Currently, only DWARF trampolines are
+  supported.
+
 maint set backtrace-on-fatal-signal on|off
 maint show backtrace-on-fatal-signal
   This setting is 'on' by default.  When 'on' GDB will print a limited
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 1463f501768..59a93f72895 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -6391,6 +6391,42 @@ debug information.  This is the default.
 Show whether @value{GDBN} will stop in or step over functions without
 source line debug information.
 
+@kindex set skip-trampoline-functions
+@cindex trampoline functions
+@cindex stepping through trampoline functions
+@item set skip-trampoline-functions
+@itemx set skip-trampoline-functions on
+When calling a function in any language, some compilers might generate
+so-called @dfn{trampoline functions}, which wrap the actual function call
+(the target of the trampoline).  The compiler might mark such a trampoline in
+its debug information.  Often, such trampolines do not have any source line
+information associated with them which will lead the @code{step} command to
+behave like a @code{next} and skip the function call completely.
+
+The default is @code{set skip-trampoline-functions on} and it will cause the
+@code{step} command to treat these trampolines differently.  When issuing a
+@code{step} at the call site of a trampoline function if
+@code{skip-trampoline-functions} is set @value{GDBN} will attempt to determine
+the target of the trampoline and then step through the trampoline stopping at
+the target.  If the target could not be found or was not given in the debug
+info, @value{GDBN} will simply continue execution until it leaves the
+trampoline code again, even if the trampoline has no line info associated with
+it.  When returning from a target function call and stepping back into the
+trampoline, @value{GDBN} will again step through the trampoline towards the
+call site.  Additionally, even if stopped in a trampoline function with source
+line information, issuing a @code{step} will prompt @value{GDBN} to resume
+execution until leaving the trampoline region again.  The @code{stepi} command
+is not affected by the setting which is enabled by default.  Currently, only
+DWARF trampolines marked via DW_AT_trampoline are supported by this.
+
+@item set skip-trampoline-functions off
+Causes the @code{step} command to completely ignore any trampoline information
+a compiler might have emitted in its debug info.  Trampolines will be treated
+like any other function when stepping.
+
+@item show skip-trampoline-functions
+Show whether @value{GDBN} tries to skip trampolines or not.
+
 @kindex finish
 @kindex fin @r{(@code{finish})}
 @item finish
diff --git a/gdb/infrun.c b/gdb/infrun.c
index 7efa0617526..0ef9c76d3c6 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -184,6 +184,12 @@ get_previous_thread ()
 
 static bool detach_fork = true;
 
+/* If set (default) GDB will step through functions/inlined subroutines marked
+   DW_AT_trampoline by the compiler.  If false, GDB will ignore the
+   attribute.  */
+
+static bool skip_trampoline_functions = true;
+
 bool debug_infrun = false;
 static void
 show_debug_infrun (struct ui_file *file, int from_tty,
@@ -7328,7 +7334,6 @@ process_event_stop_test (struct execution_control_state *ecs)
 		  != find_pc_function (ecs->event_thread->stop_pc ())))))
     {
       CORE_ADDR stop_pc = ecs->event_thread->stop_pc ();
-      CORE_ADDR real_stop_pc;
 
       infrun_debug_printf ("stepped into subroutine");
 
@@ -7400,8 +7405,46 @@ process_event_stop_test (struct execution_control_state *ecs)
 	 calling routine and the real function), locate the real
 	 function.  That's what tells us (a) whether we want to step
 	 into it at all, and (b) what prologue we want to run to the
-	 end of, if we do step into it.  */
-      real_stop_pc = skip_language_trampoline (frame, stop_pc);
+	 end of, if we do step into it.  For functions marked as
+	 trampoline functions we try to find their target and step
+	 towards it (if skip_trampoline_functions is not set to false by the
+	 user).  If no target can be determined we just step into the
+	 trampoline and hand control back to the user.  */
+      CORE_ADDR real_stop_pc = 0;
+      bool in_trampoline = skip_trampoline_functions
+			   && in_trampoline_function (stop_pc);
+
+      if (in_trampoline)
+	{
+	  if (execution_direction == EXEC_REVERSE)
+	    {
+	      keep_going (ecs);
+	      return;
+	    }
+	  real_stop_pc = find_function_trampoline_target (stop_pc);
+
+	  for (int i = 0; i < MAX_TRAMPOLINE_CHAIN_SIZE
+			  && in_trampoline_function (real_stop_pc); ++i)
+	    {
+		real_stop_pc = find_function_trampoline_target (real_stop_pc);
+		/* Exit if find_function_trampoline_target failed to find the
+		   trampoline target.  Do not try to resolve the trampolines
+		   in this case.  */
+		if (real_stop_pc == 0x0)
+		  break;
+	    }
+
+	  /* If we failed to find a target we will just single step in the
+	     hope of leaving the trampoline again soon.  */
+	  if (real_stop_pc == 0x0)
+	    {
+	      keep_going (ecs);
+	      return;
+	    }
+	}
+
+      if (real_stop_pc == 0)
+	real_stop_pc = skip_language_trampoline (frame, stop_pc);
       if (real_stop_pc == 0)
 	real_stop_pc = gdbarch_skip_trampoline_code (gdbarch, frame, stop_pc);
       if (real_stop_pc != 0)
@@ -7422,6 +7465,8 @@ process_event_stop_test (struct execution_control_state *ecs)
       /* If we have line number information for the function we are
 	 thinking of stepping into and the function isn't on the skip
 	 list, step into it.
+	 If we are about to step into a function marked trampoline with no
+	 line number information, we still want to enter it here.
 
 	 If there are several symtabs at that PC (e.g. with include
 	 files), just want to know whether *any* of them have line
@@ -7430,7 +7475,9 @@ process_event_stop_test (struct execution_control_state *ecs)
 	struct symtab_and_line tmp_sal;
 
 	tmp_sal = find_pc_line (ecs->stop_func_start, 0);
-	if (tmp_sal.line != 0
+	if ((tmp_sal.line != 0
+	     || (skip_trampoline_functions
+		 && in_trampoline_function (ecs->stop_func_start)))
 	    && !function_name_is_marked_for_skip (ecs->stop_func_name,
 						  tmp_sal)
 	    && !inline_frame_is_marked_for_skip (true, ecs->event_thread))
@@ -7583,6 +7630,19 @@ process_event_stop_test (struct execution_control_state *ecs)
       return;
     }
 
+  /* If we ended up in a function trampoline without stepping into a new
+     function we are either in some inlined trampoline or returning through a
+     trampoline function.  In either case we continue to single step until we
+     are out of the trampoline code again.  This check has to be done before
+     stop_pc_sal.line == 0 below, as trampolines usually don't have source
+     line information associated with them.  */
+  if (skip_trampoline_functions && in_trampoline_function (stop_pc_sal.pc))
+    {
+      infrun_debug_printf ("stepped into trampoline code");
+      keep_going (ecs);
+      return;
+    }
+
   if (stop_pc_sal.line == 0)
     {
       /* We have no line number information.  That means to stop
@@ -9794,6 +9854,14 @@ show_exec_direction_func (struct ui_file *out, int from_tty,
   }
 }
 
+static void
+show_skip_trampoline_functions (ui_file *file, int from_tty,
+				cmd_list_element *c,
+				const char *value)
+{
+  gdb_printf (file, _("Skipping trampoline functions is %s.\n"), value);
+}
+
 static void
 show_schedule_multiple (struct ui_file *file, int from_tty,
 			struct cmd_list_element *c, const char *value)
@@ -10139,6 +10207,16 @@ Options are 'forward' or 'reverse'."),
 			set_exec_direction_func, show_exec_direction_func,
 			&setlist, &showlist);
 
+  add_setshow_boolean_cmd ("skip-trampoline-functions", class_run,
+			  &skip_trampoline_functions, _("\
+Set whether gdb attempts to hide trampolines marked in the debug info."), _("\
+Show whether gdb attempts to hide trampolines marked in the debug info."), _("\
+If on, while stepping gdb will skip through functions and inlined functions\n\
+marked as trampolines by the compiler.  If off, gdb will ignore such function\n\
+trampolines."),
+			  nullptr, show_skip_trampoline_functions, &setlist,
+			  &showlist);
+
   /* Set/show detach-on-fork: user-settable mode.  */
 
   add_setshow_boolean_cmd ("detach-on-fork", class_run, &detach_fork, _("\
diff --git a/gdb/infrun.h b/gdb/infrun.h
index a343d27f72d..d4a6b7892b5 100644
--- a/gdb/infrun.h
+++ b/gdb/infrun.h
@@ -76,6 +76,10 @@ infrun_debug_show_threads (const char *title, ThreadRange threads)
 }
 
 
+/* Maximum size of trampoline chain to process while resolving
+   trampolines.  */
+#define MAX_TRAMPOLINE_CHAIN_SIZE 10
+
 /* Nonzero if we want to give control to the user when we're notified
    of shared library events by the dynamic linker.  */
 extern int stop_on_solib_events;
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-function-trampolines.c b/gdb/testsuite/gdb.dwarf2/dw2-function-trampolines.c
new file mode 100644
index 00000000000..1f0eb2e0efa
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/dw2-function-trampolines.c
@@ -0,0 +1,80 @@
+/* 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/>.  */
+
+/* This test relies on inlined_trampoline being inlined into main and the other
+   functions not.  All functions except target will be marked via
+   DW_AT_trampoline in the debug info and we'll check whether one can step
+   through the trampolines towards target.  */
+volatile int global_var;
+
+int __attribute__ ((noinline))
+target ()					/* target decl line */
+{						/* target prologue */
+  asm ("target_label: .globl target_label");
+  ++global_var;					/* target add */
+  asm ("target_label2: .globl target_label2");
+  return 9 + 10;				/* target return */
+}						/* target end */
+
+int __attribute__ ((noinline))
+trampoline ()
+{						/* trampoline prologue */
+  asm ("trampoline_label: .globl trampoline_label");
+  ++global_var;
+  return target ();				/* trampoline target call */
+}						/* trampoline end */
+
+static inline int __attribute__ ((always_inline))
+inlined_trampoline ()
+{						/* inlined_trampoline prologue */
+  asm ("inlined_trampoline_label: .globl inlined_trampoline_label");
+  ++global_var;					/* inlined_trampoline add */
+  asm ("inlined_trampoline_label2: .globl inlined_trampoline_label2");
+  return target ();				/* inlined_trampoline target call */
+}						/* inlined_trampoline end */
+
+int __attribute__ ((noinline))
+chained_trampoline ()
+{						/* chained_trampoline prologue */
+  asm ("chained_trampoline_label: .globl chained_trampoline_label");
+  ++global_var;
+  return trampoline ();				/* chained_trampoline trampoline call */
+}						/* chained_trampoline end */
+
+int __attribute__ ((noinline))
+doubly_chained_trampoline ()
+{						/* doubly_chained_trampoline prologue */
+  asm ("doubly_chained_trampoline_label: .globl doubly_chained_trampoline_label");
+  ++global_var;
+  return chained_trampoline ();			/* doubly_chained_trampoline chained_trampoline call */
+}						/* doubly_chained_trampoline end */
+
+int
+main ()						/* main decl line */
+{						/* main prologue */
+  int ans;
+  asm ("main_label: .globl main_label");
+  global_var = 0;				/* main set global_var */
+  asm ("main_label2: .globl main_label2");
+  ans = inlined_trampoline ();			/* main call inlined_trampoline */
+  asm ("main_label3: .globl main_label3");
+  ans = trampoline ();				/* main call trampoline */
+  asm ("main_label4: .globl main_label4");
+  ans = chained_trampoline ();			/* main call chained_trampoline */
+  asm ("main_label5: .globl main_label5");
+  ans = doubly_chained_trampoline ();		/* main call doubly_chained_trampoline */
+  asm ("main_label6: .globl main_label6");
+  return ans;					/* main call return */
+}						/* main end */
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-function-trampolines.exp b/gdb/testsuite/gdb.dwarf2/dw2-function-trampolines.exp
new file mode 100644
index 00000000000..67eec2b5540
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/dw2-function-trampolines.exp
@@ -0,0 +1,245 @@
+# 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/>.
+
+# This test checks GDB's handling of functions/inlined functions marked
+# DW_AT_trampoline by the compiler.  A function marked as trampoline should
+# generally be hidden from the user.  We check whether we can step through
+# trampolines.  Every trampoline is defined using a different type for its
+# target: a string, an address, a DIE reference or a flag.
+# Setting skip-trampoline-functions to false inside GDB should make it return
+# to its 'normal' behavior, ignore the DW_AT_trampoline, and skip all of the
+# non-inlined trampoline calls (as their DIEs don't have any source
+# information).
+
+load_lib dwarf.exp
+
+# This test can only be run on targets which support DWARF-2 and use gas.
+if {![dwarf2_support]} {
+    return 0
+}
+
+standard_testfile .c .S
+
+set asm_file [standard_output_file $srcfile2]
+Dwarf::assemble $asm_file {
+    global srcdir subdir srcfile
+    declare_labels lines_label trampoline_label
+
+    get_func_info target
+    get_func_info trampoline
+    get_func_info chained_trampoline
+    get_func_info doubly_chained_trampoline
+    get_func_info main
+
+    set target_decl_line [gdb_get_line_number "target decl line"]
+    set main_decl_line [gdb_get_line_number "main decl line"]
+    set main_call_inlined_trampoline_line [gdb_get_line_number "main call inlined_trampoline"]
+
+    cu {} {
+	compile_unit {
+	    {language @DW_LANG_C}
+	    {name dw2-function-trampolines.c}
+	    {low_pc 0 addr}
+	    {stmt_list ${lines_label} DW_FORM_sec_offset}
+	} {
+	    subprogram {
+		{name target}
+		{low_pc $target_start addr}
+		{high_pc "$target_start + $target_len" addr}
+		{decl_file 1 data1}
+		{decl_line $target_decl_line data1}
+	    }
+	    # The 'trampoline' subprogram declares its target by name.
+	    trampoline_label: subprogram {
+		    {name trampoline}
+		    {low_pc $trampoline_start addr}
+		    {high_pc "$trampoline_start + $trampoline_len" addr}
+		    {trampoline target string}
+		}
+	    # The 'chained_trampoline' subprogram declares its target as die
+	    # reference.
+	    subprogram {
+		{name chained_trampoline}
+		{low_pc $chained_trampoline_start addr}
+		{high_pc "$chained_trampoline_start + $chained_trampoline_len" addr}
+		{trampoline %$trampoline_label}
+	    }
+	    # The 'doubly_chained_trampoline' subprogram declares no target.
+	    # Its DW_AT_trampoline is a flag set to true.
+	    subprogram {
+		{name doubly_chained_trampoline}
+		{low_pc $doubly_chained_trampoline_start addr}
+		{high_pc "$doubly_chained_trampoline_start + $doubly_chained_trampoline_len" addr}
+		{trampoline 1 flag}
+	    }
+	    subprogram {
+		{external 1 flag}
+		{name main}
+		{main_subprogram 1 flag}
+		{low_pc $main_start addr}
+		{high_pc "$main_start + $main_len" addr}
+		{decl_file 1 data1}
+		{decl_line $main_decl_line data1}
+	    } {
+		# The 'inlined_trampoline' subroutine declares its target as
+		# an address.
+		inlined_subroutine {
+		    {name inlined_trampoline}
+		    {low_pc main_label2 addr}
+		    {high_pc main_label3 addr}
+		    {trampoline $target_start addr}
+		    {call_file 1 data1}
+		    {call_line $main_call_inlined_trampoline_line data1}
+		}
+	    }
+	}
+    }
+
+    lines {version 2} lines_label {
+	include_dir "${srcdir}/${subdir}"
+	file_name "$srcfile" 1
+
+	program {
+	    DW_LNE_set_address $main_start
+	    line [gdb_get_line_number "main set global_var"]
+	    DW_LNS_copy
+	    DW_LNE_set_address main_label
+	    line [gdb_get_line_number "main set global_var"]
+	    DW_LNS_copy
+	    DW_LNE_set_address main_label2
+	    line [gdb_get_line_number "main call inlined_trampoline"]
+	    DW_LNS_copy
+	    DW_LNE_set_address inlined_trampoline_label
+	    line [gdb_get_line_number "inlined_trampoline add"]
+	    DW_LNS_copy
+	    DW_LNE_set_address inlined_trampoline_label2
+	    line [gdb_get_line_number "inlined_trampoline target call"]
+	    DW_LNS_copy
+	    DW_LNE_set_address main_label3
+	    line [gdb_get_line_number "main call trampoline"]
+	    DW_LNS_copy
+	    DW_LNE_set_address main_label4
+	    line [gdb_get_line_number "main call chained_trampoline"]
+	    DW_LNS_copy
+	    DW_LNE_set_address main_label5
+	    line [gdb_get_line_number "main call doubly_chained_trampoline"]
+	    DW_LNS_copy
+	    DW_LNE_set_address main_label6
+	    line [gdb_get_line_number "main call return"]
+	    DW_LNS_copy
+	    DW_LNE_set_address $main_end
+	    DW_LNE_end_sequence
+
+	    DW_LNE_set_address $target_start
+	    line [gdb_get_line_number "target prologue"]
+	    DW_LNS_negate_stmt
+	    DW_LNS_copy
+	    DW_LNE_set_address target_label
+	    line [gdb_get_line_number "target add"]
+	    DW_LNS_negate_stmt
+	    DW_LNS_copy
+	    DW_LNE_set_address target_label2
+	    line [gdb_get_line_number "target return"]
+	    DW_LNS_copy
+	    DW_LNE_set_address $target_end
+	    DW_LNE_end_sequence
+	}
+    }
+}
+
+if {[prepare_for_testing "failed to prepare" ${testfile} \
+	[list $srcfile $asm_file] {nodebug additional_flags=-O0}]} {
+    return -1
+}
+
+set target_first_line_pattern ".*target add.*"
+set target_second_line_pattern ".*target return.*"
+
+if ![runto_main] {
+    return -1
+}
+
+gdb_test "show skip-trampoline-functions" \
+    "Skipping trampoline functions is on\." \
+    "check skip-trampoline-functions is enabled"
+
+with_test_prefix "with trampoline handling" {
+    foreach {trampoline return_line} { "inlined_trampoline" "trampoline" \
+	"trampoline" "chained_trampoline" \
+	"chained_trampoline" "doubly_chained_trampoline" } {
+
+	gdb_test "s" "$target_first_line_pattern" "step through $trampoline"
+	gdb_test "s" "$target_second_line_pattern" \
+	    "step target second line from $trampoline"
+	gdb_test "s" ".*main call $return_line.*" \
+	    "step back through $trampoline"
+    }
+
+    # The doubly_chained_trampoline has only been marked as trampoline but no
+    # target was given.  In this case GDB steps into the trampoline and then
+    # continues until the trampoline section is left again.
+
+    # When compiled with gcc 7.5 (and possibly others) on a 32 bit system, the
+    # trampoline function contains a call to __x86.get_pc_thunk.ax before the
+    # actual target call.  So, we end up in __x86.get_pc_thunk.ax.  Issuing a
+    # second step command will return from the function call back into the
+    # trampoline and go on inside the trampoline towards the actual target call.
+    # On other targets we step directly towards the target call.
+    gdb_test_multiple "s" "step through double_chained_trampoline" {
+	-re -wrap "$target_first_line_pattern" {
+	    pass $gdb_test_name
+	}
+	-re -wrap ".*__x86.get_pc_thunk.ax.*" {
+	    gdb_test "s" "$target_first_line_pattern" \
+		"step through double_chained_trampoline 2nd try"
+	}
+    }
+    gdb_test "s" "$target_second_line_pattern" \
+	"step target second line fromdoubly_chained_trampoline"
+    gdb_test "s" ".*main call return.*" \
+	"step back through doubly_chained_trampoline"
+}
+
+clean_restart ${testfile}
+
+if ![runto_main] {
+    return -1
+}
+
+gdb_test_no_output "set skip-trampoline-functions off" \
+    "disable trampoline handling"
+gdb_test "show skip-trampoline-functions" \
+    "Skipping trampoline functions is off." \
+    "check skip-trampoline-functions is disabled"
+
+with_test_prefix "without trampoline handling" {
+    gdb_test "s" ".*main call inlined_trampoline.*"
+    gdb_test "s" ".*inlined_trampoline add.*" \
+	"step into inlined_trampoline with skip-trampoline off"
+    gdb_test "s" ".*inlined_trampoline target call.*" \
+	"step in inlined_trampoline with skip-trampoline off"
+    gdb_test "s" "$target_first_line_pattern" \
+	"step into target with skip-trampoline off"
+    gdb_test "s" "$target_second_line_pattern" \
+	"step second line in target with skip-trampoline off"
+    gdb_test "s" ".*main call trampoline.*" \
+	"step brack from target with skip-trampoline off"
+    gdb_test "s" ".*main call chained_trampoline.*" \
+	"skip trampoline call with no line info"
+    gdb_test "s" ".*main call doubly_chained_trampoline.*" \
+	"skip chained_trampoline call with no line info"
+    gdb_test "s" ".*main call return.*" \
+	"skip doubly_chained_trampoline call with no line info"
+}
-- 
2.34.1

Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva  
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928

[PATCH v4 4/4] gdb: Skip trampoline frames in the stack for printing or finish command.

[Abdul Basit Ijaz]

Before the change, GDB prints the frames indicated by the compiler with DIE
"DW_AT_trampoline" in the backtrace and finish command, but for better user
experience, all such frames can be hidden from the user.  So, after this
change, now such frames are not printed any more in the backtrace command
and also the 'finish' command skips the trampoline calls.

So far, this DIE is added to DWARF only by the IFX compiler, so
gdb.fortran/mixed-lang-stack test used to fail for this compiler because of
these extra trampoline frames in the backtrace.  After the commit, those
trampoline frames are filtered so test is updated accordingly to handle the
frame level of the filtered frames.

Backtrace output in this test before the change:

bt -frame-arguments all
'''
'''
(gdb) FAIL: gdb.fortran/mixed-lang-stack.exp: lang=auto: bt -frame-arguments all

Now, after the change:
bt -frame-arguments all
'''
'''
(gdb) PASS: gdb.fortran/mixed-lang-stack.exp: lang=auto: bt -frame-arguments all

Below is the example where IFX emits DW_at_trampoline for functions first
and second trampoline calls as following:

F90 reproducer info:
function second (x, y) result(z)
  integer, intent(in) :: x, y
  integer :: z
  z = x * y ! breakpt-backtrace
end function second

function first (num1, num2) result(total)
  integer, intent(in) :: num1, num2
  integer  :: total
  total = second (num1 + 4, num2 * 3) ! first-breakpt
  total = total + 30
end function first

Related Dwarf:

0x0000013f:   DW_TAG_subprogram
                DW_AT_low_pc    (0x0000000000404350)
                DW_AT_high_pc   (0x000000000040435f)
                DW_AT_frame_base        (DW_OP_reg6 RBP)
                DW_AT_linkage_name      ("second_.t74p.t75p")
                DW_AT_name      ("second_.t74p.t75p")
                DW_AT_trampoline        ("second_")

0x0000015a:   DW_TAG_subprogram
                DW_AT_low_pc    (0x00000000004044a0)
                DW_AT_high_pc   (0x00000000004044af)
                DW_AT_frame_base        (DW_OP_reg6 RBP)
                DW_AT_linkage_name      ("first_.t104p.t105p")
                DW_AT_name      ("first_.t104p.t105p")
                DW_AT_trampoline        ("first_")

Before this change at line with comment 'breakpt-backtrace':
(gdb) backtrace 3
(gdb) finish
Run till exit from #0  second (x=20, y=9) at test.f90:4
second_.t74p.t75p () at test.f90:10
10        total = second (num1 + 4, num2 * 3) ! first-breakpt
Value returned is $1 = 180

After the change:
(gdb) backtrace 3
(gdb) finish
Run till exit from #0  second (x=20, y=9) at test.f90:4
0x0000000000404333 in first (num1=16, num2=3) at test.f90:10
10        total = second (num1 + 4, num2 * 3) ! first-breakpt
Value returned is $1 = 180

New test func-trampoline is also added to test this in both backtrace and
finish commands for Intel(R) Fortran Compiler.

2023-08-01 Abdul Basit Ijaz <abdul.b.ijaz@intel.com>
---
 gdb/doc/gdb.texinfo                           | 35 ++++++----
 gdb/infcmd.c                                  | 13 ++++
 gdb/infrun.c                                  |  8 +--
 gdb/infrun.h                                  |  6 ++
 gdb/stack.c                                   |  8 +++
 gdb/symtab.c                                  | 12 ++++
 gdb/symtab.h                                  |  5 ++
 gdb/testsuite/gdb.fortran/func-trampoline.exp | 69 +++++++++++++++++++
 gdb/testsuite/gdb.fortran/func-trampoline.f90 | 39 +++++++++++
 .../gdb.fortran/mixed-lang-stack.exp          | 10 ++-
 10 files changed, 186 insertions(+), 19 deletions(-)
 create mode 100644 gdb/testsuite/gdb.fortran/func-trampoline.exp
 create mode 100644 gdb/testsuite/gdb.fortran/func-trampoline.f90

diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 59a93f72895..e39db2e3156 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -6401,23 +6401,34 @@ so-called @dfn{trampoline functions}, which wrap the actual function call
 (the target of the trampoline).  The compiler might mark such a trampoline in
 its debug information.  Often, such trampolines do not have any source line
 information associated with them which will lead the @code{step} command to
-behave like a @code{next} and skip the function call completely.
+behave like a @code{next} and skip the function call completely.  Similarly,
+the @code{finish} command will return to some trampoline frame entry instead
+of returning to the call site.
 
 The default is @code{set skip-trampoline-functions on} and it will cause the
-@code{step} command to treat these trampolines differently.  When issuing a
-@code{step} at the call site of a trampoline function if
-@code{skip-trampoline-functions} is set @value{GDBN} will attempt to determine
+@code{step} and the @code{finish} command to treat these trampolines
+differently.
+
+When issuing a @code{step} at the call site of a trampoline function, if
+@code{skip-trampoline-functions} is set, @value{GDBN} will attempt to determine
 the target of the trampoline and then step through the trampoline stopping at
 the target.  If the target could not be found or was not given in the debug
 info, @value{GDBN} will simply continue execution until it leaves the
-trampoline code again, even if the trampoline has no line info associated with
-it.  When returning from a target function call and stepping back into the
-trampoline, @value{GDBN} will again step through the trampoline towards the
-call site.  Additionally, even if stopped in a trampoline function with source
-line information, issuing a @code{step} will prompt @value{GDBN} to resume
-execution until leaving the trampoline region again.  The @code{stepi} command
-is not affected by the setting which is enabled by default.  Currently, only
-DWARF trampolines marked via DW_AT_trampoline are supported by this.
+trampoline code again, even if the trampoline has no line info associated
+with it.  When returning from a target function call and stepping back into
+the trampoline, @value{GDBN} will again step through the trampoline towards
+the call site.  Additionally, even if stopped in a trampoline function with
+source line information, issuing a @code{step} will prompt @value{GDBN} to
+resume execution until leaving the trampoline region again.  The @code{stepi}
+command is not affected by the setting which is enabled by default.
+
+When issuing a @code{finish} command at the target of a trampoline function, if
+@code{skip-trampoline-functions} is set, @value{GDBN} will ignore all the
+trampoline frames and will reach the first non-trampoline frame to return to
+the call site of the current function.
+
+Currently, only DWARF trampolines marked via DW_AT_trampoline are supported by
+this.
 
 @item set skip-trampoline-functions off
 Causes the @code{step} command to completely ignore any trampoline information
diff --git a/gdb/infcmd.c b/gdb/infcmd.c
index 15702f84894..e64ac5dfe43 100644
--- a/gdb/infcmd.c
+++ b/gdb/infcmd.c
@@ -1853,6 +1853,19 @@ finish_command (const char *arg, int from_tty)
   if (frame == 0)
     error (_("\"finish\" not meaningful in the outermost frame."));
 
+  if (skip_trampoline_functions)
+    {
+      for (int i = 0; i < MAX_TRAMPOLINE_CHAIN_SIZE
+		      && (frame != nullptr)
+		      && in_trampoline_frame (frame); ++i)
+	frame = get_prev_frame (frame);
+
+      if (frame == nullptr)
+	error (_("\"finish\" not meaningful in the outermost non-trampoline \
+frame.  Consider running \"set skip-trampoline-functions off\", to stop in \
+trampoline frames for the \"finish\" command."));
+    }
+
   clear_proceed_status (0);
 
   tp = inferior_thread ();
diff --git a/gdb/infrun.c b/gdb/infrun.c
index 0ef9c76d3c6..42e96556e66 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -188,7 +188,7 @@ static bool detach_fork = true;
    DW_AT_trampoline by the compiler.  If false, GDB will ignore the
    attribute.  */
 
-static bool skip_trampoline_functions = true;
+bool skip_trampoline_functions = true;
 
 bool debug_infrun = false;
 static void
@@ -10211,9 +10211,9 @@ Options are 'forward' or 'reverse'."),
 			  &skip_trampoline_functions, _("\
 Set whether gdb attempts to hide trampolines marked in the debug info."), _("\
 Show whether gdb attempts to hide trampolines marked in the debug info."), _("\
-If on, while stepping gdb will skip through functions and inlined functions\n\
-marked as trampolines by the compiler.  If off, gdb will ignore such function\n\
-trampolines."),
+If on, the step and finish commands will skip through functions and inlined\n\
+functions marked as trampolines by the compiler.  If off, gdb will ignore\n\
+such function trampolines."),
 			  nullptr, show_skip_trampoline_functions, &setlist,
 			  &showlist);
 
diff --git a/gdb/infrun.h b/gdb/infrun.h
index d4a6b7892b5..feb5f4414ec 100644
--- a/gdb/infrun.h
+++ b/gdb/infrun.h
@@ -106,6 +106,12 @@ extern bool non_stop;
    starting an inferior.  */
 extern bool disable_randomization;
 
+/* If set (default) GDB will step through functions/inlined subroutines marked
+   DW_AT_trampoline by the compiler.  If false, GDB will ignore the
+   attribute.  */
+
+extern bool skip_trampoline_functions;
+
 /* Returns a unique identifier for the current stop.  This can be used
    to tell whether a command has proceeded the inferior past the
    current location.  */
diff --git a/gdb/stack.c b/gdb/stack.c
index 002bf580634..915ac398e09 100644
--- a/gdb/stack.c
+++ b/gdb/stack.c
@@ -2054,6 +2054,14 @@ backtrace_command_1 (const frame_print_options &fp_opts,
 	{
 	  QUIT;
 
+	  if (in_trampoline_frame (fi))
+	    {
+	      /* Trampoline frames are not printed so they are not counted in
+		 the backtrace limit.  */
+	      count++;
+	      continue;
+	    }
+
 	  /* Don't use print_stack_frame; if an error() occurs it probably
 	     means further attempts to backtrace would fail (on the other
 	     hand, perhaps the code does or could be fixed to make sure
diff --git a/gdb/symtab.c b/gdb/symtab.c
index 745fd7d5d25..f396aca78c3 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -4066,6 +4066,18 @@ in_trampoline_function (CORE_ADDR pc)
 
 /* See symtab.h.  */
 
+bool
+in_trampoline_frame (frame_info_ptr fi)
+{
+  CORE_ADDR pc;
+  if (get_frame_pc_if_available (fi, &pc))
+    return in_trampoline_function (pc);
+
+  return false;
+}
+
+/* See symtab.h.  */
+
 CORE_ADDR
 find_function_trampoline_target (CORE_ADDR pc)
 {
diff --git a/gdb/symtab.h b/gdb/symtab.h
index fc2d82e6567..f02b3840f00 100644
--- a/gdb/symtab.h
+++ b/gdb/symtab.h
@@ -2317,6 +2317,11 @@ extern CORE_ADDR find_solib_trampoline_target (frame_info_ptr, CORE_ADDR);
 
 extern bool in_trampoline_function (CORE_ADDR pc);
 
+/* Return whether or not the pc of current frame is within a block that belongs
+   to a function that is marked as a trampoline by the compiler.  */
+
+extern bool in_trampoline_frame (frame_info_ptr);
+
 /* Find the target of a trampoline function marked via the DW_AT_trampoline
    attribute and return its address.  Returns 0 if the pc is not contained
    in a trampoline function (inlined or not).  If DW_AT_trampoline
diff --git a/gdb/testsuite/gdb.fortran/func-trampoline.exp b/gdb/testsuite/gdb.fortran/func-trampoline.exp
new file mode 100644
index 00000000000..3bc4d1c0714
--- /dev/null
+++ b/gdb/testsuite/gdb.fortran/func-trampoline.exp
@@ -0,0 +1,69 @@
+# 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/> .
+
+# Test finish and backtrace commands for functions with trampoline
+# calls.  Also checks if trampoline frames are filtered while printing
+# stack and finish command returns to real function.
+
+require allow_fortran_tests
+
+if {![test_compiler_info {ifx-*} f90]} {
+    untested "Test is only applicable for IFX where\
+	     compiler emits trampoline DIE in Dwarf"
+    return -1
+}
+
+standard_testfile ".f90"
+load_lib fortran.exp
+
+if {[prepare_for_testing "failed to prepare" ${testfile} ${srcfile} \
+    {debug f90}]} {
+    return -1
+}
+
+if {![fortran_runto_main]} {
+    return -1
+}
+
+set inner_loc [gdb_get_line_number "second-breakpt"]
+set middle_loc [gdb_get_line_number "first-breakpt"]
+set outer_loc [gdb_get_line_number "main-outer-loc"]
+set fill "\[^\r\n\]*"
+
+set inner_desc  "second \\(x=20, y=9\\) at ${fill}$srcfile:$inner_loc"
+set middle_desc "first \\(num1=16, num2=3\\) at ${fill}$srcfile:$middle_loc"
+set outer_desc  ".* at .*$srcfile:$outer_loc"
+
+# Set breakpoint inside the innermost function 'second'.
+gdb_breakpoint "$srcfile:$inner_loc"
+gdb_continue_to_breakpoint "innermost-body" ".*$srcfile:$inner_loc.*"
+
+# Limit the backtrace to 3 frames and ensure both frames for first
+# and second function are shown only and trampoline frames are filtered.
+gdb_test "backtrace 3" [multi_line \
+    "#$decimal.* $inner_desc" \
+    "#$decimal.* $middle_desc" \
+    "#$decimal.* $outer_desc.*"] \
+"backtrace, test trampoline frames are filtered"
+
+# Finish the function calls.  Normally we expect to see a "Value
+# returned is ..." line.
+set value_returned "(\r\nValue returned is $valnum_re = 180)"
+
+gdb_test "finish" [multi_line \
+    "Run till exit from #0 $fill second \\(x=20, y=9\\) $fill" \
+    "${fill}first \\(num1=16, num2=3\\)${fill}" \
+    "${fill}${value_returned}"] \
+    "finish the innermost frame"
diff --git a/gdb/testsuite/gdb.fortran/func-trampoline.f90 b/gdb/testsuite/gdb.fortran/func-trampoline.f90
new file mode 100644
index 00000000000..652733b44fd
--- /dev/null
+++ b/gdb/testsuite/gdb.fortran/func-trampoline.f90
@@ -0,0 +1,39 @@
+! 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/> .
+
+! Source code for func-trampoline.exp.
+
+integer(kind=4) function second(x, y)
+  integer(kind=4), intent(in) :: x
+  integer(kind=4), intent(in) :: y
+
+  second = x * y ! second-breakpt
+end function
+
+integer(kind=4) function first(num1, num2)
+  integer(kind=4), intent(in) :: num1
+  integer(kind=4), intent(in) :: num2
+
+  first = second (num1 + 4, num2 * 3) ! first-breakpt
+end function
+
+program func_trampoline
+  integer(kind=4) :: total
+
+  total = first(16, 3) ! main-outer-loc
+
+  write(*,*)  "Result is ", total, "\n"
+  ! Expected: 180
+end program func_trampoline
diff --git a/gdb/testsuite/gdb.fortran/mixed-lang-stack.exp b/gdb/testsuite/gdb.fortran/mixed-lang-stack.exp
index 5fc36f7b95b..71a171edcf0 100644
--- a/gdb/testsuite/gdb.fortran/mixed-lang-stack.exp
+++ b/gdb/testsuite/gdb.fortran/mixed-lang-stack.exp
@@ -41,7 +41,7 @@ set have_index [exec_has_index_section $binfile]
 # value to pass to GDB's 'set language ...' command.
 proc run_tests { lang } {
     with_test_prefix "lang=${lang}" {
-	global binfile hex have_index
+	global binfile hex have_index decimal
 
 	clean_restart ${binfile}
 
@@ -63,6 +63,10 @@ proc run_tests { lang } {
 	set e_arg "\['\"\]abcdef\['\"\]"
 	set 1b_args "\[^\r\n\]+$e_arg\[^\r\n\]+"
 	set 1g_args "obj=\[^\r\n\]+"
+	# Generic decimal number is checked in regex for Frame #8 and #9 to
+	# handle filtered trampoline frames.  Since Frame#8 and Frame#10 are
+	# set to trampoline in DWARF by Intel® Fortran Compiler (ifx), they
+	# are not shown by the backtrace command.
 	set bt_stack \
 	    [multi_line \
 		 "#0\\s+breakpt \\(\\) at \[^\r\n\]+" \
@@ -73,8 +77,8 @@ proc run_tests { lang } {
 		 "#5\\s+$hex in mixed_func_1d \\(\[^\r\n\]+\\) at \[^\r\n\]+" \
 		 "#6\\s+$hex in mixed_func_1c \\(\[^\r\n\]+\\) at \[^\r\n\]+" \
 		 "#7\\s+$hex in mixed_func_1b \\($1b_args\\) at \[^\r\n\]+" \
-		 "#8\\s+$hex in mixed_func_1a \\(\\) at \[^\r\n\]+" \
-		 "#9\\s+$hex in mixed_stack_main \\(\\) at \[^\r\n\]+" ]
+		 "#$decimal\\s+$hex in mixed_func_1a \\(\\) at \[^\r\n\]+" \
+		 "#$decimal\\s+$hex in mixed_stack_main \\(\\) at \[^\r\n\]+" ]
 	set main_args "argc=1, argv=${hex}( \[^\r\n\]+)?"
 	set bt_stack_kfail \
 	    [multi_line \
-- 
2.34.1

Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva  
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928

Re: [PATCH v4 3/4] gdb/infrun: handle stepping through functions with DW_AT_trampoline

[Eli Zaretskii]

> From: Abdul Basit Ijaz <abdul.b.ijaz@intel.com>
> Cc: abdul.b.ijaz@intel.com,
> 	JiniSusan.George@amd.com,
> 	tom@tromey.com,
> 	eliz@gnu.org,
> 	blarsen@redhat.com,
> 	Nils-Christian Kempke <nils-christian.kempke@intel.com>
> Date: Wed,  2 Aug 2023 00:47:43 +0200
> 
> From: Nils-Christian Kempke <nils-christian.kempke@intel.com>
> 
> This patch makes infrun continue stepping into and through trampoline
> functions marked via the DW_AT_trampoline in DWARF.  The attribute can
> be emitted by the compiler for certain subroutines/inlined subroutines
> that are compiler generated and should be hidden from a user.

Thanks.  I believe I already approved the documentation parts, and
there was no changes since then in the documentation parts, right?

Re: [PATCH v4 4/4] gdb: Skip trampoline frames in the stack for printing or finish command.

[Eli Zaretskii]

> From: Abdul Basit Ijaz <abdul.b.ijaz@intel.com>
> Cc: abdul.b.ijaz@intel.com,
> 	JiniSusan.George@amd.com,
> 	tom@tromey.com,
> 	eliz@gnu.org,
> 	blarsen@redhat.com
> Date: Wed,  2 Aug 2023 00:47:44 +0200
> 
> Before the change, GDB prints the frames indicated by the compiler with DIE
> "DW_AT_trampoline" in the backtrace and finish command, but for better user
> experience, all such frames can be hidden from the user.  So, after this
> change, now such frames are not printed any more in the backtrace command
> and also the 'finish' command skips the trampoline calls.

Thanks, I believe I already approved the documentation parts of this,
right?

RE: [PATCH v4 4/4] gdb: Skip trampoline frames in the stack for printing or finish command.

[Ijaz, Abdul B]

Hi Eli,
Yes nothing change in documentation since last patch you approved. Thanks

Best Regards
Abdul Basit

-----Original Message-----
From: Eli Zaretskii <eliz@gnu.org> 
Sent: Wednesday, August 2, 2023 1:53 PM
To: Ijaz, Abdul B <abdul.b.ijaz@intel.com>
Cc: gdb-patches@sourceware.org; Ijaz, Abdul B <abdul.b.ijaz@intel.com>; JiniSusan.George@amd.com; tom@tromey.com; blarsen@redhat.com
Subject: Re: [PATCH v4 4/4] gdb: Skip trampoline frames in the stack for printing or finish command.

> From: Abdul Basit Ijaz <abdul.b.ijaz@intel.com>
> Cc: abdul.b.ijaz@intel.com,
> 	JiniSusan.George@amd.com,
> 	tom@tromey.com,
> 	eliz@gnu.org,
> 	blarsen@redhat.com
> Date: Wed,  2 Aug 2023 00:47:44 +0200
> 
> Before the change, GDB prints the frames indicated by the compiler 
> with DIE "DW_AT_trampoline" in the backtrace and finish command, but 
> for better user experience, all such frames can be hidden from the 
> user.  So, after this change, now such frames are not printed any more 
> in the backtrace command and also the 'finish' command skips the trampoline calls.

Thanks, I believe I already approved the documentation parts of this, right?
Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva  
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928

RE: [PATCH v4 3/4] gdb/infrun: handle stepping through functions with DW_AT_trampoline

[Ijaz, Abdul B]

Hi Eli,
Yes nothing change in documentation since last patch you approved. Thanks

Best Regards
Abdul Basit

-----Original Message-----
From: Eli Zaretskii <eliz@gnu.org> 
Sent: Wednesday, August 2, 2023 1:52 PM
To: Ijaz, Abdul B <abdul.b.ijaz@intel.com>
Cc: gdb-patches@sourceware.org; JiniSusan.George@amd.com; tom@tromey.com; blarsen@redhat.com; nils-christian.kempke@intel.com
Subject: Re: [PATCH v4 3/4] gdb/infrun: handle stepping through functions with DW_AT_trampoline

> From: Abdul Basit Ijaz <abdul.b.ijaz@intel.com>
> Cc: abdul.b.ijaz@intel.com,
> 	JiniSusan.George@amd.com,
> 	tom@tromey.com,
> 	eliz@gnu.org,
> 	blarsen@redhat.com,
> 	Nils-Christian Kempke <nils-christian.kempke@intel.com>
> Date: Wed,  2 Aug 2023 00:47:43 +0200
> 
> From: Nils-Christian Kempke <nils-christian.kempke@intel.com>
> 
> This patch makes infrun continue stepping into and through trampoline 
> functions marked via the DW_AT_trampoline in DWARF.  The attribute can 
> be emitted by the compiler for certain subroutines/inlined subroutines 
> that are compiler generated and should be hidden from a user.

Thanks.  I believe I already approved the documentation parts, and there was no changes since then in the documentation parts, right?
Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva  
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928

Re: [PATCH v4 1/4] gdb, dwarf: add support for DW_AT_trampoline in DWARF reader

[Tom Tromey]

>>>>> "Abdul" == Abdul Basit Ijaz via Gdb-patches <gdb-patches@sourceware.org> writes:

Abdul> This patch adds support to GDB for reading the DW_AT_trampoline
Abdul> attribute.  It stores the attribute and its value in the type_specific
Abdul> part of a GDB type.  This patch is implemented in preparation of the
Abdul> following patches, which will add a mechanism to hide DW_AT_trampoline
Abdul> subroutines from the user.

Thanks for the patch.

Abdul> +	  if (target_name == NULL)
Abdul> +	    target_name = dwarf2_physname (NULL, target_die, target_cu);

Probably your patch predates this but nowadays we're using nullptr
instead of NULL.

Abdul> +	  unrelocated_addr target_addr_reloc = attr->as_address ();
Abdul> +	  CORE_ADDR target_addr
Abdul> +	    = cu->per_objfile->relocate (target_addr_reloc);
Abdul> +	  target_addr = gdbarch_adjust_dwarf2_addr (objfile->arch (),
Abdul> +						    target_addr);

dwarf2_per_objfile::relocate already calls gdbarch_adjust_dwarf2_addr,
so that call isn't needed.

Abdul> +	gdb_printf ("%*sfunc_type_flags %u\n", spaces, "",
Abdul> +		    (unsigned int) TYPE_FUNC_FLAGS (type));

If we're just printing the flags as an int, how about using hex instead?

Abdul> -    unsigned int is_noreturn : 1;
Abdul> +    func_type_flags flags;

This should use ENUM_BITFIELD like the other spot in the patch.

Tom

Re: [PATCH v4 1/4] gdb, dwarf: add support for DW_AT_trampoline in DWARF reader

[Tom Tromey]

>>>>> "Abdul" == Abdul Basit Ijaz via Gdb-patches <gdb-patches@sourceware.org> writes:

Abdul> +      else if (attr->form_is_ref ())
Abdul> +	{
Abdul> +	  die_info *target_die;
Abdul> +	  dwarf2_cu *target_cu = cu;
Abdul> +
Abdul> +	  target_die = follow_die_ref (die, attr, &target_cu);
Abdul> +	  const char *target_name;
Abdul> +	  /* Prefer the mangled name; otherwise compute the demangled one.  */
Abdul> +	  target_name = dw2_linkage_name (target_die, target_cu);
Abdul> +	  if (target_name == NULL)
Abdul> +	    target_name = dwarf2_physname (NULL, target_die, target_cu);
Abdul> +	  if (target_name == nullptr)
Abdul> +	    {
Abdul> +	      complaint (_("DW_AT_trampoline target DIE has no name for"
Abdul> +			 "referencing DIE %s [in module %s]"),
Abdul> +			 sect_offset_str (die->sect_off),
Abdul> +			 objfile_name (objfile));
Abdul> +	    }
Abdul> +	  TYPE_TRAMPOLINE_TARGET (ftype)->set_target_name (target_name);

I didn't realize this until starting to read patch 2, but it seems
strange to use the name here.  It seems to me it would be better to use
the address attached to the referenced subroutine DIE.  The "bad"
scenario would be where there are multiple functions with the same name
here, and gdb's symbol lookup returns the wrong one.  You can normally
trigger these bugs by carefully ordering shared libraries.

Tom

Re: [PATCH v4 2/4] gdb/symtab: add lookup for trampoline functions

[Tom Tromey]

>>>>> "Abdul" == Abdul Basit Ijaz via Gdb-patches <gdb-patches@sourceware.org> writes:

Abdul> From: Nils-Christian Kempke <nils-christian.kempke@intel.com>
Abdul> In order to query information about the DW_AT_trampoline tag for
Abdul> subroutines and inlined subroutines, two function were added to symtab.

Thanks for the patch.

Abdul> +CORE_ADDR
Abdul> +find_function_trampoline_target (CORE_ADDR pc)
Abdul> +{
...
Abdul> +      else if (trampoline->target_kind () == TRAMPOLINE_TARGET_PHYSADDR)
Abdul> +	{
Abdul> +	  /* If the function symbol containing this trampoline target has
Abdul> +	     been relocated we assume the target_address also needs relocation.
Abdul> +	     If it has not been relocated the offset should be zero.  */
Abdul> +	  target_address
Abdul> +	    = (trampoline->target_physaddr ()
Abdul> +	       + sym->objfile ()->section_offsets[sym->section_index ()]);

First, I'm all in favor of storing unrelocated addresses in data
structures and then relocating them at point of use.

However, in this case, I think the stored address is already relocated.
From the previous patch:

+	  unrelocated_addr target_addr_reloc = attr->as_address ();
+	  CORE_ADDR target_addr
+	    = cu->per_objfile->relocate (target_addr_reloc);
+	  target_addr = gdbarch_adjust_dwarf2_addr (objfile->arch (),
+						    target_addr);
+	  TYPE_TRAMPOLINE_TARGET (ftype)->set_target_physaddr (target_addr);

... this brings up another issue, though -- if a relocated address is
stored, then it must be relocated by objfile_relocate.

So perhaps storing the unrelocated address here really would be better.
This can be done using 'adjust' rather than 'relocate' in the first
patch, and by using the unrelocated_addr type rather than CORE_ADDR for
storage.

Tom

Re: [PATCH v4 3/4] gdb/infrun: handle stepping through functions with DW_AT_trampoline

[Tom Tromey]

>>>>> "Abdul" == Abdul Basit Ijaz via Gdb-patches <gdb-patches@sourceware.org> writes:

Abdul> From: Nils-Christian Kempke <nils-christian.kempke@intel.com>
Abdul> This patch makes infrun continue stepping into and through trampoline
Abdul> functions marked via the DW_AT_trampoline in DWARF.  The attribute can
Abdul> be emitted by the compiler for certain subroutines/inlined subroutines
Abdul> that are compiler generated and should be hidden from a user.

Thanks for the patch.

infrun isn't really my area but I do have some nits.

Abdul> +      bool in_trampoline = skip_trampoline_functions
Abdul> +			   && in_trampoline_function (stop_pc);
Abdul> +
Abdul> +      if (in_trampoline)

The assignment is not formatted correctly, but it seems to me that
there's no need for this variable at all, and the expression can be
inlined into the 'if' -- which will also fix the formatting.

Abdul> +	  real_stop_pc = find_function_trampoline_target (stop_pc);
Abdul> +
Abdul> +	  for (int i = 0; i < MAX_TRAMPOLINE_CHAIN_SIZE
Abdul> +			  && in_trampoline_function (real_stop_pc); ++i)

Normally we'd split at the ';':

    for (int i = 0;
         i < MAX_TRAMPOLINE_CHAIN_SIZE
         && in_trampoline_function (real_stop_pc);
         ++i)

... with the middle expression being all on one line if it fits.

Abdul> +	    {
Abdul> +		real_stop_pc = find_function_trampoline_target (real_stop_pc);
Abdul> +		/* Exit if find_function_trampoline_target failed to find the
Abdul> +		   trampoline target.  Do not try to resolve the trampolines
Abdul> +		   in this case.  */
Abdul> +		if (real_stop_pc == 0x0)

Just '0', not '0x0'.

Abdul> +	  /* If we failed to find a target we will just single step in the
Abdul> +	     hope of leaving the trampoline again soon.  */
Abdul> +	  if (real_stop_pc == 0x0)

Ditto.

Abdul> +      if (real_stop_pc == 0)
Abdul> +	real_stop_pc = skip_language_trampoline (frame, stop_pc);
Abdul>        if (real_stop_pc == 0)
Abdul>  	real_stop_pc = gdbarch_skip_trampoline_code (gdbarch, frame, stop_pc);

This is pre-existing code, but I wonder how all these different pieces
interact, or are supposed to interact.

I guess the language stuff is there to work around a GCC deficiency, in
that it does not emit DW_AT_trampoline.

The gdbarch stuff... I don't know but I see a lot of arches implement
this (though mostly deferring to the solib code).

Anyway, is it possible to have a gdbarch or c++ trampoline then call a
DW_AT_trampoline function?  If so then wouldn't we end up with an
erroneous stop?

I'm just wondering if these ought to be unified in some way, like say
look for any sort of trampoline target in the loop.

Also LOL at the gnu-v3-abi.c code:

  if (thunk_name == NULL || strstr (thunk_name, " thunk to ") == NULL)
    return 0;

It also seems a bit weird that skip_trampoline_functions only affects
this one subset of trampoline functions.

Tom

Re: [PATCH v4 4/4] gdb: Skip trampoline frames in the stack for printing or finish command.

[Tom Tromey]

>>>>> "Abdul" == Abdul Basit Ijaz via Gdb-patches <gdb-patches@sourceware.org> writes:

Abdul> Before the change, GDB prints the frames indicated by the compiler with DIE
Abdul> "DW_AT_trampoline" in the backtrace and finish command, but for better user
Abdul> experience, all such frames can be hidden from the user.  So, after this
Abdul> change, now such frames are not printed any more in the backtrace command
Abdul> and also the 'finish' command skips the trampoline calls.

Abdul> So far, this DIE is added to DWARF only by the IFX compiler, so
Abdul> gdb.fortran/mixed-lang-stack test used to fail for this compiler because of
Abdul> these extra trampoline frames in the backtrace.  After the commit, those
Abdul> trampoline frames are filtered so test is updated accordingly to handle the
Abdul> frame level of the filtered frames.

Thanks for the patch.

I'm not totally sure how I feel about the stack trace part of this one.

On the one hand, normally I think it makes sense to elide these frames.
They aren't very interesting.

On the other hand, we have frame filters for frame elision, but then
this works in a totally different way.

If I am stopped in a frame and go "up", will I be in the trampoline
frame anyway?  And will "bt" then not print the selected frame?  But
"frame" will?

Abdul> +  if (skip_trampoline_functions)
Abdul> +    {
Abdul> +      for (int i = 0; i < MAX_TRAMPOLINE_CHAIN_SIZE
Abdul> +		      && (frame != nullptr)
Abdul> +		      && in_trampoline_frame (frame); ++i)
Abdul> +	frame = get_prev_frame (frame);

Formatting looks weird here.

Abdul> +++ b/gdb/stack.c
Abdul> @@ -2054,6 +2054,14 @@ backtrace_command_1 (const frame_print_options &fp_opts,
Abdul>  	{
Abdul>  	  QUIT;
 
Abdul> +	  if (in_trampoline_frame (fi))
Abdul> +	    {
Abdul> +	      /* Trampoline frames are not printed so they are not counted in
Abdul> +		 the backtrace limit.  */
Abdul> +	      count++;
Abdul> +	      continue;

I guess the frame numbers will skip as well, because the frame level is
a property of the frame, not of the loop displaying frames.

Also this approach ignores frame filters -- if one is in use, then I
think trampoline frames will show up again.  You can try this by writing
a dummy frame filter that just returns all frames as-is.

Tom

Re: [PATCH v4 4/4] gdb: Skip trampoline frames in the stack for printing or finish command.

[Tom Tromey]

>>>>> "Abdul" == Abdul Basit Ijaz via Gdb-patches <gdb-patches@sourceware.org> writes:

Abdul> @@ -1853,6 +1853,19 @@ finish_command (const char *arg, int from_tty)
Abdul>    if (frame == 0)
Abdul>      error (_("\"finish\" not meaningful in the outermost frame."));
 
Abdul> +  if (skip_trampoline_functions)
Abdul> +    {

I forgot to ask -- I don't really know much about this either, but does
this patch properly handle reverse finish?

thanks,
Tom

RE: [PATCH v4 4/4] gdb: Skip trampoline frames in the stack for printing or finish command.

[Ijaz, Abdul B]

Hi Tom,

Thanks for the reply.

Tom> I forgot to ask -- I don't really know much about this either, but does this patch properly handle reverse finish?
 
In V4 it did not but will add in V5 reverse-finish so here it will also be working fine. Thanks for pointing this out. 

Best Regards,
Abdul Basit

-----Original Message-----
From: Tom Tromey <tom@tromey.com> 
Sent: Wednesday, August 2, 2023 10:43 PM
To: Abdul Basit Ijaz via Gdb-patches <gdb-patches@sourceware.org>
Cc: Ijaz, Abdul B <abdul.b.ijaz@intel.com>; JiniSusan.George@amd.com; tom@tromey.com; eliz@gnu.org; blarsen@redhat.com
Subject: Re: [PATCH v4 4/4] gdb: Skip trampoline frames in the stack for printing or finish command.

>>>>> "Abdul" == Abdul Basit Ijaz via Gdb-patches <gdb-patches@sourceware.org> writes:

Abdul> @@ -1853,6 +1853,19 @@ finish_command (const char *arg, int from_tty)
Abdul>    if (frame == 0)
Abdul>      error (_("\"finish\" not meaningful in the outermost 
Abdul> frame."));
 
Abdul> +  if (skip_trampoline_functions)
Abdul> +    {

I forgot to ask -- I don't really know much about this either, but does this patch properly handle reverse finish?

thanks,
Tom
Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva  
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928

RE: [PATCH v4 2/4] gdb/symtab: add lookup for trampoline functions

[Ijaz, Abdul B]

Hi Tom,

Thanks for the feedback.

Tom> This can be done using 'adjust' rather than 'relocate' in the first patch, and by using the unrelocated_addr type rather than CORE_ADDR for storage.

Will be updated accordingly in V5 patch series.

Thanks & Best Regards
Abdul Basit

-----Original Message-----
From: Tom Tromey <tom@tromey.com> 
Sent: Wednesday, August 2, 2023 10:18 PM
To: Abdul Basit Ijaz via Gdb-patches <gdb-patches@sourceware.org>
Cc: Ijaz, Abdul B <abdul.b.ijaz@intel.com>; JiniSusan.George@amd.com; tom@tromey.com; eliz@gnu.org; blarsen@redhat.com; Nils-Christian Kempke <nils-christian.kempke@intel.com>
Subject: Re: [PATCH v4 2/4] gdb/symtab: add lookup for trampoline functions

>>>>> "Abdul" == Abdul Basit Ijaz via Gdb-patches <gdb-patches@sourceware.org> writes:

Abdul> From: Nils-Christian Kempke <nils-christian.kempke@intel.com> In 
Abdul> order to query information about the DW_AT_trampoline tag for 
Abdul> subroutines and inlined subroutines, two function were added to symtab.

Thanks for the patch.

Abdul> +CORE_ADDR
Abdul> +find_function_trampoline_target (CORE_ADDR pc) {
...
Abdul> +      else if (trampoline->target_kind () == TRAMPOLINE_TARGET_PHYSADDR)
Abdul> +	{
Abdul> +	  /* If the function symbol containing this trampoline target has
Abdul> +	     been relocated we assume the target_address also needs relocation.
Abdul> +	     If it has not been relocated the offset should be zero.  */
Abdul> +	  target_address
Abdul> +	    = (trampoline->target_physaddr ()
Abdul> +	       + sym->objfile ()->section_offsets[sym->section_index 
Abdul> +()]);

First, I'm all in favor of storing unrelocated addresses in data structures and then relocating them at point of use.

However, in this case, I think the stored address is already relocated.
From the previous patch:

+	  unrelocated_addr target_addr_reloc = attr->as_address ();
+	  CORE_ADDR target_addr
+	    = cu->per_objfile->relocate (target_addr_reloc);
+	  target_addr = gdbarch_adjust_dwarf2_addr (objfile->arch (),
+						    target_addr);
+	  TYPE_TRAMPOLINE_TARGET (ftype)->set_target_physaddr (target_addr);

... this brings up another issue, though -- if a relocated address is stored, then it must be relocated by objfile_relocate.

So perhaps storing the unrelocated address here really would be better.
This can be done using 'adjust' rather than 'relocate' in the first patch, and by using the unrelocated_addr type rather than CORE_ADDR for storage.

Tom
Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva  
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928

RE: [PATCH v4 3/4] gdb/infrun: handle stepping through functions with DW_AT_trampoline

[Ijaz, Abdul B]

Hi Tom,

Thanks for the feedback. Added replies below.

Abdul> +      if (in_trampoline)

Tom > The assignment is not formatted correctly, but it seems to me that there's no need for this variable at all, and the expression can be inlined into the 'if' -- which will also fix the formatting.
Will fix in V5.

Abdul> +	  for (int i = 0; i < MAX_TRAMPOLINE_CHAIN_SIZE
Abdul> +			  && in_trampoline_function (real_stop_pc); ++i)
Tom > Normally we'd split at the ';':
Will fix in V5.

Abdul> +		if (real_stop_pc == 0x0)
Tom>Just '0', not '0x0'.
Will fix both instances in V5.

Tom> I guess the language stuff is there to work around a GCC deficiency, in that it does not emit DW_AT_trampoline.
Not sure about deficiency but I think there are Signal trampolines which need to be handled separately for every arch while for thunks as far as I understand for its handling there could be  language stuff which might be needed for different languages.

Tom > "
The gdbarch stuff... I don't know but I see a lot of arches implement this (though mostly deferring to the solib code).

Anyway, is it possible to have a gdbarch or c++ trampoline then call a DW_AT_trampoline function?  If so then wouldn't we end up with an erroneous stop?

I'm just wondering if these ought to be unified in some way, like say look for any sort of trampoline target in the loop.
"

All these trampoline cases are really different way of detecting and the processing them and I am not sure compiler decision about DW_AT_trampoline. So far able to see them only for Intel Fortran compilers for all the function calls.  For unifying need better understanding of existing trampolines and I think I am lacking the understanding of the implementation of other trampolines to handle thunks or signal trampolines. Now regarding erroneous stop if I understand rightly in case of some targets where compiler emit thunks for trampoline calls and also it emit trampoline dwarf for same targets then I think the trampoline detected using thunks handling  should be done first or which ever I processed first should process the trampolines. In case of DW_AT_trampoline it is set for the respective frame and if it is not detected in case those frames were processed already then nothing should be affected, so should not be an issue. But if we want to make sure then have to try on some target not sure how so that I may verify to confirm if  it will work fine in such case.

Tom >"
Also LOL at the gnu-v3-abi.c code:

  if (thunk_name == NULL || strstr (thunk_name, " thunk to ") == NULL)
    return 0;

It also seems a bit weird that skip_trampoline_functions only affects this one subset of trampoline functions.
"

So skip_trampoline_functions is so far only handling the skipping case for trampolines indicated by DW_AT_trampoline and not for any other case of these thunks emitted by compiler or by signal trampolines. Since this case is pretty much defined case for subroutines calling the target subroutine but not sure about others but may be these solib calls filtering may also be handled with the same. Please let me know if we want to filter other trampolines cases using the same filter. Mostly it should be "on" case but DW_AT_trampoline  could be interesting for user if compiler emit it for virtual functions and user want to see all the trampoline calls so here there could be some cases for end user to see those trampoline calls but for thunks or rest I don’t know so let me know then I can asses of using the filter option for other trampolines.

Thanks  & Best Regards,
Abdul Basit

-----Original Message-----
From: Tom Tromey <tom@tromey.com> 
Sent: Wednesday, August 2, 2023 10:34 PM
To: Abdul Basit Ijaz via Gdb-patches <gdb-patches@sourceware.org>
Cc: Ijaz, Abdul B <abdul.b.ijaz@intel.com>; JiniSusan.George@amd.com; tom@tromey.com; eliz@gnu.org; blarsen@redhat.com; Nils-Christian Kempke <nils-christian.kempke@intel.com>
Subject: Re: [PATCH v4 3/4] gdb/infrun: handle stepping through functions with DW_AT_trampoline

>>>>> "Abdul" == Abdul Basit Ijaz via Gdb-patches <gdb-patches@sourceware.org> writes:

Abdul> From: Nils-Christian Kempke <nils-christian.kempke@intel.com> 
Abdul> This patch makes infrun continue stepping into and through 
Abdul> trampoline functions marked via the DW_AT_trampoline in DWARF.  
Abdul> The attribute can be emitted by the compiler for certain 
Abdul> subroutines/inlined subroutines that are compiler generated and should be hidden from a user.

Thanks for the patch.

infrun isn't really my area but I do have some nits.

Abdul> +      bool in_trampoline = skip_trampoline_functions
Abdul> +			   && in_trampoline_function (stop_pc);
Abdul> +
Abdul> +      if (in_trampoline)

The assignment is not formatted correctly, but it seems to me that there's no need for this variable at all, and the expression can be inlined into the 'if' -- which will also fix the formatting.

Abdul> +	  real_stop_pc = find_function_trampoline_target (stop_pc);
Abdul> +
Abdul> +	  for (int i = 0; i < MAX_TRAMPOLINE_CHAIN_SIZE
Abdul> +			  && in_trampoline_function (real_stop_pc); ++i)

Normally we'd split at the ';':

    for (int i = 0;
         i < MAX_TRAMPOLINE_CHAIN_SIZE
         && in_trampoline_function (real_stop_pc);
         ++i)

... with the middle expression being all on one line if it fits.

Abdul> +	    {
Abdul> +		real_stop_pc = find_function_trampoline_target (real_stop_pc);
Abdul> +		/* Exit if find_function_trampoline_target failed to find the
Abdul> +		   trampoline target.  Do not try to resolve the trampolines
Abdul> +		   in this case.  */
Abdul> +		if (real_stop_pc == 0x0)

Just '0', not '0x0'.

Abdul> +	  /* If we failed to find a target we will just single step in the
Abdul> +	     hope of leaving the trampoline again soon.  */
Abdul> +	  if (real_stop_pc == 0x0)

Ditto.

Abdul> +      if (real_stop_pc == 0)
Abdul> +	real_stop_pc = skip_language_trampoline (frame, stop_pc);
Abdul>        if (real_stop_pc == 0)
Abdul>  	real_stop_pc = gdbarch_skip_trampoline_code (gdbarch, frame, 
Abdul> stop_pc);

This is pre-existing code, but I wonder how all these different pieces interact, or are supposed to interact.

I guess the language stuff is there to work around a GCC deficiency, in that it does not emit DW_AT_trampoline.

The gdbarch stuff... I don't know but I see a lot of arches implement this (though mostly deferring to the solib code).

Anyway, is it possible to have a gdbarch or c++ trampoline then call a DW_AT_trampoline function?  If so then wouldn't we end up with an erroneous stop?

I'm just wondering if these ought to be unified in some way, like say look for any sort of trampoline target in the loop.

Also LOL at the gnu-v3-abi.c code:

  if (thunk_name == NULL || strstr (thunk_name, " thunk to ") == NULL)
    return 0;

It also seems a bit weird that skip_trampoline_functions only affects this one subset of trampoline functions.

Tom
Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva  
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928

RE: [PATCH v4 4/4] gdb: Skip trampoline frames in the stack for printing or finish command.

[Ijaz, Abdul B]

Hi Tom,

Thanks for the feedback. Added replies below:

Tom> If I am stopped in a frame and go "up", will I be in the trampoline frame anyway?  And will "bt" then not print the selected frame?  But "frame" will?

Yes in current implementation bt will not print those trampoline frames and user may see from the frame number in bt since in output  frame numbers will be missing/elided for those trampoline calls. So "up" command will still take it to trampoline call. Do we need to handle "up" call ? . One sample run from program where first function call the second function and DW_at_trampoline exist for both of them.

22        second = x * y ! second-breakpt
(gdb) bt
#0  second (x=20, y=9) at db.fortran/func-trampoline.f90:22
#2  0x0000000000405203 in first (num1=16, num2=3) at gdb.fortran/func-trampoline.f90:29
#4  0x0000000000405254 in func_trampoline () at gdb.fortran/func-trampoline.f90:35
(gdb) up
#1  0x0000000000405229 in second_.t74p.t75p () at gdb.fortran/func-trampoline.f90:30
(gdb) frame 1
#1  0x0000000000405229 in second_.t74p.t75p () at gdb.fortran/func-trampoline.f90:30
30      end function

Tom > Also this approach ignores frame filters -- if one is in use, then I think trampoline frames will show up again.  You can try this by writing a dummy frame filter that just returns all frames as-is.

Yes in case of frame filter these trampolines frames are shown up.  Do we need to cover it in this change since I think we need to handle in such case in frame filter script or something need to be handled here for this ?

Thanks & Best Regards
Abdul Basit

-----Original Message-----
From: Tom Tromey <tom@tromey.com> 
Sent: Wednesday, August 2, 2023 10:42 PM
To: Abdul Basit Ijaz via Gdb-patches <gdb-patches@sourceware.org>
Cc: Ijaz, Abdul B <abdul.b.ijaz@intel.com>; JiniSusan.George@amd.com; tom@tromey.com; eliz@gnu.org; blarsen@redhat.com
Subject: Re: [PATCH v4 4/4] gdb: Skip trampoline frames in the stack for printing or finish command.

>>>>> "Abdul" == Abdul Basit Ijaz via Gdb-patches <gdb-patches@sourceware.org> writes:

Abdul> Before the change, GDB prints the frames indicated by the 
Abdul> compiler with DIE "DW_AT_trampoline" in the backtrace and finish 
Abdul> command, but for better user experience, all such frames can be 
Abdul> hidden from the user.  So, after this change, now such frames are 
Abdul> not printed any more in the backtrace command and also the 'finish' command skips the trampoline calls.

Abdul> So far, this DIE is added to DWARF only by the IFX compiler, so 
Abdul> gdb.fortran/mixed-lang-stack test used to fail for this compiler 
Abdul> because of these extra trampoline frames in the backtrace.  After 
Abdul> the commit, those trampoline frames are filtered so test is 
Abdul> updated accordingly to handle the frame level of the filtered frames.

Thanks for the patch.

I'm not totally sure how I feel about the stack trace part of this one.

On the one hand, normally I think it makes sense to elide these frames.
They aren't very interesting.

On the other hand, we have frame filters for frame elision, but then this works in a totally different way.

If I am stopped in a frame and go "up", will I be in the trampoline frame anyway?  And will "bt" then not print the selected frame?  But "frame" will?

Abdul> +  if (skip_trampoline_functions)
Abdul> +    {
Abdul> +      for (int i = 0; i < MAX_TRAMPOLINE_CHAIN_SIZE
Abdul> +		      && (frame != nullptr)
Abdul> +		      && in_trampoline_frame (frame); ++i)
Abdul> +	frame = get_prev_frame (frame);

Formatting looks weird here.

Abdul> +++ b/gdb/stack.c
Abdul> @@ -2054,6 +2054,14 @@ backtrace_command_1 (const frame_print_options &fp_opts,
Abdul>  	{
Abdul>  	  QUIT;
 
Abdul> +	  if (in_trampoline_frame (fi))
Abdul> +	    {
Abdul> +	      /* Trampoline frames are not printed so they are not counted in
Abdul> +		 the backtrace limit.  */
Abdul> +	      count++;
Abdul> +	      continue;

I guess the frame numbers will skip as well, because the frame level is a property of the frame, not of the loop displaying frames.

Also this approach ignores frame filters -- if one is in use, then I think trampoline frames will show up again.  You can try this by writing a dummy frame filter that just returns all frames as-is.

Tom
Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva  
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928

RE: [PATCH v4 1/4] gdb, dwarf: add support for DW_AT_trampoline in DWARF reader

[Ijaz, Abdul B]

Hi Tom,

Thanks for the feedback.

Tom> I didn't realize this until starting to read patch 2, but it seems strange to use the name here.  It seems to me it would be better to use the address attached to the referenced subroutine DIE The "bad"
scenario would be where there are multiple functions with the same name here, and gdb's symbol lookup returns the wrong one

For this case of  value of attributes for this DIE may contain mangled name of the target. So for such cases we are preferring the linkage name to store. Shouldn't it cover such issues. Otherwise let me know then I may update accordingly.
As per dwarf spec: "
If the value is of class string, then the value is the (possibly mangled) name  of the target subprogram.
"

Thanks & Best Regards
Abdul Basit

-----Original Message-----
From: Tom Tromey <tom@tromey.com> 
Sent: Wednesday, August 2, 2023 10:13 PM
To: Abdul Basit Ijaz via Gdb-patches <gdb-patches@sourceware.org>
Cc: Ijaz, Abdul B <abdul.b.ijaz@intel.com>; JiniSusan.George@amd.com; tom@tromey.com; eliz@gnu.org; blarsen@redhat.com; Nils-Christian Kempke <nils-christian.kempke@intel.com>
Subject: Re: [PATCH v4 1/4] gdb, dwarf: add support for DW_AT_trampoline in DWARF reader

>>>>> "Abdul" == Abdul Basit Ijaz via Gdb-patches <gdb-patches@sourceware.org> writes:

Abdul> +      else if (attr->form_is_ref ())
Abdul> +	{
Abdul> +	  die_info *target_die;
Abdul> +	  dwarf2_cu *target_cu = cu;
Abdul> +
Abdul> +	  target_die = follow_die_ref (die, attr, &target_cu);
Abdul> +	  const char *target_name;
Abdul> +	  /* Prefer the mangled name; otherwise compute the demangled one.  */
Abdul> +	  target_name = dw2_linkage_name (target_die, target_cu);
Abdul> +	  if (target_name == NULL)
Abdul> +	    target_name = dwarf2_physname (NULL, target_die, target_cu);
Abdul> +	  if (target_name == nullptr)
Abdul> +	    {
Abdul> +	      complaint (_("DW_AT_trampoline target DIE has no name for"
Abdul> +			 "referencing DIE %s [in module %s]"),
Abdul> +			 sect_offset_str (die->sect_off),
Abdul> +			 objfile_name (objfile));
Abdul> +	    }
Abdul> +	  TYPE_TRAMPOLINE_TARGET (ftype)->set_target_name 
Abdul> +(target_name);

I didn't realize this until starting to read patch 2, but it seems strange to use the name here.  It seems to me it would be better to use the address attached to the referenced subroutine DIE.  The "bad"
scenario would be where there are multiple functions with the same name here, and gdb's symbol lookup returns the wrong one.  You can normally trigger these bugs by carefully ordering shared libraries.

Tom
Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva  
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928

RE: [PATCH v4 1/4] gdb, dwarf: add support for DW_AT_trampoline in DWARF reader

[Ijaz, Abdul B]

Hi Tom,

Thanks for the feedback.

Abdul> +	  if (target_name == NULL)
Tom> Probably your patch predates this but nowadays we're using nullptr instead of NULL.
Will fix in V5 patch.

Abdul> +		    (unsigned int) TYPE_FUNC_FLAGS (type));
Tom> If we're just printing the flags as an int, how about using hex instead?
Will update print format to hex in V5 patch.

Abdul> +    func_type_flags flags;
Tom>This should use ENUM_BITFIELD like the other spot in the patch.
This flag may store multiple values together with no return and trampoline types for a function. That is the reason why it is used like this here. So value is already bitfield like:
enum func_type_flag_value : unsigned
{
  FUNC_TYPE_NO_RETURN = (1 << 0),
  FUNC_TYPE_TRAMPOLINE = (1 << 1)
};

Thanks & Best Regards
Abdul Basit

-----Original Message-----
From: Tom Tromey <tom@tromey.com> 
Sent: Wednesday, August 2, 2023 10:08 PM
To: Abdul Basit Ijaz via Gdb-patches <gdb-patches@sourceware.org>
Cc: Ijaz, Abdul B <abdul.b.ijaz@intel.com>; JiniSusan.George@amd.com; tom@tromey.com; eliz@gnu.org; blarsen@redhat.com; Nils-Christian Kempke <nils-christian.kempke@intel.com>
Subject: Re: [PATCH v4 1/4] gdb, dwarf: add support for DW_AT_trampoline in DWARF reader

>>>>> "Abdul" == Abdul Basit Ijaz via Gdb-patches <gdb-patches@sourceware.org> writes:

Abdul> This patch adds support to GDB for reading the DW_AT_trampoline 
Abdul> attribute.  It stores the attribute and its value in the 
Abdul> type_specific part of a GDB type.  This patch is implemented in 
Abdul> preparation of the following patches, which will add a mechanism 
Abdul> to hide DW_AT_trampoline subroutines from the user.

Thanks for the patch.

Abdul> +	  if (target_name == NULL)
Abdul> +	    target_name = dwarf2_physname (NULL, target_die, 
Abdul> +target_cu);

Probably your patch predates this but nowadays we're using nullptr instead of NULL.

Abdul> +	  unrelocated_addr target_addr_reloc = attr->as_address ();
Abdul> +	  CORE_ADDR target_addr
Abdul> +	    = cu->per_objfile->relocate (target_addr_reloc);
Abdul> +	  target_addr = gdbarch_adjust_dwarf2_addr (objfile->arch (),
Abdul> +						    target_addr);

dwarf2_per_objfile::relocate already calls gdbarch_adjust_dwarf2_addr, so that call isn't needed.

Abdul> +	gdb_printf ("%*sfunc_type_flags %u\n", spaces, "",
Abdul> +		    (unsigned int) TYPE_FUNC_FLAGS (type));

If we're just printing the flags as an int, how about using hex instead?

Abdul> -    unsigned int is_noreturn : 1;
Abdul> +    func_type_flags flags;

This should use ENUM_BITFIELD like the other spot in the patch.

Tom
Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva  
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928

Re: [PATCH v4 1/4] gdb, dwarf: add support for DW_AT_trampoline in DWARF reader

[Tom Tromey]

Tom> I didn't realize this until starting to read patch 2, but it seems
Tom> strange to use the name here.  It seems to me it would be better to
Tom> use the address attached to the referenced subroutine DIE

> For this case of value of attributes for this DIE may contain mangled
> name of the target. So for such cases we are preferring the linkage
> name to store. Shouldn't it cover such issues. Otherwise let me know
> then I may update accordingly.

> As per dwarf spec: "
> If the value is of class string, then the value is the (possibly mangled) name  of the target subprogram.
> "

The case I was referring to is the one where the attribute is a
reference to another DIE:

> +      else if (attr->form_is_ref ())

And here, DWARF 5 says:

    If the value is of class reference, then the value specifies the
    debugging information entry of the target subprogram.

That is, the attribute references the subprogram DIE directly, so it
seems like using the name should not be needed.

Tom

Re: [PATCH v4 1/4] gdb, dwarf: add support for DW_AT_trampoline in DWARF reader

[Tom Tromey]

>>>>> Ijaz, Abdul B via Gdb-patches <gdb-patches@sourceware.org> writes:

Abdul> +    func_type_flags flags;

Tom> This should use ENUM_BITFIELD like the other spot in the patch.

> This flag may store multiple values together with no return and
> trampoline types for a function. That is the reason why it is used
> like this here. So value is already bitfield like:

Actually I think I misread and thought this was still using a bitfield,
which it is not.  So I think that bit is fine.

Tom

Re: [PATCH v4 3/4] gdb/infrun: handle stepping through functions with DW_AT_trampoline

[Tom Tromey]

>> It also seems a bit weird that skip_trampoline_functions only affects
>> this one subset of trampoline functions.

> So skip_trampoline_functions is so far only handling the skipping case
> for trampolines indicated by DW_AT_trampoline and not for any other
> case of these thunks emitted by compiler or by signal
> trampolines. Since this case is pretty much defined case for
> subroutines calling the target subroutine but not sure about others
> but may be these solib calls filtering may also be handled with the
> same. Please let me know if we want to filter other trampolines cases
> using the same filter. Mostly it should be "on" case but
> DW_AT_trampoline could be interesting for user if compiler emit it for
> virtual functions and user want to see all the trampoline calls so
> here there could be some cases for end user to see those trampoline
> calls but for thunks or rest I don’t know so let me know then I can
> asses of using the filter option for other trampolines.

Well, it just seems strange to me that after this series we would have
two different ways of handling trampolines, that depend on how the
trampoline is represented by the compiler (something nearly no user will
understand or care about), and that there will be two different ways to
elide frames from a "bt" (there's another one for frame filters).

If we think users don't want to see trampoline frames by default, by all
means elide them, but the setting ought to make sense from a user
perspective, not from a compiler implementer perspective.

Tom

Re: [PATCH v4 4/4] gdb: Skip trampoline frames in the stack for printing or finish command.

[Tom Tromey]

Tom> If I am stopped in a frame and go "up", will I be in the trampoline
Tom> frame anyway?  And will "bt" then not print the selected frame?
Tom> But "frame" will?

> Yes in current implementation bt will not print those trampoline
> frames and user may see from the frame number in bt since in output
> frame numbers will be missing/elided for those trampoline calls. So
> "up" command will still take it to trampoline call. Do we need to
> handle "up" call ?

I don't think so.
It's not handled for other elision cases.

Tom

RE: [PATCH v4 1/4] gdb, dwarf: add support for DW_AT_trampoline in DWARF reader

[Ijaz, Abdul B]

Tom>That is, the attribute references the subprogram DIE directly, so it seems like using the name should not be needed.
Yes right, will use  the address of target subroutine instead of name. Will be updated it in v5.

Thanks & Best Regards
Abdul Basit

-----Original Message-----
From: Tom Tromey <tom@tromey.com> 
Sent: Thursday, August 10, 2023 6:19 PM
To: Ijaz, Abdul B via Gdb-patches <gdb-patches@sourceware.org>
Cc: Tom Tromey <tom@tromey.com>; Ijaz, Abdul B <abdul.b.ijaz@intel.com>; JiniSusan.George@amd.com; eliz@gnu.org; blarsen@redhat.com
Subject: Re: [PATCH v4 1/4] gdb, dwarf: add support for DW_AT_trampoline in DWARF reader

Tom> I didn't realize this until starting to read patch 2, but it seems 
Tom> strange to use the name here.  It seems to me it would be better to 
Tom> use the address attached to the referenced subroutine DIE

> For this case of value of attributes for this DIE may contain mangled 
> name of the target. So for such cases we are preferring the linkage 
> name to store. Shouldn't it cover such issues. Otherwise let me know 
> then I may update accordingly.

> As per dwarf spec: "
> If the value is of class string, then the value is the (possibly mangled) name  of the target subprogram.
> "

The case I was referring to is the one where the attribute is a reference to another DIE:

> +      else if (attr->form_is_ref ())

And here, DWARF 5 says:

    If the value is of class reference, then the value specifies the
    debugging information entry of the target subprogram.

That is, the attribute references the subprogram DIE directly, so it seems like using the name should not be needed.

Tom
Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva  
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928

RE: [PATCH v4 3/4] gdb/infrun: handle stepping through functions with DW_AT_trampoline

[Ijaz, Abdul B]

>Well, it just seems strange to me that after this series we would have two different ways of handling trampolines, that depend on how the trampoline is represented by the compiler (something nearly no user will understand or care about), and that there will be two different ways to elide frames from a "bt" (there's another one for frame filters).

Will elide the frames in V5 series of patch for frame filters also by using similar way in function "frame_object_to_frame_info" used by python which will elide the trampoline frames.

>If we think users don't want to see trampoline frames by default, by all means elide them, but the setting ought to make sense from a user perspective, not from a compiler implementer perspective
So what's the best in your opinion, shall we just elide like other cases which we anyways do as default and only let the user to disable eliding if he wants by using this option.

Thanks & Best Regards
Abdul Basit

-----Original Message-----
From: Tom Tromey <tom@tromey.com> 
Sent: Thursday, August 10, 2023 6:56 PM
To: Ijaz, Abdul B via Gdb-patches <gdb-patches@sourceware.org>
Cc: Tom Tromey <tom@tromey.com>; Ijaz, Abdul B <abdul.b.ijaz@intel.com>; JiniSusan.George@amd.com; eliz@gnu.org; blarsen@redhat.com
Subject: Re: [PATCH v4 3/4] gdb/infrun: handle stepping through functions with DW_AT_trampoline

>> It also seems a bit weird that skip_trampoline_functions only affects 
>> this one subset of trampoline functions.

> So skip_trampoline_functions is so far only handling the skipping case 
> for trampolines indicated by DW_AT_trampoline and not for any other 
> case of these thunks emitted by compiler or by signal trampolines. 
> Since this case is pretty much defined case for subroutines calling 
> the target subroutine but not sure about others but may be these solib 
> calls filtering may also be handled with the same. Please let me know 
> if we want to filter other trampolines cases using the same filter. 
> Mostly it should be "on" case but DW_AT_trampoline could be 
> interesting for user if compiler emit it for virtual functions and 
> user want to see all the trampoline calls so here there could be some 
> cases for end user to see those trampoline calls but for thunks or 
> rest I don’t know so let me know then I can asses of using the filter 
> option for other trampolines.

Well, it just seems strange to me that after this series we would have two different ways of handling trampolines, that depend on how the trampoline is represented by the compiler (something nearly no user will understand or care about), and that there will be two different ways to elide frames from a "bt" (there's another one for frame filters).

If we think users don't want to see trampoline frames by default, by all means elide them, but the setting ought to make sense from a user perspective, not from a compiler implementer perspective.

Tom
Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva  
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928