public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH 1/3] gdb: add support for DW_AT_trampoline in DWARF reader
@ 2023-01-03  1:07 Ijaz, Abdul B
  0 siblings, 0 replies; 7+ messages in thread
From: Ijaz, Abdul B @ 2023-01-03  1:07 UTC (permalink / raw)
  To: JiniSusan.George, gdb-patches; +Cc: Ijaz, Abdul B

[-- Attachment #1: Type: text/plain, Size: 25945 bytes --]

Hi Jini,



Thanks for your feedback on the patch.



1)  Regarding solib and signal trampolines, Symbol trampoline or solib trampoline not just identification but handling also is completely different as far as I can understand as compare to DW_AT_trampoline DIE. Since for the signal case it is handled differently for each target, for solib it trampoline symbol is initialized  when symbol table is created for undefined symbols that are defined in some shared lib info so later then it is used for handling trampolines whereas DW_AT_trampoline handling is common for each target and each trampoline has a different frame for caller and callee functions.



2)  Regarding the mangled/demangled name it will be updated in V2 to look first mangled and then demangled name.

3)  Regarding bitfield alignment in struct func_type both is_noreturn & is_trampoline will be put together to single unsigned int type value in V2 patch.

4)  In addition, regarding this patch I think so far important missing part  regarding this change would be if we should filter the trampoline frames for commands like backtrace or finish commands. Since in case we filter trampoline frame while printing then frame levels need some special handling which seems to be tricky so would be nice to have others opinion regarding the expected behavior for these trampoline frames if we should handle frame->level for trampoline frames in get_prev_frame_raw() or just skipping such frames in stack print would be ok:



As an example where first and second functions having trampoline calls in the DWARF:

Backtrace without filtering so far shows trampoline calls in backtrace:

(gdb) backtrace 3

#0  second (x=20, y=9) at call-stack.f90:20

#1  0x00000000004049f9 in second_.t74p.t75p () at call-stack.f90:29

#2  0x00000000004049d3 in first (num1=16, num2=3) at call-stack.f90:27



If we filter the trampoline frames while printing and handle them for commands like finish then Backtrace with filtering where frame #1 and #3 are trampoline frames so they are not shown in backtrace:

(gdb) backtrace 3

#0  second (x=20, y=9) at call-stack.f90:20

#2  0x00000000004049d3 in first (num1=16, num2=3) at call-stack.f90:27

#4  0x0000000000405183 in MAIN__ at call-stack.f90:57





Thanks & Best Regards

Abdul Basit Ijaz



>>-----Original Message-----

>>> -----Original Message-----

>>> From: George, Jini Susan <JiniSusan.George@amd.com<mailto:JiniSusan.George@amd.com>>

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

>>

>>I don't quite understand what you mean here but am also not super familiar with

>>the solib handling part of GDB.  The attached patches treat any function that

>>was marked as DW_AT_Trampoline in DWARF by the compiler.  As far as I know

>>not a single compiler (except ifx) emits this attribute up until now.  If a compiler

>>were to generate DWARF for the solib trampolines and if it were to add the

>>attribute, we could also treat solib trampolines with this approach.

>>But generally, the DW_AT_trampoline is independent of any other GDB

>>trampoline handling.

>>Not sure whether this answers your question.  Maybe you can elaborate a bit

>>more?



>From what I can make out, the solib trampolines (marked in the minimal symbol table as ms_type = mst_solib_trampoline) are identified in gdb from the elf symbol table based on their type. (The identification of these trampolines are different, and I would not think there would be any common ground here wrt the identification). But there could be some common ground wrt the handling. For example, while stepping through the code, we don't step into the solib trampolines, which typically are named as <target_func_name@plt<mailto:target_func_name@plt>>, but we end up stepping into the resolved target function. It would be great if that could be checked out. If it looks like there is nothing in common, that's fine, but it would be great if we make that decision after checking it out.





>>> * Also, do we want to resolve and display the target of the trampoline

>>> code (if possible) in the disassembled code also ?

>>>

>>

>>So you mean, when I am about to step into a wrapper/step through a wrapper I

>>would display the target at the respective call statement with some note like

>>

>>(gdb) disassemble

