|
From: <sv...@va...> - 2007-02-21 10:51:44
|
Author: njn
Date: 2007-02-21 10:51:41 +0000 (Wed, 21 Feb 2007)
New Revision: 6607
Log:
Improved stats gathering about how many origins might be identified.
Added an extra number to "origin unknown" origin which gives some info (to
the educated reader) about why no origin was found. This breaks the
origin-* reg tests.
Added some notes about ideas to improve the frequency of correct
identification.
A few other things.
Modified:
branches/ORIGIN_TRACKING/coregrind/m_execontext.c
branches/ORIGIN_TRACKING/coregrind/m_tooliface.c
branches/ORIGIN_TRACKING/coregrind/pub_core_tooliface.h
branches/ORIGIN_TRACKING/helgrind/hg_main.c
branches/ORIGIN_TRACKING/include/pub_tool_execontext.h
branches/ORIGIN_TRACKING/memcheck/mc_main.c
branches/ORIGIN_TRACKING/memcheck/mc_translate.c
Modified: branches/ORIGIN_TRACKING/coregrind/m_execontext.c
===================================================================
--- branches/ORIGIN_TRACKING/coregrind/m_execontext.c 2007-02-20 19:23:19 UTC (rev 6606)
+++ branches/ORIGIN_TRACKING/coregrind/m_execontext.c 2007-02-21 10:51:41 UTC (rev 6607)
@@ -325,7 +325,7 @@
// iterator, or a first-match predicate? But don't want to slow this down
// too much? Maybe it doesn't matter, since it's only called on non-dup
// errors.
-ExeContext* is_an_ExeContext(UInt maybe_ec_low32)
+ExeContext* VG_(is_an_ExeContext)(UInt maybe_ec_low32)
{
Int i;
ExeContext* curr;
Modified: branches/ORIGIN_TRACKING/coregrind/m_tooliface.c
===================================================================
--- branches/ORIGIN_TRACKING/coregrind/m_tooliface.c 2007-02-20 19:23:19 UTC (rev 6606)
+++ branches/ORIGIN_TRACKING/coregrind/m_tooliface.c 2007-02-21 10:51:41 UTC (rev 6607)
@@ -270,8 +270,8 @@
VG_(tdict).fn = f; \
}
-#define DEF2(fn, args...) \
-void VG_(fn)(VG_REGPARM(1) void(*f)(args)) \
+#define DEF2(regparm, fn, args...) \
+void VG_(fn)(VG_REGPARM(regparm) void(*f)(args)) \
{ \
VG_(tdict).fn = f; \
}
@@ -287,27 +287,27 @@
DEF(track_die_mem_brk, Addr, SizeT)
DEF(track_die_mem_munmap, Addr, SizeT)
-DEF2(track_new_mem_stack_4, Addr)
-DEF2(track_new_mem_stack_8, Addr)
-DEF2(track_new_mem_stack_12, Addr)
-DEF2(track_new_mem_stack_16, Addr)
-DEF2(track_new_mem_stack_32, Addr)
+DEF2(1, track_new_mem_stack_4, Addr)
+DEF2(1, track_new_mem_stack_8, Addr)
+DEF2(1, track_new_mem_stack_12, Addr)
+DEF2(1, track_new_mem_stack_16, Addr)
+DEF2(1, track_new_mem_stack_32, Addr)
//DEF2(track_new_mem_stack_112, Addr)
//DEF2(track_new_mem_stack_128, Addr)
//DEF2(track_new_mem_stack_144, Addr)
//DEF2(track_new_mem_stack_160, Addr)
-DEF (track_new_mem_stack, Addr, SizeT)
+DEF2(2, track_new_mem_stack, Addr, SizeT)
-DEF2(track_die_mem_stack_4, Addr)
-DEF2(track_die_mem_stack_8, Addr)
-DEF2(track_die_mem_stack_12, Addr)
-DEF2(track_die_mem_stack_16, Addr)
-DEF2(track_die_mem_stack_32, Addr)
+DEF2(1, track_die_mem_stack_4, Addr)
+DEF2(1, track_die_mem_stack_8, Addr)
+DEF2(1, track_die_mem_stack_12, Addr)
+DEF2(1, track_die_mem_stack_16, Addr)
+DEF2(1, track_die_mem_stack_32, Addr)
//DEF2(track_die_mem_stack_112, Addr)
//DEF2(track_die_mem_stack_128, Addr)
//DEF2(track_die_mem_stack_144, Addr)
//DEF2(track_die_mem_stack_160, Addr)
-DEF (track_die_mem_stack, Addr, SizeT)
+DEF2(2, track_die_mem_stack, Addr, SizeT)
DEF(track_ban_mem_stack, Addr, SizeT)
Modified: branches/ORIGIN_TRACKING/coregrind/pub_core_tooliface.h
===================================================================
--- branches/ORIGIN_TRACKING/coregrind/pub_core_tooliface.h 2007-02-20 19:23:19 UTC (rev 6606)
+++ branches/ORIGIN_TRACKING/coregrind/pub_core_tooliface.h 2007-02-21 10:51:41 UTC (rev 6607)
@@ -177,7 +177,7 @@
// void VG_REGPARM(1) (*track_new_mem_stack_128)(Addr);
// void VG_REGPARM(1) (*track_new_mem_stack_144)(Addr);
// void VG_REGPARM(1) (*track_new_mem_stack_160)(Addr);
- void (*track_new_mem_stack)(Addr, SizeT);
+ void VG_REGPARM(2) (*track_new_mem_stack)(Addr, SizeT);
void VG_REGPARM(1) (*track_die_mem_stack_4) (Addr);
void VG_REGPARM(1) (*track_die_mem_stack_8) (Addr);
@@ -188,7 +188,7 @@
// void VG_REGPARM(1) (*track_die_mem_stack_128)(Addr);
// void VG_REGPARM(1) (*track_die_mem_stack_144)(Addr);
// void VG_REGPARM(1) (*track_die_mem_stack_160)(Addr);
- void (*track_die_mem_stack)(Addr, SizeT);
+ void VG_REGPARM(2) (*track_die_mem_stack)(Addr, SizeT);
void (*track_ban_mem_stack)(Addr, SizeT);
Modified: branches/ORIGIN_TRACKING/helgrind/hg_main.c
===================================================================
--- branches/ORIGIN_TRACKING/helgrind/hg_main.c 2007-02-20 19:23:19 UTC (rev 6606)
+++ branches/ORIGIN_TRACKING/helgrind/hg_main.c 2007-02-21 10:51:41 UTC (rev 6607)
@@ -3348,8 +3348,9 @@
else
stack_tracker = & hg_new_mem_stack;
- VG_(track_new_mem_stack) (stack_tracker);
- VG_(track_new_mem_stack_signal) (stack_tracker);
+// XXX: commented out because the prototypes changed...
+// VG_(track_new_mem_stack) (stack_tracker);
+// VG_(track_new_mem_stack_signal) (stack_tracker);
}
Modified: branches/ORIGIN_TRACKING/include/pub_tool_execontext.h
===================================================================
--- branches/ORIGIN_TRACKING/include/pub_tool_execontext.h 2007-02-20 19:23:19 UTC (rev 6606)
+++ branches/ORIGIN_TRACKING/include/pub_tool_execontext.h 2007-02-21 10:51:41 UTC (rev 6607)
@@ -60,7 +60,7 @@
// Returns true if the given pointer is a real ExeContext pointer. Does
// a linear (slow) lookup by value.
-extern ExeContext* is_an_ExeContext(UInt maybe_ec_low32);
+extern ExeContext* VG_(is_an_ExeContext)(UInt maybe_ec_low32);
// Apply a function to every element in the ExeContext. The parameter 'n'
// gives the index of the passed ip. Doesn't go below main() unless
Modified: branches/ORIGIN_TRACKING/memcheck/mc_main.c
===================================================================
--- branches/ORIGIN_TRACKING/memcheck/mc_main.c 2007-02-20 19:23:19 UTC (rev 6606)
+++ branches/ORIGIN_TRACKING/memcheck/mc_main.c 2007-02-21 10:51:41 UTC (rev 6607)
@@ -30,6 +30,22 @@
The GNU General Public License is contained in the file COPYING.
*/
+// XXX: improving things:
+// - chase backwards through put/get pairs. Will allow FP ones to be
+// recognised.
+// - For 1B and 2B cases, maybe try to match the 1B or 2B value against the
+// ExeContext. Likely to have more possible matches, but that's ok,
+// better to give several possibilities, one of which is correct, than
+// nothing. Could even extend this more generally, and make the
+// origin-values word-sized instead of 32B?
+// - Could do similar things to handle misalignment -- try rotating the
+// values.
+// - Could also do similar things to handle some modifications -- eg. a
+// change may well only affect the bottom few bits of a 32-bit value.
+// - With all these changes, seems like we'd want to report some kind of
+// confidence level on the matches. Eg. a full match is much more likely
+// to be right than a partial one.
+
#include "pub_tool_basics.h"
#include "pub_tool_aspacemgr.h"
#include "pub_tool_hashtable.h" // For mc_include.h
@@ -2813,7 +2829,8 @@
VG_(pp_ExeContext)( VG_(get_error_where)(err) );
}
-static void mc_pp_origins ( ExeContext* origins[], Int n_origins )
+static void mc_pp_origins ( ExeContext* origins[], Int n_origins,
+ Int n_obfusc_origins_low32 )
{
// XXX: in origin-yes, we get two origins for the 64-bit stack case --
// should remove dup'd origins from the list.
@@ -2833,8 +2850,8 @@
}
} else {
VG_(message)(Vg_UserMsg,
- "%sUninitialised value has unknown origin%s",
- xpre, xpost);
+ "%sUninitialised value has unknown origin (%d)%s",
+ xpre, n_obfusc_origins_low32, xpost);
}
}
@@ -2863,7 +2880,9 @@
"Use of uninitialised value of size %d",
extra->Err.Value.szB);
}
- mc_pp_origins(extra->Err.Value.origins, extra->Err.Value.n_origins);
+ mc_pp_origins(extra->Err.Value.origins,
+ extra->Err.Value.n_origins,
+ extra->Err.Value.n_obfusc_origins_low32);
break;
case Err_RegParam:
@@ -3367,7 +3386,7 @@
for (i = 0; i < extra->Err.Value.n_obfusc_origins_low32; i++) {
UInt origin_low32 = MC_(deobfuscate_ExeContext_low32)(
extra->Err.Value.obfusc_origins_low32[i]);
- ExeContext* origin = is_an_ExeContext(origin_low32);
+ ExeContext* origin = VG_(is_an_ExeContext)(origin_low32);
if (NULL != origin) {
origins_tmp[ n_origins++ ] = origin;
}
@@ -3384,7 +3403,10 @@
// Zero the "obfusc" fields (obfusc_origins_low32 is stack-allocated
// so we don't need to free it). And set the "origins" fields,
// copying the origins from stack memory into new heap memory.
- extra->Err.Value.n_obfusc_origins_low32 = 0;
+
+// XXX: don't zero n_obfusc_origins_low32 yet -- it can tell us why an origin
+// failed to be identified.
+// extra->Err.Value.n_obfusc_origins_low32 = 0;
extra->Err.Value.obfusc_origins_low32 = NULL;
extra->Err.Value.n_origins = n_origins;
extra->Err.Value.origins = origins;
@@ -4101,6 +4123,8 @@
mc_record_value_error ( VG_(get_running_tid)(), (Int)szB, origins, n_origins );
}
+// XXX: move all these into mc_translate, make mc_record_value_error public.
+
// Here we specialise the common cases (0, 1 and 2 origins with szB of 0 and
// VG_WORDSIZE) to reduce the number of args passed. Remaining cases are
// done with the more general functions.
Modified: branches/ORIGIN_TRACKING/memcheck/mc_translate.c
===================================================================
--- branches/ORIGIN_TRACKING/memcheck/mc_translate.c 2007-02-20 19:23:19 UTC (rev 6606)
+++ branches/ORIGIN_TRACKING/memcheck/mc_translate.c 2007-02-21 10:51:41 UTC (rev 6607)
@@ -812,30 +812,35 @@
/*--- Undefined value origin-tracking ---*/
/*------------------------------------------------------------*/
-static UInt origin_tracking_0 = 0,
- origin_tracking_1 = 0,
- origin_tracking_2 = 0;
+// Buckets: 0..6 and 7+
+static UInt n_found_origins[8];
void MC_(print_origin_tracking_stats)(void)
{
- Char perc0[7], perc1[7], perc2[7];
- UInt n_total = origin_tracking_2 + origin_tracking_1 + origin_tracking_0;
+// Char perc0[7], perc1[7], perc2[7];
+// UInt n_total = origin_tracking_2 + origin_tracking_1 + origin_tracking_0;
+//
+// // XXX: there's an off-by-one error in percentify -- if I use 6 instead
+// // of 5 here, the buffers get overrun. (there's one in snprintf, too)
+// // [XXX: the snprintf one has been fixed, I think]
+// VG_(percentify)(origin_tracking_2, n_total, 1, 7, perc2);
+// VG_(percentify)(origin_tracking_1, n_total, 1, 7, perc1);
+// VG_(percentify)(origin_tracking_0, n_total, 1, 7, perc0);
- // XXX: there's an off-by-one error in percentify -- if I use 6 instead
- // of 5 here, the buffers get overrun. (there's one in snprintf, too)
- // [XXX: the snprintf one has been fixed, I think]
- VG_(percentify)(origin_tracking_2, n_total, 1, 7, perc2);
- VG_(percentify)(origin_tracking_1, n_total, 1, 7, perc1);
- VG_(percentify)(origin_tracking_0, n_total, 1, 7, perc0);
- VG_(message)(Vg_DebugMsg,
- " memcheck: origins: static cmps with 2 args = %6d (%s)",
- origin_tracking_2, perc2);
- VG_(message)(Vg_DebugMsg,
- " memcheck: origins: static cmps with 1 arg = %6d (%s)",
- origin_tracking_1, perc1);
- VG_(message)(Vg_DebugMsg,
- " memcheck: origins: static cmps with 0 args = %6d (%s)",
- origin_tracking_0, perc0);
+ Char perc[7];
+ UInt n_total;
+ Int i;
+
+ n_total = 0;
+ for (i = 0; i <= 7; i++) {
+ n_total += n_found_origins[i];
+ }
+ for (i = 0; i <= 7; i++) {
+ VG_(percentify)(n_found_origins[i], n_total, 1, 6, perc);
+ VG_(message)(Vg_DebugMsg,
+ " memcheck: origins: found %d%s origins = %8u (%6s)",
+ i, ( i == 7 ? "+" : " " ), n_found_origins[i], perc);
+ }
}
/*------------------------------------------------------------*/
@@ -995,6 +1000,11 @@
case Iex_Load: {
// tX = GET:I<ty>(N), or tX = LDxx:I<ty>(tY)
// If tX is 32 or 64 bits, add it to the finish_list.
+ //
+ // Otherwise (or sometimes even if -- giving longer
+ // chains back?), should look for a put to the same
+ // register, and trace back through it.
+ // That will allow the float cases to work.
Int szB =
sizeofIRType(typeOfIRTemp(mce->bb_in->tyenv, tX));
if (4 == szB || 8 == szB) {
@@ -1005,6 +1015,12 @@
case Iex_GetI:
// XXX: should treat similarly to Get/Load
+ //
+ // XXX: a temp used as the ix should arguably be
+ // ignored!
+ // XXX: can also ignore the temp assigned by a GETI?
+ // can certainly ignore it if it's an I8 (ie. an
+ // FPTAG)
break;
case Iex_CCall: {
@@ -1227,6 +1243,11 @@
szHWordExpr = mkIRExpr_HWord( sz );
+ if (n_origins_found >= 7)
+ n_found_origins[7]++;
+ else
+ n_found_origins[n_origins_found]++;
+
if (0 == n_origins_found) {
if (VG_WORDSIZE == sz) {
// Specialise the word-sized case
|