From: Florian Krohm Date: Thu, 19 Jul 2012 17:23:42 +0000 (+0000) Subject: Observe guards on dirty helpers in memcheck. X-Git-Tag: svn/VALGRIND_3_8_0~106 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=b54d735e927b67b7695668e47c57063cd667f815;p=thirdparty%2Fvalgrind.git Observe guards on dirty helpers in memcheck. This means, that any guest state and/or memory accesses of the helper (and complaints about those) only occur if the guard expression is true at runtime. Definedness of parameters that the helper might have is *always* checked, as parameters are evaluated regardless of the guard expression. New functions: expr2vbits_guarded_Load and gen_guarded_load_b. git-svn-id: svn://svn.valgrind.org/valgrind/trunk@12762 --- diff --git a/memcheck/mc_translate.c b/memcheck/mc_translate.c index af9442a00b..cf35c71cff 100644 --- a/memcheck/mc_translate.c +++ b/memcheck/mc_translate.c @@ -1091,7 +1091,7 @@ static void setHelperAnns ( MCEnv* mce, IRDirty* di ) { value to an existing shadow tmp as this breaks SSAness -- resulting in the post-instrumentation sanity checker spluttering in disapproval. */ -static void complainIfUndefined ( MCEnv* mce, IRAtom* atom ) +static void complainIfUndefined ( MCEnv* mce, IRAtom* atom, IRExpr *guard ) { IRAtom* vatom; IRType ty; @@ -1223,6 +1223,17 @@ static void complainIfUndefined ( MCEnv* mce, IRAtom* atom ) di = unsafeIRDirty_0_N( nargs/*regparms*/, nm, VG_(fnptr_to_fnentry)( fn ), args ); di->guard = cond; + + /* If the complaint is to be issued under a guard condition, AND that + guard condition. */ + if (guard) { + IRAtom *g1 = assignNew('V', mce, Ity_I32, unop(Iop_1Uto32, di->guard)); + IRAtom *g2 = assignNew('V', mce, Ity_I32, unop(Iop_1Uto32, guard)); + IRAtom *e = assignNew('V', mce, Ity_I32, binop(Iop_And32, g1, g2)); + + di->guard = assignNew('V', mce, Ity_I1, unop(Iop_32to1, e)); + } + setHelperAnns( mce, di ); stmt( 'V', mce, IRStmt_Dirty(di)); @@ -1279,10 +1290,11 @@ static Bool isAlwaysDefd ( MCEnv* mce, Int offset, Int size ) supplied V bits to the shadow state. We can pass in either an original atom or a V-atom, but not both. In the former case the relevant V-bits are then generated from the original. + We assume here, that the definedness of GUARD has already been checked. */ static void do_shadow_PUT ( MCEnv* mce, Int offset, - IRAtom* atom, IRAtom* vatom ) + IRAtom* atom, IRAtom* vatom, IRExpr *guard ) { IRType ty; @@ -1310,7 +1322,17 @@ void do_shadow_PUT ( MCEnv* mce, Int offset, /* complainIfUndefined(mce, atom); */ } else { /* Do a plain shadow Put. */ - stmt( 'V', mce, IRStmt_Put( offset + mce->layout->total_sizeB, vatom ) ); + if (guard) { + /* If the guard expression evaluates to false we simply Put the value + that is already stored in the guest state slot */ + IRAtom *cond, *iffalse; + + cond = assignNew('V', mce, Ity_I8, unop(Iop_1Uto8, guard)); + iffalse = assignNew('V', mce, ty, + IRExpr_Get(offset + mce->layout->total_sizeB, ty)); + vatom = assignNew('V', mce, ty, IRExpr_Mux0X(cond, iffalse, vatom)); + } + stmt( 'V', mce, IRStmt_Put( offset + mce->layout->total_sizeB, vatom )); } } @@ -1343,7 +1365,7 @@ void do_shadow_PUTI ( MCEnv* mce, IRPutI *puti) arrSize = descr->nElems * sizeofIRType(ty); tl_assert(ty != Ity_I1); tl_assert(isOriginalAtom(mce,ix)); - complainIfUndefined(mce,ix); + complainIfUndefined(mce, ix, NULL); if (isAlwaysDefd(mce, descr->base, arrSize)) { /* later: no ... */ /* emit code to emit a complaint if any of the vbits are 1. */ @@ -1392,7 +1414,7 @@ IRExpr* shadow_GETI ( MCEnv* mce, Int arrSize = descr->nElems * sizeofIRType(ty); tl_assert(ty != Ity_I1); tl_assert(isOriginalAtom(mce,ix)); - complainIfUndefined(mce,ix); + complainIfUndefined(mce, ix, NULL); if (isAlwaysDefd(mce, descr->base, arrSize)) { /* Always defined, return all zeroes of the relevant type */ return definedOfType(tyS); @@ -2484,15 +2506,15 @@ IRAtom* expr2vbits_Triop ( MCEnv* mce, /* IRRoundingModeDFP(I32) x I8 x D128 -> D128 */ return mkLazy3(mce, Ity_I128, vatom1, vatom2, vatom3); case Iop_ExtractV128: - complainIfUndefined(mce, atom3); + complainIfUndefined(mce, atom3, NULL); return assignNew('V', mce, Ity_V128, triop(op, vatom1, vatom2, atom3)); case Iop_Extract64: - complainIfUndefined(mce, atom3); + complainIfUndefined(mce, atom3, NULL); return assignNew('V', mce, Ity_I64, triop(op, vatom1, vatom2, atom3)); case Iop_SetElem8x8: case Iop_SetElem16x4: case Iop_SetElem32x2: - complainIfUndefined(mce, atom2); + complainIfUndefined(mce, atom2, NULL); return assignNew('V', mce, Ity_I64, triop(op, vatom1, atom2, vatom3)); default: ppIROp(op); @@ -2558,7 +2580,7 @@ IRAtom* expr2vbits_Binop ( MCEnv* mce, case Iop_ShlN32x2: case Iop_ShlN8x8: /* Same scheme as with all other shifts. */ - complainIfUndefined(mce, atom2); + complainIfUndefined(mce, atom2, NULL); return assignNew('V', mce, Ity_I64, binop(op, vatom1, atom2)); case Iop_QNarrowBin32Sto16Sx4: @@ -2641,25 +2663,25 @@ IRAtom* expr2vbits_Binop ( MCEnv* mce, case Iop_QShlN8Sx8: case Iop_QShlN8x8: case Iop_QSalN8x8: - complainIfUndefined(mce, atom2); + complainIfUndefined(mce, atom2, NULL); return mkPCast8x8(mce, vatom1); case Iop_QShlN16Sx4: case Iop_QShlN16x4: case Iop_QSalN16x4: - complainIfUndefined(mce, atom2); + complainIfUndefined(mce, atom2, NULL); return mkPCast16x4(mce, vatom1); case Iop_QShlN32Sx2: case Iop_QShlN32x2: case Iop_QSalN32x2: - complainIfUndefined(mce, atom2); + complainIfUndefined(mce, atom2, NULL); return mkPCast32x2(mce, vatom1); case Iop_QShlN64Sx1: case Iop_QShlN64x1: case Iop_QSalN64x1: - complainIfUndefined(mce, atom2); + complainIfUndefined(mce, atom2, NULL); return mkPCast32x2(mce, vatom1); case Iop_PwMax32Sx2: @@ -2756,13 +2778,13 @@ IRAtom* expr2vbits_Binop ( MCEnv* mce, return assignNew('V', mce, Ity_I64, binop(op, vatom1, vatom2)); case Iop_GetElem8x8: - complainIfUndefined(mce, atom2); + complainIfUndefined(mce, atom2, NULL); return assignNew('V', mce, Ity_I8, binop(op, vatom1, atom2)); case Iop_GetElem16x4: - complainIfUndefined(mce, atom2); + complainIfUndefined(mce, atom2, NULL); return assignNew('V', mce, Ity_I16, binop(op, vatom1, atom2)); case Iop_GetElem32x2: - complainIfUndefined(mce, atom2); + complainIfUndefined(mce, atom2, NULL); return assignNew('V', mce, Ity_I32, binop(op, vatom1, atom2)); /* Perm8x8: rearrange values in left arg using steering values @@ -2792,7 +2814,7 @@ IRAtom* expr2vbits_Binop ( MCEnv* mce, /* Same scheme as with all other shifts. Note: 22 Oct 05: this is wrong now, scalar shifts are done properly lazily. Vector shifts should be fixed too. */ - complainIfUndefined(mce, atom2); + complainIfUndefined(mce, atom2, NULL); return assignNew('V', mce, Ity_V128, binop(op, vatom1, atom2)); /* V x V shifts/rotates are done using the standard lazy scheme. */ @@ -2839,14 +2861,14 @@ IRAtom* expr2vbits_Binop ( MCEnv* mce, case Iop_F32ToFixed32Sx4_RZ: case Iop_Fixed32UToF32x4_RN: case Iop_Fixed32SToF32x4_RN: - complainIfUndefined(mce, atom2); + complainIfUndefined(mce, atom2, NULL); return mkPCast32x4(mce, vatom1); case Iop_F32ToFixed32Ux2_RZ: case Iop_F32ToFixed32Sx2_RZ: case Iop_Fixed32UToF32x2_RN: case Iop_Fixed32SToF32x2_RN: - complainIfUndefined(mce, atom2); + complainIfUndefined(mce, atom2, NULL); return mkPCast32x2(mce, vatom1); case Iop_QSub8Ux16: @@ -3003,25 +3025,25 @@ IRAtom* expr2vbits_Binop ( MCEnv* mce, case Iop_QShlN8Sx16: case Iop_QShlN8x16: case Iop_QSalN8x16: - complainIfUndefined(mce, atom2); + complainIfUndefined(mce, atom2, NULL); return mkPCast8x16(mce, vatom1); case Iop_QShlN16Sx8: case Iop_QShlN16x8: case Iop_QSalN16x8: - complainIfUndefined(mce, atom2); + complainIfUndefined(mce, atom2, NULL); return mkPCast16x8(mce, vatom1); case Iop_QShlN32Sx4: case Iop_QShlN32x4: case Iop_QSalN32x4: - complainIfUndefined(mce, atom2); + complainIfUndefined(mce, atom2, NULL); return mkPCast32x4(mce, vatom1); case Iop_QShlN64Sx2: case Iop_QShlN64x2: case Iop_QSalN64x2: - complainIfUndefined(mce, atom2); + complainIfUndefined(mce, atom2, NULL); return mkPCast32x4(mce, vatom1); case Iop_Mull32Sx2: @@ -3084,16 +3106,16 @@ IRAtom* expr2vbits_Binop ( MCEnv* mce, return assignNew('V', mce, Ity_V128, binop(op, vatom1, vatom2)); case Iop_GetElem8x16: - complainIfUndefined(mce, atom2); + complainIfUndefined(mce, atom2, NULL); return assignNew('V', mce, Ity_I8, binop(op, vatom1, atom2)); case Iop_GetElem16x8: - complainIfUndefined(mce, atom2); + complainIfUndefined(mce, atom2, NULL); return assignNew('V', mce, Ity_I16, binop(op, vatom1, atom2)); case Iop_GetElem32x4: - complainIfUndefined(mce, atom2); + complainIfUndefined(mce, atom2, NULL); return assignNew('V', mce, Ity_I32, binop(op, vatom1, atom2)); case Iop_GetElem64x2: - complainIfUndefined(mce, atom2); + complainIfUndefined(mce, atom2, NULL); return assignNew('V', mce, Ity_I64, binop(op, vatom1, atom2)); /* Perm8x16: rearrange values in left arg using steering values @@ -3152,7 +3174,7 @@ IRAtom* expr2vbits_Binop ( MCEnv* mce, /* Same scheme as with all other shifts. Note: 10 Nov 05: this is wrong now, scalar shifts are done properly lazily. Vector shifts should be fixed too. */ - complainIfUndefined(mce, atom2); + complainIfUndefined(mce, atom2, NULL); return assignNew('V', mce, Ity_V128, binop(op, vatom1, atom2)); /* I128-bit data-steering */ @@ -3818,7 +3840,7 @@ IRAtom* expr2vbits_Load_WRK ( MCEnv* mce, /* First, emit a definedness test for the address. This also sets the address (shadow) to 'defined' following the test. */ - complainIfUndefined( mce, addr ); + complainIfUndefined( mce, addr, NULL ); /* Now cook up a call to the relevant helper function, to read the data V bits from shadow memory. */ @@ -3932,6 +3954,32 @@ IRAtom* expr2vbits_Load ( MCEnv* mce, } +/* If there is no guard expression or the guard is always TRUE this function + behaves like expr2vbits_Load. If the guard is not true at runtime, an + all-bits-defined bit pattern will be returned. + It is assumed that definedness of GUARD has already been checked at the call + site. */ +static +IRAtom* expr2vbits_guarded_Load ( MCEnv* mce, + IREndness end, IRType ty, + IRAtom* addr, UInt bias, IRAtom *guard ) +{ + if (guard) { + IRAtom *cond, *iffalse, *iftrue; + + cond = assignNew('V', mce, Ity_I8, unop(Iop_1Uto8, guard)); + iftrue = assignNew('V', mce, ty, + expr2vbits_Load(mce, end, ty, addr, bias)); + iffalse = assignNew('V', mce, ty, definedOfType(ty)); + + return assignNew('V', mce, ty, IRExpr_Mux0X(cond, iffalse, iftrue)); + } + + /* No guard expression or unconditional load */ + return expr2vbits_Load(mce, end, ty, addr, bias); +} + + static IRAtom* expr2vbits_Mux0X ( MCEnv* mce, IRAtom* cond, IRAtom* expr0, IRAtom* exprX ) @@ -4141,7 +4189,7 @@ void do_shadow_Store ( MCEnv* mce, /* First, emit a definedness test for the address. This also sets the address (shadow) to 'defined' following the test. */ - complainIfUndefined( mce, addr ); + complainIfUndefined( mce, addr, guard ); /* Now decide which helper function to call to write the data V bits into shadow memory. */ @@ -4368,12 +4416,13 @@ void do_shadow_Dirty ( MCEnv* mce, IRDirty* d ) # endif /* First check the guard. */ - complainIfUndefined(mce, d->guard); + complainIfUndefined(mce, d->guard, NULL); /* Now round up all inputs and PCast over them. */ curr = definedOfType(Ity_I32); - /* Inputs: unmasked args */ + /* Inputs: unmasked args + Note: arguments are evaluated REGARDLESS of the guard expression */ for (i = 0; d->args[i]; i++) { if (d->cee->mcx_mask & (1<guard)); + iftrue = assignNew('V', mce, tySrc, shadow_GET(mce, gOff, tySrc)); + iffalse = assignNew('V', mce, tySrc, definedOfType(tySrc)); + src = assignNew('V', mce, tySrc, + IRExpr_Mux0X(cond, iffalse, iftrue)); + here = mkPCastTo( mce, Ity_I32, src ); curr = mkUifU32(mce, here, curr); gSz -= n; @@ -4432,7 +4490,7 @@ void do_shadow_Dirty ( MCEnv* mce, IRDirty* d ) should remove all but this test. */ IRType tyAddr; tl_assert(d->mAddr); - complainIfUndefined(mce, d->mAddr); + complainIfUndefined(mce, d->mAddr, d->guard); tyAddr = typeOfIRExpr(mce->sb->tyenv, d->mAddr); tl_assert(tyAddr == Ity_I32 || tyAddr == Ity_I64); @@ -4449,8 +4507,8 @@ void do_shadow_Dirty ( MCEnv* mce, IRDirty* d ) while (toDo >= 4) { here = mkPCastTo( mce, Ity_I32, - expr2vbits_Load ( mce, end, Ity_I32, - d->mAddr, d->mSize - toDo ) + expr2vbits_guarded_Load ( mce, end, Ity_I32, d->mAddr, + d->mSize - toDo, d->guard ) ); curr = mkUifU32(mce, here, curr); toDo -= 4; @@ -4459,8 +4517,8 @@ void do_shadow_Dirty ( MCEnv* mce, IRDirty* d ) while (toDo >= 2) { here = mkPCastTo( mce, Ity_I32, - expr2vbits_Load ( mce, end, Ity_I16, - d->mAddr, d->mSize - toDo ) + expr2vbits_guarded_Load ( mce, end, Ity_I16, d->mAddr, + d->mSize - toDo, d->guard ) ); curr = mkUifU32(mce, here, curr); toDo -= 2; @@ -4469,8 +4527,8 @@ void do_shadow_Dirty ( MCEnv* mce, IRDirty* d ) if (toDo == 1) { here = mkPCastTo( mce, Ity_I32, - expr2vbits_Load ( mce, end, Ity_I8, - d->mAddr, d->mSize - toDo ) + expr2vbits_guarded_Load ( mce, end, Ity_I8, d->mAddr, + d->mSize - toDo, d->guard ) ); curr = mkUifU32(mce, here, curr); toDo -= 1; @@ -4516,7 +4574,7 @@ void do_shadow_Dirty ( MCEnv* mce, IRDirty* d ) tyDst = szToITy( n ); do_shadow_PUT( mce, gOff, NULL, /* original atom */ - mkPCastTo( mce, tyDst, curr ) ); + mkPCastTo( mce, tyDst, curr ), d->guard ); gSz -= n; gOff += n; } @@ -4532,7 +4590,7 @@ void do_shadow_Dirty ( MCEnv* mce, IRDirty* d ) do_shadow_Store( mce, end, d->mAddr, d->mSize - toDo, NULL, /* original data */ mkPCastTo( mce, Ity_I32, curr ), - NULL/*guard*/ ); + d->guard ); toDo -= 4; } /* chew off 16-bit chunks */ @@ -4540,7 +4598,7 @@ void do_shadow_Dirty ( MCEnv* mce, IRDirty* d ) do_shadow_Store( mce, end, d->mAddr, d->mSize - toDo, NULL, /* original data */ mkPCastTo( mce, Ity_I16, curr ), - NULL/*guard*/ ); + d->guard ); toDo -= 2; } /* chew off the remaining 8-bit chunk, if any */ @@ -4548,7 +4606,7 @@ void do_shadow_Dirty ( MCEnv* mce, IRDirty* d ) do_shadow_Store( mce, end, d->mAddr, d->mSize - toDo, NULL, /* original data */ mkPCastTo( mce, Ity_I8, curr ), - NULL/*guard*/ ); + d->guard ); toDo -= 1; } tl_assert(toDo == 0); @@ -5419,7 +5477,7 @@ IRSB* MC_(instrument) ( VgCallbackClosure* closure, do_shadow_PUT( &mce, st->Ist.Put.offset, st->Ist.Put.data, - NULL /* shadow atom */ ); + NULL /* shadow atom */, NULL /* guard */ ); break; case Ist_PutI: @@ -5435,7 +5493,7 @@ IRSB* MC_(instrument) ( VgCallbackClosure* closure, break; case Ist_Exit: - complainIfUndefined( &mce, st->Ist.Exit.guard ); + complainIfUndefined( &mce, st->Ist.Exit.guard, NULL ); break; case Ist_IMark: @@ -5506,7 +5564,7 @@ IRSB* MC_(instrument) ( VgCallbackClosure* closure, VG_(printf)("\n\n"); } - complainIfUndefined( &mce, sb_in->next ); + complainIfUndefined( &mce, sb_in->next, NULL ); if (0 && verboze) { for (j = first_stmt; j < sb_out->stmts_used; j++) { @@ -5762,6 +5820,23 @@ static IRAtom* gen_load_b ( MCEnv* mce, Int szB, } } +static IRAtom* gen_guarded_load_b ( MCEnv* mce, Int szB, IRAtom* baseaddr, + Int offset, IRAtom* guard ) +{ + if (guard) { + IRAtom *cond, *iffalse, *iftrue; + + cond = assignNew('B', mce, Ity_I8, unop(Iop_1Uto8, guard)); + iftrue = assignNew('B', mce, Ity_I32, + gen_load_b(mce, szB, baseaddr, offset)); + iffalse = mkU32(0); + + return assignNew('B', mce, Ity_I32, IRExpr_Mux0X(cond, iffalse, iftrue)); + } + + return gen_load_b(mce, szB, baseaddr, offset); +} + /* Generate a shadow store. guard :: Ity_I1 controls whether the store really happens; NULL means it unconditionally does. */ static void gen_store_b ( MCEnv* mce, Int szB, @@ -5984,7 +6059,8 @@ static void do_origins_Dirty ( MCEnv* mce, IRDirty* d ) /* Now round up all inputs and maxU32 over them. */ - /* Inputs: unmasked args */ + /* Inputs: unmasked args + Note: arguments are evaluated REGARDLESS of the guard expression */ for (i = 0; d->args[i]; i++) { if (d->cee->mcx_mask & (1<layout->total_sizeB, - Ity_I32)); + /* Observe the guard expression. If it is false use 0, i.e. + nothing is known about the origin */ + IRAtom *cond, *iffalse, *iftrue; + + cond = assignNew( 'B', mce, Ity_I8, unop(Iop_1Uto8, d->guard)); + iffalse = mkU32(0); + iftrue = assignNew( 'B', mce, Ity_I32, + IRExpr_Get(b_offset + + 2*mce->layout->total_sizeB, + Ity_I32)); + here = assignNew( 'B', mce, Ity_I32, + IRExpr_Mux0X(cond, iffalse, iftrue)); curr = gen_maxU32( mce, curr, here ); } gSz -= n; @@ -6058,19 +6141,22 @@ static void do_origins_Dirty ( MCEnv* mce, IRDirty* d ) but nevertheless choose an endianness which is hopefully native to the platform. */ while (toDo >= 4) { - here = gen_load_b( mce, 4, d->mAddr, d->mSize - toDo ); + here = gen_guarded_load_b( mce, 4, d->mAddr, d->mSize - toDo, + d->guard ); curr = gen_maxU32( mce, curr, here ); toDo -= 4; } /* handle possible 16-bit excess */ while (toDo >= 2) { - here = gen_load_b( mce, 2, d->mAddr, d->mSize - toDo ); + here = gen_guarded_load_b( mce, 2, d->mAddr, d->mSize - toDo, + d->guard ); curr = gen_maxU32( mce, curr, here ); toDo -= 2; } /* chew off the remaining 8-bit chunk, if any */ if (toDo == 1) { - here = gen_load_b( mce, 1, d->mAddr, d->mSize - toDo ); + here = gen_guarded_load_b( mce, 1, d->mAddr, d->mSize - toDo, + d->guard ); curr = gen_maxU32( mce, curr, here ); toDo -= 1; } @@ -6113,6 +6199,20 @@ static void do_origins_Dirty ( MCEnv* mce, IRDirty* d ) /* Write 'curr' to the state slice gOff .. gOff+n-1 */ b_offset = MC_(get_otrack_shadow_offset)(gOff, 4); if (b_offset != -1) { + if (d->guard) { + /* If the guard expression evaluates to false we simply Put + the value that is already stored in the guest state slot */ + IRAtom *cond, *iffalse; + + cond = assignNew('B', mce, Ity_I8, + unop(Iop_1Uto8, d->guard)); + iffalse = assignNew('B', mce, Ity_I32, + IRExpr_Get(b_offset + + 2*mce->layout->total_sizeB, + Ity_I32)); + curr = assignNew('V', mce, Ity_I32, + IRExpr_Mux0X(cond, iffalse, curr)); + } stmt( 'B', mce, IRStmt_Put(b_offset + 2*mce->layout->total_sizeB, curr )); @@ -6130,19 +6230,19 @@ static void do_origins_Dirty ( MCEnv* mce, IRDirty* d ) /* chew off 32-bit chunks */ while (toDo >= 4) { gen_store_b( mce, 4, d->mAddr, d->mSize - toDo, curr, - NULL/*guard*/ ); + d->guard ); toDo -= 4; } /* handle possible 16-bit excess */ while (toDo >= 2) { gen_store_b( mce, 2, d->mAddr, d->mSize - toDo, curr, - NULL/*guard*/ ); + d->guard ); toDo -= 2; } /* chew off the remaining 8-bit chunk, if any */ if (toDo == 1) { gen_store_b( mce, 1, d->mAddr, d->mSize - toDo, curr, - NULL/*guard*/ ); + d->guard ); toDo -= 1; } tl_assert(toDo == 0);