From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Cooper Subject: [RFC Patch 1/2] x86/traps: Refactor show_trace() Date: Thu, 8 Aug 2013 17:19:09 +0100 Message-ID: <1375978750-25898-2-git-send-email-andrew.cooper3@citrix.com> References: <1375978750-25898-1-git-send-email-andrew.cooper3@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1375978750-25898-1-git-send-email-andrew.cooper3@citrix.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Xen-devel Cc: Andrew Cooper List-Id: xen-devel@lists.xenproject.org Before, show_trace() had two implementations depending on CONFIG_FRAME_POINTER. Some parts were common, while the loops to wander up the stack were different. The version aided by frame pointers had a special case for function calls on wild function pointers, but this doesn't need to be a special case. After the refactoring, there are now two implementations of __show_trace() which differ depending on CONFIG_FRAME_POINTER, and a single show_trace() with the common bits, including the logic for wild function pointers. Signed-off-by: Andrew Cooper --- The new wild pointer logic is rather larger than its pre-refactor version, but is rather more legible. I cant think of a way to compact it without making it substantially less legible. --- xen/arch/x86/traps.c | 66 +++++++++++++++++++++++++++++--------------------- 1 file changed, 39 insertions(+), 27 deletions(-) diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c index 57dbd0c..324b0f1 100644 --- a/xen/arch/x86/traps.c +++ b/xen/arch/x86/traps.c @@ -192,14 +192,13 @@ 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 rbp parameter. + */ +static void __show_trace(unsigned long sp, unsigned long bp) { - unsigned long *stack = ESP_BEFORE_EXCEPTION(regs), addr; - - printk("Xen call trace:\n "); - - printk("[<%p>]", _p(regs->eip)); - print_symbol(" %s\n ", regs->eip); + unsigned long *stack = (unsigned long *)sp, addr; while ( ((long)stack & (STACK_SIZE-BYTES_PER_LONG)) != 0 ) { @@ -210,36 +209,22 @@ static void show_trace(struct cpu_user_regs *regs) print_symbol(" %s\n ", addr); } } - - printk("\n"); } #else -static void show_trace(struct cpu_user_regs *regs) +/* Stack trace from frames in the stack, using frame pointers */ +static void __show_trace(unsigned long sp, unsigned long bp) { unsigned long *frame, next, addr, low, high; - printk("Xen call trace:\n "); - - /* - * 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)) ? - regs->eip : *ESP_BEFORE_EXCEPTION(regs); - printk("[<%p>]", _p(addr)); - print_symbol(" %s\n ", addr); - /* Bounds for range of valid frame pointer. */ - low = (unsigned long)(ESP_BEFORE_EXCEPTION(regs) - 2); + low = sp - 2*sizeof(unsigned long); high = (low & ~(STACK_SIZE - 1)) + (STACK_SIZE - sizeof(struct cpu_info) - 2*sizeof(unsigned long)); /* The initial frame pointer. */ - next = regs->ebp; + next = bp; for ( ; ; ) { @@ -272,12 +257,39 @@ static void show_trace(struct cpu_user_regs *regs) low = (unsigned long)&frame[2]; } - - printk("\n"); } #endif +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 look sensible, + * print RIP at the top of the stack trace. */ + if ( is_active_kernel_text(regs->rip) || + !is_active_kernel_text(regs->rsp) ) + { + printk("[<%p>]", _p(regs->rip)); + print_symbol(" %s\n ", regs->rip); + } + /* else RIP looks bad but the top of the stack looks ok. Perhaps we + * followed a wild function pointer, so lets assume the top of the stack is + * a return address. Skip past it so__show_trace() doesn't print it + * again. */ + else + { + printk("[<%p>]", _p(sp)); + print_symbol(" %s\n ", sp); + sp += sizeof (unsigned long); + } + + __show_trace(sp, regs->rbp); + + printk("\n"); +} + void show_stack(struct cpu_user_regs *regs) { unsigned long *stack = ESP_BEFORE_EXCEPTION(regs), addr; -- 1.7.10.4