Date: Sat, 04 May 2013 09:08:19 -0600 From: Ian Lepore <ian@FreeBSD.org> To: Andrew Turner <andrew@fubar.geek.nz> Cc: freebsd-arm <freebsd-arm@FreeBSD.org> Subject: Re: A fix for the clang + eabi + kdb backtrace endless loop Message-ID: <1367680099.1180.162.camel@revolution.hippie.lan> In-Reply-To: <20130503115141.6c9cd15a@bender> References: <1367439452.1180.92.camel@revolution.hippie.lan> <20130503115141.6c9cd15a@bender>
next in thread | previous in thread | raw e-mail | index | archive | help
--=-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 <ian@FreeBSD.org> 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--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?1367680099.1180.162.camel>