In vg_memory.c, startup_segment_callback, fix initialisation ordering
problem which caused the leak checker to misbehave following recent
PLT-bypass workaround.
In short, it is an error to announce to the skin, segments found which
belong to the low-level memory manager, because the skin may then mark
them as accessible to the client. This is wrong, and the client
should only acquire accessible memory via malloc etc and stack
movement. Now we carefully avoid mentioning any segment belonging to
the low level memory manager.
Take the opportunity to improve VG_(within_m_state_static) so that it
also detects pointers within the thread table. This can reduce the
number of blocks the leak checker spuriously thinks are still
reachable.
git-svn-id: svn://svn.valgrind.org/valgrind/trunk@1751 a5019735-40e9-0310-863c-91ae7b9d1cf9
diff --git a/memcheck/mac_leakcheck.c b/memcheck/mac_leakcheck.c
index 42e9205..9ad2360 100644
--- a/memcheck/mac_leakcheck.c
+++ b/memcheck/mac_leakcheck.c
@@ -311,10 +311,10 @@
where the .bss segment has been put. If you can, drop me a
line.
*/
- if (VG_(within_stack)(a)) return;
- if (VG_(within_m_state_static)(a)) return;
- if (a == (Addr)(&lc_min_mallocd_addr)) return;
- if (a == (Addr)(&lc_max_mallocd_addr)) return;
+ if (VG_(within_stack)(a)) return;
+ if (VG_(within_m_state_static_OR_threads)(a)) return;
+ if (a == (Addr)(&lc_min_mallocd_addr)) return;
+ if (a == (Addr)(&lc_max_mallocd_addr)) return;
/* OK, let's get on and do something Useful for a change. */
diff --git a/memcheck/mac_malloc_wrappers.c b/memcheck/mac_malloc_wrappers.c
index 42434de..71757d9 100644
--- a/memcheck/mac_malloc_wrappers.c
+++ b/memcheck/mac_malloc_wrappers.c
@@ -45,10 +45,13 @@
UInt VG_(vg_malloc_redzone_szB) = 16;
/* Function pointers for the two skins to track interesting events. */
-void (*MAC_(new_mem_heap)) ( Addr a, UInt len, Bool is_inited );
-void (*MAC_(ban_mem_heap)) ( Addr a, UInt len );
-void (*MAC_(die_mem_heap)) ( Addr a, UInt len );
-void (*MAC_(copy_mem_heap))( Addr from, Addr to, UInt len );
+void (*MAC_(new_mem_heap)) ( Addr a, UInt len, Bool is_inited ) = NULL;
+void (*MAC_(ban_mem_heap)) ( Addr a, UInt len ) = NULL;
+void (*MAC_(die_mem_heap)) ( Addr a, UInt len ) = NULL;
+void (*MAC_(copy_mem_heap))( Addr from, Addr to, UInt len ) = NULL;
+
+/* Function pointers for internal sanity checking. */
+Bool (*MAC_(check_noaccess))( Addr a, UInt len, Addr* bad_addr ) = NULL;
/*------------------------------------------------------------*/
@@ -134,13 +137,14 @@
mc->allockind = kind;
mc->where = VG_(get_ExeContext)(tst);
- /* The following line puts the shadow chunk, and hence the pointer
- to the real chunk, off-limits to the client. This seems to be
- necessary to make the leak checker work reliably. Problem is,
- this seems to point to something deeper being wrong: this chunk
- is allocated in the AR_SKIN arena and so should by default be
- off-limits to the client anyway. */
- MAC_(ban_mem_heap)( (Addr)mc, sizeof(MAC_Chunk));
+ /* Paranoia ... ensure this area is off-limits to the client, so
+ the mc->data field isn't visible to the leak checker. If memory
+ management is working correctly, anything pointer returned by
+ VG_(malloc) should be noaccess as far as the client is
+ concerned. */
+ if (!MAC_(check_noaccess)( (Addr)mc, sizeof(MAC_Chunk), NULL )) {
+ VG_(skin_panic)("add_MAC_chunk: shadow area is accessible");
+ }
VG_(HT_add_node)( MAC_(malloc_list), (VgHashNode*)mc );
}
diff --git a/memcheck/mac_shared.h b/memcheck/mac_shared.h
index 93bedab..5cb9893 100644
--- a/memcheck/mac_shared.h
+++ b/memcheck/mac_shared.h
@@ -267,6 +267,9 @@
extern void (*MAC_(die_mem_heap)) ( Addr a, UInt len );
extern void (*MAC_(copy_mem_heap))( Addr from, Addr to, UInt len );
+/* Function pointers for internal sanity checking. */
+extern Bool (*MAC_(check_noaccess))( Addr a, UInt len, Addr* bad_addr );
+
/* Used in describe_addr() */
extern Bool (*MAC_(describe_addr_supp)) ( Addr a, AddrInfo* ai );
diff --git a/memcheck/mc_main.c b/memcheck/mc_main.c
index 167a2ba..28a9712 100644
--- a/memcheck/mc_main.c
+++ b/memcheck/mc_main.c
@@ -518,6 +518,27 @@
exist, *bad_addr is set to the offending address, so the caller can
know what it is. */
+/* Returns True if [a .. a+len) is not addressible. Otherwise,
+ returns False, and if bad_addr is non-NULL, sets *bad_addr to
+ indicate the lowest failing address. Functions below are
+ similar. */
+Bool MC_(check_noaccess) ( Addr a, UInt len, Addr* bad_addr )
+{
+ UInt i;
+ UChar abit;
+ PROF_EVENT(42);
+ for (i = 0; i < len; i++) {
+ PROF_EVENT(43);
+ abit = get_abit(a);
+ if (abit == VGM_BIT_VALID) {
+ if (bad_addr != NULL) *bad_addr = a;
+ return False;
+ }
+ a++;
+ }
+ return True;
+}
+
Bool MC_(check_writable) ( Addr a, UInt len, Addr* bad_addr )
{
UInt i;
@@ -1655,6 +1676,7 @@
MAC_( ban_mem_heap) = & MC_(make_noaccess);
MAC_(copy_mem_heap) = & mc_copy_address_range_state;
MAC_( die_mem_heap) = & MC_(make_noaccess);
+ MAC_(check_noaccess) = & MC_(check_noaccess);
VG_(track_new_mem_startup) ( & mc_new_mem_startup );
VG_(track_new_mem_stack_signal) ( & MC_(make_writable) );