[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH] x86: slightly improve stack trace on debug builds
>>> On 25.09.12 at 17:48, Keir Fraser <keir@xxxxxxx> wrote: > On 25/09/2012 16:07, "Jan Beulich" <JBeulich@xxxxxxxx> wrote: > >> + addr = regs->eip; >> + while ( !is_kernel_text(addr) && >> + (system_state > SYS_STATE_boot || !is_kernel_inittext(addr)) ) >> + { >> + /* Special case when a bad pointer was called. */ >> + addr ^= regs->eip ^ *ESP_BEFORE_EXCEPTION(regs); >> + if ( addr == regs->eip ) >> + break; >> + } > > Lol, how does your brain work this way? It took me 15 minutes to decode this > to something like (also I added range checks on ESP_BEFORE_EXCEPTION(regs), > what do you think?): Sorry for this - just wanted to avoid having the condition evaluated (and spelled out) twice (and it took me a minute at most to find this iterate-at-most-twice solution). With the helper function ... > bool_t is_current_kernel_text(unsigned long addr) > { > return (is_kernel_text(addr) || > (system_state == SYS_STATE_boot && is_kernel_inittext(addr))); > } ... it at least would be reasonable to read at the source level. So adding (and at once using at wherever else is appropriate) that helper function is certainly worthwhile. And while I like the way the rest was coded better than yours, I guess I ought to change it for your and future readers' sake. > /* > * If RIP is not valid hypervisor code then someone may have called into > * oblivion. Peek to see if they left a return address at top of stack. > */ > addr = (!is_current_kernel_text(regs->eip) && > (ESP_BEFORE_EXCEPTION(regs) >= low) && > (ESP_BEFORE_EXCEPTION(regs) < high) && > is_current_kernel_text(*ESP_BEFORE_EXCEPTION(regs))) > ? *ESP_BEFORE_EXCEPTION(regs) : regs->eip; Checking against "low" is pointless, as that one is guaranteed lower than the stack pointer (unless the stack pointer was within 16 bytes from NULL). Checking against "high" is almost pointless too, as there's only a very small range where %rsp could have been to make that check fail (plus the 2 slots offset wouldn't be right in this special case, i.e. we could get a false negative here), and it wouldn't help preventing eventual #PF on the deref (since "high" and the possible window above are on the same page). Jan _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |