[Crash-utility] Re: [RFC] Crash patch for DWARF CFI based unwind support
Dave Anderson
anderson at redhat.com
Wed Nov 15 15:05:35 UTC 2006
Rachita Kothiyal wrote:
> Hi Dave
>
> The following patch is a consolidated cleanup and minor fixes attempt.
> Specifically it does the following:
>
> 1. Saves register values(rip, rsp, rbp) got from the exception frame.
> These are used to unwind the process stack. Added a 'bptr' field
> in the bt_info structure to save the rbp value from the eframe.
> 2. Added checks for offset being zero while resolving the text address
> to a symbol name.
> 3. Added dwarf_print_stack_entry() as a parallel to x86_print_stack_entry().
> Could probably move the offset checking routine as a separate function.
> Would save us some lines of code. Pushed that for later.
> 4. Fixed 'levels' in the backtrace output.
>
> Thanks
> Rachita
>
> Signed-off-by: Rachita Kothiyal <rachita at in.ibm.com>
> ---
Hi Rachita,
Great -- I'll tinker with this today and get out a new release ASAP.
One question re: this hanging thread:
> > So, in other words, if we hardwire the user_regs_struct so that
> > it uses the NT_PRSTATUS registers all the time, then we get
> > the second (preferred/better) budget back trace when unwind
> > is off.
> >
> > That being the case, I argue for hardwiring them all the time.
>
> Yes, we can(should) do that for all the active tasks.
>
> Rachita
>
Correct me if I'm wrong here, but...
If, during machdep_init(POST_GBD), if we force-initialize
(hardwire when necessary) the relevant user_regs_struct items,
then we could change this line from:
if (((NETDUMP_DUMPFILE() || KDUMP_DUMPFILE()) &&
VALID_STRUCT(user_regs_struct) && (bt->task == tt->panic_task)) ||
(KDUMP_DUMPFILE() && (kt->flags & DWARF_UNWIND) &&
(bt->flags & BT_DUMPFILE_SEARCH))) {
to:
if (((NETDUMP_DUMPFILE() || KDUMP_DUMPFILE()) &&
VALID_STRUCT(user_regs_struct) && (bt->task == tt->panic_task)) ||
(KDUMP_DUMPFILE() && (bt->flags & BT_DUMPFILE_SEARCH))) {
To be absolutely safe in a backwards-compatibility sense, perhaps
we should only do the hardwiring if it's a KDUMP_DUMPFILE().
Make sense?
Dave
>
>
> defs.h | 3
> unwind_x86_32_64.c | 77 ++++++++++++++++++++++---
> x86_64.c | 163 ++++++-----------------------------------------------
> 3 files changed, 91 insertions(+), 152 deletions(-)
>
> diff -puN x86_64.c~cfi_backtrace_minor_fixes x86_64.c
> --- crash-4.0-3.9/x86_64.c~cfi_backtrace_minor_fixes 2006-11-15 18:29:04.848359480 +0530
> +++ crash-4.0-3.9-rachita/x86_64.c 2006-11-15 19:00:40.687148176 +0530
> @@ -2548,6 +2548,7 @@ x86_64_dwarf_back_trace_cmd(struct bt_in
> irq_eframe = 0;
> last_process_stack_eframe = 0;
> bt->call_target = NULL;
> + bt->bptr = 0;
> rsp = bt->stkptr;
> if (!rsp) {
> error(INFO, "cannot determine starting stack pointer\n");
> @@ -2617,31 +2618,9 @@ in_exception_stack:
>
> stacktop = bt->stacktop - SIZE(pt_regs);
>
> - if ((kt->flags & DWARF_UNWIND) && !done)
> - done = dwarf_backtrace(bt, stacktop);
> -
> - for (i = (rsp - bt->stackbase)/sizeof(ulong);
> - !done && (rsp < stacktop); i++, rsp += sizeof(ulong)) {
> -
> - up = (ulong *)(&bt->stackbuf[i*sizeof(ulong)]);
> -
> - if (!is_kernel_text(*up))
> - continue;
> -
> - switch (x86_64_print_stack_entry(bt, ofp, level, i,*up))
> - {
> - case BACKTRACE_ENTRY_AND_EFRAME_DISPLAYED:
> - rsp += SIZE(pt_regs);
> - i += SIZE(pt_regs)/sizeof(ulong);
> - case BACKTRACE_ENTRY_DISPLAYED:
> - level++;
> - break;
> - case BACKTRACE_ENTRY_IGNORED:
> - break;
> - case BACKTRACE_COMPLETE:
> - done = TRUE;
> - break;
> - }
> + if ((kt->flags & DWARF_UNWIND) && !done) {
> + level = dwarf_backtrace(bt, level, stacktop);
> + done = TRUE;
> }
>
> cs = x86_64_exception_frame(EFRAME_PRINT|EFRAME_CS, 0,
> @@ -2702,32 +2681,10 @@ in_exception_stack:
>
> stacktop = bt->stacktop - 64; /* from kernel code */
>
> - if ((kt->flags & DWARF_UNWIND) && !done)
> - done = dwarf_backtrace(bt, stacktop);
> -
> - for (i = (rsp - bt->stackbase)/sizeof(ulong);
> - !done && (rsp < stacktop); i++, rsp += sizeof(ulong)) {
> -
> - up = (ulong *)(&bt->stackbuf[i*sizeof(ulong)]);
> -
> - if (!is_kernel_text(*up))
> - continue;
> -
> - switch (x86_64_print_stack_entry(bt, ofp, level, i,*up))
> - {
> - case BACKTRACE_ENTRY_AND_EFRAME_DISPLAYED:
> - rsp += SIZE(pt_regs);
> - i += SIZE(pt_regs)/sizeof(ulong);
> - case BACKTRACE_ENTRY_DISPLAYED:
> - level++;
> - break;
> - case BACKTRACE_ENTRY_IGNORED:
> - break;
> - case BACKTRACE_COMPLETE:
> - done = TRUE;
> - break;
> - }
> - }
> + if ((kt->flags & DWARF_UNWIND) && !done) {
> + level = dwarf_backtrace(bt, level, stacktop);
> + done = TRUE;
> + }
>
> if (!BT_REFERENCE_CHECK(bt))
> fprintf(fp, "--- <IRQ stack> ---\n");
> @@ -2792,21 +2749,6 @@ in_exception_stack:
> }
>
> /*
> - * For a normally blocked task, hand-create the first level.
> - */
> - if (!done && !(kt->flags & DWARF_UNWIND) &&
> - !(bt->flags & (BT_TEXT_SYMBOLS|BT_EXCEPTION_STACK|BT_IRQSTACK)) &&
> - STREQ(closest_symbol(bt->instptr), "thread_return")) {
> - bt->flags |= BT_SCHEDULE;
> - i = (rsp - bt->stackbase)/sizeof(ulong);
> - x86_64_print_stack_entry(bt, ofp, level,
> - i, bt->instptr);
> - bt->flags &= ~(ulonglong)BT_SCHEDULE;
> - rsp += sizeof(ulong);
> - level++;
> - }
> -
> - /*
> * Dump the IRQ exception frame from the process stack.
> * If the CS register indicates a user exception frame,
> * then set done to TRUE to avoid the process stack walk-through.
> @@ -2814,8 +2756,7 @@ in_exception_stack:
> */
> if (irq_eframe) {
> bt->flags |= BT_EXCEPTION_FRAME;
> - i = (irq_eframe - bt->stackbase)/sizeof(ulong);
> - x86_64_print_stack_entry(bt, ofp, level, i, bt->instptr);
> + level = dwarf_print_stack_entry(bt, level);
> bt->flags &= ~(ulonglong)BT_EXCEPTION_FRAME;
> cs = x86_64_exception_frame(EFRAME_PRINT|EFRAME_CS, 0,
> bt->stackbuf + (irq_eframe - bt->stackbase), bt, ofp);
> @@ -2833,81 +2774,10 @@ in_exception_stack:
> /*
> * Walk the process stack.
> */
> - if ((kt->flags & DWARF_UNWIND) && !done)
> - done = dwarf_backtrace(bt, bt->stacktop);
> -
> - for (i = (rsp - bt->stackbase)/sizeof(ulong);
> - !done && (rsp < bt->stacktop); i++, rsp += sizeof(ulong)) {
> -
> - up = (ulong *)(&bt->stackbuf[i*sizeof(ulong)]);
> -
> - if (!is_kernel_text(*up))
> - continue;
> -
> - if ((bt->flags & BT_CHECK_CALLER)) {
> - /*
> - * A non-zero offset value from the value_search()
> - * lets us know if it's a real text return address.
> - */
> - spt = value_search(*up, &offset);
> - /*
> - * sp gets the syment of the function that the text
> - * routine above called before leaving its return
> - * address on the stack -- if it can be determined.
> - */
> - sp = x86_64_function_called_by((*up)-5);
> -
> - if (sp == NULL) {
> - /*
> - * We were unable to get the called function.
> - * If the text address had an offset, then
> - * it must have made an indirect call, and
> - * can't have called our target function.
> - */
> - if (offset) {
> - if (CRASHDEBUG(1))
> - fprintf(ofp,
> - "< ignoring %s() -- makes indirect call and NOT %s()>\n",
> - spt->name,
> - bt->call_target);
> - continue;
> - }
> - } else if ((machdep->flags & SCHED_TEXT) &&
> - STREQ(bt->call_target, "schedule") &&
> - STREQ(sp->name, "__sched_text_start")) {
> - ; /* bait and switch */
> - } else if (!STREQ(sp->name, bt->call_target)) {
> - /*
> - * We got function called by the text routine,
> - * but it's not our target function.
> - */
> - if (CRASHDEBUG(2))
> - fprintf(ofp,
> - "< ignoring %s() -- calls %s() and NOT %s()>\n",
> - spt->name, sp->name,
> - bt->call_target);
> - continue;
> - }
> - }
> -
> - switch (x86_64_print_stack_entry(bt, ofp, level, i,*up))
> - {
> - case BACKTRACE_ENTRY_AND_EFRAME_DISPLAYED:
> - last_process_stack_eframe = rsp + 8;
> - if (x86_64_print_eframe_location(last_process_stack_eframe, level, ofp))
> - level++;
> - rsp += SIZE(pt_regs);
> - i += SIZE(pt_regs)/sizeof(ulong);
> - case BACKTRACE_ENTRY_DISPLAYED:
> - level++;
> - break;
> - case BACKTRACE_ENTRY_IGNORED:
> - break;
> - case BACKTRACE_COMPLETE:
> - done = TRUE;
> - break;
> - }
> - }
> + if ((kt->flags & DWARF_UNWIND) && !done) {
> + level = dwarf_backtrace(bt, level, bt->stacktop);
> + done = TRUE;
> + }
>
> if (!irq_eframe && !is_kernel_thread(bt->tc->task) &&
> (GET_STACKBASE(bt->tc->task) == bt->stackbase)) {
> @@ -3193,6 +3063,13 @@ x86_64_exception_frame(ulong flags, ulon
> x86_64_do_bt_reference_check(bt, r15, NULL);
> }
>
> + /* Remember the rip and rsp for unwinding the process stack */
> + if (kt->flags & DWARF_UNWIND){
> + bt->instptr = rip;
> + bt->stkptr = rsp;
> + bt->bptr = rbp;
> + }
> +
> if (kvaddr)
> FREEBUF(pt_regs_buf);
>
> diff -puN unwind_x86_32_64.c~cfi_backtrace_minor_fixes unwind_x86_32_64.c
> --- crash-4.0-3.9/unwind_x86_32_64.c~cfi_backtrace_minor_fixes 2006-11-15 18:35:50.519688064 +0530
> +++ crash-4.0-3.9-rachita/unwind_x86_32_64.c 2006-11-15 19:31:45.705622272 +0530
> @@ -1036,9 +1036,8 @@ dump_local_unwind_tables(void)
>
>
> int
> -dwarf_backtrace(struct bt_info *bt, ulong stacktop)
> +dwarf_backtrace(struct bt_info *bt, int level, ulong stacktop)
> {
> - int n = 0;
> unsigned long bp, offset;
> struct syment *sp;
> char *name;
> @@ -1051,7 +1050,7 @@ dwarf_backtrace(struct bt_info *bt, ulon
> UNW_PC(frame) = bt->instptr;
>
> /* read rbp from stack for non active tasks */
> - if (!(bt->flags & BT_DUMPFILE_SEARCH) ) {
> + if (!(bt->flags & BT_DUMPFILE_SEARCH) && !bt->bptr) {
> // readmem(frame->regs.rsp, KVADDR, &bp,
> readmem(UNW_SP(frame), KVADDR, &bp,
> sizeof(unsigned long), "reading bp", FAULT_ON_ERROR);
> @@ -1084,7 +1083,7 @@ dwarf_backtrace(struct bt_info *bt, ulon
>
>
> name = sp->name;
> - fprintf(fp, " #0 [%016lx] %s at %016lx \n", UNW_SP(frame), name, UNW_PC(frame));
> + fprintf(fp, " #%d [%016lx] %s at %016lx \n", level, UNW_SP(frame), name, UNW_PC(frame));
>
> if (CRASHDEBUG(2))
> fprintf(fp, " < SP: %lx PC: %lx FP: %lx >\n", UNW_SP(frame),
> @@ -1092,7 +1091,12 @@ dwarf_backtrace(struct bt_info *bt, ulon
>
> while ((UNW_SP(frame) < stacktop)
> && !unwind(frame) && UNW_PC(frame)) {
> - n++;
> + /* To prevent rip pushed on IRQ stack being reported both
> + * both on the IRQ and process stacks
> + */
> + if ((bt->flags & BT_IRQSTACK) && (UNW_SP(frame) >= stacktop - 16))
> + break;
> + level++;
> sp = value_search(UNW_PC(frame), &offset);
> if (!sp) {
> if (CRASHDEBUG(1))
> @@ -1101,9 +1105,24 @@ dwarf_backtrace(struct bt_info *bt, ulon
> UNW_PC(frame));
> break;
> }
> +
> + /*
> + * If offset is zero, it means we have crossed over to the next
> + * function. Recalculate by adjusting the text address
> + */
> + if (!offset) {
> + sp = value_search(UNW_PC(frame) - 1, &offset);
> + if (!sp) {
> + if (CRASHDEBUG(1))
> + fprintf(fp,
> + "unwind: cannot find symbol for PC: %lx\n",
> + UNW_PC(frame)-1);
> + goto bailout;
> + }
> + }
> name = sp->name;
> - fprintf(fp, "%s#%d [%016lx] %s at %016lx \n", n < 10 ? " " : "",
> - n, UNW_SP(frame), name, UNW_PC(frame));
> + fprintf(fp, "%s#%d [%016lx] %s at %016lx \n", level < 10 ? " " : "",
> + level, UNW_SP(frame), name, UNW_PC(frame));
>
> if (CRASHDEBUG(2))
> fprintf(fp, " < SP: %lx PC: %lx FP: %lx >\n", UNW_SP(frame),
> @@ -1112,7 +1131,49 @@ dwarf_backtrace(struct bt_info *bt, ulon
>
> bailout:
> FREEBUF(frame);
> - return TRUE;
> + return ++level;
> +}
> +
> +int dwarf_print_stack_entry(struct bt_info *bt, int level)
> +{
> + int n = 0;
> + unsigned long offset;
> + struct syment *sp;
> + char *name;
> + struct unwind_frame_info *frame;
> +
> + frame = (struct unwind_frame_info *)GETBUF(sizeof(struct unwind_frame_info));
> + UNW_SP(frame) = bt->stkptr;
> + UNW_PC(frame) = bt->instptr;
> +
> + sp = value_search(UNW_PC(frame), &offset);
> + if (!sp) {
> + if (CRASHDEBUG(1))
> + fprintf(fp, "unwind: cannot find symbol for PC: %lx\n",
> + UNW_PC(frame));
> + goto bailout;
> + }
> +
> + /*
> + * If offset is zero, it means we have crossed over to the next
> + * function. Recalculate by adjusting the text address
> + */
> + if (!offset) {
> + sp = value_search(UNW_PC(frame) - 1, &offset);
> + if (!sp) {
> + if (CRASHDEBUG(1))
> + fprintf(fp,
> + "unwind: cannot find symbol for PC: %lx\n",
> + UNW_PC(frame)-1);
> + goto bailout;
> + }
> + }
> + name = sp->name;
> + fprintf(fp, " #%d [%016lx] %s at %016lx \n", level, UNW_SP(frame), name, UNW_PC(frame));
> +
> +bailout:
> + FREEBUF(frame);
> + return level;
> }
>
> void
> diff -puN defs.h~cfi_backtrace_minor_fixes defs.h
> --- crash-4.0-3.9/defs.h~cfi_backtrace_minor_fixes 2006-11-15 18:51:28.030164808 +0530
> +++ crash-4.0-3.9-rachita/defs.h 2006-11-15 18:51:46.272391568 +0530
> @@ -645,6 +645,7 @@ struct bt_info {
> ulonglong flags;
> ulong instptr;
> ulong stkptr;
> + ulong bptr;
> ulong stackbase;
> ulong stacktop;
> char *stackbuf;
> @@ -3626,7 +3627,7 @@ struct machine_specific {
> * unwind_x86_32_64.c
> */
> void init_unwind_table(void);
> -int dwarf_backtrace(struct bt_info *, ulong);
> +int dwarf_backtrace(struct bt_info *, int, ulong);
> void dwarf_debug(struct bt_info *);
>
> #endif
> _
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/crash-utility/attachments/20061115/e66258cb/attachment.htm>
More information about the Crash-utility
mailing list