|
From: <sv...@va...> - 2008-04-15 10:09:27
|
Author: sewardj
Date: 2008-04-15 11:09:21 +0100 (Tue, 15 Apr 2008)
New Revision: 7878
Log:
Add a new flag, --track-origins=no|yes [no], which enables origin
tracking. Since origin tracking is disabled by default, the default
behaviour of Memcheck is unchanged.
Modified:
branches/OTRACK_BY_INSTRUMENTATION/memcheck/mc_include.h
branches/OTRACK_BY_INSTRUMENTATION/memcheck/mc_main.c
branches/OTRACK_BY_INSTRUMENTATION/memcheck/mc_translate.c
Modified: branches/OTRACK_BY_INSTRUMENTATION/memcheck/mc_include.h
===================================================================
--- branches/OTRACK_BY_INSTRUMENTATION/memcheck/mc_include.h 2008-04-15 10:07:16 UTC (rev 7877)
+++ branches/OTRACK_BY_INSTRUMENTATION/memcheck/mc_include.h 2008-04-15 10:09:21 UTC (rev 7878)
@@ -279,18 +279,6 @@
* default: NO */
extern Bool MC_(clo_workaround_gcc296_bugs);
-/* Do undefined value checking? "No" gives Addrcheck-style behaviour, ie.
- * faster but fewer errors found. Note that although Addrcheck had 1 bit
- * per byte overhead vs the old Memcheck's 9 bits per byte, with this mode
- * and compressed V bits, no memory is saved with this mode -- it's still
- * 2 bits per byte overhead. This is a little wasteful -- it could be done
- * with 1 bit per byte -- but lets us reuse the many shadow memory access
- * functions. Note also that in this mode the secondary V bit table is
- * never used.
- *
- * default: YES */
-extern Bool MC_(clo_undef_value_errors);
-
/* Fill malloc-d/free-d client blocks with a specific value? -1 if
not, else 0x00 .. 0xFF indicating the fill value to use. Can be
useful for causing programs with bad heap corruption to fail in
@@ -300,7 +288,32 @@
extern Int MC_(clo_malloc_fill);
extern Int MC_(clo_free_fill);
+/* Indicates the level of instrumentation/checking done by Memcheck.
+ 1 = No undefined value checking, Addrcheck-style behaviour only:
+ only address checking is done. This is faster but finds fewer
+ errors. Note that although Addrcheck had 1 bit per byte
+ overhead vs the old Memcheck's 9 bits per byte, with this mode
+ and compressed V bits, no memory is saved with this mode --
+ it's still 2 bits per byte overhead. This is a little wasteful
+ -- it could be done with 1 bit per byte -- but lets us reuse
+ the many shadow memory access functions. Note that in this
+ mode neither the secondary V bit table nor the origin-tag cache
+ are used.
+
+ 2 = Address checking and Undefined value checking are performed,
+ but origins are not tracked. So the origin-tag cache is not
+ used in this mode. This setting is the default and corresponds
+ to the "normal" Memcheck behaviour that has shipped for years.
+
+ 3 = Address checking, undefined value checking, and origins for
+ undefined values are tracked.
+
+ The default is 2.
+*/
+extern Int MC_(clo_mc_level);
+
+
/*------------------------------------------------------------*/
/*--- Instrumentation ---*/
/*------------------------------------------------------------*/
Modified: branches/OTRACK_BY_INSTRUMENTATION/memcheck/mc_main.c
===================================================================
--- branches/OTRACK_BY_INSTRUMENTATION/memcheck/mc_main.c 2008-04-15 10:07:16 UTC (rev 7877)
+++ branches/OTRACK_BY_INSTRUMENTATION/memcheck/mc_main.c 2008-04-15 10:09:21 UTC (rev 7878)
@@ -2752,7 +2752,7 @@
// --undef-value-errors=yes.
if (bad_addr != NULL) *bad_addr = a;
if ( VA_BITS2_NOACCESS == vabits2 ) return MC_AddrErr;
- else if ( MC_(clo_undef_value_errors) ) return MC_ValueErr;
+ else if ( MC_(clo_mc_level) >= 2 ) return MC_ValueErr;
}
a++;
}
@@ -2779,7 +2779,7 @@
// --undef-value-errors=yes.
if (bad_addr != NULL) *bad_addr = a;
if ( VA_BITS2_NOACCESS == vabits2 ) return MC_AddrErr;
- else if ( MC_(clo_undef_value_errors) ) return MC_ValueErr;
+ else if ( MC_(clo_mc_level) >= 2 ) return MC_ValueErr;
}
/* Ok, a is safe to read. */
if (* ((UChar*)a) == 0) {
@@ -3297,9 +3297,15 @@
}
case Err_Value:
- mc_pp_msg("UninitValue", err,
- "Use of uninitialised value of size %d (otag %u)",
- extra->Err.Value.szB, extra->Err.Value.otag);
+ if (extra->Err.Value.otag == 0) {
+ mc_pp_msg("UninitValue", err,
+ "Use of uninitialised value of size %d",
+ extra->Err.Value.szB);
+ } else {
+ mc_pp_msg("UninitValue", err,
+ "Use of uninitialised value of size %d (otag %u)",
+ extra->Err.Value.szB, extra->Err.Value.otag);
+ }
if (extra->Err.Value.origin_ec) {
if (extra->Err.Value.is_stack_origin) {
VG_(message)(Vg_UserMsg, " Uninitialised value originates "
@@ -3313,10 +3319,16 @@
break;
case Err_Cond:
- mc_pp_msg("UninitCondition", err,
- "Conditional jump or move depends"
- " on uninitialised value(s) (otag %u)",
- extra->Err.Cond.otag);
+ if (extra->Err.Cond.otag == 0) {
+ mc_pp_msg("UninitCondition", err,
+ "Conditional jump or move depends"
+ " on uninitialised value(s)");
+ } else {
+ mc_pp_msg("UninitCondition", err,
+ "Conditional jump or move depends"
+ " on uninitialised value(s) (otag %u)",
+ extra->Err.Cond.otag);
+ }
if (extra->Err.Cond.origin_ec) {
if (extra->Err.Cond.is_stack_origin) {
VG_(message)(Vg_UserMsg, " Uninitialised value originates "
@@ -3543,7 +3555,7 @@
static void mc_record_value_error ( ThreadId tid, Int szB, UInt otag )
{
MC_Error extra;
- tl_assert(MC_(clo_undef_value_errors));
+ tl_assert( MC_(clo_mc_level) >= 2 );
extra.Err.Value.szB = szB;
extra.Err.Value.otag = otag;
extra.Err.Value.origin_ec = NULL; /* Filled in later */
@@ -3554,7 +3566,7 @@
static void mc_record_cond_error ( ThreadId tid, UInt otag )
{
MC_Error extra;
- tl_assert(MC_(clo_undef_value_errors));
+ tl_assert( MC_(clo_mc_level) >= 2 );
extra.Err.Cond.otag = otag;
extra.Err.Cond.origin_ec = NULL; /* Filled in later */
extra.Err.Cond.is_stack_origin = False; /* Filled in later */
@@ -3582,7 +3594,7 @@
MC_Error extra;
tl_assert(VG_INVALID_THREADID != tid);
if (!isAddrErr)
- tl_assert(MC_(clo_undef_value_errors));
+ tl_assert( MC_(clo_mc_level) >= 2 );
extra.Err.MemParam.isAddrErr = isAddrErr;
extra.Err.MemParam.ai.tag = Addr_Undescribed;
VG_(maybe_record_error)( tid, Err_MemParam, a, msg, &extra );
@@ -4776,9 +4788,12 @@
static Bool mc_cheap_sanity_check ( void )
{
- /* nothing useful we can rapidly check */
n_sanity_cheap++;
PROF_EVENT(490, "cheap_sanity_check");
+ /* Check for sane operating level */
+ if (MC_(clo_mc_level) < 1 || MC_(clo_mc_level) > 3)
+ return False;
+ /* nothing else useful we can rapidly check */
return True;
}
@@ -4796,6 +4811,10 @@
n_sanity_expensive++;
PROF_EVENT(491, "expensive_sanity_check");
+ /* Check for sane operating level */
+ if (MC_(clo_mc_level) < 1 || MC_(clo_mc_level) > 3)
+ return False;
+
/* Check that the 3 distinguished SMs are still as they should be. */
/* Check noaccess DSM. */
@@ -4824,7 +4843,7 @@
/* If we're not checking for undefined value errors, the secondary V bit
* table should be empty. */
- if (!MC_(clo_undef_value_errors)) {
+ if (MC_(clo_mc_level) == 1) {
if (0 != VG_(OSetGen_Size)(secVBitTable))
return False;
}
@@ -4881,18 +4900,50 @@
VgRes MC_(clo_leak_resolution) = Vg_LowRes;
Bool MC_(clo_show_reachable) = False;
Bool MC_(clo_workaround_gcc296_bugs) = False;
-Bool MC_(clo_undef_value_errors) = True;
Int MC_(clo_malloc_fill) = -1;
Int MC_(clo_free_fill) = -1;
+Int MC_(clo_mc_level) = 2;
static Bool mc_process_cmd_line_options(Char* arg)
{
+ tl_assert( MC_(clo_mc_level) >= 1 && MC_(clo_mc_level) <= 3 );
+
+ /* Set MC_(clo_mc_level):
+ 1 = A bit tracking only
+ 2 = A and V bit tracking, but no V bit origins
+ 3 = A and V bit tracking, and V bit origins
+
+ Do this by inspecting --undef-value-errors= and
+ --track-origins=. Reject the case --undef-value-errors=no
+ --track-origins=yes as meaningless.
+ */
+ if (0 == VG_(strcmp)(arg, "--undef-value-errors=no")) {
+ if (MC_(clo_mc_level) == 3)
+ goto mc_level_error;
+ MC_(clo_mc_level) = 1;
+ return True;
+ }
+ if (0 == VG_(strcmp)(arg, "--undef-value-errors=yes")) {
+ if (MC_(clo_mc_level) == 1)
+ MC_(clo_mc_level) = 2;
+ return True;
+ }
+ if (0 == VG_(strcmp)(arg, "--track-origins=no")) {
+ if (MC_(clo_mc_level) == 3)
+ MC_(clo_mc_level) = 2;
+ return True;
+ }
+ if (0 == VG_(strcmp)(arg, "--track-origins=yes")) {
+ if (MC_(clo_mc_level) == 1)
+ goto mc_level_error;
+ MC_(clo_mc_level) = 3;
+ return True;
+ }
+
VG_BOOL_CLO(arg, "--partial-loads-ok", MC_(clo_partial_loads_ok))
else VG_BOOL_CLO(arg, "--show-reachable", MC_(clo_show_reachable))
else VG_BOOL_CLO(arg, "--workaround-gcc296-bugs",MC_(clo_workaround_gcc296_bugs))
- else VG_BOOL_CLO(arg, "--undef-value-errors", MC_(clo_undef_value_errors))
-
else VG_BNUM_CLO(arg, "--freelist-vol", MC_(clo_freelist_vol),
0, 10*1000*1000*1000LL)
@@ -4947,6 +4998,12 @@
return VG_(replacement_malloc_process_cmd_line_option)(arg);
return True;
+ /*NOTREACHED*/
+
+ mc_level_error:
+ VG_(message)(Vg_DebugMsg, "ERROR: --track-origins=yes has no effect "
+ "when --undef-value-errors=no");
+ return False;
}
static void mc_print_usage(void)
@@ -4956,6 +5013,7 @@
" --leak-resolution=low|med|high how much bt merging in leak check [low]\n"
" --show-reachable=no|yes show reachable blocks in leak check? [no]\n"
" --undef-value-errors=no|yes check for undefined value errors [yes]\n"
+" --track-origins=no|yes show origins of undefined values? [no]\n"
" --partial-loads-ok=no|yes too hard to explain here; see manual [no]\n"
" --freelist-vol=<number> volume of freed blocks queue [10000000]\n"
" --workaround-gcc296-bugs=no|yes self explanatory [no]\n"
@@ -5897,7 +5955,7 @@
VG_(track_pre_mem_write) ( check_mem_is_addressable );
VG_(track_post_mem_write) ( mc_post_mem_write );
- if (MC_(clo_undef_value_errors))
+ if (MC_(clo_mc_level) >= 2)
VG_(track_pre_reg_read) ( mc_pre_reg_read );
VG_(track_post_reg_write) ( mc_post_reg_write );
Modified: branches/OTRACK_BY_INSTRUMENTATION/memcheck/mc_translate.c
===================================================================
--- branches/OTRACK_BY_INSTRUMENTATION/memcheck/mc_translate.c 2008-04-15 10:07:16 UTC (rev 7877)
+++ branches/OTRACK_BY_INSTRUMENTATION/memcheck/mc_translate.c 2008-04-15 10:09:21 UTC (rev 7878)
@@ -903,7 +903,7 @@
IRAtom* origin;
// Don't do V bit tests if we're not reporting undefined value errors.
- if (!MC_(clo_undef_value_errors))
+ if (MC_(clo_mc_level) == 1)
return;
/* Since the original expression is atomic, there's no duplicated
@@ -923,10 +923,16 @@
cond = mkPCastTo( mce, Ity_I1, vatom );
/* cond will be 0 if all defined, and 1 if any not defined. */
- /* Get the origin info for the value we are about to check. */
- origin = schemeE( mce, atom );
- if (mce->hWordTy == Ity_I64) {
- origin = assignNew( 'B', mce, Ity_I64, unop(Iop_32Uto64, origin) );
+ /* Get the origin info for the value we are about to check. At
+ least, if we are doing origin tracking. If not, use a dummy
+ zero origin. */
+ if (MC_(clo_mc_level) == 3) {
+ origin = schemeE( mce, atom );
+ if (mce->hWordTy == Ity_I64) {
+ origin = assignNew( 'B', mce, Ity_I64, unop(Iop_32Uto64, origin) );
+ }
+ } else {
+ origin = mce->hWordTy == Ity_I64 ? mkU64(0) : mkU32(0);
}
switch (sz) {
@@ -1038,7 +1044,7 @@
// Don't do shadow PUTs if we're not doing undefined value checking.
// Their absence lets Vex's optimiser remove all the shadow computation
// that they depend on, which includes GETs of the shadow registers.
- if (!MC_(clo_undef_value_errors))
+ if (MC_(clo_mc_level) == 1)
return;
if (atom) {
@@ -1078,7 +1084,7 @@
// Don't do shadow PUTIs if we're not doing undefined value checking.
// Their absence lets Vex's optimiser remove all the shadow computation
// that they depend on, which includes GETIs of the shadow registers.
- if (!MC_(clo_undef_value_errors))
+ if (MC_(clo_mc_level) == 1)
return;
tl_assert(isOriginalAtom(mce,atom));
@@ -2893,7 +2899,7 @@
// If we're not doing undefined value checking, pretend that this value
// is "all valid". That lets Vex's optimiser remove some of the V bit
// shadow computation ops that precede it.
- if (!MC_(clo_undef_value_errors)) {
+ if (MC_(clo_mc_level) == 1) {
switch (ty) {
case Ity_V128: c = IRConst_V128(V_BITS16_DEFINED); break; // V128 weirdness
case Ity_I64: c = IRConst_U64 (V_BITS64_DEFINED); break;
@@ -3408,13 +3414,17 @@
}
/* Check we're not completely nuts */
- tl_assert(sizeof(UWord) == sizeof(void*));
- tl_assert(sizeof(Word) == sizeof(void*));
- tl_assert(sizeof(ULong) == 8);
- tl_assert(sizeof(Long) == 8);
- tl_assert(sizeof(UInt) == 4);
- tl_assert(sizeof(Int) == 4);
+ tl_assert(sizeof(UWord) == sizeof(void*));
+ tl_assert(sizeof(Word) == sizeof(void*));
+ tl_assert(sizeof(Addr) == sizeof(void*));
+ tl_assert(sizeof(ULong) == 8);
+ tl_assert(sizeof(Long) == 8);
+ tl_assert(sizeof(Addr64) == 8);
+ tl_assert(sizeof(UInt) == 4);
+ tl_assert(sizeof(Int) == 4);
+ tl_assert(MC_(clo_mc_level) >= 1 && MC_(clo_mc_level) <= 3);
+
/* Set up SB */
bb = deepCopyIRSBExceptStmts(bb_in);
@@ -3527,7 +3537,8 @@
VG_(printf)("\n");
}
- schemeS( &mce, st );
+ if (MC_(clo_mc_level) == 3)
+ schemeS( &mce, st );
/* Generate instrumentation code for each stmt ... */
@@ -3913,6 +3924,8 @@
static IRAtom* schemeE ( MCEnv* mce, IRExpr* e )
{
+ tl_assert(MC_(clo_mc_level) == 3);
+
switch (e->tag) {
case Iex_GetI: {
@@ -4193,6 +4206,8 @@
static void schemeS ( MCEnv* mce, IRStmt* st )
{
+ tl_assert(MC_(clo_mc_level) == 3);
+
switch (st->tag) {
case Ist_AbiHint:
|
|
From: Nicholas N. <nj...@cs...> - 2008-04-15 22:34:09
|
On Tue, 15 Apr 2008 sv...@va... wrote: > Author: sewardj > Date: 2008-04-15 11:09:21 +0100 (Tue, 15 Apr 2008) > New Revision: 7878 > > Log: > Add a new flag, --track-origins=no|yes [no], which enables origin > tracking. Since origin tracking is disabled by default, the default > behaviour of Memcheck is unchanged. I don't like it when certain options affect others. Since --undef-value-errors and --track-origins only have three sensible combinations (--undef-value-errors=no and --track-origins=yes doesn't make sense) would it be better to have a tri-state option, eg: --undef-value-errors=no --undef-value-errors=yes --undef-value-errors=yes-with-origins ? Better names are certainly possible, perhaps "none/short/long"? Nick |
|
From: Julian S. <js...@ac...> - 2008-04-15 23:14:20
|
> I don't like it when certain options affect others.
No .. me neither. I only did it like that since I couldn't think of
decent "=xxx" names for a 3-way option.
> --undef-value-errors and --track-origins only have three sensible
> combinations (--undef-value-errors=no and --track-origins=yes doesn't make
> sense)
It does at least check for that combination at startup, and complains
and won't start up.
--9840-- ERROR: --track-origins=yes has no effect when --undef-value-errors=no
valgrind: Bad option '--track-origins=yes'; aborting.
valgrind: Use --help for more information.
> would it be better to have a tri-state option, eg:
>
> --undef-value-errors=no
> --undef-value-errors=yes
> --undef-value-errors=yes-with-origins
>
> ? Better names are certainly possible, perhaps "none/short/long"?
One effect of this is that it would look a bit strange to programmers.
With the two-flag scheme we would say
To see origins of uninitialised values, use --track-origins=yes
whereas with the 3-way scheme we'd say
To see origins of uninitialised values,
use --undef-value-errors=yes-with-origins
which somehow seems less intuitive.
Hmm. Dunno.
J
|
|
From: Nicholas N. <nj...@cs...> - 2008-04-16 23:40:27
|
On Wed, 16 Apr 2008, Julian Seward wrote: > One effect of this is that it would look a bit strange to programmers. > With the two-flag scheme we would say > > To see origins of uninitialised values, use --track-origins=yes > > whereas with the 3-way scheme we'd say > > To see origins of uninitialised values, > use --undef-value-errors=yes-with-origins > > which somehow seems less intuitive. We have other tri-state options: --smc-check=none|stack|all --gen-suppressions=no|yes|all --leak-check=no|summary|full --leak-resolution=low|med|high Nick |