If a race error is detected, check to see whether the raced-on address
is inside a heap block, and if so, print the allocation point of the
heap block. It's stupid not to do this considering that the
implementation already keeps track of all mallocs and frees.
git-svn-id: svn://svn.valgrind.org/valgrind/trunk@11089 a5019735-40e9-0310-863c-91ae7b9d1cf9
diff --git a/helgrind/hg_errors.c b/helgrind/hg_errors.c
index 981f4bf..a4d0bdb 100644
--- a/helgrind/hg_errors.c
+++ b/helgrind/hg_errors.c
@@ -178,8 +178,15 @@
Int szB;
Bool isWrite;
Thread* thr;
+ /* descr1/2 provide a description of stack/global locs */
XArray* descr1; /* XArray* of HChar */
XArray* descr2; /* XArray* of HChar */
+ /* halloc/haddr/hszB describe the addr if it is a heap block. */
+ ExeContext* hctxt;
+ Addr haddr;
+ SizeT hszB;
+ /* h1_* and h2_* provide some description of a previously
+ observed access with which we are conflicting. */
Thread* h1_ct; /* non-NULL means h1 info present */
ExeContext* h1_ct_mbsegstartEC;
ExeContext* h1_ct_mbsegendEC;
@@ -263,33 +270,47 @@
if (0)
VG_(printf)("HG_(update_extra): "
"%d conflicting-event queries\n", xxx);
+
+ tl_assert(!xe->XE.Race.hctxt);
tl_assert(!xe->XE.Race.descr1);
tl_assert(!xe->XE.Race.descr2);
- xe->XE.Race.descr1
- = VG_(newXA)( HG_(zalloc), "hg.update_extra.Race.descr1",
- HG_(free), sizeof(HChar) );
- xe->XE.Race.descr2
- = VG_(newXA)( HG_(zalloc), "hg.update_extra.Race.descr2",
- HG_(free), sizeof(HChar) );
+ /* First, see if it's in any heap block. Unfortunately this
+ means a linear search through all allocated heap blocks. */
+ HG_(mm_find_containing_block)(
+ &xe->XE.Race.hctxt, &xe->XE.Race.haddr, &xe->XE.Race.hszB,
+ xe->XE.Race.data_addr
+ );
- (void) VG_(get_data_description)( xe->XE.Race.descr1,
- xe->XE.Race.descr2,
- xe->XE.Race.data_addr );
+ if (!xe->XE.Race.hctxt) {
+ /* It's not in any heap block. See if we can map it to a
+ stack or global symbol. */
- /* If there's nothing in descr1/2, free it. Why is it safe to
- to VG_(indexXA) at zero here? Because
- VG_(get_data_description) guarantees to zero terminate
- descr1/2 regardless of the outcome of the call. So there's
- always at least one element in each XA after the call.
- */
- if (0 == VG_(strlen)( VG_(indexXA)( xe->XE.Race.descr1, 0 ))) {
- VG_(deleteXA)( xe->XE.Race.descr1 );
- xe->XE.Race.descr1 = NULL;
- }
- if (0 == VG_(strlen)( VG_(indexXA)( xe->XE.Race.descr2, 0 ))) {
- VG_(deleteXA)( xe->XE.Race.descr2 );
- xe->XE.Race.descr2 = NULL;
+ xe->XE.Race.descr1
+ = VG_(newXA)( HG_(zalloc), "hg.update_extra.Race.descr1",
+ HG_(free), sizeof(HChar) );
+ xe->XE.Race.descr2
+ = VG_(newXA)( HG_(zalloc), "hg.update_extra.Race.descr2",
+ HG_(free), sizeof(HChar) );
+
+ (void) VG_(get_data_description)( xe->XE.Race.descr1,
+ xe->XE.Race.descr2,
+ xe->XE.Race.data_addr );
+
+ /* If there's nothing in descr1/2, free it. Why is it safe to
+ to VG_(indexXA) at zero here? Because
+ VG_(get_data_description) guarantees to zero terminate
+ descr1/2 regardless of the outcome of the call. So there's
+ always at least one element in each XA after the call.
+ */
+ if (0 == VG_(strlen)( VG_(indexXA)( xe->XE.Race.descr1, 0 ))) {
+ VG_(deleteXA)( xe->XE.Race.descr1 );
+ xe->XE.Race.descr1 = NULL;
+ }
+ if (0 == VG_(strlen)( VG_(indexXA)( xe->XE.Race.descr2, 0 ))) {
+ VG_(deleteXA)( xe->XE.Race.descr2 );
+ xe->XE.Race.descr2 = NULL;
+ }
}
/* And poke around in the conflicting-event map, to see if we
@@ -993,6 +1014,23 @@
}
+ /* If we have a description of the address in terms of a heap
+ block, show it. */
+ if (xe->XE.Race.hctxt) {
+ SizeT delta = err_ga - xe->XE.Race.haddr;
+ if (xml) {
+ emit(" <auxwhat>Address %#lx is %ld bytes inside a block "
+ "of size %ld alloc'd</auxwhat>\n", err_ga, delta,
+ xe->XE.Race.hszB);
+ VG_(pp_ExeContext)( xe->XE.Race.hctxt );
+ } else {
+ emit(" Address %#lx is %ld bytes inside a block "
+ "of size %ld alloc'd\n", err_ga, delta,
+ xe->XE.Race.hszB);
+ VG_(pp_ExeContext)( xe->XE.Race.hctxt );
+ }
+ }
+
/* If we have a better description of the address, show it.
Note that in XML mode, it will already by nicely wrapped up
in tags, either <auxwhat> or <xauxwhat>, so we can just emit
diff --git a/helgrind/hg_errors.h b/helgrind/hg_errors.h
index ee49491..3594825 100644
--- a/helgrind/hg_errors.h
+++ b/helgrind/hg_errors.h
@@ -67,6 +67,14 @@
extern ULong HG_(stats__string_table_queries);
extern ULong HG_(stats__string_table_get_map_size) ( void );
+/* For error creation: map 'data_addr' to a malloc'd chunk, if any.
+ Slow linear search. This is an abuse of the normal file structure
+ since this is exported by hg_main.c, not hg_errors.c. Oh Well. */
+void HG_(mm_find_containing_block)( /*OUT*/ExeContext** where,
+ /*OUT*/Addr* payload,
+ /*OUT*/SizeT* szB,
+ Addr data_addr );
+
#endif /* ! __HG_ERRORS_H */
/*--------------------------------------------------------------------*/
diff --git a/helgrind/hg_main.c b/helgrind/hg_main.c
index 19dd1f9..2f4e13b 100644
--- a/helgrind/hg_main.c
+++ b/helgrind/hg_main.c
@@ -3868,6 +3868,36 @@
}
+/* For error creation: map 'data_addr' to a malloc'd chunk, if any.
+ Slow linear search. */
+
+static inline Bool addr_is_in_MM_Chunk( MallocMeta* mm, Addr a )
+{
+ if (a < mm->payload) return False;
+ if (a >= mm->payload + mm->szB) return False;
+ return True;
+}
+
+void HG_(mm_find_containing_block)( /*OUT*/ExeContext** where,
+ /*OUT*/Addr* payload,
+ /*OUT*/SizeT* szB,
+ Addr data_addr )
+{
+ MallocMeta* mm;
+ /* Well, this totally sucks. But without using an interval tree or
+ some such, it's hard to see how to do better. */
+ VG_(HT_ResetIter)(hg_mallocmeta_table);
+ while ( (mm = VG_(HT_Next)(hg_mallocmeta_table)) ) {
+ if (UNLIKELY(addr_is_in_MM_Chunk(mm, data_addr))) {
+ *where = mm->where;
+ *payload = mm->payload;
+ *szB = mm->szB;
+ return;
+ }
+ }
+}
+
+
/*--------------------------------------------------------------*/
/*--- Instrumentation ---*/
/*--------------------------------------------------------------*/