This is the mail archive of the
gdb@sources.redhat.com
mailing list for the GDB project.
Re: Failures in store.exp caused by regcache
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