Re: [Xen-devel] [Patch v3 1/4] x86/stack: Refactor show_trace()

["Jan Beulich" <JBeulich@xxxxxxxx>]

> +++ b/xen/arch/x86/traps.c
> @@ -195,12 +195,14 @@ static void show_guest_stack(struct vcpu *v, struct 
> cpu_user_regs *regs)
>  
>  #if !defined(CONFIG_FRAME_POINTER)
>  
> -static void show_trace(struct cpu_user_regs *regs)
> +/*
> + * Stack trace from pointers found in stack, unaided by frame pointers.  For
> + * caller convenience, this has the same prototype as its alternative, and
> + * simply ignores the base pointer parameter.
> + */
> +static void __show_trace(unsigned long sp, unsigned long __maybe_unused bp)

A single leading underscore only please.

> -    /*
> -     * If RIP is not pointing into hypervisor code then someone may have
> -     * called into oblivion. Peek to see if they left a return address at
> -     * top of stack.
> -     */
> -    addr = is_active_kernel_text(regs->eip) ||
> -           !is_active_kernel_text(*ESP_BEFORE_EXCEPTION(regs)) ?

Please note the pointer deref here.

> +static void show_trace(const struct cpu_user_regs *regs)
> +{
> +    unsigned long sp = regs->rsp;
> +
> +    printk("Xen call trace:\n");
> +
> +    /*
> +     * If RIP looks sensible, or the top of the stack doesn't, print RIP at
> +     * the top of the stack trace.
> +     */
> +    if ( is_active_kernel_text(regs->rip) ||
> +         !is_active_kernel_text(regs->rsp) )

Which is gone here. In this shape, I don't see what the purpose of
the check is.

> +        printk("   [<%p>] %pS\n", _p(regs->eip), _p(regs->eip));
> +    /*
> +     * else RIP looks bad but the top of the stack looks good.  Perhaps we

"Else"

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel