From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Mailing-List: contact gdb-help@sources.redhat.com; run by ezmlm Received: (qmail 24757 invoked from network); 9 Jan 2003 17:14:03 -0000 Received: from unknown (HELO crack.them.org) (65.125.64.184) by 209.249.29.67 with SMTP; 9 Jan 2003 17:14:03 -0000 Received: from nevyn.them.org ([66.93.61.169] ident=mail) by crack.them.org with asmtp (Exim 3.12 #1 (Debian)) id 18Wi8L-0002Iq-00; Thu, 09 Jan 2003 13:14:29 -0600 Received: from drow by nevyn.them.org with local (Exim 3.36 #1 (Debian)) id 18WgFy-0006ux-00; Thu, 09 Jan 2003 12:14:14 -0500 Date: Thu, 09 Jan 2003 17:14:00 -0000 From: Daniel Jacobowitz To: Andrew Cagney Cc: gdb@sources.redhat.com Subject: Re: Failures in store.exp caused by regcache Message-ID: <20030109171414.GA26304@nevyn.them.org> Mail-Followup-To: Andrew Cagney , gdb@sources.redhat.com References: <20030108190506.GA558@nevyn.them.org> <3E1C77B4.8070401@redhat.com> <20030109032346.GA10532@nevyn.them.org> <3E1DA834.5060906@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <3E1DA834.5060906@redhat.com> User-Agent: Mutt/1.5.1i X-SW-Source: 2003-01/txt/msg00136.txt.bz2 On Thu, Jan 09, 2003 at 11:49:56AM -0500, Andrew Cagney wrote: > >On Wed, Jan 08, 2003 at 02:10:44PM -0500, Andrew Cagney wrote: > > > >>>(Or rather, by the value code's interaction with the regcache) > >>> > >>>Andrew, this is more your area; I'd like your advice before I dig any > >>>further. Here's what's going wrong. Consider the command sequence: > >>>"up; print u; set u = s_1; print u". > >>> - u has class LOC_REGISTER > >>> - The register's home is memory > >>> - read_var_value therefore returns an lval_memory > >>> - the value of the register is in the register unwind cache at this > >>point > >>> - we modify the memory backing the store > >>> - we have no way to tell that we've just modified the value of a saved > >>> register on the stack > >>> - the second print returns the cached value > >>> > >>>So, what do we do? > > > >> > >>Flush the frame cache. > > > > > >Ugg. Well, if we have to, then we have to. I suppose we do. > > It isn't that bad. In fact (per previous discussion), the code needed > to avoid flushing the caches would be far worse than what we have now. > > The only time the frame cache gets (well, ok, should get ...) flushed is: > > - when the target resumes > The recovery time here is critical. > - when the target is modified > The recovery time here is bounded by the recovery time from a target resume. > > People `never' modify the target. That leaves the time taken to recover > from a resume and then: > - we must flush the cache > - since it is an upper bound on target modify recovery time, making it > faster is a win win. Makes sense. If this ever gets to be a problem (I'm not sure people won't find creative ways to make a mockery of that "never" :) we can be cleverer then. > >We obviously want to preserve things like the selected frame, however. > > I'm actually a bit puzzled. I recently fixed a case and added a > testcase (store.exp) that handles stores. Look for frame_find_by_id() > in valops.c. Yes. On GCC 2.95 that test shows failures still - different optimization. Thanks for pointing me at your change; I see the problem now. You only added the flush for the lval_register case. This is an lval_memory, for all that it's in the frame cache. Slightly different problem, same general solution. Something to think about: theoretically, any call to write_memory can cause the frame cache to become out of date. For now I'll just move the flushing within value_assign; but long-term I think it might be generally worthwhile to have better information on when the cache needs to be flushed, so that we can do it from within functions that modify the target. It would mean something like a list of memory locations read in the process of building the frame cache. Even per frame so that we don't need to flush the whole thing. Yeah, the complexity's gross; but there's a whole class of bugs here... > > Andrew, should I do this the way I do for "set backtrace-below-main", > > and should there be a general function for that? I.E.: > > Have a look at the comments in "frame.h" around reinit_frame_cache() and > get_selected_frame(). In particular: > > FIXME: cagney/2002-11-28: The only difference between > flush_cached_frames() and reinit_frame_cache() is that the latter > explicitly sets the selected frame back to the current frame there > isn't any real difference (except that one delays the selection of > a new frame). Code can instead simply rely on get_selected_frame() > to reinit's the selected frame as needed. As for invalidating the > cache, there should be two methods one that reverts the thread's > selected frame back to current frame (for when the inferior > resumes) and one that does not (for when the user modifies the > target invalidating the frame cache). */ > > /* FIXME: cagney/2002-11-28: At present, when there is no selected > frame, this function always returns the current (inner most) frame. > It should instead, when a thread has previously had its frame > selected (but not resumed) and the frame cache invalidated, find > and then return that thread's previously selected frame. */ > > This only works if we're storing the selected frame in the selected > thread. Unfortunatly (arrrrrrg), GDB first needs to be convinced that > `there is always a thread' so that there is always a `struct > thread_info' into which the selected frame's id can be stored (or, I > guess, as a temp, extend the existing `there might be a thread' hack to > handle this case, double arrrrg). > > The below does raise an interesting question: Right. These comments are why I wrote the below. > >void > >do_flush_frames_sfunc (char *args, int from_tty, struct cmd_list_element > >*c) > >{ > > int saved_level; > > struct frame_info *cur_frame; > > > > if (! target_has_stack) > > return; > > > > saved_level = frame_relative_level (get_selected_frame ()); > > > > flush_cached_frames (); > > > > cur_frame = find_relative_frame (get_current_frame (), &saved_level); > > select_frame (cur_frame); > > > > /* If we were below main and backtrace-below-main was turned off, > > SAVED_LEVEL will be non-zero. CUR_FRAME will point to main. > > Accept this but print the new frame. */ > > if (saved_level != 0) > > print_stack_frame (get_selected_frame (), -1, 0); > >} > > what should: > > (gdb) set backtrace-below-main on > (gdb) up ; up ; up > (gdb) set backtrace-below-main off > (gdb) set variable x = 1 > > do? get_next_frame() is going to refuse to go beyond main, no matter > how many times you try. I decided "it should put you in main". That's what the comment above is for. It's a kind of arbitrary decision but it was the most consistent thing I could come up with. > (btw, a frame_id is safer than a level). Sure; but I think a level is more appropriate here. I don't expect anything above main to change when I rebuild the frame cache. And this way select_level () has the exact behavior I wanted: if the frame we wanted isn't there any more, stop at the bottom. -- Daniel Jacobowitz MontaVista Software Debian GNU/Linux Developer