[PATCHv5] Make "skip" work on inline frames

Message ID VI1PR08MB532540E290894F9CC98BF87EE4430@VI1PR08MB5325.eurprd08.prod.outlook.com
State New
Headers show
Series
  • [PATCHv5] Make "skip" work on inline frames
Related show

Commit Message

Bernd Edlinger Dec. 2, 2019, 4:47 p.m.
On 12/2/19 3:34 AM, Simon Marchi wrote:
> On 2019-11-24 6:22 a.m., Bernd Edlinger wrote:

>> This is just a minor update on the patch

>> since the function SYMBOL_PRINT_NAME was removed with

>> commit 987012b89bce7f6385ed88585547f852a8005a3f

>> I replaced it with sym->print_name (), otherwise the

>> patch is unchanged.

> 

> Hi Bernd,

> 

> Sorry, I had lost this in the mailing list noise.

> 

> I played a bit with the patch and different cases of figure.  I am not able to understand

> the purpose of each of your changes (due to the complexity of that particular code), but

> I didn't find anything that stood out as wrong to me.  Pedro might be able to do a more

> in-depth review of the event handling code.

> 

> If the test tests specifically skipping of inline functions, I'd name it something more

> descriptive than "skip2.exp", maybe "skip-inline.exp"?

> 

> Unfortunately, your test doesn't pass on my computer (gcc 9.2.0), but neither does the

> gdb.base/skip.exp.  I am attaching the gdb.log when running your test, if it can help.

> 

> Simon

> 


Hi Simon,

I only tested that with gcc-4.8, and both test cases worked with that gcc version.

I tried now with gcc-trunk version from a few days ago, and I think I see
what you mean.

skip2.c (now skip-inline.c) can be fixed by removing the assignment
to x in the first line, which is superfluous (and copied from skip.c).
But skip.c cannot be fixed this way.  I only see a chance to allow
the stepping back to main and then to foo happen.

Does this modified test case work for you?



Thanks
Bernd.

Comments

Simon Marchi Dec. 3, 2019, 4:22 a.m. | #1
On 2019-12-02 11:47 a.m., Bernd Edlinger wrote:
> I only tested that with gcc-4.8, and both test cases worked with that gcc version.

> 

> I tried now with gcc-trunk version from a few days ago, and I think I see

> what you mean.

> 

> skip2.c (now skip-inline.c) can be fixed by removing the assignment

> to x in the first line, which is superfluous (and copied from skip.c).

> But skip.c cannot be fixed this way.  I only see a chance to allow

> the stepping back to main and then to foo happen.

> 

> Does this modified test case work for you?


Yes, I confirm it passes now, thanks!

Simon
Bernd Edlinger Dec. 14, 2019, 1:54 p.m. | #2
Ping...

The latest version of this patch can be found here:
https://sourceware.org/ml/gdb-patches/2019-12/msg00047.html


Thanks
Bernd.

On 12/2/19 5:47 PM, Bernd Edlinger wrote:
> On 12/2/19 3:34 AM, Simon Marchi wrote:

>> On 2019-11-24 6:22 a.m., Bernd Edlinger wrote:

>>> This is just a minor update on the patch

>>> since the function SYMBOL_PRINT_NAME was removed with

>>> commit 987012b89bce7f6385ed88585547f852a8005a3f

>>> I replaced it with sym->print_name (), otherwise the

>>> patch is unchanged.

>>

>> Hi Bernd,

>>

>> Sorry, I had lost this in the mailing list noise.

>>

>> I played a bit with the patch and different cases of figure.  I am not able to understand

>> the purpose of each of your changes (due to the complexity of that particular code), but

>> I didn't find anything that stood out as wrong to me.  Pedro might be able to do a more

>> in-depth review of the event handling code.

>>

>> If the test tests specifically skipping of inline functions, I'd name it something more

>> descriptive than "skip2.exp", maybe "skip-inline.exp"?

>>

>> Unfortunately, your test doesn't pass on my computer (gcc 9.2.0), but neither does the

>> gdb.base/skip.exp.  I am attaching the gdb.log when running your test, if it can help.

>>

>> Simon

>>

> 

> Hi Simon,

> 

