|
From: Konstantin S. <kon...@gm...> - 2010-03-11 14:05:41
|
On Thu, Mar 11, 2010 at 4:43 PM, <sv...@va...> wrote:
> Author: sewardj
> Date: 2010-03-11 13:43:18 +0000 (Thu, 11 Mar 2010)
> New Revision: 11089
>
> Log:
> 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.
.. and considering that this functionality was in helgrind 3.3 :)
>
>
>
> Modified:
> trunk/helgrind/hg_errors.c
> trunk/helgrind/hg_errors.h
> trunk/helgrind/hg_main.c
>
>
> Modified: trunk/helgrind/hg_errors.c
> ===================================================================
> --- trunk/helgrind/hg_errors.c 2010-03-10 07:05:17 UTC (rev 11088)
> +++ trunk/helgrind/hg_errors.c 2010-03-11 13:43:18 UTC (rev 11089)
> @@ -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,34 +270,48 @@
> 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;
> + 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;
> + }
> }
> - 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
> can rustle up a plausible-looking conflicting memory access
> @@ -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
>
> Modified: trunk/helgrind/hg_errors.h
> ===================================================================
> --- trunk/helgrind/hg_errors.h 2010-03-10 07:05:17 UTC (rev 11088)
> +++ trunk/helgrind/hg_errors.h 2010-03-11 13:43:18 UTC (rev 11089)
> @@ -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 */
>
> /*--------------------------------------------------------------------*/
>
> Modified: trunk/helgrind/hg_main.c
> ===================================================================
> --- trunk/helgrind/hg_main.c 2010-03-10 07:05:17 UTC (rev 11088)
> +++ trunk/helgrind/hg_main.c 2010-03-11 13:43:18 UTC (rev 11089)
> @@ -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 ---*/
> /*--------------------------------------------------------------*/
>
>
> ------------------------------------------------------------------------------
> Download Intel® Parallel Studio Eval
> Try the new software tools for yourself. Speed compiling, find bugs
> proactively, and fine-tune applications for parallel performance.
> See why Intel Parallel Studio got high marks during beta.
> http://p.sf.net/sfu/intel-sw-dev
> _______________________________________________
> Valgrind-developers mailing list
> Val...@li...
> https://lists.sourceforge.net/lists/listinfo/valgrind-developers
>
|