From owner-freebsd-arm@FreeBSD.ORG Sat May 4 15:08:25 2013 Return-Path: Delivered-To: freebsd-arm@FreeBSD.org Received: from mx1.freebsd.org (mx1.FreeBSD.org [8.8.178.115]) by hub.freebsd.org (Postfix) with ESMTP id BBADC46D for ; Sat, 4 May 2013 15:08:25 +0000 (UTC) (envelope-from ian@FreeBSD.org) Received: from mho-01-ewr.mailhop.org (mho-03-ewr.mailhop.org [204.13.248.66]) by mx1.freebsd.org (Postfix) with ESMTP id 945A21ECE for ; Sat, 4 May 2013 15:08:25 +0000 (UTC) Received: from c-24-8-230-52.hsd1.co.comcast.net ([24.8.230.52] helo=damnhippie.dyndns.org) by mho-01-ewr.mailhop.org with esmtpsa (TLSv1:AES256-SHA:256) (Exim 4.72) (envelope-from ) id 1UYe4W-0003EY-Gv; Sat, 04 May 2013 15:08:24 +0000 Received: from [172.22.42.240] (revolution.hippie.lan [172.22.42.240]) by damnhippie.dyndns.org (8.14.3/8.14.3) with ESMTP id r44F8Jx1012111; Sat, 4 May 2013 09:08:20 -0600 (MDT) (envelope-from ian@FreeBSD.org) X-Mail-Handler: Dyn Standard SMTP by Dyn X-Originating-IP: 24.8.230.52 X-Report-Abuse-To: abuse@dyndns.com (see http://www.dyndns.com/services/sendlabs/outbound_abuse.html for abuse reporting information) X-MHO-User: U2FsdGVkX1+mxz+XtdzYhV+3w07xi26G Subject: Re: A fix for the clang + eabi + kdb backtrace endless loop From: Ian Lepore To: Andrew Turner In-Reply-To: <20130503115141.6c9cd15a@bender> References: <1367439452.1180.92.camel@revolution.hippie.lan> <20130503115141.6c9cd15a@bender> Content-Type: multipart/mixed; boundary="=-ln0HrZPFdHK2Vh66Xe3w" Date: Sat, 04 May 2013 09:08:19 -0600 Message-ID: <1367680099.1180.162.camel@revolution.hippie.lan> Mime-Version: 1.0 X-Mailer: Evolution 2.32.1 FreeBSD GNOME Team Port Cc: freebsd-arm X-BeenThere: freebsd-arm@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: Porting FreeBSD to the StrongARM Processor List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 04 May 2013 15:08:25 -0000 --=-ln0HrZPFdHK2Vh66Xe3w Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit On Fri, 2013-05-03 at 11:51 +0100, Andrew Turner wrote: > On Wed, 01 May 2013 14:17:32 -0600 > Ian Lepore wrote: > > > The attached patch fixes the problem where a kdb backtrace loops > > endlessly on an eabi kernel. > > > > I'm no expert on this stuff, so while this fixes the problem for me, > > I'm not sure it's right, especially the STOP_UNWINDING in > > exception_exit (which handles a test case of breaking into the > > debugger with the keyboard and typing 'bt'). I'm not going to commit > > this until it's been reviewed by someone who actually knows what > > they're doing. :) > STOP_UNWINDING tells binutils to generate the required unwind op-code > to tell the kernel's unwinder it needs to stop in this function. There > is a bug with the unwinder where it should tell the user it is in the > function, however it doesn't currently. > > See below for more comments. Thanks for the review and info. I didn't realize FP was unused in EABI. I guess keying off it just accidentally worked right in the few cases I tested. I was also a bit worried about the recursion case with using the PC to see if unwinding seemed to be stuck. I studied the code some more and discovered that there's a mask of which registers changed, used for pretty-printing. I think that's a safer way to determine whether unwinding is stuck in a loop -- if no registers changed then nothing was unwound. I also restructured things a bit so that the decision to stop unwinding isn't acted on until after the current frame is printed; that seems to ensure that the frame for a STOP_UNWINDING function gets printed before exiting the loop. So, here's a somewhat more clueful patch; how's this look? -- Ian --=-ln0HrZPFdHK2Vh66Xe3w Content-Disposition: inline; filename="eabi_unwind_fixes2.diff" Content-Type: text/x-patch; name="eabi_unwind_fixes2.diff"; charset="us-ascii" Content-Transfer-Encoding: 7bit Index: sys/arm/arm/db_trace.c =================================================================== --- sys/arm/arm/db_trace.c (revision 250163) +++ sys/arm/arm/db_trace.c (working copy) @@ -342,8 +342,11 @@ db_stack_trace_cmd(struct unwind_state *state) c_db_sym_t sym; u_int reg, i; char *sep; + uint16_t upd_mask; + bool finished; - while (1) { + finished = false; + while (!finished) { /* Reset the mask of updated registers */ state->update_mask = 0; @@ -353,28 +356,20 @@ db_stack_trace_cmd(struct unwind_state *state) /* Find the item to run */ index = db_find_index(state->start_pc); - if (index->insn == EXIDX_CANTUNWIND) { - db_printf("Unable to unwind\n"); - break; - } else if (index->insn & (1 << 31)) { - /* The data is within the instruction */ - state->insn = &index->insn; - } else { - /* We have a prel31 offset to the unwind table */ - uint32_t prel31_tbl = db_expand_prel31(index->insn); - - state->insn = (uint32_t *)((uintptr_t)&index->insn + - prel31_tbl); + if (index->insn != EXIDX_CANTUNWIND) { + if (index->insn & (1 << 31)) { + /* The data is within the instruction */ + state->insn = &index->insn; + } else { + /* A prel31 offset to the unwind table */ + state->insn = (uint32_t *) + ((uintptr_t)&index->insn + + db_expand_prel31(index->insn)); + } + /* Run the unwind function */ + finished = db_unwind_tab(state); } - /* Run the unwind function */ - if (db_unwind_tab(state) != 0) - break; - - /* This is not a kernel address, stop processing */ - if (state->registers[PC] < VM_MIN_KERNEL_ADDRESS) - break; - /* Print the frame details */ sym = db_search_symbol(state->start_pc, DB_STGY_ANY, &offset); if (sym == C_DB_SYM_NULL) { @@ -393,12 +388,11 @@ db_stack_trace_cmd(struct unwind_state *state) state->registers[SP], state->registers[FP]); /* Don't print the registers we have already printed */ - state->update_mask &= ~((1 << SP) | (1 << FP) | (1 << LR) | - (1 << PC)); + upd_mask = state->update_mask & + ~((1 << SP) | (1 << FP) | (1 << LR) | (1 << PC)); sep = "\n\t"; - for (i = 0, reg = 0; state->update_mask != 0; - state->update_mask >>= 1, reg++) { - if ((state->update_mask & 1) != 0) { + for (i = 0, reg = 0; upd_mask != 0; upd_mask >>= 1, reg++) { + if ((upd_mask & 1) != 0) { db_printf("%s%sr%d = 0x%08x", sep, (reg < 10) ? " " : "", reg, state->registers[reg]); @@ -412,6 +406,25 @@ db_stack_trace_cmd(struct unwind_state *state) } } db_printf("\n"); + + /* Stop if directed to do so, or if we've unwound back to the + * kernel entry point, or if the unwind function didn't change + * anything (to avoid getting stuck in this loop forever). + * If the latter happens, it's an indication that the unwind + * information is incorrect somehow for the function named in + * the last frame printed before you see the unwind failure + * message (maybe it needs a STOP_UNWINDING). + */ + if (index->insn == EXIDX_CANTUNWIND) { + db_printf("Unable to unwind further\n"); + finished = true; + } else if (state->registers[PC] < VM_MIN_KERNEL_ADDRESS) { + db_printf("Unable to unwind into user mode\n"); + finished = true; + } else if (state->update_mask == 0) { + db_printf("Unwind failure (no registers changed)\n"); + finished = true; + } } } #endif Index: sys/arm/arm/exception.S =================================================================== --- sys/arm/arm/exception.S (revision 250163) +++ sys/arm/arm/exception.S (working copy) @@ -196,15 +196,19 @@ END(address_exception_entry) * Interrupts are disabled at suitable points to avoid ASTs * being posted between testing and exit to user mode. * - * This function uses PULLFRAMEFROMSVCANDEXIT and - * DO_AST - * only be called if the exception handler used PUSHFRAMEINSVC + * This function uses PULLFRAMEFROMSVCANDEXIT and DO_AST and can + * only be called if the exception handler used PUSHFRAMEINSVC. * + * For EABI, don't try to unwind any further than this, because the unwind + * info generated here is a single INSN_FINISH instruction. Nothing gets + * unwound (no registers change) so the unwind would loop here forever. */ -exception_exit: +ASENTRY_NP(exception_exit) + STOP_UNWINDING DO_AST PULLFRAMEFROMSVCANDEXIT +END(exception_exit) /* * undefined_entry: Index: sys/arm/arm/locore.S =================================================================== --- sys/arm/arm/locore.S (revision 250163) +++ sys/arm/arm/locore.S (working copy) @@ -77,6 +77,8 @@ __FBSDID("$FreeBSD$"); */ ENTRY_NP(btext) ASENTRY_NP(_start) + STOP_UNWINDING /* Can't unwind into the bootloader! */ + mov r9, r0 /* 0 or boot mode from boot2 */ mov r8, r1 /* Save Machine type */ mov ip, r2 /* Save meta data */ Index: sys/arm/arm/swtch.S =================================================================== --- sys/arm/arm/swtch.S (revision 250163) +++ sys/arm/arm/swtch.S (working copy) @@ -540,6 +540,7 @@ ENTRY(savectx) END(savectx) ENTRY(fork_trampoline) + STOP_UNWINDING /* Can't unwind beyond the thread enty point */ mov r1, r5 mov r2, sp mov r0, r4 --=-ln0HrZPFdHK2Vh66Xe3w--