[PATCHv3] Make "skip" work on inline frames

Message ID AM6PR03MB45190FCF5B3282E1D905CA9EE4630@AM6PR03MB4519.eurprd03.prod.outlook.com
State New, archived
Headers

Commit Message

Bernd Edlinger Oct. 31, 2019, 7:19 p.m. UTC
  On 10/31/19 7:00 PM, Pedro Alves wrote:
> On 10/31/19 4:53 PM, Simon Marchi wrote:
>> On 2019-10-31 12:42 p.m., Pedro Alves wrote:
>>> On 10/30/19 9:56 PM, Bernd Edlinger wrote:
>>>> +if { [prepare_for_testing "failed to prepare" "skip2" \
>>>> +                          {skip2.c skip1.c } \
>>>> +                          {debug nowarnings optimize=-O2}] } {
>>>
>>> Instead of -O2, could you make this use -O0 (the default),
>>> and then use attribute((always_inline)) to force inlining? 
>>> We do that in some tests.  E.g., gdb.opt/inline-locals.c.
>>
>> I think that's a good suggestion, but just be aware that there used to be
>> some problems with always_inline, e.g.:
> 
> I don't think always_inline changes anything in the debug info special,
> it just tells the compiler to inline the function even at -O0, which is
> what we're after.
> 
>>
>> https://sourceware.org/bugzilla/show_bug.cgi?id=13263
> 
> This one seems to be about attribute((artificial)), and the desire
> to not step into such functions automatically:
> 
>    "Given that I marked the function always_inline and artificial, I
>     was expecting not to step into its body."
> 
>> https://sourceware.org/bugzilla/show_bug.cgi?id=12429
>>
> 
> This one seems like a generic inlining issue.
> 
>> I'm not sure if those are still valid.  If they are, it might be more difficult
>> that expected to use always_inline.
> I don't think always_inline is anything special compared to inlining
> because of -O2.
> 
> Thanks,
> Pedro Alves
> 

Ah, thanks for that hint!

always_inline works quite well.

The debug session started (using gcc 4.8.4) with -O2 -g on the line with "{" in main,
and with -O0 -g at the first real statement, so I had to remove one step.
I did not worry about it initially, but now I agree that would have caused trouble.


I attached both parts of the patch, the fist part unchanged from previous.
The test case now makes sure to not repeat the names.


Thanks
Bernd.
  

Patch

From d8fc12de59c47c94ef42f91e0793554df1e4d714 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

---
 gdb/testsuite/gdb.base/skip2.c   | 64 ++++++++++++++++++++++++++++++++
 gdb/testsuite/gdb.base/skip2.exp | 80 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 144 insertions(+)
 create mode 100644 gdb/testsuite/gdb.base/skip2.c
 create mode 100644 gdb/testsuite/gdb.base/skip2.exp

diff --git a/gdb/testsuite/gdb.base/skip2.c b/gdb/testsuite/gdb.base/skip2.c
new file mode 100644
index 0000000..d1abd45
--- /dev/null
+++ b/gdb/testsuite/gdb.base/skip2.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 */
+  x = 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/skip2.exp b/gdb/testsuite/gdb.base/skip2.exp
new file mode 100644
index 0000000..a59dd0f
--- /dev/null
+++ b/gdb/testsuite/gdb.base/skip2.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" "skip2" \
+                          {skip2.c skip1.c } \
+                          {debug nowarnings}] } {
+    return -1
+}
+
+set srcfile skip2.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"
+}
-- 
1.9.1