From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 7884 invoked by alias); 27 Oct 2019 02:18:03 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 5978 invoked by uid 89); 27 Oct 2019 02:17:34 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-10.9 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_2,GIT_PATCH_3,SPF_HELO_PASS,SPF_PASS autolearn=ham version=3.3.1 spammy=somebody X-HELO: simark.ca Received: from simark.ca (HELO simark.ca) (158.69.221.121) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Sun, 27 Oct 2019 02:17:31 +0000 Received: from [10.0.0.11] (unknown [192.222.164.54]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by simark.ca (Postfix) with ESMTPSA id 16B391E592; Sat, 26 Oct 2019 22:17:30 -0400 (EDT) Subject: Re: [PATCHv2] Make "skip" work on inline frames From: Simon Marchi To: Bernd Edlinger , "gdb-patches@sourceware.org" References: <8fc93db4-8906-4f4e-53f4-225ebfa0115d@simark.ca> Message-ID: <215bbf9c-4c3c-5cd2-c657-51aa7262f234@simark.ca> Date: Sun, 27 Oct 2019 02:18:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.1.1 MIME-Version: 1.0 In-Reply-To: <8fc93db4-8906-4f4e-53f4-225ebfa0115d@simark.ca> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit X-SW-Source: 2019-10/txt/msg00946.txt.bz2 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. Thanks, Simon