[Pdbg] [PATCH v2] stack: guess endian for stack frame walking

[Nicholas Piggin]

On Fri, 21 Sep 2018 15:17:45 +1000
Alistair Popple <alistair at popple.id.au> wrote:

> Nice! Must admit I haven't read the code in depth but what condition does the
> haphazard endian detection use to detemine endianess?

Mainly it's comparing r1 with the saved stack pointer in r1's frame.
If they are more than 1GB apart, try flipping and see if they're
close.

Another check (which may not catch endian bugs but helps verify we
have a stack) is that stack grows down, so if what we think is the
saved pointer is less than current pointer (we're walking backwards),
then it's also not right.

Thanks,
Nick

> 
> - Alistair
> 
> On Friday, 21 September 2018 2:34:17 PM AEST Nicholas Piggin wrote:
> > The stack unwinder currently does not do any endian conversion, which
> > means it won't work correctly if the stack does not match pdbg endian.
> > 
> > This patch attempts an endian flip if the stack looks wrong, and goes
> > with that if it's an improvement. This was nice for debugging skiboot.
> > 
> > Signed-off-by: Nicholas Piggin <npiggin at gmail.com>
> > ---
> > v2: Improve ugly code slightly
> > 
> >  src/thread.c | 60 +++++++++++++++++++++++++++++++++++++++++++++++-----
> >  1 file changed, 55 insertions(+), 5 deletions(-)
> > 
> > diff --git a/src/thread.c b/src/thread.c
> > index d282307..b53dccd 100644
> > --- a/src/thread.c
> > +++ b/src/thread.c
> > @@ -105,6 +105,15 @@ static int load8(struct pdbg_target *target, uint64_t addr, uint64_t *value)
> >  	return 1;
> >  }
> >  
> > +uint64_t flip_endian(uint64_t v)
> > +{
> > +#if __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__
> > +	return be64toh(v);
> > +#else
> > +	return le64toh(v);
> > +#endif
> > +}
> > +
> >  static int dump_stack(struct thread_regs *regs)
> >  {
> >  	struct pdbg_target *target;
> > @@ -117,24 +126,65 @@ static int dump_stack(struct thread_regs *regs)
> >  		break;
> >  	}
> >  
> > -	printf("STACK:\n");
> > +	printf("STACK:           SP                NIA\n");
> >  	if (!target)
> >  		pdbg_log(PDBG_ERROR, "Unable to read memory (no ADU found)\n");
> >  
> >  	if (sp && is_real_address(regs, sp)) {
> > -		if (!load8(target, sp, &sp))
> > +		uint64_t tmp;
> > +		bool flip = false;
> > +#if __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__
> > +		bool be = false;
> > +#else
> > +		bool be = true;
> > +#endif
> > +
> > +		if (!load8(target, sp, &tmp))
> >  			return 1;
> > -		while (sp && is_real_address(regs, sp)) {
> > +
> > +		if (!tmp) {
> > +badstack:
> > +			printf("SP:0x%016" PRIx64 " points to 0x%016" PRIx64 ", not decoding\n", sp, tmp);
> > +			return 0;
> > +		}
> > +
> > +		/* Haphazard endian detection */
> > +		if (tmp < sp || (tmp - sp > (1024*1024*1024))) {
> > +			uint64_t tmp2;
> > +			tmp2 = flip_endian(tmp);
> > +			if (tmp2 < sp || (tmp2 - sp > (1024*1024*1024)))
> > +				goto badstack;
> > +			sp = tmp2;
> > +			flip = true;
> > +			be = !be;
> > +		} else {
> > +			sp = tmp;
> > +		}
> > +
> > +		if (be)
> > +			printf("Looks like big-endian\n");
> > +		else
> > +			printf("Looks like little-endian\n");
> > +
> > +		while (sp) {
> > +			if (!is_real_address(regs, sp))
> > +				break;
> > +
> >  			if (!load8(target, sp + 16, &pc))
> >  				return 1;
> > +			if (flip)
> > +				pc = flip_endian(pc);
> >  
> > -			printf(" 0x%016" PRIx64 " 0x%16" PRIx64 "\n", sp, pc);
> > +			printf(" 0x%016" PRIx64 " 0x%016" PRIx64 "\n", sp, pc);
> >  
> >  			if (!load8(target, sp, &sp))
> >  				return 1;
> > +			if (flip)
> > +				sp = flip_endian(sp);
> >  		}
> > +	} else {
> > +		printf("SP:0x%016" PRIx64 " does not appear to be a stack\n", sp);
> >  	}
> > -
> >  	return 0;
> >  }
> >  
> >   
> 
>