Re: [xen-devel] [Patch 2/4] x86/stack: Adjust boundary conditions for printed stacks.

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 12/08/13 09:46, Jan Beulich wrote:
>>>> On 09.08.13 at 21:55, Andrew Cooper <andrew.cooper3@citrix.com> wrote:
>> --- a/xen/include/asm-x86/current.h
>> +++ b/xen/include/asm-x86/current.h
>> @@ -50,6 +50,15 @@ static inline struct cpu_info *get_cpu_info(void)
>>  #define get_stack_bottom()                      \
>>      ((unsigned long)&get_cpu_info()->guest_cpu_user_regs.es)
>>  
>> +/*
>> + * Get the bottom-of-stack, as useful for printing stack traces. This is an
>> + * equivalent place on the stack as guest_cpu_user_regs(), but works on an
>> + * arbitrary stack pointer rather than the current stack.
>> + */
>> +#define get_printable_stack_bottom(sp)          \
>> +    ((sp & (~(STACK_SIZE-1))) +                 \
>> +     (STACK_SIZE - sizeof(struct cpu_info) - 2*sizeof(unsigned long)))
> Iirc in the prior RFC version of these patches you were convinced
> that the adjustment by 2 stack slots was wrong (and that was the
> primary reason why you had asked for comments). Now you're
> keeping it, without making clear what made you change your
> opinion.
>
> Jan
>

I was concerned about the -2 adjustment to 'low', which could end up
causing a triple fault.

The adjustment here covers call instructions out of assembly code, which
lack a traditional stack layout.  Specifically, removing the -2 causes
the frame pointer code to find a frame pointer of value 0x0...001 as an
unknown text symbol.

I suppose one side effect would be that the non-frame pointer case will
miss the bottom return address; That could be worked around by having
the -2 conditional on the frame pointer case.  This would still have the
advantage of the non-frame pointer case not attempting to interpret the
guest_cpu_user_regs for xen return addresses.

~Andrew