Re: [PATCH v2 07/11] s390: Hook s390 into OSABI mechanism

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

Hi Uli,

On Tue, 5 Dec 2017 18:44:21 +0100 (CET)
"Ulrich Weigand" <uweigand@de.ibm.com> wrote:

> Philipp Rudo wrote:
> 
> > > All of that *except* the sigtramp unwinder is OS-independent.  In fact,
> > > if move the prolog-based sniffer to common code, you'll see that you no
> > > longer need to export various internal routines from s390-tdep.c to
> > > s390-linux-tdep.c.
> > > 
> > > This may require swapping the order of the stub and the sigtramp unwinder,
> > > but that should be harmless.  In the end you should have this sequence:
> > > 
> > > - first, in common code, announce all the DWARF-based unwinders
> > > - then, in Linux ABI code, announce the sigtramp unwinder
> > > - finally, back in common code, announce all the fallback unwinders  
> > 
> > That's not true.  At least for the kernel the unwinders (except the DWARF-based)
> > failed and I had to write my own one.  Especially the fact that the kernel has 
> > multiple stack locations and no distinct 'End of Stack' caused problems.
> > 
> > However if I go with your approach and add my unwinder in the kernel ABI code
> > as default, i.e. that it always catches the call, it should work.  But that
> > won't be nice code.  That's why I moved them to the Linux ABI.  
> 
> That's surprising, I would have thought the prolog parser generic enough
> to handle kernel code as well (and it really has to be anyway, since you
> install the s390_skip_prologue callback in generic code ...).

My best guess is that the prologue unwinder cannot handle the fact that there
is no classical branch to the interrupt handler.  So the old pc cannot be taken
from %r14 or the psw but has to be read from lowcore.  But I'd had to spend
more time to investigate it further.  However there are more pressing things
to do.  Thats why I'll stick with the solution I have for the time.  The
backchain unwinder I have also is a good fall-back, so implementing it wasn't a
waste of time.

Most likely I'll have to get back to the unwinders soon anyway.  At the moment
it looks like my next kernel item will be to port the ORC unwinder to s390.  So
support for it has to be added to GDB, too.

To be honest I don't know if the s390_skip_prologue works for kernel
debugging.  As I only support (and tested) debugging of dumps, the impact
should be minimal if it were broken.

> I agree that there may be some special cases.  About the issues you mention:
> 
> - By "multiple stack locations" I assume you refer to the interrupt stack
>   vs. the regular per-task kernel stacks?  I agree that we need kernel-
>   specific code to switch between the two.  But that should simply be
>   the equivalent to the user-space signal stack handling, so you'll install
>   a sniffer in the kernel-specific code that detects the frame where you
>   need to *switch* stacks.  All the normal frames should be handled fine
>   by the standard prolog-parser code.

Yes, "multiple stack locations" refers to interrupt vs. per-task kernel stack
(plus restart and panic stack).

The unwinder I have implemented mostly works like the user-space signal stack
handling.  However I needed a backchain unwinder to handle the assembler code
for two reasons.  First it doesn't contain debug info the dwarf unwinder could
use.  Second a backchain = 0 marks the End-of-this-Stack so the backchain
unwinder has to trigger the sigtramp unwinder to go to the next stack location
(or stop unwinding if its the end of the per-task kernel stack).
 
> - End-of-stack detection is always a bit hit-and-miss with the prolog
>   parser, and uses heuristics anyway.  If you need to tweak those a
>   bit so they work well for kernel code, I'd hope (without having seen
>   the code) that it should be possible to simply update the generic
>   code to handle both.

Currently I'm using brute-force to detect the End-of-Stack, i.e. simply check
if the SP points to any of the given stack locations or not.  Something similar
has to be added to the heuristics of the prologue unwinder.  The only other
chance would be to check if the SP is a kernel- or user-space address.  But you
had to do the heuristics for that by only using an unsigned long, so it will be
very error prone.

Thanks
Philipp