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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 20.11.13 at 14:09, Andrew Cooper <andrew.cooper3@xxxxxxxxxx> wrote:
> +static void show_trace(const struct cpu_user_regs *regs)
> +{
> +    unsigned long *sp = ESP_BEFORE_EXCEPTION(regs);

So you correctly made it a pointer here...

> +
> +    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(*sp) )

... and de-reference it here ...

> +        printk("   [<%p>] %pS\n", _p(regs->rip), _p(regs->rip));
> +    /*
> +     * Else RIP looks bad but the top of the stack looks good.  Perhaps we
> +     * followed a wild function pointer? Lets assume the top of the stack is 
> a
> +     * return address; print it and skip past so _show_trace() doesn't print
> +     * it again.
> +     */
> +    else
> +    {
> +        printk("   [<%p>] %pS\n", _p(*sp), _p(*sp));
> +        sp++;
> +    }
> +
> +    _show_trace(*sp, regs->rbp);

... but then you now also de-reference it here? How did that end
up producing sane stack dumps?

Since one of the two _show_trace() variants wants a pointer here
anyway, you would probably best switch its first argument and use
the inverse casting in the other variant.

Jan


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