From: Julian Seward Date: Thu, 11 Jan 2018 18:40:12 +0000 (+0100) Subject: Bug 79362 - Debug info is lost for .so files when they are dlclose'd. Majorly rework... X-Git-Tag: VALGRIND_3_14_0~183 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=cceed053ce876560b9a7512125dd93c7fa059778;p=thirdparty%2Fvalgrind.git Bug 79362 - Debug info is lost for .so files when they are dlclose'd. Majorly reworked by Philippe Waroquiers. --- diff --git a/.gitignore b/.gitignore index bb2cf9d255..81bbc1dad3 100644 --- a/.gitignore +++ b/.gitignore @@ -1075,6 +1075,8 @@ /memcheck/tests/linux/.deps /memcheck/tests/linux/brk /memcheck/tests/linux/capget +/memcheck/tests/linux/dlclose_leak +/memcheck/tests/linux/dlclose_leak_so.so /memcheck/tests/linux/getregset /memcheck/tests/linux/ioctl-tiocsig /memcheck/tests/linux/lsframe1 diff --git a/NEWS b/NEWS index e27c3ea7f8..46c4db2c24 100644 --- a/NEWS +++ b/NEWS @@ -13,6 +13,11 @@ support for X86/macOS 10.13, AMD64/macOS 10.13. * ==================== CORE CHANGES =================== +* The new option --keep-debuginfo=no|yes (default no) can be used to + keep symbols etc for unloaded code. This allows saved stack traces + (e.g. memory leaks) to include file/line info for code that has been + dlclose'd (or similar). See user manual for more information and + known limitations. * ================== PLATFORM CHANGES ================= @@ -43,6 +48,7 @@ To see details of a given bug, visit https://bugs.kde.org/show_bug.cgi?id=XXXXXX where XXXXXX is the bug number as listed below. +79362 Debug info is lost for .so files when they are dlclose'd 208052 strlcpy error when n = 0 255603 exp-sgcheck Assertion '!already_present' failed 376257 helgrind history full speed up using a cached stack diff --git a/cachegrind/cg_main.c b/cachegrind/cg_main.c index 5d44667f02..cd4de769c9 100644 --- a/cachegrind/cg_main.c +++ b/cachegrind/cg_main.c @@ -210,12 +210,14 @@ static HChar* get_perm_string(const HChar* s) static void get_debug_info(Addr instr_addr, const HChar **dir, const HChar **file, const HChar **fn, UInt* line) { + DiEpoch ep = VG_(current_DiEpoch)(); Bool found_file_line = VG_(get_filename_linenum)( + ep, instr_addr, file, dir, line ); - Bool found_fn = VG_(get_fnname)(instr_addr, fn); + Bool found_fn = VG_(get_fnname)(ep, instr_addr, fn); if (!found_file_line) { *file = "???"; diff --git a/callgrind/bb.c b/callgrind/bb.c index b5459cf04c..06dd771df8 100644 --- a/callgrind/bb.c +++ b/callgrind/bb.c @@ -199,7 +199,8 @@ obj_node* obj_of_address(Addr addr) DebugInfo* di; PtrdiffT offset; - di = VG_(find_DebugInfo)(addr); + DiEpoch ep = VG_(current_DiEpoch)(); + di = VG_(find_DebugInfo)(ep, addr); obj = CLG_(get_obj_node)( di ); /* Update symbol offset in object if remapped */ diff --git a/callgrind/dump.c b/callgrind/dump.c index 1997c7179c..12ecf66f6c 100644 --- a/callgrind/dump.c +++ b/callgrind/dump.c @@ -373,7 +373,8 @@ Bool get_debug_pos(BBCC* bbcc, Addr addr, AddrPos* p) found_file_line = debug_cache_info[cachepos]; } else { - found_file_line = VG_(get_filename_linenum)(addr, + DiEpoch ep = VG_(current_DiEpoch)(); + found_file_line = VG_(get_filename_linenum)(ep, addr, &file, &dir, &(p->line)); diff --git a/callgrind/fn.c b/callgrind/fn.c index d4c7b24c18..209c02190a 100644 --- a/callgrind/fn.c +++ b/callgrind/fn.c @@ -434,17 +434,18 @@ Bool CLG_(get_debug_info)(Addr instr_addr, CLG_DEBUG(6, " + get_debug_info(%#lx)\n", instr_addr); + DiEpoch ep = VG_(current_DiEpoch)(); if (pDebugInfo) { - *pDebugInfo = VG_(find_DebugInfo)(instr_addr); + *pDebugInfo = VG_(find_DebugInfo)(ep, instr_addr); // for generated code in anonymous space, pSegInfo is 0 } - found_file_line = VG_(get_filename_linenum)(instr_addr, + found_file_line = VG_(get_filename_linenum)(ep, instr_addr, file, dir, &line); - found_fn = VG_(get_fnname)(instr_addr, fn_name); + found_fn = VG_(get_fnname)(ep, instr_addr, fn_name); if (!found_file_line && !found_fn) { CLG_(stat).no_debug_BBs++; @@ -503,6 +504,7 @@ fn_node* CLG_(get_fn_node)(BB* bb) CLG_(get_debug_info)(bb_addr(bb), &dirname, &filename, &fnname, &line_num, &di); + DiEpoch ep = VG_(current_DiEpoch)(); if (0 == VG_(strcmp)(fnname, "???")) { int p; static HChar buf[32]; // for sure large enough @@ -521,7 +523,7 @@ fn_node* CLG_(get_fn_node)(BB* bb) fnname = buf; } else { - if (VG_(get_fnname_if_entry)(bb_addr(bb), &fnname)) + if (VG_(get_fnname_if_entry)(ep, bb_addr(bb), &fnname)) bb->is_entry = 1; } diff --git a/coregrind/m_addrinfo.c b/coregrind/m_addrinfo.c index c3edc13b6b..d22a99e58d 100644 --- a/coregrind/m_addrinfo.c +++ b/coregrind/m_addrinfo.c @@ -86,7 +86,7 @@ static ThreadId find_tid_with_stack_containing (Addr a) return VG_INVALID_THREADID; } -void VG_(describe_addr) ( Addr a, /*OUT*/AddrInfo* ai ) +void VG_(describe_addr) ( DiEpoch ep, Addr a, /*OUT*/AddrInfo* ai ) { VgSectKind sect; @@ -99,7 +99,7 @@ void VG_(describe_addr) ( Addr a, /*OUT*/AddrInfo* ai ) VG_(free), sizeof(HChar) ); (void) VG_(get_data_description)( ai->Addr.Variable.descr1, - ai->Addr.Variable.descr2, a ); + ai->Addr.Variable.descr2, ep, a ); /* If there's nothing in descr1/2, free them. Why is it safe to VG_(indexXA) at zero here? Because VG_(get_data_description) guarantees to zero terminate descr1/2 regardless of the outcome @@ -127,7 +127,7 @@ void VG_(describe_addr) ( Addr a, /*OUT*/AddrInfo* ai ) there. -- */ const HChar *name; if (VG_(get_datasym_and_offset)( - a, &name, + ep, a, &name, &ai->Addr.DataSym.offset )) { ai->Addr.DataSym.name = VG_(strdup)("mc.da.dsname", name); ai->tag = Addr_DataSym; @@ -148,6 +148,7 @@ void VG_(describe_addr) ( Addr a, /*OUT*/AddrInfo* ai ) ai->tag = Addr_Stack; VG_(initThreadInfo)(&ai->Addr.Stack.tinfo); ai->Addr.Stack.tinfo.tid = tid; + ai->Addr.Stack.epoch = ep; ai->Addr.Stack.IP = 0; ai->Addr.Stack.frameNo = -1; ai->Addr.Stack.stackPos = StackPos_stacked; @@ -248,6 +249,7 @@ void VG_(describe_addr) ( Addr a, /*OUT*/AddrInfo* ai ) ai->tag = Addr_Stack; VG_(initThreadInfo)(&ai->Addr.Stack.tinfo); ai->Addr.Stack.tinfo.tid = tid; + ai->Addr.Stack.epoch = ep; ai->Addr.Stack.IP = 0; ai->Addr.Stack.frameNo = -1; vg_assert (stackPos != StackPos_stacked); @@ -447,20 +449,24 @@ static void pp_addrinfo_WRK ( Addr a, const AddrInfo* ai, Bool mc, Bool haslinenum; PtrdiffT offset; - if (VG_(get_inst_offset_in_function)( ai->Addr.Stack.IP, + if (VG_(get_inst_offset_in_function)( ai->Addr.Stack.epoch, + ai->Addr.Stack.IP, &offset)) - haslinenum = VG_(get_linenum) (ai->Addr.Stack.IP - offset, + haslinenum = VG_(get_linenum) (ai->Addr.Stack.epoch, + ai->Addr.Stack.IP - offset, &linenum); else haslinenum = False; - hasfile = VG_(get_filename)(ai->Addr.Stack.IP, &file); + hasfile = VG_(get_filename)(ai->Addr.Stack.epoch, + ai->Addr.Stack.IP, &file); HChar strlinenum[16] = ""; // large enough if (hasfile && haslinenum) VG_(sprintf)(strlinenum, "%u", linenum); - hasfn = VG_(get_fnname)(ai->Addr.Stack.IP, &fn); + hasfn = VG_(get_fnname)(ai->Addr.Stack.epoch, + ai->Addr.Stack.IP, &fn); if (hasfn || hasfile) VG_(emit)( "%sin frame #%d, created by %ps (%ps:%s)%s\n", @@ -603,7 +609,7 @@ static void pp_addrinfo_WRK ( Addr a, const AddrInfo* ai, Bool mc, if (ai->Addr.SectKind.kind == Vg_SectText) { /* To better describe the address in a text segment, pp a dummy stacktrace made of this single address. */ - VG_(pp_StackTrace)( &a, 1 ); + VG_(pp_StackTrace)( VG_(current_DiEpoch)(), &a, 1 ); } break; diff --git a/coregrind/m_debuginfo/debuginfo.c b/coregrind/m_debuginfo/debuginfo.c index e5ce4bef99..d481458436 100644 --- a/coregrind/m_debuginfo/debuginfo.c +++ b/coregrind/m_debuginfo/debuginfo.c @@ -47,6 +47,7 @@ #include "pub_core_machine.h" // VG_PLAT_USES_PPCTOC #include "pub_core_xarray.h" #include "pub_core_oset.h" +#include "pub_core_execontext.h" #include "pub_core_stacktrace.h" // VG_(get_StackTrace) XXX: circular dependency #include "pub_core_ume.h" @@ -70,6 +71,10 @@ should-we-load-debuginfo-now? finite state machine. */ #define DEBUG_FSM 0 +/* Set this to 1 to enable somewhat minimal debug printing for the + debuginfo-epoch machinery. */ +#define DEBUG_EPOCHS 0 + /*------------------------------------------------------------*/ /*--- The _svma / _avma / _image / _bias naming scheme ---*/ @@ -108,6 +113,110 @@ static void caches__invalidate (void); +/*------------------------------------------------------------*/ +/*--- Epochs ---*/ +/*------------------------------------------------------------*/ + +/* The DebugInfo epoch is incremented every time we either load debuginfo in + response to an object mapping, or an existing DebugInfo becomes + non-current (or will be discarded) due to an object unmap. By storing, + in each DebugInfo, the first and last epoch for which it is valid, we can + unambiguously identify the set of DebugInfos which should be used to + provide metadata for a code or data address, provided we know the epoch + to which that address pertains. + + Note, this isn't the same as the "handle_counter" below. That only + advances when new DebugInfos are created. "current_epoch" advances both + at DebugInfo created and destruction-or-making-non-current. +*/ + +// The value zero is reserved for indicating an invalid epoch number. +static UInt current_epoch = 1; + +inline DiEpoch VG_(current_DiEpoch) ( void ) { + DiEpoch dep; dep.n = current_epoch; return dep; +} + +static void advance_current_DiEpoch ( const HChar* msg ) { + current_epoch++; + if (DEBUG_EPOCHS) + VG_(printf)("Advancing current epoch to %u due to %s\n", + current_epoch, msg); +} + +static inline Bool eq_DiEpoch ( DiEpoch dep1, DiEpoch dep2 ) { + return dep1.n == dep2.n && /*neither is invalid*/dep1.n != 0; +} + +// Is this DebugInfo currently "allocated" (pre-use state, only FSM active) ? +static inline Bool is_DebugInfo_allocated ( const DebugInfo* di ) +{ + if (is_DiEpoch_INVALID(di->first_epoch) + && is_DiEpoch_INVALID(di->last_epoch)) { + return True; + } else { + return False; + } +} + +// Is this DebugInfo currently "active" (valid for the current epoch) ? +static inline Bool is_DebugInfo_active ( const DebugInfo* di ) +{ + if (!is_DiEpoch_INVALID(di->first_epoch) + && is_DiEpoch_INVALID(di->last_epoch)) { + // Yes it is active. Sanity check .. + vg_assert(di->first_epoch.n <= current_epoch); + return True; + } else { + return False; + } +} + +// Is this DebugInfo currently "archived" ? +static inline Bool is_DebugInfo_archived ( const DebugInfo* di ) +{ + if (!is_DiEpoch_INVALID(di->first_epoch) + && !is_DiEpoch_INVALID(di->last_epoch)) { + // Yes it is archived. Sanity checks .. + vg_assert(di->first_epoch.n <= di->last_epoch.n); + vg_assert(di->last_epoch.n <= current_epoch); + return True; + } else { + return False; + } +} + +// Is this DebugInfo valid for the specified epoch? +static inline Bool is_DI_valid_for_epoch ( const DebugInfo* di, DiEpoch ep ) +{ + // Stay sane + vg_assert(ep.n > 0 && ep.n <= current_epoch); + + Bool first_valid = !is_DiEpoch_INVALID(di->first_epoch); + Bool last_valid = !is_DiEpoch_INVALID(di->last_epoch); + + if (first_valid) { + if (last_valid) { + // Both valid. di is in Archived state. + return di->first_epoch.n <= ep.n && ep.n <= di->last_epoch.n; + } else { + // First is valid, last is invalid. di is in Active state. + return di->first_epoch.n <= ep.n; + } + } else { + vg_assert (!last_valid); // First invalid, last valid is a bad state. + // Neither is valid. di is in Allocated state. + return False; + } + +} + +static inline UInt ROL32 ( UInt x, UInt n ) +{ + return (x << n) | (x >> (32-n)); +} + + /*------------------------------------------------------------*/ /*--- Root structure ---*/ /*------------------------------------------------------------*/ @@ -162,6 +271,23 @@ static void move_DebugInfo_one_step_forward ( DebugInfo* di ) } +// Debugging helper for epochs +static void show_epochs ( const HChar* msg ) +{ + if (DEBUG_EPOCHS) { + DebugInfo* di; + VG_(printf)("\nDebugInfo epoch display, requested by \"%s\"\n", msg); + VG_(printf)(" Current epoch (note: 0 means \"invalid epoch\") = %u\n", + current_epoch); + for (di = debugInfo_list; di; di = di->next) { + VG_(printf)(" [di=%p] first %u last %u %s\n", + di, di->first_epoch.n, di->last_epoch.n, di->fsm.filename); + } + VG_(printf)("\n"); + } +} + + /*------------------------------------------------------------*/ /*--- Notification (acquire/discard) helpers ---*/ /*------------------------------------------------------------*/ @@ -182,6 +308,8 @@ DebugInfo* alloc_DebugInfo( const HChar* filename ) di = ML_(dinfo_zalloc)("di.debuginfo.aDI.1", sizeof(DebugInfo)); di->handle = handle_counter++; + di->first_epoch = DiEpoch_INVALID(); + di->last_epoch = DiEpoch_INVALID(); di->fsm.filename = ML_(dinfo_strdup)("di.debuginfo.aDI.2", filename); di->fsm.maps = VG_(newXA)( ML_(dinfo_zalloc), "di.debuginfo.aDI.3", @@ -302,34 +430,54 @@ static void free_DebugInfo ( DebugInfo* di ) } -/* 'si' is a member of debugInfo_list. Find it, remove it from the - list, notify m_redir that this has happened, and free all storage - reachable from it. +/* 'di' is a member of debugInfo_list. Find it, and either (remove it from + the list and free all storage reachable from it) or archive it. + Notify m_redir that this removal/archiving has happened. + + Note that 'di' can't be archived. Is a DebugInfo is archived then we + want to hold on to it forever. This is asserted for. + + Note also, we don't advance the current epoch here. That's the + responsibility of some (non-immediate) caller. */ -static void discard_DebugInfo ( DebugInfo* di ) +static void discard_or_archive_DebugInfo ( DebugInfo* di ) { - const HChar* reason = "munmap"; + const HChar* reason = "munmap"; + const Bool archive = VG_(clo_keep_debuginfo); DebugInfo** prev_next_ptr = &debugInfo_list; DebugInfo* curr = debugInfo_list; + /* It must be active! */ + vg_assert( is_DebugInfo_active(di)); while (curr) { if (curr == di) { - /* Found it; remove from list and free it. */ + /* Found it; (remove from list and free it), or archive it. */ if (curr->have_dinfo && (VG_(clo_verbosity) > 1 || VG_(clo_trace_redir))) VG_(message)(Vg_DebugMsg, - "Discarding syms at %#lx-%#lx in %s due to %s()\n", + "%s syms at %#lx-%#lx in %s due to %s()\n", + archive ? "Archiving" : "Discarding", di->text_avma, di->text_avma + di->text_size, curr->fsm.filename ? curr->fsm.filename : "???", reason); vg_assert(*prev_next_ptr == curr); - *prev_next_ptr = curr->next; - if (curr->have_dinfo) + if (!archive) { + *prev_next_ptr = curr->next; + } + if (curr->have_dinfo) { VG_(redir_notify_delete_DebugInfo)( curr ); - free_DebugInfo(curr); + } + if (archive) { + /* Adjust the epoch markers appropriately. */ + di->last_epoch = VG_(current_DiEpoch)(); + VG_(archive_ExeContext_in_range) (di->last_epoch, + di->text_avma, di->text_size); + } else { + free_DebugInfo(curr); + } return; } prev_next_ptr = &curr->next; @@ -358,10 +506,12 @@ static Bool discard_syms_in_range ( Addr start, SizeT length ) while (True) { if (curr == NULL) break; - if (curr->text_present - && curr->text_size > 0 - && (start+length - 1 < curr->text_avma - || curr->text_avma + curr->text_size - 1 < start)) { + if (is_DebugInfo_archived(curr) + || !curr->text_present + || (curr->text_present + && curr->text_size > 0 + && (start+length - 1 < curr->text_avma + || curr->text_avma + curr->text_size - 1 < start))) { /* no overlap */ } else { found = True; @@ -372,7 +522,7 @@ static Bool discard_syms_in_range ( Addr start, SizeT length ) if (!found) break; anyFound = True; - discard_DebugInfo( curr ); + discard_or_archive_DebugInfo( curr ); } return anyFound; @@ -418,9 +568,9 @@ static Bool do_DebugInfos_overlap ( const DebugInfo* di1, const DebugInfo* di2 ) } -/* Discard all elements of debugInfo_list whose .mark bit is set. +/* Discard or archive all elements of debugInfo_list whose .mark bit is set. */ -static void discard_marked_DebugInfos ( void ) +static void discard_or_archive_marked_DebugInfos ( void ) { DebugInfo* curr; @@ -436,7 +586,7 @@ static void discard_marked_DebugInfos ( void ) } if (!curr) break; - discard_DebugInfo( curr ); + discard_or_archive_DebugInfo( curr ); } } @@ -446,19 +596,22 @@ static void discard_marked_DebugInfos ( void ) Clearly diRef must have its mapping information set to something sane. */ static void discard_DebugInfos_which_overlap_with ( DebugInfo* diRef ) { + vg_assert(is_DebugInfo_allocated(diRef)); DebugInfo* di; /* Mark all the DebugInfos in debugInfo_list that need to be deleted. First, clear all the mark bits; then set them if they overlap with siRef. Since siRef itself is in this list we at least expect its own mark bit to be set. */ for (di = debugInfo_list; di; di = di->next) { + if (is_DebugInfo_archived(di)) + continue; di->mark = do_DebugInfos_overlap( di, diRef ); if (di == diRef) { vg_assert(di->mark); di->mark = False; } } - discard_marked_DebugInfos(); + discard_or_archive_marked_DebugInfos(); } @@ -470,6 +623,8 @@ static DebugInfo* find_or_create_DebugInfo_for ( const HChar* filename ) DebugInfo* di; vg_assert(filename); for (di = debugInfo_list; di; di = di->next) { + if (is_DebugInfo_archived(di)) + continue; vg_assert(di->fsm.filename); if (0==VG_(strcmp)(di->fsm.filename, filename)) break; @@ -480,6 +635,7 @@ static DebugInfo* find_or_create_DebugInfo_for ( const HChar* filename ) di->next = debugInfo_list; debugInfo_list = di; } + vg_assert(!is_DebugInfo_archived(di)); return di; } @@ -723,6 +879,8 @@ static ULong di_notify_ACHIEVE_ACCEPT_STATE ( struct _DebugInfo* di ) ULong di_handle; Bool ok; + advance_current_DiEpoch("di_notify_ACHIEVE_ACCEPT_STATE"); + vg_assert(di->fsm.filename); TRACE_SYMTAB("\n"); TRACE_SYMTAB("------ start ELF OBJECT " @@ -734,7 +892,8 @@ static ULong di_notify_ACHIEVE_ACCEPT_STATE ( struct _DebugInfo* di ) /* We're going to read symbols and debug info for the avma ranges specified in the _DebugInfoFsm mapping array. First get rid of any other DebugInfos which overlap any of those - ranges (to avoid total confusion). */ + ranges (to avoid total confusion). But only those valid in + the current epoch. We don't want to discard archived DebugInfos. */ discard_DebugInfos_which_overlap_with( di ); /* The DebugInfoMappings that now exist in the FSM may involve @@ -765,6 +924,15 @@ static ULong di_notify_ACHIEVE_ACCEPT_STATE ( struct _DebugInfo* di ) priv_storage.h. */ check_CFSI_related_invariants(di); ML_(finish_CFSI_arrays)(di); + + // Mark di's first epoch point as a valid epoch. Because its + // last_epoch value is still invalid, this changes di's state from + // "allocated" to "active". + vg_assert(is_DebugInfo_allocated(di)); + di->first_epoch = VG_(current_DiEpoch)(); + vg_assert(is_DebugInfo_active(di)); + show_epochs("di_notify_ACHIEVE_ACCEPT_STATE success"); + /* notify m_redir about it */ TRACE_SYMTAB("\n------ Notifying m_redir ------\n"); VG_(redir_notify_new_DebugInfo)( di ); @@ -1077,8 +1245,11 @@ void VG_(di_notify_munmap)( Addr a, SizeT len ) Bool anyFound; if (0) VG_(printf)("DISCARD %#lx %#lx\n", a, a+len); anyFound = discard_syms_in_range(a, len); - if (anyFound) + if (anyFound) { caches__invalidate(); + advance_current_DiEpoch("VG_(di_notify_munmap)"); + show_epochs("VG_(di_notify_munmap)"); + } } @@ -1094,8 +1265,10 @@ void VG_(di_notify_mprotect)( Addr a, SizeT len, UInt prot ) # endif if (0 && !exe_ok) { Bool anyFound = discard_syms_in_range(a, len); - if (anyFound) + if (anyFound) { caches__invalidate(); + advance_current_DiEpoch("VG_(di_notify_mprotect)"); + } } } @@ -1395,6 +1568,7 @@ void VG_(di_notify_pdb_debuginfo)( Int fd_obj, Addr avma_obj, caches__invalidate(); /* dump old info for this range, if any */ discard_syms_in_range( avma_obj, total_size ); + advance_current_DiEpoch("VG_(di_notify_pdb_debuginfo)"); { DebugInfo* di = find_or_create_DebugInfo_for(exename); @@ -1415,7 +1589,14 @@ void VG_(di_notify_pdb_debuginfo)( Int fd_obj, Addr avma_obj, } else { VG_(message)(Vg_UserMsg, "LOAD_PDB_DEBUGINFO: failed loading info " "from %s\n", pdbname); - discard_DebugInfo (di); + /* We cannot make any sense of this pdb, so (force) discard it, + even if VG_(clo_keep_debuginfo) is True. */ + const Bool save_clo_keep_debuginfo = VG_(clo_keep_debuginfo); + VG_(clo_keep_debuginfo) = False; + // The below will assert if di is not active. Not too sure what + // the state of di in this failed loading state. + discard_or_archive_DebugInfo (di); + VG_(clo_keep_debuginfo) = save_clo_keep_debuginfo; } VG_(am_munmap_valgrind)( (Addr)pdbimage, n_pdbimage ); VG_(close)(fd_pdbimage); @@ -1474,6 +1655,7 @@ DebugInfoMapping* ML_(find_rx_mapping) ( DebugInfo* di, Addr lo, Addr hi ) /*------------------------------------------------------------*/ /*--- Types and functions for inlined IP cursor ---*/ /*------------------------------------------------------------*/ + struct _InlIPCursor { Addr eip; // Cursor used to describe calls at eip. DebugInfo* di; // DebugInfo describing inlined calls at eip @@ -1537,8 +1719,8 @@ Bool VG_(next_IIPC)(InlIPCursor *iipc) } /* Forward */ -static void search_all_loctabs ( Addr ptr, /*OUT*/DebugInfo** pdi, - /*OUT*/Word* locno ); +static void search_all_loctabs ( DiEpoch ep, Addr ptr, + /*OUT*/DebugInfo** pdi, /*OUT*/Word* locno ); /* Returns the position after which eip would be inserted in inltab. (-1 if eip should be inserted before position 0). @@ -1568,7 +1750,7 @@ static Word inltab_insert_pos (DebugInfo *di, Addr eip) return lo - 1; } -InlIPCursor* VG_(new_IIPC)(Addr eip) +InlIPCursor* VG_(new_IIPC)(DiEpoch ep, Addr eip) { DebugInfo* di; Word locno; @@ -1579,8 +1761,8 @@ InlIPCursor* VG_(new_IIPC)(Addr eip) if (!VG_(clo_read_inline_info)) return NULL; // No way we can find inlined calls. - /* Search the DebugInfo for eip */ - search_all_loctabs ( eip, &di, &locno ); + /* Search the DebugInfo for (ep, eip) */ + search_all_loctabs ( ep, eip, &di, &locno ); if (di == NULL || di->inltab_used == 0) return NULL; // No di (with inltab) containing eip. @@ -1644,8 +1826,8 @@ void VG_(delete_IIPC)(InlIPCursor *iipc) If findText==True, only text symbols are searched for. If findText==False, only data symbols are searched for. */ -static void search_all_symtabs ( Addr ptr, /*OUT*/DebugInfo** pdi, - /*OUT*/Word* symno, +static void search_all_symtabs ( DiEpoch ep, Addr ptr, + /*OUT*/DebugInfo** pdi, /*OUT*/Word* symno, Bool findText ) { Word sno; @@ -1654,6 +1836,9 @@ static void search_all_symtabs ( Addr ptr, /*OUT*/DebugInfo** pdi, for (di = debugInfo_list; di != NULL; di = di->next) { + if (!is_DI_valid_for_epoch(di, ep)) + continue; + if (findText) { /* Consider any symbol in the r-x mapped area to be text. See Comment_Regarding_Text_Range_Checks in storage.c for @@ -1701,15 +1886,17 @@ static void search_all_symtabs ( Addr ptr, /*OUT*/DebugInfo** pdi, } -/* Search all loctabs that we know about to locate ptr. If found, set - *pdi to the relevant DebugInfo, and *locno to the loctab entry +/* Search all loctabs that we know about to locate ptr at epoch ep. If + *found, set pdi to the relevant DebugInfo, and *locno to the loctab entry *number within that. If not found, *pdi is set to NULL. */ -static void search_all_loctabs ( Addr ptr, /*OUT*/DebugInfo** pdi, - /*OUT*/Word* locno ) +static void search_all_loctabs ( DiEpoch ep, Addr ptr, + /*OUT*/DebugInfo** pdi, /*OUT*/Word* locno ) { Word lno; DebugInfo* di; for (di = debugInfo_list; di != NULL; di = di->next) { + if (!is_DI_valid_for_epoch(di, ep)) + continue; if (di->text_present && di->text_size > 0 && di->text_avma <= ptr @@ -1732,19 +1919,22 @@ static void search_all_loctabs ( Addr ptr, /*OUT*/DebugInfo** pdi, typedef struct { - Addr sym_avma; + // (sym_epoch, sym_avma) are the hash table key. + DiEpoch sym_epoch; + Addr sym_avma; + // Fields below here are not part of the key. const HChar* sym_name; PtrdiffT offset : (sizeof(PtrdiffT)*8)-1; Bool isText : 1; } Sym_Name_CacheEnt; -/* Sym_Name_CacheEnt associates a queried address to the sym name found. - By nature, if a sym name was found, it means the searched address - stored in the cache is an avma (see e.g. search_all_symtabs). - Note however that the caller is responsibe to work with 'avma' - addresses e.g. when calling VG_(get_fnname) : m_debuginfo.c has - no way to differentiate an 'svma a' from an 'avma a'. It is however - unlikely that svma would percolate outside of this module. */ +/* Sym_Name_CacheEnt associates a queried (epoch, address) pair to the sym + name found. By nature, if a sym name was found, it means the searched + address stored in the cache is an avma (see e.g. search_all_symtabs). + Note however that the caller is responsible to work with 'avma' addresses + e.g. when calling VG_(get_fnname) : m_debuginfo.c has no way to + differentiate an 'svma a' from an 'avma a'. It is however unlikely that + svma would percolate outside of this module. */ static Sym_Name_CacheEnt sym_name_cache[N_SYM_NAME_CACHE]; @@ -1760,13 +1950,15 @@ static void sym_name_cache__invalidate ( void ) { sym_name_cache[0].sym_name = no_sym_name; } -/* The whole point of this whole big deal: map a code address to a - plausible symbol name. Returns False if no idea; otherwise True. +/* The whole point of this whole big deal: map an (epoch, code address) pair + to a plausible symbol name. Returns False if no idea; otherwise True. + Caller supplies buf. If do_cxx_demangling is False, don't do C++ demangling, regardless of VG_(clo_demangle) -- probably because the call has come from VG_(get_fnname_raw)(). findText indicates whether we're looking for a text symbol or a data symbol -- caller must choose one kind or the other. + NOTE: See IMPORTANT COMMENT above about persistence and ownership in pub_tool_debuginfo.h get_sym_name and the fact it calls the demangler is the main reason @@ -1775,22 +1967,32 @@ static void sym_name_cache__invalidate ( void ) { (1) the DebugInfo it belongs to is not discarded (2) the demangler is not invoked again Also, the returned string is owned by "somebody else". Callers must - not free it or modify it.*/ + not free it or modify it. */ static Bool get_sym_name ( Bool do_cxx_demangling, Bool do_z_demangling, Bool do_below_main_renaming, - Addr a, const HChar** buf, + DiEpoch ep, Addr a, const HChar** buf, Bool match_anywhere_in_sym, Bool show_offset, Bool findText, /*OUT*/PtrdiffT* offsetP ) { - UWord hash = a % N_SYM_NAME_CACHE; - Sym_Name_CacheEnt* se = &sym_name_cache[hash]; - - if (UNLIKELY(se->sym_avma != a || se->isText != findText)) { + // Compute the hash from 'ep' and 'a'. The latter contains lots of + // significant bits, but 'ep' is expected to be a small number, typically + // less than 500. So rotate it around a bit in the hope of spreading the + // bits out somewhat. + vg_assert(!is_DiEpoch_INVALID(ep)); + UWord hash = a ^ (UWord)(ep.n ^ ROL32(ep.n, 5) + ^ ROL32(ep.n, 13) ^ ROL32(ep.n, 19)); + hash %= N_SYM_NAME_CACHE; + + Sym_Name_CacheEnt* se = &sym_name_cache[hash]; + + if (UNLIKELY(se->sym_epoch.n != ep.n || se->sym_avma != a + || se->isText != findText)) { DebugInfo* di; Word sno; - search_all_symtabs ( a, &di, &sno, findText ); + search_all_symtabs ( ep, a, &di, &sno, findText ); + se->sym_epoch = ep; se->sym_avma = a; se->isText = findText; if (di == NULL || a == 0) @@ -1849,12 +2051,12 @@ Bool get_sym_name ( Bool do_cxx_demangling, Bool do_z_demangling, /* ppc64be-linux only: find the TOC pointer (R2 value) that should be in force at the entry point address of the function containing guest_code_addr. Returns 0 if not known. */ -Addr VG_(get_tocptr) ( Addr guest_code_addr ) +Addr VG_(get_tocptr) ( DiEpoch ep, Addr guest_code_addr ) { #if defined(VGA_ppc64be) || defined(VGA_ppc64le) DebugInfo* si; Word sno; - search_all_symtabs ( guest_code_addr, + search_all_symtabs ( ep, guest_code_addr, &si, &sno, True/*consider text symbols only*/ ); if (si == NULL) @@ -1870,11 +2072,11 @@ Addr VG_(get_tocptr) ( Addr guest_code_addr ) match anywhere in function, but don't show offsets. NOTE: See IMPORTANT COMMENT above about persistence and ownership in pub_tool_debuginfo.h */ -Bool VG_(get_fnname) ( Addr a, const HChar** buf ) +Bool VG_(get_fnname) ( DiEpoch ep, Addr a, const HChar** buf ) { return get_sym_name ( /*C++-demangle*/True, /*Z-demangle*/True, /*below-main-renaming*/True, - a, buf, + ep, a, buf, /*match_anywhere_in_fun*/True, /*show offset?*/False, /*text sym*/True, @@ -1885,11 +2087,11 @@ Bool VG_(get_fnname) ( Addr a, const HChar** buf ) match anywhere in function, and show offset if nonzero. NOTE: See IMPORTANT COMMENT above about persistence and ownership in pub_tool_debuginfo.h */ -Bool VG_(get_fnname_w_offset) ( Addr a, const HChar** buf ) +Bool VG_(get_fnname_w_offset) ( DiEpoch ep, Addr a, const HChar** buf ) { return get_sym_name ( /*C++-demangle*/True, /*Z-demangle*/True, /*below-main-renaming*/True, - a, buf, + ep, a, buf, /*match_anywhere_in_fun*/True, /*show offset?*/True, /*text sym*/True, @@ -1901,14 +2103,14 @@ Bool VG_(get_fnname_w_offset) ( Addr a, const HChar** buf ) and don't show offsets. NOTE: See IMPORTANT COMMENT above about persistence and ownership in pub_tool_debuginfo.h */ -Bool VG_(get_fnname_if_entry) ( Addr a, const HChar** buf ) +Bool VG_(get_fnname_if_entry) ( DiEpoch ep, Addr a, const HChar** buf ) { const HChar *tmp; Bool res; res = get_sym_name ( /*C++-demangle*/True, /*Z-demangle*/True, /*below-main-renaming*/True, - a, &tmp, + ep, a, &tmp, /*match_anywhere_in_fun*/False, /*show offset?*/False, /*text sym*/True, @@ -1923,11 +2125,11 @@ Bool VG_(get_fnname_if_entry) ( Addr a, const HChar** buf ) offsets. NOTE: See IMPORTANT COMMENT above about persistence and ownership in pub_tool_debuginfo.h */ -Bool VG_(get_fnname_raw) ( Addr a, const HChar** buf ) +Bool VG_(get_fnname_raw) ( DiEpoch ep, Addr a, const HChar** buf ) { return get_sym_name ( /*C++-demangle*/False, /*Z-demangle*/False, /*below-main-renaming*/False, - a, buf, + ep, a, buf, /*match_anywhere_in_fun*/True, /*show offset?*/False, /*text sym*/True, @@ -1939,14 +2141,23 @@ Bool VG_(get_fnname_raw) ( Addr a, const HChar** buf ) don't show offsets. NOTE: See IMPORTANT COMMENT above about persistence and ownership in pub_tool_debuginfo.h */ -Bool VG_(get_fnname_no_cxx_demangle) ( Addr a, const HChar** buf, +Bool VG_(get_fnname_no_cxx_demangle) ( DiEpoch ep, Addr a, const HChar** buf, const InlIPCursor* iipc ) { + // All the callers of VG_(get_fnname_no_cxx_demangle) must build + // the iipc with the same ep as provided to VG_(get_fnname_no_cxx_demangle). + // So, if we have an iipc, iipc->di must be valid in the provided ep. + // Functionally, we could equally use iipc->di->first_epoch or ep, as + // all the inlined fn calls will be described by the same di. + if (iipc) { + vg_assert(is_DI_valid_for_epoch(iipc->di, ep)); + } + if (is_bottom(iipc)) { // At the bottom (towards main), we describe the fn at eip. return get_sym_name ( /*C++-demangle*/False, /*Z-demangle*/True, /*below-main-renaming*/True, - a, buf, + ep, a, buf, /*match_anywhere_in_fun*/True, /*show offset?*/False, /*text sym*/True, @@ -1965,13 +2176,13 @@ Bool VG_(get_fnname_no_cxx_demangle) ( Addr a, const HChar** buf, /* mips-linux only: find the offset of current address. This is needed for stack unwinding for MIPS. */ -Bool VG_(get_inst_offset_in_function)( Addr a, +Bool VG_(get_inst_offset_in_function)( DiEpoch ep, Addr a, /*OUT*/PtrdiffT* offset ) { const HChar *fnname; return get_sym_name ( /*C++-demangle*/False, /*Z-demangle*/False, /*below-main-renaming*/False, - a, &fnname, + ep, a, &fnname, /*match_anywhere_in_sym*/True, /*show offset?*/False, /*text sym*/True, @@ -2006,13 +2217,13 @@ Vg_FnNameKind VG_(get_fnname_kind) ( const HChar* name ) } } -Vg_FnNameKind VG_(get_fnname_kind_from_IP) ( Addr ip ) +Vg_FnNameKind VG_(get_fnname_kind_from_IP) ( DiEpoch ep, Addr ip ) { const HChar *buf; // We don't demangle, because it's faster not to, and the special names // we're looking for won't be mangled. - if (VG_(get_fnname_raw) ( ip, &buf )) { + if (VG_(get_fnname_raw) ( ep, ip, &buf )) { return VG_(get_fnname_kind)(buf); } else { @@ -2025,13 +2236,13 @@ Vg_FnNameKind VG_(get_fnname_kind_from_IP) ( Addr ip ) Also data_addr's offset from the symbol start is put into *offset. NOTE: See IMPORTANT COMMENT above about persistence and ownership in pub_tool_debuginfo.h */ -Bool VG_(get_datasym_and_offset)( Addr data_addr, +Bool VG_(get_datasym_and_offset)( DiEpoch ep, Addr data_addr, /*OUT*/const HChar** dname, /*OUT*/PtrdiffT* offset ) { return get_sym_name ( /*C++-demangle*/False, /*Z-demangle*/False, /*below-main-renaming*/False, - data_addr, dname, + ep, data_addr, dname, /*match_anywhere_in_sym*/True, /*show offset?*/False, /*text sym*/False, @@ -2044,7 +2255,7 @@ Bool VG_(get_datasym_and_offset)( Addr data_addr, (1) the DebugInfo it belongs to is not discarded (2) the segment containing the address is not merged with another segment */ -Bool VG_(get_objname) ( Addr a, const HChar** objname ) +Bool VG_(get_objname) ( DiEpoch ep, Addr a, const HChar** objname ) { DebugInfo* di; const NSegment *seg; @@ -2053,6 +2264,8 @@ Bool VG_(get_objname) ( Addr a, const HChar** objname ) /* Look in the debugInfo_list to find the name. In most cases we expect this to produce a result. */ for (di = debugInfo_list; di != NULL; di = di->next) { + if (!is_DI_valid_for_epoch(di, ep)) + continue; if (di->text_present && di->text_size > 0 && di->text_avma <= a @@ -2065,8 +2278,13 @@ Bool VG_(get_objname) ( Addr a, const HChar** objname ) the debugInfo_list, ask the address space manager whether it knows the name of the file associated with this mapping. This allows us to print the names of exe/dll files in the stack trace - when running programs under wine. */ - if ( (seg = VG_(am_find_nsegment)(a)) != NULL + when running programs under wine. + + Restrict this to the case where 'ep' is the current epoch, though, so + that we don't return information about this epoch when the caller was + enquiring about a different one. */ + if ( eq_DiEpoch(ep, VG_(current_DiEpoch)()) + && (seg = VG_(am_find_nsegment)(a)) != NULL && (filename = VG_(am_get_filename)(seg)) != NULL ) { *objname = filename; return True; @@ -2076,12 +2294,14 @@ Bool VG_(get_objname) ( Addr a, const HChar** objname ) /* Map a code address to its DebugInfo. Returns NULL if not found. Doesn't require debug info. */ -DebugInfo* VG_(find_DebugInfo) ( Addr a ) +DebugInfo* VG_(find_DebugInfo) ( DiEpoch ep, Addr a ) { static UWord n_search = 0; DebugInfo* di; n_search++; for (di = debugInfo_list; di != NULL; di = di->next) { + if (!is_DI_valid_for_epoch(di, ep)) + continue; if (di->text_present && di->text_size > 0 && di->text_avma <= a @@ -2097,13 +2317,13 @@ DebugInfo* VG_(find_DebugInfo) ( Addr a ) /* Map a code address to a filename. Returns True if successful. The returned string is persistent as long as the DebugInfo to which it belongs is not discarded. */ -Bool VG_(get_filename)( Addr a, const HChar** filename ) +Bool VG_(get_filename)( DiEpoch ep, Addr a, const HChar** filename ) { DebugInfo* si; Word locno; UInt fndn_ix; - search_all_loctabs ( a, &si, &locno ); + search_all_loctabs ( ep, a, &si, &locno ); if (si == NULL) return False; fndn_ix = ML_(fndn_ix) (si, locno); @@ -2112,11 +2332,11 @@ Bool VG_(get_filename)( Addr a, const HChar** filename ) } /* Map a code address to a line number. Returns True if successful. */ -Bool VG_(get_linenum)( Addr a, UInt* lineno ) +Bool VG_(get_linenum)( DiEpoch ep, Addr a, UInt* lineno ) { DebugInfo* si; Word locno; - search_all_loctabs ( a, &si, &locno ); + search_all_loctabs ( ep, a, &si, &locno ); if (si == NULL) return False; *lineno = si->loctab[locno].lineno; @@ -2127,7 +2347,7 @@ Bool VG_(get_linenum)( Addr a, UInt* lineno ) /* Map a code address to a filename/line number/dir name info. See prototype for detailed description of behaviour. */ -Bool VG_(get_filename_linenum) ( Addr a, +Bool VG_(get_filename_linenum) ( DiEpoch ep, Addr a, /*OUT*/const HChar** filename, /*OUT*/const HChar** dirname, /*OUT*/UInt* lineno ) @@ -2136,7 +2356,7 @@ Bool VG_(get_filename_linenum) ( Addr a, Word locno; UInt fndn_ix; - search_all_loctabs ( a, &si, &locno ); + search_all_loctabs ( ep, a, &si, &locno ); if (si == NULL) { if (dirname) { *dirname = ""; @@ -2165,7 +2385,8 @@ Bool VG_(get_filename_linenum) ( Addr a, Therefore specify "*" to search all the objects. On TOC-afflicted platforms, a symbol is deemed to be found only if it has a nonzero TOC pointer. */ -Bool VG_(lookup_symbol_SLOW)(const HChar* sopatt, const HChar* name, +Bool VG_(lookup_symbol_SLOW)(DiEpoch ep, + const HChar* sopatt, const HChar* name, SymAVMAs* avmas) { Bool require_pToc = False; @@ -2178,6 +2399,8 @@ Bool VG_(lookup_symbol_SLOW)(const HChar* sopatt, const HChar* name, for (si = debugInfo_list; si; si = si->next) { if (debug) VG_(printf)("lookup_symbol_SLOW: considering %s\n", si->soname); + if (!is_DI_valid_for_epoch(si, ep)) + continue; if (!VG_(string_match)(sopatt, si->soname)) { if (debug) VG_(printf)(" ... skip\n"); @@ -2260,7 +2483,7 @@ putStrEsc( SizeT n, HChar** buf, SizeT *bufsiz, const HChar* str ) return n; } -const HChar* VG_(describe_IP)(Addr eip, const InlIPCursor *iipc) +const HChar* VG_(describe_IP)(DiEpoch ep, Addr eip, const InlIPCursor *iipc) { static HChar *buf = NULL; static SizeT bufsiz = 0; @@ -2273,7 +2496,10 @@ const HChar* VG_(describe_IP)(Addr eip, const InlIPCursor *iipc) HChar ibuf[50]; // large enough SizeT n = 0; - vg_assert (!iipc || iipc->eip == eip); + // An InlIPCursor is associated with one specific DebugInfo. So if + // it exists, make sure that it is valid for the specified DiEpoch. + vg_assert (!iipc + || (is_DI_valid_for_epoch(iipc->di, ep) && iipc->eip == eip)); const HChar *buf_fn; const HChar *buf_obj; @@ -2288,8 +2514,8 @@ const HChar* VG_(describe_IP)(Addr eip, const InlIPCursor *iipc) if (is_bottom(iipc)) { // At the bottom (towards main), we describe the fn at eip. know_fnname = VG_(clo_sym_offsets) - ? VG_(get_fnname_w_offset) (eip, &buf_fn) - : VG_(get_fnname) (eip, &buf_fn); + ? VG_(get_fnname_w_offset) (ep, eip, &buf_fn) + : VG_(get_fnname) (ep, eip, &buf_fn); } else { const DiInlLoc *next_inl = iipc && iipc->next_inltab >= 0 ? & iipc->di->inltab[iipc->next_inltab] @@ -2307,15 +2533,15 @@ const HChar* VG_(describe_IP)(Addr eip, const InlIPCursor *iipc) // ??? Currently never showing an offset. } - know_objname = VG_(get_objname)(eip, &buf_obj); + know_objname = VG_(get_objname)(ep, eip, &buf_obj); if (is_top(iipc)) { // The source for the highest level is in the loctab entry. know_srcloc = VG_(get_filename_linenum)( - eip, - &buf_srcloc, + ep, eip, + &buf_srcloc, &buf_dirname, - &lineno + &lineno ); know_dirinfo = buf_dirname[0] != '\0'; } else { @@ -2465,6 +2691,20 @@ const HChar* VG_(describe_IP)(Addr eip, const InlIPCursor *iipc) /*--- ---*/ /*--------------------------------------------------------------*/ +/* Note that the CFI machinery pertains to unwinding the stack "right now". + There is no support for unwinding stack images obtained from some time in + the past. That means that: + + (1) We only deal with CFI from DebugInfos that are valid for the current + debuginfo epoch. Unlike in the rest of the file, there is no + epoch-awareness. + + (2) We assume that the CFI cache will be invalidated every time the the + epoch changes. This is done by ensuring (in the file above) that + every call to advance_current_DiEpoch has a call to + caches__invalidate alongside it. +*/ + /* Gather up all the constant pieces of info needed to evaluate a CfiExpr into one convenient struct. */ typedef @@ -2585,6 +2825,9 @@ UWord evalCfiExpr ( const XArray* exprs, Int ix, *cfsi_mP to the cfsi_m pointer in that DebugInfo's cfsi_m_pool. If not found, set *diP to (DebugInfo*)1 and *cfsi_mP to zero. + + Per comments at the top of this section, we only look for CFI in + DebugInfos that are valid for the current epoch. */ __attribute__((noinline)) static void find_DiCfSI ( /*OUT*/DebugInfo** diP, @@ -2600,10 +2843,15 @@ static void find_DiCfSI ( /*OUT*/DebugInfo** diP, if (0) VG_(printf)("search for %#lx\n", ip); + DiEpoch curr_epoch = VG_(current_DiEpoch)(); + for (di = debugInfo_list; di != NULL; di = di->next) { Word j; n_steps++; + if (!is_DI_valid_for_epoch(di, curr_epoch)) + continue; + /* Use the per-DebugInfo summary address ranges to skip inapplicable DebugInfos quickly. */ if (di->cfsi_used == 0) @@ -2611,6 +2859,11 @@ static void find_DiCfSI ( /*OUT*/DebugInfo** diP, if (ip < di->cfsi_minavma || ip > di->cfsi_maxavma) continue; + // This di must be active (because we have explicitly chosen not to + // allow unwinding stacks that pertain to some past epoch). It can't + // be archived or not-yet-active. + vg_assert(is_DebugInfo_active(di)); + /* It might be in this DebugInfo. Search it. */ j = ML_(search_one_cfitab)( di, ip ); vg_assert(j >= -1 && j < (Word)di->cfsi_used); @@ -3044,6 +3297,7 @@ Bool VG_(use_CF_info) ( /*MOD*/D3UnwindRegs* uregsHere, Bool VG_(use_FPO_info) ( /*MOD*/Addr* ipP, /*MOD*/Addr* spP, /*MOD*/Addr* fpP, + DiEpoch ep, Addr min_accessible, Addr max_accessible ) { @@ -3061,6 +3315,9 @@ Bool VG_(use_FPO_info) ( /*MOD*/Addr* ipP, for (di = debugInfo_list; di != NULL; di = di->next) { n_steps++; + if (!is_DI_valid_for_epoch(di, ep)) + continue; + /* Use the per-DebugInfo summary address ranges to skip inapplicable DebugInfos quickly. */ if (di->fpo == NULL) @@ -3564,6 +3821,7 @@ static void format_message ( /*MOD*/XArray* /* of HChar */ dn1, static Bool consider_vars_in_frame ( /*MOD*/XArray* /* of HChar */ dname1, /*MOD*/XArray* /* of HChar */ dname2, + DiEpoch ep, Addr data_addr, Addr ip, Addr sp, Addr fp, /* shown to user: */ @@ -3582,6 +3840,8 @@ Bool consider_vars_in_frame ( /*MOD*/XArray* /* of HChar */ dname1, /* first, find the DebugInfo that pertains to 'ip'. */ for (di = debugInfo_list; di; di = di->next) { n_steps++; + if (!is_DI_valid_for_epoch(di, ep)) + continue; /* text segment missing? unlikely, but handle it .. */ if (!di->text_present || di->text_size == 0) continue; @@ -3699,7 +3959,7 @@ Bool consider_vars_in_frame ( /*MOD*/XArray* /* of HChar */ dname1, Bool VG_(get_data_description)( /*MOD*/ XArray* /* of HChar */ dname1, /*MOD*/ XArray* /* of HChar */ dname2, - Addr data_addr + DiEpoch ep, Addr data_addr ) { # define N_FRAMES 8 @@ -3819,7 +4079,7 @@ Bool VG_(get_data_description)( vg_assert(n_frames >= 0 && n_frames <= N_FRAMES); for (j = 0; j < n_frames; j++) { if (consider_vars_in_frame( dname1, dname2, - data_addr, + ep, data_addr, ips[j], sps[j], fps[j], tid, j )) { zterm_XA( dname1 ); @@ -3846,7 +4106,7 @@ Bool VG_(get_data_description)( equivalent kludge. */ if (j > 0 /* this is a non-innermost frame */ && consider_vars_in_frame( dname1, dname2, - data_addr, + ep, data_addr, ips[j] + 1, sps[j], fps[j], tid, j )) { zterm_XA( dname1 ); diff --git a/coregrind/m_debuginfo/priv_storage.h b/coregrind/m_debuginfo/priv_storage.h index 061ecf276f..6c471a00f7 100644 --- a/coregrind/m_debuginfo/priv_storage.h +++ b/coregrind/m_debuginfo/priv_storage.h @@ -588,6 +588,36 @@ struct _DebugInfo { structure is allocated. */ ULong handle; + /* The range of epochs for which this DebugInfo is valid. These also + divide the DebugInfo's lifetime into three parts: + + (1) Allocated: but with only .fsm holding useful info -- in + particular, not yet holding any debug info. + .first_epoch == DebugInfoEpoch_INVALID + .last_epoch == DebugInfoEpoch_INVALID + + (2) Active: containing debug info, and current. + .first_epoch != DebugInfoEpoch_INVALID + .last_epoch == DebugInfoEpoch_INVALID + + (3) Archived: containing debug info, but no longer current. + .first_epoch != DebugInfoEpoch_INVALID + .last_epoch != DebugInfoEpoch_INVALID + + State (2) corresponds to an object which is currently mapped. When + the object is unmapped, what happens depends on the setting of + --keep-debuginfo: + + * when =no, the DebugInfo is removed from debugInfo_list and + deleted. + + * when =yes, the DebugInfo is retained in debugInfo_list, but its + .last_epoch field is filled in, and current_epoch is advanced. This + effectively moves the DebugInfo into state (3). + */ + DiEpoch first_epoch; + DiEpoch last_epoch; + /* Used for debugging only - indicate what stuff to dump whilst reading stuff into the seginfo. Are computed as early in the lifetime of the DebugInfo as possible -- at the point when it is diff --git a/coregrind/m_errormgr.c b/coregrind/m_errormgr.c index c1f8006cfc..38e67699f1 100644 --- a/coregrind/m_errormgr.c +++ b/coregrind/m_errormgr.c @@ -321,15 +321,15 @@ static Bool eq_Error ( VgRes res, const Error* e1, const Error* e2 ) */ #define ERRTXT_LEN 4096 -static void printSuppForIp_XML(UInt n, Addr ip, void* uu_opaque) +static void printSuppForIp_XML(UInt n, DiEpoch ep, Addr ip, void* uu_opaque) { const HChar *buf; - InlIPCursor* iipc = VG_(new_IIPC)(ip); + InlIPCursor* iipc = VG_(new_IIPC)(ep, ip); do { - if ( VG_(get_fnname_no_cxx_demangle) (ip, &buf, iipc) ) { + if ( VG_(get_fnname_no_cxx_demangle) (ep, ip, &buf, iipc) ) { VG_(printf_xml)(" %pS \n", buf); } else - if ( VG_(get_objname)(ip, &buf) ) { + if ( VG_(get_objname)(ep, ip, &buf) ) { VG_(printf_xml)(" %pS \n", buf); } else { VG_(printf_xml)(" * \n"); @@ -338,16 +338,16 @@ static void printSuppForIp_XML(UInt n, Addr ip, void* uu_opaque) VG_(delete_IIPC)(iipc); } -static void printSuppForIp_nonXML(UInt n, Addr ip, void* textV) +static void printSuppForIp_nonXML(UInt n, DiEpoch ep, Addr ip, void* textV) { const HChar *buf; XArray* /* of HChar */ text = (XArray*)textV; - InlIPCursor* iipc = VG_(new_IIPC)(ip); + InlIPCursor* iipc = VG_(new_IIPC)(ep, ip); do { - if ( VG_(get_fnname_no_cxx_demangle) (ip, &buf, iipc) ) { + if ( VG_(get_fnname_no_cxx_demangle) (ep, ip, &buf, iipc) ) { VG_(xaprintf)(text, " fun:%s\n", buf); } else - if ( VG_(get_objname)(ip, &buf) ) { + if ( VG_(get_objname)(ep, ip, &buf) ) { VG_(xaprintf)(text, " obj:%s\n", buf); } else { VG_(xaprintf)(text, " obj:*\n"); @@ -412,7 +412,7 @@ static void gen_suppression(const Error* err) vg_assert(n_ips > 0); vg_assert(n_ips <= VG_DEEPEST_BACKTRACE); VG_(apply_StackTrace)(printSuppForIp_nonXML, - text, + text, VG_(get_ExeContext_epoch)(ec), VG_(get_ExeContext_StackTrace)(ec), n_ips); @@ -441,7 +441,7 @@ static void gen_suppression(const Error* err) // Print stack trace elements VG_(apply_StackTrace)(printSuppForIp_XML, - NULL, + NULL, VG_(get_ExeContext_epoch)(ec), VG_(get_ExeContext_StackTrace)(ec), VG_(get_ExeContext_n_ips)(ec)); @@ -660,7 +660,7 @@ void construct_error ( Error* err, ThreadId tid, ErrorKind ekind, Addr a, err->count = 1; err->tid = tid; if (NULL == where) - err->where = VG_(record_ExeContext)( tid, 0 ); + err->where = VG_(record_ExeContext)( tid, 0 ); else err->where = where; @@ -1509,6 +1509,7 @@ static Bool supploc_IsQuery ( const void* supplocV ) allocations and the nr of debuginfo search. */ typedef struct { + DiEpoch epoch; // used to interpret .ips StackTrace ips; // stack trace we are lazily completing. UWord n_ips; // nr of elements in ips. @@ -1604,9 +1605,10 @@ static void clearIPtoFunOrObjCompleter ( const Supp *su, su->sname, filename, su->sname_lineno); - } else + } else { VG_(dmsg)("errormgr matching end no suppression matched:\n"); - VG_(pp_StackTrace) (ip2fo->ips, ip2fo->n_ips); + } + VG_(pp_StackTrace) (ip2fo->epoch, ip2fo->ips, ip2fo->n_ips); pp_ip2fo(ip2fo); } if (ip2fo->n_offsets_per_ip) VG_(free)(ip2fo->n_offsets_per_ip); @@ -1669,7 +1671,8 @@ static HChar* foComplete(IPtoFunOrObjCompleter* ip2fo, // up comparing "malloc" in the suppression against // "_vgrZU_libcZdsoZa_malloc" in the backtrace, and the // two of them need to be made to match. - if (!VG_(get_fnname_no_cxx_demangle)(ip2fo->ips[ixInput], + if (!VG_(get_fnname_no_cxx_demangle)(ip2fo->epoch, + ip2fo->ips[ixInput], &caller, NULL)) caller = "???"; @@ -1690,7 +1693,7 @@ static HChar* foComplete(IPtoFunOrObjCompleter* ip2fo, last_expand_pos_ips is the last offset in fun/obj where ips[pos_ips] has been expanded. */ - if (!VG_(get_objname)(ip2fo->ips[pos_ips], &caller)) + if (!VG_(get_objname)(ip2fo->epoch, ip2fo->ips[pos_ips], &caller)) caller = "???"; // Have all inlined calls pointing at this object name @@ -1760,7 +1763,7 @@ static void expandInput (IPtoFunOrObjCompleter* ip2fo, UWord ixInput ) const Addr IP = ip2fo->ips[ip2fo->n_ips_expanded]; InlIPCursor *iipc; - iipc = VG_(new_IIPC)(IP); + iipc = VG_(new_IIPC)(ip2fo->epoch, IP); // The only thing we really need is the nr of inlined fn calls // corresponding to the IP we will expand. // However, computing this is mostly the same as finding @@ -1769,7 +1772,7 @@ static void expandInput (IPtoFunOrObjCompleter* ip2fo, UWord ixInput ) const HChar *caller; grow_offsets(ip2fo, ip2fo->n_expanded+1); ip2fo->fun_offsets[ip2fo->n_expanded] = ip2fo->names_free; - if (!VG_(get_fnname_no_cxx_demangle)(IP, + if (!VG_(get_fnname_no_cxx_demangle)(ip2fo->epoch, IP, &caller, iipc)) caller = "???"; @@ -1797,18 +1800,18 @@ static void expandInput (IPtoFunOrObjCompleter* ip2fo, UWord ixInput ) } } -static Bool haveInputInpC (void* inputCompleter, UWord ixInput ) +static Bool haveInputInpC (void* inputCompleterV, UWord ixInput ) { - IPtoFunOrObjCompleter* ip2fo = inputCompleter; + IPtoFunOrObjCompleter* ip2fo = (IPtoFunOrObjCompleter*)inputCompleterV; expandInput(ip2fo, ixInput); return ixInput < ip2fo->n_expanded; } static Bool supp_pattEQinp ( const void* supplocV, const void* addrV, - void* inputCompleter, UWord ixInput ) + void* inputCompleterV, UWord ixInput ) { - const SuppLoc* supploc = supplocV; /* PATTERN */ - IPtoFunOrObjCompleter* ip2fo = inputCompleter; + const SuppLoc* supploc = (const SuppLoc*)supplocV; /* PATTERN */ + IPtoFunOrObjCompleter* ip2fo = (IPtoFunOrObjCompleter*)inputCompleterV; HChar* funobj_name; // Fun or Obj name. Bool ret; @@ -1935,6 +1938,7 @@ static Supp* is_suppressible_error ( const Error* err ) em_supplist_searches++; /* Prepare the lazy input completer. */ + ip2fo.epoch = VG_(get_ExeContext_epoch)(err->where); ip2fo.ips = VG_(get_ExeContext_StackTrace)(err->where); ip2fo.n_ips = VG_(get_ExeContext_n_ips)(err->where); ip2fo.n_ips_expanded = 0; diff --git a/coregrind/m_execontext.c b/coregrind/m_execontext.c index e655b09b79..0bcbaadb38 100644 --- a/coregrind/m_execontext.c +++ b/coregrind/m_execontext.c @@ -81,6 +81,21 @@ struct _ExeContext { be a multiple of four, and must be unique. Hence they start at 4. */ UInt ecu; + /* epoch in which the ExeContext can be symbolised. In other words, epoch + identifies the set of debug info to use to symbolise the Addr in ips + using e.g. VG_(get_filename), VG_(get_fnname), ... + Note 1: 2 ExeContexts are equal when their ips array are equal and + their epoch are equal. + Note 2: a freshly created ExeContext has a DiEpoch_INVALID epoch. + DiEpoch_INVALID is used as a special value to indicate that ExeContext + is valid in the current epoch. VG_(get_ExeContext_epoch) translates + this invalid value in the real current epoch. + When a debug info is archived, the set of ExeContext is scanned : + If an ExeContext with epoch == DiEpoch_INVALID has one or more + ips Addr corresponding to the just archived debug info, the ExeContext + epoch is changed to the last epoch identifying the set containing the + archived debug info. */ + DiEpoch epoch; /* Variable-length array. The size is 'n_ips'; at least 1, at most VG_DEEPEST_BACKTRACE. [0] is the current IP, [1] is its caller, [2] is the caller of [1], etc. */ @@ -116,7 +131,7 @@ static ULong ec_cmpAlls; /*------------------------------------------------------------*/ -/*--- Exported functions. ---*/ +/*--- ExeContext functions. ---*/ /*------------------------------------------------------------*/ static ExeContext* record_ExeContext_wrk2 ( const Addr* ips, UInt n_ips ); @@ -153,6 +168,13 @@ static void init_ExeContext_storage ( void ) init_done = True; } +DiEpoch VG_(get_ExeContext_epoch)( const ExeContext* e ) +{ + if (is_DiEpoch_INVALID (e->epoch)) + return VG_(current_DiEpoch)(); + else + return e->epoch; +} /* Print stats. */ void VG_(print_ExeContext_stats) ( Bool with_stacktraces ) @@ -167,9 +189,11 @@ void VG_(print_ExeContext_stats) ( Bool with_stacktraces ) VG_(message)(Vg_DebugMsg, " exectx: Printing contexts stacktraces\n"); for (i = 0; i < ec_htab_size; i++) { for (ec = ec_htab[i]; ec; ec = ec->chain) { - VG_(message)(Vg_DebugMsg, " exectx: stacktrace ecu %u n_ips %u\n", - ec->ecu, ec->n_ips); - VG_(pp_StackTrace)( ec->ips, ec->n_ips ); + VG_(message)(Vg_DebugMsg, + " exectx: stacktrace ecu %u epoch %u n_ips %u\n", + ec->ecu, ec->epoch.n, ec->n_ips); + VG_(pp_StackTrace)( VG_(get_ExeContext_epoch)(ec), + ec->ips, ec->n_ips ); } } VG_(message)(Vg_DebugMsg, @@ -205,10 +229,39 @@ void VG_(print_ExeContext_stats) ( Bool with_stacktraces ) /* Print an ExeContext. */ void VG_(pp_ExeContext) ( ExeContext* ec ) { - VG_(pp_StackTrace)( ec->ips, ec->n_ips ); + VG_(pp_StackTrace)( VG_(get_ExeContext_epoch)(ec), ec->ips, ec->n_ips ); } +void VG_(archive_ExeContext_in_range) (DiEpoch last_epoch, + Addr text_avma, SizeT length ) +{ + Int i; + ExeContext* ec; + ULong n_archived = 0; + const Addr text_avma_end = text_avma + length - 1; + + if (VG_(clo_verbosity) > 1) + VG_(message)(Vg_DebugMsg, "Scanning and archiving ExeContexts ...\n"); + for (i = 0; i < ec_htab_size; i++) { + for (ec = ec_htab[i]; ec; ec = ec->chain) { + if (is_DiEpoch_INVALID (ec->epoch)) + for (UInt j = 0; j < ec->n_ips; j++) { + if (UNLIKELY(ec->ips[j] >= text_avma + && ec->ips[j] <= text_avma_end)) { + ec->epoch = last_epoch; + n_archived++; + break; + } + } + } + } + if (VG_(clo_verbosity) > 1) + VG_(message)(Vg_DebugMsg, + "Scanned %'llu ExeContexts archived %'llu ExeContexts\n", + ec_totstored, n_archived); +} + /* Compare two ExeContexts. Number of callers considered depends on res. */ Bool VG_(eq_ExeContext) ( VgRes res, const ExeContext* e1, const ExeContext* e2 ) @@ -221,6 +274,9 @@ Bool VG_(eq_ExeContext) ( VgRes res, const ExeContext* e1, // Must be at least one address in each trace. vg_assert(e1->n_ips >= 1 && e2->n_ips >= 1); + // Note: we compare the epoch in the case below, and not here + // to have the ec_cmp* stats correct. + switch (res) { case Vg_LowRes: /* Just compare the top two callers. */ @@ -231,7 +287,7 @@ Bool VG_(eq_ExeContext) ( VgRes res, const ExeContext* e1, if (!(e1->n_ips <= i) && (e2->n_ips <= i)) return False; if (e1->ips[i] != e2->ips[i]) return False; } - return True; + return e1->epoch.n == e2->epoch.n; case Vg_MedRes: /* Just compare the top four callers. */ @@ -242,7 +298,7 @@ Bool VG_(eq_ExeContext) ( VgRes res, const ExeContext* e1, if (!(e1->n_ips <= i) && (e2->n_ips <= i)) return False; if (e1->ips[i] != e2->ips[i]) return False; } - return True; + return e1->epoch.n == e2->epoch.n; case Vg_HighRes: ec_cmpAlls++; @@ -425,7 +481,7 @@ static ExeContext* record_ExeContext_wrk2 ( const Addr* ips, UInt n_ips ) while (True) { if (list == NULL) break; ec_searchcmps++; - same = list->n_ips == n_ips; + same = list->n_ips == n_ips && is_DiEpoch_INVALID (list->epoch); for (i = 0; i < n_ips && same ; i++) { same = list->ips[i] == ips[i]; } diff --git a/coregrind/m_gdbserver/m_gdbserver.c b/coregrind/m_gdbserver/m_gdbserver.c index 24716ade19..9913c88230 100644 --- a/coregrind/m_gdbserver/m_gdbserver.c +++ b/coregrind/m_gdbserver/m_gdbserver.c @@ -142,14 +142,17 @@ static HChar* sym (Addr addr, Bool is_code) PtrdiffT offset; if (w == 2) w = 0; + // sym is used for debugging/tracing, so cur_ep is a reasonable choice. + const DiEpoch cur_ep = VG_(current_DiEpoch)(); + if (is_code) { const HChar *name; - name = VG_(describe_IP) (addr, NULL); + name = VG_(describe_IP) (cur_ep, addr, NULL); if (buf[w]) VG_(free)(buf[w]); buf[w] = VG_(strdup)("gdbserver sym", name); } else { const HChar *name; - VG_(get_datasym_and_offset) (addr, &name, &offset); + VG_(get_datasym_and_offset) (cur_ep, addr, &name, &offset); if (buf[w]) VG_(free)(buf[w]); buf[w] = VG_(strdup)("gdbserver sym", name); } diff --git a/coregrind/m_gdbserver/server.c b/coregrind/m_gdbserver/server.c index ca0d7bd548..d11584c1b5 100644 --- a/coregrind/m_gdbserver/server.c +++ b/coregrind/m_gdbserver/server.c @@ -195,6 +195,8 @@ int handle_gdb_valgrind_command (char *mon, OutputSink *sink_wanted_at_return) int kwdid; int int_value; + const DiEpoch cur_ep = VG_(current_DiEpoch)(); + vg_assert (initial_valgrind_sink_saved); strcpy (s, mon); @@ -334,7 +336,7 @@ int handle_gdb_valgrind_command (char *mon, OutputSink *sink_wanted_at_return) } if (hostvisibility) { const DebugInfo *tooldi - = VG_(find_DebugInfo) ((Addr)handle_gdb_valgrind_command); + = VG_(find_DebugInfo) (cur_ep, (Addr)handle_gdb_valgrind_command); /* Normally, we should always find the tooldi. In case we do not, suggest a 'likely somewhat working' address: */ const Addr tool_text_start @@ -442,14 +444,14 @@ int handle_gdb_valgrind_command (char *mon, OutputSink *sink_wanted_at_return) &dummy_sz, &ssaveptr)) { // If tool provides location information, use that. if (VG_(needs).info_location) { - VG_TDICT_CALL(tool_info_location, address); + VG_TDICT_CALL(tool_info_location, cur_ep, address); } // If tool does not provide location info, use the common one. // Also use the common to compare with tool when debug log is set. if (!VG_(needs).info_location || VG_(debugLog_getLevel)() > 0 ) { AddrInfo ai; ai.tag = Addr_Undescribed; - VG_(describe_addr) (address, &ai); + VG_(describe_addr) (cur_ep, address, &ai); VG_(pp_addrinfo) (address, &ai); VG_(clear_addrinfo) (&ai); } diff --git a/coregrind/m_gdbserver/target.c b/coregrind/m_gdbserver/target.c index 1f03c1290c..0093ce828d 100644 --- a/coregrind/m_gdbserver/target.c +++ b/coregrind/m_gdbserver/target.c @@ -209,7 +209,10 @@ void gdbserver_process_exit_encountered (unsigned char status, Int code) static const HChar* sym (Addr addr) { - return VG_(describe_IP) (addr, NULL); + // Tracing/debugging so cur_ep is reasonable. + const DiEpoch cur_ep = VG_(current_DiEpoch)(); + + return VG_(describe_IP) (cur_ep, addr, NULL); } ThreadId vgdb_interrupted_tid = 0; diff --git a/coregrind/m_gdbserver/valgrind-low-arm.c b/coregrind/m_gdbserver/valgrind-low-arm.c index c6f33aac9b..fecbb071c9 100644 --- a/coregrind/m_gdbserver/valgrind-low-arm.c +++ b/coregrind/m_gdbserver/valgrind-low-arm.c @@ -149,8 +149,13 @@ Addr thumb_pc (Addr pc) // the debug info with the bit0 set // (why can't debug info do that for us ???) // (why if this is a 4 bytes thumb instruction ???) - if (VG_(get_fnname_raw) (pc | 1, &fnname)) { - if (VG_(lookup_symbol_SLOW)( "*", fnname, &avmas )) { + + // Used to check if the instruction is a thumb instruction, + // typically for a live address, so cur_ep is a reasonable choice. + const DiEpoch cur_ep = VG_(current_DiEpoch)(); + + if (VG_(get_fnname_raw) (cur_ep, pc | 1, &fnname)) { + if (VG_(lookup_symbol_SLOW)( cur_ep, "*", fnname, &avmas )) { dlog (1, "fnname %s lookupsym %p => %p %s.\n", fnname, C2v(avmas.main), C2v(pc), (avmas.main & 1 ? "thumb" : "arm")); diff --git a/coregrind/m_gdbserver/valgrind-low-mips32.c b/coregrind/m_gdbserver/valgrind-low-mips32.c index 300371db86..5f965f54a3 100644 --- a/coregrind/m_gdbserver/valgrind-low-mips32.c +++ b/coregrind/m_gdbserver/valgrind-low-mips32.c @@ -228,7 +228,11 @@ static Addr mips_adjust_breakpoint_address (Addr pc) /* Make sure we don't scan back before the beginning of the current function, since we may fetch constant data or insns that look like a jump. */ - if (VG_(get_inst_offset_in_function) (bpaddr, &offset)) { + + // Placing a breakpoint, so pc should be in di of current epoch. + const DiEpoch cur_ep = VG_(current_DiEpoch)(); + + if (VG_(get_inst_offset_in_function) (cur_ep, bpaddr, &offset)) { func_addr = bpaddr - offset; if (func_addr > boundary && func_addr <= bpaddr) boundary = func_addr; diff --git a/coregrind/m_gdbserver/valgrind-low-mips64.c b/coregrind/m_gdbserver/valgrind-low-mips64.c index 4aac5743f8..20323a3b69 100644 --- a/coregrind/m_gdbserver/valgrind-low-mips64.c +++ b/coregrind/m_gdbserver/valgrind-low-mips64.c @@ -229,7 +229,11 @@ static Addr mips_adjust_breakpoint_address (Addr pc) /* Make sure we don't scan back before the beginning of the current function, since we may fetch constant data or insns that look like a jump. */ - if (VG_(get_inst_offset_in_function) (bpaddr, &offset)) { + + // Placing a breakpoint, so pc should be in di of current epoch. + const DiEpoch cur_ep = VG_(current_DiEpoch)(); + + if (VG_(get_inst_offset_in_function) (cur_ep, bpaddr, &offset)) { func_addr = bpaddr - offset; if (func_addr > boundary && func_addr <= bpaddr) boundary = func_addr; diff --git a/coregrind/m_libcassert.c b/coregrind/m_libcassert.c index 63fb64b240..42a54a5825 100644 --- a/coregrind/m_libcassert.c +++ b/coregrind/m_libcassert.c @@ -369,7 +369,7 @@ static void show_sched_status_wrk ( Bool host_stacktrace, ); VG_(printf)("\nhost stacktrace:\n"); VG_(clo_xml) = False; - VG_(pp_StackTrace) (ips, n_ips); + VG_(pp_StackTrace) (VG_(current_DiEpoch)(), ips, n_ips); VG_(clo_xml) = save_clo_xml; } diff --git a/coregrind/m_main.c b/coregrind/m_main.c index e3d5f65c97..a7574c3e5b 100644 --- a/coregrind/m_main.c +++ b/coregrind/m_main.c @@ -129,6 +129,10 @@ static void usage_NORETURN ( Bool debug_help ) " --error-exitcode= exit code to return if errors found [0=disable]\n" " --error-markers=, add lines with begin/end markers before/after\n" " each error output in plain text mode [none]\n" +" --keep-debuginfo=no|yes Keep symbols etc for unloaded code [no]\n" +" This allows saved stack traces (e.g. memory leaks)\n" +" to include file/line info for code that has been\n" +" dlclose'd (or similar)\n" " --show-below-main=no|yes continue stack traces below main() [no]\n" " --default-suppressions=yes|no\n" " load default suppressions [yes]\n" @@ -634,6 +638,7 @@ void main_process_cmd_line_options( void ) else if VG_BOOL_CLO(arg, "--run-libc-freeres", VG_(clo_run_libc_freeres)) {} else if VG_BOOL_CLO(arg, "--run-cxx-freeres", VG_(clo_run_cxx_freeres)) {} else if VG_BOOL_CLO(arg, "--show-below-main", VG_(clo_show_below_main)) {} + else if VG_BOOL_CLO(arg, "--keep-debuginfo", VG_(clo_keep_debuginfo)) {} else if VG_BOOL_CLO(arg, "--time-stamp", VG_(clo_time_stamp)) {} else if VG_BOOL_CLO(arg, "--track-fds", VG_(clo_track_fds)) {} else if VG_BOOL_CLO(arg, "--trace-children", VG_(clo_trace_children)) {} @@ -2266,7 +2271,8 @@ static void final_tidyup(ThreadId tid) } # if defined(VGP_ppc64be_linux) - Addr r2 = VG_(get_tocptr)(freeres_wrapper); + Addr r2 = VG_(get_tocptr)(VG_(current_DiEpoch)(), + freeres_wrapper); if (r2 == 0) { VG_(message)(Vg_UserMsg, "Caught __NR_exit, but can't run __gnu_cxx::__freeres()\n"); diff --git a/coregrind/m_options.c b/coregrind/m_options.c index bae161baed..72173ddfa2 100644 --- a/coregrind/m_options.c +++ b/coregrind/m_options.c @@ -130,6 +130,7 @@ Bool VG_(clo_run_libc_freeres) = True; Bool VG_(clo_run_cxx_freeres) = True; Bool VG_(clo_track_fds) = False; Bool VG_(clo_show_below_main)= False; +Bool VG_(clo_keep_debuginfo) = False; Bool VG_(clo_show_emwarns) = False; Word VG_(clo_max_stackframe) = 2000000; UInt VG_(clo_max_threads) = MAX_THREADS_DEFAULT; diff --git a/coregrind/m_redir.c b/coregrind/m_redir.c index d54cae7966..992427a762 100644 --- a/coregrind/m_redir.c +++ b/coregrind/m_redir.c @@ -1870,15 +1870,16 @@ static void show_active ( const HChar* left, const Active* act ) { Bool ok; const HChar *buf; - - ok = VG_(get_fnname_w_offset)(act->from_addr, &buf); + + DiEpoch ep = VG_(current_DiEpoch)(); + ok = VG_(get_fnname_w_offset)(ep, act->from_addr, &buf); if (!ok) buf = "???"; // Stash away name1 HChar name1[VG_(strlen)(buf) + 1]; VG_(strcpy)(name1, buf); const HChar *name2; - ok = VG_(get_fnname_w_offset)(act->to_addr, &name2); + ok = VG_(get_fnname_w_offset)(ep, act->to_addr, &name2); if (!ok) name2 = "???"; VG_(message)(Vg_DebugMsg, "%s0x%08lx (%-20s) %s-> (%04d.%d) 0x%08lx %s\n", diff --git a/coregrind/m_sbprofile.c b/coregrind/m_sbprofile.c index 45c26d8f2b..2878a70562 100644 --- a/coregrind/m_sbprofile.c +++ b/coregrind/m_sbprofile.c @@ -74,6 +74,9 @@ void show_SB_profile ( const SBProfEntry tops[], UInt n_tops, VG_(printf)("Total score = %'llu\n\n", score_total); + // FIXME JRS EPOCH 28 July 2017: this is probably not right in general + DiEpoch cur_ep = VG_(current_DiEpoch)(); + /* Print an initial per-block summary. */ VG_(printf)("rank ---cumulative--- -----self-----\n"); score_cumul = 0; @@ -84,7 +87,7 @@ void show_SB_profile ( const SBProfEntry tops[], UInt n_tops, continue; const HChar *name; - VG_(get_fnname_w_offset)(tops[r].addr, &name); + VG_(get_fnname_w_offset)(cur_ep, tops[r].addr, &name); score_here = tops[r].score; score_cumul += score_here; @@ -123,7 +126,7 @@ void show_SB_profile ( const SBProfEntry tops[], UInt n_tops, continue; const HChar *name; - VG_(get_fnname_w_offset)(tops[r].addr, &name); + VG_(get_fnname_w_offset)(cur_ep, tops[r].addr, &name); score_here = tops[r].score; score_cumul += score_here; @@ -159,7 +162,7 @@ void show_SB_profile ( const SBProfEntry tops[], UInt n_tops, continue; const HChar *name; - VG_(get_fnname_w_offset)(tops[r].addr, &name); + VG_(get_fnname_w_offset)(cur_ep, tops[r].addr, &name); score_here = tops[r].score; diff --git a/coregrind/m_scheduler/scheduler.c b/coregrind/m_scheduler/scheduler.c index 39f10f8882..54b8008d61 100644 --- a/coregrind/m_scheduler/scheduler.c +++ b/coregrind/m_scheduler/scheduler.c @@ -2037,8 +2037,14 @@ void do_client_request ( ThreadId tid ) VG_(memset)(buf64, 0, 64); UInt linenum = 0; + + // Unless the guest would become epoch aware (and would need to + // describe IP addresses of dlclosed libs), using cur_ep is a + // reasonable choice. + const DiEpoch cur_ep = VG_(current_DiEpoch)(); + Bool ok = VG_(get_filename_linenum)( - ip, &buf, NULL, &linenum + cur_ep, ip, &buf, NULL, &linenum ); if (ok) { /* For backward compatibility truncate the filename to diff --git a/coregrind/m_stacktrace.c b/coregrind/m_stacktrace.c index 787368b90a..a0f9a9bda4 100644 --- a/coregrind/m_stacktrace.c +++ b/coregrind/m_stacktrace.c @@ -446,6 +446,7 @@ UInt VG_(get_StackTrace_wrk) ( ThreadId tid_if_known, /* And, similarly, try for MSVC FPO unwind info. */ if (FPO_info_present && VG_(use_FPO_info)( &uregs.xip, &uregs.xsp, &uregs.xbp, + VG_(current_DiEpoch)(), fp_min, fp_max ) ) { if (debug) unwind_case = "MS"; if (do_stats) stats.MS++; @@ -730,6 +731,7 @@ UInt VG_(get_StackTrace_wrk) ( ThreadId tid_if_known, Word redirs_used = 0; # endif const Int cmrf = VG_(clo_merge_recursive_frames); + const DiEpoch cur_ep = VG_(current_DiEpoch)(); Bool debug = False; Int i; @@ -813,10 +815,10 @@ UInt VG_(get_StackTrace_wrk) ( ThreadId tid_if_known, const HChar *buf_lr, *buf_ip; /* The following conditional looks grossly inefficient and surely could be majorly improved, with not much effort. */ - if (VG_(get_fnname_raw) (lr, &buf_lr)) { + if (VG_(get_fnname_raw) (cur_ep, lr, &buf_lr)) { HChar buf_lr_copy[VG_(strlen)(buf_lr) + 1]; VG_(strcpy)(buf_lr_copy, buf_lr); - if (VG_(get_fnname_raw) (ip, &buf_ip)) + if (VG_(get_fnname_raw) (cur_ep, ip, &buf_ip)) if (VG_(strcmp)(buf_lr_copy, buf_ip)) lr_is_first_RA = True; } @@ -1558,12 +1560,12 @@ UInt VG_(get_StackTrace) ( ThreadId tid, ); } -static void printIpDesc(UInt n, Addr ip, void* uu_opaque) +static void printIpDesc(UInt n, DiEpoch ep, Addr ip, void* uu_opaque) { - InlIPCursor *iipc = VG_(new_IIPC)(ip); + InlIPCursor *iipc = VG_(new_IIPC)(ep, ip); do { - const HChar *buf = VG_(describe_IP)(ip, iipc); + const HChar *buf = VG_(describe_IP)(ep, ip, iipc); if (VG_(clo_xml)) { VG_(printf_xml)(" %s\n", buf); } else { @@ -1577,14 +1579,14 @@ static void printIpDesc(UInt n, Addr ip, void* uu_opaque) } /* Print a StackTrace. */ -void VG_(pp_StackTrace) ( StackTrace ips, UInt n_ips ) +void VG_(pp_StackTrace) ( DiEpoch ep, StackTrace ips, UInt n_ips ) { vg_assert( n_ips > 0 ); if (VG_(clo_xml)) VG_(printf_xml)(" \n"); - VG_(apply_StackTrace)( printIpDesc, NULL, ips, n_ips ); + VG_(apply_StackTrace)( printIpDesc, NULL, ep, ips, n_ips ); if (VG_(clo_xml)) VG_(printf_xml)(" \n"); @@ -1599,13 +1601,13 @@ void VG_(get_and_pp_StackTrace) ( ThreadId tid, UInt max_n_ips ) NULL/*array to dump SP values in*/, NULL/*array to dump FP values in*/, 0/*first_ip_delta*/); - VG_(pp_StackTrace)(ips, n_ips); + VG_(pp_StackTrace)(VG_(current_DiEpoch)(), ips, n_ips); } void VG_(apply_StackTrace)( - void(*action)(UInt n, Addr ip, void* opaque), + void(*action)(UInt n, DiEpoch ep, Addr ip, void* opaque), void* opaque, - StackTrace ips, UInt n_ips + DiEpoch ep, StackTrace ips, UInt n_ips ) { Int i; @@ -1616,7 +1618,7 @@ void VG_(apply_StackTrace)( // or the last appearance of a below main function. // Then decrease n_ips so as to not call action for the below main for (i = n_ips - 1; i >= 0; i--) { - Vg_FnNameKind kind = VG_(get_fnname_kind_from_IP)(ips[i]); + Vg_FnNameKind kind = VG_(get_fnname_kind_from_IP)(ep, ips[i]); if (Vg_FnNameMain == kind || Vg_FnNameBelowMain == kind) n_ips = i + 1; if (Vg_FnNameMain == kind) @@ -1626,7 +1628,7 @@ void VG_(apply_StackTrace)( for (i = 0; i < n_ips; i++) // Act on the ip - action(i, ips[i], opaque); + action(i, ep, ips[i], opaque); } diff --git a/coregrind/m_tooliface.c b/coregrind/m_tooliface.c index d2e7ad98db..dcb233f02f 100644 --- a/coregrind/m_tooliface.c +++ b/coregrind/m_tooliface.c @@ -328,7 +328,7 @@ void VG_(needs_print_stats) ( } void VG_(needs_info_location) ( - void (*info_location)(Addr) + void (*info_location)(DiEpoch, Addr) ) { VG_(needs).info_location = True; diff --git a/coregrind/m_translate.c b/coregrind/m_translate.c index d03df5d33a..480a25990a 100644 --- a/coregrind/m_translate.c +++ b/coregrind/m_translate.c @@ -1363,7 +1363,8 @@ Bool mk_preamble__set_NRADDR_to_zero ( void* closureV, IRSB* bb ) VG_WORDSIZE==8 ? mkU64(0) : mkU32(0) ) ); - gen_push_and_set_LR_R2 ( bb, VG_(get_tocptr)( closure->readdr ) ); + gen_push_and_set_LR_R2 ( bb, VG_(get_tocptr)( VG_(current_DiEpoch)(), + closure->readdr ) ); } # endif @@ -1421,7 +1422,8 @@ Bool mk_preamble__set_NRADDR_to_nraddr ( void* closureV, IRSB* bb ) VG_WORDSIZE==8 ? Ity_I64 : Ity_I32) ) ); - gen_push_and_set_LR_R2 ( bb, VG_(get_tocptr)( closure->readdr ) ); + gen_push_and_set_LR_R2 ( bb, VG_(get_tocptr)( VG_(current_DiEpoch)(), + closure->readdr ) ); # endif #if defined(VGP_ppc64le_linux) /* This saves the r2 before leaving the function. We need to move @@ -1538,24 +1540,25 @@ Bool VG_(translate) ( ThreadId tid, Bool ok; const HChar *buf; const HChar *name2; + const DiEpoch ep = VG_(current_DiEpoch)(); /* Try also to get the soname (not the filename) of the "from" object. This makes it much easier to debug redirection problems. */ const HChar* nraddr_soname = "???"; - DebugInfo* nraddr_di = VG_(find_DebugInfo)(nraddr); + DebugInfo* nraddr_di = VG_(find_DebugInfo)(ep, nraddr); if (nraddr_di) { const HChar* t = VG_(DebugInfo_get_soname)(nraddr_di); if (t) nraddr_soname = t; } - ok = VG_(get_fnname_w_offset)(nraddr, &buf); + ok = VG_(get_fnname_w_offset)(ep, nraddr, &buf); if (!ok) buf = "???"; // Stash away name1 HChar name1[VG_(strlen)(buf) + 1]; VG_(strcpy)(name1, buf); - ok = VG_(get_fnname_w_offset)(addr, &name2); + ok = VG_(get_fnname_w_offset)(ep, addr, &name2); if (!ok) name2 = "???"; VG_(message)(Vg_DebugMsg, @@ -1572,7 +1575,8 @@ Bool VG_(translate) ( ThreadId tid, if (VG_(clo_trace_flags) || debugging_translation) { const HChar* objname = "UNKNOWN_OBJECT"; OffT objoff = 0; - DebugInfo* di = VG_(find_DebugInfo)( addr ); + const DiEpoch ep = VG_(current_DiEpoch)(); + DebugInfo* di = VG_(find_DebugInfo)( ep, addr ); if (di) { objname = VG_(DebugInfo_get_filename)(di); objoff = addr - VG_(DebugInfo_get_text_bias)(di); @@ -1580,7 +1584,7 @@ Bool VG_(translate) ( ThreadId tid, vg_assert(objname); const HChar *fnname; - Bool ok = VG_(get_fnname_w_offset)(addr, &fnname); + Bool ok = VG_(get_fnname_w_offset)(ep, addr, &fnname); if (!ok) fnname = "UNKNOWN_FUNCTION"; VG_(printf)( "==== SB %u (evchecks %llu) [tid %u] 0x%lx %s %s%c0x%lx\n", diff --git a/coregrind/m_xtree.c b/coregrind/m_xtree.c index 98d36bb643..7d57662226 100644 --- a/coregrind/m_xtree.c +++ b/coregrind/m_xtree.c @@ -501,7 +501,7 @@ void VG_(XT_callgrind_print) // the strings called_filename/called_fnname. #define CALLED_FLF(n) \ if ((n) < 0 \ - || !VG_(get_filename_linenum)(ips[(n)], \ + || !VG_(get_filename_linenum)(ep, ips[(n)], \ &filename_name, \ &filename_dir, \ &called_linenum)) { \ @@ -509,7 +509,7 @@ void VG_(XT_callgrind_print) called_linenum = 0; \ } \ if ((n) < 0 \ - || !VG_(get_fnname)(ips[(n)], &called_fnname)) { \ + || !VG_(get_fnname)(ep, ips[(n)], &called_fnname)) { \ called_fnname = "UnknownFn???"; \ } \ { \ @@ -550,6 +550,8 @@ void VG_(XT_callgrind_print) UInt prev_linenum; const Addr* ips = VG_(get_ExeContext_StackTrace)(xe->ec) + xe->top; + const DiEpoch ep = VG_(get_ExeContext_epoch)(xe->ec); + Int ips_idx = xe->n_ips_sel - 1; if (0) { @@ -762,11 +764,24 @@ static void ms_output_group (VgFile* fp, UInt depth, Ms_Group* group, ms_make_groups(depth+1, group->ms_ec, group->n_ec, sig_sz, &n_groups, &groups); + // FIXME JRS EPOCH 28 July 2017: HACK! Is this correct? + const DiEpoch cur_ep = VG_(current_DiEpoch)(); + // // FIXME PW EPOCH : No, the above is not correct. + // Xtree Massif output regroups execontext in the layout of a 'tree'. + // So, possibly, the same IP address value can be in 2 different ec, but + // the epoch to symbolise this address must be retrieved from the ec it + // originates from. + // So, to fix this, it is not enough to make a group based on identical + // IP addr value, one must also find the di used to symbolise this address, + // A group will then be defined as 'same IP and same di'. + // Fix not trivial to do, so for the moment, --keep-debuginfo=yes will + // have no impact on xtree massif output. + FP("%*s" "n%u: %ld %s\n", depth + 1, "", n_groups, group->total, - VG_(describe_IP)(group->ms_ec->ips[depth] - 1, NULL)); + VG_(describe_IP)(cur_ep, group->ms_ec->ips[depth] - 1, NULL)); /* XTREE??? Massif original code removes 1 to get the IP description. I am wondering if this is not something that predates revision r8818, which introduced a -1 in the stack unwind (see m_stacktrace.c) @@ -955,7 +970,7 @@ void VG_(XT_massif_print) } } -Int VG_(XT_offset_main_or_below_main)(Addr* ips, Int n_ips) +Int VG_(XT_offset_main_or_below_main)(DiEpoch ep, Addr* ips, Int n_ips) { /* Search for main or below main function. To limit the nr of ips to examine, we maintain the deepest @@ -972,7 +987,7 @@ Int VG_(XT_offset_main_or_below_main)(Addr* ips, Int n_ips) for (i = n_ips - 1 - deepest_main; i < n_ips; i++) { - mbmkind = VG_(get_fnname_kind_from_IP)(ips[i]); + mbmkind = VG_(get_fnname_kind_from_IP)(ep, ips[i]); if (mbmkind != Vg_FnNameNormal) { mbm = i; break; @@ -983,7 +998,7 @@ Int VG_(XT_offset_main_or_below_main)(Addr* ips, Int n_ips) for (i = mbm - 1; i >= 0 && mbmkind != Vg_FnNameMain; i--) { - kind = VG_(get_fnname_kind_from_IP)(ips[i]); + kind = VG_(get_fnname_kind_from_IP)(ep, ips[i]); if (kind != Vg_FnNameNormal) { mbm = i; mbmkind = kind; @@ -1014,8 +1029,11 @@ void VG_(XT_filter_1top_and_maybe_below_main) if (VG_(clo_show_below_main)) mbm = n_ips - 1; - else - mbm = VG_(XT_offset_main_or_below_main)(ips, n_ips); + else { + // FIXME PW EPOCH : use the real ips epoch + const DiEpoch cur_ep = VG_(current_DiEpoch)(); + mbm = VG_(XT_offset_main_or_below_main)(cur_ep, ips, n_ips); + } *n_ips_sel = mbm - *top + 1; } @@ -1033,9 +1051,11 @@ void VG_(XT_filter_maybe_below_main) if (VG_(clo_show_below_main)) mbm = n_ips - 1; - else - mbm = VG_(XT_offset_main_or_below_main)(ips, n_ips); - + else { + // FIXME PW EPOCH : use the real ips epoch + const DiEpoch cur_ep = VG_(current_DiEpoch)(); + mbm = VG_(XT_offset_main_or_below_main)(cur_ep, ips, n_ips); + } *n_ips_sel = mbm - *top + 1; } diff --git a/coregrind/pub_core_debuginfo.h b/coregrind/pub_core_debuginfo.h index ae688e48d4..e3d8453ded 100644 --- a/coregrind/pub_core_debuginfo.h +++ b/coregrind/pub_core_debuginfo.h @@ -86,20 +86,21 @@ extern void VG_(di_discard_ALL_debuginfo)( void ); * It should only be used in cases where the names of interest will have * particular (ie. non-mangled) forms, or the mangled form is acceptable. */ extern -Bool VG_(get_fnname_raw) ( Addr a, const HChar** buf ); +Bool VG_(get_fnname_raw) ( DiEpoch ep, Addr a, const HChar** buf ); /* Like VG_(get_fnname), but without C++ demangling. (But it does Z-demangling and below-main renaming.) iipc argument: same usage as in VG_(describe_IP) in pub_tool_debuginfo.h. */ extern -Bool VG_(get_fnname_no_cxx_demangle) ( Addr a, const HChar** buf, +Bool VG_(get_fnname_no_cxx_demangle) ( DiEpoch ep, Addr a, const HChar** buf, const InlIPCursor* iipc ); /* mips-linux only: find the offset of current address. This is needed for stack unwinding for MIPS. */ extern -Bool VG_(get_inst_offset_in_function)( Addr a, /*OUT*/PtrdiffT* offset ); +Bool VG_(get_inst_offset_in_function)( DiEpoch ep, Addr a, + /*OUT*/PtrdiffT* offset ); /* Use DWARF2/3 CFA information to do one step of stack unwinding. @@ -158,6 +159,7 @@ extern Bool VG_(FPO_info_present)(void); extern Bool VG_(use_FPO_info) ( /*MOD*/Addr* ipP, /*MOD*/Addr* spP, /*MOD*/Addr* fpP, + DiEpoch ep, Addr min_accessible, Addr max_accessible ); @@ -217,7 +219,7 @@ void VG_(DebugInfo_syms_getidx) ( const DebugInfo *di, /* ppc64-linux only: find the TOC pointer (R2 value) that should be in force at the entry point address of the function containing guest_code_addr. Returns 0 if not known. */ -extern Addr VG_(get_tocptr) ( Addr guest_code_addr ); +extern Addr VG_(get_tocptr) ( DiEpoch ep, Addr guest_code_addr ); /* Map a function name to its SymAVMAs. Is done by sequential search of all symbol tables, so is very slow. To @@ -227,7 +229,8 @@ extern Addr VG_(get_tocptr) ( Addr guest_code_addr ); platforms, a symbol is deemed to be found only if it has a nonzero TOC pointer. */ extern -Bool VG_(lookup_symbol_SLOW)(const HChar* sopatt, const HChar* name, +Bool VG_(lookup_symbol_SLOW)(DiEpoch ep, + const HChar* sopatt, const HChar* name, SymAVMAs* avmas); #endif // __PUB_CORE_DEBUGINFO_H diff --git a/coregrind/pub_core_execontext.h b/coregrind/pub_core_execontext.h index cb52b46dc8..d42962ce85 100644 --- a/coregrind/pub_core_execontext.h +++ b/coregrind/pub_core_execontext.h @@ -47,6 +47,12 @@ // If with_stacktraces, outputs all the recorded stacktraces. extern void VG_(print_ExeContext_stats) ( Bool with_stacktraces ); +// All ExeContext that are valid in the current epoch and have one or more +// ips in the given range are archived, i.e. their epoch is frozen to +// the given last_epoch. +extern void VG_(archive_ExeContext_in_range) (DiEpoch last_epoch, + Addr text_avma, SizeT length ); + // Extract the StackTrace from an ExeContext. // (Minor hack: we use Addr* as the return type instead of StackTrace so // that modules #including this file don't also have to #include diff --git a/coregrind/pub_core_tooliface.h b/coregrind/pub_core_tooliface.h index 7fb8117455..2ea95a0101 100644 --- a/coregrind/pub_core_tooliface.h +++ b/coregrind/pub_core_tooliface.h @@ -156,7 +156,7 @@ typedef struct { void (*tool_print_stats)(void); // VG_(needs).info_location - void (*tool_info_location)(Addr a); + void (*tool_info_location)(DiEpoch ep, Addr a); // VG_(needs).malloc_replacement void* (*tool_malloc) (ThreadId, SizeT); diff --git a/docs/xml/manual-core.xml b/docs/xml/manual-core.xml index ff596b5214..6a038829ee 100644 --- a/docs/xml/manual-core.xml +++ b/docs/xml/manual-core.xml @@ -1232,6 +1232,28 @@ that can report errors, e.g. Memcheck, but not Cachegrind. + + + + + + When enabled, keep ("archive") symbols and all other debuginfo + for unloaded code. This allows saved stack traces to include file/line + info for code that has been dlclose'd (or similar). Be careful with + this, since it can lead to unbounded memory use for programs which + repeatedly load and unload shared objects. + Some tools and some functionalities have only limited support + for archived debug info. Memcheck fully supports it. Generally, + tools that report errors can use archived debug info to show the error + stack traces. The known limitations are: Helgrind's past access stack + trace of a race condition is does not use archived debug info. Massif + (and more generally the xtree Massif output format) does not make use + of archived debug info. Only Memcheck has been (somewhat) tested + with , so other tools may have + unknown limitations. + + + diff --git a/drd/drd_error.c b/drd/drd_error.c index 0dfab2c477..b9e62f1309 100644 --- a/drd/drd_error.c +++ b/drd/drd_error.c @@ -161,6 +161,7 @@ void drd_report_data_race(const Error* const err, const HChar* const indent = xml ? " " : ""; AddrInfo ai; + DiEpoch cur_ep = VG_(current_DiEpoch)(); XArray* /* of HChar */ descr1 = VG_(newXA)( VG_(malloc), "drd.error.drdr2.1", VG_(free), sizeof(HChar) ); @@ -172,7 +173,7 @@ void drd_report_data_race(const Error* const err, tl_assert(dri->addr); tl_assert(dri->size > 0); - (void) VG_(get_data_description)(descr1, descr2, dri->addr); + (void) VG_(get_data_description)(descr1, descr2, cur_ep, dri->addr); /* If there's nothing in descr1/2, free them. Why is it safe to VG_(indexXA) at zero here? Because VG_(get_data_description) guarantees to zero terminate descr1/2 regardless of the outcome diff --git a/exp-bbv/bbv_main.c b/exp-bbv/bbv_main.c index fa3a077e0b..ee2460d51e 100644 --- a/exp-bbv/bbv_main.c +++ b/exp-bbv/bbv_main.c @@ -346,6 +346,7 @@ static IRSB* bbv_instrument ( VgCallbackClosure* closure, IRDirty *di; IRExpr **argv, *arg1; Int regparms,opcode_type; + DiEpoch ep = VG_(current_DiEpoch)(); /* We don't handle a host/guest word size mismatch */ if (gWordTy != hWordTy) { @@ -392,8 +393,8 @@ static IRSB* bbv_instrument ( VgCallbackClosure* closure, block_num++; /* get function name and entry point information */ const HChar *fn_name; - VG_(get_fnname)(origAddr, &fn_name); - bbInfo->is_entry=VG_(get_fnname_if_entry)(origAddr, &fn_name); + VG_(get_fnname)(ep, origAddr, &fn_name); + bbInfo->is_entry=VG_(get_fnname_if_entry)(ep, origAddr, &fn_name); bbInfo->fn_name =VG_(strdup)("bbv_strings", fn_name); /* insert structure into table */ VG_(OSetGen_Insert)( instr_info_table, bbInfo ); diff --git a/exp-sgcheck/pc_common.c b/exp-sgcheck/pc_common.c index e5a3364217..ba44347aaf 100644 --- a/exp-sgcheck/pc_common.c +++ b/exp-sgcheck/pc_common.c @@ -650,6 +650,8 @@ void pc_pp_Error ( const Error* err ) UInt pc_update_Error_extra ( const Error* err ) { XError *xe = (XError*)VG_(get_error_extra)(err); + const DiEpoch ep = VG_(get_ExeContext_epoch)(VG_(get_error_where)(err)); + tl_assert(xe); switch (xe->tag) { case XE_SorG: @@ -675,7 +677,7 @@ UInt pc_update_Error_extra ( const Error* err ) have_descr = VG_(get_data_description)( xe->XE.Heap.descr1, xe->XE.Heap.descr2, - xe->XE.Heap.addr ); + ep, xe->XE.Heap.addr ); /* If there's nothing in descr1/2, free it. Why is it safe to to VG_(indexXA) at zero here? Because @@ -699,7 +701,7 @@ UInt pc_update_Error_extra ( const Error* err ) if (!have_descr) { const HChar *name; if (VG_(get_datasym_and_offset)( - xe->XE.Heap.addr, &name, + ep, xe->XE.Heap.addr, &name, &xe->XE.Heap.datasymoff ) ) { xe->XE.Heap.datasym = diff --git a/exp-sgcheck/sg_main.c b/exp-sgcheck/sg_main.c index 1daf65e724..d504afe358 100644 --- a/exp-sgcheck/sg_main.c +++ b/exp-sgcheck/sg_main.c @@ -1936,7 +1936,8 @@ void shadowStack_new_frame ( ThreadId tid, const HChar *fnname; Bool ok; Addr ip = ip_post_call_insn; - ok = VG_(get_fnname_w_offset)( ip, &fnname ); + DiEpoch ep = VG_(current_DiEpoch)(); + ok = VG_(get_fnname_w_offset)( ep, ip, &fnname ); while (d > 0) { VG_(printf)(" "); d--; diff --git a/helgrind/hg_addrdescr.c b/helgrind/hg_addrdescr.c index 90a9aa58ec..56f9a7369c 100644 --- a/helgrind/hg_addrdescr.c +++ b/helgrind/hg_addrdescr.c @@ -45,7 +45,7 @@ #include "hg_lock_n_thread.h" #include "hg_addrdescr.h" /* self */ -void HG_(describe_addr) ( Addr a, /*OUT*/AddrInfo* ai ) +void HG_(describe_addr) ( DiEpoch ep, Addr a, /*OUT*/AddrInfo* ai ) { tl_assert(ai->tag == Addr_Undescribed); @@ -81,7 +81,7 @@ void HG_(describe_addr) ( Addr a, /*OUT*/AddrInfo* ai ) ai->Addr.Block.freed_at = VG_(null_ExeContext)();; } else { /* No block found. Search a non-heap block description. */ - VG_(describe_addr) (a, ai); + VG_(describe_addr) (ep, a, ai); /* In case ai contains a tid, set tnr to the corresponding helgrind thread number. */ @@ -100,14 +100,14 @@ void HG_(describe_addr) ( Addr a, /*OUT*/AddrInfo* ai ) } } -Bool HG_(get_and_pp_addrdescr) (Addr addr) +Bool HG_(get_and_pp_addrdescr) (DiEpoch ep, Addr addr) { Bool ret; AddrInfo glai; glai.tag = Addr_Undescribed; - HG_(describe_addr) (addr, &glai); + HG_(describe_addr) (ep, addr, &glai); VG_(pp_addrinfo) (addr, &glai); ret = glai.tag != Addr_Unknown; diff --git a/helgrind/hg_addrdescr.h b/helgrind/hg_addrdescr.h index f7995973e6..012d5caaa3 100644 --- a/helgrind/hg_addrdescr.h +++ b/helgrind/hg_addrdescr.h @@ -37,12 +37,12 @@ lock description, putting the result in ai. This might allocate some memory in ai, to be cleared with VG_(clear_addrinfo). */ -extern void HG_(describe_addr) ( Addr a, /*OUT*/AddrInfo* ai ); +extern void HG_(describe_addr) ( DiEpoch ep, Addr a, /*OUT*/AddrInfo* ai ); /* Get a readable description of addr, then print it using HG_(pp_addrdescr) using xml False and VG_(printf) to emit the characters. Returns True if a description was found/printed, False otherwise. */ -extern Bool HG_(get_and_pp_addrdescr) (Addr a); +extern Bool HG_(get_and_pp_addrdescr) (DiEpoch ep, Addr a); /* For error creation/address description: map 'data_addr' to a malloc'd chunk, if any. diff --git a/helgrind/hg_errors.c b/helgrind/hg_errors.c index 273a7fdaf9..2eb9f0ef98 100644 --- a/helgrind/hg_errors.c +++ b/helgrind/hg_errors.c @@ -421,7 +421,8 @@ UInt HG_(update_extra) ( const Error* err ) VG_(printf)("HG_(update_extra): " "%d conflicting-event queries\n", xxx); - HG_(describe_addr) (xe->XE.Race.data_addr, &xe->XE.Race.data_addrinfo); + HG_(describe_addr) (VG_(get_ExeContext_epoch)(VG_(get_error_where)(err)), + xe->XE.Race.data_addr, &xe->XE.Race.data_addrinfo); /* And poke around in the conflicting-event map, to see if we can rustle up a plausible-looking conflicting memory access @@ -801,7 +802,11 @@ static void announce_LockP ( Lock* lk ) VG_(umsg)( " Lock at %p : no stacktrace for first observation\n", (void*)lk->guestaddr ); } - HG_(get_and_pp_addrdescr) (lk->guestaddr); + HG_(get_and_pp_addrdescr) + (lk->appeared_at + ? VG_(get_ExeContext_epoch)(lk->appeared_at) + : VG_(current_DiEpoch)(), + lk->guestaddr); VG_(umsg)("\n"); } } @@ -1307,7 +1312,8 @@ void HG_(print_access) (StackTrace ips, UInt n_ips, show_LockP_summary_textmode( locksHeldW_P, "" ); HG_(free) (locksHeldW_P); } - VG_(pp_StackTrace) (ips, n_ips); + // FIXME PW EPOCH : need the real ips epoch. + VG_(pp_StackTrace)( VG_(current_DiEpoch)(), ips, n_ips ); VG_(printf) ("\n"); } diff --git a/helgrind/hg_main.c b/helgrind/hg_main.c index e68ff2d9cf..0a3beea7d0 100644 --- a/helgrind/hg_main.c +++ b/helgrind/hg_main.c @@ -484,13 +484,15 @@ static void pp_Lock ( Int d, Lock* lk, Bool show_lock_addrdescr, Bool show_internal_data) { + // FIXME PW EPOCH should use the epoch of the allocated_at ec. + const DiEpoch cur_ep = VG_(current_DiEpoch)(); space(d+0); if (show_internal_data) VG_(printf)("Lock %p (ga %#lx) {\n", lk, lk->guestaddr); else VG_(printf)("Lock ga %#lx {\n", lk->guestaddr); if (!show_lock_addrdescr - || !HG_(get_and_pp_addrdescr) ((Addr) lk->guestaddr)) + || !HG_(get_and_pp_addrdescr) (cur_ep, (Addr) lk->guestaddr)) VG_(printf)("\n"); if (sHOW_ADMIN) { @@ -4688,7 +4690,7 @@ static Bool is_in_dynamic_linker_shared_object( Addr ga ) DebugInfo* dinfo; const HChar* soname; - dinfo = VG_(find_DebugInfo)( ga ); + dinfo = VG_(find_DebugInfo)( VG_(current_DiEpoch)(), ga ); if (!dinfo) return False; soname = VG_(DebugInfo_get_soname)(dinfo); @@ -5985,9 +5987,9 @@ static void hg_post_clo_init ( void ) VG_(XTMemory_Full_init)(VG_(XT_filter_1top_and_maybe_below_main)); } -static void hg_info_location (Addr a) +static void hg_info_location (DiEpoch ep, Addr a) { - (void) HG_(get_and_pp_addrdescr) (a); + (void) HG_(get_and_pp_addrdescr) (ep, a); } static void hg_pre_clo_init ( void ) diff --git a/helgrind/libhb_core.c b/helgrind/libhb_core.c index 792a3746ad..7169d0c169 100644 --- a/helgrind/libhb_core.c +++ b/helgrind/libhb_core.c @@ -4462,6 +4462,7 @@ static Bool check_cached_rcec_ok (Thr* thr, Addr previous_frame0) UWord frames[N_FRAMES]; UWord sps[N_FRAMES]; UWord fps[N_FRAMES]; + const DiEpoch cur_ep = VG_(current_DiEpoch)(); for (i = 0; i < N_FRAMES; i++) frames[i] = sps[i] = fps[i] = 0; @@ -4510,7 +4511,7 @@ static Bool check_cached_rcec_ok (Thr* thr, Addr previous_frame0) Vg_FnNameKind fr_kind; for (fr_main = 0; fr_main < N_FRAMES; fr_main++) { fr_kind = VG_(get_fnname_kind_from_IP) - (frames[fr_main]); + (cur_ep, frames[fr_main]); if (fr_kind == Vg_FnNameMain || fr_kind == Vg_FnNameBelowMain) break; } @@ -4518,7 +4519,7 @@ static Bool check_cached_rcec_ok (Thr* thr, Addr previous_frame0) Vg_FnNameKind kh_kind; for (kh_main = 0; kh_main < N_FRAMES; kh_main++) { kh_kind = VG_(get_fnname_kind_from_IP) - (thr->cached_rcec.frames[kh_main]); + (cur_ep, thr->cached_rcec.frames[kh_main]); if (kh_kind == Vg_FnNameMain || kh_kind == Vg_FnNameBelowMain) break; } @@ -4587,7 +4588,7 @@ static Bool check_cached_rcec_ok (Thr* thr, Addr previous_frame0) if (reason == NULL) { const HChar *fnname; for (UInt f = 0; f < N_FRAMES; f++) { - if (VG_(get_fnname)( frames[f], &fnname) + if (VG_(get_fnname)( cur_ep, frames[f], &fnname) && VG_(strcmp) ("__run_exit_handlers", fnname) == 0) { reason = "exit handlers"; break; @@ -4632,11 +4633,11 @@ static Bool check_cached_rcec_ok (Thr* thr, Addr previous_frame0) (void*)frames[i]); VG_(printf)("cached stack trace previous_frame0 %p\n", (void*)previous_frame0); - VG_(pp_StackTrace)(&previous_frame0, 1); + VG_(pp_StackTrace)(cur_ep, &previous_frame0, 1); VG_(printf)("resulting cached stack trace:\n"); - VG_(pp_StackTrace)(thr->cached_rcec.frames, N_FRAMES); + VG_(pp_StackTrace)(cur_ep, thr->cached_rcec.frames, N_FRAMES); VG_(printf)("check stack trace:\n"); - VG_(pp_StackTrace)(frames, N_FRAMES); + VG_(pp_StackTrace)(cur_ep, frames, N_FRAMES); VG_(show_sched_status) (False, // host_stacktrace False, // stack_usage @@ -4703,7 +4704,8 @@ static RCEC* get_RCEC ( Thr* thr ) Bool save_show_below_main = VG_(clo_show_below_main); VG_(clo_show_below_main) = True; VG_(printf)("caching stack trace:\n"); - VG_(pp_StackTrace)(&thr->cached_rcec.frames[0], N_FRAMES); + VG_(pp_StackTrace)(VG_(current_DiEpoch)(), + &thr->cached_rcec.frames[0], N_FRAMES); VG_(clo_show_below_main) = save_show_below_main; } stats__cached_rcec_fresh++; diff --git a/include/pub_tool_addrinfo.h b/include/pub_tool_addrinfo.h index 5808878372..5c10404f4e 100644 --- a/include/pub_tool_addrinfo.h +++ b/include/pub_tool_addrinfo.h @@ -126,7 +126,7 @@ struct _AddrInfo { // On a stack. tinfo indicates which thread's stack? // IP is the address of an instruction of the function where the - // stack address was. 0 if not found. + // stack address was. 0 if not found. IP can be symbolised using epoch. // frameNo is the frame nr of the call where the stack address was. // -1 if not found. // stackPos describes the address 'position' in the stack. @@ -135,6 +135,7 @@ struct _AddrInfo { // (spoffset will be negative, as stacks are assumed growing down). struct { ThreadInfo tinfo; + DiEpoch epoch; Addr IP; Int frameNo; StackPos stackPos; @@ -151,9 +152,9 @@ struct _AddrInfo { const HChar* block_desc; // "block","mempool","user-defined",arena SizeT block_szB; PtrdiffT rwoffset; - ExeContext* allocated_at; // might be null_ExeContext. - ThreadInfo alloc_tinfo; // which thread did alloc this block. - ExeContext* freed_at; // might be null_ExeContext. + ExeContext* allocated_at; // might contain null_ExeContext. + ThreadInfo alloc_tinfo; // which thread alloc'd this block. + ExeContext* freed_at; // might contain null_ExeContext. } Block; // In a global .data symbol. This holds @@ -204,7 +205,7 @@ struct _AddrInfo { On entry, ai->tag must be equal to Addr_Undescribed. This might allocate some memory, that can be cleared with VG_(clear_addrinfo). */ -extern void VG_(describe_addr) ( Addr a, /*OUT*/AddrInfo* ai ); +extern void VG_(describe_addr) ( DiEpoch ep, Addr a, /*OUT*/AddrInfo* ai ); extern void VG_(clear_addrinfo) ( AddrInfo* ai); diff --git a/include/pub_tool_basics.h b/include/pub_tool_basics.h index e3af28339f..757422679f 100644 --- a/include/pub_tool_basics.h +++ b/include/pub_tool_basics.h @@ -129,6 +129,24 @@ typedef struct { UWord uw1; UWord uw2; } UWordPair; /* ThreadIds are simply indices into the VG_(threads)[] array. */ typedef UInt ThreadId; + +/* You need a debuginfo epoch in order to convert an address into any source + level entity, since that conversion depends on what objects were mapped + in at the time. An epoch is simply a monotonically increasing counter, + which we wrap up in a struct so as to enable the C type system to + distinguish it from other kinds of numbers. m_debuginfo holds and + maintains the current epoch number. */ +typedef struct { UInt n; } DiEpoch; + +static inline DiEpoch DiEpoch_INVALID ( void ) { + DiEpoch dep; dep.n = 0; return dep; +} + +static inline Bool is_DiEpoch_INVALID ( DiEpoch dep ) { + return dep.n == 0; +} + + /* Many data structures need to allocate and release memory. The allocation/release functions must be provided by the caller. The Alloc_Fn_t function must allocate a chunk of memory of size szB. diff --git a/include/pub_tool_debuginfo.h b/include/pub_tool_debuginfo.h index 86ab2380e3..6098b9ec51 100644 --- a/include/pub_tool_debuginfo.h +++ b/include/pub_tool_debuginfo.h @@ -31,11 +31,20 @@ #ifndef __PUB_TOOL_DEBUGINFO_H #define __PUB_TOOL_DEBUGINFO_H -#include "pub_tool_basics.h" // VG_ macro +#include "pub_tool_basics.h" // VG_ macro, DiEpoch #include "pub_tool_xarray.h" // XArray + /*====================================================================*/ -/*=== Obtaining debug information ===*/ +/*=== Debuginfo epochs. ===*/ +/*====================================================================*/ + +// This returns the current epoch. +DiEpoch VG_(current_DiEpoch)(void); + + +/*====================================================================*/ +/*=== Obtaining information pertaining to source artefacts. ===*/ /*====================================================================*/ /* IMPORTANT COMMENT about memory persistence and ownership. @@ -76,11 +85,11 @@ demangles C++ function names. VG_(get_fnname_w_offset) is the same, except it appends "+N" to symbol names to indicate offsets. NOTE: See IMPORTANT COMMENT above about persistence and ownership. */ -extern Bool VG_(get_filename) ( Addr a, const HChar** filename ); -extern Bool VG_(get_fnname) ( Addr a, const HChar** fnname ); -extern Bool VG_(get_linenum) ( Addr a, UInt* linenum ); +extern Bool VG_(get_filename) ( DiEpoch ep, Addr a, const HChar** filename ); +extern Bool VG_(get_fnname) ( DiEpoch ep, Addr a, const HChar** fnname ); +extern Bool VG_(get_linenum) ( DiEpoch ep, Addr a, UInt* linenum ); extern Bool VG_(get_fnname_w_offset) - ( Addr a, const HChar** fnname ); + ( DiEpoch ep, Addr a, const HChar** fnname ); /* This one is the most general. It gives filename, line number and optionally directory name. filename and linenum may not be NULL. @@ -95,7 +104,7 @@ extern Bool VG_(get_fnname_w_offset) Returned value indicates whether any filename/line info could be found. */ extern Bool VG_(get_filename_linenum) - ( Addr a, + ( DiEpoch ep, Addr a, /*OUT*/const HChar** filename, /*OUT*/const HChar** dirname, /*OUT*/UInt* linenum ); @@ -108,7 +117,8 @@ extern Bool VG_(get_filename_linenum) of its symbols, this function will not be able to recognise function entry points within it. NOTE: See IMPORTANT COMMENT above about persistence and ownership. */ -extern Bool VG_(get_fnname_if_entry) ( Addr a, const HChar** fnname ); +extern Bool VG_(get_fnname_if_entry) ( DiEpoch ep, Addr a, + const HChar** fnname ); typedef enum { @@ -121,13 +131,13 @@ typedef extern Vg_FnNameKind VG_(get_fnname_kind) ( const HChar* name ); /* Like VG_(get_fnname_kind), but takes a code address. */ -extern Vg_FnNameKind VG_(get_fnname_kind_from_IP) ( Addr ip ); +extern Vg_FnNameKind VG_(get_fnname_kind_from_IP) ( DiEpoch ep, Addr ip ); /* Looks up data_addr in the collection of data symbols, and if found puts its name (or as much as will fit) into dname[0 .. n_dname-1], which is guaranteed to be zero terminated. Also data_addr's offset from the symbol start is put into *offset. */ -extern Bool VG_(get_datasym_and_offset)( Addr data_addr, +extern Bool VG_(get_datasym_and_offset)( DiEpoch ep, Addr data_addr, /*OUT*/const HChar** dname, /*OUT*/PtrdiffT* offset ); @@ -147,7 +157,7 @@ extern Bool VG_(get_datasym_and_offset)( Addr data_addr, Bool VG_(get_data_description)( /*MOD*/ XArray* /* of HChar */ dname1v, /*MOD*/ XArray* /* of HChar */ dname2v, - Addr data_addr + DiEpoch ep, Addr data_addr ); /* True if we have some Call Frame unwindo debuginfo for Addr a */ @@ -157,7 +167,7 @@ extern Bool VG_(has_CF_info)(Addr a); It first searches if Addr a belongs to the text segment of debug info. If not found, it asks the address space manager whether it knows the name of the file associated with this mapping. */ -extern Bool VG_(get_objname) ( Addr a, const HChar** objname ); +extern Bool VG_(get_objname) ( DiEpoch ep, Addr a, const HChar** objname ); /* Cursor allowing to describe inlined function calls at an IP, @@ -172,7 +182,7 @@ typedef struct _InlIPCursor InlIPCursor; eip can possibly corresponds to inlined function call(s). To describe eip and the inlined function calls, the following must be done: - InlIPCursor *iipc = VG_(new_IIPC)(eip); + InlIPCursor *iipc = VG_(new_IIPC)(ep, eip); do { buf = VG_(describe_IP)(eip, iipc); ... use buf ... @@ -185,12 +195,16 @@ typedef struct _InlIPCursor InlIPCursor; Note, that the returned string is allocated in a static buffer local to VG_(describe_IP). That buffer will be overwritten with every invocation. Therefore, callers need to possibly stash away the string. + + Since this maps a code location to a source artefact (function names), + new_IIPC requires a DiEpoch argument (ep) too. */ -extern const HChar* VG_(describe_IP)(Addr eip, const InlIPCursor* iipc); +extern const HChar* VG_(describe_IP)(DiEpoch ep, Addr eip, + const InlIPCursor* iipc); /* Builds a IIPC (Inlined IP Cursor) to describe eip and all the inlined calls at eip. Such a cursor must be deleted after use using VG_(delete_IIPC). */ -extern InlIPCursor* VG_(new_IIPC)(Addr eip); +extern InlIPCursor* VG_(new_IIPC)(DiEpoch ep, Addr eip); /* Move the cursor to the next call to describe. Returns True if there are still calls to describe. False if nothing to describe anymore. */ @@ -242,7 +256,7 @@ VG_(di_get_global_blocks_from_dihandle) ( ULong di_handle, /*====================================================================*/ -/*=== Obtaining debug information ===*/ +/*=== Obtaining information pertaining to shared objects. ===*/ /*====================================================================*/ /* A way to make limited debuginfo queries on a per-mapped-object @@ -251,7 +265,7 @@ typedef struct _DebugInfo DebugInfo; /* Returns NULL if the DebugInfo isn't found. It doesn't matter if debug info is present or not. */ -DebugInfo* VG_(find_DebugInfo) ( Addr a ); +DebugInfo* VG_(find_DebugInfo) ( DiEpoch ep, Addr a ); /* Fish bits out of DebugInfos. */ Addr VG_(DebugInfo_get_text_avma) ( const DebugInfo *di ); diff --git a/include/pub_tool_execontext.h b/include/pub_tool_execontext.h index 98419629f0..5dbbb67810 100644 --- a/include/pub_tool_execontext.h +++ b/include/pub_tool_execontext.h @@ -30,7 +30,13 @@ #ifndef __PUB_TOOL_EXECONTEXT_H #define __PUB_TOOL_EXECONTEXT_H -#include "pub_tool_basics.h" // ThreadID +#include "pub_tool_basics.h" // ThreadID +#include "pub_tool_debuginfo.h" // DiEpoch + + +/*====================================================================*/ +/*=== ExeContext ===*/ +/*====================================================================*/ // It's an abstract type. typedef @@ -74,6 +80,9 @@ ExeContext* VG_(record_depth_1_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 // --show-below-main=yes is set. +// Currently, the below function is unused. If ever it is used one day, +// we should add epoch args similarly to function VG_(apply_StackTrace) +// in pub_tool_stacktrace.h. extern void VG_(apply_ExeContext)( void(*action)(UInt n, Addr ip), ExeContext* ec, UInt n_ips ); @@ -93,6 +102,9 @@ extern void VG_(pp_ExeContext) ( ExeContext* ec ); // be zero, so that callers can store other information there. extern UInt VG_(get_ECU_from_ExeContext)( const ExeContext* e ); +// Returns the epoch in which the ips of e can be symbolised. +extern DiEpoch VG_(get_ExeContext_epoch)( const ExeContext* e ); + // How many entries (frames) in this ExeContext? extern Int VG_(get_ExeContext_n_ips)( const ExeContext* e ); diff --git a/include/pub_tool_options.h b/include/pub_tool_options.h index 0f93483c1c..1b0169acbc 100644 --- a/include/pub_tool_options.h +++ b/include/pub_tool_options.h @@ -249,6 +249,12 @@ extern Int VG_(clo_backtrace_size); /* Continue stack traces below main()? Default: NO */ extern Bool VG_(clo_show_below_main); +/* Keep symbols (and all other debuginfo) for code that is unloaded (dlclose + or similar) so that stack traces can still give line/file info for + previously captured stack traces. e.g. ... showing where a block was + allocated e.g. leaks of or accesses just outside a block. */ +extern Bool VG_(clo_keep_debuginfo); + /* Used to expand file names. "option_name" is the option name, eg. "--log-file". 'format' is what follows, eg. "cachegrind.out.%p". In diff --git a/include/pub_tool_stacktrace.h b/include/pub_tool_stacktrace.h index 848d026d13..17eca4196a 100644 --- a/include/pub_tool_stacktrace.h +++ b/include/pub_tool_stacktrace.h @@ -31,7 +31,7 @@ #ifndef __PUB_TOOL_STACKTRACE_H #define __PUB_TOOL_STACKTRACE_H -#include "pub_tool_basics.h" // Addr +#include "pub_tool_basics.h" // Addr, DiEpoch // The basic stack trace type: just an array of code addresses. typedef Addr* StackTrace; @@ -75,19 +75,19 @@ extern UInt VG_(get_StackTrace_with_deltas)( Word first_sp_delta ); -// Apply a function to every element in the StackTrace. The parameter -// 'n' gives the index of the passed ip. 'opaque' is an arbitrary -// pointer provided to each invocation of 'action' (a poor man's -// closure). Doesn't go below main() unless --show-below-main=yes is -// set. +// Apply a function to every element in the StackTrace. The parameter 'n' +// gives the index of the passed ip. 'opaque' is an arbitrary pointer +// provided to each invocation of 'action' (a poor man's closure). 'ep' is +// the debuginfo epoch assumed to apply to all code addresses in the stack +// trace. Doesn't go below main() unless --show-below-main=yes is set. extern void VG_(apply_StackTrace)( - void(*action)(UInt n, Addr ip, void* opaque), + void(*action)(UInt n, DiEpoch ep, Addr ip, void* opaque), void* opaque, - StackTrace ips, UInt n_ips + DiEpoch ep, StackTrace ips, UInt n_ips ); // Print a StackTrace. -extern void VG_(pp_StackTrace) ( StackTrace ips, UInt n_ips ); +extern void VG_(pp_StackTrace) ( DiEpoch ep, StackTrace ips, UInt n_ips ); // Gets and immediately prints a StackTrace. Just a bit simpler than // calling VG_(get_StackTrace)() then VG_(pp_StackTrace)(). diff --git a/include/pub_tool_tooliface.h b/include/pub_tool_tooliface.h index 9f9d20af9b..00ec5fef90 100644 --- a/include/pub_tool_tooliface.h +++ b/include/pub_tool_tooliface.h @@ -463,7 +463,7 @@ extern void VG_(needs_print_stats) ( of an address ? */ extern void VG_(needs_info_location) ( // Get and pp information about Addr - void (*info_location)(Addr) + void (*info_location)(DiEpoch, Addr) ); /* Do we need to see variable type and location information? */ diff --git a/include/pub_tool_xtree.h b/include/pub_tool_xtree.h index ba9ad34c1f..c71a95bd1a 100644 --- a/include/pub_tool_xtree.h +++ b/include/pub_tool_xtree.h @@ -121,7 +121,7 @@ extern void VG_(XT_filter_1top_and_maybe_below_main) /* Search in ips[0..n_ips-1] the first function which is main or below main and return its offset. If no main or below main is found, return n_ips-1 */ -extern Int VG_(XT_offset_main_or_below_main)(Addr* ips, Int n_ips); +extern Int VG_(XT_offset_main_or_below_main)(DiEpoch ep, Addr* ips, Int n_ips); /* Take a (frozen) snapshot of xt. diff --git a/lackey/lk_main.c b/lackey/lk_main.c index 707ba97227..1bcd8ed566 100644 --- a/lackey/lk_main.c +++ b/lackey/lk_main.c @@ -664,6 +664,7 @@ IRSB* lk_instrument ( VgCallbackClosure* closure, Addr iaddr = 0, dst; UInt ilen = 0; Bool condition_inverted = False; + DiEpoch ep = VG_(current_DiEpoch)(); if (gWordTy != hWordTy) { /* We don't currently support this case. */ @@ -750,7 +751,7 @@ IRSB* lk_instrument ( VgCallbackClosure* closure, tl_assert(clo_fnname); tl_assert(clo_fnname[0]); const HChar *fnname; - if (VG_(get_fnname_if_entry)(st->Ist.IMark.addr, + if (VG_(get_fnname_if_entry)(ep, st->Ist.IMark.addr, &fnname) && 0 == VG_(strcmp)(fnname, clo_fnname)) { di = unsafeIRDirty_0_N( diff --git a/massif/ms_main.c b/massif/ms_main.c index 3748821240..95ba944aca 100644 --- a/massif/ms_main.c +++ b/massif/ms_main.c @@ -520,8 +520,9 @@ void filter_IPs (Addr* ips, Int n_ips, // alloc function 'inside' a stacktrace e.g. // 0x1 0x2 0x3 alloc func1 main // became 0x1 0x2 0x3 func1 main + const DiEpoch ep = VG_(current_DiEpoch)(); for (i = *top; i < n_ips; i++) { - top_has_fnname = VG_(get_fnname)(ips[*top], &fnname); + top_has_fnname = VG_(get_fnname)(ep, ips[*top], &fnname); if (top_has_fnname && VG_(strIsMemberXA)(alloc_fns, fnname)) { VERB(4, "filtering alloc fn %s\n", fnname); (*top)++; @@ -536,7 +537,7 @@ void filter_IPs (Addr* ips, Int n_ips, if (!top_has_fnname) { // top has no fnname => search for the first entry that has a fnname for (i = *top; i < n_ips && !top_has_fnname; i++) { - top_has_fnname = VG_(get_fnname)(ips[i], &fnname); + top_has_fnname = VG_(get_fnname)(ep, ips[i], &fnname); } } if (top_has_fnname && VG_(strIsMemberXA)(ignore_fns, fnname)) { @@ -544,10 +545,18 @@ void filter_IPs (Addr* ips, Int n_ips, *top = n_ips; *n_ips_sel = 0; } - } + } if (!VG_(clo_show_below_main) && *n_ips_sel > 0 ) { - Int mbm = VG_(XT_offset_main_or_below_main)(ips, n_ips); + // Technically, it would be better to use the 'real' epoch that + // was used to capture ips/n_ips. However, this searches + // for a main or below_main function. It is technically possible + // but unlikely that main or below main fn is in a dlclose-d library, + // so current epoch is reasonable enough, even if not perfect. + // FIXME PW EPOCH: would be better to also use the real ips epoch here, + // once m_xtree.c massif output format properly supports epoch. + const DiEpoch cur_ep = VG_(current_DiEpoch)(); + Int mbm = VG_(XT_offset_main_or_below_main)(cur_ep, ips, n_ips); if (mbm < *top) { // Special case: the first main (or below main) function is an @@ -581,7 +590,8 @@ static ExeContext* make_ec(ThreadId tid, Bool exclude_first_entry) if (exclude_first_entry && n_ips > 0) { const HChar *fnname; VERB(4, "removing top fn %s from stacktrace\n", - VG_(get_fnname)(ips[0], &fnname) ? fnname : "???"); + VG_(get_fnname)(VG_(current_DiEpoch)(), ips[0], &fnname) + ? fnname : "???"); return VG_(make_ExeContext_from_StackTrace)(ips+1, n_ips-1); } else return VG_(make_ExeContext_from_StackTrace)(ips, n_ips); diff --git a/memcheck/mc_errors.c b/memcheck/mc_errors.c index 739034849e..d9f18476b4 100644 --- a/memcheck/mc_errors.c +++ b/memcheck/mc_errors.c @@ -1053,7 +1053,7 @@ static Bool mempool_block_maybe_describe( Addr a, Bool is_metapool, /* Describe an address as best you can, for error messages, putting the result in ai. */ -static void describe_addr ( Addr a, /*OUT*/AddrInfo* ai ) +static void describe_addr ( DiEpoch ep, Addr a, /*OUT*/AddrInfo* ai ) { MC_Chunk* mc; @@ -1121,15 +1121,15 @@ static void describe_addr ( Addr a, /*OUT*/AddrInfo* ai ) } /* No block found. Search a non-heap block description. */ - VG_(describe_addr) (a, ai); + VG_(describe_addr) (ep, a, ai); } -void MC_(pp_describe_addr) ( Addr a ) +void MC_(pp_describe_addr) ( DiEpoch ep, Addr a ) { AddrInfo ai; ai.tag = Addr_Undescribed; - describe_addr (a, &ai); + describe_addr (ep, a, &ai); VG_(pp_addrinfo_mc) (a, &ai, /* maybe_gcc */ False); VG_(clear_addrinfo) (&ai); } @@ -1150,6 +1150,7 @@ static void update_origin ( /*OUT*/ExeContext** origin_ec, UInt MC_(update_Error_extra)( const Error* err ) { MC_Error* extra = VG_(get_error_extra)(err); + DiEpoch ep = VG_(get_ExeContext_epoch)(VG_(get_error_where)(err)); switch (VG_(get_error_kind)(err)) { // These ones don't have addresses associated with them, and so don't @@ -1183,31 +1184,31 @@ UInt MC_(update_Error_extra)( const Error* err ) // These ones always involve a memory address. case Err_Addr: - describe_addr ( VG_(get_error_address)(err), + describe_addr ( ep, VG_(get_error_address)(err), &extra->Err.Addr.ai ); return sizeof(MC_Error); case Err_MemParam: - describe_addr ( VG_(get_error_address)(err), + describe_addr ( ep, VG_(get_error_address)(err), &extra->Err.MemParam.ai ); update_origin( &extra->Err.MemParam.origin_ec, extra->Err.MemParam.otag ); return sizeof(MC_Error); case Err_Jump: - describe_addr ( VG_(get_error_address)(err), + describe_addr ( ep, VG_(get_error_address)(err), &extra->Err.Jump.ai ); return sizeof(MC_Error); case Err_User: - describe_addr ( VG_(get_error_address)(err), + describe_addr ( ep, VG_(get_error_address)(err), &extra->Err.User.ai ); update_origin( &extra->Err.User.origin_ec, extra->Err.User.otag ); return sizeof(MC_Error); case Err_Free: - describe_addr ( VG_(get_error_address)(err), + describe_addr ( ep, VG_(get_error_address)(err), &extra->Err.Free.ai ); return sizeof(MC_Error); case Err_IllegalMempool: - describe_addr ( VG_(get_error_address)(err), + describe_addr ( ep, VG_(get_error_address)(err), &extra->Err.IllegalMempool.ai ); return sizeof(MC_Error); diff --git a/memcheck/mc_include.h b/memcheck/mc_include.h index e0f5fbeff7..765634b912 100644 --- a/memcheck/mc_include.h +++ b/memcheck/mc_include.h @@ -569,8 +569,8 @@ Bool MC_(record_fishy_value_error) ( ThreadId tid, const HChar* function, /* Leak kinds tokens to call VG_(parse_enum_set). */ extern const HChar* MC_(parse_leak_kinds_tokens); -/* prints a description of address a */ -void MC_(pp_describe_addr) (Addr a); +/* prints a description of address a in the specified debuginfo epoch */ +void MC_(pp_describe_addr) ( DiEpoch ep, Addr a ); /* Is this address in a user-specified "ignored range" ? */ Bool MC_(in_ignored_range) ( Addr a ); diff --git a/memcheck/mc_leakcheck.c b/memcheck/mc_leakcheck.c index c026d27c2a..782244481f 100644 --- a/memcheck/mc_leakcheck.c +++ b/memcheck/mc_leakcheck.c @@ -1137,16 +1137,18 @@ lc_scan_memory(Addr start, SizeT len, Bool is_prior_definite, // let's see if its contents point to a chunk. if (UNLIKELY(searched)) { if (addr >= searched && addr < searched + szB) { + const DiEpoch cur_ep = VG_(current_DiEpoch)(); + // The found addr is 'live', so use cur_ep to describe it. if (addr == searched) { VG_(umsg)("*%#lx points at %#lx\n", ptr, searched); - MC_(pp_describe_addr) (ptr); + MC_(pp_describe_addr) (cur_ep, ptr); } else { Int ch_no; MC_Chunk *ch; LC_Extra *ex; VG_(umsg)("*%#lx interior points at %lu bytes inside %#lx\n", ptr, (long unsigned) addr - searched, searched); - MC_(pp_describe_addr) (ptr); + MC_(pp_describe_addr) (cur_ep, ptr); if (lc_is_a_chunk_ptr(addr, &ch_no, &ch, &ex) ) { Int h; for (h = LchStdString; h < N_LEAK_CHECK_HEURISTICS; h++) { diff --git a/memcheck/mc_main.c b/memcheck/mc_main.c index 15bf014a21..834f5976e2 100644 --- a/memcheck/mc_main.c +++ b/memcheck/mc_main.c @@ -6746,7 +6746,8 @@ static Bool handle_gdb_monitor_command (ThreadId tid, HChar *req) VG_(printf) ("Address %p len %lu not addressable:\nbad address %p\n", (void *)address, szB, (void *) bad_addr); - MC_(pp_describe_addr) (address); + // Describe this (probably live) address with current epoch + MC_(pp_describe_addr) (VG_(current_DiEpoch)(), address); break; case 1: /* defined */ res = is_mem_defined ( address, szB, &bad_addr, &otag ); @@ -6780,7 +6781,8 @@ static Bool handle_gdb_monitor_command (ThreadId tid, HChar *req) else VG_(printf) ("Address %p len %lu defined\n", (void *)address, szB); - MC_(pp_describe_addr) (address); + // Describe this (probably live) address with current epoch + MC_(pp_describe_addr) (VG_(current_DiEpoch)(), address); break; default: tl_assert(0); } diff --git a/memcheck/tests/linux/Makefile.am b/memcheck/tests/linux/Makefile.am index 04a031beef..5f49a3fe22 100644 --- a/memcheck/tests/linux/Makefile.am +++ b/memcheck/tests/linux/Makefile.am @@ -6,6 +6,10 @@ dist_noinst_SCRIPTS = filter_stderr EXTRA_DIST = \ brk.stderr.exp brk.vgtest \ capget.vgtest capget.stderr.exp capget.stderr.exp2 capget.stderr.exp3 \ + dlclose_leak-no-keep.stderr.exp dlclose_leak-no-keep.stdout.exp \ + dlclose_leak-no-keep.vgtest \ + dlclose_leak.stderr.exp dlclose_leak.stdout.exp \ + dlclose_leak.vgtest \ ioctl-tiocsig.vgtest ioctl-tiocsig.stderr.exp \ lsframe1.vgtest lsframe1.stdout.exp lsframe1.stderr.exp \ lsframe2.vgtest lsframe2.stdout.exp lsframe2.stderr.exp \ @@ -25,6 +29,7 @@ EXTRA_DIST = \ check_PROGRAMS = \ brk \ capget \ + dlclose_leak dlclose_leak_so.so \ ioctl-tiocsig \ getregset \ lsframe1 \ @@ -48,3 +53,17 @@ AM_CXXFLAGS += $(AM_FLAG_M3264_PRI) stack_switch_LDADD = -lpthread timerfd_syscall_LDADD = -lrt +# Build shared object for dlclose_leak +dlclose_leak_so_so_SOURCES = dlclose_leak_so.c +dlclose_leak_so_so_CFLAGS = $(AM_CFLAGS) -fpic -g -O0 +dlclose_leak_so_so_LDFLAGS = -fpic $(AM_FLAG_M3264_PRI) -shared -Wl,-soname \ + -Wl,dlclose_leak_so.so + +dlclose_leak_SOURCES = dlclose_leak.c +dlclose_leak_DEPENDENCIES = dlclose_leak_so.so +# Do NOT uncomment the below line: we must not link with the .so, +# in order to properly test a 'fully dynamic' use of dlopen/dlclose +# dlclose_leak_LDADD = dlclose_leak_so.so +dlclose_leak_LDADD = -ldl +dlclose_leak_LDFLAGS = $(AM_FLAG_M3264_PRI) \ + -Wl,-rpath,$(top_builddir)/memcheck/tests/linux diff --git a/memcheck/tests/linux/dlclose_leak-no-keep.stderr.exp b/memcheck/tests/linux/dlclose_leak-no-keep.stderr.exp new file mode 100644 index 0000000000..107b7dd79e --- /dev/null +++ b/memcheck/tests/linux/dlclose_leak-no-keep.stderr.exp @@ -0,0 +1,17 @@ +Conditional jump or move depends on uninitialised value(s) + at 0x........: jmp_on_uninit (dlclose_leak_so.c:10) + by 0x........: main (dlclose_leak.c:26) + +Invalid read of size 1 + at 0x........: main (dlclose_leak.c:29) + Address 0x........ is 1 bytes before a block of size 1 alloc'd + at 0x........: malloc (vg_replace_malloc.c:...) + ... + by 0x........: main (dlclose_leak.c:27) + +done! +1 bytes in 1 blocks are definitely lost in loss record ... of ... + at 0x........: malloc (vg_replace_malloc.c:...) + ... + by 0x........: main (dlclose_leak.c:27) + diff --git a/memcheck/tests/linux/dlclose_leak-no-keep.stdout.exp b/memcheck/tests/linux/dlclose_leak-no-keep.stdout.exp new file mode 100644 index 0000000000..e69de29bb2 diff --git a/memcheck/tests/linux/dlclose_leak-no-keep.vgtest b/memcheck/tests/linux/dlclose_leak-no-keep.vgtest new file mode 100644 index 0000000000..306ef2e7f8 --- /dev/null +++ b/memcheck/tests/linux/dlclose_leak-no-keep.vgtest @@ -0,0 +1,4 @@ +prog: dlclose_leak +stderr_filter: ../filter_stderr +stderr_filter_args: dlclose_leak +vgopts: -q --leak-check=yes --keep-debuginfo=no diff --git a/memcheck/tests/linux/dlclose_leak.c b/memcheck/tests/linux/dlclose_leak.c new file mode 100644 index 0000000000..33fd2200f6 --- /dev/null +++ b/memcheck/tests/linux/dlclose_leak.c @@ -0,0 +1,32 @@ +/* Test reporting of memory leaks in objects that have been dlopen'ed. + * File: dlclose_leak.c */ + +#include +#include +#include +#include + +int (*jmp_on_uninit)(void); +char* (*alloc_1_byte)(void); + +int main(int argc, char** argv) { + char* memToLeak; + char x __attribute__((unused)); + void* handle = dlopen("./dlclose_leak_so.so", RTLD_NOW); + if(!handle) { + printf("FAILURE to dlopen dlclose_leak_so.so\n"); + return EXIT_FAILURE; + } + jmp_on_uninit = dlsym(handle,"jmp_on_uninit"); + //fprintf(stderr, "jmp_on_uninit: %p\n", jmp_on_uninit); + assert(jmp_on_uninit); + alloc_1_byte = dlsym(handle,"alloc_1_byte"); + //fprintf(stderr, "alloc_1_byte: %p\n", alloc_1_byte); + assert(alloc_1_byte); + (void)jmp_on_uninit(); + memToLeak = alloc_1_byte(); + dlclose(handle); + x = memToLeak[-1]; + fprintf(stderr, "done!\n"); + return (EXIT_SUCCESS); +} diff --git a/memcheck/tests/linux/dlclose_leak.stderr.exp b/memcheck/tests/linux/dlclose_leak.stderr.exp new file mode 100644 index 0000000000..3c5b44e38a --- /dev/null +++ b/memcheck/tests/linux/dlclose_leak.stderr.exp @@ -0,0 +1,17 @@ +Conditional jump or move depends on uninitialised value(s) + at 0x........: jmp_on_uninit (dlclose_leak_so.c:10) + by 0x........: main (dlclose_leak.c:26) + +Invalid read of size 1 + at 0x........: main (dlclose_leak.c:29) + Address 0x........ is 1 bytes before a block of size 1 alloc'd + at 0x........: malloc (vg_replace_malloc.c:...) + by 0x........: alloc_1_byte (dlclose_leak_so.c:20) + by 0x........: main (dlclose_leak.c:27) + +done! +1 bytes in 1 blocks are definitely lost in loss record ... of ... + at 0x........: malloc (vg_replace_malloc.c:...) + by 0x........: alloc_1_byte (dlclose_leak_so.c:20) + by 0x........: main (dlclose_leak.c:27) + diff --git a/memcheck/tests/linux/dlclose_leak.stdout.exp b/memcheck/tests/linux/dlclose_leak.stdout.exp new file mode 100644 index 0000000000..e69de29bb2 diff --git a/memcheck/tests/linux/dlclose_leak.vgtest b/memcheck/tests/linux/dlclose_leak.vgtest new file mode 100644 index 0000000000..caddcbe478 --- /dev/null +++ b/memcheck/tests/linux/dlclose_leak.vgtest @@ -0,0 +1,3 @@ +prog: dlclose_leak +stderr_filter: ../filter_stderr +vgopts: -q --leak-check=yes --keep-debuginfo=yes diff --git a/memcheck/tests/linux/dlclose_leak_so.c b/memcheck/tests/linux/dlclose_leak_so.c new file mode 100644 index 0000000000..fa3205e21e --- /dev/null +++ b/memcheck/tests/linux/dlclose_leak_so.c @@ -0,0 +1,21 @@ +/* dlclose_leak_so.c */ + +#include + +/** Makes a jump based on an uninitialized variable in order to make sure + * errors reported while the dlopen'ed object is loaded work. */ +int jmp_on_uninit(void) { + int uninit[27]; + __asm__ __volatile("":::"cc","memory"); + if(uninit[13]) { + return 1; + } else { + return 0; + } +} + +/** Leak 1 byte of memory. This is to test the stack check reported after the + * object has been dlclose'd. */ +char* alloc_1_byte(void) { + return (char*)malloc(1); +} diff --git a/none/tests/cmdline1.stdout.exp b/none/tests/cmdline1.stdout.exp index 248511936d..bc2eb29bcc 100644 --- a/none/tests/cmdline1.stdout.exp +++ b/none/tests/cmdline1.stdout.exp @@ -43,6 +43,10 @@ usage: valgrind [options] prog-and-args --error-exitcode= exit code to return if errors found [0=disable] --error-markers=, add lines with begin/end markers before/after each error output in plain text mode [none] + --keep-debuginfo=no|yes Keep symbols etc for unloaded code [no] + This allows saved stack traces (e.g. memory leaks) + to include file/line info for code that has been + dlclose'd (or similar) --show-below-main=no|yes continue stack traces below main() [no] --default-suppressions=yes|no load default suppressions [yes] diff --git a/none/tests/cmdline2.stdout.exp b/none/tests/cmdline2.stdout.exp index df8f85a684..2599a1b2f1 100644 --- a/none/tests/cmdline2.stdout.exp +++ b/none/tests/cmdline2.stdout.exp @@ -43,6 +43,10 @@ usage: valgrind [options] prog-and-args --error-exitcode= exit code to return if errors found [0=disable] --error-markers=, add lines with begin/end markers before/after each error output in plain text mode [none] + --keep-debuginfo=no|yes Keep symbols etc for unloaded code [no] + This allows saved stack traces (e.g. memory leaks) + to include file/line info for code that has been + dlclose'd (or similar) --show-below-main=no|yes continue stack traces below main() [no] --default-suppressions=yes|no load default suppressions [yes]