RE: [PATCH 1/3] gdb: add support for DW_AT_trampoline in DWARF reader

["George, Jini Susan" <JiniSusan.George@amd.com>]

[Public]

Hi Nils,

Thanks for this. A few questions/ comments.

* Is there any common ground we can find with the way solib (linker PLT) trampolines are handled ? I am OK if we can't, but I think it might be better to check and rule it out.

* Also, do we want to resolve and display the target of the trampoline code (if possible) in the disassembled code also ?

* Also, do we want to refer to these trampolines differently so that these can be distinguished from the linker PLT trampolines and the signal trampolines ?

* In read.c, in read_subroutine_type(), where the DW_AT_trampoline attribute is of form reference, we seem to be storing the value corresponding to the  DW_AT_name attribute, whereas in many cases (C++ and other languages), it might make sense to store the mangled name typically obtained from the DW_AT_linkage_name attribute. Given this it might be good to have a C++ test case for this with trampoline target functions inside namespaces also. Maybe with the same target function name under multiple namespaces. Also, the comment in gdbtypes.h (Line 1883) mentions mangled name, which does not seem to match ?

* In gdbtypes.h:

1817
1818     unsigned int is_trampoline : 1;

Might be better to position this field as the 3rd field for better structure packing ?

* infrun.c: Line 7156: It might be better to have the '10' representing the number of chained trampoline traversals allowed as a macro.

A few nits below:
* infrun.c : Nits:
               Line 7154: spelling: certian
         Line 9941: spelling : fucntion

*symtab.h: Nit: line 2247: spelling: wether

Thanks,
Jini.

