Re: [downstream patch FYI] workaround stale frame_info * (PR 13866)

Pedro Alves
2012-05-18T18:01:05Z

On 05/18/2012 06:44 PM, Tom Tromey wrote:

>>>>>> "Jan" == Jan Kratochvil <jan.kratochvil@redhat.com> writes:
>
> Jan> With proper C++ sanity check of stale live frame_info references
> Jan> the testcase would be simple without the "frame_garbage_collection"
> Jan> reproducer below. It is also reproducible just with valgrind but
> Jan> regularly running the whole testsuite under valgrind I did not find
> Jan> feasible.
>
> I was thinking about this problem recently.
>
> One idea would be to only let unwinders access struct frame_info, and
> have all other code use struct frame_id, perhaps with some simple cache
> so that repeated calls for a given frame_id will not cause too much
> work.
>
> This would make it much harder to cache information when not allowed.
>
> On the minus side, there are 1300 uses of struct frame_info to audit.

We invalidate the frame chain when something, anything, changes
memory or registers in the inferior, or we do something else that might
change our view of the frame stack (such as loading symbols). But, setting a
breakpoint doesn't really change the inferior's memory from most of GDB code's
perspective, including the frame machinery, because reading back the memory we just
written to always gives back the contents as if the breakpoint wasn't planted in
the first place (due to shadowing). So it's garanteed (minus bugs), that
the rebuilt cache will be a clone of the original cache.

We also only have one global cache, not a cache per thread (which I don't think
would be a good idea, considering inferiors with many many threads), as and
such we flush the cache whenever we change threads, even if for some internal
operation, like installing a breakpoint. But, if we build a cache for thread A,
flip to thread B for a second, and then flip back to thread A, while having done
nothing that might have changed our view of the frame chain, the frame chain
built for thread A the second time should be the exact same as what
was build before flipping to thread B.

It seems to me that combining the both points, we should be able to get back
the old behavior, where calling a breakpoint insertion function wouldn't
invalidate the frame chain.

--
Pedro Alves