> I only tested that with gcc-4.8, and both test cases worked with that gcc version.

> 

> I tried now with gcc-trunk version from a few days ago, and I think I see

> what you mean.

> 

> skip2.c (now skip-inline.c) can be fixed by removing the assignment

> to x in the first line, which is superfluous (and copied from skip.c).

> But skip.c cannot be fixed this way.  I only see a chance to allow

> the stepping back to main and then to foo happen.

> 

> Does this modified test case work for you?

> 

> 

> 

> Thanks

> Bernd.

>

Patch

From 32627e64b9b7b437c5f6d46d1138ecba09816273 Mon Sep 17 00:00:00 2001
From: Bernd Edlinger <bernd.edlinger@hotmail.de>
Date: Wed, 30 Oct 2019 21:35:22 +0100
Subject: [PATCH 2/2] Add a test case for skip with inlined functions

Fixed an issue in skip.exp that made it fail with recent
gcc versions, while already at it.
---
 gdb/testsuite/gdb.base/skip-inline.c   | 64 +++++++++++++++++++++++++++
 gdb/testsuite/gdb.base/skip-inline.exp | 80 ++++++++++++++++++++++++++++++++++
 gdb/testsuite/gdb.base/skip.exp        | 12 +++--
 3 files changed, 153 insertions(+), 3 deletions(-)
 create mode 100644 gdb/testsuite/gdb.base/skip-inline.c
 create mode 100644 gdb/testsuite/gdb.base/skip-inline.exp

