Make skip without argument skip the current inline function

Message ID AM0PR08MB37141AC1C52E85784FD4BD77E4250@AM0PR08MB3714.eurprd08.prod.outlook.com
State New, archived
Headers

Commit Message

Bernd Edlinger Dec. 28, 2019, 1:30 p.m. UTC
  Hi,

this would in a way complete the skip inlined function feature.
That is make the skip command, when used without arguments, figure
out if there is an inlined function currently executing, and skip
that in future.

Well, not sure if that is worth a mention in the NEWS, maybe when the
remaining patch 'Fix an issue with the gdb step-over aka. "n" command'
which prevents accidentally stepping over a possibly skipped inline
function accidentally to step back into the inline function again.


gdb:
2019-12-28  Bernd Edlinger  <bernd.edlinger@hotmail.de>

	* skip.c (skip_function_command): Make skip w/o arguments use the
	name of the inlined function if pc is inside any inlined function.

gdb/testsuite:
2019-12-28  Bernd Edlinger  <bernd.edlinger@hotmail.de>

	* gdb.base/skip-inline.exp: Extend test.


Thanks
Bernd.


From 75d3db50829ceac30fd92124b2df9ea730bf71de Mon Sep 17 00:00:00 2001
From: Bernd Edlinger <bernd.edlinger@hotmail.de>
Date: Wed, 25 Dec 2019 16:35:32 +0100
Subject: [PATCH] Make skip without argument skip the current inline function

Previously always the outermost function block was used, but
since skip is now able to skip over inline functions it is more
natural to skip the inline function that the program is currently
executing.
---
 gdb/skip.c                             | 17 +++++++----------
 gdb/testsuite/gdb.base/skip-inline.exp | 14 ++++++++++++++
 2 files changed, 21 insertions(+), 10 deletions(-)
  

Comments

Bernd Edlinger Jan. 6, 2020, 8:07 a.m. UTC | #1
Hi,

I'd like to ping for this patch here:
https://sourceware.org/ml/gdb-patches/2019-12/msg01056.html

Thanks
Bernd.

On 12/28/19 2:30 PM, Bernd Edlinger wrote:
> Hi,
> 
> this would in a way complete the skip inlined function feature.
> That is make the skip command, when used without arguments, figure
> out if there is an inlined function currently executing, and skip
> that in future.
> 
> Well, not sure if that is worth a mention in the NEWS, maybe when the
> remaining patch 'Fix an issue with the gdb step-over aka. "n" command'
> which prevents accidentally stepping over a possibly skipped inline
> function accidentally to step back into the inline function again.
> 
> 
> gdb:
> 2019-12-28  Bernd Edlinger  <bernd.edlinger@hotmail.de>
> 
> 	* skip.c (skip_function_command): Make skip w/o arguments use the
> 	name of the inlined function if pc is inside any inlined function.
> 
> gdb/testsuite:
> 2019-12-28  Bernd Edlinger  <bernd.edlinger@hotmail.de>
> 
> 	* gdb.base/skip-inline.exp: Extend test.
> 
> 
> Thanks
> Bernd.
> 
> 
> From 75d3db50829ceac30fd92124b2df9ea730bf71de Mon Sep 17 00:00:00 2001
> From: Bernd Edlinger <bernd.edlinger@hotmail.de>
> Date: Wed, 25 Dec 2019 16:35:32 +0100
> Subject: [PATCH] Make skip without argument skip the current inline function
> 
> Previously always the outermost function block was used, but
> since skip is now able to skip over inline functions it is more
> natural to skip the inline function that the program is currently
> executing.
> ---
>  gdb/skip.c                             | 17 +++++++----------
>  gdb/testsuite/gdb.base/skip-inline.exp | 14 ++++++++++++++
>  2 files changed, 21 insertions(+), 10 deletions(-)
> 
> diff --git a/gdb/skip.c b/gdb/skip.c
> index a869aaa..9f2e5c6 100644
> --- a/gdb/skip.c
> +++ b/gdb/skip.c
> @@ -209,18 +209,15 @@ skip_function_command (const char *arg, int from_tty)
>    /* Default to the current function if no argument is given.  */
>    if (arg == NULL)
>      {
> +      frame_info *fi = get_selected_frame (_("No default function now."));
> +      struct symbol *sym = get_frame_function (fi);
>        const char *name = NULL;
> -      CORE_ADDR pc;
>  
> -      if (!last_displayed_sal_is_valid ())
> -	error (_("No default function now."));
> -
> -      pc = get_last_displayed_addr ();
> -      if (!find_pc_partial_function (pc, &name, NULL, NULL))
> -	{
> -	  error (_("No function found containing current program point %s."),
> -		  paddress (get_current_arch (), pc));
> -	}
> +      if (sym != NULL)
> +	name = sym->print_name ();
> +      else
> +	error (_("No function found containing current program point %s."),
> +	       paddress (get_current_arch (), get_frame_pc (fi)));
>        skip_function (name);
>        return;
>      }
> diff --git a/gdb/testsuite/gdb.base/skip-inline.exp b/gdb/testsuite/gdb.base/skip-inline.exp
> index 4ab9ecf..c1a0556 100644
> --- a/gdb/testsuite/gdb.base/skip-inline.exp
> +++ b/gdb/testsuite/gdb.base/skip-inline.exp
> @@ -76,3 +76,17 @@ with_test_prefix "triple step" {
>      gdb_test "step 3" ".*" "step over baz, again"
>      gdb_test "bt" "\\s*\\#0\\s+main.*" "again back to main"
>  }
> +
> +if ![runto_main] {
> +    fail "can't run to main"
> +    return
> +}
> +
> +gdb_test "skip delete" ".*" "skip delete"
> +
> +with_test_prefix "skip current frame" {
> +    gdb_test "bt" "\\s*\\#0\\s+main.*" "in the main"
> +    gdb_test "step" ".*" "step into foo"
> +    gdb_test "bt" "\\s*\\#0\\s+foo.*" "in the foo"
> +    gdb_test "skip" "Function foo will be skipped when stepping\." "skip"
> +}
>
  
