* [PATCH v3 0/4] GDB support for DW_AT_trampoline
@ 2023-07-10 22:56 Abdul Basit Ijaz
2023-07-10 22:56 ` [PATCH v3 1/4] gdb, dwarf: add support for DW_AT_trampoline in DWARF reader Abdul Basit Ijaz
` (3 more replies)
0 siblings, 4 replies; 12+ messages in thread
From: Abdul Basit Ijaz @ 2023-07-10 22:56 UTC (permalink / raw)
To: gdb-patches; +Cc: abdul.b.ijaz, JiniSusan.George, tom, eliz
From: "Ijaz, Abdul B" <abdul.b.ijaz@intel.com>
Hi,
please find attached a series of patches that attempts to enable GDB to
handle the DW_AT_trampoline attribute on functions and inlined
functions.
DW_AT_trampoline can be emitted by the compiler for functions that are
compiler generated trampolines (e.g. wrapping other function calls).
In doing so, the compiler can specify a target as the value of
DW_AT_trampoline which the trampoline is wrapping.
This series enables GDB to recognize the DW_AT_trampoline and store the
target for a given function. Patch 3 adapts GDB's stepping behavior
when dealing with trampolines and attempts to - by default - hide these
trampolines from the user. When about to step into a trampoline,
instead, GDB will try and step through the trampoline and directly
towards the target. Patch 4 in V2 now skip trampoline frames
in the stack for printing or finish command. A new setting has been
introduced to turn off this modified stepping behavior or printing of stack
for trampolines.
The motivation for these patches comes from ifx which emits the
attribute for some of its compiler generated functions. As I do not
know of any other compiler (especially gcc/gfortran) emitting
DW_AT_trampoline, I added a gdb.dwarf2 test to this series in order to
test the trampoline handling within GDB.
Changes since V2:
* Mention "on" as default value for skip-trampoline-functions in NEWS
and gdb.textinfo according to the latest feedback.
* Update copyright years from 2022 to 2023 in test files which are
newly added.
V2 series of patches can be found here:
https://sourceware.org/pipermail/gdb-patches/2023-June/200066.html
No regression seen on testing x64/x86/gdbserver test configurations for
these changes using Ubuntu 22.
Abdul Basit Ijaz (1):
gdb: Skip trampoline frames in the stack for printing or finish
command.
Nils-Christian Kempke (3):
gdb, dwarf: add support for DW_AT_trampoline in DWARF reader
gdb/symtab: add lookup for trampoline functions
gdb/infrun: handle stepping through functions with DW_AT_trampoline
gdb/NEWS | 15 ++
gdb/doc/gdb.texinfo | 47 ++++
gdb/dwarf2/read.c | 45 +++-
gdb/gdbtypes.c | 35 ++-
gdb/gdbtypes.h | 103 +++++++-
gdb/infcmd.c | 13 +
gdb/infrun.c | 81 +++++-
gdb/infrun.h | 10 +
gdb/stack.c | 8 +
gdb/symtab.c | 80 ++++++
gdb/symtab.h | 19 ++
.../gdb.dwarf2/dw2-function-trampolines.c | 80 ++++++
.../gdb.dwarf2/dw2-function-trampolines.exp | 245 ++++++++++++++++++
gdb/testsuite/gdb.fortran/func-trampoline.exp | 69 +++++
gdb/testsuite/gdb.fortran/func-trampoline.f90 | 39 +++
.../gdb.fortran/mixed-lang-stack.exp | 10 +-
16 files changed, 885 insertions(+), 14 deletions(-)
create mode 100644 gdb/testsuite/gdb.dwarf2/dw2-function-trampolines.c
create mode 100644 gdb/testsuite/gdb.dwarf2/dw2-function-trampolines.exp
create mode 100644 gdb/testsuite/gdb.fortran/func-trampoline.exp
create mode 100644 gdb/testsuite/gdb.fortran/func-trampoline.f90
--
2.34.1
Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v3 1/4] gdb, dwarf: add support for DW_AT_trampoline in DWARF reader
2023-07-10 22:56 [PATCH v3 0/4] GDB support for DW_AT_trampoline Abdul Basit Ijaz
@ 2023-07-10 22:56 ` Abdul Basit Ijaz
2023-07-27 9:09 ` Bruno Larsen
2023-07-10 22:56 ` [PATCH v3 2/4] gdb/symtab: add lookup for trampoline functions Abdul Basit Ijaz
` (2 subsequent siblings)
3 siblings, 1 reply; 12+ messages in thread
From: Abdul Basit Ijaz @ 2023-07-10 22:56 UTC (permalink / raw)
To: gdb-patches
Cc: abdul.b.ijaz, JiniSusan.George, tom, eliz, Nils-Christian Kempke
From: Nils-Christian Kempke <nils-christian.kempke@intel.com>
DW_AT_trampoline can be used to describe compiler generated functions
that serve some intermediary purpose on making a call to another
function. A compiler can emit this tag in order to help a debugger hide
the trampolines from a user.
The attribute is only applicable to DW_TAG_subroutine and
DW_TAG_inlined_subroutine tags. It contains information about the
trampoline target either as a reference to its DIE, as its address or
its name. DW_AT_trampoline can also be a flag indicating that the DIE
is a trampoline or not without specifying the target (e.g. if it is
unknown).
This patch adds support to GDB for reading the DW_AT_trampoline
attribute. It stores the attribute and its value in the type_specific
part of a GDB type. This patch is implemented in preparation of the
following patches, which will add a mechanism to hide DW_AT_trampoline
subroutines from the user.
2023-07-10 Nils-Christian Kempke <nils-christian.kempke@intel.com>
---
gdb/dwarf2/read.c | 45 +++++++++++++++++++-
gdb/gdbtypes.c | 35 +++++++++++++++-
gdb/gdbtypes.h | 103 +++++++++++++++++++++++++++++++++++++++++++---
3 files changed, 176 insertions(+), 7 deletions(-)
diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index 3508f2c29ee..1cfa8178d94 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -14657,6 +14657,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)
+ {
+ TYPE_FUNC_FLAGS (ftype) |= FUNC_TYPE_TRAMPOLINE;
+ TYPE_TRAMPOLINE_TARGET (ftype)
+ = (trampoline_target *) TYPE_ZALLOC (ftype,
+ sizeof (trampoline_target));
+
+ /* A DW_AT_trampoline can be either an address, a flag, a reference or a
+ string. */
+ if (attr->form_is_string ())
+ TYPE_TRAMPOLINE_TARGET (ftype)->set_target_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
+ {
+ unrelocated_addr target_addr_reloc = attr->as_address ();
+ CORE_ADDR target_addr
+ = cu->per_objfile->relocate (target_addr_reloc);
+ target_addr = gdbarch_adjust_dwarf2_addr (objfile->arch (),
+ target_addr);
+ TYPE_TRAMPOLINE_TARGET (ftype)->set_target_physaddr (target_addr);
+ }
+ }
+
/* Store the calling convention in the type if it's available in
the subroutine die. Otherwise set the calling convention to
the default value DW_CC_normal. */
@@ -14674,7 +14717,7 @@ read_subroutine_type (struct die_info *die, struct dwarf2_cu *cu)
if the DWARF producer set that information. */
attr = dwarf2_attr (die, DW_AT_noreturn, cu);
if (attr && attr->as_boolean ())
- TYPE_NO_RETURN (ftype) = 1;
+ TYPE_FUNC_FLAGS (ftype) |= FUNC_TYPE_NO_RETURN;
/* We need to add the subroutine type to the die immediately so
we don't infinitely recurse when dealing with parameters
diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c
index c5272979cb9..7355317f7db 100644
--- a/gdb/gdbtypes.c
+++ b/gdb/gdbtypes.c
@@ -5091,6 +5091,33 @@ print_fixed_point_type_info (struct type *type, int spaces)
type->fixed_point_scaling_factor ().str ().c_str ());
}
+/* Print the contents of the TYPE's self_trampoline_target, assuming that its
+ type-specific kind is TYPE_SPECIFIC_FUNC and is_trampoline is not 0. */
+static void
+print_trampoline_target_info (struct type *type, int spaces)
+{
+ switch (TYPE_TRAMPOLINE_TARGET (type)->target_kind ())
+ {
+ case TRAMPOLINE_TARGET_PHYSADDR:
+ gdb_printf ("%*starget physaddr: 0x%s\n", spaces + 2, "",
+ print_core_address (type->arch_owner (),
+ TYPE_TRAMPOLINE_TARGET (type)
+ ->target_physaddr ()));
+ break;
+ case TRAMPOLINE_TARGET_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. */
@@ -5417,6 +5444,10 @@ recursive_dump_type (struct type *type, int spaces)
gdb_printf ("%*scalling_convention %d\n", spaces, "",
TYPE_CALLING_CONVENTION (type));
/* tail_call_list is not printed. */
+ gdb_printf ("%*sfunc_type_flags %u\n", spaces, "",
+ (unsigned int) TYPE_FUNC_FLAGS (type));
+ if (TYPE_IS_TRAMPOLINE (type))
+ print_trampoline_target_info (type, spaces);
break;
case TYPE_SPECIFIC_SELF_TYPE:
@@ -5633,8 +5664,10 @@ copy_type_recursive (struct type *type, htab_t copied_types)
case TYPE_SPECIFIC_FUNC:
INIT_FUNC_SPECIFIC (new_type);
TYPE_CALLING_CONVENTION (new_type) = TYPE_CALLING_CONVENTION (type);
- TYPE_NO_RETURN (new_type) = TYPE_NO_RETURN (type);
+ TYPE_FUNC_FLAGS (new_type) |= TYPE_NO_RETURN (type);
TYPE_TAIL_CALL_LIST (new_type) = NULL;
+ TYPE_FUNC_FLAGS (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 aedaf53cd5d..a1789223cd5 100644
--- a/gdb/gdbtypes.h
+++ b/gdb/gdbtypes.h
@@ -106,6 +106,21 @@ enum type_instance_flag_value : unsigned
DEF_ENUM_FLAGS_TYPE (enum type_instance_flag_value, type_instance_flags);
+/* * Define flags for function types. */
+enum func_type_flag_value : unsigned
+{
+ /* * Flag indicates, whether this function normally returns to its
+ caller. It is set from the DW_AT_noreturn attribute if set on
+ the DW_TAG_subprogram. */
+ FUNC_TYPE_NO_RETURN = (1 << 0),
+
+ /* * Flag is used for functions marked with DW_AT_trampoline. These
+ are compiler generated wrappers that should be hidden from the user. */
+ FUNC_TYPE_TRAMPOLINE = (1 << 1)
+};
+
+DEF_ENUM_FLAGS_TYPE (enum func_type_flag_value, func_type_flags);
+
/* * Not textual. By default, GDB treats all single byte integers as
characters (or elements of strings) unless this flag is set. */
@@ -1730,11 +1745,9 @@ struct func_type
ENUM_BITFIELD (dwarf_calling_convention) calling_convention : 8;
- /* * Whether this function normally returns to its caller. It is
- set from the DW_AT_noreturn attribute if set on the
- DW_TAG_subprogram. */
+ /* * For storing function types defined in eunm func_type_flag_value. */
- unsigned int is_noreturn : 1;
+ func_type_flags flags;
/* * Only those DW_TAG_call_site's in this function that have
DW_AT_call_tail_call set are linked in this list. Function
@@ -1749,6 +1762,78 @@ struct func_type
contains the method. */
struct type *self_type;
+
+ struct trampoline_target *self_trampoline_target;
+ };
+
+/* The kind of location held by this call site target. */
+enum trampoline_target_kind
+ {
+ /* An address. */
+ TRAMPOLINE_TARGET_PHYSADDR,
+ /* A (mangled) name. */
+ TRAMPOLINE_TARGET_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;
};
/* The type-specific info for TYPE_CODE_FIXED_POINT types. */
@@ -1891,8 +1976,16 @@ extern void set_type_vptr_basetype (struct type *, struct type *);
#define TYPE_GNAT_SPECIFIC(thistype) TYPE_MAIN_TYPE(thistype)->type_specific.gnat_stuff
#define TYPE_DESCRIPTIVE_TYPE(thistype) TYPE_GNAT_SPECIFIC(thistype)->descriptive_type
#define TYPE_CALLING_CONVENTION(thistype) TYPE_MAIN_TYPE(thistype)->type_specific.func_stuff->calling_convention
-#define TYPE_NO_RETURN(thistype) TYPE_MAIN_TYPE(thistype)->type_specific.func_stuff->is_noreturn
+#define TYPE_FUNC_FLAGS(thistype) \
+ TYPE_MAIN_TYPE(thistype)->type_specific.func_stuff->flags
+#define TYPE_NO_RETURN(thistype) \
+ (TYPE_MAIN_TYPE(thistype)->type_specific.func_stuff->flags \
+ & FUNC_TYPE_NO_RETURN)
#define TYPE_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->flags \
+ & FUNC_TYPE_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.34.1
Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v3 2/4] gdb/symtab: add lookup for trampoline functions
2023-07-10 22:56 [PATCH v3 0/4] GDB support for DW_AT_trampoline Abdul Basit Ijaz
2023-07-10 22:56 ` [PATCH v3 1/4] gdb, dwarf: add support for DW_AT_trampoline in DWARF reader Abdul Basit Ijaz
@ 2023-07-10 22:56 ` Abdul Basit Ijaz
2023-07-10 22:56 ` [PATCH v3 3/4] gdb/infrun: handle stepping through functions with DW_AT_trampoline Abdul Basit Ijaz
2023-07-10 22:56 ` [PATCH v3 4/4] gdb: Skip trampoline frames in the stack for printing or finish command Abdul Basit Ijaz
3 siblings, 0 replies; 12+ messages in thread
From: Abdul Basit Ijaz @ 2023-07-10 22:56 UTC (permalink / raw)
To: gdb-patches
Cc: abdul.b.ijaz, JiniSusan.George, tom, eliz, Nils-Christian Kempke
From: Nils-Christian Kempke <nils-christian.kempke@intel.com>
In order to query information about the DW_AT_trampoline tag for
subroutines and inlined subroutines, two function were added to symtab.
First, a routine for querying whether the given pc belongs to a block
that is associated with a function (maybe inlined) marked
DW_AT_trampoline.
Second, a routine for querying a trampoline function's target.
Subroutines and inlined subroutines marked with DW_AT_trampoline usually
contain information about the target subroutine they are 'wrapping'/
passing control to.
2023-07-10 Nils-Christian Kempke <nils-christian.kempke@intel.com>
---
gdb/symtab.c | 68 ++++++++++++++++++++++++++++++++++++++++++++++++++++
gdb/symtab.h | 14 +++++++++++
2 files changed, 82 insertions(+)
diff --git a/gdb/symtab.c b/gdb/symtab.c
index 0117a2a59d7..8e6f81f8e1e 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -73,6 +73,7 @@
#include "gdbsupport/gdb_string_view.h"
#include "gdbsupport/pathstuff.h"
#include "gdbsupport/common-utils.h"
+#include "gdbsupport/symbol.h"
/* Forward declarations for local functions. */
@@ -4051,6 +4052,73 @@ find_function_alias_target (bound_minimal_symbol msymbol)
return NULL;
}
+/* See symtab.h. */
+
+bool
+in_trampoline_function (CORE_ADDR pc)
+{
+ /* Find the innermost function containing pc. This might be an inlined
+ function. */
+ symbol *sym = find_pc_sect_containing_function (pc,
+ find_pc_mapped_section (pc));
+ return sym != nullptr && TYPE_IS_TRAMPOLINE (sym->type ());
+}
+
+/* See symtab.h. */
+
+CORE_ADDR
+find_function_trampoline_target (CORE_ADDR pc)
+{
+ /* Find the innermost function containing pc. This might be an inlined
+ function. */
+ symbol *sym = find_pc_sect_containing_function (pc,
+ find_pc_mapped_section (pc));
+ CORE_ADDR target_address = 0;
+
+ if (sym != nullptr && TYPE_IS_TRAMPOLINE (sym->type ()))
+ {
+ trampoline_target *trampoline = TYPE_TRAMPOLINE_TARGET (sym->type ());
+
+ /* DW_AT_trampoline can be given as an address, name, or flag here (die
+ references have been resolved as names at this point. In the case
+ where DW_AT_trampoline contains a flag we do not know the target
+ address and return 0. */
+ if (trampoline->target_kind () == TRAMPOLINE_TARGET_PHYSNAME)
+ {
+ /* Handle both the mangled and demangled PHYSNAME. */
+ const char *physname = trampoline->target_physname ();
+
+ /* First, check whether there exists a symbol matching the
+ physname. If we cannot find one also check for minimal
+ symbols. */
+ const block *blk = block_for_pc (pc);
+ struct block_symbol bs = lookup_symbol (physname, blk, VAR_DOMAIN, 0);
+ if (bs.symbol != nullptr)
+ {
+ const struct block *block = bs.symbol->value_block ();
+ gdb_assert (block != nullptr);
+ target_address = block->start ();
+ }
+ else
+ {
+ if (find_minimal_symbol_address (physname, &target_address,
+ nullptr) != 0)
+ target_address = 0;
+ }
+ }
+ else if (trampoline->target_kind () == TRAMPOLINE_TARGET_PHYSADDR)
+ {
+ /* If the function symbol containing this trampoline target has
+ been relocated we assume the target_address also needs relocation.
+ If it has not been relocated the offset should be zero. */
+ target_address
+ = (trampoline->target_physaddr ()
+ + sym->objfile ()->section_offsets[sym->section_index ()]);
+ }
+ }
+
+ return target_address;
+}
\f
/* If P is of the form "operator[ \t]+..." where `...' is
some legitimate operator text, return a pointer to the
diff --git a/gdb/symtab.h b/gdb/symtab.h
index ee4729b14cd..fc2d82e6567 100644
--- a/gdb/symtab.h
+++ b/gdb/symtab.h
@@ -2312,6 +2312,20 @@ extern const struct gnu_ifunc_fns *gnu_ifunc_fns_p;
extern CORE_ADDR find_solib_trampoline_target (frame_info_ptr, CORE_ADDR);
+/* Return whether or not the current pc is within a block that belongs to a
+ function that is marked as a trampoline by the compiler. */
+
+extern bool in_trampoline_function (CORE_ADDR pc);
+
+/* Find the target of a trampoline function marked via the DW_AT_trampoline
+ attribute and return its address. Returns 0 if the pc is not contained
+ in a trampoline function (inlined or not). If DW_AT_trampoline
+ is given as a flag, the target is unknown and the function will still return
+ 0. One has to additionally query in_trampoline_function to cover this
+ case. */
+
+extern CORE_ADDR find_function_trampoline_target (CORE_ADDR pc);
+
struct symtab_and_line
{
/* The program space of this sal. */
--
2.34.1
Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v3 3/4] gdb/infrun: handle stepping through functions with DW_AT_trampoline
2023-07-10 22:56 [PATCH v3 0/4] GDB support for DW_AT_trampoline Abdul Basit Ijaz
2023-07-10 22:56 ` [PATCH v3 1/4] gdb, dwarf: add support for DW_AT_trampoline in DWARF reader Abdul Basit Ijaz
2023-07-10 22:56 ` [PATCH v3 2/4] gdb/symtab: add lookup for trampoline functions Abdul Basit Ijaz
@ 2023-07-10 22:56 ` Abdul Basit Ijaz
2023-07-11 11:21 ` Eli Zaretskii
2023-07-27 11:46 ` Bruno Larsen
2023-07-10 22:56 ` [PATCH v3 4/4] gdb: Skip trampoline frames in the stack for printing or finish command Abdul Basit Ijaz
3 siblings, 2 replies; 12+ messages in thread
From: Abdul Basit Ijaz @ 2023-07-10 22:56 UTC (permalink / raw)
To: gdb-patches
Cc: abdul.b.ijaz, JiniSusan.George, tom, eliz, Nils-Christian Kempke
From: Nils-Christian Kempke <nils-christian.kempke@intel.com>
This patch makes infrun continue stepping into and through trampoline
functions marked via the DW_AT_trampoline in DWARF. The attribute can
be emitted by the compiler for certain subroutines/inlined subroutines
that are compiler generated and should be hidden from a user.
Mainly, infrun is modified in 3 ways.
First, GDB will now attempt to step through trampoline functions.
Whenever we issued a step command that would make GDB step into a
function that is marked trampoline, GDB will try to step directly towards
the trampoline's 'target' instead and, e.g., not stop at the first
instruction of the trampoline.
The target can be specified by the compiler by the value of
DW_AT_trampoline if its form is either an address, a name, or a DIE
reference. DW_AT_trampoline is also allowed to be specified as a flag
(containing true or false), in which case the target is assumed to be
unknown. If GDB successfully finds a target, so if the value of
DW_AT_trampoline was not a flag and could be resolved successfully, GDB
steps directly towards the target and through the trampoline, hiding
the trampoline from the user. If GDB cannot, however deduce a target,
most likely because the DW_AT_trampoline was given as a flag or because
of broken debug info, it will instead continue inside execution in the
trampoline function until it reaches an instruction that is not
associated with a trampoline function, which is usually the target
function. It will then stop and give control back to the user.
It should be noted, that there might be the cases, where trampolines
call functions other than the target before the actual target call. If,
in such a situation, GDB fails to resolve the target, it would resume
execution until stepping into this other function call, and hand back
control to the user, without actually having reached the target. A
second step would have to be issued by the user to arrive a the target
(by resuming in the trampoline and then until leaving it a second time).
As this is a rather pathological case and no real instance of this is
known, I think the current behavior here is good enough and seems to be
the best GDB can do in such a situation.
Secondly, as trampoline functions normally do not have any real source
code correlation, it is likely that they mostly appear without line
info.
Normally, GDB would skip completely over a function call to a function
that has no source line information, so we would never get to the
aforementioned stepping through a trampoline and target resolution. To
remedy this, for debug info trampolines, GDB now attempts to step through
them regardless of them having source line information or not. So
issuing a step at a function call wrapped by a trampoline without source
line information will no longer skip the whole function call, but now
step through the trampoline and attempt to resolve the trampoline target
as described above (so usually, a single step at the call site will step
through the trampoline and towards the target, even if the trampoline had
not source line info).
Last, in all other cases when GDB is about to stop at a location that is
included in a trampoline region (e.g. after a step from the target back
into the trampoline) GDB will instead continue until the trampoline
region is left again and only then give control back to the user. This
change serves the purpose of allowing stepping back from a target call
through the trampoline without the user noticing the artificial function
call inbetween call site and target.
Together, these changes attempt to hide the trampoline function from the
user while stepping. Additionally, the skip-trampoline-functions option
has been introduced in infrun. It is set by default, and, when turned
off, GDB will return to its 'normal' stepping behavior and ignore any
possible DW_AT_trampoline.
As currently only ifx emits the DW_AT_trampoline tag, a test has been
added to gdb.dwarf2 that artificially creates a set of trampoline
functions.
2023-07-10 Nils-Christian Kempke <nils-christian.kempke@intel.com>
---
gdb/NEWS | 15 ++
gdb/doc/gdb.texinfo | 36 +++
gdb/infrun.c | 81 +++++-
gdb/infrun.h | 4 +
.../gdb.dwarf2/dw2-function-trampolines.c | 80 ++++++
.../gdb.dwarf2/dw2-function-trampolines.exp | 245 ++++++++++++++++++
6 files changed, 457 insertions(+), 4 deletions(-)
create mode 100644 gdb/testsuite/gdb.dwarf2/dw2-function-trampolines.c
create mode 100644 gdb/testsuite/gdb.dwarf2/dw2-function-trampolines.exp
diff --git a/gdb/NEWS b/gdb/NEWS
index b924834d3d7..8bca5540f58 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -571,6 +571,21 @@ platform.
* New commands
+set skip-trampoline-functions on|off
+show skip-trampoline-functions
+ This setting is 'on' by default. When 'on' it controls whether GDB's
+ stepping behavior will recognize function calls that have been marked as
+ trampolines in the debug info. It improves stepping behavior in that it
+ steps through trampoline code and hides it from the user. GDB can now step
+ through trampolines that are correctly marked as such in the compiler's
+ debug info. If the target of a trampoline is unknown, GDB will continue
+ until the trampoline section is left again and only then hand control back
+ to the user. GDB does this even if the trampoline has no associated line
+ info. If this is turned off, GDB will step into trampolines if there is
+ line table information for them or step over the trampoline calls if there
+ is no line table information. Currently, only DWARF trampolines are
+ supported.
+
maint set backtrace-on-fatal-signal on|off
maint show backtrace-on-fatal-signal
This setting is 'on' by default. When 'on' GDB will print a limited
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index b10c06ae91f..839c3e6a0f8 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -6391,6 +6391,42 @@ debug information. This is the default.
Show whether @value{GDBN} will stop in or step over functions without
source line debug information.
+@kindex set skip-trampoline-functions
+@cindex trampoline functions
+@cindex stepping through trampoline functions
+@item set skip-trampoline-functions
+@itemx set skip-trampoline-functions on
+When calling a function in any language, some compilers might generate
+so-called @dfn{trampoline functions}, which wrap the actual function call
+(the target of the trampoline). The compiler might mark such a trampoline in
+its debug information. Often, such trampolines do not have any source line
+information associated with them which will lead the @code{step} command to
+behave like a @code{next} and skip the function call completely.
+
+The default is @code{set skip-trampoline-functions on} and it will cause the
+@code{step} command to treat these trampolines differently. When issuing a
+@code{step} at the call site of a trampoline function if
+@code{skip-trampoline-functions} is set @value{GDBN} will attempt to determine
+the target of the trampoline and then step through the trampoline stopping at
+the target. If the target could not be found or was not given in the debug
+info, @value{GDBN} will simply continue execution until it leaves the
+trampoline code again, even if the trampoline has no line info associated with
+it. When returning from a target function call and stepping back into the
+trampoline, @value{GDBN} will again step through the trampoline towards the
+call site. Additionally, even if stopped in a trampoline function with source
+line information, issuing a @code{step} will prompt @value{GDBN} to resume
+execution until leaving the trampoline region again. The @code{stepi} command
+is not affected by the setting which is enabled by default. Currently, only
+DWARF trampolines marked via DW_AT_trampoline are supported by this.
+
+@item set skip-trampoline-functions off
+Causes the @code{step} command to completely ignore any trampoline information
+a compiler might have emitted in its debug info. Trampolines will be treated
+like any other function when stepping.
+
+@item show skip-trampoline-functions
+Show whether @value{GDBN} tries to skip trampolines or not.
+
@kindex finish
@kindex fin @r{(@code{finish})}
@item finish
diff --git a/gdb/infrun.c b/gdb/infrun.c
index 58da1cef29e..dfafe7cdc74 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -184,6 +184,12 @@ get_previous_thread ()
static bool detach_fork = true;
+/* If set (default) GDB will step through functions/inlined subroutines marked
+ DW_AT_trampoline by the compiler. If false, GDB will ignore the
+ attribute. */
+
+static bool skip_trampoline_functions = true;
+
bool debug_infrun = false;
static void
show_debug_infrun (struct ui_file *file, int from_tty,
@@ -7265,7 +7271,6 @@ process_event_stop_test (struct execution_control_state *ecs)
!= find_pc_function (ecs->event_thread->stop_pc ())))))
{
CORE_ADDR stop_pc = ecs->event_thread->stop_pc ();
- CORE_ADDR real_stop_pc;
infrun_debug_printf ("stepped into subroutine");
@@ -7337,8 +7342,41 @@ process_event_stop_test (struct execution_control_state *ecs)
calling routine and the real function), locate the real
function. That's what tells us (a) whether we want to step
into it at all, and (b) what prologue we want to run to the
- end of, if we do step into it. */
- real_stop_pc = skip_language_trampoline (frame, stop_pc);
+ end of, if we do step into it. For functions marked as
+ trampoline functions we try to find their target and step
+ towards it (if skip_trampoline_functions is not set to false by the
+ user). If no target can be determined we just step into the
+ trampoline and hand control back to the user. */
+ CORE_ADDR real_stop_pc = 0;
+ bool in_trampoline = skip_trampoline_functions
+ && in_trampoline_function (stop_pc);
+
+ if (in_trampoline)
+ {
+ real_stop_pc = find_function_trampoline_target (stop_pc);
+
+ for (int i = 0; i < MAX_TRAMPOLINE_CHAIN_SIZE
+ && in_trampoline_function (real_stop_pc); ++i)
+ {
+ real_stop_pc = find_function_trampoline_target (real_stop_pc);
+ /* Exit if find_function_trampoline_target failed to find the
+ trampoline target. Do not try to resolve the trampolines
+ in this case. */
+ if (real_stop_pc == 0x0)
+ break;
+ }
+
+ /* If we failed to find a target we will just single step in the
+ hope of leaving the trampoline again soon. */
+ if (real_stop_pc == 0x0)
+ {
+ keep_going (ecs);
+ return;
+ }
+ }
+
+ if (real_stop_pc == 0)
+ real_stop_pc = skip_language_trampoline (frame, stop_pc);
if (real_stop_pc == 0)
real_stop_pc = gdbarch_skip_trampoline_code (gdbarch, frame, stop_pc);
if (real_stop_pc != 0)
@@ -7359,6 +7397,8 @@ process_event_stop_test (struct execution_control_state *ecs)
/* If we have line number information for the function we are
thinking of stepping into and the function isn't on the skip
list, step into it.
+ If we are about to step into a function marked trampoline with no
+ line number information, we still want to enter it here.
If there are several symtabs at that PC (e.g. with include
files), just want to know whether *any* of them have line
@@ -7367,7 +7407,9 @@ process_event_stop_test (struct execution_control_state *ecs)
struct symtab_and_line tmp_sal;
tmp_sal = find_pc_line (ecs->stop_func_start, 0);
- if (tmp_sal.line != 0
+ if ((tmp_sal.line != 0
+ || (skip_trampoline_functions
+ && in_trampoline_function (ecs->stop_func_start)))
&& !function_name_is_marked_for_skip (ecs->stop_func_name,
tmp_sal)
&& !inline_frame_is_marked_for_skip (true, ecs->event_thread))
@@ -7520,6 +7562,19 @@ process_event_stop_test (struct execution_control_state *ecs)
return;
}
+ /* If we ended up in a function trampoline without stepping into a new
+ function we are either in some inlined trampoline or returning through a
+ trampoline function. In either case we continue to single step until we
+ are out of the trampoline code again. This check has to be done before
+ stop_pc_sal.line == 0 below, as trampolines usually don't have source
+ line information associated with them. */
+ if (skip_trampoline_functions && in_trampoline_function (stop_pc_sal.pc))
+ {
+ infrun_debug_printf ("stepped into trampoline code");
+ keep_going (ecs);
+ return;
+ }
+
if (stop_pc_sal.line == 0)
{
/* We have no line number information. That means to stop
@@ -9731,6 +9786,14 @@ show_exec_direction_func (struct ui_file *out, int from_tty,
}
}
+static void
+show_skip_trampoline_functions (ui_file *file, int from_tty,
+ cmd_list_element *c,
+ const char *value)
+{
+ gdb_printf (file, _("Skipping trampoline functions is %s.\n"), value);
+}
+
static void
show_schedule_multiple (struct ui_file *file, int from_tty,
struct cmd_list_element *c, const char *value)
@@ -10076,6 +10139,16 @@ Options are 'forward' or 'reverse'."),
set_exec_direction_func, show_exec_direction_func,
&setlist, &showlist);
+ add_setshow_boolean_cmd ("skip-trampoline-functions", class_run,
+ &skip_trampoline_functions, _("\
+Set whether gdb attempts to hide trampolines marked in the debug info."), _("\
+Show whether gdb attempts to hide trampolines marked in the debug info."), _("\
+If on, while stepping gdb will skip through functions and inlined functions\n\
+marked as trampolines by the compiler. If off, gdb will ignore such function\n\
+trampolines."),
+ nullptr, show_skip_trampoline_functions, &setlist,
+ &showlist);
+
/* Set/show detach-on-fork: user-settable mode. */
add_setshow_boolean_cmd ("detach-on-fork", class_run, &detach_fork, _("\
diff --git a/gdb/infrun.h b/gdb/infrun.h
index a343d27f72d..d4a6b7892b5 100644
--- a/gdb/infrun.h
+++ b/gdb/infrun.h
@@ -76,6 +76,10 @@ infrun_debug_show_threads (const char *title, ThreadRange threads)
}
+/* Maximum size of trampoline chain to process while resolving
+ trampolines. */
+#define MAX_TRAMPOLINE_CHAIN_SIZE 10
+
/* Nonzero if we want to give control to the user when we're notified
of shared library events by the dynamic linker. */
extern int stop_on_solib_events;
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-function-trampolines.c b/gdb/testsuite/gdb.dwarf2/dw2-function-trampolines.c
new file mode 100644
index 00000000000..1f0eb2e0efa
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/dw2-function-trampolines.c
@@ -0,0 +1,80 @@
+/* Copyright 2023 Free Software Foundation, Inc.
+
+ This program is free software; you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation; either version 3 of the License, or
+ (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program. If not, see <http://www.gnu.org/licenses/>. */
+
+/* This test relies on inlined_trampoline being inlined into main and the other
+ functions not. All functions except target will be marked via
+ DW_AT_trampoline in the debug info and we'll check whether one can step
+ through the trampolines towards target. */
+volatile int global_var;
+
+int __attribute__ ((noinline))
+target () /* target decl line */
+{ /* target prologue */
+ asm ("target_label: .globl target_label");
+ ++global_var; /* target add */
+ asm ("target_label2: .globl target_label2");
+ return 9 + 10; /* target return */
+} /* target end */
+
+int __attribute__ ((noinline))
+trampoline ()
+{ /* trampoline prologue */
+ asm ("trampoline_label: .globl trampoline_label");
+ ++global_var;
+ return target (); /* trampoline target call */
+} /* trampoline end */
+
+static inline int __attribute__ ((always_inline))
+inlined_trampoline ()
+{ /* inlined_trampoline prologue */
+ asm ("inlined_trampoline_label: .globl inlined_trampoline_label");
+ ++global_var; /* inlined_trampoline add */
+ asm ("inlined_trampoline_label2: .globl inlined_trampoline_label2");
+ return target (); /* inlined_trampoline target call */
+} /* inlined_trampoline end */
+
+int __attribute__ ((noinline))
+chained_trampoline ()
+{ /* chained_trampoline prologue */
+ asm ("chained_trampoline_label: .globl chained_trampoline_label");
+ ++global_var;
+ return trampoline (); /* chained_trampoline trampoline call */
+} /* chained_trampoline end */
+
+int __attribute__ ((noinline))
+doubly_chained_trampoline ()
+{ /* doubly_chained_trampoline prologue */
+ asm ("doubly_chained_trampoline_label: .globl doubly_chained_trampoline_label");
+ ++global_var;
+ return chained_trampoline (); /* doubly_chained_trampoline chained_trampoline call */
+} /* doubly_chained_trampoline end */
+
+int
+main () /* main decl line */
+{ /* main prologue */
+ int ans;
+ asm ("main_label: .globl main_label");
+ global_var = 0; /* main set global_var */
+ asm ("main_label2: .globl main_label2");
+ ans = inlined_trampoline (); /* main call inlined_trampoline */
+ asm ("main_label3: .globl main_label3");
+ ans = trampoline (); /* main call trampoline */
+ asm ("main_label4: .globl main_label4");
+ ans = chained_trampoline (); /* main call chained_trampoline */
+ asm ("main_label5: .globl main_label5");
+ ans = doubly_chained_trampoline (); /* main call doubly_chained_trampoline */
+ asm ("main_label6: .globl main_label6");
+ return ans; /* main call return */
+} /* main end */
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-function-trampolines.exp b/gdb/testsuite/gdb.dwarf2/dw2-function-trampolines.exp
new file mode 100644
index 00000000000..67eec2b5540
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/dw2-function-trampolines.exp
@@ -0,0 +1,245 @@
+# Copyright 2023 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program. If not, see <http://www.gnu.org/licenses/>.
+
+# This test checks GDB's handling of functions/inlined functions marked
+# DW_AT_trampoline by the compiler. A function marked as trampoline should
+# generally be hidden from the user. We check whether we can step through
+# trampolines. Every trampoline is defined using a different type for its
+# target: a string, an address, a DIE reference or a flag.
+# Setting skip-trampoline-functions to false inside GDB should make it return
+# to its 'normal' behavior, ignore the DW_AT_trampoline, and skip all of the
+# non-inlined trampoline calls (as their DIEs don't have any source
+# information).
+
+load_lib dwarf.exp
+
+# This test can only be run on targets which support DWARF-2 and use gas.
+if {![dwarf2_support]} {
+ return 0
+}
+
+standard_testfile .c .S
+
+set asm_file [standard_output_file $srcfile2]
+Dwarf::assemble $asm_file {
+ global srcdir subdir srcfile
+ declare_labels lines_label trampoline_label
+
+ get_func_info target
+ get_func_info trampoline
+ get_func_info chained_trampoline
+ get_func_info doubly_chained_trampoline
+ get_func_info main
+
+ set target_decl_line [gdb_get_line_number "target decl line"]
+ set main_decl_line [gdb_get_line_number "main decl line"]
+ set main_call_inlined_trampoline_line [gdb_get_line_number "main call inlined_trampoline"]
+
+ cu {} {
+ compile_unit {
+ {language @DW_LANG_C}
+ {name dw2-function-trampolines.c}
+ {low_pc 0 addr}
+ {stmt_list ${lines_label} DW_FORM_sec_offset}
+ } {
+ subprogram {
+ {name target}
+ {low_pc $target_start addr}
+ {high_pc "$target_start + $target_len" addr}
+ {decl_file 1 data1}
+ {decl_line $target_decl_line data1}
+ }
+ # The 'trampoline' subprogram declares its target by name.
+ trampoline_label: subprogram {
+ {name trampoline}
+ {low_pc $trampoline_start addr}
+ {high_pc "$trampoline_start + $trampoline_len" addr}
+ {trampoline target string}
+ }
+ # The 'chained_trampoline' subprogram declares its target as die
+ # reference.
+ subprogram {
+ {name chained_trampoline}
+ {low_pc $chained_trampoline_start addr}
+ {high_pc "$chained_trampoline_start + $chained_trampoline_len" addr}
+ {trampoline %$trampoline_label}
+ }
+ # The 'doubly_chained_trampoline' subprogram declares no target.
+ # Its DW_AT_trampoline is a flag set to true.
+ subprogram {
+ {name doubly_chained_trampoline}
+ {low_pc $doubly_chained_trampoline_start addr}
+ {high_pc "$doubly_chained_trampoline_start + $doubly_chained_trampoline_len" addr}
+ {trampoline 1 flag}
+ }
+ subprogram {
+ {external 1 flag}
+ {name main}
+ {main_subprogram 1 flag}
+ {low_pc $main_start addr}
+ {high_pc "$main_start + $main_len" addr}
+ {decl_file 1 data1}
+ {decl_line $main_decl_line data1}
+ } {
+ # The 'inlined_trampoline' subroutine declares its target as
+ # an address.
+ inlined_subroutine {
+ {name inlined_trampoline}
+ {low_pc main_label2 addr}
+ {high_pc main_label3 addr}
+ {trampoline $target_start addr}
+ {call_file 1 data1}
+ {call_line $main_call_inlined_trampoline_line data1}
+ }
+ }
+ }
+ }
+
+ lines {version 2} lines_label {
+ include_dir "${srcdir}/${subdir}"
+ file_name "$srcfile" 1
+
+ program {
+ DW_LNE_set_address $main_start
+ line [gdb_get_line_number "main set global_var"]
+ DW_LNS_copy
+ DW_LNE_set_address main_label
+ line [gdb_get_line_number "main set global_var"]
+ DW_LNS_copy
+ DW_LNE_set_address main_label2
+ line [gdb_get_line_number "main call inlined_trampoline"]
+ DW_LNS_copy
+ DW_LNE_set_address inlined_trampoline_label
+ line [gdb_get_line_number "inlined_trampoline add"]
+ DW_LNS_copy
+ DW_LNE_set_address inlined_trampoline_label2
+ line [gdb_get_line_number "inlined_trampoline target call"]
+ DW_LNS_copy
+ DW_LNE_set_address main_label3
+ line [gdb_get_line_number "main call trampoline"]
+ DW_LNS_copy
+ DW_LNE_set_address main_label4
+ line [gdb_get_line_number "main call chained_trampoline"]
+ DW_LNS_copy
+ DW_LNE_set_address main_label5
+ line [gdb_get_line_number "main call doubly_chained_trampoline"]
+ DW_LNS_copy
+ DW_LNE_set_address main_label6
+ line [gdb_get_line_number "main call return"]
+ DW_LNS_copy
+ DW_LNE_set_address $main_end
+ DW_LNE_end_sequence
+
+ DW_LNE_set_address $target_start
+ line [gdb_get_line_number "target prologue"]
+ DW_LNS_negate_stmt
+ DW_LNS_copy
+ DW_LNE_set_address target_label
+ line [gdb_get_line_number "target add"]
+ DW_LNS_negate_stmt
+ DW_LNS_copy
+ DW_LNE_set_address target_label2
+ line [gdb_get_line_number "target return"]
+ DW_LNS_copy
+ DW_LNE_set_address $target_end
+ DW_LNE_end_sequence
+ }
+ }
+}
+
+if {[prepare_for_testing "failed to prepare" ${testfile} \
+ [list $srcfile $asm_file] {nodebug additional_flags=-O0}]} {
+ return -1
+}
+
+set target_first_line_pattern ".*target add.*"
+set target_second_line_pattern ".*target return.*"
+
+if ![runto_main] {
+ return -1
+}
+
+gdb_test "show skip-trampoline-functions" \
+ "Skipping trampoline functions is on\." \
+ "check skip-trampoline-functions is enabled"
+
+with_test_prefix "with trampoline handling" {
+ foreach {trampoline return_line} { "inlined_trampoline" "trampoline" \
+ "trampoline" "chained_trampoline" \
+ "chained_trampoline" "doubly_chained_trampoline" } {
+
+ gdb_test "s" "$target_first_line_pattern" "step through $trampoline"
+ gdb_test "s" "$target_second_line_pattern" \
+ "step target second line from $trampoline"
+ gdb_test "s" ".*main call $return_line.*" \
+ "step back through $trampoline"
+ }
+
+ # The doubly_chained_trampoline has only been marked as trampoline but no
+ # target was given. In this case GDB steps into the trampoline and then
+ # continues until the trampoline section is left again.
+
+ # When compiled with gcc 7.5 (and possibly others) on a 32 bit system, the
+ # trampoline function contains a call to __x86.get_pc_thunk.ax before the
+ # actual target call. So, we end up in __x86.get_pc_thunk.ax. Issuing a
+ # second step command will return from the function call back into the
+ # trampoline and go on inside the trampoline towards the actual target call.
+ # On other targets we step directly towards the target call.
+ gdb_test_multiple "s" "step through double_chained_trampoline" {
+ -re -wrap "$target_first_line_pattern" {
+ pass $gdb_test_name
+ }
+ -re -wrap ".*__x86.get_pc_thunk.ax.*" {
+ gdb_test "s" "$target_first_line_pattern" \
+ "step through double_chained_trampoline 2nd try"
+ }
+ }
+ gdb_test "s" "$target_second_line_pattern" \
+ "step target second line fromdoubly_chained_trampoline"
+ gdb_test "s" ".*main call return.*" \
+ "step back through doubly_chained_trampoline"
+}
+
+clean_restart ${testfile}
+
+if ![runto_main] {
+ return -1
+}
+
+gdb_test_no_output "set skip-trampoline-functions off" \
+ "disable trampoline handling"
+gdb_test "show skip-trampoline-functions" \
+ "Skipping trampoline functions is off." \
+ "check skip-trampoline-functions is disabled"
+
+with_test_prefix "without trampoline handling" {
+ gdb_test "s" ".*main call inlined_trampoline.*"
+ gdb_test "s" ".*inlined_trampoline add.*" \
+ "step into inlined_trampoline with skip-trampoline off"
+ gdb_test "s" ".*inlined_trampoline target call.*" \
+ "step in inlined_trampoline with skip-trampoline off"
+ gdb_test "s" "$target_first_line_pattern" \
+ "step into target with skip-trampoline off"
+ gdb_test "s" "$target_second_line_pattern" \
+ "step second line in target with skip-trampoline off"
+ gdb_test "s" ".*main call trampoline.*" \
+ "step brack from target with skip-trampoline off"
+ gdb_test "s" ".*main call chained_trampoline.*" \
+ "skip trampoline call with no line info"
+ gdb_test "s" ".*main call doubly_chained_trampoline.*" \
+ "skip chained_trampoline call with no line info"
+ gdb_test "s" ".*main call return.*" \
+ "skip doubly_chained_trampoline call with no line info"
+}
--
2.34.1
Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v3 4/4] gdb: Skip trampoline frames in the stack for printing or finish command.
2023-07-10 22:56 [PATCH v3 0/4] GDB support for DW_AT_trampoline Abdul Basit Ijaz
` (2 preceding siblings ...)
2023-07-10 22:56 ` [PATCH v3 3/4] gdb/infrun: handle stepping through functions with DW_AT_trampoline Abdul Basit Ijaz
@ 2023-07-10 22:56 ` Abdul Basit Ijaz
2023-07-11 11:23 ` Eli Zaretskii
3 siblings, 1 reply; 12+ messages in thread
From: Abdul Basit Ijaz @ 2023-07-10 22:56 UTC (permalink / raw)
To: gdb-patches; +Cc: abdul.b.ijaz, JiniSusan.George, tom, eliz
Before the change, GDB prints the frames indicated by the compiler with DIE
"DW_AT_trampoline" in the backtrace and finish command, but for better user
experience, all such frames can be hidden from the user. So, after this
change, now such frames are not printed any more in the backtrace command
and also the 'finish' command skips the trampoline calls.
So far, this DIE is added to DWARF only by the IFX compiler, so
gdb.fortran/mixed-lang-stack test used to fail for this compiler because of
these extra trampoline frames in the backtrace. After the commit, those
trampoline frames are filtered so test is updated accordingly to handle the
frame level of the filtered frames.
Backtrace output in this test before the change:
bt -frame-arguments all
'''
'''
(gdb) FAIL: gdb.fortran/mixed-lang-stack.exp: lang=auto: bt -frame-arguments all
Now, after the change:
bt -frame-arguments all
'''
'''
(gdb) PASS: gdb.fortran/mixed-lang-stack.exp: lang=auto: bt -frame-arguments all
Below is the example where IFX emits DW_at_trampoline for functions first
and second trampoline calls as following:
F90 reproducer info:
function second (x, y) result(z)
integer, intent(in) :: x, y
integer :: z
z = x * y ! breakpt-backtrace
end function second
function first (num1, num2) result(total)
integer, intent(in) :: num1, num2
integer :: total
total = second (num1 + 4, num2 * 3) ! first-breakpt
total = total + 30
end function first
Related Dwarf:
0x0000013f: DW_TAG_subprogram
DW_AT_low_pc (0x0000000000404350)
DW_AT_high_pc (0x000000000040435f)
DW_AT_frame_base (DW_OP_reg6 RBP)
DW_AT_linkage_name ("second_.t74p.t75p")
DW_AT_name ("second_.t74p.t75p")
DW_AT_trampoline ("second_")
0x0000015a: DW_TAG_subprogram
DW_AT_low_pc (0x00000000004044a0)
DW_AT_high_pc (0x00000000004044af)
DW_AT_frame_base (DW_OP_reg6 RBP)
DW_AT_linkage_name ("first_.t104p.t105p")
DW_AT_name ("first_.t104p.t105p")
DW_AT_trampoline ("first_")
Before this change at line with comment 'breakpt-backtrace':
(gdb) backtrace 3
(gdb) finish
Run till exit from #0 second (x=20, y=9) at test.f90:4
second_.t74p.t75p () at test.f90:10
10 total = second (num1 + 4, num2 * 3) ! first-breakpt
Value returned is $1 = 180
After the change:
(gdb) backtrace 3
(gdb) finish
Run till exit from #0 second (x=20, y=9) at test.f90:4
0x0000000000404333 in first (num1=16, num2=3) at test.f90:10
10 total = second (num1 + 4, num2 * 3) ! first-breakpt
Value returned is $1 = 180
New test func-trampoline is also added to test this in both backtrace and
finish commands for Intel(R) Fortran Compiler.
2023-07-10 Abdul Basit Ijaz <abdul.b.ijaz@intel.com>
---
gdb/doc/gdb.texinfo | 35 ++++++----
gdb/infcmd.c | 13 ++++
gdb/infrun.c | 8 +--
gdb/infrun.h | 6 ++
gdb/stack.c | 8 +++
gdb/symtab.c | 12 ++++
gdb/symtab.h | 5 ++
gdb/testsuite/gdb.fortran/func-trampoline.exp | 69 +++++++++++++++++++
gdb/testsuite/gdb.fortran/func-trampoline.f90 | 39 +++++++++++
.../gdb.fortran/mixed-lang-stack.exp | 10 ++-
10 files changed, 186 insertions(+), 19 deletions(-)
create mode 100644 gdb/testsuite/gdb.fortran/func-trampoline.exp
create mode 100644 gdb/testsuite/gdb.fortran/func-trampoline.f90
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 839c3e6a0f8..a8e4f47ef07 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -6401,23 +6401,34 @@ so-called @dfn{trampoline functions}, which wrap the actual function call
(the target of the trampoline). The compiler might mark such a trampoline in
its debug information. Often, such trampolines do not have any source line
information associated with them which will lead the @code{step} command to
-behave like a @code{next} and skip the function call completely.
+behave like a @code{next} and skip the function call completely. Similarly,
+the @code{finish} command will return to some trampoline frame entry instead
+of returning to the call site.
The default is @code{set skip-trampoline-functions on} and it will cause the
-@code{step} command to treat these trampolines differently. When issuing a
-@code{step} at the call site of a trampoline function if
-@code{skip-trampoline-functions} is set @value{GDBN} will attempt to determine
+@code{step} and the @code{finish} command to treat these trampolines
+differently.
+
+When issuing a @code{step} at the call site of a trampoline function, if
+@code{skip-trampoline-functions} is set, @value{GDBN} will attempt to determine
the target of the trampoline and then step through the trampoline stopping at
the target. If the target could not be found or was not given in the debug
info, @value{GDBN} will simply continue execution until it leaves the
-trampoline code again, even if the trampoline has no line info associated with
-it. When returning from a target function call and stepping back into the
-trampoline, @value{GDBN} will again step through the trampoline towards the
-call site. Additionally, even if stopped in a trampoline function with source
-line information, issuing a @code{step} will prompt @value{GDBN} to resume
-execution until leaving the trampoline region again. The @code{stepi} command
-is not affected by the setting which is enabled by default. Currently, only
-DWARF trampolines marked via DW_AT_trampoline are supported by this.
+trampoline code again, even if the trampoline has no line info associated
+with it. When returning from a target function call and stepping back into
+the trampoline, @value{GDBN} will again step through the trampoline towards
+the call site. Additionally, even if stopped in a trampoline function with
+source line information, issuing a @code{step} will prompt @value{GDBN} to
+resume execution until leaving the trampoline region again. The @code{stepi}
+command is not affected by the setting which is enabled by default.
+
+When issuing a @code{finish} command at the target of a trampoline function, if
+@code{skip-trampoline-functions} is set, @value{GDBN} will ignore all the
+trampoline frames and will reach the first non-trampoline frame to return to
+the call site of the current function.
+
+Currently, only DWARF trampolines marked via DW_AT_trampoline are supported by
+this.
@item set skip-trampoline-functions off
Causes the @code{step} command to completely ignore any trampoline information
diff --git a/gdb/infcmd.c b/gdb/infcmd.c
index 15702f84894..e64ac5dfe43 100644
--- a/gdb/infcmd.c
+++ b/gdb/infcmd.c
@@ -1853,6 +1853,19 @@ finish_command (const char *arg, int from_tty)
if (frame == 0)
error (_("\"finish\" not meaningful in the outermost frame."));
+ if (skip_trampoline_functions)
+ {
+ for (int i = 0; i < MAX_TRAMPOLINE_CHAIN_SIZE
+ && (frame != nullptr)
+ && in_trampoline_frame (frame); ++i)
+ frame = get_prev_frame (frame);
+
+ if (frame == nullptr)
+ error (_("\"finish\" not meaningful in the outermost non-trampoline \
+frame. Consider running \"set skip-trampoline-functions off\", to stop in \
+trampoline frames for the \"finish\" command."));
+ }
+
clear_proceed_status (0);
tp = inferior_thread ();
diff --git a/gdb/infrun.c b/gdb/infrun.c
index dfafe7cdc74..edbf393214d 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -188,7 +188,7 @@ static bool detach_fork = true;
DW_AT_trampoline by the compiler. If false, GDB will ignore the
attribute. */
-static bool skip_trampoline_functions = true;
+bool skip_trampoline_functions = true;
bool debug_infrun = false;
static void
@@ -10143,9 +10143,9 @@ Options are 'forward' or 'reverse'."),
&skip_trampoline_functions, _("\
Set whether gdb attempts to hide trampolines marked in the debug info."), _("\
Show whether gdb attempts to hide trampolines marked in the debug info."), _("\
-If on, while stepping gdb will skip through functions and inlined functions\n\
-marked as trampolines by the compiler. If off, gdb will ignore such function\n\
-trampolines."),
+If on, the step and finish commands will skip through functions and inlined\n\
+functions marked as trampolines by the compiler. If off, gdb will ignore\n\
+such function trampolines."),
nullptr, show_skip_trampoline_functions, &setlist,
&showlist);
diff --git a/gdb/infrun.h b/gdb/infrun.h
index d4a6b7892b5..feb5f4414ec 100644
--- a/gdb/infrun.h
+++ b/gdb/infrun.h
@@ -106,6 +106,12 @@ extern bool non_stop;
starting an inferior. */
extern bool disable_randomization;
+/* If set (default) GDB will step through functions/inlined subroutines marked
+ DW_AT_trampoline by the compiler. If false, GDB will ignore the
+ attribute. */
+
+extern bool skip_trampoline_functions;
+
/* Returns a unique identifier for the current stop. This can be used
to tell whether a command has proceeded the inferior past the
current location. */
diff --git a/gdb/stack.c b/gdb/stack.c
index 002bf580634..915ac398e09 100644
--- a/gdb/stack.c
+++ b/gdb/stack.c
@@ -2054,6 +2054,14 @@ backtrace_command_1 (const frame_print_options &fp_opts,
{
QUIT;
+ if (in_trampoline_frame (fi))
+ {
+ /* Trampoline frames are not printed so they are not counted in
+ the backtrace limit. */
+ count++;
+ continue;
+ }
+
/* Don't use print_stack_frame; if an error() occurs it probably
means further attempts to backtrace would fail (on the other
hand, perhaps the code does or could be fixed to make sure
diff --git a/gdb/symtab.c b/gdb/symtab.c
index 8e6f81f8e1e..df64469d7ac 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -4066,6 +4066,18 @@ in_trampoline_function (CORE_ADDR pc)
/* See symtab.h. */
+bool
+in_trampoline_frame (frame_info_ptr fi)
+{
+ CORE_ADDR pc;
+ if (get_frame_pc_if_available (fi, &pc))
+ return in_trampoline_function (pc);
+
+ return false;
+}
+
+/* See symtab.h. */
+
CORE_ADDR
find_function_trampoline_target (CORE_ADDR pc)
{
diff --git a/gdb/symtab.h b/gdb/symtab.h
index fc2d82e6567..f02b3840f00 100644
--- a/gdb/symtab.h
+++ b/gdb/symtab.h
@@ -2317,6 +2317,11 @@ extern CORE_ADDR find_solib_trampoline_target (frame_info_ptr, CORE_ADDR);
extern bool in_trampoline_function (CORE_ADDR pc);
+/* Return whether or not the pc of current frame is within a block that belongs
+ to a function that is marked as a trampoline by the compiler. */
+
+extern bool in_trampoline_frame (frame_info_ptr);
+
/* Find the target of a trampoline function marked via the DW_AT_trampoline
attribute and return its address. Returns 0 if the pc is not contained
in a trampoline function (inlined or not). If DW_AT_trampoline
diff --git a/gdb/testsuite/gdb.fortran/func-trampoline.exp b/gdb/testsuite/gdb.fortran/func-trampoline.exp
new file mode 100644
index 00000000000..3bc4d1c0714
--- /dev/null
+++ b/gdb/testsuite/gdb.fortran/func-trampoline.exp
@@ -0,0 +1,69 @@
+# Copyright 2023 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program. If not, see <http://www.gnu.org/licenses/> .
+
+# Test finish and backtrace commands for functions with trampoline
+# calls. Also checks if trampoline frames are filtered while printing
+# stack and finish command returns to real function.
+
+require allow_fortran_tests
+
+if {![test_compiler_info {ifx-*} f90]} {
+ untested "Test is only applicable for IFX where\
+ compiler emits trampoline DIE in Dwarf"
+ return -1
+}
+
+standard_testfile ".f90"
+load_lib fortran.exp
+
+if {[prepare_for_testing "failed to prepare" ${testfile} ${srcfile} \
+ {debug f90}]} {
+ return -1
+}
+
+if {![fortran_runto_main]} {
+ return -1
+}
+
+set inner_loc [gdb_get_line_number "second-breakpt"]
+set middle_loc [gdb_get_line_number "first-breakpt"]
+set outer_loc [gdb_get_line_number "main-outer-loc"]
+set fill "\[^\r\n\]*"
+
+set inner_desc "second \\(x=20, y=9\\) at ${fill}$srcfile:$inner_loc"
+set middle_desc "first \\(num1=16, num2=3\\) at ${fill}$srcfile:$middle_loc"
+set outer_desc ".* at .*$srcfile:$outer_loc"
+
+# Set breakpoint inside the innermost function 'second'.
+gdb_breakpoint "$srcfile:$inner_loc"
+gdb_continue_to_breakpoint "innermost-body" ".*$srcfile:$inner_loc.*"
+
+# Limit the backtrace to 3 frames and ensure both frames for first
+# and second function are shown only and trampoline frames are filtered.
+gdb_test "backtrace 3" [multi_line \
+ "#$decimal.* $inner_desc" \
+ "#$decimal.* $middle_desc" \
+ "#$decimal.* $outer_desc.*"] \
+"backtrace, test trampoline frames are filtered"
+
+# Finish the function calls. Normally we expect to see a "Value
+# returned is ..." line.
+set value_returned "(\r\nValue returned is $valnum_re = 180)"
+
+gdb_test "finish" [multi_line \
+ "Run till exit from #0 $fill second \\(x=20, y=9\\) $fill" \
+ "${fill}first \\(num1=16, num2=3\\)${fill}" \
+ "${fill}${value_returned}"] \
+ "finish the innermost frame"
diff --git a/gdb/testsuite/gdb.fortran/func-trampoline.f90 b/gdb/testsuite/gdb.fortran/func-trampoline.f90
new file mode 100644
index 00000000000..652733b44fd
--- /dev/null
+++ b/gdb/testsuite/gdb.fortran/func-trampoline.f90
@@ -0,0 +1,39 @@
+! Copyright 2023 Free Software Foundation, Inc.
+!
+! This program is free software; you can redistribute it and/or modify
+! it under the terms of the GNU General Public License as published by
+! the Free Software Foundation; either version 3 of the License, or
+! (at your option) any later version.
+!
+! This program is distributed in the hope that it will be useful,
+! but WITHOUT ANY WARRANTY; without even the implied warranty of
+! MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+! GNU General Public License for more details.
+!
+! You should have received a copy of the GNU General Public License
+! along with this program. If not, see <http://www.gnu.org/licenses/> .
+
+! Source code for func-trampoline.exp.
+
+integer(kind=4) function second(x, y)
+ integer(kind=4), intent(in) :: x
+ integer(kind=4), intent(in) :: y
+
+ second = x * y ! second-breakpt
+end function
+
+integer(kind=4) function first(num1, num2)
+ integer(kind=4), intent(in) :: num1
+ integer(kind=4), intent(in) :: num2
+
+ first = second (num1 + 4, num2 * 3) ! first-breakpt
+end function
+
+program func_trampoline
+ integer(kind=4) :: total
+
+ total = first(16, 3) ! main-outer-loc
+
+ write(*,*) "Result is ", total, "\n"
+ ! Expected: 180
+end program func_trampoline
diff --git a/gdb/testsuite/gdb.fortran/mixed-lang-stack.exp b/gdb/testsuite/gdb.fortran/mixed-lang-stack.exp
index 5fc36f7b95b..71a171edcf0 100644
--- a/gdb/testsuite/gdb.fortran/mixed-lang-stack.exp
+++ b/gdb/testsuite/gdb.fortran/mixed-lang-stack.exp
@@ -41,7 +41,7 @@ set have_index [exec_has_index_section $binfile]
# value to pass to GDB's 'set language ...' command.
proc run_tests { lang } {
with_test_prefix "lang=${lang}" {
- global binfile hex have_index
+ global binfile hex have_index decimal
clean_restart ${binfile}
@@ -63,6 +63,10 @@ proc run_tests { lang } {
set e_arg "\['\"\]abcdef\['\"\]"
set 1b_args "\[^\r\n\]+$e_arg\[^\r\n\]+"
set 1g_args "obj=\[^\r\n\]+"
+ # Generic decimal number is checked in regex for Frame #8 and #9 to
+ # handle filtered trampoline frames. Since Frame#8 and Frame#10 are
+ # set to trampoline in DWARF by Intel® Fortran Compiler (ifx), they
+ # are not shown by the backtrace command.
set bt_stack \
[multi_line \
"#0\\s+breakpt \\(\\) at \[^\r\n\]+" \
@@ -73,8 +77,8 @@ proc run_tests { lang } {
"#5\\s+$hex in mixed_func_1d \\(\[^\r\n\]+\\) at \[^\r\n\]+" \
"#6\\s+$hex in mixed_func_1c \\(\[^\r\n\]+\\) at \[^\r\n\]+" \
"#7\\s+$hex in mixed_func_1b \\($1b_args\\) at \[^\r\n\]+" \
- "#8\\s+$hex in mixed_func_1a \\(\\) at \[^\r\n\]+" \
- "#9\\s+$hex in mixed_stack_main \\(\\) at \[^\r\n\]+" ]
+ "#$decimal\\s+$hex in mixed_func_1a \\(\\) at \[^\r\n\]+" \
+ "#$decimal\\s+$hex in mixed_stack_main \\(\\) at \[^\r\n\]+" ]
set main_args "argc=1, argv=${hex}( \[^\r\n\]+)?"
set bt_stack_kfail \
[multi_line \
--
2.34.1
Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v3 3/4] gdb/infrun: handle stepping through functions with DW_AT_trampoline
2023-07-10 22:56 ` [PATCH v3 3/4] gdb/infrun: handle stepping through functions with DW_AT_trampoline Abdul Basit Ijaz
@ 2023-07-11 11:21 ` Eli Zaretskii
2023-07-27 11:46 ` Bruno Larsen
1 sibling, 0 replies; 12+ messages in thread
From: Eli Zaretskii @ 2023-07-11 11:21 UTC (permalink / raw)
To: Abdul Basit Ijaz
Cc: gdb-patches, JiniSusan.George, tom, nils-christian.kempke
> From: Abdul Basit Ijaz <abdul.b.ijaz@intel.com>
> Cc: abdul.b.ijaz@intel.com,
> JiniSusan.George@amd.com,
> tom@tromey.com,
> eliz@gnu.org,
> Nils-Christian Kempke <nils-christian.kempke@intel.com>
> Date: Tue, 11 Jul 2023 00:56:42 +0200
>
> From: Nils-Christian Kempke <nils-christian.kempke@intel.com>
>
> This patch makes infrun continue stepping into and through trampoline
> functions marked via the DW_AT_trampoline in DWARF. The attribute can
> be emitted by the compiler for certain subroutines/inlined subroutines
> that are compiler generated and should be hidden from a user.
>
> Mainly, infrun is modified in 3 ways.
>
> First, GDB will now attempt to step through trampoline functions.
> Whenever we issued a step command that would make GDB step into a
> function that is marked trampoline, GDB will try to step directly towards
> the trampoline's 'target' instead and, e.g., not stop at the first
> instruction of the trampoline.
> The target can be specified by the compiler by the value of
> DW_AT_trampoline if its form is either an address, a name, or a DIE
> reference. DW_AT_trampoline is also allowed to be specified as a flag
> (containing true or false), in which case the target is assumed to be
> unknown. If GDB successfully finds a target, so if the value of
> DW_AT_trampoline was not a flag and could be resolved successfully, GDB
> steps directly towards the target and through the trampoline, hiding
> the trampoline from the user. If GDB cannot, however deduce a target,
> most likely because the DW_AT_trampoline was given as a flag or because
> of broken debug info, it will instead continue inside execution in the
> trampoline function until it reaches an instruction that is not
> associated with a trampoline function, which is usually the target
> function. It will then stop and give control back to the user.
> It should be noted, that there might be the cases, where trampolines
> call functions other than the target before the actual target call. If,
> in such a situation, GDB fails to resolve the target, it would resume
> execution until stepping into this other function call, and hand back
> control to the user, without actually having reached the target. A
> second step would have to be issued by the user to arrive a the target
> (by resuming in the trampoline and then until leaving it a second time).
> As this is a rather pathological case and no real instance of this is
> known, I think the current behavior here is good enough and seems to be
> the best GDB can do in such a situation.
>
> Secondly, as trampoline functions normally do not have any real source
> code correlation, it is likely that they mostly appear without line
> info.
> Normally, GDB would skip completely over a function call to a function
> that has no source line information, so we would never get to the
> aforementioned stepping through a trampoline and target resolution. To
> remedy this, for debug info trampolines, GDB now attempts to step through
> them regardless of them having source line information or not. So
> issuing a step at a function call wrapped by a trampoline without source
> line information will no longer skip the whole function call, but now
> step through the trampoline and attempt to resolve the trampoline target
> as described above (so usually, a single step at the call site will step
> through the trampoline and towards the target, even if the trampoline had
> not source line info).
>
> Last, in all other cases when GDB is about to stop at a location that is
> included in a trampoline region (e.g. after a step from the target back
> into the trampoline) GDB will instead continue until the trampoline
> region is left again and only then give control back to the user. This
> change serves the purpose of allowing stepping back from a target call
> through the trampoline without the user noticing the artificial function
> call inbetween call site and target.
>
> Together, these changes attempt to hide the trampoline function from the
> user while stepping. Additionally, the skip-trampoline-functions option
> has been introduced in infrun. It is set by default, and, when turned
> off, GDB will return to its 'normal' stepping behavior and ignore any
> possible DW_AT_trampoline.
>
> As currently only ifx emits the DW_AT_trampoline tag, a test has been
> added to gdb.dwarf2 that artificially creates a set of trampoline
> functions.
>
> 2023-07-10 Nils-Christian Kempke <nils-christian.kempke@intel.com>
> ---
> gdb/NEWS | 15 ++
> gdb/doc/gdb.texinfo | 36 +++
> gdb/infrun.c | 81 +++++-
> gdb/infrun.h | 4 +
> .../gdb.dwarf2/dw2-function-trampolines.c | 80 ++++++
> .../gdb.dwarf2/dw2-function-trampolines.exp | 245 ++++++++++++++++++
> 6 files changed, 457 insertions(+), 4 deletions(-)
> create mode 100644 gdb/testsuite/gdb.dwarf2/dw2-function-trampolines.c
> create mode 100644 gdb/testsuite/gdb.dwarf2/dw2-function-trampolines.exp
Thanks, the documentation parts are okay.
Reviewed-By: Eli Zaretskii <eliz@gnu.org>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v3 4/4] gdb: Skip trampoline frames in the stack for printing or finish command.
2023-07-10 22:56 ` [PATCH v3 4/4] gdb: Skip trampoline frames in the stack for printing or finish command Abdul Basit Ijaz
@ 2023-07-11 11:23 ` Eli Zaretskii
0 siblings, 0 replies; 12+ messages in thread
From: Eli Zaretskii @ 2023-07-11 11:23 UTC (permalink / raw)
To: Abdul Basit Ijaz; +Cc: gdb-patches, JiniSusan.George, tom
> From: Abdul Basit Ijaz <abdul.b.ijaz@intel.com>
> Cc: abdul.b.ijaz@intel.com,
> JiniSusan.George@amd.com,
> tom@tromey.com,
> eliz@gnu.org
> Date: Tue, 11 Jul 2023 00:56:43 +0200
>
> Before the change, GDB prints the frames indicated by the compiler with DIE
> "DW_AT_trampoline" in the backtrace and finish command, but for better user
> experience, all such frames can be hidden from the user. So, after this
> change, now such frames are not printed any more in the backtrace command
> and also the 'finish' command skips the trampoline calls.
>
> So far, this DIE is added to DWARF only by the IFX compiler, so
> gdb.fortran/mixed-lang-stack test used to fail for this compiler because of
> these extra trampoline frames in the backtrace. After the commit, those
> trampoline frames are filtered so test is updated accordingly to handle the
> frame level of the filtered frames.
>
> Backtrace output in this test before the change:
>
> bt -frame-arguments all
> '''
> '''
> (gdb) FAIL: gdb.fortran/mixed-lang-stack.exp: lang=auto: bt -frame-arguments all
>
> Now, after the change:
> bt -frame-arguments all
> '''
> '''
> (gdb) PASS: gdb.fortran/mixed-lang-stack.exp: lang=auto: bt -frame-arguments all
>
> Below is the example where IFX emits DW_at_trampoline for functions first
> and second trampoline calls as following:
>
> F90 reproducer info:
> function second (x, y) result(z)
> integer, intent(in) :: x, y
> integer :: z
> z = x * y ! breakpt-backtrace
> end function second
>
> function first (num1, num2) result(total)
> integer, intent(in) :: num1, num2
> integer :: total
> total = second (num1 + 4, num2 * 3) ! first-breakpt
> total = total + 30
> end function first
>
> Related Dwarf:
>
> 0x0000013f: DW_TAG_subprogram
> DW_AT_low_pc (0x0000000000404350)
> DW_AT_high_pc (0x000000000040435f)
> DW_AT_frame_base (DW_OP_reg6 RBP)
> DW_AT_linkage_name ("second_.t74p.t75p")
> DW_AT_name ("second_.t74p.t75p")
> DW_AT_trampoline ("second_")
>
> 0x0000015a: DW_TAG_subprogram
> DW_AT_low_pc (0x00000000004044a0)
> DW_AT_high_pc (0x00000000004044af)
> DW_AT_frame_base (DW_OP_reg6 RBP)
> DW_AT_linkage_name ("first_.t104p.t105p")
> DW_AT_name ("first_.t104p.t105p")
> DW_AT_trampoline ("first_")
>
> Before this change at line with comment 'breakpt-backtrace':
> (gdb) backtrace 3
> (gdb) finish
> Run till exit from #0 second (x=20, y=9) at test.f90:4
> second_.t74p.t75p () at test.f90:10
> 10 total = second (num1 + 4, num2 * 3) ! first-breakpt
> Value returned is $1 = 180
>
> After the change:
> (gdb) backtrace 3
> (gdb) finish
> Run till exit from #0 second (x=20, y=9) at test.f90:4
> 0x0000000000404333 in first (num1=16, num2=3) at test.f90:10
> 10 total = second (num1 + 4, num2 * 3) ! first-breakpt
> Value returned is $1 = 180
>
> New test func-trampoline is also added to test this in both backtrace and
> finish commands for Intel(R) Fortran Compiler.
>
> 2023-07-10 Abdul Basit Ijaz <abdul.b.ijaz@intel.com>
> ---
> gdb/doc/gdb.texinfo | 35 ++++++----
> gdb/infcmd.c | 13 ++++
> gdb/infrun.c | 8 +--
> gdb/infrun.h | 6 ++
> gdb/stack.c | 8 +++
> gdb/symtab.c | 12 ++++
> gdb/symtab.h | 5 ++
> gdb/testsuite/gdb.fortran/func-trampoline.exp | 69 +++++++++++++++++++
> gdb/testsuite/gdb.fortran/func-trampoline.f90 | 39 +++++++++++
> .../gdb.fortran/mixed-lang-stack.exp | 10 ++-
> 10 files changed, 186 insertions(+), 19 deletions(-)
> create mode 100644 gdb/testsuite/gdb.fortran/func-trampoline.exp
> create mode 100644 gdb/testsuite/gdb.fortran/func-trampoline.f90
OK for the documentation parts, thanks.
Reviewed-By: Eli Zaretskii <eliz@gnu.org>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v3 1/4] gdb, dwarf: add support for DW_AT_trampoline in DWARF reader
2023-07-10 22:56 ` [PATCH v3 1/4] gdb, dwarf: add support for DW_AT_trampoline in DWARF reader Abdul Basit Ijaz
@ 2023-07-27 9:09 ` Bruno Larsen
2023-07-29 11:36 ` Ijaz, Abdul B
0 siblings, 1 reply; 12+ messages in thread
From: Bruno Larsen @ 2023-07-27 9:09 UTC (permalink / raw)
To: Abdul Basit Ijaz, gdb-patches
Cc: JiniSusan.George, tom, eliz, Nils-Christian Kempke
On 11/07/2023 00:56, Abdul Basit Ijaz via Gdb-patches wrote:
> From: Nils-Christian Kempke <nils-christian.kempke@intel.com>
>
> DW_AT_trampoline can be used to describe compiler generated functions
> that serve some intermediary purpose on making a call to another
> function. A compiler can emit this tag in order to help a debugger hide
> the trampolines from a user.
>
> The attribute is only applicable to DW_TAG_subroutine and
> DW_TAG_inlined_subroutine tags. It contains information about the
> trampoline target either as a reference to its DIE, as its address or
> its name. DW_AT_trampoline can also be a flag indicating that the DIE
> is a trampoline or not without specifying the target (e.g. if it is
> unknown).
>
> This patch adds support to GDB for reading the DW_AT_trampoline
> attribute. It stores the attribute and its value in the type_specific
> part of a GDB type. This patch is implemented in preparation of the
> following patches, which will add a mechanism to hide DW_AT_trampoline
> subroutines from the user.
Sorry for jumping in late for the review. If I say something that's been
previously agreed upon, feel free to ignore me :)
Thanks for working on this! Some comments and nits inlined.
> 2023-07-10 Nils-Christian Kempke <nils-christian.kempke@intel.com>
> ---
> gdb/dwarf2/read.c | 45 +++++++++++++++++++-
> gdb/gdbtypes.c | 35 +++++++++++++++-
> gdb/gdbtypes.h | 103 +++++++++++++++++++++++++++++++++++++++++++---
> 3 files changed, 176 insertions(+), 7 deletions(-)
>
> diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
> index 3508f2c29ee..1cfa8178d94 100644
> --- a/gdb/dwarf2/read.c
> +++ b/gdb/dwarf2/read.c
> @@ -14657,6 +14657,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)
> + {
> + TYPE_FUNC_FLAGS (ftype) |= FUNC_TYPE_TRAMPOLINE;
> + TYPE_TRAMPOLINE_TARGET (ftype)
> + = (trampoline_target *) TYPE_ZALLOC (ftype,
> + sizeof (trampoline_target));
> +
> + /* A DW_AT_trampoline can be either an address, a flag, a reference or a
> + string. */
> + if (attr->form_is_string ())
> + TYPE_TRAMPOLINE_TARGET (ftype)->set_target_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);
As was mentioned in v1 of this patch, this bit gets the "regular" name
of the function. This could be a problem if there are 2 functions with
the same in different contexts (say in and out of a namespace). I would
suggest that you first attempt to take the linkage_name and only if that
doesn't exist do you take the name of the function. This way, if GDB
misidentifies a function it would probably be a compiler error, not
adding the linkage name.
> + 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
> + {
> + unrelocated_addr target_addr_reloc = attr->as_address ();
> + CORE_ADDR target_addr
> + = cu->per_objfile->relocate (target_addr_reloc);
> + target_addr = gdbarch_adjust_dwarf2_addr (objfile->arch (),
> + target_addr);
> + TYPE_TRAMPOLINE_TARGET (ftype)->set_target_physaddr (target_addr);
> + }
> + }
> +
> /* Store the calling convention in the type if it's available in
> the subroutine die. Otherwise set the calling convention to
> the default value DW_CC_normal. */
> @@ -14674,7 +14717,7 @@ read_subroutine_type (struct die_info *die, struct dwarf2_cu *cu)
> if the DWARF producer set that information. */
> attr = dwarf2_attr (die, DW_AT_noreturn, cu);
> if (attr && attr->as_boolean ())
> - TYPE_NO_RETURN (ftype) = 1;
> + TYPE_FUNC_FLAGS (ftype) |= FUNC_TYPE_NO_RETURN;
>
> /* We need to add the subroutine type to the die immediately so
> we don't infinitely recurse when dealing with parameters
> diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c
> index c5272979cb9..7355317f7db 100644
> --- a/gdb/gdbtypes.c
> +++ b/gdb/gdbtypes.c
> @@ -5091,6 +5091,33 @@ print_fixed_point_type_info (struct type *type, int spaces)
> type->fixed_point_scaling_factor ().str ().c_str ());
> }
>
> +/* Print the contents of the TYPE's self_trampoline_target, assuming that its
> + type-specific kind is TYPE_SPECIFIC_FUNC and is_trampoline is not 0. */
> +static void
> +print_trampoline_target_info (struct type *type, int spaces)
> +{
> + switch (TYPE_TRAMPOLINE_TARGET (type)->target_kind ())
> + {
> + case TRAMPOLINE_TARGET_PHYSADDR:
> + gdb_printf ("%*starget physaddr: 0x%s\n", spaces + 2, "",
> + print_core_address (type->arch_owner (),
> + TYPE_TRAMPOLINE_TARGET (type)
> + ->target_physaddr ()));
> + break;
> + case TRAMPOLINE_TARGET_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. */
> @@ -5417,6 +5444,10 @@ recursive_dump_type (struct type *type, int spaces)
> gdb_printf ("%*scalling_convention %d\n", spaces, "",
> TYPE_CALLING_CONVENTION (type));
> /* tail_call_list is not printed. */
> + gdb_printf ("%*sfunc_type_flags %u\n", spaces, "",
> + (unsigned int) TYPE_FUNC_FLAGS (type));
> + if (TYPE_IS_TRAMPOLINE (type))
> + print_trampoline_target_info (type, spaces);
> break;
>
> case TYPE_SPECIFIC_SELF_TYPE:
> @@ -5633,8 +5664,10 @@ copy_type_recursive (struct type *type, htab_t copied_types)
> case TYPE_SPECIFIC_FUNC:
> INIT_FUNC_SPECIFIC (new_type);
> TYPE_CALLING_CONVENTION (new_type) = TYPE_CALLING_CONVENTION (type);
> - TYPE_NO_RETURN (new_type) = TYPE_NO_RETURN (type);
> + TYPE_FUNC_FLAGS (new_type) |= TYPE_NO_RETURN (type);
> TYPE_TAIL_CALL_LIST (new_type) = NULL;
> + TYPE_FUNC_FLAGS (new_type) |= TYPE_IS_TRAMPOLINE (type);
Why not just have
TYPE_FUNC_FLAGS (new_type) = TYPE_FUNC_FLAGS (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 aedaf53cd5d..a1789223cd5 100644
> --- a/gdb/gdbtypes.h
> +++ b/gdb/gdbtypes.h
> @@ -106,6 +106,21 @@ enum type_instance_flag_value : unsigned
>
> DEF_ENUM_FLAGS_TYPE (enum type_instance_flag_value, type_instance_flags);
>
> +/* * Define flags for function types. */
> +enum func_type_flag_value : unsigned
> +{
> + /* * Flag indicates, whether this function normally returns to its
> + caller. It is set from the DW_AT_noreturn attribute if set on
> + the DW_TAG_subprogram. */
> + FUNC_TYPE_NO_RETURN = (1 << 0),
> +
> + /* * Flag is used for functions marked with DW_AT_trampoline. These
> + are compiler generated wrappers that should be hidden from the user. */
> + FUNC_TYPE_TRAMPOLINE = (1 << 1)
> +};
> +
> +DEF_ENUM_FLAGS_TYPE (enum func_type_flag_value, func_type_flags);
> +
> /* * Not textual. By default, GDB treats all single byte integers as
> characters (or elements of strings) unless this flag is set. */
>
> @@ -1730,11 +1745,9 @@ struct func_type
>
> ENUM_BITFIELD (dwarf_calling_convention) calling_convention : 8;
>
> - /* * Whether this function normally returns to its caller. It is
> - set from the DW_AT_noreturn attribute if set on the
> - DW_TAG_subprogram. */
> + /* * For storing function types defined in eunm func_type_flag_value. */
>
> - unsigned int is_noreturn : 1;
> + func_type_flags flags;
>
> /* * Only those DW_TAG_call_site's in this function that have
> DW_AT_call_tail_call set are linked in this list. Function
> @@ -1749,6 +1762,78 @@ struct func_type
> contains the method. */
>
> struct type *self_type;
> +
> + struct trampoline_target *self_trampoline_target;
This member should have a comment describing it.
Also, why did you call it self_trampoline_target instead of
trampoline_target? The only other member prefixed with self_ in this
struct is self_type, and that has it because when a method wants to know
the type pointed at by the "self" or "this" variable, it will check that
pointer, not due to self referencing stuff, if I understood it correctly.
> + };
> +
> +/* 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,
After some time looking into how GDB calculates names and what not, I
don't think calling this "physname" is a good idea, I think it would be
better to call it TRAMPOLINE_TARGET_MANGLED_NAME.
My reasoning for that is that physname is not some well defined thing,
it is a name that a GDB developer came up with to call what happens when
GDB needs to calculate mangled names ourselves because the compiler
didn't add a linkage name, as far as I understood it. Since the struct
trampoline_target doesn't care about how a mangled name was calculated,
and mangled name is a well defined thing while physname is not, I think
its better to use the latter.
> + /* An flag (target is unknown). */
s/An/A
> + TRAMPOLINE_TARGET_FLAG,
> + };
> +
> +struct trampoline_target
The struct and all methods in it should also have comments.
> + {
> + 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;
> };
>
> /* The type-specific info for TYPE_CODE_FIXED_POINT types. */
> @@ -1891,8 +1976,16 @@ extern void set_type_vptr_basetype (struct type *, struct type *);
> #define TYPE_GNAT_SPECIFIC(thistype) TYPE_MAIN_TYPE(thistype)->type_specific.gnat_stuff
> #define TYPE_DESCRIPTIVE_TYPE(thistype) TYPE_GNAT_SPECIFIC(thistype)->descriptive_type
> #define TYPE_CALLING_CONVENTION(thistype) TYPE_MAIN_TYPE(thistype)->type_specific.func_stuff->calling_convention
> -#define TYPE_NO_RETURN(thistype) TYPE_MAIN_TYPE(thistype)->type_specific.func_stuff->is_noreturn
> +#define TYPE_FUNC_FLAGS(thistype) \
> + TYPE_MAIN_TYPE(thistype)->type_specific.func_stuff->flags
> +#define TYPE_NO_RETURN(thistype) \
> + (TYPE_MAIN_TYPE(thistype)->type_specific.func_stuff->flags \
> + & FUNC_TYPE_NO_RETURN)
> #define TYPE_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->flags \
> + & FUNC_TYPE_TRAMPOLINE)
Minor nit, but I'd move this up above type_tail_call_list, just to keep
all func_flag stuff together.
> +#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 ())
--
Cheers,
Bruno
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v3 3/4] gdb/infrun: handle stepping through functions with DW_AT_trampoline
2023-07-10 22:56 ` [PATCH v3 3/4] gdb/infrun: handle stepping through functions with DW_AT_trampoline Abdul Basit Ijaz
2023-07-11 11:21 ` Eli Zaretskii
@ 2023-07-27 11:46 ` Bruno Larsen
2023-07-29 11:03 ` Ijaz, Abdul B
1 sibling, 1 reply; 12+ messages in thread
From: Bruno Larsen @ 2023-07-27 11:46 UTC (permalink / raw)
To: Abdul Basit Ijaz, gdb-patches
Cc: JiniSusan.George, tom, eliz, Nils-Christian Kempke
On 11/07/2023 00:56, Abdul Basit Ijaz via Gdb-patches wrote:
> From: Nils-Christian Kempke <nils-christian.kempke@intel.com>
>
> This patch makes infrun continue stepping into and through trampoline
> functions marked via the DW_AT_trampoline in DWARF. The attribute can
> be emitted by the compiler for certain subroutines/inlined subroutines
> that are compiler generated and should be hidden from a user.
>
> Mainly, infrun is modified in 3 ways.
>
> First, GDB will now attempt to step through trampoline functions.
> Whenever we issued a step command that would make GDB step into a
> function that is marked trampoline, GDB will try to step directly towards
> the trampoline's 'target' instead and, e.g., not stop at the first
> instruction of the trampoline.
> The target can be specified by the compiler by the value of
> DW_AT_trampoline if its form is either an address, a name, or a DIE
> reference. DW_AT_trampoline is also allowed to be specified as a flag
> (containing true or false), in which case the target is assumed to be
> unknown. If GDB successfully finds a target, so if the value of
> DW_AT_trampoline was not a flag and could be resolved successfully, GDB
> steps directly towards the target and through the trampoline, hiding
> the trampoline from the user. If GDB cannot, however deduce a target,
> most likely because the DW_AT_trampoline was given as a flag or because
> of broken debug info, it will instead continue inside execution in the
> trampoline function until it reaches an instruction that is not
> associated with a trampoline function, which is usually the target
> function. It will then stop and give control back to the user.
> It should be noted, that there might be the cases, where trampolines
> call functions other than the target before the actual target call. If,
> in such a situation, GDB fails to resolve the target, it would resume
> execution until stepping into this other function call, and hand back
> control to the user, without actually having reached the target. A
> second step would have to be issued by the user to arrive a the target
> (by resuming in the trampoline and then until leaving it a second time).
> As this is a rather pathological case and no real instance of this is
> known, I think the current behavior here is good enough and seems to be
> the best GDB can do in such a situation.
>
> Secondly, as trampoline functions normally do not have any real source
> code correlation, it is likely that they mostly appear without line
> info.
> Normally, GDB would skip completely over a function call to a function
> that has no source line information, so we would never get to the
> aforementioned stepping through a trampoline and target resolution. To
> remedy this, for debug info trampolines, GDB now attempts to step through
> them regardless of them having source line information or not. So
> issuing a step at a function call wrapped by a trampoline without source
> line information will no longer skip the whole function call, but now
> step through the trampoline and attempt to resolve the trampoline target
> as described above (so usually, a single step at the call site will step
> through the trampoline and towards the target, even if the trampoline had
> not source line info).
>
> Last, in all other cases when GDB is about to stop at a location that is
> included in a trampoline region (e.g. after a step from the target back
> into the trampoline) GDB will instead continue until the trampoline
> region is left again and only then give control back to the user. This
> change serves the purpose of allowing stepping back from a target call
> through the trampoline without the user noticing the artificial function
> call inbetween call site and target.
>
> Together, these changes attempt to hide the trampoline function from the
> user while stepping. Additionally, the skip-trampoline-functions option
> has been introduced in infrun. It is set by default, and, when turned
> off, GDB will return to its 'normal' stepping behavior and ignore any
> possible DW_AT_trampoline.
I really like this feature. Unfortunately, this isn't working when first
entering a trampoline in reverse stepping. ie:
➜ gdb ./gdb -q
testsuite/outputs/gdb.dwarf2/dw2-function-trampolines/dw2-function-trampolines
Reading symbols from
testsuite/outputs/gdb.dwarf2/dw2-function-trampolines/dw2-function-trampolines...
(gdb) start
Temporary breakpoint 1, main ()
at
/home/blarsen/Documents/fsf_build/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.dwarf2/dw2-function-trampolines.c:69
warning: Source file is more recent than executable.
69 global_var = 0; /* main set
global_var */
(gdb) record
(gdb) n
73 ans = trampoline (); /* main call
trampoline */
(gdb) n
75 ans = chained_trampoline (); /* main call
chained_trampoline */
(gdb) rs
0x000000000040114b in trampoline ()
(gdb)
Single stepping until exit from function trampoline,
which has no line number information.
target ()
at
/home/blarsen/Documents/fsf_build/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.dwarf2/dw2-function-trampolines.c:28
28 return 9 + 10; /* target return */
(gdb)
26 ++global_var; /* target add */
(gdb)
main ()
at
/home/blarsen/Documents/fsf_build/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.dwarf2/dw2-function-trampolines.c:73
73 ans = trampoline (); /* main call
trampoline */
(gdb)
I think this should be fixed before this patch went in.
--
Cheers,
Bruno
>
> As currently only ifx emits the DW_AT_trampoline tag, a test has been
> added to gdb.dwarf2 that artificially creates a set of trampoline
> functions.
>
> 2023-07-10 Nils-Christian Kempke <nils-christian.kempke@intel.com>
> ---
> gdb/NEWS | 15 ++
> gdb/doc/gdb.texinfo | 36 +++
> gdb/infrun.c | 81 +++++-
> gdb/infrun.h | 4 +
> .../gdb.dwarf2/dw2-function-trampolines.c | 80 ++++++
> .../gdb.dwarf2/dw2-function-trampolines.exp | 245 ++++++++++++++++++
> 6 files changed, 457 insertions(+), 4 deletions(-)
> create mode 100644 gdb/testsuite/gdb.dwarf2/dw2-function-trampolines.c
> create mode 100644 gdb/testsuite/gdb.dwarf2/dw2-function-trampolines.exp
>
> diff --git a/gdb/NEWS b/gdb/NEWS
> index b924834d3d7..8bca5540f58 100644
> --- a/gdb/NEWS
> +++ b/gdb/NEWS
> @@ -571,6 +571,21 @@ platform.
>
> * New commands
>
> +set skip-trampoline-functions on|off
> +show skip-trampoline-functions
> + This setting is 'on' by default. When 'on' it controls whether GDB's
> + stepping behavior will recognize function calls that have been marked as
> + trampolines in the debug info. It improves stepping behavior in that it
> + steps through trampoline code and hides it from the user. GDB can now step
> + through trampolines that are correctly marked as such in the compiler's
> + debug info. If the target of a trampoline is unknown, GDB will continue
> + until the trampoline section is left again and only then hand control back
> + to the user. GDB does this even if the trampoline has no associated line
> + info. If this is turned off, GDB will step into trampolines if there is
> + line table information for them or step over the trampoline calls if there
> + is no line table information. Currently, only DWARF trampolines are
> + supported.
> +
> maint set backtrace-on-fatal-signal on|off
> maint show backtrace-on-fatal-signal
> This setting is 'on' by default. When 'on' GDB will print a limited
> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
> index b10c06ae91f..839c3e6a0f8 100644
> --- a/gdb/doc/gdb.texinfo
> +++ b/gdb/doc/gdb.texinfo
> @@ -6391,6 +6391,42 @@ debug information. This is the default.
> Show whether @value{GDBN} will stop in or step over functions without
> source line debug information.
>
> +@kindex set skip-trampoline-functions
> +@cindex trampoline functions
> +@cindex stepping through trampoline functions
> +@item set skip-trampoline-functions
> +@itemx set skip-trampoline-functions on
> +When calling a function in any language, some compilers might generate
> +so-called @dfn{trampoline functions}, which wrap the actual function call
> +(the target of the trampoline). The compiler might mark such a trampoline in
> +its debug information. Often, such trampolines do not have any source line
> +information associated with them which will lead the @code{step} command to
> +behave like a @code{next} and skip the function call completely.
> +
> +The default is @code{set skip-trampoline-functions on} and it will cause the
> +@code{step} command to treat these trampolines differently. When issuing a
> +@code{step} at the call site of a trampoline function if
> +@code{skip-trampoline-functions} is set @value{GDBN} will attempt to determine
> +the target of the trampoline and then step through the trampoline stopping at
> +the target. If the target could not be found or was not given in the debug
> +info, @value{GDBN} will simply continue execution until it leaves the
> +trampoline code again, even if the trampoline has no line info associated with
> +it. When returning from a target function call and stepping back into the
> +trampoline, @value{GDBN} will again step through the trampoline towards the
> +call site. Additionally, even if stopped in a trampoline function with source
> +line information, issuing a @code{step} will prompt @value{GDBN} to resume
> +execution until leaving the trampoline region again. The @code{stepi} command
> +is not affected by the setting which is enabled by default. Currently, only
> +DWARF trampolines marked via DW_AT_trampoline are supported by this.
> +
> +@item set skip-trampoline-functions off
> +Causes the @code{step} command to completely ignore any trampoline information
> +a compiler might have emitted in its debug info. Trampolines will be treated
> +like any other function when stepping.
> +
> +@item show skip-trampoline-functions
> +Show whether @value{GDBN} tries to skip trampolines or not.
> +
> @kindex finish
> @kindex fin @r{(@code{finish})}
> @item finish
> diff --git a/gdb/infrun.c b/gdb/infrun.c
> index 58da1cef29e..dfafe7cdc74 100644
> --- a/gdb/infrun.c
> +++ b/gdb/infrun.c
> @@ -184,6 +184,12 @@ get_previous_thread ()
>
> static bool detach_fork = true;
>
> +/* If set (default) GDB will step through functions/inlined subroutines marked
> + DW_AT_trampoline by the compiler. If false, GDB will ignore the
> + attribute. */
> +
> +static bool skip_trampoline_functions = true;
> +
> bool debug_infrun = false;
> static void
> show_debug_infrun (struct ui_file *file, int from_tty,
> @@ -7265,7 +7271,6 @@ process_event_stop_test (struct execution_control_state *ecs)
> != find_pc_function (ecs->event_thread->stop_pc ())))))
> {
> CORE_ADDR stop_pc = ecs->event_thread->stop_pc ();
> - CORE_ADDR real_stop_pc;
>
> infrun_debug_printf ("stepped into subroutine");
>
> @@ -7337,8 +7342,41 @@ process_event_stop_test (struct execution_control_state *ecs)
> calling routine and the real function), locate the real
> function. That's what tells us (a) whether we want to step
> into it at all, and (b) what prologue we want to run to the
> - end of, if we do step into it. */
> - real_stop_pc = skip_language_trampoline (frame, stop_pc);
> + end of, if we do step into it. For functions marked as
> + trampoline functions we try to find their target and step
> + towards it (if skip_trampoline_functions is not set to false by the
> + user). If no target can be determined we just step into the
> + trampoline and hand control back to the user. */
> + CORE_ADDR real_stop_pc = 0;
> + bool in_trampoline = skip_trampoline_functions
> + && in_trampoline_function (stop_pc);
> +
> + if (in_trampoline)
> + {
> + real_stop_pc = find_function_trampoline_target (stop_pc);
> +
> + for (int i = 0; i < MAX_TRAMPOLINE_CHAIN_SIZE
> + && in_trampoline_function (real_stop_pc); ++i)
> + {
> + real_stop_pc = find_function_trampoline_target (real_stop_pc);
> + /* Exit if find_function_trampoline_target failed to find the
> + trampoline target. Do not try to resolve the trampolines
> + in this case. */
> + if (real_stop_pc == 0x0)
> + break;
> + }
> +
> + /* If we failed to find a target we will just single step in the
> + hope of leaving the trampoline again soon. */
> + if (real_stop_pc == 0x0)
> + {
> + keep_going (ecs);
> + return;
> + }
> + }
> +
> + if (real_stop_pc == 0)
> + real_stop_pc = skip_language_trampoline (frame, stop_pc);
> if (real_stop_pc == 0)
> real_stop_pc = gdbarch_skip_trampoline_code (gdbarch, frame, stop_pc);
> if (real_stop_pc != 0)
> @@ -7359,6 +7397,8 @@ process_event_stop_test (struct execution_control_state *ecs)
> /* If we have line number information for the function we are
> thinking of stepping into and the function isn't on the skip
> list, step into it.
> + If we are about to step into a function marked trampoline with no
> + line number information, we still want to enter it here.
>
> If there are several symtabs at that PC (e.g. with include
> files), just want to know whether *any* of them have line
> @@ -7367,7 +7407,9 @@ process_event_stop_test (struct execution_control_state *ecs)
> struct symtab_and_line tmp_sal;
>
> tmp_sal = find_pc_line (ecs->stop_func_start, 0);
> - if (tmp_sal.line != 0
> + if ((tmp_sal.line != 0
> + || (skip_trampoline_functions
> + && in_trampoline_function (ecs->stop_func_start)))
> && !function_name_is_marked_for_skip (ecs->stop_func_name,
> tmp_sal)
> && !inline_frame_is_marked_for_skip (true, ecs->event_thread))
> @@ -7520,6 +7562,19 @@ process_event_stop_test (struct execution_control_state *ecs)
> return;
> }
>
> + /* If we ended up in a function trampoline without stepping into a new
> + function we are either in some inlined trampoline or returning through a
> + trampoline function. In either case we continue to single step until we
> + are out of the trampoline code again. This check has to be done before
> + stop_pc_sal.line == 0 below, as trampolines usually don't have source
> + line information associated with them. */
> + if (skip_trampoline_functions && in_trampoline_function (stop_pc_sal.pc))
> + {
> + infrun_debug_printf ("stepped into trampoline code");
> + keep_going (ecs);
> + return;
> + }
> +
> if (stop_pc_sal.line == 0)
> {
> /* We have no line number information. That means to stop
> @@ -9731,6 +9786,14 @@ show_exec_direction_func (struct ui_file *out, int from_tty,
> }
> }
>
> +static void
> +show_skip_trampoline_functions (ui_file *file, int from_tty,
> + cmd_list_element *c,
> + const char *value)
> +{
> + gdb_printf (file, _("Skipping trampoline functions is %s.\n"), value);
> +}
> +
> static void
> show_schedule_multiple (struct ui_file *file, int from_tty,
> struct cmd_list_element *c, const char *value)
> @@ -10076,6 +10139,16 @@ Options are 'forward' or 'reverse'."),
> set_exec_direction_func, show_exec_direction_func,
> &setlist, &showlist);
>
> + add_setshow_boolean_cmd ("skip-trampoline-functions", class_run,
> + &skip_trampoline_functions, _("\
> +Set whether gdb attempts to hide trampolines marked in the debug info."), _("\
> +Show whether gdb attempts to hide trampolines marked in the debug info."), _("\
> +If on, while stepping gdb will skip through functions and inlined functions\n\
> +marked as trampolines by the compiler. If off, gdb will ignore such function\n\
> +trampolines."),
> + nullptr, show_skip_trampoline_functions, &setlist,
> + &showlist);
> +
> /* Set/show detach-on-fork: user-settable mode. */
>
> add_setshow_boolean_cmd ("detach-on-fork", class_run, &detach_fork, _("\
> diff --git a/gdb/infrun.h b/gdb/infrun.h
> index a343d27f72d..d4a6b7892b5 100644
> --- a/gdb/infrun.h
> +++ b/gdb/infrun.h
> @@ -76,6 +76,10 @@ infrun_debug_show_threads (const char *title, ThreadRange threads)
> }
>
>
> +/* Maximum size of trampoline chain to process while resolving
> + trampolines. */
> +#define MAX_TRAMPOLINE_CHAIN_SIZE 10
> +
> /* Nonzero if we want to give control to the user when we're notified
> of shared library events by the dynamic linker. */
> extern int stop_on_solib_events;
> diff --git a/gdb/testsuite/gdb.dwarf2/dw2-function-trampolines.c b/gdb/testsuite/gdb.dwarf2/dw2-function-trampolines.c
> new file mode 100644
> index 00000000000..1f0eb2e0efa
> --- /dev/null
> +++ b/gdb/testsuite/gdb.dwarf2/dw2-function-trampolines.c
> @@ -0,0 +1,80 @@
> +/* Copyright 2023 Free Software Foundation, Inc.
> +
> + This program is free software; you can redistribute it and/or modify
> + it under the terms of the GNU General Public License as published by
> + the Free Software Foundation; either version 3 of the License, or
> + (at your option) any later version.
> +
> + This program is distributed in the hope that it will be useful,
> + but WITHOUT ANY WARRANTY; without even the implied warranty of
> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + GNU General Public License for more details.
> +
> + You should have received a copy of the GNU General Public License
> + along with this program. If not, see <http://www.gnu.org/licenses/>. */
> +
> +/* This test relies on inlined_trampoline being inlined into main and the other
> + functions not. All functions except target will be marked via
> + DW_AT_trampoline in the debug info and we'll check whether one can step
> + through the trampolines towards target. */
> +volatile int global_var;
> +
> +int __attribute__ ((noinline))
> +target () /* target decl line */
> +{ /* target prologue */
> + asm ("target_label: .globl target_label");
> + ++global_var; /* target add */
> + asm ("target_label2: .globl target_label2");
> + return 9 + 10; /* target return */
> +} /* target end */
> +
> +int __attribute__ ((noinline))
> +trampoline ()
> +{ /* trampoline prologue */
> + asm ("trampoline_label: .globl trampoline_label");
> + ++global_var;
> + return target (); /* trampoline target call */
> +} /* trampoline end */
> +
> +static inline int __attribute__ ((always_inline))
> +inlined_trampoline ()
> +{ /* inlined_trampoline prologue */
> + asm ("inlined_trampoline_label: .globl inlined_trampoline_label");
> + ++global_var; /* inlined_trampoline add */
> + asm ("inlined_trampoline_label2: .globl inlined_trampoline_label2");
> + return target (); /* inlined_trampoline target call */
> +} /* inlined_trampoline end */
> +
> +int __attribute__ ((noinline))
> +chained_trampoline ()
> +{ /* chained_trampoline prologue */
> + asm ("chained_trampoline_label: .globl chained_trampoline_label");
> + ++global_var;
> + return trampoline (); /* chained_trampoline trampoline call */
> +} /* chained_trampoline end */
> +
> +int __attribute__ ((noinline))
> +doubly_chained_trampoline ()
> +{ /* doubly_chained_trampoline prologue */
> + asm ("doubly_chained_trampoline_label: .globl doubly_chained_trampoline_label");
> + ++global_var;
> + return chained_trampoline (); /* doubly_chained_trampoline chained_trampoline call */
> +} /* doubly_chained_trampoline end */
> +
> +int
> +main () /* main decl line */
> +{ /* main prologue */
> + int ans;
> + asm ("main_label: .globl main_label");
> + global_var = 0; /* main set global_var */
> + asm ("main_label2: .globl main_label2");
> + ans = inlined_trampoline (); /* main call inlined_trampoline */
> + asm ("main_label3: .globl main_label3");
> + ans = trampoline (); /* main call trampoline */
> + asm ("main_label4: .globl main_label4");
> + ans = chained_trampoline (); /* main call chained_trampoline */
> + asm ("main_label5: .globl main_label5");
> + ans = doubly_chained_trampoline (); /* main call doubly_chained_trampoline */
> + asm ("main_label6: .globl main_label6");
> + return ans; /* main call return */
> +} /* main end */
> diff --git a/gdb/testsuite/gdb.dwarf2/dw2-function-trampolines.exp b/gdb/testsuite/gdb.dwarf2/dw2-function-trampolines.exp
> new file mode 100644
> index 00000000000..67eec2b5540
> --- /dev/null
> +++ b/gdb/testsuite/gdb.dwarf2/dw2-function-trampolines.exp
> @@ -0,0 +1,245 @@
> +# Copyright 2023 Free Software Foundation, Inc.
> +
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 3 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program. If not, see <http://www.gnu.org/licenses/>.
> +
> +# This test checks GDB's handling of functions/inlined functions marked
> +# DW_AT_trampoline by the compiler. A function marked as trampoline should
> +# generally be hidden from the user. We check whether we can step through
> +# trampolines. Every trampoline is defined using a different type for its
> +# target: a string, an address, a DIE reference or a flag.
> +# Setting skip-trampoline-functions to false inside GDB should make it return
> +# to its 'normal' behavior, ignore the DW_AT_trampoline, and skip all of the
> +# non-inlined trampoline calls (as their DIEs don't have any source
> +# information).
> +
> +load_lib dwarf.exp
> +
> +# This test can only be run on targets which support DWARF-2 and use gas.
> +if {![dwarf2_support]} {
> + return 0
> +}
> +
> +standard_testfile .c .S
> +
> +set asm_file [standard_output_file $srcfile2]
> +Dwarf::assemble $asm_file {
> + global srcdir subdir srcfile
> + declare_labels lines_label trampoline_label
> +
> + get_func_info target
> + get_func_info trampoline
> + get_func_info chained_trampoline
> + get_func_info doubly_chained_trampoline
> + get_func_info main
> +
> + set target_decl_line [gdb_get_line_number "target decl line"]
> + set main_decl_line [gdb_get_line_number "main decl line"]
> + set main_call_inlined_trampoline_line [gdb_get_line_number "main call inlined_trampoline"]
> +
> + cu {} {
> + compile_unit {
> + {language @DW_LANG_C}
> + {name dw2-function-trampolines.c}
> + {low_pc 0 addr}
> + {stmt_list ${lines_label} DW_FORM_sec_offset}
> + } {
> + subprogram {
> + {name target}
> + {low_pc $target_start addr}
> + {high_pc "$target_start + $target_len" addr}
> + {decl_file 1 data1}
> + {decl_line $target_decl_line data1}
> + }
> + # The 'trampoline' subprogram declares its target by name.
> + trampoline_label: subprogram {
> + {name trampoline}
> + {low_pc $trampoline_start addr}
> + {high_pc "$trampoline_start + $trampoline_len" addr}
> + {trampoline target string}
> + }
> + # The 'chained_trampoline' subprogram declares its target as die
> + # reference.
> + subprogram {
> + {name chained_trampoline}
> + {low_pc $chained_trampoline_start addr}
> + {high_pc "$chained_trampoline_start + $chained_trampoline_len" addr}
> + {trampoline %$trampoline_label}
> + }
> + # The 'doubly_chained_trampoline' subprogram declares no target.
> + # Its DW_AT_trampoline is a flag set to true.
> + subprogram {
> + {name doubly_chained_trampoline}
> + {low_pc $doubly_chained_trampoline_start addr}
> + {high_pc "$doubly_chained_trampoline_start + $doubly_chained_trampoline_len" addr}
> + {trampoline 1 flag}
> + }
> + subprogram {
> + {external 1 flag}
> + {name main}
> + {main_subprogram 1 flag}
> + {low_pc $main_start addr}
> + {high_pc "$main_start + $main_len" addr}
> + {decl_file 1 data1}
> + {decl_line $main_decl_line data1}
> + } {
> + # The 'inlined_trampoline' subroutine declares its target as
> + # an address.
> + inlined_subroutine {
> + {name inlined_trampoline}
> + {low_pc main_label2 addr}
> + {high_pc main_label3 addr}
> + {trampoline $target_start addr}
> + {call_file 1 data1}
> + {call_line $main_call_inlined_trampoline_line data1}
> + }
> + }
> + }
> + }
> +
> + lines {version 2} lines_label {
> + include_dir "${srcdir}/${subdir}"
> + file_name "$srcfile" 1
> +
> + program {
> + DW_LNE_set_address $main_start
> + line [gdb_get_line_number "main set global_var"]
> + DW_LNS_copy
> + DW_LNE_set_address main_label
> + line [gdb_get_line_number "main set global_var"]
> + DW_LNS_copy
> + DW_LNE_set_address main_label2
> + line [gdb_get_line_number "main call inlined_trampoline"]
> + DW_LNS_copy
> + DW_LNE_set_address inlined_trampoline_label
> + line [gdb_get_line_number "inlined_trampoline add"]
> + DW_LNS_copy
> + DW_LNE_set_address inlined_trampoline_label2
> + line [gdb_get_line_number "inlined_trampoline target call"]
> + DW_LNS_copy
> + DW_LNE_set_address main_label3
> + line [gdb_get_line_number "main call trampoline"]
> + DW_LNS_copy
> + DW_LNE_set_address main_label4
> + line [gdb_get_line_number "main call chained_trampoline"]
> + DW_LNS_copy
> + DW_LNE_set_address main_label5
> + line [gdb_get_line_number "main call doubly_chained_trampoline"]
> + DW_LNS_copy
> + DW_LNE_set_address main_label6
> + line [gdb_get_line_number "main call return"]
> + DW_LNS_copy
> + DW_LNE_set_address $main_end
> + DW_LNE_end_sequence
> +
> + DW_LNE_set_address $target_start
> + line [gdb_get_line_number "target prologue"]
> + DW_LNS_negate_stmt
> + DW_LNS_copy
> + DW_LNE_set_address target_label
> + line [gdb_get_line_number "target add"]
> + DW_LNS_negate_stmt
> + DW_LNS_copy
> + DW_LNE_set_address target_label2
> + line [gdb_get_line_number "target return"]
> + DW_LNS_copy
> + DW_LNE_set_address $target_end
> + DW_LNE_end_sequence
> + }
> + }
> +}
> +
> +if {[prepare_for_testing "failed to prepare" ${testfile} \
> + [list $srcfile $asm_file] {nodebug additional_flags=-O0}]} {
> + return -1
> +}
> +
> +set target_first_line_pattern ".*target add.*"
> +set target_second_line_pattern ".*target return.*"
> +
> +if ![runto_main] {
> + return -1
> +}
> +
> +gdb_test "show skip-trampoline-functions" \
> + "Skipping trampoline functions is on\." \
> + "check skip-trampoline-functions is enabled"
> +
> +with_test_prefix "with trampoline handling" {
> + foreach {trampoline return_line} { "inlined_trampoline" "trampoline" \
> + "trampoline" "chained_trampoline" \
> + "chained_trampoline" "doubly_chained_trampoline" } {
> +
> + gdb_test "s" "$target_first_line_pattern" "step through $trampoline"
> + gdb_test "s" "$target_second_line_pattern" \
> + "step target second line from $trampoline"
> + gdb_test "s" ".*main call $return_line.*" \
> + "step back through $trampoline"
> + }
> +
> + # The doubly_chained_trampoline has only been marked as trampoline but no
> + # target was given. In this case GDB steps into the trampoline and then
> + # continues until the trampoline section is left again.
> +
> + # When compiled with gcc 7.5 (and possibly others) on a 32 bit system, the
> + # trampoline function contains a call to __x86.get_pc_thunk.ax before the
> + # actual target call. So, we end up in __x86.get_pc_thunk.ax. Issuing a
> + # second step command will return from the function call back into the
> + # trampoline and go on inside the trampoline towards the actual target call.
> + # On other targets we step directly towards the target call.
> + gdb_test_multiple "s" "step through double_chained_trampoline" {
> + -re -wrap "$target_first_line_pattern" {
> + pass $gdb_test_name
> + }
> + -re -wrap ".*__x86.get_pc_thunk.ax.*" {
> + gdb_test "s" "$target_first_line_pattern" \
> + "step through double_chained_trampoline 2nd try"
> + }
> + }
> + gdb_test "s" "$target_second_line_pattern" \
> + "step target second line fromdoubly_chained_trampoline"
> + gdb_test "s" ".*main call return.*" \
> + "step back through doubly_chained_trampoline"
> +}
> +
> +clean_restart ${testfile}
> +
> +if ![runto_main] {
> + return -1
> +}
> +
> +gdb_test_no_output "set skip-trampoline-functions off" \
> + "disable trampoline handling"
> +gdb_test "show skip-trampoline-functions" \
> + "Skipping trampoline functions is off." \
> + "check skip-trampoline-functions is disabled"
> +
> +with_test_prefix "without trampoline handling" {
> + gdb_test "s" ".*main call inlined_trampoline.*"
> + gdb_test "s" ".*inlined_trampoline add.*" \
> + "step into inlined_trampoline with skip-trampoline off"
> + gdb_test "s" ".*inlined_trampoline target call.*" \
> + "step in inlined_trampoline with skip-trampoline off"
> + gdb_test "s" "$target_first_line_pattern" \
> + "step into target with skip-trampoline off"
> + gdb_test "s" "$target_second_line_pattern" \
> + "step second line in target with skip-trampoline off"
> + gdb_test "s" ".*main call trampoline.*" \
> + "step brack from target with skip-trampoline off"
> + gdb_test "s" ".*main call chained_trampoline.*" \
> + "skip trampoline call with no line info"
> + gdb_test "s" ".*main call doubly_chained_trampoline.*" \
> + "skip chained_trampoline call with no line info"
> + gdb_test "s" ".*main call return.*" \
> + "skip doubly_chained_trampoline call with no line info"
> +}
^ permalink raw reply [flat|nested] 12+ messages in thread
* RE: [PATCH v3 3/4] gdb/infrun: handle stepping through functions with DW_AT_trampoline
2023-07-27 11:46 ` Bruno Larsen
@ 2023-07-29 11:03 ` Ijaz, Abdul B
0 siblings, 0 replies; 12+ messages in thread
From: Ijaz, Abdul B @ 2023-07-29 11:03 UTC (permalink / raw)
To: Bruno Larsen, gdb-patches; +Cc: JiniSusan.George, tom, eliz
Thanks a lot Bruno for the feedback and catching the problem in reverse stepping for trampolines.
> I really like this feature. Unfortunately, this isn't working when first entering a trampoline in reverse stepping. ie:
Will fix the problem in V4 patch series. We should avoid processing the targets for trampoline calls in the reverse direction and just keep going in "infrunc.c\process_event_stop_test function". So hopefully this should be fixed by the V4 series of patch. So will test it then hopefully soon I will push it for review.
Thanks & Best Regards
Abdul Basit
-----Original Message-----
From: Bruno Larsen <blarsen@redhat.com>
Sent: Thursday, July 27, 2023 1:47 PM
To: Ijaz, Abdul B <abdul.b.ijaz@intel.com>; gdb-patches@sourceware.org
Cc: JiniSusan.George@amd.com; tom@tromey.com; eliz@gnu.org; Nils-Christian Kempke <nils-christian.kempke@intel.com>
Subject: Re: [PATCH v3 3/4] gdb/infrun: handle stepping through functions with DW_AT_trampoline
On 11/07/2023 00:56, Abdul Basit Ijaz via Gdb-patches wrote:
> From: Nils-Christian Kempke <nils-christian.kempke@intel.com>
>
> This patch makes infrun continue stepping into and through trampoline
> functions marked via the DW_AT_trampoline in DWARF. The attribute can
> be emitted by the compiler for certain subroutines/inlined subroutines
> that are compiler generated and should be hidden from a user.
>
> Mainly, infrun is modified in 3 ways.
>
> First, GDB will now attempt to step through trampoline functions.
> Whenever we issued a step command that would make GDB step into a
> function that is marked trampoline, GDB will try to step directly
> towards the trampoline's 'target' instead and, e.g., not stop at the
> first instruction of the trampoline.
> The target can be specified by the compiler by the value of
> DW_AT_trampoline if its form is either an address, a name, or a DIE
> reference. DW_AT_trampoline is also allowed to be specified as a flag
> (containing true or false), in which case the target is assumed to be
> unknown. If GDB successfully finds a target, so if the value of
> DW_AT_trampoline was not a flag and could be resolved successfully,
> GDB steps directly towards the target and through the trampoline,
> hiding the trampoline from the user. If GDB cannot, however deduce a
> target, most likely because the DW_AT_trampoline was given as a flag
> or because of broken debug info, it will instead continue inside
> execution in the trampoline function until it reaches an instruction
> that is not associated with a trampoline function, which is usually
> the target function. It will then stop and give control back to the user.
> It should be noted, that there might be the cases, where trampolines
> call functions other than the target before the actual target call.
> If, in such a situation, GDB fails to resolve the target, it would
> resume execution until stepping into this other function call, and
> hand back control to the user, without actually having reached the
> target. A second step would have to be issued by the user to arrive a
> the target (by resuming in the trampoline and then until leaving it a second time).
> As this is a rather pathological case and no real instance of this is
> known, I think the current behavior here is good enough and seems to
> be the best GDB can do in such a situation.
>
> Secondly, as trampoline functions normally do not have any real source
> code correlation, it is likely that they mostly appear without line
> info.
> Normally, GDB would skip completely over a function call to a function
> that has no source line information, so we would never get to the
> aforementioned stepping through a trampoline and target resolution.
> To remedy this, for debug info trampolines, GDB now attempts to step
> through them regardless of them having source line information or not.
> So issuing a step at a function call wrapped by a trampoline without
> source line information will no longer skip the whole function call,
> but now step through the trampoline and attempt to resolve the
> trampoline target as described above (so usually, a single step at the
> call site will step through the trampoline and towards the target,
> even if the trampoline had not source line info).
>
> Last, in all other cases when GDB is about to stop at a location that
> is included in a trampoline region (e.g. after a step from the target
> back into the trampoline) GDB will instead continue until the
> trampoline region is left again and only then give control back to the
> user. This change serves the purpose of allowing stepping back from a
> target call through the trampoline without the user noticing the
> artificial function call inbetween call site and target.
>
> Together, these changes attempt to hide the trampoline function from
> the user while stepping. Additionally, the skip-trampoline-functions
> option has been introduced in infrun. It is set by default, and, when
> turned off, GDB will return to its 'normal' stepping behavior and
> ignore any possible DW_AT_trampoline.
I really like this feature. Unfortunately, this isn't working when first entering a trampoline in reverse stepping. ie:
➜ gdb ./gdb -q
testsuite/outputs/gdb.dwarf2/dw2-function-trampolines/dw2-function-trampolines
Reading symbols from
testsuite/outputs/gdb.dwarf2/dw2-function-trampolines/dw2-function-trampolines...
(gdb) start
Temporary breakpoint 1, main ()
at
/home/blarsen/Documents/fsf_build/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.dwarf2/dw2-function-trampolines.c:69
warning: Source file is more recent than executable.
69 global_var = 0; /* main set global_var */
(gdb) record
(gdb) n
73 ans = trampoline (); /* main call trampoline */
(gdb) n
75 ans = chained_trampoline (); /* main call chained_trampoline */
(gdb) rs
0x000000000040114b in trampoline ()
(gdb)
Single stepping until exit from function trampoline, which has no line number information.
target ()
at
/home/blarsen/Documents/fsf_build/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.dwarf2/dw2-function-trampolines.c:28
28 return 9 + 10; /* target return */
(gdb)
26 ++global_var; /* target add */
(gdb)
main ()
at
/home/blarsen/Documents/fsf_build/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.dwarf2/dw2-function-trampolines.c:73
73 ans = trampoline (); /* main call trampoline */
(gdb)
I think this should be fixed before this patch went in.
--
Cheers,
Bruno
>
> As currently only ifx emits the DW_AT_trampoline tag, a test has been
> added to gdb.dwarf2 that artificially creates a set of trampoline
> functions.
>
> 2023-07-10 Nils-Christian Kempke <nils-christian.kempke@intel.com>
> ---
> gdb/NEWS | 15 ++
> gdb/doc/gdb.texinfo | 36 +++
> gdb/infrun.c | 81 +++++-
> gdb/infrun.h | 4 +
> .../gdb.dwarf2/dw2-function-trampolines.c | 80 ++++++
> .../gdb.dwarf2/dw2-function-trampolines.exp | 245 ++++++++++++++++++
> 6 files changed, 457 insertions(+), 4 deletions(-)
> create mode 100644 gdb/testsuite/gdb.dwarf2/dw2-function-trampolines.c
> create mode 100644
> gdb/testsuite/gdb.dwarf2/dw2-function-trampolines.exp
>
> diff --git a/gdb/NEWS b/gdb/NEWS
> index b924834d3d7..8bca5540f58 100644
> --- a/gdb/NEWS
> +++ b/gdb/NEWS
> @@ -571,6 +571,21 @@ platform.
>
> * New commands
>
> +set skip-trampoline-functions on|off
> +show skip-trampoline-functions
> + This setting is 'on' by default. When 'on' it controls whether
> +GDB's
> + stepping behavior will recognize function calls that have been
> +marked as
> + trampolines in the debug info. It improves stepping behavior in
> +that it
> + steps through trampoline code and hides it from the user. GDB can
> +now step
> + through trampolines that are correctly marked as such in the
> +compiler's
> + debug info. If the target of a trampoline is unknown, GDB will
> +continue
> + until the trampoline section is left again and only then hand
> +control back
> + to the user. GDB does this even if the trampoline has no
> +associated line
> + info. If this is turned off, GDB will step into trampolines if
> +there is
> + line table information for them or step over the trampoline calls
> +if there
> + is no line table information. Currently, only DWARF trampolines
> +are
> + supported.
> +
> maint set backtrace-on-fatal-signal on|off
> maint show backtrace-on-fatal-signal
> This setting is 'on' by default. When 'on' GDB will print a
> limited diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo index
> b10c06ae91f..839c3e6a0f8 100644
> --- a/gdb/doc/gdb.texinfo
> +++ b/gdb/doc/gdb.texinfo
> @@ -6391,6 +6391,42 @@ debug information. This is the default.
> Show whether @value{GDBN} will stop in or step over functions without
> source line debug information.
>
> +@kindex set skip-trampoline-functions @cindex trampoline functions
> +@cindex stepping through trampoline functions @item set
> +skip-trampoline-functions @itemx set skip-trampoline-functions on
> +When calling a function in any language, some compilers might
> +generate so-called @dfn{trampoline functions}, which wrap the actual
> +function call (the target of the trampoline). The compiler might
> +mark such a trampoline in its debug information. Often, such
> +trampolines do not have any source line information associated with
> +them which will lead the @code{step} command to behave like a @code{next} and skip the function call completely.
> +
> +The default is @code{set skip-trampoline-functions on} and it will
> +cause the @code{step} command to treat these trampolines differently.
> +When issuing a @code{step} at the call site of a trampoline function
> +if @code{skip-trampoline-functions} is set @value{GDBN} will attempt
> +to determine the target of the trampoline and then step through the
> +trampoline stopping at the target. If the target could not be found
> +or was not given in the debug info, @value{GDBN} will simply continue
> +execution until it leaves the trampoline code again, even if the
> +trampoline has no line info associated with it. When returning from
> +a target function call and stepping back into the trampoline,
> +@value{GDBN} will again step through the trampoline towards the call
> +site. Additionally, even if stopped in a trampoline function with
> +source line information, issuing a @code{step} will prompt
> +@value{GDBN} to resume execution until leaving the trampoline region
> +again. The @code{stepi} command is not affected by the setting which is enabled by default. Currently, only DWARF trampolines marked via DW_AT_trampoline are supported by this.
> +
> +@item set skip-trampoline-functions off Causes the @code{step}
> +command to completely ignore any trampoline information a compiler
> +might have emitted in its debug info. Trampolines will be treated
> +like any other function when stepping.
> +
> +@item show skip-trampoline-functions
> +Show whether @value{GDBN} tries to skip trampolines or not.
> +
> @kindex finish
> @kindex fin @r{(@code{finish})}
> @item finish
> diff --git a/gdb/infrun.c b/gdb/infrun.c index
> 58da1cef29e..dfafe7cdc74 100644
> --- a/gdb/infrun.c
> +++ b/gdb/infrun.c
> @@ -184,6 +184,12 @@ get_previous_thread ()
>
> static bool detach_fork = true;
>
> +/* If set (default) GDB will step through functions/inlined subroutines marked
> + DW_AT_trampoline by the compiler. If false, GDB will ignore the
> + attribute. */
> +
> +static bool skip_trampoline_functions = true;
> +
> bool debug_infrun = false;
> static void
> show_debug_infrun (struct ui_file *file, int from_tty, @@ -7265,7
> +7271,6 @@ process_event_stop_test (struct execution_control_state *ecs)
> != find_pc_function (ecs->event_thread->stop_pc ())))))
> {
> CORE_ADDR stop_pc = ecs->event_thread->stop_pc ();
> - CORE_ADDR real_stop_pc;
>
> infrun_debug_printf ("stepped into subroutine");
>
> @@ -7337,8 +7342,41 @@ process_event_stop_test (struct execution_control_state *ecs)
> calling routine and the real function), locate the real
> function. That's what tells us (a) whether we want to step
> into it at all, and (b) what prologue we want to run to the
> - end of, if we do step into it. */
> - real_stop_pc = skip_language_trampoline (frame, stop_pc);
> + end of, if we do step into it. For functions marked as
> + trampoline functions we try to find their target and step
> + towards it (if skip_trampoline_functions is not set to false by the
> + user). If no target can be determined we just step into the
> + trampoline and hand control back to the user. */
> + CORE_ADDR real_stop_pc = 0;
> + bool in_trampoline = skip_trampoline_functions
> + && in_trampoline_function (stop_pc);
> +
> + if (in_trampoline)
> + {
> + real_stop_pc = find_function_trampoline_target (stop_pc);
> +
> + for (int i = 0; i < MAX_TRAMPOLINE_CHAIN_SIZE
> + && in_trampoline_function (real_stop_pc); ++i)
> + {
> + real_stop_pc = find_function_trampoline_target (real_stop_pc);
> + /* Exit if find_function_trampoline_target failed to find the
> + trampoline target. Do not try to resolve the trampolines
> + in this case. */
> + if (real_stop_pc == 0x0)
> + break;
> + }
> +
> + /* If we failed to find a target we will just single step in the
> + hope of leaving the trampoline again soon. */
> + if (real_stop_pc == 0x0)
> + {
> + keep_going (ecs);
> + return;
> + }
> + }
> +
> + if (real_stop_pc == 0)
> + real_stop_pc = skip_language_trampoline (frame, stop_pc);
> if (real_stop_pc == 0)
> real_stop_pc = gdbarch_skip_trampoline_code (gdbarch, frame, stop_pc);
> if (real_stop_pc != 0)
> @@ -7359,6 +7397,8 @@ process_event_stop_test (struct execution_control_state *ecs)
> /* If we have line number information for the function we are
> thinking of stepping into and the function isn't on the skip
> list, step into it.
> + If we are about to step into a function marked trampoline with no
> + line number information, we still want to enter it here.
>
> If there are several symtabs at that PC (e.g. with include
> files), just want to know whether *any* of them have line @@
> -7367,7 +7407,9 @@ process_event_stop_test (struct execution_control_state *ecs)
> struct symtab_and_line tmp_sal;
>
> tmp_sal = find_pc_line (ecs->stop_func_start, 0);
> - if (tmp_sal.line != 0
> + if ((tmp_sal.line != 0
> + || (skip_trampoline_functions
> + && in_trampoline_function (ecs->stop_func_start)))
> && !function_name_is_marked_for_skip (ecs->stop_func_name,
> tmp_sal)
> && !inline_frame_is_marked_for_skip (true, ecs->event_thread))
> @@ -7520,6 +7562,19 @@ process_event_stop_test (struct execution_control_state *ecs)
> return;
> }
>
> + /* If we ended up in a function trampoline without stepping into a new
> + function we are either in some inlined trampoline or returning through a
> + trampoline function. In either case we continue to single step until we
> + are out of the trampoline code again. This check has to be done before
> + stop_pc_sal.line == 0 below, as trampolines usually don't have source
> + line information associated with them. */ if
> + (skip_trampoline_functions && in_trampoline_function (stop_pc_sal.pc))
> + {
> + infrun_debug_printf ("stepped into trampoline code");
> + keep_going (ecs);
> + return;
> + }
> +
> if (stop_pc_sal.line == 0)
> {
> /* We have no line number information. That means to stop @@
> -9731,6 +9786,14 @@ show_exec_direction_func (struct ui_file *out, int from_tty,
> }
> }
>
> +static void
> +show_skip_trampoline_functions (ui_file *file, int from_tty,
> + cmd_list_element *c,
> + const char *value)
> +{
> + gdb_printf (file, _("Skipping trampoline functions is %s.\n"),
> +value); }
> +
> static void
> show_schedule_multiple (struct ui_file *file, int from_tty,
> struct cmd_list_element *c, const char *value) @@ -10076,6
> +10139,16 @@ Options are 'forward' or 'reverse'."),
> set_exec_direction_func, show_exec_direction_func,
> &setlist, &showlist);
>
> + add_setshow_boolean_cmd ("skip-trampoline-functions", class_run,
> + &skip_trampoline_functions, _("\ Set whether gdb attempts to
> +hide trampolines marked in the debug info."), _("\ Show whether gdb
> +attempts to hide trampolines marked in the debug info."), _("\ If on,
> +while stepping gdb will skip through functions and inlined
> +functions\n\ marked as trampolines by the compiler. If off, gdb will
> +ignore such function\n\ trampolines."),
> + nullptr, show_skip_trampoline_functions, &setlist,
> + &showlist);
> +
> /* Set/show detach-on-fork: user-settable mode. */
>
> add_setshow_boolean_cmd ("detach-on-fork", class_run,
> &detach_fork, _("\ diff --git a/gdb/infrun.h b/gdb/infrun.h index
> a343d27f72d..d4a6b7892b5 100644
> --- a/gdb/infrun.h
> +++ b/gdb/infrun.h
> @@ -76,6 +76,10 @@ infrun_debug_show_threads (const char *title, ThreadRange threads)
> }
>
>
> +/* Maximum size of trampoline chain to process while resolving
> + trampolines. */
> +#define MAX_TRAMPOLINE_CHAIN_SIZE 10
> +
> /* Nonzero if we want to give control to the user when we're notified
> of shared library events by the dynamic linker. */
> extern int stop_on_solib_events;
> diff --git a/gdb/testsuite/gdb.dwarf2/dw2-function-trampolines.c
> b/gdb/testsuite/gdb.dwarf2/dw2-function-trampolines.c
> new file mode 100644
> index 00000000000..1f0eb2e0efa
> --- /dev/null
> +++ b/gdb/testsuite/gdb.dwarf2/dw2-function-trampolines.c
> @@ -0,0 +1,80 @@
> +/* Copyright 2023 Free Software Foundation, Inc.
> +
> + This program is free software; you can redistribute it and/or modify
> + it under the terms of the GNU General Public License as published by
> + the Free Software Foundation; either version 3 of the License, or
> + (at your option) any later version.
> +
> + This program is distributed in the hope that it will be useful,
> + but WITHOUT ANY WARRANTY; without even the implied warranty of
> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + GNU General Public License for more details.
> +
> + You should have received a copy of the GNU General Public License
> + along with this program. If not, see
> + <http://www.gnu.org/licenses/>. */
> +
> +/* This test relies on inlined_trampoline being inlined into main and the other
> + functions not. All functions except target will be marked via
> + DW_AT_trampoline in the debug info and we'll check whether one can step
> + through the trampolines towards target. */ volatile int
> +global_var;
> +
> +int __attribute__ ((noinline))
> +target () /* target decl line */
> +{ /* target prologue */
> + asm ("target_label: .globl target_label");
> + ++global_var; /* target add */
> + asm ("target_label2: .globl target_label2");
> + return 9 + 10; /* target return */
> +} /* target end */
> +
> +int __attribute__ ((noinline))
> +trampoline ()
> +{ /* trampoline prologue */
> + asm ("trampoline_label: .globl trampoline_label");
> + ++global_var;
> + return target (); /* trampoline target call */
> +} /* trampoline end */
> +
> +static inline int __attribute__ ((always_inline)) inlined_trampoline
> +()
> +{ /* inlined_trampoline prologue */
> + asm ("inlined_trampoline_label: .globl inlined_trampoline_label");
> + ++global_var; /* inlined_trampoline add */
> + asm ("inlined_trampoline_label2: .globl inlined_trampoline_label2");
> + return target (); /* inlined_trampoline target call */
> +} /* inlined_trampoline end */
> +
> +int __attribute__ ((noinline))
> +chained_trampoline ()
> +{ /* chained_trampoline prologue */
> + asm ("chained_trampoline_label: .globl chained_trampoline_label");
> + ++global_var;
> + return trampoline (); /* chained_trampoline trampoline call */
> +} /* chained_trampoline end */
> +
> +int __attribute__ ((noinline))
> +doubly_chained_trampoline ()
> +{ /* doubly_chained_trampoline prologue */
> + asm ("doubly_chained_trampoline_label: .globl
> +doubly_chained_trampoline_label");
> + ++global_var;
> + return chained_trampoline (); /* doubly_chained_trampoline chained_trampoline call */
> +} /* doubly_chained_trampoline end */
> +
> +int
> +main () /* main decl line */
> +{ /* main prologue */
> + int ans;
> + asm ("main_label: .globl main_label");
> + global_var = 0; /* main set global_var */
> + asm ("main_label2: .globl main_label2");
> + ans = inlined_trampoline (); /* main call inlined_trampoline */
> + asm ("main_label3: .globl main_label3");
> + ans = trampoline (); /* main call trampoline */
> + asm ("main_label4: .globl main_label4");
> + ans = chained_trampoline (); /* main call chained_trampoline */
> + asm ("main_label5: .globl main_label5");
> + ans = doubly_chained_trampoline (); /* main call doubly_chained_trampoline */
> + asm ("main_label6: .globl main_label6");
> + return ans; /* main call return */
> +} /* main end */
> diff --git a/gdb/testsuite/gdb.dwarf2/dw2-function-trampolines.exp
> b/gdb/testsuite/gdb.dwarf2/dw2-function-trampolines.exp
> new file mode 100644
> index 00000000000..67eec2b5540
> --- /dev/null
> +++ b/gdb/testsuite/gdb.dwarf2/dw2-function-trampolines.exp
> @@ -0,0 +1,245 @@
> +# Copyright 2023 Free Software Foundation, Inc.
> +
> +# This program is free software; you can redistribute it and/or
> +modify # it under the terms of the GNU General Public License as
> +published by # the Free Software Foundation; either version 3 of the
> +License, or # (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful, #
> +but WITHOUT ANY WARRANTY; without even the implied warranty of #
> +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the # GNU
> +General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License #
> +along with this program. If not, see <http://www.gnu.org/licenses/>.
> +
> +# This test checks GDB's handling of functions/inlined functions
> +marked # DW_AT_trampoline by the compiler. A function marked as
> +trampoline should # generally be hidden from the user. We check
> +whether we can step through # trampolines. Every trampoline is
> +defined using a different type for its # target: a string, an address, a DIE reference or a flag.
> +# Setting skip-trampoline-functions to false inside GDB should make
> +it return # to its 'normal' behavior, ignore the DW_AT_trampoline,
> +and skip all of the # non-inlined trampoline calls (as their DIEs
> +don't have any source # information).
> +
> +load_lib dwarf.exp
> +
> +# This test can only be run on targets which support DWARF-2 and use gas.
> +if {![dwarf2_support]} {
> + return 0
> +}
> +
> +standard_testfile .c .S
> +
> +set asm_file [standard_output_file $srcfile2] Dwarf::assemble
> +$asm_file {
> + global srcdir subdir srcfile
> + declare_labels lines_label trampoline_label
> +
> + get_func_info target
> + get_func_info trampoline
> + get_func_info chained_trampoline
> + get_func_info doubly_chained_trampoline
> + get_func_info main
> +
> + set target_decl_line [gdb_get_line_number "target decl line"]
> + set main_decl_line [gdb_get_line_number "main decl line"]
> + set main_call_inlined_trampoline_line [gdb_get_line_number "main
> + call inlined_trampoline"]
> +
> + cu {} {
> + compile_unit {
> + {language @DW_LANG_C}
> + {name dw2-function-trampolines.c}
> + {low_pc 0 addr}
> + {stmt_list ${lines_label} DW_FORM_sec_offset}
> + } {
> + subprogram {
> + {name target}
> + {low_pc $target_start addr}
> + {high_pc "$target_start + $target_len" addr}
> + {decl_file 1 data1}
> + {decl_line $target_decl_line data1}
> + }
> + # The 'trampoline' subprogram declares its target by name.
> + trampoline_label: subprogram {
> + {name trampoline}
> + {low_pc $trampoline_start addr}
> + {high_pc "$trampoline_start + $trampoline_len" addr}
> + {trampoline target string}
> + }
> + # The 'chained_trampoline' subprogram declares its target as die
> + # reference.
> + subprogram {
> + {name chained_trampoline}
> + {low_pc $chained_trampoline_start addr}
> + {high_pc "$chained_trampoline_start + $chained_trampoline_len" addr}
> + {trampoline %$trampoline_label}
> + }
> + # The 'doubly_chained_trampoline' subprogram declares no target.
> + # Its DW_AT_trampoline is a flag set to true.
> + subprogram {
> + {name doubly_chained_trampoline}
> + {low_pc $doubly_chained_trampoline_start addr}
> + {high_pc "$doubly_chained_trampoline_start + $doubly_chained_trampoline_len" addr}
> + {trampoline 1 flag}
> + }
> + subprogram {
> + {external 1 flag}
> + {name main}
> + {main_subprogram 1 flag}
> + {low_pc $main_start addr}
> + {high_pc "$main_start + $main_len" addr}
> + {decl_file 1 data1}
> + {decl_line $main_decl_line data1}
> + } {
> + # The 'inlined_trampoline' subroutine declares its target as
> + # an address.
> + inlined_subroutine {
> + {name inlined_trampoline}
> + {low_pc main_label2 addr}
> + {high_pc main_label3 addr}
> + {trampoline $target_start addr}
> + {call_file 1 data1}
> + {call_line $main_call_inlined_trampoline_line data1}
> + }
> + }
> + }
> + }
> +
> + lines {version 2} lines_label {
> + include_dir "${srcdir}/${subdir}"
> + file_name "$srcfile" 1
> +
> + program {
> + DW_LNE_set_address $main_start
> + line [gdb_get_line_number "main set global_var"]
> + DW_LNS_copy
> + DW_LNE_set_address main_label
> + line [gdb_get_line_number "main set global_var"]
> + DW_LNS_copy
> + DW_LNE_set_address main_label2
> + line [gdb_get_line_number "main call inlined_trampoline"]
> + DW_LNS_copy
> + DW_LNE_set_address inlined_trampoline_label
> + line [gdb_get_line_number "inlined_trampoline add"]
> + DW_LNS_copy
> + DW_LNE_set_address inlined_trampoline_label2
> + line [gdb_get_line_number "inlined_trampoline target call"]
> + DW_LNS_copy
> + DW_LNE_set_address main_label3
> + line [gdb_get_line_number "main call trampoline"]
> + DW_LNS_copy
> + DW_LNE_set_address main_label4
> + line [gdb_get_line_number "main call chained_trampoline"]
> + DW_LNS_copy
> + DW_LNE_set_address main_label5
> + line [gdb_get_line_number "main call doubly_chained_trampoline"]
> + DW_LNS_copy
> + DW_LNE_set_address main_label6
> + line [gdb_get_line_number "main call return"]
> + DW_LNS_copy
> + DW_LNE_set_address $main_end
> + DW_LNE_end_sequence
> +
> + DW_LNE_set_address $target_start
> + line [gdb_get_line_number "target prologue"]
> + DW_LNS_negate_stmt
> + DW_LNS_copy
> + DW_LNE_set_address target_label
> + line [gdb_get_line_number "target add"]
> + DW_LNS_negate_stmt
> + DW_LNS_copy
> + DW_LNE_set_address target_label2
> + line [gdb_get_line_number "target return"]
> + DW_LNS_copy
> + DW_LNE_set_address $target_end
> + DW_LNE_end_sequence
> + }
> + }
> +}
> +
> +if {[prepare_for_testing "failed to prepare" ${testfile} \
> + [list $srcfile $asm_file] {nodebug additional_flags=-O0}]} {
> + return -1
> +}
> +
> +set target_first_line_pattern ".*target add.*"
> +set target_second_line_pattern ".*target return.*"
> +
> +if ![runto_main] {
> + return -1
> +}
> +
> +gdb_test "show skip-trampoline-functions" \
> + "Skipping trampoline functions is on\." \
> + "check skip-trampoline-functions is enabled"
> +
> +with_test_prefix "with trampoline handling" {
> + foreach {trampoline return_line} { "inlined_trampoline" "trampoline" \
> + "trampoline" "chained_trampoline" \
> + "chained_trampoline" "doubly_chained_trampoline" } {
> +
> + gdb_test "s" "$target_first_line_pattern" "step through $trampoline"
> + gdb_test "s" "$target_second_line_pattern" \
> + "step target second line from $trampoline"
> + gdb_test "s" ".*main call $return_line.*" \
> + "step back through $trampoline"
> + }
> +
> + # The doubly_chained_trampoline has only been marked as trampoline but no
> + # target was given. In this case GDB steps into the trampoline and then
> + # continues until the trampoline section is left again.
> +
> + # When compiled with gcc 7.5 (and possibly others) on a 32 bit system, the
> + # trampoline function contains a call to __x86.get_pc_thunk.ax before the
> + # actual target call. So, we end up in __x86.get_pc_thunk.ax. Issuing a
> + # second step command will return from the function call back into the
> + # trampoline and go on inside the trampoline towards the actual target call.
> + # On other targets we step directly towards the target call.
> + gdb_test_multiple "s" "step through double_chained_trampoline" {
> + -re -wrap "$target_first_line_pattern" {
> + pass $gdb_test_name
> + }
> + -re -wrap ".*__x86.get_pc_thunk.ax.*" {
> + gdb_test "s" "$target_first_line_pattern" \
> + "step through double_chained_trampoline 2nd try"
> + }
> + }
> + gdb_test "s" "$target_second_line_pattern" \
> + "step target second line fromdoubly_chained_trampoline"
> + gdb_test "s" ".*main call return.*" \
> + "step back through doubly_chained_trampoline"
> +}
> +
> +clean_restart ${testfile}
> +
> +if ![runto_main] {
> + return -1
> +}
> +
> +gdb_test_no_output "set skip-trampoline-functions off" \
> + "disable trampoline handling"
> +gdb_test "show skip-trampoline-functions" \
> + "Skipping trampoline functions is off." \
> + "check skip-trampoline-functions is disabled"
> +
> +with_test_prefix "without trampoline handling" {
> + gdb_test "s" ".*main call inlined_trampoline.*"
> + gdb_test "s" ".*inlined_trampoline add.*" \
> + "step into inlined_trampoline with skip-trampoline off"
> + gdb_test "s" ".*inlined_trampoline target call.*" \
> + "step in inlined_trampoline with skip-trampoline off"
> + gdb_test "s" "$target_first_line_pattern" \
> + "step into target with skip-trampoline off"
> + gdb_test "s" "$target_second_line_pattern" \
> + "step second line in target with skip-trampoline off"
> + gdb_test "s" ".*main call trampoline.*" \
> + "step brack from target with skip-trampoline off"
> + gdb_test "s" ".*main call chained_trampoline.*" \
> + "skip trampoline call with no line info"
> + gdb_test "s" ".*main call doubly_chained_trampoline.*" \
> + "skip chained_trampoline call with no line info"
> + gdb_test "s" ".*main call return.*" \
> + "skip doubly_chained_trampoline call with no line info"
> +}
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] 12+ messages in thread
* RE: [PATCH v3 1/4] gdb, dwarf: add support for DW_AT_trampoline in DWARF reader
2023-07-27 9:09 ` Bruno Larsen
@ 2023-07-29 11:36 ` Ijaz, Abdul B
2023-07-31 7:16 ` Bruno Larsen
0 siblings, 1 reply; 12+ messages in thread
From: Ijaz, Abdul B @ 2023-07-29 11:36 UTC (permalink / raw)
To: Bruno Larsen, gdb-patches
Cc: JiniSusan.George, tom, eliz, Nils-Christian Kempke
Thanks a lot Bruno for your feedback.
Can you please have a look on the replies if its fine with you otherwise will I update accordingly in V4 patch series.
1)
> As was mentioned in v1 of this patch, this bit gets the "regular" name of the function ...
Yes, this should have been fixed. Somehow missed it, will first try to read linkage name and they regular. Modified locally so should be fixed in next series.
2)
> Why not just have
> TYPE_FUNC_FLAGS (new_type) = TYPE_FUNC_FLAGS (type);
Yes this make sense, changed locally to replace 2 lines for return and trampoline type with this line and now only need to test so hopefully should be updated in v4 series.
3)
>> + struct trampoline_target *self_trampoline_target;
>This member should have a comment describing it.
>Also, why did you call it self_trampoline_target instead of trampoline_target? The only other member prefixed with self_ in this struct is self_type, and that has it because when a method wants to know the type pointed at by the "self" or "this" variable, it will check that pointer, not due to self referencing stuff, if I understood it correctly.
Intention behind self_ is only that the trampoline calls itself are just for calling to the target function. So each trampoline entry is just pointing to the target call. Not sure if self_* usage is restricted for this where source is just referencing to the target. So please let me know if adding comment with this information will be sufficient otherwise I will remove the self_ suffix from the variable name.
4)
> After some time looking into how GDB calculates names and what not, I don't think calling this "physname" is a good idea, I think it would be better to call it TRAMPOLINE_TARGET_MANGLED_NAME.
> My reasoning for that is that physname is not some well defined thing, it is a name that a GDB developer came up with to call what happens when GDB needs to calculate mangled names ourselves because the compiler didn't add a linkage name, as far as I understood it. Since the struct trampoline_target doesn't care about how a mangled name was calculated, and mangled name is a well defined thing while physname is not, I think its better to use the latter.
Since as per (1) comment above we are first going to read mangled and if not then demangled name. So I would then just prefer to call it TRAMPOLINE_TARGET_NAME, hopefully it would be fine with you.
5)
> s/An/A
>> + TRAMPOLINE_TARGET_FLAG,
Will be fixed in V4 series.
6)
> The struct and all methods in it should also have comments.
>> +struct trampoline_target
Will add in V4 series.
7)
> Minor nit, but I'd move this up above type_tail_call_list, just to keep all func_flag stuff together.
Will be updated accordingly in V4 series.
Thanks & Best Regards,
Abdul Basit
-----Original Message-----
From: Bruno Larsen <blarsen@redhat.com>
Sent: Thursday, July 27, 2023 11:10 AM
To: Ijaz, Abdul B <abdul.b.ijaz@intel.com>; gdb-patches@sourceware.org
Cc: JiniSusan.George@amd.com; tom@tromey.com; eliz@gnu.org; Nils-Christian Kempke <nils-christian.kempke@intel.com>
Subject: Re: [PATCH v3 1/4] gdb, dwarf: add support for DW_AT_trampoline in DWARF reader
On 11/07/2023 00:56, Abdul Basit Ijaz via Gdb-patches wrote:
> From: Nils-Christian Kempke <nils-christian.kempke@intel.com>
>
> DW_AT_trampoline can be used to describe compiler generated functions
> that serve some intermediary purpose on making a call to another
> function. A compiler can emit this tag in order to help a debugger
> hide the trampolines from a user.
>
> The attribute is only applicable to DW_TAG_subroutine and
> DW_TAG_inlined_subroutine tags. It contains information about the
> trampoline target either as a reference to its DIE, as its address or
> its name. DW_AT_trampoline can also be a flag indicating that the DIE
> is a trampoline or not without specifying the target (e.g. if it is
> unknown).
>
> This patch adds support to GDB for reading the DW_AT_trampoline
> attribute. It stores the attribute and its value in the type_specific
> part of a GDB type. This patch is implemented in preparation of the
> following patches, which will add a mechanism to hide DW_AT_trampoline
> subroutines from the user.
Sorry for jumping in late for the review. If I say something that's been previously agreed upon, feel free to ignore me :)
Thanks for working on this! Some comments and nits inlined.
> 2023-07-10 Nils-Christian Kempke <nils-christian.kempke@intel.com>
> ---
> gdb/dwarf2/read.c | 45 +++++++++++++++++++-
> gdb/gdbtypes.c | 35 +++++++++++++++-
> gdb/gdbtypes.h | 103 +++++++++++++++++++++++++++++++++++++++++++---
> 3 files changed, 176 insertions(+), 7 deletions(-)
>
> diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c index
> 3508f2c29ee..1cfa8178d94 100644
> --- a/gdb/dwarf2/read.c
> +++ b/gdb/dwarf2/read.c
> @@ -14657,6 +14657,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)
> + {
> + TYPE_FUNC_FLAGS (ftype) |= FUNC_TYPE_TRAMPOLINE;
> + TYPE_TRAMPOLINE_TARGET (ftype)
> + = (trampoline_target *) TYPE_ZALLOC (ftype,
> + sizeof (trampoline_target));
> +
> + /* A DW_AT_trampoline can be either an address, a flag, a reference or a
> + string. */
> + if (attr->form_is_string ())
> + TYPE_TRAMPOLINE_TARGET (ftype)->set_target_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);
As was mentioned in v1 of this patch, this bit gets the "regular" name of the function. This could be a problem if there are 2 functions with the same in different contexts (say in and out of a namespace). I would suggest that you first attempt to take the linkage_name and only if that doesn't exist do you take the name of the function. This way, if GDB misidentifies a function it would probably be a compiler error, not adding the linkage name.
> + 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
> + {
> + unrelocated_addr target_addr_reloc = attr->as_address ();
> + CORE_ADDR target_addr
> + = cu->per_objfile->relocate (target_addr_reloc);
> + target_addr = gdbarch_adjust_dwarf2_addr (objfile->arch (),
> + target_addr);
> + TYPE_TRAMPOLINE_TARGET (ftype)->set_target_physaddr (target_addr);
> + }
> + }
> +
> /* Store the calling convention in the type if it's available in
> the subroutine die. Otherwise set the calling convention to
> the default value DW_CC_normal. */ @@ -14674,7 +14717,7 @@
> read_subroutine_type (struct die_info *die, struct dwarf2_cu *cu)
> if the DWARF producer set that information. */
> attr = dwarf2_attr (die, DW_AT_noreturn, cu);
> if (attr && attr->as_boolean ())
> - TYPE_NO_RETURN (ftype) = 1;
> + TYPE_FUNC_FLAGS (ftype) |= FUNC_TYPE_NO_RETURN;
>
> /* We need to add the subroutine type to the die immediately so
> we don't infinitely recurse when dealing with parameters diff
> --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c index c5272979cb9..7355317f7db
> 100644
> --- a/gdb/gdbtypes.c
> +++ b/gdb/gdbtypes.c
> @@ -5091,6 +5091,33 @@ print_fixed_point_type_info (struct type *type, int spaces)
> type->fixed_point_scaling_factor ().str ().c_str ());
> }
>
> +/* Print the contents of the TYPE's self_trampoline_target, assuming that its
> + type-specific kind is TYPE_SPECIFIC_FUNC and is_trampoline is not
> +0. */ static void print_trampoline_target_info (struct type *type,
> +int spaces) {
> + switch (TYPE_TRAMPOLINE_TARGET (type)->target_kind ())
> + {
> + case TRAMPOLINE_TARGET_PHYSADDR:
> + gdb_printf ("%*starget physaddr: 0x%s\n", spaces + 2, "",
> + print_core_address (type->arch_owner (),
> + TYPE_TRAMPOLINE_TARGET (type)
> + ->target_physaddr ()));
> + break;
> + case TRAMPOLINE_TARGET_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. */
> @@ -5417,6 +5444,10 @@ recursive_dump_type (struct type *type, int spaces)
> gdb_printf ("%*scalling_convention %d\n", spaces, "",
> TYPE_CALLING_CONVENTION (type));
> /* tail_call_list is not printed. */
> + gdb_printf ("%*sfunc_type_flags %u\n", spaces, "",
> + (unsigned int) TYPE_FUNC_FLAGS (type));
> + if (TYPE_IS_TRAMPOLINE (type))
> + print_trampoline_target_info (type, spaces);
> break;
>
> case TYPE_SPECIFIC_SELF_TYPE:
> @@ -5633,8 +5664,10 @@ copy_type_recursive (struct type *type, htab_t copied_types)
> case TYPE_SPECIFIC_FUNC:
> INIT_FUNC_SPECIFIC (new_type);
> TYPE_CALLING_CONVENTION (new_type) = TYPE_CALLING_CONVENTION (type);
> - TYPE_NO_RETURN (new_type) = TYPE_NO_RETURN (type);
> + TYPE_FUNC_FLAGS (new_type) |= TYPE_NO_RETURN (type);
> TYPE_TAIL_CALL_LIST (new_type) = NULL;
> + TYPE_FUNC_FLAGS (new_type) |= TYPE_IS_TRAMPOLINE (type);
Why not just have
TYPE_FUNC_FLAGS (new_type) = TYPE_FUNC_FLAGS (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 aedaf53cd5d..a1789223cd5
> 100644
> --- a/gdb/gdbtypes.h
> +++ b/gdb/gdbtypes.h
> @@ -106,6 +106,21 @@ enum type_instance_flag_value : unsigned
>
> DEF_ENUM_FLAGS_TYPE (enum type_instance_flag_value,
> type_instance_flags);
>
> +/* * Define flags for function types. */ enum func_type_flag_value :
> +unsigned {
> + /* * Flag indicates, whether this function normally returns to its
> + caller. It is set from the DW_AT_noreturn attribute if set on
> + the DW_TAG_subprogram. */
> + FUNC_TYPE_NO_RETURN = (1 << 0),
> +
> + /* * Flag is used for functions marked with DW_AT_trampoline. These
> + are compiler generated wrappers that should be hidden from the
> +user. */
> + FUNC_TYPE_TRAMPOLINE = (1 << 1)
> +};
> +
> +DEF_ENUM_FLAGS_TYPE (enum func_type_flag_value, func_type_flags);
> +
> /* * Not textual. By default, GDB treats all single byte integers as
> characters (or elements of strings) unless this flag is set. */
>
> @@ -1730,11 +1745,9 @@ struct func_type
>
> ENUM_BITFIELD (dwarf_calling_convention) calling_convention : 8;
>
> - /* * Whether this function normally returns to its caller. It is
> - set from the DW_AT_noreturn attribute if set on the
> - DW_TAG_subprogram. */
> + /* * For storing function types defined in eunm
> + func_type_flag_value. */
>
> - unsigned int is_noreturn : 1;
> + func_type_flags flags;
>
> /* * Only those DW_TAG_call_site's in this function that have
> DW_AT_call_tail_call set are linked in this list. Function
> @@ -1749,6 +1762,78 @@ struct func_type
> contains the method. */
>
> struct type *self_type;
> +
> + struct trampoline_target *self_trampoline_target;
This member should have a comment describing it.
Also, why did you call it self_trampoline_target instead of trampoline_target? The only other member prefixed with self_ in this struct is self_type, and that has it because when a method wants to know the type pointed at by the "self" or "this" variable, it will check that pointer, not due to self referencing stuff, if I understood it correctly.
> + };
> +
> +/* 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,
After some time looking into how GDB calculates names and what not, I don't think calling this "physname" is a good idea, I think it would be better to call it TRAMPOLINE_TARGET_MANGLED_NAME.
My reasoning for that is that physname is not some well defined thing, it is a name that a GDB developer came up with to call what happens when GDB needs to calculate mangled names ourselves because the compiler didn't add a linkage name, as far as I understood it. Since the struct trampoline_target doesn't care about how a mangled name was calculated, and mangled name is a well defined thing while physname is not, I think its better to use the latter.
> + /* An flag (target is unknown). */
s/An/A
> + TRAMPOLINE_TARGET_FLAG,
> + };
> +
> +struct trampoline_target
The struct and all methods in it should also have comments.
> + {
> + 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;
> };
>
> /* The type-specific info for TYPE_CODE_FIXED_POINT types. */ @@
> -1891,8 +1976,16 @@ extern void set_type_vptr_basetype (struct type *, struct type *);
> #define TYPE_GNAT_SPECIFIC(thistype) TYPE_MAIN_TYPE(thistype)->type_specific.gnat_stuff
> #define TYPE_DESCRIPTIVE_TYPE(thistype) TYPE_GNAT_SPECIFIC(thistype)->descriptive_type
> #define TYPE_CALLING_CONVENTION(thistype)
> TYPE_MAIN_TYPE(thistype)->type_specific.func_stuff->calling_convention
> -#define TYPE_NO_RETURN(thistype)
> TYPE_MAIN_TYPE(thistype)->type_specific.func_stuff->is_noreturn
> +#define TYPE_FUNC_FLAGS(thistype) \
> + TYPE_MAIN_TYPE(thistype)->type_specific.func_stuff->flags
> +#define TYPE_NO_RETURN(thistype) \
> + (TYPE_MAIN_TYPE(thistype)->type_specific.func_stuff->flags \
> + & FUNC_TYPE_NO_RETURN)
> #define TYPE_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->flags \
> + & FUNC_TYPE_TRAMPOLINE)
Minor nit, but I'd move this up above type_tail_call_list, just to keep all func_flag stuff together.
> +#define TYPE_TRAMPOLINE_TARGET(thistype)
> +TYPE_MAIN_TYPE(thistype)->type_specific.func_stuff->self_trampoline_t
> +arget
> #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 ())
--
Cheers,
Bruno
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] 12+ messages in thread
* Re: [PATCH v3 1/4] gdb, dwarf: add support for DW_AT_trampoline in DWARF reader
2023-07-29 11:36 ` Ijaz, Abdul B
@ 2023-07-31 7:16 ` Bruno Larsen
0 siblings, 0 replies; 12+ messages in thread
From: Bruno Larsen @ 2023-07-31 7:16 UTC (permalink / raw)
To: Ijaz, Abdul B, gdb-patches
Cc: JiniSusan.George, tom, eliz, Nils-Christian Kempke
On 29/07/2023 13:36, Ijaz, Abdul B wrote:
> 3)
>>> + struct trampoline_target *self_trampoline_target;
>> This member should have a comment describing it.
>> Also, why did you call it self_trampoline_target instead of trampoline_target? The only other member prefixed with self_ in this struct is self_type, and that has it because when a method wants to know the type pointed at by the "self" or "this" variable, it will check that pointer, not due to self referencing stuff, if I understood it correctly.
> Intention behind self_ is only that the trampoline calls itself are just for calling to the target function. So each trampoline entry is just pointing to the target call. Not sure if self_* usage is restricted for this where source is just referencing to the target. So please let me know if adding comment with this information will be sufficient otherwise I will remove the self_ suffix from the variable name.
I think adding a comment is enough. As far as I know there isn't any
actual restriction in that regard, I just thought it was an odd choice.
The explanation makes sense, thank you!
>
> 4)
>> After some time looking into how GDB calculates names and what not, I don't think calling this "physname" is a good idea, I think it would be better to call it TRAMPOLINE_TARGET_MANGLED_NAME.
>> My reasoning for that is that physname is not some well defined thing, it is a name that a GDB developer came up with to call what happens when GDB needs to calculate mangled names ourselves because the compiler didn't add a linkage name, as far as I understood it. Since the struct trampoline_target doesn't care about how a mangled name was calculated, and mangled name is a well defined thing while physname is not, I think its better to use the latter.
> Since as per (1) comment above we are first going to read mangled and if not then demangled name. So I would then just prefer to call it TRAMPOLINE_TARGET_NAME, hopefully it would be fine with you.
Yeah, TRAMPOLINE_TARGET_NAME works just fine. Thank you!
--
Cheers,
Bruno
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2023-07-31 7:16 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-07-10 22:56 [PATCH v3 0/4] GDB support for DW_AT_trampoline Abdul Basit Ijaz
2023-07-10 22:56 ` [PATCH v3 1/4] gdb, dwarf: add support for DW_AT_trampoline in DWARF reader Abdul Basit Ijaz
2023-07-27 9:09 ` Bruno Larsen
2023-07-29 11:36 ` Ijaz, Abdul B
2023-07-31 7:16 ` Bruno Larsen
2023-07-10 22:56 ` [PATCH v3 2/4] gdb/symtab: add lookup for trampoline functions Abdul Basit Ijaz
2023-07-10 22:56 ` [PATCH v3 3/4] gdb/infrun: handle stepping through functions with DW_AT_trampoline Abdul Basit Ijaz
2023-07-11 11:21 ` Eli Zaretskii
2023-07-27 11:46 ` Bruno Larsen
2023-07-29 11:03 ` Ijaz, Abdul B
2023-07-10 22:56 ` [PATCH v3 4/4] gdb: Skip trampoline frames in the stack for printing or finish command Abdul Basit Ijaz
2023-07-11 11:23 ` Eli Zaretskii
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).