From patchwork Wed Oct 30 21:56:20 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Bernd Edlinger X-Patchwork-Id: 35487 Received: (qmail 124579 invoked by alias); 30 Oct 2019 21:56:26 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 124566 invoked by uid 89); 30 Oct 2019 21:56:26 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-17.5 required=5.0 tests=AWL, BAYES_00, FREEMAIL_FROM, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_SHORT, RCVD_IN_DNSWL_NONE, SPF_HELO_PASS, SPF_PASS autolearn=ham version=3.3.1 spammy=191, (unknown) X-HELO: EUR01-DB5-obe.outbound.protection.outlook.com Received: from mail-oln040092064097.outbound.protection.outlook.com (HELO EUR01-DB5-obe.outbound.protection.outlook.com) (40.92.64.97) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 30 Oct 2019 21:56:24 +0000 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== 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-Authentication-Results: i=1; mx.microsoft.com 1; spf=none; dmarc=none; dkim=none; arc=none Received: from VE1EUR01FT009.eop-EUR01.prod.protection.outlook.com (10.152.2.58) by VE1EUR01HT066.eop-EUR01.prod.protection.outlook.com (10.152.2.191) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384) id 15.20.2387.20; Wed, 30 Oct 2019 21:56:20 +0000 Received: from VI1PR03MB4528.eurprd03.prod.outlook.com (10.152.2.51) by VE1EUR01FT009.mail.protection.outlook.com (10.152.2.141) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.2387.20 via Frontend Transport; Wed, 30 Oct 2019 21:56:20 +0000 Received: from VI1PR03MB4528.eurprd03.prod.outlook.com ([fe80::49b8:a7e6:9f6f:862d]) by VI1PR03MB4528.eurprd03.prod.outlook.com ([fe80::49b8:a7e6:9f6f:862d%3]) with mapi id 15.20.2387.028; Wed, 30 Oct 2019 21:56:20 +0000 From: Bernd Edlinger To: Simon Marchi , "gdb-patches@sourceware.org" Subject: Re: [PATCHv2] Make "skip" work on inline frames Date: Wed, 30 Oct 2019 21:56:20 +0000 Message-ID: References: <8fc93db4-8906-4f4e-53f4-225ebfa0115d@simark.ca> <215bbf9c-4c3c-5cd2-c657-51aa7262f234@simark.ca> In-Reply-To: <215bbf9c-4c3c-5cd2-c657-51aa7262f234@simark.ca> x-microsoft-original-message-id: <2150651f-148f-03dc-e307-9ca33399facd@hotmail.de> x-ms-exchange-purlcount: 3 x-ms-exchange-transport-forked: True MIME-Version: 1.0 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 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 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 . */ + +#include + +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 . + +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