Bernd Edlinger Jan. 12, 2020, 8:13 a.m. UTC | #2
Ping...

On 1/6/20 9:07 AM, Bernd Edlinger wrote:
> Hi,
> 
> I'd like to ping for this patch here:
> https://sourceware.org/ml/gdb-patches/2019-12/msg01056.html
> 
> Thanks
> Bernd.
> 
> On 12/28/19 2:30 PM, Bernd Edlinger wrote:
>> Hi,
>>
>> this would in a way complete the skip inlined function feature.
>> That is make the skip command, when used without arguments, figure
>> out if there is an inlined function currently executing, and skip
>> that in future.
>>
>> Well, not sure if that is worth a mention in the NEWS, maybe when the
>> remaining patch 'Fix an issue with the gdb step-over aka. "n" command'
>> which prevents accidentally stepping over a possibly skipped inline
>> function accidentally to step back into the inline function again.
>>
>>
>> gdb:
>> 2019-12-28  Bernd Edlinger  <bernd.edlinger@hotmail.de>
>>
>> 	* skip.c (skip_function_command): Make skip w/o arguments use the
>> 	name of the inlined function if pc is inside any inlined function.
>>
>> gdb/testsuite:
>> 2019-12-28  Bernd Edlinger  <bernd.edlinger@hotmail.de>
>>
>> 	* gdb.base/skip-inline.exp: Extend test.
>>
>>
>> Thanks
>> Bernd.
>>
>>
>> From 75d3db50829ceac30fd92124b2df9ea730bf71de Mon Sep 17 00:00:00 2001
>> From: Bernd Edlinger <bernd.edlinger@hotmail.de>
>> Date: Wed, 25 Dec 2019 16:35:32 +0100
>> Subject: [PATCH] Make skip without argument skip the current inline function
>>
>> Previously always the outermost function block was used, but
>> since skip is now able to skip over inline functions it is more
>> natural to skip the inline function that the program is currently
>> executing.
>> ---
>>  gdb/skip.c                             | 17 +++++++----------
>>  gdb/testsuite/gdb.base/skip-inline.exp | 14 ++++++++++++++
>>  2 files changed, 21 insertions(+), 10 deletions(-)
>>
>> diff --git a/gdb/skip.c b/gdb/skip.c
>> index a869aaa..9f2e5c6 100644
>> --- a/gdb/skip.c
>> +++ b/gdb/skip.c
>> @@ -209,18 +209,15 @@ skip_function_command (const char *arg, int from_tty)
>>    /* Default to the current function if no argument is given.  */
>>    if (arg == NULL)
>>      {
>> +      frame_info *fi = get_selected_frame (_("No default function now."));
>> +      struct symbol *sym = get_frame_function (fi);
>>        const char *name = NULL;
>> -      CORE_ADDR pc;
>>  
>> -      if (!last_displayed_sal_is_valid ())
>> -	error (_("No default function now."));
>> -
>> -      pc = get_last_displayed_addr ();
>> -      if (!find_pc_partial_function (pc, &name, NULL, NULL))
>> -	{
>> -	  error (_("No function found containing current program point %s."),
>> -		  paddress (get_current_arch (), pc));
>> -	}
>> +      if (sym != NULL)
>> +	name = sym->print_name ();
>> +      else
>> +	error (_("No function found containing current program point %s."),
>> +	       paddress (get_current_arch (), get_frame_pc (fi)));
>>        skip_function (name);
>>        return;
>>      }
>> diff --git a/gdb/testsuite/gdb.base/skip-inline.exp b/gdb/testsuite/gdb.base/skip-inline.exp
>> index 4ab9ecf..c1a0556 100644
>> --- a/gdb/testsuite/gdb.base/skip-inline.exp
>> +++ b/gdb/testsuite/gdb.base/skip-inline.exp
>> @@ -76,3 +76,17 @@ with_test_prefix "triple step" {
>>      gdb_test "step 3" ".*" "step over baz, again"
>>      gdb_test "bt" "\\s*\\#0\\s+main.*" "again back to main"
>>  }
>> +
>> +if ![runto_main] {
>> +    fail "can't run to main"
>> +    return
>> +}
>> +
>> +gdb_test "skip delete" ".*" "skip delete"
>> +
>> +with_test_prefix "skip current frame" {
>> +    gdb_test "bt" "\\s*\\#0\\s+main.*" "in the main"
>> +    gdb_test "step" ".*" "step into foo"
>> +    gdb_test "bt" "\\s*\\#0\\s+foo.*" "in the foo"
>> +    gdb_test "skip" "Function foo will be skipped when stepping\." "skip"
>> +}
>>
  
