|
From: <sv...@va...> - 2005-08-15 01:52:05
|
Author: njn
Date: 2005-08-15 02:52:02 +0100 (Mon, 15 Aug 2005)
New Revision: 4415
Log:
Something I realised recently: in C, iterators are much better than
higher-order functions for traversing data structures. The higher-order
approach is too clumsy due to the lack of polymorphism and closures; you
have to use void* too much and it is more verbose than it should be.
Hence, I replaced all the uses of HT_first_match() and
HT_apply_to_all_nodes() with equivalent uses of the hashtable iterator.
Also replaced higher-order traversal functions for Memcheck's freed-list
and the thread stacks with iterators. That last change changes the
core/tool interface, so I've increased the version number.
Modified:
trunk/coregrind/m_machine.c
trunk/include/pub_tool_machine.h
trunk/include/pub_tool_tooliface.h
trunk/massif/ms_main.c
trunk/memcheck/mac_malloc_wrappers.c
trunk/memcheck/mac_shared.c
trunk/memcheck/mac_shared.h
trunk/memcheck/mc_main.c
Modified: trunk/coregrind/m_machine.c
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
--- trunk/coregrind/m_machine.c 2005-08-14 23:52:32 UTC (rev 4414)
+++ trunk/coregrind/m_machine.c 2005-08-15 01:52:02 UTC (rev 4415)
@@ -178,25 +178,28 @@
}
}
=20
-// Try and identify a thread whose stack satisfies the predicate p, or
-// return VG_INVALID_THREADID if none do.
-ThreadId VG_(first_matching_thread_stack)
- ( Bool (*p) ( Addr stack_min, Addr stack_max, void* d ),
- void* d )
+static ThreadId thread_stack_iter =3D VG_INVALID_THREADID;
+
+void VG_(thread_stack_reset_iter)(void)
{
- ThreadId tid;
+ thread_stack_iter =3D 1;
+}
=20
- for (tid =3D 1; tid < VG_N_THREADS; tid++) {
- if (VG_(threads)[tid].status =3D=3D VgTs_Empty) continue;
-
- if ( p ( VG_(get_SP)(tid),
- VG_(threads)[tid].client_stack_highest_word, d ) )
- return tid;
+Bool VG_(thread_stack_next)(ThreadId* tid, Addr* stack_min, Addr* stack_=
max)
+{
+ ThreadId i;
+ for (i =3D thread_stack_iter; i < VG_N_THREADS; i++) {
+ if (VG_(threads)[i].status !=3D VgTs_Empty) {
+ *tid =3D i;
+ *stack_min =3D VG_(get_SP)(i);
+ *stack_max =3D VG_(threads)[i].client_stack_highest_word;
+ thread_stack_iter =3D i + 1;
+ return True;
+ }
}
- return VG_INVALID_THREADID;
+ return False;
}
=20
-
//////////////////////////////////////////////////////////////////
// Architecture specifics
=20
Modified: trunk/include/pub_tool_machine.h
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
--- trunk/include/pub_tool_machine.h 2005-08-14 23:52:32 UTC (rev 4414)
+++ trunk/include/pub_tool_machine.h 2005-08-15 01:52:02 UTC (rev 4415)
@@ -70,11 +70,11 @@
// doing leak checking.
extern void VG_(apply_to_GP_regs)(void (*f)(UWord val));
=20
-// Searches through all thread stacks to see if any match. Returns
-// VG_INVALID_THREADID if none match.
-extern ThreadId VG_(first_matching_thread_stack)
- ( Bool (*p) ( Addr stack_min, Addr stack_max, vo=
id* d ),
- void* d );
+// This iterator lets you inspect each live thread's stack bounds. The
+// params are all 'out' params. Returns False at the end.
+extern void VG_(thread_stack_reset_iter) ( void );
+extern Bool VG_(thread_stack_next) ( ThreadId* tid, Addr* stack_mi=
n,
+ Addr* stack_ma=
x );
=20
#endif // __PUB_TOOL_MACHINE_H
=20
Modified: trunk/include/pub_tool_tooliface.h
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
--- trunk/include/pub_tool_tooliface.h 2005-08-14 23:52:32 UTC (rev 4414)
+++ trunk/include/pub_tool_tooliface.h 2005-08-15 01:52:02 UTC (rev 4415)
@@ -40,7 +40,7 @@
/* The version number indicates binary-incompatible changes to the
interface; if the core and tool versions don't match, Valgrind
will abort. */
-#define VG_CORE_INTERFACE_VERSION 8
+#define VG_CORE_INTERFACE_VERSION 9
=20
typedef struct _ToolInfo {
Int sizeof_ToolInfo;
Modified: trunk/massif/ms_main.c
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
--- trunk/massif/ms_main.c 2005-08-14 23:52:32 UTC (rev 4414)
+++ trunk/massif/ms_main.c 2005-08-15 01:52:02 UTC (rev 4415)
@@ -835,13 +835,6 @@
static Census censi[MAX_N_CENSI];
static UInt curr_census =3D 0;
=20
-// Must return False so that all stacks are traversed
-static Bool count_stack_size( Addr stack_min, Addr stack_max, void *cp )
-{
- *(UInt *)cp +=3D (stack_max - stack_min);
- return False;
-}
-
static UInt get_xtree_size(XPt* xpt, UInt ix)
{
UInt i;
@@ -1065,9 +1058,13 @@
=20
// Stack(s) ---------------------------------------------------------
if (clo_stacks) {
+ ThreadId tid;
+ Addr stack_min, stack_max;
census->stacks_space =3D sigstacks_space;
- // slightly abusing this function
- VG_(first_matching_thread_stack)( count_stack_size, &census->stack=
s_space );
+ VG_(thread_stack_reset_iter)();
+ while ( VG_(thread_stack_next)(&tid, &stack_min, &stack_max) ) {
+ census->stacks_space +=3D (stack_max - stack_min);
+ }
}
=20
// Finish, update interval if necessary -----------------------------
Modified: trunk/memcheck/mac_malloc_wrappers.c
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
--- trunk/memcheck/mac_malloc_wrappers.c 2005-08-14 23:52:32 UTC (rev 441=
4)
+++ trunk/memcheck/mac_malloc_wrappers.c 2005-08-15 01:52:02 UTC (rev 441=
5)
@@ -124,19 +124,9 @@
}
}
=20
-/* Return the first shadow chunk satisfying the predicate p. */
-MAC_Chunk* MAC_(first_matching_freed_MAC_Chunk) ( Bool (*p)(MAC_Chunk*, =
void*),
- void* d )
+MAC_Chunk* MAC_(get_freed_list_head)(void)
{
- MAC_Chunk* mc;
-
- /* No point looking through freed blocks if we're not keeping
- them around for a while... */
- for (mc =3D freed_list_start; mc !=3D NULL; mc =3D mc->next)
- if (p(mc, d))
- return mc;
-
- return NULL;
+ return freed_list_start;
}
=20
/* Allocate its shadow chunk, put it on the appropriate list. */
@@ -447,20 +437,9 @@
=20
}
=20
-static void destroy_mempool_nuke_chunk(VgHashNode *node, void *d)
-{
- MAC_Chunk *mc =3D (MAC_Chunk *)node;
- MAC_Mempool *mp =3D (MAC_Mempool *)d;
- =20
- /* Note: ban redzones again -- just in case user de-banned them
- with a client request... */
- MAC_(ban_mem_heap)(mc->data-mp->rzB, mp->rzB );
- MAC_(die_mem_heap)(mc->data, mc->size );
- MAC_(ban_mem_heap)(mc->data+mc->size, mp->rzB );
-}
-
void MAC_(destroy_mempool)(Addr pool)
{
+ MAC_Chunk* mc;
MAC_Mempool* mp;
=20
mp =3D VG_(HT_remove) ( MAC_(mempool_list), (UWord)pool );
@@ -471,7 +450,16 @@
return;
}
=20
- VG_(HT_apply_to_all_nodes)(mp->chunks, destroy_mempool_nuke_chunk, mp=
);
+ // Clean up the chunks, one by one
+ VG_(HT_ResetIter)(mp->chunks);
+ while ( (mc =3D VG_(HT_Next)(mp->chunks)) ) {
+ /* Note: ban redzones again -- just in case user de-banned them
+ with a client request... */
+ MAC_(ban_mem_heap)(mc->data-mp->rzB, mp->rzB );
+ MAC_(die_mem_heap)(mc->data, mc->size );
+ MAC_(ban_mem_heap)(mc->data+mc->size, mp->rzB );
+ }
+ // Destroy the chunk table
VG_(HT_destruct)(mp->chunks);
=20
VG_(free)(mp);
@@ -517,28 +505,11 @@
/*--- Statistics printing ---*/
/*------------------------------------------------------------*/
=20
-typedef
- struct {
- UInt nblocks;
- SizeT nbytes;
- }
- MallocStats;
-
-static void malloc_stats_count_chunk(VgHashNode* node, void* d)=20
-{
- MAC_Chunk* mc =3D (MAC_Chunk*)node;
- MallocStats *ms =3D (MallocStats *)d;
-
- ms->nblocks ++;
- ms->nbytes +=3D mc->size;
-}
-
void MAC_(print_malloc_stats) ( void )
{
- MallocStats ms;
- =20
- ms.nblocks =3D 0;
- ms.nbytes =3D 0;
+ MAC_Chunk* mc;
+ UInt nblocks =3D 0;
+ SizeT nbytes =3D 0;
=20
if (VG_(clo_verbosity) =3D=3D 0)
return;
@@ -546,11 +517,15 @@
return;
=20
/* Count memory still in use. */
- VG_(HT_apply_to_all_nodes)(MAC_(malloc_list), malloc_stats_count_chun=
k, &ms);
+ VG_(HT_ResetIter)(MAC_(malloc_list));
+ while ( (mc =3D VG_(HT_Next)(MAC_(malloc_list))) ) {
+ nblocks++;
+ nbytes +=3D mc->size;
+ }
=20
VG_(message)(Vg_UserMsg,=20
"malloc/free: in use at exit: %d bytes in %d blocks.",
- ms.nbytes, ms.nblocks);
+ nbytes, nblocks);
VG_(message)(Vg_UserMsg,=20
"malloc/free: %d allocs, %d frees, %u bytes allocated.",
cmalloc_n_mallocs,
Modified: trunk/memcheck/mac_shared.c
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
--- trunk/memcheck/mac_shared.c 2005-08-14 23:52:32 UTC (rev 4414)
+++ trunk/memcheck/mac_shared.c 2005-08-15 01:52:02 UTC (rev 4415)
@@ -410,35 +410,20 @@
MemCheck for user blocks, which Addrcheck doesn't support. */
Bool (*MAC_(describe_addr_supp)) ( Addr a, AddrInfo* ai ) =3D NULL;
=20
-/* Callback for searching thread stacks */
-static Bool addr_is_in_bounds(Addr stack_min, Addr stack_max, void *ap)
+/* Function used when searching MAC_Chunk lists */
+static Bool addr_is_in_MAC_Chunk(MAC_Chunk* mc, Addr a)
{
- Addr a =3D *(Addr *)ap;
- =20
- return (stack_min <=3D a && a <=3D stack_max);
-}
-
-/* Callback for searching free'd list */
-static Bool addr_is_in_MAC_Chunk(MAC_Chunk* mc, void *ap)
-{
- Addr a =3D *(Addr *)ap;
- =20
return VG_(addr_is_in_block)( a, mc->data, mc->size,
MAC_MALLOC_REDZONE_SZB );
}
=20
-/* Callback for searching malloc'd lists */
-static Bool addr_is_in_HashNode(VgHashNode* sh_ch, void *ap)
-{
- return addr_is_in_MAC_Chunk( (MAC_Chunk*)sh_ch, ap );
-}
-
/* Describe an address as best you can, for error messages,
putting the result in ai. */
static void describe_addr ( Addr a, AddrInfo* ai )
{
MAC_Chunk* mc;
ThreadId tid;
+ Addr stack_min, stack_max;
=20
/* Perhaps it's a user-def'd block ? (only check if requested, thoug=
h) */
if (NULL !=3D MAC_(describe_addr_supp)) {
@@ -446,29 +431,36 @@
return;
}
/* Perhaps it's on a thread's stack? */
- tid =3D VG_(first_matching_thread_stack)(addr_is_in_bounds, &a);
- if (tid !=3D VG_INVALID_THREADID) {
- ai->akind =3D Stack;
- ai->stack_tid =3D tid;
- return;
+ VG_(thread_stack_reset_iter)();
+ while ( VG_(thread_stack_next)(&tid, &stack_min, &stack_max) ) {
+ if (stack_min <=3D a && a <=3D stack_max) {
+ ai->akind =3D Stack;
+ ai->stack_tid =3D tid;
+ return;
+ }
}
/* Search for a recently freed block which might bracket it. */
- mc =3D MAC_(first_matching_freed_MAC_Chunk)(addr_is_in_MAC_Chunk, &a)=
;
- if (NULL !=3D mc) {
- ai->akind =3D Freed;
- ai->blksize =3D mc->size;
- ai->rwoffset =3D (Int)a - (Int)mc->data;
- ai->lastchange =3D mc->where;
- return;
+ mc =3D MAC_(get_freed_list_head)();
+ while (mc) {
+ if (addr_is_in_MAC_Chunk(mc, a)) {
+ ai->akind =3D Freed;
+ ai->blksize =3D mc->size;
+ ai->rwoffset =3D (Int)a - (Int)mc->data;
+ ai->lastchange =3D mc->where;
+ return;
+ }
+ mc =3D mc->next;=20
}
/* Search for a currently malloc'd block which might bracket it. */
- mc =3D (MAC_Chunk*)VG_(HT_first_match)(MAC_(malloc_list), addr_is_in_=
HashNode, &a);
- if (NULL !=3D mc) {
- ai->akind =3D Mallocd;
- ai->blksize =3D mc->size;
- ai->rwoffset =3D (Int)(a) - (Int)mc->data;
- ai->lastchange =3D mc->where;
- return;
+ VG_(HT_ResetIter)(MAC_(malloc_list));
+ while ( (mc =3D VG_(HT_Next)(MAC_(malloc_list))) ) {
+ if (addr_is_in_MAC_Chunk(mc, a)) {
+ ai->akind =3D Mallocd;
+ ai->blksize =3D mc->size;
+ ai->rwoffset =3D (Int)(a) - (Int)mc->data;
+ ai->lastchange =3D mc->where;
+ return;
+ }
}
/* Clueless ... */
ai->akind =3D Unknown;
Modified: trunk/memcheck/mac_shared.h
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
--- trunk/memcheck/mac_shared.h 2005-08-14 23:52:32 UTC (rev 4414)
+++ trunk/memcheck/mac_shared.h 2005-08-15 01:52:02 UTC (rev 4415)
@@ -402,7 +402,7 @@
=20
extern void MAC_(pp_shared_Error) ( Error* err);
=20
-extern MAC_Chunk* MAC_(first_matching_freed_MAC_Chunk)( Bool (*p)(MAC_Ch=
unk*, void*), void* d );
+extern MAC_Chunk* MAC_(get_freed_list_head)( void );
=20
extern void MAC_(common_pre_clo_init) ( void );
extern void MAC_(common_fini) ( void (*leak_check)(ThreadId tid,
Modified: trunk/memcheck/mc_main.c
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
--- trunk/memcheck/mc_main.c 2005-08-14 23:52:32 UTC (rev 4414)
+++ trunk/memcheck/mc_main.c 2005-08-15 01:52:02 UTC (rev 4415)
@@ -2308,14 +2308,6 @@
);
}
=20
-static Bool find_addr(VgHashNode* sh_ch, void* ap)
-{
- MAC_Chunk *m =3D (MAC_Chunk*)sh_ch;
- Addr a =3D *(Addr*)ap;
-
- return VG_(addr_is_in_block)(a, m->data, m->size, MAC_MALLOC_REDZONE_S=
ZB);
-}
-
static Bool client_perm_maybe_describe( Addr a, AddrInfo* ai )
{
UInt i;
@@ -2330,30 +2322,32 @@
/* OK - maybe it's a mempool, too? */
MAC_Mempool* mp =3D VG_(HT_lookup)(MAC_(mempool_list),
(UWord)cgbs[i].start);
- if(mp !=3D NULL) {
- if(mp->chunks !=3D NULL) {
- MAC_Chunk *mc;
-
- mc =3D (MAC_Chunk*)VG_(HT_first_match)(mp->chunks, find_a=
ddr, &a);
- if(mc !=3D NULL) {
- ai->akind =3D UserG;
- ai->blksize =3D mc->size;
- ai->rwoffset =3D (Int)(a) - (Int)mc->data;
- ai->lastchange =3D mc->where;
- return True;
+ if (mp !=3D NULL) {
+ if (mp->chunks !=3D NULL) {
+ MAC_Chunk* mc;
+ VG_(OSet_ResetIter)(mp->chunks);
+ while ( (mc =3D VG_(OSet_Next)(mp->chunks)) ) {
+ if (VG_(addr_is_in_block)(a, mc->data, mc->size,
+ MAC_MALLOC_REDZONE_SZB)) {
+ ai->akind =3D UserG;
+ ai->blksize =3D mc->size;
+ ai->rwoffset =3D (Int)(a) - (Int)mc->data;
+ ai->lastchange =3D mc->where;
+ return True;
+ }
}
}
- ai->akind =3D Mempool;
- ai->blksize =3D cgbs[i].size;
- ai->rwoffset =3D (Int)(a) - (Int)(cgbs[i].start);
+ ai->akind =3D Mempool;
+ ai->blksize =3D cgbs[i].size;
+ ai->rwoffset =3D (Int)(a) - (Int)(cgbs[i].start);
ai->lastchange =3D cgbs[i].where;
return True;
}
- ai->akind =3D UserG;
- ai->blksize =3D cgbs[i].size;
- ai->rwoffset =3D (Int)(a) - (Int)(cgbs[i].start);
+ ai->akind =3D UserG;
+ ai->blksize =3D cgbs[i].size;
+ ai->rwoffset =3D (Int)(a) - (Int)(cgbs[i].start);
ai->lastchange =3D cgbs[i].where;
- ai->desc =3D cgbs[i].desc;
+ ai->desc =3D cgbs[i].desc;
return True;
}
}
|