Re: [RFC v4 8/9] Link frame_info to thread_info

[Philipp Rudo <prudo@linux.vnet.ibm.com>]

Hi Yao

On Mon, 19 Jun 2017 14:07:11 +0100
Yao Qi <qiyaoltc@gmail.com> wrote:

> Philipp Rudo <prudo@linux.vnet.ibm.com> writes:
> 
> > Linux kernel stacks on S390 are spread over several memory locations.
> > These locations differ for every kernel task/thread.  Thus we need to know
> > to which task/thread a frame belongs to check whether a stack pointer is
> > valid and when to stop unwinding.  To do so add a pointer to corresponding
> > thread_info to frame_info.
> >
> > This connection already exists implicitly by the fact that switch_to_thread
> > reinitializes the frame cache.  
> 
> The whole frame cache is created for current thread, from sentinel
> frame.  When the current thread is changed, the frame cache will be
> re-created.  If we see different frame_info objects are about different
> threads, it must be a bug somewhere.  I think frame_info.thread always
> points to the current thread, no?

That's also the way I understand it how it should work.

My problem with it is that such implicit connections via global variables are
extremely error prone and hard to debug.  For example look at
linux-tdep.c:linux_corefile_thread, here the code looks like this

static void                                                                     
linux_corefile_thread (struct thread_info *info,
                       struct linux_corefile_thread_data *args)
{
  [...]

  old_chain = save_inferior_ptid ();
  inferior_ptid = info->ptid;
  target_fetch_registers (regcache, -1);

  [...]
}

At this point the inferior_ptid is changed without re-creating the frame
cache.  Thus the assumption that a existing frame_info always belongs to
current_thread is wrong at this point.  What makes it worse is that right after
a target hook is called.  So suddenly you have a connection between a
target_obs and a gdbarch you'd never expect which can lead to a bug...

For me the easiest way to prevent such long range bugs is not to use global
variables but explicit connections between the different subsystems.

That's why I made this patch.

Philipp

P.S. I know that the example is bad and you shouldn't use the frame cache to
fetch registers but I hope you get the point I was trying to make.