RE: [PATCH v6 03/10] gdb: handle stepping through functions with DW_AT_trampoline

["Ijaz, Abdul B" <abdul.b.ijaz@intel.com>]

Thanks a lot Eli for the feedback. Will wait for the overall feedback and then will fix the issues you mentioned in next series for this patch.

>> +  If this is turned off, GDB gdb will handle trampoline functions the 
>> + same
                            ^^^^^^^
>Redundant "gdb" there.

Will remove "gdb".

>> +some compilers might generate so-called trampoline functions, which 
>> +wrap the actual function call (the

>"trampoline functions" should be in @dfn, as you are introducing new terminology here.

Wil add @dfn for "trampoline functions".

>> +ignore such function\n\ trampolines."),

>In the last sentence, I think "ignore such function trampolines" is misleading.  I suggest to replace this with "treat trampolines as normal functions".
Will update accordingly.

Thanks & Best Regards
Abdul Basit


-----Original Message-----
From: Eli Zaretskii <eliz@gnu.org> 
Sent: Thursday, March 28, 2024 2:59 PM
To: Ijaz, Abdul B <abdul.b.ijaz@intel.com>
Cc: gdb-patches@sourceware.org; JiniSusan.George@amd.com; tom@tromey.com; nils-christian.kempke@intel.com
Subject: Re: [PATCH v6 03/10] gdb: handle stepping through functions with DW_AT_trampoline

> From: Abdul Basit Ijaz <abdul.b.ijaz@intel.com>
> Cc: abdul.b.ijaz@intel.com,
> 	JiniSusan.George@amd.com,
> 	tom@tromey.com,
> 	eliz@gnu.org,
> 	Nils-Christian Kempke <nils-christian.kempke@intel.com>
> Date: Thu, 28 Mar 2024 13:05:21 +0100
> 
>  gdb/NEWS                                      |  11 +
>  gdb/doc/gdb.texinfo                           |  35 +++
>  gdb/infrun.c                                  |  87 ++++++-
>  gdb/infrun.h                                  |   5 +
>  .../gdb.dwarf2/dw2-function-trampolines.c     |  80 ++++++
>  .../gdb.dwarf2/dw2-function-trampolines.exp   | 245 ++++++++++++++++++
>  6 files changed, 459 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.

> diff --git a/gdb/NEWS b/gdb/NEWS
> index feb3a37393a..da0d936ff70 100644
> --- a/gdb/NEWS
> +++ b/gdb/NEWS
> @@ -892,6 +892,17 @@ 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 
> +will
> +  recognize function calls that have been marked as trampolines in 
> +the debug
> +  info.  It improves stepping behavior in that it steps over the 
> +trampoline
> +  code and hides it from the user.  Currently, only DWARF trampolines 
> +are
> +  supported.
> +
> +  If this is turned off, GDB gdb will handle trampoline functions the 
> + same
                            ^^^^^^^
Redundant "gdb" there.

> +@kindex set skip-trampoline-functions @item set 
> +skip-trampoline-functions @cindex trampoline functions @cindex 
> +stepping through trampoline functions @itemx set 
> +skip-trampoline-functions on When calling a function in any language, 
> +some compilers might generate so-called trampoline functions, which 
> +wrap the actual function call (the

"trampoline functions" should be in @dfn, as you are introducing new terminology here.

> +  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."),

In the last sentence, I think "ignore such function trampolines" is misleading.  I suggest to replace this with "treat trampolines as normal functions".

Reviewed-By: Eli Zaretskii <eliz@gnu.org>
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