From: Julian Seward Date: Sat, 8 Nov 2008 20:36:26 +0000 (+0000) Subject: A bit of tidying up: X-Git-Tag: svn/VALGRIND_3_4_0~145 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=a500c38d138de4d9e28e58e1b64f09596e4e936e;p=thirdparty%2Fvalgrind.git A bit of tidying up: * get rid of 'struct _EC' (a.k.a 'struct EC_') and use ExeContext everywhere * remove stacktrace_to_EC and call VG_(make_ExeContext_from_StackTrace) directly * comment out some unused code git-svn-id: svn://svn.valgrind.org/valgrind/trunk@8749 --- diff --git a/helgrind/hg_main.c b/helgrind/hg_main.c index d290c4dc8c..da2c9b8d1f 100644 --- a/helgrind/hg_main.c +++ b/helgrind/hg_main.c @@ -356,7 +356,7 @@ static void remove_Lock_from_locksets_of_all_owning_Threads( Lock* lk ) /*--- Print out the primary data structures ---*/ /*----------------------------------------------------------------*/ -static WordSetID del_BHL ( WordSetID lockset ); /* fwds */ +//static WordSetID del_BHL ( WordSetID lockset ); /* fwds */ #define PP_THREADS (1<<1) #define PP_LOCKS (1<<2) @@ -909,130 +909,130 @@ static void all__sanity_check ( Char* who ) { /*--- the core memory state machine (msm__* functions) ---*/ /*----------------------------------------------------------------*/ -static WordSetID add_BHL ( WordSetID lockset ) { - return HG_(addToWS)( univ_lsets, lockset, (Word)__bus_lock_Lock ); -} -static WordSetID del_BHL ( WordSetID lockset ) { - return HG_(delFromWS)( univ_lsets, lockset, (Word)__bus_lock_Lock ); -} - - -/* Last-lock-lossage records. This mechanism exists to help explain - to programmers why we are complaining about a race. The idea is to - monitor all lockset transitions. When a previously nonempty - lockset becomes empty, the lock(s) that just disappeared (the - "lossage") are the locks that have consistently protected the - location (ga_of_access) in question for the longest time. Most of - the time the lossage-set is a single lock. Because the - lossage-lock is the one that has survived longest, there is there - is a good chance that it is indeed the lock that the programmer - intended to use to protect the location. - - Note that we cannot in general just look at the lossage set when we - see a transition to ShM(...,empty-set), because a transition to an - empty lockset can happen arbitrarily far before the point where we - want to report an error. This is in the case where there are many - transitions ShR -> ShR, all with an empty lockset, and only later - is there a transition to ShM. So what we want to do is note the - lossage lock at the point where a ShR -> ShR transition empties out - the lockset, so we can present it later if there should be a - transition to ShM. - - So this function finds such transitions. For each, it associates - in ga_to_lastlock, the guest address and the lossage lock. In fact - we do not record the Lock* directly as that may disappear later, - but instead the ExeContext inside the Lock which says where it was - initialised or first locked. ExeContexts are permanent so keeping - them indefinitely is safe. - - A boring detail: the hardware bus lock is not interesting in this - respect, so we first remove that from the pre/post locksets. -*/ - -static UWord stats__ga_LL_adds = 0; - -static WordFM* ga_to_lastlock = NULL; /* GuestAddr -> ExeContext* */ - -static -void record_last_lock_lossage ( Addr ga_of_access, - WordSetID lset_old, WordSetID lset_new ) -{ - Lock* lk; - Int card_old, card_new; - - tl_assert(lset_old != lset_new); - - if (0) VG_(printf)("XX1: %d (card %ld) -> %d (card %ld) %#lx\n", - (Int)lset_old, - HG_(cardinalityWS)(univ_lsets,lset_old), - (Int)lset_new, - HG_(cardinalityWS)(univ_lsets,lset_new), - ga_of_access ); - - /* This is slow, but at least it's simple. The bus hardware lock - just confuses the logic, so remove it from the locksets we're - considering before doing anything else. */ - lset_new = del_BHL( lset_new ); - - if (!HG_(isEmptyWS)( univ_lsets, lset_new )) { - /* The post-transition lock set is not empty. So we are not - interested. We're only interested in spotting transitions - that make locksets become empty. */ - return; - } - - /* lset_new is now empty */ - card_new = HG_(cardinalityWS)( univ_lsets, lset_new ); - tl_assert(card_new == 0); - - lset_old = del_BHL( lset_old ); - card_old = HG_(cardinalityWS)( univ_lsets, lset_old ); - - if (0) VG_(printf)(" X2: %d (card %d) -> %d (card %d)\n", - (Int)lset_old, card_old, (Int)lset_new, card_new ); - - if (card_old == 0) { - /* The old lockset was also empty. Not interesting. */ - return; - } - - tl_assert(card_old > 0); - tl_assert(!HG_(isEmptyWS)( univ_lsets, lset_old )); - - /* Now we know we've got a transition from a nonempty lockset to an - empty one. So lset_old must be the set of locks lost. Record - some details. If there is more than one element in the lossage - set, just choose one arbitrarily -- not the best, but at least - it's simple. */ - - lk = (Lock*)HG_(anyElementOfWS)( univ_lsets, lset_old ); - if (0) VG_(printf)("lossage %ld %p\n", - HG_(cardinalityWS)( univ_lsets, lset_old), lk ); - if (lk->appeared_at) { - if (ga_to_lastlock == NULL) - ga_to_lastlock = VG_(newFM)( HG_(zalloc), "hg.rlll.1", HG_(free), NULL ); - VG_(addToFM)( ga_to_lastlock, ga_of_access, (Word)lk->appeared_at ); - stats__ga_LL_adds++; - } -} - -/* This queries the table (ga_to_lastlock) made by - record_last_lock_lossage, when constructing error messages. It - attempts to find the ExeContext of the allocation or initialisation - point for the lossage lock associated with 'ga'. */ - -static ExeContext* maybe_get_lastlock_initpoint ( Addr ga ) -{ - ExeContext* ec_hint = NULL; - if (ga_to_lastlock != NULL - && VG_(lookupFM)(ga_to_lastlock, - NULL, (Word*)&ec_hint, ga)) { - tl_assert(ec_hint != NULL); - return ec_hint; - } else { - return NULL; - } -} +//static WordSetID add_BHL ( WordSetID lockset ) { +// return HG_(addToWS)( univ_lsets, lockset, (Word)__bus_lock_Lock ); +//} +//static WordSetID del_BHL ( WordSetID lockset ) { +// return HG_(delFromWS)( univ_lsets, lockset, (Word)__bus_lock_Lock ); +//} + + +///* Last-lock-lossage records. This mechanism exists to help explain +// to programmers why we are complaining about a race. The idea is to +// monitor all lockset transitions. When a previously nonempty +// lockset becomes empty, the lock(s) that just disappeared (the +// "lossage") are the locks that have consistently protected the +// location (ga_of_access) in question for the longest time. Most of +// the time the lossage-set is a single lock. Because the +// lossage-lock is the one that has survived longest, there is there +// is a good chance that it is indeed the lock that the programmer +// intended to use to protect the location. +// +// Note that we cannot in general just look at the lossage set when we +// see a transition to ShM(...,empty-set), because a transition to an +// empty lockset can happen arbitrarily far before the point where we +// want to report an error. This is in the case where there are many +// transitions ShR -> ShR, all with an empty lockset, and only later +// is there a transition to ShM. So what we want to do is note the +// lossage lock at the point where a ShR -> ShR transition empties out +// the lockset, so we can present it later if there should be a +// transition to ShM. +// +// So this function finds such transitions. For each, it associates +// in ga_to_lastlock, the guest address and the lossage lock. In fact +// we do not record the Lock* directly as that may disappear later, +// but instead the ExeContext inside the Lock which says where it was +// initialised or first locked. ExeContexts are permanent so keeping +// them indefinitely is safe. +// +// A boring detail: the hardware bus lock is not interesting in this +// respect, so we first remove that from the pre/post locksets. +//*/ +// +//static UWord stats__ga_LL_adds = 0; +// +//static WordFM* ga_to_lastlock = NULL; /* GuestAddr -> ExeContext* */ +// +//static +//void record_last_lock_lossage ( Addr ga_of_access, +// WordSetID lset_old, WordSetID lset_new ) +//{ +// Lock* lk; +// Int card_old, card_new; +// +// tl_assert(lset_old != lset_new); +// +// if (0) VG_(printf)("XX1: %d (card %ld) -> %d (card %ld) %#lx\n", +// (Int)lset_old, +// HG_(cardinalityWS)(univ_lsets,lset_old), +// (Int)lset_new, +// HG_(cardinalityWS)(univ_lsets,lset_new), +// ga_of_access ); +// +// /* This is slow, but at least it's simple. The bus hardware lock +// just confuses the logic, so remove it from the locksets we're +// considering before doing anything else. */ +// lset_new = del_BHL( lset_new ); +// +// if (!HG_(isEmptyWS)( univ_lsets, lset_new )) { +// /* The post-transition lock set is not empty. So we are not +// interested. We're only interested in spotting transitions +// that make locksets become empty. */ +// return; +// } +// +// /* lset_new is now empty */ +// card_new = HG_(cardinalityWS)( univ_lsets, lset_new ); +// tl_assert(card_new == 0); +// +// lset_old = del_BHL( lset_old ); +// card_old = HG_(cardinalityWS)( univ_lsets, lset_old ); +// +// if (0) VG_(printf)(" X2: %d (card %d) -> %d (card %d)\n", +// (Int)lset_old, card_old, (Int)lset_new, card_new ); +// +// if (card_old == 0) { +// /* The old lockset was also empty. Not interesting. */ +// return; +// } +// +// tl_assert(card_old > 0); +// tl_assert(!HG_(isEmptyWS)( univ_lsets, lset_old )); +// +// /* Now we know we've got a transition from a nonempty lockset to an +// empty one. So lset_old must be the set of locks lost. Record +// some details. If there is more than one element in the lossage +// set, just choose one arbitrarily -- not the best, but at least +// it's simple. */ +// +// lk = (Lock*)HG_(anyElementOfWS)( univ_lsets, lset_old ); +// if (0) VG_(printf)("lossage %ld %p\n", +// HG_(cardinalityWS)( univ_lsets, lset_old), lk ); +// if (lk->appeared_at) { +// if (ga_to_lastlock == NULL) +// ga_to_lastlock = VG_(newFM)( HG_(zalloc), "hg.rlll.1", HG_(free), NULL ); +// VG_(addToFM)( ga_to_lastlock, ga_of_access, (Word)lk->appeared_at ); +// stats__ga_LL_adds++; +// } +//} +// +///* This queries the table (ga_to_lastlock) made by +// record_last_lock_lossage, when constructing error messages. It +// attempts to find the ExeContext of the allocation or initialisation +// point for the lossage lock associated with 'ga'. */ +// +//static ExeContext* maybe_get_lastlock_initpoint ( Addr ga ) +//{ +// ExeContext* ec_hint = NULL; +// if (ga_to_lastlock != NULL +// && VG_(lookupFM)(ga_to_lastlock, +// NULL, (Word*)&ec_hint, ga)) { +// tl_assert(ec_hint != NULL); +// return ec_hint; +// } else { +// return NULL; +// } +//} /*----------------------------------------------------------------*/ @@ -1797,20 +1797,20 @@ void evh__mem_help_write_N(Addr a, SizeT size) { LIBHB_WRITE_N(hbthr, a, size); } -static void evh__bus_lock(void) { - Thread* thr; - if (0) VG_(printf)("evh__bus_lock()\n"); - thr = get_current_Thread(); - tl_assert(thr); /* cannot fail - Thread* must already exist */ - evhH__post_thread_w_acquires_lock( thr, LK_nonRec, (Addr)&__bus_lock ); -} -static void evh__bus_unlock(void) { - Thread* thr; - if (0) VG_(printf)("evh__bus_unlock()\n"); - thr = get_current_Thread(); - tl_assert(thr); /* cannot fail - Thread* must already exist */ - evhH__pre_thread_releases_lock( thr, (Addr)&__bus_lock, False/*!isRDWR*/ ); -} +//static void evh__bus_lock(void) { +// Thread* thr; +// if (0) VG_(printf)("evh__bus_lock()\n"); +// thr = get_current_Thread(); +// tl_assert(thr); /* cannot fail - Thread* must already exist */ +// evhH__post_thread_w_acquires_lock( thr, LK_nonRec, (Addr)&__bus_lock ); +//} +//static void evh__bus_unlock(void) { +// Thread* thr; +// if (0) VG_(printf)("evh__bus_unlock()\n"); +// thr = get_current_Thread(); +// tl_assert(thr); /* cannot fail - Thread* must already exist */ +// evhH__pre_thread_releases_lock( thr, (Addr)&__bus_lock, False/*!isRDWR*/ ); +//} /* -------------- events to do with mutexes -------------- */ @@ -3277,38 +3277,38 @@ static void instrument_mem_access ( IRSB* bbOut, } -static void instrument_memory_bus_event ( IRSB* bbOut, IRMBusEvent event ) -{ - switch (event) { - case Imbe_SnoopedStoreBegin: - case Imbe_SnoopedStoreEnd: - /* These arise from ppc stwcx. insns. They should perhaps be - handled better. */ - break; - case Imbe_Fence: - break; /* not interesting */ - case Imbe_BusLock: - case Imbe_BusUnlock: - addStmtToIRSB( - bbOut, - IRStmt_Dirty( - unsafeIRDirty_0_N( - 0/*regparms*/, - event == Imbe_BusLock ? "evh__bus_lock" - : "evh__bus_unlock", - VG_(fnptr_to_fnentry)( - event == Imbe_BusLock ? &evh__bus_lock - : &evh__bus_unlock - ), - mkIRExprVec_0() - ) - ) - ); - break; - default: - tl_assert(0); - } -} +//static void instrument_memory_bus_event ( IRSB* bbOut, IRMBusEvent event ) +//{ +// switch (event) { +// case Imbe_SnoopedStoreBegin: +// case Imbe_SnoopedStoreEnd: +// /* These arise from ppc stwcx. insns. They should perhaps be +// handled better. */ +// break; +// case Imbe_Fence: +// break; /* not interesting */ +// case Imbe_BusLock: +// case Imbe_BusUnlock: +// addStmtToIRSB( +// bbOut, +// IRStmt_Dirty( +// unsafeIRDirty_0_N( +// 0/*regparms*/, +// event == Imbe_BusLock ? "evh__bus_lock" +// : "evh__bus_unlock", +// VG_(fnptr_to_fnentry)( +// event == Imbe_BusLock ? &evh__bus_lock +// : &evh__bus_unlock +// ), +// mkIRExprVec_0() +// ) +// ) +// ); +// break; +// default: +// tl_assert(0); +// } +//} static @@ -3799,9 +3799,9 @@ static void hg_fini ( Int exitcode ) VG_(printf)(" univ_laog: %'8d unique lock sets\n", (Int)HG_(cardinalityWSU)( univ_laog )); - VG_(printf)("L(ast)L(ock) map: %'8lu inserts (%d map size)\n", - stats__ga_LL_adds, - (Int)(ga_to_lastlock ? VG_(sizeFM)( ga_to_lastlock ) : 0) ); + //VG_(printf)("L(ast)L(ock) map: %'8lu inserts (%d map size)\n", + // stats__ga_LL_adds, + // (Int)(ga_to_lastlock ? VG_(sizeFM)( ga_to_lastlock ) : 0) ); VG_(printf)(" LockN-to-P map: %'8llu queries (%llu map size)\n", HG_(stats__LockN_to_P_queries), @@ -3846,13 +3846,7 @@ void for_libhb__get_stacktrace ( Thr* hbt, Addr* frames, UWord nRequest ) } static -struct EC_* for_libhb__stacktrace_to_EC ( Addr* frames, UWord nFrames ) -{ - return VG_(make_ExeContext_from_StackTrace)( frames, (UInt)nFrames ); -} - -static -struct EC_* for_libhb__get_EC ( Thr* hbt ) +ExeContext* for_libhb__get_EC ( Thr* hbt ) { Thread* thr; ThreadId tid; @@ -3862,7 +3856,7 @@ struct EC_* for_libhb__get_EC ( Thr* hbt ) tl_assert(thr); tid = map_threads_maybe_reverse_lookup_SLOW(thr); ec = VG_(record_ExeContext)( tid, 0 ); - return (struct EC_*) ec; + return ec; } @@ -3948,7 +3942,6 @@ static void hg_pre_clo_init ( void ) ///////////////////////////////////////////// hbthr_root = libhb_init( for_libhb__get_stacktrace, - for_libhb__stacktrace_to_EC, for_libhb__get_EC ); ///////////////////////////////////////////// diff --git a/helgrind/libhb.h b/helgrind/libhb.h index 6031332c3a..082150c43e 100644 --- a/helgrind/libhb.h +++ b/helgrind/libhb.h @@ -39,17 +39,12 @@ /* Abstract to user: synchronisation objects */ /* typedef struct _SO SO; */ /* now in hg_lock_n_thread.h */ -/* Abstract to the lib: execution contexts */ -/* struct _EC will be defined by user at some point. */ -typedef struct _EC EC; - /* Initialise library; returns Thr* for root thread. 'shadow_alloc' should never return NULL, instead it should simply not return if they encounter an out-of-memory condition. */ Thr* libhb_init ( void (*get_stacktrace)( Thr*, Addr*, UWord ), - struct _EC* (*stacktrace_to_EC)( Addr*, UWord ), - struct _EC* (*get_EC)( Thr* ) + ExeContext* (*get_EC)( Thr* ) ); /* Shut down the library, and print stats (in fact that's _all_ diff --git a/helgrind/libhb_core.c b/helgrind/libhb_core.c index 8903cf027d..82159ba39e 100644 --- a/helgrind/libhb_core.c +++ b/helgrind/libhb_core.c @@ -55,8 +55,7 @@ Globals needed by other parts of the library. These are set once at startup and then never changed. */ static void (*main_get_stacktrace)( Thr*, Addr*, UWord ) = NULL; -static struct _EC* (*main_stacktrace_to_EC)( Addr*, UWord ) = NULL; -static struct _EC* (*main_get_EC)( Thr* ) = NULL; +static ExeContext* (*main_get_EC)( Thr* ) = NULL; ///////////////////////////////////////////////////////////////// ///////////////////////////////////////////////////////////////// @@ -2343,16 +2342,16 @@ static void VtsID__invalidate_caches ( void ) { } ////////////////////////// -static Bool VtsID__is_valid ( VtsID vi ) { - VtsTE* ve; - if (vi >= (VtsID)VG_(sizeXA)( vts_tab )) - return False; - ve = VG_(indexXA)( vts_tab, vi ); - if (!ve->vts) - return False; - tl_assert(ve->vts->id == vi); - return True; -} +//static Bool VtsID__is_valid ( VtsID vi ) { +// VtsTE* ve; +// if (vi >= (VtsID)VG_(sizeXA)( vts_tab )) +// return False; +// ve = VG_(indexXA)( vts_tab, vi ); +// if (!ve->vts) +// return False; +// tl_assert(ve->vts->id == vi); +// return True; +//} static VTS* VtsID__to_VTS ( VtsID vi ) { VtsTE* te = VG_(indexXA)( vts_tab, vi ); @@ -2790,14 +2789,6 @@ typedef } OldRef; -static Word OldRef__cmp_by_EA ( OldRef* r1, OldRef* r2 ) { - tl_assert(r1 && r1->magic == OldRef_MAGIC); - tl_assert(r2 && r2->magic == OldRef_MAGIC); - if (r1->ea < r2->ea) return -1; - if (r1->ea > r2->ea) return 1; - return 0; -} - static OSet* oldrefTree = NULL; /* OSet* of OldRef */ static UWord oldrefGen = 0; /* current LRU generation # */ static UWord oldrefTreeN = 0; /* # elems in oldrefTree */ @@ -2898,7 +2889,7 @@ static void event_map_bind ( Addr a, Thr* thr ) static -Bool event_map_lookup ( /*OUT*/struct _EC** resEC, +Bool event_map_lookup ( /*OUT*/ExeContext** resEC, /*OUT*/Thr** resThr, Thr* thr_acc, Addr a ) { @@ -2931,7 +2922,9 @@ Bool event_map_lookup ( /*OUT*/struct _EC** resEC, tl_assert(ref->accs[i].rcec); tl_assert(ref->accs[i].rcec->magic == RCEC_MAGIC); - *resEC = main_stacktrace_to_EC(&ref->accs[i].rcec->frames[1], N_FRAMES); + *resEC = VG_(make_ExeContext_from_StackTrace)( + &ref->accs[i].rcec->frames[1], N_FRAMES + ); *resThr = ref->accs[i].thr; return True; } else { @@ -3180,8 +3173,8 @@ static void record_race_info ( Thr* acc_thr, { Bool found; Thr* thrp = NULL; - struct _EC* where = NULL; - struct _EC* wherep = NULL; + ExeContext* where = NULL; + ExeContext* wherep = NULL; where = main_get_EC( acc_thr ); found = event_map_lookup( &wherep, &thrp, acc_thr, acc_addr ); if (found) { @@ -4179,17 +4172,14 @@ static void show_thread_state ( HChar* str, Thr* t ) Thr* libhb_init ( void (*get_stacktrace)( Thr*, Addr*, UWord ), - struct _EC* (*stacktrace_to_EC)( Addr*, UWord ), - struct _EC* (*get_EC)( Thr* ) + ExeContext* (*get_EC)( Thr* ) ) { Thr* thr; VtsID vi; tl_assert(get_stacktrace); - tl_assert(stacktrace_to_EC); tl_assert(get_EC); main_get_stacktrace = get_stacktrace; - main_stacktrace_to_EC = stacktrace_to_EC; main_get_EC = get_EC; // No need to initialise hg_wordfm.