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 a5019735-40e9-0310-863c-91ae7b9d1cf9
diff --git a/coregrind/m_libcassert.c b/coregrind/m_libcassert.c
index 72a5517..ae8f406 100644
--- a/coregrind/m_libcassert.c
+++ b/coregrind/m_libcassert.c
@@ -94,7 +94,8 @@
}
__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 @@
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 @@
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 402f652..896aa15 100644
--- a/coregrind/m_machine.c
+++ b/coregrind/m_machine.c
@@ -53,6 +53,17 @@
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 b806f42..0aa4417 100644
--- a/coregrind/m_signals.c
+++ b/coregrind/m_signals.c
@@ -134,28 +134,31 @@
# 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 @@
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 41f15fa..5f3b6d7 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*));
- // 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.
+ /* 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. */
+
+ 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 @@
* 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)
- /* Try to derive a new (ip,sp,fp) triple from the current
- set. */
+ /* fp is %ebp/%rbp. sp is %esp/%rsp. ip is %eip/%rip. */
- /* 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;
- }
+ ips[0] = ip;
+ i = 1;
- /* 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;
- }
+ while (True) {
- /* 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;
}
+
+ /*--------------------- 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
+ }
+
+ 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;
-
- /* Put zeroes in the rest. */
- for (; i < n_ips; i++) {
- ips[i] = 0;
- }
VGP_POPCC(VgpExeContext);
-
return n_found;
}
@@ -171,6 +230,7 @@
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 @@
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 1453669..fced6f3 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 2f306a4..04e405b 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 127b6a9..1b24f0f 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 );