From: Julian Seward Date: Fri, 19 Aug 2005 16:02:59 +0000 (+0000) Subject: Incorporate a patch from Craig Chaney which gives better stack X-Git-Tag: svn/VALGRIND_3_1_0~560 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=ae8dfb45318bd5e979d6c573269d0219ed24fd87;p=thirdparty%2Fvalgrind.git Incorporate a patch from Craig Chaney which gives better stack snapshots on ppc32-linux in the presence of functions subject to leaf-function optimisations. At the same time, simplify the stack unwinding logic by basically implementing it separately for each target. Having a single piece of logic for amd64 and x86 was tenable, but merging ppc32 into it is too confusing. So now there is an x86/amd64 unwinder and a ppc32 unwinder. This requires plumbing a link-register value into VG_(get_StackTrace2), and that in turn requires passing it around several other stack-trace-related functions. Hence 7 changed files. git-svn-id: svn://svn.valgrind.org/valgrind/trunk@4464 --- diff --git a/coregrind/m_libcassert.c b/coregrind/m_libcassert.c index 72a55176fb..ae8f406680 100644 --- a/coregrind/m_libcassert.c +++ b/coregrind/m_libcassert.c @@ -94,7 +94,8 @@ static void pp_sched_status ( void ) } __attribute__ ((noreturn)) -static void report_and_quit ( const Char* report, Addr ip, Addr sp, Addr fp ) +static void report_and_quit ( const Char* report, + Addr ip, Addr sp, Addr fp, Addr lr ) { Addr stacktop; Addr ips[BACKTRACE_DEPTH]; @@ -111,7 +112,7 @@ static void report_and_quit ( const Char* report, Addr ip, Addr sp, Addr fp ) stacktop = tst->os_state.valgrind_stack_base + tst->os_state.valgrind_stack_szB; - VG_(get_StackTrace2)(ips, BACKTRACE_DEPTH, ip, sp, fp, sp, stacktop); + VG_(get_StackTrace2)(ips, BACKTRACE_DEPTH, ip, sp, fp, lr, sp, stacktop); VG_(pp_StackTrace) (ips, BACKTRACE_DEPTH); // Don't print this, as it's not terribly interesting and avoids a @@ -166,30 +167,30 @@ void VG_(assert_fail) ( Bool isCore, const Char* expr, const Char* file, if (!VG_STREQ(buf, "")) VG_(printf)("%s: %s\n", component, buf ); - report_and_quit(bugs_to, 0,0,0); + report_and_quit(bugs_to, 0,0,0,0); } __attribute__ ((noreturn)) static void panic ( Char* name, Char* report, Char* str, - Addr ip, Addr sp, Addr fp ) + Addr ip, Addr sp, Addr fp, Addr lr ) { VG_(printf)("\n%s: the 'impossible' happened:\n %s\n", name, str); - report_and_quit(report, ip, sp, fp); + report_and_quit(report, ip, sp, fp, lr); } -void VG_(core_panic_at) ( Char* str, Addr ip, Addr sp, Addr fp ) +void VG_(core_panic_at) ( Char* str, Addr ip, Addr sp, Addr fp, Addr lr ) { - panic("valgrind", VG_BUGS_TO, str, ip, sp, fp); + panic("valgrind", VG_BUGS_TO, str, ip, sp, fp, lr); } void VG_(core_panic) ( Char* str ) { - VG_(core_panic_at)(str, 0,0,0); + VG_(core_panic_at)(str, 0,0,0,0); } void VG_(tool_panic) ( Char* str ) { - panic(VG_(details).name, VG_(details).bug_reports_to, str, 0,0,0); + panic(VG_(details).name, VG_(details).bug_reports_to, str, 0,0,0,0); } /* Print some helpful-ish text about unimplemented things, and give up. */ diff --git a/coregrind/m_machine.c b/coregrind/m_machine.c index 402f652479..896aa15cbe 100644 --- a/coregrind/m_machine.c +++ b/coregrind/m_machine.c @@ -53,6 +53,17 @@ Addr VG_(get_FP) ( ThreadId tid ) return FRAME_PTR( VG_(threads)[tid].arch ); } +Addr VG_(get_LR) ( ThreadId tid ) +{ +# if defined(VGA_ppc32) + return VG_(threads)[tid].arch.vex.guest_LR; +# elif defined(VGA_x86) || defined(VGA_amd64) + return 0; +# else +# error "Unknown arch" +# endif +} + void VG_(set_SP) ( ThreadId tid, Addr sp ) { STACK_PTR( VG_(threads)[tid].arch ) = sp; diff --git a/coregrind/m_signals.c b/coregrind/m_signals.c index b806f421ac..0aa44176ab 100644 --- a/coregrind/m_signals.c +++ b/coregrind/m_signals.c @@ -134,28 +134,31 @@ typedef struct SigQueue { # define VG_UCONTEXT_STACK_PTR(uc) ((uc)->uc_mcontext.esp) # define VG_UCONTEXT_FRAME_PTR(uc) ((uc)->uc_mcontext.ebp) # define VG_UCONTEXT_SYSCALL_NUM(uc) ((uc)->uc_mcontext.eax) -# define VG_UCONTEXT_SYSCALL_SYSRES(uc) \ +# define VG_UCONTEXT_SYSCALL_SYSRES(uc) \ /* Convert the value in uc_mcontext.eax into a SysRes. */ \ VG_(mk_SysRes_x86_linux)( (uc)->uc_mcontext.eax ) +# define VG_UCONTEXT_LINK_REG(uc) 0 /* Dude, where's my LR? */ #elif defined(VGP_amd64_linux) # define VG_UCONTEXT_INSTR_PTR(uc) ((uc)->uc_mcontext.rip) # define VG_UCONTEXT_STACK_PTR(uc) ((uc)->uc_mcontext.rsp) # define VG_UCONTEXT_FRAME_PTR(uc) ((uc)->uc_mcontext.rbp) # define VG_UCONTEXT_SYSCALL_NUM(uc) ((uc)->uc_mcontext.rax) -# define VG_UCONTEXT_SYSCALL_SYSRES(uc) \ +# define VG_UCONTEXT_SYSCALL_SYSRES(uc) \ /* Convert the value in uc_mcontext.rax into a SysRes. */ \ VG_(mk_SysRes_amd64_linux)( (uc)->uc_mcontext.rax ) +# define VG_UCONTEXT_LINK_REG(uc) 0 /* No LR on amd64 either */ #elif defined(VGP_ppc32_linux) # define VG_UCONTEXT_INSTR_PTR(uc) ((uc)->uc_mcontext.mc_gregs[VKI_PT_NIP]) # define VG_UCONTEXT_STACK_PTR(uc) ((uc)->uc_mcontext.mc_gregs[1]) # define VG_UCONTEXT_FRAME_PTR(uc) ((uc)->uc_mcontext.mc_gregs[1]) # define VG_UCONTEXT_SYSCALL_NUM(uc) ((uc)->uc_mcontext.mc_gregs[0]) -# define VG_UCONTEXT_SYSCALL_SYSRES(uc) \ +# define VG_UCONTEXT_SYSCALL_SYSRES(uc) \ /* Convert the values in uc_mcontext r3,cr into a SysRes. */ \ VG_(mk_SysRes_ppc32_linux)( (uc)->uc_mcontext.mc_gregs[3], \ (uc)->uc_mcontext.mc_gregs[VKI_PT_CCR] ) +# define VG_UCONTEXT_LINK_REG(uc) ((uc)->uc_mcontext.mc_gregs[VKI_PT_LNK]) #else # error Unknown platform @@ -1966,7 +1969,8 @@ void sync_signalhandler ( Int sigNo, vki_siginfo_t *info, struct vki_ucontext *u VG_(core_panic_at)("Killed by fatal signal", VG_UCONTEXT_INSTR_PTR(uc), VG_UCONTEXT_STACK_PTR(uc), - VG_UCONTEXT_FRAME_PTR(uc)); + VG_UCONTEXT_FRAME_PTR(uc), + VG_UCONTEXT_LINK_REG(uc)); } } diff --git a/coregrind/m_stacktrace.c b/coregrind/m_stacktrace.c index 41f15fa9b5..5f3b6d7658 100644 --- a/coregrind/m_stacktrace.c +++ b/coregrind/m_stacktrace.c @@ -62,29 +62,32 @@ # error Unknown platform #endif -/* Take a snapshot of the client's stack, putting the up to 'n_ips' IPs - into 'ips'. In order to be thread-safe, we pass in the thread's IP - and FP. Returns number of IPs put in 'ips'. */ +/* Take a snapshot of the client's stack, putting the up to 'n_ips' + IPs into 'ips'. In order to be thread-safe, we pass in the + thread's IP SP, FP if that's meaningful, and LR if that's + meaningful. Returns number of IPs put in 'ips'. +*/ UInt VG_(get_StackTrace2) ( Addr* ips, UInt n_ips, - Addr ip, Addr sp, Addr fp, + Addr ip, Addr sp, Addr fp, Addr lr, Addr fp_min, Addr fp_max_orig ) { - static const Bool debug = False; - Int i; - Addr fp_max; - UInt n_found = 0; + Bool lr_is_first_RA = False; /* ppc only */ + Bool debug = False; + Int i; + Addr fp_max; + UInt n_found = 0; VGP_PUSHCC(VgpExeContext); - /* First snaffle IPs from the client's stack into ips[0 .. n_ips-1], - putting zeroes in when the trail goes cold, which we guess to be when - FP is not a reasonable stack location. We also assert that FP - increases down the chain. */ + vg_assert(sizeof(Addr) == sizeof(UWord)); + vg_assert(sizeof(Addr) == sizeof(void*)); + + /* Snaffle IPs from the client's stack into ips[0 .. n_ips-1], + putting zeroes in when the trail goes cold, which we guess to be + when FP is not a reasonable stack location. */ - // Gives shorter stack trace for tests/badjump.c - // JRS 2002-aug-16: I don't think this is a big deal; looks ok for - // most "normal" backtraces. - // NJN 2002-sep-05: traces for pthreaded programs are particularly bad. + for (i = 0; i < n_ips; i++) + ips[i] = 0; // JRS 2002-sep-17: hack, to round up fp_max to the end of the // current page, at least. Dunno if it helps. @@ -100,68 +103,124 @@ UInt VG_(get_StackTrace2) ( Addr* ips, UInt n_ips, * offending stack traces only have one item. --njn, 2002-aug-16 */ /* vg_assert(fp_min <= fp_max);*/ - ips[0] = ip; - i = 1; - if (fp_min + VG_(clo_max_stackframe) <= fp_max) { /* If the stack is ridiculously big, don't poke around ... but don't bomb out either. Needed to make John Regehr's user-space threads package work. JRS 20021001 */ - } else { + ips[0] = ip; + VGP_POPCC(VgpExeContext); + return 1; + } - fp = FIRST_STACK_FRAME(fp); + /* Otherwise unwind the stack in a platform-specific way. Trying + to merge the x86, amd64 and ppc32 logic into a single piece of + code is just too confusing. */ - while (True) { + /*--------------------- x86 and amd64 ---------------------*/ - if (i >= n_ips) - break; +# if defined(VGP_x86_linux) || defined(VGP_amd64_linux) + + /* fp is %ebp/%rbp. sp is %esp/%rsp. ip is %eip/%rip. */ + + ips[0] = ip; + i = 1; + + while (True) { - /* Try to derive a new (ip,sp,fp) triple from the current - set. */ - - /* First off, see if there is any CFI info to hand which can - be used. */ - if ( VG_(use_CFI_info)( &ip, &sp, &fp, fp_min, fp_max ) ) { - ips[i++] = ip; - if (debug) - VG_(printf)(" ipsC[%d]=%08p\n", i-1, ips[i-1]); - continue; - } - - /* If VG_(use_CFI_info) fails, it won't modify ip/sp/fp, so - we can safely try the old-fashioned method. */ - /* This bit is supposed to deal with frames resulting from - functions which begin "pushl% ebp ; movl %esp, %ebp" (x86) - or "pushq %rbp ; movq %rsp, %rbp" (amd64). Unfortunately, - since we can't (easily) look at the insns at the start of - the fn, like GDB does, there's no reliable way to tell. - Hence the hack of first trying out CFI, and if that fails, - then use this as a fallback. */ - if (fp_min <= fp && fp <= fp_max) { - /* fp looks sane, so use it. */ - ip = STACK_FRAME_RET(fp); - sp = fp + sizeof(Addr) /*saved %ebp/%rbp*/ - + sizeof(Addr) /*ra*/; - fp = STACK_FRAME_NEXT(fp); - ips[i++] = ip; - if (debug) - VG_(printf)(" ipsF[%d]=%08p\n", i-1, ips[i-1]); - continue; - } - - /* No luck there. We have to give up. */ + if (i >= n_ips) break; + + /* Try to derive a new (ip,sp,fp) triple from the current + set. */ + + /* First off, see if there is any CFI info to hand which can + be used. */ + if ( VG_(use_CFI_info)( &ip, &sp, &fp, fp_min, fp_max ) ) { + ips[i++] = ip; + if (debug) + VG_(printf)(" ipsC[%d]=%08p\n", i-1, ips[i-1]); + continue; } + /* If VG_(use_CFI_info) fails, it won't modify ip/sp/fp, so + we can safely try the old-fashioned method. */ + /* This bit is supposed to deal with frames resulting from + functions which begin "pushl% ebp ; movl %esp, %ebp" (x86) + or "pushq %rbp ; movq %rsp, %rbp" (amd64). Unfortunately, + since we can't (easily) look at the insns at the start of + the fn, like GDB does, there's no reliable way to tell. + Hence the hack of first trying out CFI, and if that fails, + then use this as a fallback. */ + if (fp_min <= fp && fp <= fp_max) { + /* fp looks sane, so use it. */ + ip = (((UWord*)fp)[1]); + sp = fp + sizeof(Addr) /*saved %ebp/%rbp*/ + + sizeof(Addr) /*ra*/; + fp = (((UWord*)fp)[0]); + ips[i++] = ip; + if (debug) + VG_(printf)(" ipsF[%d]=%08p\n", i-1, ips[i-1]); + continue; + } + + /* No luck there. We have to give up. */ + break; } - n_found = i; - /* Put zeroes in the rest. */ - for (; i < n_ips; i++) { - ips[i] = 0; + /*--------------------- ppc32 ---------------------*/ + +# elif defined(VGP_ppc32_linux) + + /* fp is %r1. ip is %cia. Note, ppc uses r1 as both the stack and + frame pointers. */ + + lr_is_first_RA = False; + { +# define M_VG_ERRTXT 1000 + UChar buf_lr[M_VG_ERRTXT], buf_ip[M_VG_ERRTXT]; + if (VG_(get_fnname_nodemangle) (lr, buf_lr, M_VG_ERRTXT)) + if (VG_(get_fnname_nodemangle) (ip, buf_ip, M_VG_ERRTXT)) + if (VG_(strncmp)(buf_lr, buf_ip, M_VG_ERRTXT)) + lr_is_first_RA = True; +# undef M_VG_ERRTXT } - VGP_POPCC(VgpExeContext); + ips[0] = ip; + i = 1; + fp = (((UWord*)fp)[0]); + + while (True) { + + if (i >= n_ips) + break; + + /* Try to derive a new (ip,fp) pair from the current set. */ + + if (fp_min <= fp && fp <= fp_max) { + /* fp looks sane, so use it. */ + + if (i == 1 && lr_is_first_RA) + ip = lr; + else + ip = (((UWord*)fp)[1]); + + fp = (((UWord*)fp)[0]); + ips[i++] = ip; + if (debug) + VG_(printf)(" ipsF[%d]=%08p\n", i-1, ips[i-1]); + continue; + } + + /* No luck there. We have to give up. */ + break; + } + +# else +# error "Unknown platform" +# endif + + n_found = i; + VGP_POPCC(VgpExeContext); return n_found; } @@ -171,6 +230,7 @@ UInt VG_(get_StackTrace) ( ThreadId tid, StackTrace ips, UInt n_ips ) Addr ip = VG_(get_IP)(tid); Addr fp = VG_(get_FP)(tid); Addr sp = VG_(get_SP)(tid); + Addr lr = VG_(get_LR)(tid); Addr stack_highest_word = VG_(threads)[tid].client_stack_highest_word; # if defined(VGP_x86_linux) @@ -194,7 +254,7 @@ UInt VG_(get_StackTrace) ( ThreadId tid, StackTrace ips, UInt n_ips ) VG_(printf)("tid %d: stack_highest=%p ip=%p sp=%p fp=%p\n", tid, stack_highest_word, ip, sp, fp); - return VG_(get_StackTrace2)(ips, n_ips, ip, sp, fp, sp, stack_highest_word); + return VG_(get_StackTrace2)(ips, n_ips, ip, sp, fp, lr, sp, stack_highest_word); } static void printIpDesc(UInt n, Addr ip) diff --git a/coregrind/pub_core_libcassert.h b/coregrind/pub_core_libcassert.h index 1453669690..fced6f3c0c 100644 --- a/coregrind/pub_core_libcassert.h +++ b/coregrind/pub_core_libcassert.h @@ -63,7 +63,8 @@ __attribute__ ((__noreturn__)) extern void VG_(core_panic) ( Char* str ); __attribute__ ((__noreturn__)) -extern void VG_(core_panic_at) ( Char* str, Addr ip, Addr sp, Addr fp ); +extern void VG_(core_panic_at) ( Char* str, + Addr ip, Addr sp, Addr fp, Addr lr ); /* Called when some unhandleable client behaviour is detected. Prints a msg and aborts. */ diff --git a/coregrind/pub_core_stacktrace.h b/coregrind/pub_core_stacktrace.h index 2f306a4476..04e405b69f 100644 --- a/coregrind/pub_core_stacktrace.h +++ b/coregrind/pub_core_stacktrace.h @@ -40,7 +40,7 @@ // Variant that gives a little more control over the stack-walking. extern UInt VG_(get_StackTrace2) ( StackTrace ips, UInt n_ips, - Addr ip, Addr sp, Addr fp, + Addr ip, Addr sp, Addr fp, Addr lr, Addr fp_min, Addr fp_max ); #endif // __PUB_CORE_STACKTRACE_H diff --git a/include/pub_tool_machine.h b/include/pub_tool_machine.h index 127b6a9185..1b24f0f74d 100644 --- a/include/pub_tool_machine.h +++ b/include/pub_tool_machine.h @@ -53,6 +53,7 @@ extern Addr VG_(get_SP) ( ThreadId tid ); extern Addr VG_(get_IP) ( ThreadId tid ); extern Addr VG_(get_FP) ( ThreadId tid ); +extern Addr VG_(get_LR) ( ThreadId tid ); extern void VG_(set_SP) ( ThreadId tid, Addr sp ); extern void VG_(set_IP) ( ThreadId tid, Addr ip );