Patchwork [PATCHv5] Make "skip" work on inline frames

login
register
mail settings
Submitter Bernd Edlinger
Date Dec. 2, 2019, 4:47 p.m.
Message ID <VI1PR08MB532540E290894F9CC98BF87EE4430@VI1PR08MB5325.eurprd08.prod.outlook.com>
Download mbox | patch
Permalink /patch/36437/
State New
Headers show

Comments

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.
Simon Marchi - Dec. 3, 2019, 4:22 a.m.
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.
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