From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 11156 invoked by alias); 7 Feb 2004 04:01:01 -0000 Mailing-List: contact gdb-patches-help@sources.redhat.com; run by ezmlm Precedence: bulk List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sources.redhat.com Received: (qmail 11122 invoked from network); 7 Feb 2004 04:00:56 -0000 Received: from unknown (HELO mwinf0504.wanadoo.fr) (193.252.22.26) by sources.redhat.com with SMTP; 7 Feb 2004 04:00:56 -0000 Received: from takamaka.act-europe.fr (AStDenis-103-1-3-27.w81-249.abo.wanadoo.fr [81.249.113.27]) by mwinf0504.wanadoo.fr (SMTP Server) with ESMTP id BCC461000377; Sat, 7 Feb 2004 05:00:52 +0100 (CET) Received: by takamaka.act-europe.fr (Postfix, from userid 507) id 3895C47D62; Sat, 7 Feb 2004 08:00:49 +0400 (RET) Date: Sat, 07 Feb 2004 04:01:00 -0000 From: Joel Brobecker To: Elena Zannoni Cc: gdb-patches@sources.redhat.com Subject: Re: [RFA] use frame IDs to detect function calls while stepping Message-ID: <20040207040049.GH18961@gnat.com> References: <20040205044119.GC18961@gnat.com> <20040205171324.GF18961@gnat.com> <16418.37058.65446.669052@localhost.redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <16418.37058.65446.669052@localhost.redhat.com> User-Agent: Mutt/1.4i X-SW-Source: 2004-02/txt/msg00144.txt.bz2 Elena, Daniel, > Could this kind of logic be handled inside handle_step_into_function > (which already checks for UNDEBUGGABLE)? Hmmm, I realize I forgot to mention that I'd like to eventually split handle_step_into_function() into several smaller and simpler functions. That is: Now that I have divided a large condition into smaller ones, we might be able to write a smaller function dedicated to each case the smaller tests handle. This is only a wish, as I haven't had time to look more closely into this. But I wanted to keep this option open, which is why I used the UNDEBUGGABLE check in addition to the stop_func_name one. > I.e. is this case caught by the complex condition in the old frame > case which makes you call handle_step_into_function? For the new frame > case, this is regression #1, did you have this bug/regression with > the old frame code? With the old frame code (and hence using the complex condition), the test identified by regression #1 was PASSing. The reason why it was passing was because ecs->stop_func_name == NULL is part of the complex condition. This also emphasizes the fact that I slightly modified the handling of inferior events when I added the STEP_OVER_UNDEBUGGABLE check. This check wasn't there before (in the complex condition), and perhaps we would all feel less nervous if I removed it, and then moved the stop_func_name check next to the other new checks. > > + if (ecs->stop_func_name == NULL > > + && step_over_calls == STEP_OVER_UNDEBUGGABLE) > > + { > > + /* We couldn't determine where we stopped, so we just stepped > > + inside undebuggable code. Since we want to step over this > > + kind of code, we keep going until the inferior returns from > > + the current function. */ > > The test and the comment don't seem to match. Code with only minimal > Symbols will still set stop_func_name. Perhaps the comment is a bit awkward, because I really meant that we don't have any symbol (not even minimal) to determine where we landed. /* There is no symbol, not even a minimal symbol, corresponding to the address where we just stopped. So we just stepped inside undebuggable code. Since we want to step over this kind of code, we keep going until the inferior returns from the current function. */ > > . If we are in a shlib call trampoline, then: > > > > Likewise. > > (This test was already part of the previous check, BTW) > > > > . If we are in a function called from the function where we started > > stepping, as identified by frame ID unwind, then: > > > > Likewise. > > > > I tried it and no longer had any regression. > > > > I think the explanations above should go in the function as comments. That's what I tried to do in the comments I added. Which part would you like to see added, and where would you add it? > > + else > > + { > > + if (IN_SOLIB_CALL_TRAMPOLINE (stop_pc, ecs->stop_func_name)) > > + { > > + /* We landed in a shared library call trampoline, so it > > + is a subroutine call. */ > > + handle_step_into_function (ecs); > > + return; > > + } > > > I am not sure I understand why the case above is not covered by the > test below. Is this to fix regression #1? I.e multiple frames? Hmmm, good question... Although it does fix regression #2, it is not the only reason why we added this test. It was also based on logic (see "After ... here is what we found", in my previous message). I should admit that in the case at hand (regression #2), the unwinder is having a hard time unwinding out of this code, and causes the frame ID check to fail. I don't remember seeing several levels of function call. However, I still thought that this test was necessary because we could just as well have reached this trampoline one or more levels of function call down, just as we end up stepping into undebuggable code in regression #1. -- Joel