From 75fd9878c957680563355533065c4ed6ddc4e2f2 Mon Sep 17 00:00:00 2001 From: Julian Seward Date: Fri, 9 Nov 2007 23:02:28 +0000 Subject: [PATCH] Merge (from branches/THRCHECK) the following amd64-linux stack unwind kludges^H^H^H^H^H^H^Henhancements: r6802: For VG_(record_ExeContext) et al, add a new parameter (first_ip_delta) which is added to the initial IP value before the stack is unwound. A safe value to pass is zero, which causes the existing behaviour to be unchanged. This is a kludge needed to work around the incomplete amd64 stack unwind info in glibc-2.5's clone() routine. r7059: Add a last-ditch heuristic-hack to the amd64-linux stack unwinder, which is used when all other methods fail. Seems like GDB has something similar. git-svn-id: svn://svn.valgrind.org/valgrind/trunk@7118 --- coregrind/m_execontext.c | 5 +++-- coregrind/m_stacktrace.c | 34 +++++++++++++++++++++++++++++++--- exp-omega/o_main.c | 10 ++++++---- include/pub_tool_execontext.h | 7 +++++-- massif/ms_main.c | 3 ++- memcheck/mc_main.c | 3 ++- memcheck/mc_malloc_wrappers.c | 8 ++++---- 7 files changed, 53 insertions(+), 17 deletions(-) diff --git a/coregrind/m_execontext.c b/coregrind/m_execontext.c index 1b7b52a7e3..0f203793b1 100644 --- a/coregrind/m_execontext.c +++ b/coregrind/m_execontext.c @@ -277,7 +277,7 @@ static void resize_ec_htab ( void ) ec_htab_size_idx++; } -ExeContext* VG_(record_ExeContext) ( ThreadId tid ) +ExeContext* VG_(record_ExeContext) ( ThreadId tid, Word first_ip_delta ) { Int i; Addr ips[VG_DEEPEST_BACKTRACE]; @@ -297,7 +297,8 @@ ExeContext* VG_(record_ExeContext) ( ThreadId tid ) vg_assert(VG_(clo_backtrace_size) >= 1 && VG_(clo_backtrace_size) <= VG_DEEPEST_BACKTRACE); - n_ips = VG_(get_StackTrace)( tid, ips, VG_(clo_backtrace_size) ); + n_ips = VG_(get_StackTrace)( tid, ips, VG_(clo_backtrace_size), + first_ip_delta ); tl_assert(n_ips >= 1 && n_ips <= VG_(clo_backtrace_size)); /* Now figure out if we've seen this one before. First hash it so diff --git a/coregrind/m_stacktrace.c b/coregrind/m_stacktrace.c index 18b8a3574a..1c474a5d33 100644 --- a/coregrind/m_stacktrace.c +++ b/coregrind/m_stacktrace.c @@ -240,7 +240,29 @@ UInt VG_(get_StackTrace2) ( ThreadId tid_if_known, continue; } - /* No luck there. We have to give up. */ + /* Last-ditch hack (evidently GDB does something similar). We + are in the middle of nowhere and we have a nonsense value for + the frame pointer. If the stack pointer is still valid, + assume that what it points at is a return address. Yes, + desperate measures. Could do better here: + - check that the supposed return address is in + an executable page + - check that the supposed return address is just after a call insn + - given those two checks, don't just consider *sp as the return + address; instead scan a likely section of stack (eg sp .. sp+256) + and use suitable values found there. + */ + if (fp_min <= sp && sp < fp_max) { + ip = ((UWord*)sp)[0]; + ips[i++] = ip; + if (debug) + VG_(printf)(" ipsH[%d]=%08p\n", i-1, ips[i-1]); + ip = ip - 1; + sp += 8; + continue; + } + + /* No luck at all. We have to give up. */ break; } @@ -369,7 +391,8 @@ UInt VG_(get_StackTrace2) ( ThreadId tid_if_known, return n_found; } -UInt VG_(get_StackTrace) ( ThreadId tid, StackTrace ips, UInt n_ips ) +UInt VG_(get_StackTrace) ( ThreadId tid, StackTrace ips, UInt n_ips, + Word first_ip_delta ) { /* thread in thread table */ Addr ip = VG_(get_IP)(tid); @@ -405,6 +428,10 @@ UInt VG_(get_StackTrace) ( ThreadId tid, StackTrace ips, UInt n_ips ) } # endif + /* Take into account the first_ip_delta. */ + vg_assert( sizeof(Addr) == sizeof(Word) ); + ip += first_ip_delta; + if (0) VG_(printf)("tid %d: stack_highest=0x%08lx ip=0x%08lx sp=0x%08lx fp=0x%08lx\n", tid, stack_highest_word, ip, sp, fp); @@ -446,7 +473,8 @@ void VG_(pp_StackTrace) ( StackTrace ips, UInt n_ips ) void VG_(get_and_pp_StackTrace) ( ThreadId tid, UInt n_ips ) { Addr ips[n_ips]; - UInt n_ips_obtained = VG_(get_StackTrace)(tid, ips, n_ips); + UInt n_ips_obtained = VG_(get_StackTrace)(tid, ips, n_ips, + 0/*first_ip_delta*/); VG_(pp_StackTrace)(ips, n_ips_obtained); } diff --git a/exp-omega/o_main.c b/exp-omega/o_main.c index 6ffb251f36..0215f7863c 100644 --- a/exp-omega/o_main.c +++ b/exp-omega/o_main.c @@ -15,7 +15,7 @@ providing moral support.) Partly based upon other Valgrind tools - Copyright (C) 2000-2006 Julian Seward, Nicholas Nethercote et al. + Copyright (C) 2000-2007 Julian Seward, Nicholas Nethercote et al. jseward@acm.org njn@valgrind.org @@ -1582,7 +1582,8 @@ static void o_doLeakReport(MemBlock *mb) { O_MDEBUG("creating new context maybeLast=0"); // Get the current stacktrace - mb->leaked = VG_(record_ExeContext)(VG_(get_running_tid)()); + mb->leaked = VG_(record_ExeContext)(VG_(get_running_tid)(), + 0/*first_ip_delta*/); } doReport = o_addLeakedBlock(mb->where, mb->leaked, mb->length); @@ -1945,7 +1946,7 @@ static void o_createMemBlock(ThreadId tid, Addr start, SizeT size) */ mb->hdr.key = start; mb->length = size; - mb->where = VG_(record_ExeContext)(tid); + mb->where = VG_(record_ExeContext)(tid, 0/*first_ip_delta*/); /* O_DEBUG("Creating new MemBlock (%p) key = %p, length %d", @@ -2036,7 +2037,8 @@ static void o_setupMaybeLast(Addr a) if(!tp->memBlock->maybeLast) { tp->memBlock->maybeLast = tp; - tp->memBlock->funcEnd = VG_(record_ExeContext)(VG_(get_running_tid)()); + tp->memBlock->funcEnd = VG_(record_ExeContext)(VG_(get_running_tid)(), + 0/*first_ip_delta*/); O_MDEBUG("setting maybeLast to %p in block at %p", FROM_TRACKED_KEY(tp->hdr.key), tp->block); } diff --git a/include/pub_tool_execontext.h b/include/pub_tool_execontext.h index 3e598ea327..917281c62f 100644 --- a/include/pub_tool_execontext.h +++ b/include/pub_tool_execontext.h @@ -51,8 +51,11 @@ typedef // // If called from generated code, use VG_(get_running_tid)() to get the // current ThreadId. If called from non-generated code, the current -// ThreadId should be passed in by the core. -extern ExeContext* VG_(record_ExeContext) ( ThreadId tid ); +// ThreadId should be passed in by the core. The initial IP value to +// use is adjusted by first_ip_delta before the stack is unwound. +// A safe value to pass is zero. +extern +ExeContext* VG_(record_ExeContext) ( ThreadId tid, Word first_ip_delta ); // Apply a function to every element in the ExeContext. The parameter 'n' // gives the index of the passed ip. Doesn't go below main() unless diff --git a/massif/ms_main.c b/massif/ms_main.c index b197afd013..5633dbe9e9 100644 --- a/massif/ms_main.c +++ b/massif/ms_main.c @@ -875,7 +875,8 @@ Int get_IPs( ThreadId tid, Bool is_custom_alloc, Addr ips[]) VG_(tool_panic)("get_IPs: ips[] too small, inc. MAX_OVERESTIMATE?"); // Ask for more IPs than clo_depth suggests we need. - n_ips = VG_(get_StackTrace)( tid, ips, clo_depth + overestimate ); + n_ips = VG_(get_StackTrace)( tid, ips, clo_depth + overestimate, + 0/*first_ip_delta*/ ); tl_assert(n_ips > 0); // If the original stack trace is smaller than asked-for, redo=False. diff --git a/memcheck/mc_main.c b/memcheck/mc_main.c index 06546d25b2..bc9f2e7af9 100644 --- a/memcheck/mc_main.c +++ b/memcheck/mc_main.c @@ -4659,7 +4659,7 @@ static Bool mc_handle_client_request ( ThreadId tid, UWord* arg, UWord* ret ) cgbs[i].start = arg[1]; cgbs[i].size = arg[2]; cgbs[i].desc = VG_(strdup)((Char *)arg[3]); - cgbs[i].where = VG_(record_ExeContext) ( tid ); + cgbs[i].where = VG_(record_ExeContext) ( tid, 0/*first_ip_delta*/ ); *ret = i; } else @@ -4975,6 +4975,7 @@ static void mc_pre_clo_init(void) VG_(needs_core_errors) (); VG_(needs_tool_errors) (mc_eq_Error, mc_pp_Error, + True,/*show TIDs for errors*/ mc_update_extra, mc_recognised_suppression, mc_read_extra_suppression_info, diff --git a/memcheck/mc_malloc_wrappers.c b/memcheck/mc_malloc_wrappers.c index 7ed5dffece..aa9c68815f 100644 --- a/memcheck/mc_malloc_wrappers.c +++ b/memcheck/mc_malloc_wrappers.c @@ -131,7 +131,7 @@ MC_Chunk* create_MC_Chunk ( ThreadId tid, Addr p, SizeT szB, mc->data = p; mc->szB = szB; mc->allockind = kind; - mc->where = VG_(record_ExeContext)(tid); + mc->where = VG_(record_ExeContext)(tid, 0/*first_ip_delta*/); /* Paranoia ... ensure the MC_Chunk is off-limits to the client, so the mc->data field isn't visible to the leak checker. If memory @@ -271,7 +271,7 @@ void die_and_free_mem ( ThreadId tid, MC_Chunk* mc, SizeT rzB ) /* Put it out of harm's way for a while, if not from a client request */ if (MC_AllocCustom != mc->allockind) { /* Record where freed */ - mc->where = VG_(record_ExeContext) ( tid ); + mc->where = VG_(record_ExeContext) ( tid, 0/*first_ip_delta*/ ); add_to_freed_queue ( mc ); } else { VG_(free) ( mc ); @@ -349,14 +349,14 @@ void* MC_(realloc) ( ThreadId tid, void* p_old, SizeT new_szB ) if (old_szB == new_szB) { /* size unchanged */ - mc->where = VG_(record_ExeContext)(tid); + mc->where = VG_(record_ExeContext)(tid, 0/*first_ip_delta*/); p_new = p_old; } else if (old_szB > new_szB) { /* new size is smaller */ MC_(make_mem_noaccess)( mc->data+new_szB, mc->szB-new_szB ); mc->szB = new_szB; - mc->where = VG_(record_ExeContext)(tid); + mc->where = VG_(record_ExeContext)(tid, 0/*first_ip_delta*/); p_new = p_old; } else { -- 2.47.2