diff --git a/gdb/testsuite/gdb.base/skip-inline.c b/gdb/testsuite/gdb.base/skip-inline.c
new file mode 100644
index 0000000..e562149
--- /dev/null
+++ b/gdb/testsuite/gdb.base/skip-inline.c
@@ -0,0 +1,64 @@ 
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2019 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/>.  */
+
+#include <stdlib.h>
+
+int bar (void);
+int baz (int);
+void skip1_test_skip_file_and_function (void);
+void test_skip_file_and_function (void);
+
+__attribute__((__always_inline__)) static inline int
+foo (void)
+{
+  return bar ();
+}
+
+int
+main ()
+{
+  volatile int x;
+
+  /* step immediately into the inlined code */
+  baz (foo ());
+
+  /* step first over non-inline code, this involves a different code path */
+  x = 0; x = baz (foo ());
+
+  test_skip_file_and_function ();
+
+  return 0;
+}
+
+static void
+test_skip (void)
+{
+}
+
+static void
+end_test_skip_file_and_function (void)
+{
+  abort ();
+}
+
+void
+test_skip_file_and_function (void)
+{
+  test_skip ();
+  skip1_test_skip_file_and_function ();
+  end_test_skip_file_and_function ();
+}
diff --git a/gdb/testsuite/gdb.base/skip-inline.exp b/gdb/testsuite/gdb.base/skip-inline.exp
new file mode 100644
index 0000000..9d490bd
--- /dev/null
+++ b/gdb/testsuite/gdb.base/skip-inline.exp
@@ -0,0 +1,80 @@ 
+#   Copyright 2019 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/>.
+
+load_lib completion-support.exp
+
+standard_testfile
+
+if { [prepare_for_testing "failed to prepare" "skip-inline" \
+                          {skip-inline.c skip1.c } \
+                          {debug nowarnings}] } {
+    return -1
+}
+
+set srcfile skip-inline.c
+set srcfile1 skip1.c
+
+if ![runto_main] {
+    fail "can't run to main"
+    return
+}
+
+# Create a skiplist entry for a specified file and function.
+
+gdb_test "skip function foo" "Function foo will be skipped when stepping\."
+
+gdb_test "bt" "\\s*\\#0\\s+main.*" "in the main"
+gdb_test "step" ".*" "step into baz, since foo will be skipped"
+gdb_test "bt" "\\s*\\#0\\s+baz.*" "in the baz, since foo was skipped"
+gdb_test "step" ".*" "step in the baz"
+gdb_test "bt" "\\s*\\#0\\s+baz.*" "still in the baz"
+gdb_test "step" ".*" "step back to main"
+gdb_test "bt" "\\s*\\#0\\s+main.*" "again in the main"
+gdb_test "step" ".*" "step again into baz, since foo will be skipped"
+gdb_test "bt" "\\s*\\#0\\s+baz.*" "again in the baz"
+gdb_test "step" ".*" "step in the baz, again"
+gdb_test "bt" "\\s*\\#0\\s+baz.*" "still in the baz, again"
+gdb_test "step" ".*" "step back to main, again"
+gdb_test "bt" "\\s*\\#0\\s+main.*" "again back to main"
+
+if ![runto_main] {
+    fail "can't run to main"
+    return
+}
+
+with_test_prefix "double step" {
+    gdb_test "bt" "\\s*\\#0\\s+main.*" "in the main"
+    gdb_test "step 2" ".*" "step into baz, since foo will be skipped"
+    gdb_test "bt" "\\s*\\#0\\s+baz.*" "still in the baz"
+    gdb_test "step" ".*" "step back to main"
+    gdb_test "bt" "\\s*\\#0\\s+main.*" "again in the main"
+    gdb_test "step 2" ".*" "step again into baz, since foo will be skipped"
+    gdb_test "bt" "\\s*\\#0\\s+baz.*" "again in the baz"
+    gdb_test "step" ".*" "step back to main, again"
+    gdb_test "bt" "\\s*\\#0\\s+main.*" "again back to main"
+}
+
+if ![runto_main] {
+    fail "can't run to main"
+    return
+}
+
+with_test_prefix "triple step" {
+    gdb_test "bt" "\\s*\\#0\\s+main.*" "in the main"
+    gdb_test "step 3" ".*" "step over baz"
+    gdb_test "bt" "\\s*\\#0\\s+main.*" "again in the main"
+    gdb_test "step 3" ".*" "step over baz, again"
+    gdb_test "bt" "\\s*\\#0\\s+main.*" "again back to main"
+}
diff --git a/gdb/testsuite/gdb.base/skip.exp b/gdb/testsuite/gdb.base/skip.exp
index d763194..0fd44e0 100644
--- a/gdb/testsuite/gdb.base/skip.exp
+++ b/gdb/testsuite/gdb.base/skip.exp
@@ -142,7 +142,9 @@  with_test_prefix "step after disabling 3" {
 
     gdb_test "step" "bar \\(\\) at.*" "step 1"
     gdb_test "step" ".*" "step 2"; # Return from foo()
-    gdb_test "step" "foo \\(\\) at.*" "step 3"
+    # with recent gcc we jump once back to main before entering foo here
+    # if that happens try to step a second time
+    gdb_test "step" "foo \\(\\) at.*" "step 3" "main \\(\\) at .*" "step"
     gdb_test "step" ".*" "step 4"; # Return from bar()
     gdb_test "step" "main \\(\\) at.*" "step 5"
 }
@@ -261,7 +263,9 @@  with_test_prefix "step using -fu for baz" {
     gdb_test_no_output "skip enable 7"
     gdb_test "step" "bar \\(\\) at.*" "step 1"
     gdb_test "step" ".*" "step 2"; # Return from bar()
-    gdb_test "step" "foo \\(\\) at.*" "step 3"
+    # with recent gcc we jump once back to main before entering foo here
+    # if that happens try to step a second time
+    gdb_test "step" "foo \\(\\) at.*" "step 3" "main \\(\\) at.*" "step"
     gdb_test "step" ".*" "step 4"; # Return from foo()
     gdb_test "step" "main \\(\\) at.*" "step 5"
 }
@@ -276,7 +280,9 @@  with_test_prefix "step using -rfu for baz" {
     gdb_test_no_output "skip enable 8"
     gdb_test "step" "bar \\(\\) at.*" "step 1"
     gdb_test "step" ".*" "step 2"; # Return from bar()
-    gdb_test "step" "foo \\(\\) at.*" "step 3"
+    # with recent gcc we jump once back to main before entering foo here
+    # if that happens try to step a second time
+    gdb_test "step" "foo \\(\\) at.*" "step 3" "main \\(\\) at.*" "step"
     gdb_test "step" ".*" "step 4"; # Return from foo()
     gdb_test "step" "main \\(\\) at.*" "step 5"
 }
-- 
1.9.1