Simon Marchi Jan. 14, 2020, 4:11 a.m. UTC | #3
On 2019-12-28 8:30 a.m., Bernd Edlinger wrote:
> Hi,
> 
> this would in a way complete the skip inlined function feature.
> That is make the skip command, when used without arguments, figure
> out if there is an inlined function currently executing, and skip
> that in future.
> 
> Well, not sure if that is worth a mention in the NEWS, maybe when the
> remaining patch 'Fix an issue with the gdb step-over aka. "n" command'
> which prevents accidentally stepping over a possibly skipped inline
> function accidentally to step back into the inline function again.
> 
> 
> gdb:
> 2019-12-28  Bernd Edlinger  <bernd.edlinger@hotmail.de>
> 
> 	* skip.c (skip_function_command): Make skip w/o arguments use the
> 	name of the inlined function if pc is inside any inlined function.
> 
> gdb/testsuite:
> 2019-12-28  Bernd Edlinger  <bernd.edlinger@hotmail.de>
> 
> 	* gdb.base/skip-inline.exp: Extend test.
> 
> 
> Thanks
> Bernd.

Hi Bernd,

In my opinion, the current behavior is simply a bug.  As a user, I would expect
the selected frame to be used, regardless of if it's an inline frame or not.
Therefore I don't think we should put a NEWS entry (we don't do NEWS entries for
bug fixes).

The patch looks good to me though, thanks for that.

Simon
  

Patch

diff --git a/gdb/skip.c b/gdb/skip.c
index a869aaa..9f2e5c6 100644
--- a/gdb/skip.c
+++ b/gdb/skip.c
@@ -209,18 +209,15 @@  skip_function_command (const char *arg, int from_tty)
   /* Default to the current function if no argument is given.  */
   if (arg == NULL)
     {
+      frame_info *fi = get_selected_frame (_("No default function now."));
+      struct symbol *sym = get_frame_function (fi);
       const char *name = NULL;
-      CORE_ADDR pc;
 
-      if (!last_displayed_sal_is_valid ())
-	error (_("No default function now."));
-
-      pc = get_last_displayed_addr ();
-      if (!find_pc_partial_function (pc, &name, NULL, NULL))
-	{
-	  error (_("No function found containing current program point %s."),
-		  paddress (get_current_arch (), pc));
-	}
+      if (sym != NULL)
+	name = sym->print_name ();
+      else
+	error (_("No function found containing current program point %s."),
+	       paddress (get_current_arch (), get_frame_pc (fi)));
       skip_function (name);
       return;
     }
diff --git a/gdb/testsuite/gdb.base/skip-inline.exp b/gdb/testsuite/gdb.base/skip-inline.exp
index 4ab9ecf..c1a0556 100644
--- a/gdb/testsuite/gdb.base/skip-inline.exp
+++ b/gdb/testsuite/gdb.base/skip-inline.exp
@@ -76,3 +76,17 @@  with_test_prefix "triple step" {
     gdb_test "step 3" ".*" "step over baz, again"
     gdb_test "bt" "\\s*\\#0\\s+main.*" "again back to main"
 }
+
+if ![runto_main] {
+    fail "can't run to main"
+    return
+}
+
+gdb_test "skip delete" ".*" "skip delete"
+
+with_test_prefix "skip current frame" {
+    gdb_test "bt" "\\s*\\#0\\s+main.*" "in the main"
+    gdb_test "step" ".*" "step into foo"
+    gdb_test "bt" "\\s*\\#0\\s+foo.*" "in the foo"
+    gdb_test "skip" "Function foo will be skipped when stepping\." "skip"
+}