>>...

>>0x080483fa <+0>: push %ebp

>>0x080483fb <+1>: mov %esp,%ebp

>>0x080483fd <+3>: sub $0x8,%esp

>>=> 0x08048400 <+6>: movl $0x2,0x4(%esp)

>>0x08048408 <+14>: movl $0x1,(%esp)

>>0x0804840f <+21>: call 0x80483ed <func> (trampoline to: 0x12345 <other>)

>>

>>?

>>So a change in the pretty_printer for assembly I assume (or somewhere deeper).

>>

>>This would mean, GDB would have to check every time a call appears

>>somewhere in the disassembly whether or not the target is a trampoline, and, if

>>so, GDB would need to resolve this.

>>

>>Generally, I think this might be a nice feature.  I'll have a look whether it can be

>>done without too much effort, but if it turns out to be considerably difficult I'd

>>rather like to leave this for a different patch.

>>If this is already done in the other trampoline cases (solibs and signals) it might

>>be needed for the completion of this patch though.



I think you can disregard this comment of mine unless the effort involved is trivial. Wrt the solib trampolines, the target function names are embedded in the names of the trampolines (as <target_function_name>@plt, and the gdb disassembler would not have to do anything special to get that displayed.



>>> * 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 ?

>>>

>>

>>I hoped that the patches made the difference clear enough.  Or can you pinpoint

>>something that seems unclear?  How would you instead refer to them, I am a bit

>>lost here, to be honest.  They are, after all, according to their DWARF/debug

>>info trampolines.  Referring to them differently would in my opinion maybe

>>obscure things?  But maybe you had something in mind.



I was only referring to some term which could be used to refer to these trampolines (like how we already have 'solib trampolines' and 'signal trampolines'). But if none of the terms (compiler trampolines, debug info trampolines) seem to fit right, you can disregard this comment.



>>> * 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 ?

>>

>>Yes, I think initially the comment was intended to only describe the

>>DW_AT_trampoline when its FORM is string.  That string could have been both,

>>mangled and demangled.  But I get your point.  I am not sure whether storing

>>the linkage name or the 'normal' name is correct here.  I think these are two

>>different issues: which name should be stored and should it be demangled or

>>not.  But I'll check this again.  I guess I'll have to check symbol lookup again for

>>this and see what makes most sense.



lookup_minimal_symbol() does a 2 pass search over both the mangled and the demangled names. If we end up comparing the demangled names, the name lookup might be incorrect since we will not be comparing fully qualified names.



>>> * 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 ?

>>

>>The structure was poorly packed in the first place I think.  We have the

>>ENUM_BITFIELD in the beginning which can be either an enum or an unsigned

>>int.  I am not sure whether the size of an enum is defined somewhere but I'd

>>assume it is compiler dependent and usually the size of an int?

>>This is followed by another unsigned int and then by two pointers.

>>

>>If we aimed at proper structure packing I'd assume the pointers (which would

>>usually be 8 bytes to all come first, then the (unsigned) int's with their (usually)

>>4 bytes, followed finally by the enum?

>>

>>I considered this when implementing it but it seemed like an unrelated change to

>>me.  Assuming int is 4 bytes and we align to 8 bytes moving is_trampoline to the

>>3rd position would also not do anything, would it?  We'd be at

>>

>>4 byte

>>4 byte

>>4 byte

>>8 byte

>>8 byte

>>8 byte

>>

>>Instead of

>>

>>4 byte

>>4 byte

>>8 byte

>>8 byte

>>4 byte

>>8 byte



I think you missed taking the bitfields (is_noreturn and is_trampoline) into consideration. If these get placed together, they would be fitted into one int since both these are just 1 bit each.





>>

>>I think switching is_trampoline and self_trampoline_target would make the most

>>sense here? (without touching the other members).

>>

>>> * infrun.c: Line 7156: It might be better to have the '10'

>>> representing the number of chained trampoline traversals allowed as a macro.

>>>

>>

>>I also thought about this when implementing it.  I back then decided against it

>>but sure, I'll add that.



Thanks,

Jini.



>>

>>> A few nits below:

>>> * infrun.c : Nits:

>>>                Line 7154: spelling: certian

>>>          Line 9941: spelling : fucntion

>>>

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

>>

>>Thanks!

>>I'll change this in v2!

>>

>>>

>>> Thanks,

>>> Jini.

>>>

>>> >>-----Original Message-----

>>> >>From: Gdb-patches <gdb-patches-

>>> >>bounces+jigeorge=amd.com@sourceware.org<mailto: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<mailto: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<mailto: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<mailto: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%2Fw

>>> ww.int

>>> >>el.de%2F&data=05%7C01%7CJiniSusan.George%40amd.com%7C9c3f

>>> 28b5

>>>

>>>>98cf49f888f908da601272a0%7C3dd8961fe4884e608e11a82d994e183d%7C0

>>> %7

>>> >>C0%7C637927929632561872%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC

>>> 4wLj

>>> >>AwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C

>>> %7C

>>> >>%7C&sdata=MBaQ7XpSZ4XFO963dnxF%2Fy1PD0gcx6k%2FjdaifxoSUa

>>> Y%3D

>>> >>&reserved=0

>>> >><https://nam11.safelinks.protection.outlook.com/?url=http%3A%2F%2Fw

>>> ww.in

>>> >>tel.de%2F&data=05%7C01%7CJiniSusan.George%40amd.com%7C9c3

>>> f28b5

>>>

>>>>98cf49f888f908da601272a0%7C3dd8961fe4884e608e11a82d994e183d%7C0

>>> %7

>>> >>C0%7C637927929632561872%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC

>>> 4wLj

>>> >>AwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C

>>> %7C

>>> >>%7C&sdata=MBaQ7XpSZ4XFO963dnxF%2Fy1PD0gcx6k%2FjdaifxoSUa

>>> Y%3D

>>> >>&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

>>

>>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&data=05%7C01%7CJiniSusan.George%40amd.com%7Cd62c3e20

>>92aa47098d9908da64c720a7%7C3dd8961fe4884e608e11a82d994e183d%7C0%

>>7C0%7C637933103654397897%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wL

>>jAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C

>>%7C&sdata=SqOet8GRTDgMHyLH9TKkvnaiK1N0AO1NQG09Pln8iJQ%3D&a

>>mp;reserved=0

>><https://nam11.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.in

>>tel.de%2F&data=05%7C01%7CJiniSusan.George%40amd.com%7Cd62c3e2

>>092aa47098d9908da64c720a7%7C3dd8961fe4884e608e11a82d994e183d%7C0

>>%7C0%7C637933103654397897%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4

>>wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C

>>%7C%7C&sdata=SqOet8GRTDgMHyLH9TKkvnaiK1N0AO1NQG09Pln8iJQ%3

>>D&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



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

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

* Re: [PATCH 1/3] gdb: add support for DW_AT_trampoline in DWARF reader
  2022-07-17 12:57     ` George, Jini Susan
@ 2022-07-18 16:11       ` Tom Tromey
  0 siblings, 0 replies; 7+ messages in thread
From: Tom Tromey @ 2022-07-18 16:11 UTC (permalink / raw)
  To: George, Jini Susan via Gdb-patches
  Cc: Kempke, Nils-Christian, George, Jini Susan

>>> I considered this when implementing it but it seemed like an unrelated change to
>>> me.  Assuming int is 4 bytes and we align to 8 bytes moving is_trampoline to the
>>> 3rd position would also not do anything, would it?  We'd be at
[...]

> I think you missed taking the bitfields (is_noreturn and
> is_trampoline) into consideration. If these get placed together, they
> would be fitted into one int since both these are just 1 bit each.

FWIW you can check the packing using 'ptype/o'.
I'm also inclined to say the bitfields should be adjacent.

thanks,
Tom

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

* RE: [PATCH 1/3] gdb: add support for DW_AT_trampoline in DWARF reader
  2022-07-13 11:59   ` Kempke, Nils-Christian
@ 2022-07-17 12:57     ` George, Jini Susan
  2022-07-18 16:11       ` Tom Tromey
  0 siblings, 1 reply; 7+ messages in thread
From: George, Jini Susan @ 2022-07-17 12:57 UTC (permalink / raw)
  To: Kempke, Nils-Christian, gdb-patches

[Public]

Hi Nils,

My answers are inlined below.

>>-----Original Message-----
>>> -----Original Message-----
>>> From: George, Jini Susan <JiniSusan.George@amd.com>
>>> * 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.
>>
>>I don't quite understand what you mean here but am also not super familiar with
>>the solib handling part of GDB.  The attached patches treat any function that
>>was marked as DW_AT_Trampoline in DWARF by the compiler.  As far as I know
>>not a single compiler (except ifx) emits this attribute up until now.  If a compiler
>>were to generate DWARF for the solib trampolines and if it were to add the
>>attribute, we could also treat solib trampolines with this approach.
>>But generally, the DW_AT_trampoline is independent of any other GDB
>>trampoline handling.
>>Not sure whether this answers your question.  Maybe you can elaborate a bit
>>more?

From what I can make out, the solib trampolines (marked in the minimal symbol table as ms_type = mst_solib_trampoline) are identified in gdb from the elf symbol table based on their type. (The identification of these trampolines are different, and I would not think there would be any common ground here wrt the identification). But there could be some common ground wrt the handling. For example, while stepping through the code, we don't step into the solib trampolines, which typically are named as <target_func_name@plt>, but we end up stepping into the resolved target function. It would be great if that could be checked out. If it looks like there is nothing in common, that's fine, but it would be great if we make that decision after checking it out.


>>> * Also, do we want to resolve and display the target of the trampoline
>>> code (if possible) in the disassembled code also ?
>>>
>>
>>So you mean, when I am about to step into a wrapper/step through a wrapper I
>>would display the target at the respective call statement with some note like
>>
>>(gdb) disassemble
>>...
>>0x080483fa <+0>: push %ebp
>>0x080483fb <+1>: mov %esp,%ebp
>>0x080483fd <+3>: sub $0x8,%esp
>>=> 0x08048400 <+6>: movl $0x2,0x4(%esp)
>>0x08048408 <+14>: movl $0x1,(%esp)
>>0x0804840f <+21>: call 0x80483ed <func> (trampoline to: 0x12345 <other>)
>>
>>?
>>So a change in the pretty_printer for assembly I assume (or somewhere deeper).
>>
>>This would mean, GDB would have to check every time a call appears
>>somewhere in the disassembly whether or not the target is a trampoline, and, if
>>so, GDB would need to resolve this.
>>
>>Generally, I think this might be a nice feature.  I'll have a look whether it can be
>>done without too much effort, but if it turns out to be considerably difficult I'd
>>rather like to leave this for a different patch.
>>If this is already done in the other trampoline cases (solibs and signals) it might
>>be needed for the completion of this patch though.

I think you can disregard this comment of mine unless the effort involved is trivial. Wrt the solib trampolines, the target function names are embedded in the names of the trampolines (as <target_function_name>@plt, and the gdb disassembler would not have to do anything special to get that displayed.

>>> * 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 ?
>>>
>>
>>I hoped that the patches made the difference clear enough.  Or can you pinpoint
>>something that seems unclear?  How would you instead refer to them, I am a bit
>>lost here, to be honest.  They are, after all, according to their DWARF/debug
>>info trampolines.  Referring to them differently would in my opinion maybe
>>obscure things?  But maybe you had something in mind.

I was only referring to some term which could be used to refer to these trampolines (like how we already have 'solib trampolines' and 'signal trampolines'). But if none of the terms (compiler trampolines, debug info trampolines) seem to fit right, you can disregard this comment.

>>> * 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 ?
>>
>>Yes, I think initially the comment was intended to only describe the
>>DW_AT_trampoline when its FORM is string.  That string could have been both,
>>mangled and demangled.  But I get your point.  I am not sure whether storing
>>the linkage name or the 'normal' name is correct here.  I think these are two
>>different issues: which name should be stored and should it be demangled or
>>not.  But I'll check this again.  I guess I'll have to check symbol lookup again for
>>this and see what makes most sense.

lookup_minimal_symbol() does a 2 pass search over both the mangled and the demangled names. If we end up comparing the demangled names, the name lookup might be incorrect since we will not be comparing fully qualified names.

>>> * 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 ?
>>
>>The structure was poorly packed in the first place I think.  We have the
>>ENUM_BITFIELD in the beginning which can be either an enum or an unsigned
>>int.  I am not sure whether the size of an enum is defined somewhere but I'd
>>assume it is compiler dependent and usually the size of an int?
>>This is followed by another unsigned int and then by two pointers.
>>
>>If we aimed at proper structure packing I'd assume the pointers (which would
>>usually be 8 bytes to all come first, then the (unsigned) int's with their (usually)
>>4 bytes, followed finally by the enum?
>>
>>I considered this when implementing it but it seemed like an unrelated change to
>>me.  Assuming int is 4 bytes and we align to 8 bytes moving is_trampoline to the
>>3rd position would also not do anything, would it?  We'd be at
>>
>>4 byte
>>4 byte
>>4 byte
>>8 byte
>>8 byte
>>8 byte
>>
>>Instead of
>>
>>4 byte
>>4 byte
>>8 byte
>>8 byte
>>4 byte
>>8 byte

I think you missed taking the bitfields (is_noreturn and is_trampoline) into consideration. If these get placed together, they would be fitted into one int since both these are just 1 bit each.


>>
>>I think switching is_trampoline and self_trampoline_target would make the most
>>sense here? (without touching the other members).
>>
>>> * infrun.c: Line 7156: It might be better to have the '10'
>>> representing the number of chained trampoline traversals allowed as a macro.
>>>
>>
>>I also thought about this when implementing it.  I back then decided against it
>>but sure, I'll add that.

Thanks,
Jini.

>>
>>> A few nits below:
>>> * infrun.c : Nits:
>>>                Line 7154: spelling: certian
>>>          Line 9941: spelling : fucntion
>>>
>>> *symtab.h: Nit: line 2247: spelling: wether
>>
>>Thanks!
>>I'll change this in v2!
>>
>>>
>>> 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%2Fw
>>> ww.int
>>> >>el.de%2F&amp;data=05%7C01%7CJiniSusan.George%40amd.com%7C9c3f
>>> 28b5
>>>
>>>>98cf49f888f908da601272a0%7C3dd8961fe4884e608e11a82d994e183d%7C0
>>> %7
>>> >>C0%7C637927929632561872%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC
>>> 4wLj
>>> >>AwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C
>>> %7C
>>> >>%7C&amp;sdata=MBaQ7XpSZ4XFO963dnxF%2Fy1PD0gcx6k%2FjdaifxoSUa
>>> Y%3D
>>> >>&amp;reserved=0
>>> >><https://nam11.safelinks.protection.outlook.com/?url=http%3A%2F%2Fw
>>> ww.in
>>> >>tel.de%2F&amp;data=05%7C01%7CJiniSusan.George%40amd.com%7C9c3
>>> f28b5
>>>
>>>>98cf49f888f908da601272a0%7C3dd8961fe4884e608e11a82d994e183d%7C0
>>> %7
>>> >>C0%7C637927929632561872%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC
>>> 4wLj
>>> >>AwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C
>>> %7C
>>> >>%7C&amp;sdata=MBaQ7XpSZ4XFO963dnxF%2Fy1PD0gcx6k%2FjdaifxoSUa
>>> Y%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
>>
>>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%7Cd62c3e20
>>92aa47098d9908da64c720a7%7C3dd8961fe4884e608e11a82d994e183d%7C0%
>>7C0%7C637933103654397897%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wL
>>jAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C
>>%7C&amp;sdata=SqOet8GRTDgMHyLH9TKkvnaiK1N0AO1NQG09Pln8iJQ%3D&a
>>mp;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%7Cd62c3e2
>>092aa47098d9908da64c720a7%7C3dd8961fe4884e608e11a82d994e183d%7C0
>>%7C0%7C637933103654397897%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4
>>wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C
>>%7C%7C&amp;sdata=SqOet8GRTDgMHyLH9TKkvnaiK1N0AO1NQG09Pln8iJQ%3
>>D&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


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

* Re: [PATCH 1/3] gdb: add support for DW_AT_trampoline in DWARF reader
  2022-07-07 12:15 Nils-Christian Kempke
  2022-07-13  7:39 ` George, Jini Susan
@ 2022-07-15 20:05 ` Tom Tromey
  1 sibling, 0 replies; 7+ messages in thread
From: Tom Tromey @ 2022-07-15 20:05 UTC (permalink / raw)
  To: Nils-Christian Kempke via Gdb-patches

>>>>> Nils-Christian Kempke via Gdb-patches <gdb-patches@sourceware.org> writes:

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

Thank you for the patch.

> +	  const char* target_name = dwarf2_name (target_die, target_cu);

"*" comes after the space.

> +	{
> +	  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);

If a data structure holds an address that already has the section offset
applied, then objfile_relocate1 must be updated to relocate the value.

However, if you can possibly avoid adding the section offset, in favor
of handling this when the address is used, then that is better IMO.

thanks,
Tom

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

* RE: [PATCH 1/3] gdb: add support for DW_AT_trampoline in DWARF reader
  2022-07-13  7:39 ` George, Jini Susan
@ 2022-07-13 11:59   ` Kempke, Nils-Christian
  2022-07-17 12:57     ` George, Jini Susan
  0 siblings, 1 reply; 7+ messages in thread
From: Kempke, Nils-Christian @ 2022-07-13 11:59 UTC (permalink / raw)
  To: George, Jini Susan, gdb-patches

Hi Jini,

Thanks for the feedback!

I have a few questions regarding your comments as I inlined below.

Cheers,
Nils

> -----Original Message-----
> From: George, Jini Susan <JiniSusan.George@amd.com>
> Sent: Wednesday, July 13, 2022 9:40 AM
> To: Kempke, Nils-Christian <nils-christian.kempke@intel.com>; gdb-
> patches@sourceware.org
> Subject: RE: [PATCH 1/3] gdb: add support for DW_AT_trampoline in DWARF
> reader
> 
> [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.

I don't quite understand what you mean here but am also not super familiar
with the solib handling part of GDB.  The attached patches treat any function
that was marked as DW_AT_Trampoline in DWARF by the compiler.  As far as I
know not a single compiler (except ifx) emits this attribute up until now.  If a
compiler were to generate DWARF for the solib trampolines and if it were to
add the attribute, we could also treat solib trampolines with this approach.
But generally, the DW_AT_trampoline is independent of any other GDB
trampoline handling.
Not sure whether this answers your question.  Maybe you can elaborate a bit
more?

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

So you mean, when I am about to step into a wrapper/step through a wrapper
I would display the target at the respective call statement with some note like

(gdb) disassemble
...
0x080483fa <+0>: push %ebp
0x080483fb <+1>: mov %esp,%ebp
0x080483fd <+3>: sub $0x8,%esp
=> 0x08048400 <+6>: movl $0x2,0x4(%esp)
0x08048408 <+14>: movl $0x1,(%esp)
0x0804840f <+21>: call 0x80483ed <func> (trampoline to: 0x12345 <other>)

?
So a change in the pretty_printer for assembly I assume (or somewhere deeper).

This would mean, GDB would have to check every time a call appears
somewhere in the disassembly whether or not the target is a trampoline, and,
if so, GDB would need to resolve this.

Generally, I think this might be a nice feature.  I'll have a look whether it can be
done without too much effort, but if it turns out to be considerably difficult I'd
rather like to leave this for a different patch.
If this is already done in the other trampoline cases (solibs and signals) it might
be needed for the completion of this patch though.

> * 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 ?
> 

I hoped that the patches made the difference clear enough.  Or can you
pinpoint something that seems unclear?  How would you instead refer to
them, I am a bit lost here, to be honest.  They are, after all, according to
their DWARF/debug info trampolines.  Referring to them differently would
in my opinion maybe obscure things?  But maybe you had something in mind.

> * 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 ?

Yes, I think initially the comment was intended to only describe the
DW_AT_trampoline when its FORM is string.  That string could have been
both, mangled and demangled.  But I get your point.  I am not sure whether
storing the linkage name or the 'normal' name is correct here.  I think these are
two different issues: which name should be stored and should it be
demangled or not.  But I'll check this again.  I guess I'll have to check symbol
lookup again for this and see what makes most sense.

> 
> * 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 ?

The structure was poorly packed in the first place I think.  We have the
ENUM_BITFIELD in the beginning which can be either an enum or an unsigned
int.  I am not sure whether the size of an enum is defined somewhere but I'd
assume it is compiler dependent and usually the size of an int?
This is followed by another unsigned int and then by two pointers.

If we aimed at proper structure packing I'd assume the pointers (which would
usually be 8 bytes to all come first, then the (unsigned) int's with their (usually)
4 bytes, followed finally by the enum?

I considered this when implementing it but it seemed like an unrelated change
to me.  Assuming int is 4 bytes and we align to 8 bytes moving is_trampoline to
the 3rd position would also not do anything, would it?  We'd be at

4 byte
4 byte
4 byte
8 byte
8 byte
8 byte

Instead of 

4 byte
4 byte
8 byte
8 byte
4 byte
8 byte

I think switching is_trampoline and self_trampoline_target would make the
most sense here? (without touching the other members).

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

I also thought about this when implementing it.  I back then decided against it
but sure, I'll add that.

> A few nits below:
> * infrun.c : Nits:
>                Line 7154: spelling: certian
>          Line 9941: spelling : fucntion
> 
> *symtab.h: Nit: line 2247: spelling: wether

Thanks!
I'll change this in v2!

> 
> 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%2Fw
> ww.int
> >>el.de%2F&amp;data=05%7C01%7CJiniSusan.George%40amd.com%7C9c3f
> 28b5
> >>98cf49f888f908da601272a0%7C3dd8961fe4884e608e11a82d994e183d%7C0
> %7
> >>C0%7C637927929632561872%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC
> 4wLj
> >>AwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C
> %7C
> >>%7C&amp;sdata=MBaQ7XpSZ4XFO963dnxF%2Fy1PD0gcx6k%2FjdaifxoSUa
> Y%3D
> >>&amp;reserved=0
> >><https://nam11.safelinks.protection.outlook.com/?url=http%3A%2F%2Fw
> ww.in
> >>tel.de%2F&amp;data=05%7C01%7CJiniSusan.George%40amd.com%7C9c3
> f28b5
> >>98cf49f888f908da601272a0%7C3dd8961fe4884e608e11a82d994e183d%7C0
> %7
> >>C0%7C637927929632561872%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC
> 4wLj
> >>AwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C
> %7C
> >>%7C&amp;sdata=MBaQ7XpSZ4XFO963dnxF%2Fy1PD0gcx6k%2FjdaifxoSUa
> Y%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

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


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

* RE: [PATCH 1/3] gdb: add support for DW_AT_trampoline in DWARF reader
  2022-07-07 12:15 Nils-Christian Kempke
@ 2022-07-13  7:39 ` George, Jini Susan
  2022-07-13 11:59   ` Kempke, Nils-Christian
  2022-07-15 20:05 ` Tom Tromey
  1 sibling, 1 reply; 7+ messages in thread
From: George, Jini Susan @ 2022-07-13  7:39 UTC (permalink / raw)
  To: Nils-Christian Kempke, gdb-patches

[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


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

* [PATCH 1/3] gdb: add support for DW_AT_trampoline in DWARF reader
@ 2022-07-07 12:15 Nils-Christian Kempke
  2022-07-13  7:39 ` George, Jini Susan
  2022-07-15 20:05 ` Tom Tromey
  0 siblings, 2 replies; 7+ messages in thread
From: Nils-Christian Kempke @ 2022-07-07 12:15 UTC (permalink / raw)
  To: gdb-patches

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_target
 #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, 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


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

end of thread, other threads:[~2023-01-03  1:07 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-01-03  1:07 [PATCH 1/3] gdb: add support for DW_AT_trampoline in DWARF reader Ijaz, Abdul B
  -- strict thread matches above, loose matches on Subject: below --
2022-07-07 12:15 Nils-Christian Kempke
2022-07-13  7:39 ` George, Jini Susan
2022-07-13 11:59   ` Kempke, Nils-Christian
2022-07-17 12:57     ` George, Jini Susan
2022-07-18 16:11       ` Tom Tromey
2022-07-15 20:05 ` Tom Tromey

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).