This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCHv2] Make "skip" work on inline frames
- From: Bernd Edlinger <bernd dot edlinger at hotmail dot de>
- To: Simon Marchi <simark at simark dot ca>, "gdb-patches at sourceware dot org" <gdb-patches at sourceware dot org>
- Date: Wed, 30 Oct 2019 21:56:20 +0000
- Subject: Re: [PATCHv2] Make "skip" work on inline frames
- Arc-authentication-results: i=1; mx.microsoft.com 1; spf=none; dmarc=none; dkim=none; arc=none
- Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=mM60yqsq+SGWKPe4Jp3N+kkhBysTBaOKxMrR/nQbN/M=; b=ioL2t2MggA1QKtAWgTccT+3mzhwRFlEO/rETCUD8jQrUZqKzL8DL9FpXm7lgpgWYzbOE3te5P1uIHkovTBBDdJVgP1jEZ+IRJzTROXMqh1Qn2XFBHxi6XKLQNS1u9TJOkh0wJ5rxw0wurP0A1/EAKJ76xM24+QN2m4JE9dflNww/B4X+2VhcCPb2iUh3fPlz2WTZw1U6o4jkpB+CifKddz5EC6lLAazTtsqkBXZV8BmGY+anQqXLM91fz9UyJkzeXPdEkVtg6D/Drs5afGEGWcQfrhCgbzHELFaNUz6YHHLH9l/tlt/DKs87eCRoiBSh5iCgnyvLay1ma20mp/ASlg==
- Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=cJBRBPOVLYLLGnIJhYAYX/E751qC7Nfdr7shk/ui04BXksWzblU+YjtF0w1sl6k+4ZWdzCalplsjO5veZ/uilebaQKCA8EUFEKkK85y6AHIKnBC6r2q7qKqKwYk3GdqKF62O7Imjodld4SM6MbFQgix2F2GGt9vHusbe7k3r3BOoc0QtBEqwQnQeWnVa/QV/bjKiQerXSVpFOoCGMSworjK9exUMEsVoPuJmTALhOydXWtB6uFFF6QYyQF0AMgLzB4g1fBV3+zdS5OPb/9corWuhwssQLiETxefzHpcNIzaneNistD+Tk2Oft4+WFjX1cnTJkHgpwA8pBx0OhNvzqA==
- References: <VI1PR03MB4528B9076FC778EF9C0B8B61E46C0@VI1PR03MB4528.eurprd03.prod.outlook.com> <VI1PR03MB452851603771A9F8A45E74C9E46F0@VI1PR03MB4528.eurprd03.prod.outlook.com> <VI1PR03MB452814244371F04E2848AB68E46E0@VI1PR03MB4528.eurprd03.prod.outlook.com> <8fc93db4-8906-4f4e-53f4-225ebfa0115d@simark.ca> <215bbf9c-4c3c-5cd2-c657-51aa7262f234@simark.ca>
On 10/27/19 3:17 AM, Simon Marchi wrote:
> On 2019-10-26 9:52 p.m., Simon Marchi wrote:
>> On 2019-10-20 2:48 a.m., Bernd Edlinger wrote:
>>> On 10/19/19 6:38 AM, Bernd Edlinger wrote:
>>>> Hmm,
>>>>
>>>> I noticed that the patch does not yet handle
>>>> the step <count> correctly, the count is decremented
>>>> although the inline frame is skipped and should not be
>>>> counted...
>>>>
>>>> Thus I will need to change at least this:
>>>>
>>>> --- a/gdb/infcmd.c
>>>> +++ b/gdb/infcmd.c
>>>> @@ -1122,7 +1122,6 @@ prepare_one_step (struct step_command_fsm *sm)
>>>> set_running (resume_ptid, 1);
>>>>
>>>> step_into_inline_frame (tp);
>>>> - sm->count--;
>>>>
>>>> sal = find_frame_sal (frame);
>>>> sym = get_frame_function (frame);
>>>> @@ -1132,13 +1131,17 @@ prepare_one_step (struct step_command_fsm *sm)
>>>>
>>>> if (sal.line == 0
>>>> || !function_name_is_marked_for_skip (fn, sal))
>>>> - return prepare_one_step (sm);
>>>> + {
>>>> + sm->count--;
>>>> + return prepare_one_step (sm);
>>>> + }
>>>> }
>>>>
>>>>
>>>
>>> Attached is an updated patch that fixes this issue,
>>> and also adds the following after step_into_inline_frame ():
>>>
>>> frame = get_current_frame ();
>>>
>>> That I consider safer, since this function calls reinit_frame_cache ().
>>> It was probably just by chance that this did not seem to cause any
>>> problems for me.
>>>
>>>
>>> Thanks
>>> Bernd.
>>
>> Hi Bernd,
>>
>> Sorry for the delay. I'll start looking at this patch, but I first need to play with
>> it a bit first and get more familiar with that area of the code.
>>
>> In the mean time, I looked for your name in the copyright assignment list, and don't find
>> it. I think this patch is large enough to warrant one Do you already have one in place?
>> If not, please follow instructions here:
>>
>> https://git.savannah.gnu.org/cgit/gnulib.git/tree/doc/Copyright/request-assign.future
>>
>> Simon
>
> Oh, and I noticed that the patch doesn't come with a test, we'll need one before getting
> the patch in. There are already some skip tests at testsuite/gdb.base/skip*.exp, so I
> could very well imagine a new test named gdb.base/skip-inline.exp.
>
> See these pages for details on how to write and run tests:
>
> - https://sourceware.org/gdb/wiki/GDBTestcaseCookbook
> - https://sourceware.org/gdb/wiki/TestingGDB
>
> If you can't manage to make a test, at the very least please provide a minimal reproducer
> so somebody else will be able to translate that into a test.
>
While the legal stuff will probably need more time,
I quickly wrote a test case for this. Hope this helps to understand
how the patch works.
Attached you'll find a test case for the skip of inline functions,
I also added a test for the glitch in the first version of the patch.
(counting step over skipped inlined functions wrong)
Thanks
Bernd.
> Thanks,
>
> Simon
>
From bae4d08c6a69314b0073ee7d93254076e6574e55 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] Add a test case for skip with inlined functions
---
gdb/testsuite/gdb.base/skip2.c | 64 ++++++++++++++++++++++++++++++++++++++
gdb/testsuite/gdb.base/skip2.exp | 67 ++++++++++++++++++++++++++++++++++++++++
2 files changed, 131 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..295aa23
--- /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);
+
+static 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..db40601
--- /dev/null
+++ b/gdb/testsuite/gdb.base/skip2.exp
@@ -0,0 +1,67 @@
+# 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 optimize=-O2}] } {
+ 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 "step" ".*" "step in the main"
+gdb_test "bt" "\\s*\\#0\\s+main.*" "step in the main"
+gdb_test "step" ".*" "step into baz, since foo will be skipped"
+gdb_test "bt" "\\s*\\#0\\s+baz.*" "step into baz, since foo will be skipped"
+gdb_test "step" ".*" "step in the baz"
+gdb_test "bt" "\\s*\\#0\\s+baz.*" "step in the baz"
+gdb_test "step" ".*" "step back to main"
+gdb_test "bt" "\\s*\\#0\\s+main.*" "step back to main"
+gdb_test "step" ".*" "step into baz, since foo will be skipped"
+gdb_test "bt" "\\s*\\#0\\s+baz.*" "step into baz, since foo will be skipped"
+gdb_test "step" ".*" "step in the baz"
+gdb_test "bt" "\\s*\\#0\\s+baz.*" "step in the baz"
+gdb_test "step" ".*" "step back to main"
+gdb_test "bt" "\\s*\\#0\\s+main.*" "step back to main"
+
+if ![runto_main] {
+ fail "can't run to main"
+ return
+}
+
+gdb_test "step" ".*" "step in the main"
+gdb_test "bt" "\\s*\\#0\\s+main.*" "step in the main"
+gdb_test "step 2" ".*" "step into baz, since foo will be skipped"
+gdb_test "bt" "\\s*\\#0\\s+baz.*" "step into baz, since foo will be skipped"
+gdb_test "step" ".*" "step in the baz"
+gdb_test "bt" "\\s*\\#0\\s+main.*" "step back to main"
+gdb_test "step 2" ".*" "step into baz, since foo will be skipped"
+gdb_test "bt" "\\s*\\#0\\s+baz.*" "step into baz, since foo will be skipped"
+gdb_test "step" ".*" "step back to main"
+gdb_test "bt" "\\s*\\#0\\s+main.*" "step back to main"
--
1.9.1