>>-----Original Message-----
>>From: Gdb-patches <gdb-patches-
>>bounces+jigeorge=amd.com@sourceware.org> On Behalf Of Nils-Christian
>>Kempke via Gdb-patches
>>Sent: Thursday, July 7, 2022 5:46 PM
>>To: gdb-patches@sourceware.org
>>Subject: [PATCH 1/3] gdb: add support for DW_AT_trampoline in DWARF reader
>>
>>[CAUTION: External Email]
>>
>>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_subroutines 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 but not specifying the target 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 done in preparation of the following patches which will add a
>>mechanism to hide DW_AT_trampoline subroutines from the user.
>>
>>gdb/ChangeLog:
>>2022-05-22  Nils-Christian Kempke  <nils-christian.kempke@intel.com>
>>
>>        * dwarf2/read.c (read_subroutine_type): Read in DW_AT_trampoline.
>>        * gdbtypes.c (print_trampoline_target_info): New helper method.
>>        (recursive_dump_type): Add trampoline info.
>>        (copy_type_recursive): Copy trampoline info.
>>        * gdbtypes.h (func_type): Add trampoline specific fields.
>>        (trampoline_target_kind): New enum.
>>        (trampoline_target): New struct for storing DW_AT_trampoline's
>>        value.
>>        (TYPE_IS_TRAMPOLINE): New define.
>>        (TYPE_TRAMPOLINE_TARGET): New define.
>>
>>Signed-off-by: Nils-Christian Kempke <nils-christian.kempke@intel.com>
>>---
>> gdb/dwarf2/read.c | 43 ++++++++++++++++++++++++++
>> gdb/gdbtypes.c    | 31 +++++++++++++++++++
>> gdb/gdbtypes.h    | 79
>>+++++++++++++++++++++++++++++++++++++++++++++++
>> 3 files changed, 153 insertions(+)
>>
>>diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c index
>>23fe5679cbd..7f80c31d051 100644
>>--- a/gdb/dwarf2/read.c
>>+++ b/gdb/dwarf2/read.c
>>@@ -16491,6 +16491,49 @@ 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)
>>+    {
>>+      CORE_ADDR baseaddr = objfile->text_section_offset ();
>>+
>>+      TYPE_IS_TRAMPOLINE (ftype) = true;
>>+      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_physname
>>+         (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 = dwarf2_name (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_physname
>>(target_name);
>>+       }
>>+      else if (attr->form_is_unsigned ())
>>+       TYPE_TRAMPOLINE_TARGET (ftype)->set_target_flag (attr->as_boolean ());
>>+      else
>>+       {
>>+         CORE_ADDR target_addr = attr->as_address ();
>>+         target_addr = gdbarch_adjust_dwarf2_addr (objfile->arch (),
>>+                                                   target_addr + baseaddr);
>>+         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.  */ diff --git a/gdb/gdbtypes.c
>>b/gdb/gdbtypes.c index c8f98554859..66e04882d48 100644
>>--- a/gdb/gdbtypes.c
>>+++ b/gdb/gdbtypes.c
>>@@ -5205,6 +5205,31 @@ 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%lx\n", spaces + 2, "",
>>+                 TYPE_TRAMPOLINE_TARGET (type)->target_physaddr ());
>>+      break;
>>+    case TRAMPOLINE_TARGET_PHYSNAME:
>>+      gdb_printf ("%*starget physname: %s\n", spaces + 2, "",
>>+                 TYPE_TRAMPOLINE_TARGET (type)->target_physname ());
>>+      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.  */
>>@@ -5531,6 +5556,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 ("%*sis_trampoline %d\n", spaces, "",
>>+                   TYPE_IS_TRAMPOLINE (type));
>>+       if (TYPE_IS_TRAMPOLINE (type))
>>+         print_trampoline_target_info (type, spaces);
>>        break;
>>
>>       case TYPE_SPECIFIC_SELF_TYPE:
>>@@ -5762,6 +5791,8 @@ copy_type_recursive (struct objfile *objfile,
>>       TYPE_CALLING_CONVENTION (new_type) = TYPE_CALLING_CONVENTION
>>(type);
>>       TYPE_NO_RETURN (new_type) = TYPE_NO_RETURN (type);
>>       TYPE_TAIL_CALL_LIST (new_type) = NULL;
>>+      TYPE_IS_TRAMPOLINE (new_type) = TYPE_IS_TRAMPOLINE (type);
>>+      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 7437e1db8ab..846de1289ad 100644
>>--- a/gdb/gdbtypes.h
>>+++ b/gdb/gdbtypes.h
>>@@ -1811,6 +1811,83 @@ struct func_type
>>        contains the method.  */
>>
>>     struct type *self_type;
>>+
>>+    /* * For functions marked with the DW_AT_trampoline.  These are
>>+       compiler generated wrappers that should be hidden from the user.
>>+ */
>>+
>>+    unsigned int is_trampoline : 1;
>>+
>>+    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_PHYSNAME,
>>+    /* An flag (target is unknown).  */
>>+    TRAMPOLINE_TARGET_FLAG,
>>+  };
>>+
>>+struct trampoline_target
>>+  {
>>+    trampoline_target_kind target_kind () const
>>+    {
>>+      return m_target_kind;
>>+    }
>>+
>>+    void set_target_physaddr (CORE_ADDR physaddr)
>>+    {
>>+      m_target_kind = TRAMPOLINE_TARGET_PHYSADDR;
>>+      m_trampoline_target.physaddr = physaddr;
>>+    }
>>+
>>+    CORE_ADDR target_physaddr () const
>>+    {
>>+      gdb_assert (m_target_kind == TRAMPOLINE_TARGET_PHYSADDR);
>>+      return m_trampoline_target.physaddr;
>>+    }
>>+
>>+    void set_target_physname (const char *physname)
>>+    {
>>+      m_target_kind = TRAMPOLINE_TARGET_PHYSNAME;
>>+      m_trampoline_target.physname = physname;
>>+    }
>>+
>>+    const char *target_physname () const
>>+    {
>>+      gdb_assert (m_target_kind == TRAMPOLINE_TARGET_PHYSNAME);
>>+      return m_trampoline_target.physname;
>>+    }
>>+
>>+    void set_target_flag (bool flag)
>>+    {
>>+      m_target_kind = TRAMPOLINE_TARGET_FLAG;
>>+      m_trampoline_target.flag = flag;
>>+    }
>>+
>>+    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 *physname;
>>+      /* Flag.  */
>>+      bool flag;
>>+    } m_trampoline_target;
>>+
>>+    /* * Discriminant for union m_trampoline_target.  */
>>+    ENUM_BITFIELD (trampoline_target_kind) m_target_kind : 2;
>>   };
>>
>> /* struct call_site_parameter can be referenced in callees by several ways.  */
>>@@ -2172,6 +2249,8 @@ extern void set_type_vptr_basetype (struct type *,
>>struct 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_TAIL_CALL_LIST(thistype) TYPE_MAIN_TYPE(thistype)-
>>>type_specific.func_stuff->tail_call_list
>>+#define TYPE_IS_TRAMPOLINE(thistype)
>>+TYPE_MAIN_TYPE(thistype)->type_specific.func_stuff->is_trampoline
>>+#define TYPE_TRAMPOLINE_TARGET(thistype)
>>+TYPE_MAIN_TYPE(thistype)->type_specific.func_stuff->self_trampoline_tar
>>+get
>> #define TYPE_BASECLASS(thistype,index) ((thistype)->field (index).type ())
>>#define TYPE_N_BASECLASSES(thistype) TYPE_CPLUS_SPECIFIC(thistype)-
>>>n_baseclasses
>> #define TYPE_BASECLASS_NAME(thistype,index) (thistype->field (index).name ())
>>--
>>2.25.1
>>
>>Intel Deutschland GmbH
>>Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
>>Tel: +49 89 99 8853-0,
>>https://nam11.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.int
>>el.de%2F&amp;data=05%7C01%7CJiniSusan.George%40amd.com%7C9c3f28b5
>>98cf49f888f908da601272a0%7C3dd8961fe4884e608e11a82d994e183d%7C0%7
>>C0%7C637927929632561872%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLj
>>AwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C
>>%7C&amp;sdata=MBaQ7XpSZ4XFO963dnxF%2Fy1PD0gcx6k%2FjdaifxoSUaY%3D
>>&amp;reserved=0
>><https://nam11.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.in
>>tel.de%2F&amp;data=05%7C01%7CJiniSusan.George%40amd.com%7C9c3f28b5
>>98cf49f888f908da601272a0%7C3dd8961fe4884e608e11a82d994e183d%7C0%7
>>C0%7C637927929632561872%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLj
>>AwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C
>>%7C&amp;sdata=MBaQ7XpSZ4XFO963dnxF%2Fy1PD0gcx6k%2FjdaifxoSUaY%3D
>>&amp;reserved=0>
>>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