From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 27337 invoked by alias); 5 Dec 2003 20:19:13 -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 27323 invoked from network); 5 Dec 2003 20:19:10 -0000 Received: from unknown (HELO localhost.redhat.com) (216.129.200.20) by sources.redhat.com with SMTP; 5 Dec 2003 20:19:10 -0000 Received: from gnu.org (localhost [127.0.0.1]) by localhost.redhat.com (Postfix) with ESMTP id 35BBC2B8F; Fri, 5 Dec 2003 15:19:11 -0500 (EST) Message-ID: <3FD0E83F.30203@gnu.org> Date: Fri, 05 Dec 2003 20:19:00 -0000 From: Andrew Cagney User-Agent: Mozilla/5.0 (X11; U; NetBSD macppc; en-US; rv:1.0.2) Gecko/20030820 X-Accept-Language: en-us, en MIME-Version: 1.0 To: Joel Brobecker Cc: gdb-patches@sources.redhat.com Subject: Re: [RFA] OSF/1 - "next" over prologueless function call References: <20031202042646.GW1186@gnat.com> <3FCD6468.9020708@gnu.org> <20031204005521.GD716@gnat.com> <3FCE92A1.6010007@gnu.org> <20031204232410.GI1652@gnat.com> Content-Type: multipart/mixed; boundary="------------040900090807080104010407" X-SW-Source: 2003-12/txt/msg00223.txt.bz2 This is a multi-part message in MIME format. --------------040900090807080104010407 Content-Type: text/plain; charset=us-ascii; format=flowed Content-Transfer-Encoding: 7bit Content-length: 5495 > (only took 15 years to realise how "obvious" it was :-) > > > Right. It's only "obvious" because the we now have a frame ID which > particularity is to be unique... Given how long it must have taken to > "invent the wheel", maybe we didn't do too bad :-). See the comments in the attached ... >> You can use legacy_frame_p for differentiating old and new code. > > > Yes. Despite the relative success on sparc-solaris (no new regression), > I wasn't sure whether this wasn't potentially introducing new problems > on the architectures where the legacy frames are still used. So I added > it to the condition. > > That gives us a pretty horrible condition, something like that: > > if (((stop_pc == ecs->stop_func_start /* Quick test */ > || (!legacy_frame_pd (current_gdbarch) > && frame_id_eq > (get_frame_id (get_prev_frame (get_current_frame ())), > step_frame_id)) > || in_prologue (stop_pc, ecs->stop_func_start)) > && !IN_SOLIB_RETURN_TRAMPOLINE (stop_pc, ecs->stop_func_name)) > || IN_SOLIB_CALL_TRAMPOLINE (stop_pc, ecs->stop_func_name) > || ecs->stop_func_name == 0) > > How about we move this into a function? There are several things we could move to a function .... > My current version only replaces the condition I added by: > > || frame_in_procedure_called_during_step () > > So the condition stay relatively readable (not too many levels in terms > of nesting). But maybe it's time to turn the entire condition into a > function and add more comments. > > Something like this? > > static int > inside_function_called_from_step (CORE_ADDR stop_pc, > CORE_ADDR stop_func_start) > { > /* If bla bla bla */ > if (stop_pc == stop_func_start > && !IN_SOLIB_RETURN_TRAMPOLINE (stop_pc, stop_func_name)) > return 1; > > /* If the frame ID of the previous frame is equal to step_frame_id, > then this function was indeed called during a step. Only rely > on this test if we don't use the legacy frames. */ > if (!legacy_frame_p (current_gdbarch) > && frame_id_eq (...) > && !IN_SOLIB_RETURN_TRAMPOLINE (stop_pc, stop_func_name)) > return 1; > > /* cagney/2003-12-04: Do we really need this, now that we check the > previous frame id? */ > if (in_prologue (stop_pc, ecs->stop_func_start)) > && !IN_SOLIB_RETURN_TRAMPOLINE (stop_pc, stop_func_name)) > return 1; > > /* Don't stop stepping inside a solib call trampoline. */ > if (IN_SOLIB_CALL_TRAMPOLINE (stop_pc, stop_func_name)) > return 1; > > /* Hmmm, continue stepping if we don't know anything about where > we landed. */ > if (stop_func_name == 0) > return 1; > > return 0; > } > > The trouble is that this change is not completly mechanical. And > we also end up repeating the "!IN_SOLIB_CALL_TRAMPOLINE" test. > We can also coalesce the first 3 tests into one if, but then > we lose a bit the advantage of using a function. There's always plan B. Looking at the body of that IF, I believe it always returns. That should let us do: if (legacy_frame_p ()) if (all the existing tests) call a function to do the body of work () return; else if (our new improved test) call a function to do the body of work () return; that way the legacy and non legacy cases are clearly split - we're free to refine the new conditional with out worrying about breaking the old code. However .... (Oh, what exactly is that body of code doing - any ideas?) > Anyway, it's more a matter of style, so I will defer to the preference > of the maintainers, and send a patch along these lines. If we decide > to move the entire condition into a new function, I can send 2 patches: > One that moves the current condition, and then another that fixes > the osf1 problem. > > >> Hmm, is "stop_pc == ecs->stop_func_start" a valid test, what happens if >> the program is at 1: and there's a next? >> >> foo: >> ... >> 1: goto foo > > > Unless the function is completely prologueless, all should be well > because the goto will branch at a location past the first instruction. > If the function is prologueless, however... I think this would be a rare > occurrence. I'm not so sure :-( In fact any lack of such code suggests a GCC optimizer bug :-) >> Hmm, is in_prologue() adding value when frame_id always works? Unless >> it's being used to handle stepping through a prologue? > > > Not sure about the real intent of in_prologue(). I know it saved us > most of the time in the osf/1 case, but was it the primary intent > when this condition was added? I think that once we have answered > this question, we will be able to decide whether or not this condition > can be removed. ... valid point. Staring at that code its doing so many things its depressing: - step into function - step over function - step out of function - step over trampoline but the basic intent was possibly step into a function and then skip over its prologue? So I'm guessing for the moment just replace stop_pc == ecs->stop_func_start with the frame id test in the new code? :-( > I also realize that I may not have been clear about the usage of > legacy_frame_p. I am a bit nervous at doing without, but you may be > more confident than me, since I don't have much knowledge of the new > frame architecture yet. So the choice of using it or not is still open. Andrew --------------040900090807080104010407 Content-Type: text/plain; name="x" Content-Transfer-Encoding: 7bit Content-Disposition: inline; filename="x" Content-length: 13167 1.155 (grossman 24-Oct-95): #if 0 1.155 (grossman 24-Oct-95): /* I disabled this test because it was too complicated and slow. The 1.155 (grossman 24-Oct-95): SKIP_PROLOGUE was especially slow, because it caused unnecessary 1.155 (grossman 24-Oct-95): prologue examination on various architectures. The code in the #else 1.155 (grossman 24-Oct-95): clause has been tested on the Sparc, Mips, PA, and Power 1.155 (grossman 24-Oct-95): architectures, so it's pretty likely to be correct. -Stu 10/24/95 */ 1.155 (grossman 24-Oct-95): 1.137 (kingdon 08-Oct-94): /* See if we left the step range due to a subroutine call that 1.137 (kingdon 08-Oct-94): we should proceed to the end of. */ 1.137 (kingdon 08-Oct-94): 1.80 (kingdon 11-Jul-93): if (stop_func_start) 1.80 (kingdon 11-Jul-93): { 1.130 (grossman 02-Jun-94): struct symtab *s; 1.130 (grossman 02-Jun-94): 1.80 (kingdon 11-Jul-93): /* Do this after the IN_SIGTRAMP check; it might give 1.80 (kingdon 11-Jul-93): an error. */ 1.80 (kingdon 11-Jul-93): prologue_pc = stop_func_start; 1.130 (grossman 02-Jun-94): 1.130 (grossman 02-Jun-94): /* Don't skip the prologue if this is assembly source */ 1.130 (grossman 02-Jun-94): s = find_pc_symtab (stop_pc); 1.130 (grossman 02-Jun-94): if (s && s->language != language_asm) 1.130 (grossman 02-Jun-94): SKIP_PROLOGUE (prologue_pc); 1.80 (kingdon 11-Jul-93): } 1.1 (rich 28-Mar-91): 1.101 (kingdon 17-Oct-93): if ((/* Might be a non-recursive call. If the symbols are missing 1.101 (kingdon 17-Oct-93): enough that stop_func_start == prev_func_start even though 1.101 (kingdon 17-Oct-93): they are really two functions, we will treat some calls as 1.101 (kingdon 17-Oct-93): jumps. */ 1.101 (kingdon 17-Oct-93): stop_func_start != prev_func_start 1.101 (kingdon 17-Oct-93): 1.101 (kingdon 17-Oct-93): /* Might be a recursive call if either we have a prologue 1.101 (kingdon 17-Oct-93): or the call instruction itself saves the PC on the stack. */ 1.101 (kingdon 17-Oct-93): || prologue_pc != stop_func_start 1.137 (kingdon 08-Oct-94): || read_sp () != step_sp) 1.105 (grossman 26-Oct-93): && (/* PC is completely out of bounds of any known objfiles. Treat 1.105 (grossman 26-Oct-93): like a subroutine call. */ 1.105 (grossman 26-Oct-93): ! stop_func_start 1.101 (kingdon 17-Oct-93): 1.110 (kingdon 30-Dec-93): /* If we do a call, we will be at the start of a function... */ 1.101 (kingdon 17-Oct-93): || stop_pc == stop_func_start 1.37 (bothner 01-Mar-92): 1.110 (kingdon 30-Dec-93): /* ...except on the Alpha with -O (and also Irix 5 and 1.110 (kingdon 30-Dec-93): perhaps others), in which we might call the address 1.110 (kingdon 30-Dec-93): after the load of gp. Since prologues don't contain 1.110 (kingdon 30-Dec-93): calls, we can't return to within one, and we don't 1.110 (kingdon 30-Dec-93): jump back into them, so this check is OK. */ 1.110 (kingdon 30-Dec-93): 1.101 (kingdon 17-Oct-93): || stop_pc < prologue_pc 1.101 (kingdon 17-Oct-93): 1.134 (schauer 15-Jul-94): /* ...and if it is a leaf function, the prologue might 1.134 (schauer 15-Jul-94): consist of gp loading only, so the call transfers to 1.134 (schauer 15-Jul-94): the first instruction after the prologue. */ 1.134 (schauer 15-Jul-94): || (stop_pc == prologue_pc 1.134 (schauer 15-Jul-94): 1.134 (schauer 15-Jul-94): /* Distinguish this from the case where we jump back 1.134 (schauer 15-Jul-94): to the first instruction after the prologue, 1.134 (schauer 15-Jul-94): within a function. */ 1.134 (schauer 15-Jul-94): && stop_func_start != prev_func_start) 1.134 (schauer 15-Jul-94): 1.101 (kingdon 17-Oct-93): /* If we end up in certain places, it means we did a subroutine 1.101 (kingdon 17-Oct-93): call. I'm not completely sure this is necessary now that we 1.101 (kingdon 17-Oct-93): have the above checks with stop_func_start (and now that 1.102 (kingdon 17-Oct-93): find_pc_partial_function is pickier). */ 1.140 (law 24-Nov-94): || IN_SOLIB_CALL_TRAMPOLINE (stop_pc, stop_func_name) 1.101 (kingdon 17-Oct-93): 1.101 (kingdon 17-Oct-93): /* If none of the above apply, it is a jump within a function, 1.101 (kingdon 17-Oct-93): or a return from a subroutine. The other case is longjmp, 1.101 (kingdon 17-Oct-93): which can no longer happen here as long as the 1.101 (kingdon 17-Oct-93): handling_longjmp stuff is working. */ 1.101 (kingdon 17-Oct-93): )) 1.130 (grossman 02-Jun-94): #else 1.130 (grossman 02-Jun-94): /* This is experimental code which greatly simplifies the subroutine call 1.130 (grossman 02-Jun-94): test. I've actually tested on the Alpha, and it works great. -Stu */ 1.130 (grossman 02-Jun-94): 1.155 (grossman 24-Oct-95): if (stop_pc == stop_func_start /* Quick test */ 1.155 (grossman 24-Oct-95): || in_prologue (stop_pc, stop_func_start) 1.155 (grossman 24-Oct-95): || IN_SOLIB_CALL_TRAMPOLINE (stop_pc, stop_func_name) 1.155 (grossman 24-Oct-95): || stop_func_start == 0) 1.130 (grossman 02-Jun-94): #endif 1.155 (grossman 24-Oct-95): 1.80 (kingdon 11-Jul-93): { 1.80 (kingdon 11-Jul-93): /* It's a subroutine call. */ 1.34 (grossman 22-Feb-92): 1.80 (kingdon 11-Jul-93): if (step_over_calls == 0) 1.80 (kingdon 11-Jul-93): { 1.80 (kingdon 11-Jul-93): /* I presume that step_over_calls is only 0 when we're 1.80 (kingdon 11-Jul-93): supposed to be stepping at the assembly language level 1.80 (kingdon 11-Jul-93): ("stepi"). Just stop. */ 1.80 (kingdon 11-Jul-93): stop_step = 1; 1.80 (kingdon 11-Jul-93): break; 1.80 (kingdon 11-Jul-93): } 1.37 (bothner 01-Mar-92): 1.80 (kingdon 11-Jul-93): if (step_over_calls > 0) 1.80 (kingdon 11-Jul-93): /* We're doing a "next". */ 1.80 (kingdon 11-Jul-93): goto step_over_function; 1.80 (kingdon 11-Jul-93): 1.80 (kingdon 11-Jul-93): /* If we are in a function call trampoline (a stub between 1.80 (kingdon 11-Jul-93): the calling routine and the real function), locate the real 1.80 (kingdon 11-Jul-93): function. That's what tells us (a) whether we want to step 1.80 (kingdon 11-Jul-93): into it at all, and (b) what prologue we want to run to 1.80 (kingdon 11-Jul-93): the end of, if we do step into it. */ 1.80 (kingdon 11-Jul-93): tmp = SKIP_TRAMPOLINE_CODE (stop_pc); 1.80 (kingdon 11-Jul-93): if (tmp != 0) 1.80 (kingdon 11-Jul-93): stop_func_start = tmp; 1.80 (kingdon 11-Jul-93): 1.80 (kingdon 11-Jul-93): /* If we have line number information for the function we 1.80 (kingdon 11-Jul-93): are thinking of stepping into, step into it. 1.80 (kingdon 11-Jul-93): 1.80 (kingdon 11-Jul-93): If there are several symtabs at that PC (e.g. with include 1.80 (kingdon 11-Jul-93): files), just want to know whether *any* of them have line 1.80 (kingdon 11-Jul-93): numbers. find_pc_line handles this. */ 1.80 (kingdon 11-Jul-93): { 1.80 (kingdon 11-Jul-93): struct symtab_and_line tmp_sal; 1.77 (kingdon 29-Jun-93): 1.80 (kingdon 11-Jul-93): tmp_sal = find_pc_line (stop_func_start, 0); 1.80 (kingdon 11-Jul-93): if (tmp_sal.line != 0) 1.80 (kingdon 11-Jul-93): goto step_into_function; 1.80 (kingdon 11-Jul-93): } 1.77 (kingdon 29-Jun-93): 1.37 (bothner 01-Mar-92): step_over_function: 1.80 (kingdon 11-Jul-93): /* A subroutine call has happened. */ 1.80 (kingdon 11-Jul-93): { 1.80 (kingdon 11-Jul-93): /* Set a special breakpoint after the return */ 1.80 (kingdon 11-Jul-93): struct symtab_and_line sr_sal; 1.80 (kingdon 11-Jul-93): sr_sal.pc = 1.80 (kingdon 11-Jul-93): ADDR_BITS_REMOVE 1.80 (kingdon 11-Jul-93): (SAVED_PC_AFTER_CALL (get_current_frame ())); 1.80 (kingdon 11-Jul-93): sr_sal.symtab = NULL; 1.80 (kingdon 11-Jul-93): sr_sal.line = 0; 1.80 (kingdon 11-Jul-93): step_resume_breakpoint = 1.80 (kingdon 11-Jul-93): set_momentary_breakpoint (sr_sal, get_current_frame (), 1.80 (kingdon 11-Jul-93): bp_step_resume); 1.137 (kingdon 08-Oct-94): step_resume_breakpoint->frame = step_frame_address; 1.80 (kingdon 11-Jul-93): if (breakpoints_inserted) 1.80 (kingdon 11-Jul-93): insert_breakpoints (); 1.80 (kingdon 11-Jul-93): } 1.80 (kingdon 11-Jul-93): goto keep_going; 1.34 (grossman 22-Feb-92): 1.37 (bothner 01-Mar-92): step_into_function: 1.80 (kingdon 11-Jul-93): /* Subroutine call with source code we should not step over. 1.80 (kingdon 11-Jul-93): Do step to the first line of code in it. */ 1.130 (grossman 02-Jun-94): { 1.130 (grossman 02-Jun-94): struct symtab *s; 1.130 (grossman 02-Jun-94): 1.130 (grossman 02-Jun-94): s = find_pc_symtab (stop_pc); 1.130 (grossman 02-Jun-94): if (s && s->language != language_asm) 1.130 (grossman 02-Jun-94): SKIP_PROLOGUE (stop_func_start); 1.130 (grossman 02-Jun-94): } 1.80 (kingdon 11-Jul-93): sal = find_pc_line (stop_func_start, 0); 1.80 (kingdon 11-Jul-93): /* Use the step_resume_break to step until 1.80 (kingdon 11-Jul-93): the end of the prologue, even if that involves jumps 1.80 (kingdon 11-Jul-93): (as it seems to on the vax under 4.2). */ 1.80 (kingdon 11-Jul-93): /* If the prologue ends in the middle of a source line, 1.111 (schauer 01-Jan-94): continue to the end of that source line (if it is still 1.111 (schauer 01-Jan-94): within the function). Otherwise, just go to end of prologue. */ 1.1 (rich 28-Mar-91): #ifdef PROLOGUE_FIRSTLINE_OVERLAP 1.80 (kingdon 11-Jul-93): /* no, don't either. It skips any code that's 1.80 (kingdon 11-Jul-93): legitimately on the first line. */ 1.1 (rich 28-Mar-91): #else 1.111 (schauer 01-Jan-94): if (sal.end && sal.pc != stop_func_start && sal.end < stop_func_end) 1.80 (kingdon 11-Jul-93): stop_func_start = sal.end; 1.1 (rich 28-Mar-91): #endif 1.37 (bothner 01-Mar-92): 1.80 (kingdon 11-Jul-93): if (stop_func_start == stop_pc) 1.80 (kingdon 11-Jul-93): { 1.80 (kingdon 11-Jul-93): /* We are already there: stop now. */ 1.80 (kingdon 11-Jul-93): stop_step = 1; 1.80 (kingdon 11-Jul-93): break; 1.80 (kingdon 11-Jul-93): } 1.80 (kingdon 11-Jul-93): else 1.80 (kingdon 11-Jul-93): /* Put the step-breakpoint there and go until there. */ 1.80 (kingdon 11-Jul-93): { 1.80 (kingdon 11-Jul-93): struct symtab_and_line sr_sal; 1.80 (kingdon 11-Jul-93): 1.80 (kingdon 11-Jul-93): sr_sal.pc = stop_func_start; 1.80 (kingdon 11-Jul-93): sr_sal.symtab = NULL; 1.80 (kingdon 11-Jul-93): sr_sal.line = 0; 1.80 (kingdon 11-Jul-93): /* Do not specify what the fp should be when we stop 1.80 (kingdon 11-Jul-93): since on some machines the prologue 1.80 (kingdon 11-Jul-93): is where the new fp value is established. */ 1.80 (kingdon 11-Jul-93): step_resume_breakpoint = 1.89 (kingdon 18-Sep-93): set_momentary_breakpoint (sr_sal, NULL, bp_step_resume); 1.80 (kingdon 11-Jul-93): if (breakpoints_inserted) 1.80 (kingdon 11-Jul-93): insert_breakpoints (); 1.80 (kingdon 11-Jul-93): 1.80 (kingdon 11-Jul-93): /* And make sure stepping stops right away then. */ 1.80 (kingdon 11-Jul-93): step_range_end = step_range_start; 1.1 (rich 28-Mar-91): } 1.80 (kingdon 11-Jul-93): goto keep_going; 1.80 (kingdon 11-Jul-93): } --------------040900090807080104010407--