From db5907d7b94ce492f29a96c95e186fdcda23a149 Mon Sep 17 00:00:00 2001 From: sewardj Date: Thu, 26 Nov 2009 17:20:21 +0000 Subject: Track vex r1930 (Change the IR representation of load linked and store conditional.) Completes the fix of #215771. git-svn-id: svn://svn.valgrind.org/valgrind/trunk@10957 a5019735-40e9-0310-863c-91ae7b9d1cf9 --- cachegrind/cg_main.c | 16 ++ callgrind/main.c | 26 +- drd/drd_load_store.c | 40 ++- exp-ptrcheck/h_main.c | 649 ++++++++++++++++++++++++++---------------------- helgrind/hg_main.c | 40 ++- lackey/lk_main.c | 50 +++- massif/ms_main.c | 6 +- memcheck/mc_translate.c | 179 +++++++++---- 8 files changed, 628 insertions(+), 378 deletions(-) diff --git a/cachegrind/cg_main.c b/cachegrind/cg_main.c index 50d388cd..391d47af 100644 --- a/cachegrind/cg_main.c +++ b/cachegrind/cg_main.c @@ -1053,6 +1053,22 @@ IRSB* cg_instrument ( VgCallbackClosure* closure, break; } + case Ist_LLSC: { + IRType dataTy; + if (st->Ist.LLSC.storedata == NULL) { + /* LL */ + dataTy = typeOfIRTemp(tyenv, st->Ist.LLSC.result); + addEvent_Dr( &cgs, curr_inode, + sizeofIRType(dataTy), st->Ist.LLSC.addr ); + } else { + /* SC */ + dataTy = typeOfIRExpr(tyenv, st->Ist.LLSC.storedata); + addEvent_Dw( &cgs, curr_inode, + sizeofIRType(dataTy), st->Ist.LLSC.addr ); + } + break; + } + case Ist_Exit: { /* Stuff to widen the guard expression to a host word, so we can pass it to the branch predictor simulation diff --git a/callgrind/main.c b/callgrind/main.c index fa6a90e9..147e6d05 100644 --- a/callgrind/main.c +++ b/callgrind/main.c @@ -657,14 +657,8 @@ void CLG_(collectBlockInfo)(IRSB* sbIn, static void addConstMemStoreStmt( IRSB* bbOut, UWord addr, UInt val, IRType hWordTy) { - /* JRS 2009june01: re IRTemp_INVALID, am assuming that this - function is used only to create instrumentation, and not to - copy/reconstruct IRStmt_Stores that were in the incoming IR - superblock. If that is not a correct assumption, then things - will break badly on PowerPC, esp w/ threaded apps. */ addStmtToIRSB( bbOut, IRStmt_Store(CLGEndness, - IRTemp_INVALID, IRExpr_Const(hWordTy == Ity_I32 ? IRConst_U32( addr ) : IRConst_U64( addr )), @@ -864,8 +858,24 @@ IRSB* CLG_(instrument)( VgCallbackClosure* closure, addEvent_Dw( &clgs, curr_inode, dataSize, cas->addr ); break; } - - case Ist_Exit: { + + case Ist_LLSC: { + IRType dataTy; + if (st->Ist.LLSC.storedata == NULL) { + /* LL */ + dataTy = typeOfIRTemp(sbIn->tyenv, st->Ist.LLSC.result); + addEvent_Dr( &clgs, curr_inode, + sizeofIRType(dataTy), st->Ist.LLSC.addr ); + } else { + /* SC */ + dataTy = typeOfIRExpr(sbIn->tyenv, st->Ist.LLSC.storedata); + addEvent_Dw( &clgs, curr_inode, + sizeofIRType(dataTy), st->Ist.LLSC.addr ); + } + break; + } + + case Ist_Exit: { UInt jmps_passed; /* We may never reach the next statement, so need to flush diff --git a/drd/drd_load_store.c b/drd/drd_load_store.c index 930827ee..d5cf75dc 100644 --- a/drd/drd_load_store.c +++ b/drd/drd_load_store.c @@ -460,8 +460,7 @@ IRSB* DRD_(instrument)(VgCallbackClosure* const closure, { IRStmt* const st = bb_in->stmts[i]; tl_assert(st); - if (st->tag == Ist_NoOp) - continue; + tl_assert(isFlatIRStmt(st)); switch (st->tag) { @@ -489,8 +488,7 @@ IRSB* DRD_(instrument)(VgCallbackClosure* const closure, break; case Ist_Store: - if (instrument && /* ignore stores resulting from st{d,w}cx. */ - st->Ist.Store.resSC == IRTemp_INVALID) + if (instrument) { instrument_store(bb, st->Ist.Store.addr, @@ -576,10 +574,42 @@ IRSB* DRD_(instrument)(VgCallbackClosure* const closure, addStmtToIRSB(bb, st); break; - default: + case Ist_LLSC: { + /* Ignore store-conditionals, and handle load-linked's + exactly like normal loads. */ + IRType dataTy; + if (st->Ist.LLSC.storedata == NULL) + { + /* LL */ + dataTy = typeOfIRTemp(bb_in->tyenv, st->Ist.LLSC.result); + if (instrument) { + instrument_load(bb, + st->Ist.LLSC.addr, + sizeofIRType(dataTy)); + } + } + else + { + /* SC */ + /*ignore */ + } addStmtToIRSB(bb, st); break; } + + case Ist_NoOp: + case Ist_AbiHint: + case Ist_Put: + case Ist_PutI: + case Ist_Exit: + /* None of these can contain any memory references. */ + addStmtToIRSB(bb, st); + break; + + default: + ppIRStmt(st); + tl_assert(0); + } } return bb; diff --git a/exp-ptrcheck/h_main.c b/exp-ptrcheck/h_main.c index 1ea80783..2e76e953 100644 --- a/exp-ptrcheck/h_main.c +++ b/exp-ptrcheck/h_main.c @@ -4332,6 +4332,323 @@ static void gen_nonptr_or_unknown_for_III( PCEnv* pce, IntRegInfo* iii ) } } + +/* schemeS helper for doing stores, pulled out into a function because + it needs to handle both normal stores and store-conditionals. + Returns False if we see a case we don't know how to handle. +*/ +static Bool schemeS_store ( PCEnv* pce, + IRExpr* data, IRExpr* addr, IRTemp resSC ) +{ + /* We have: STle(addr) = data + if data is int-word sized, do + check_store4(addr, addr#, data, data#) + for all other stores + check_store{1,2}(addr, addr#, data) + + The helper actually *does* the store, so that it can do the + post-hoc ugly hack of inspecting and "improving" the shadow data + after the store, in the case where it isn't an aligned word + store. + + Only word-sized values are shadowed. If this is a + store-conditional, .resSC will denote a non-word-typed temp, and + so we don't need to shadow it. Assert about the type, tho. + However, since we're not re-emitting the original IRStmt_Store, + but rather doing it as part of the helper function, we need to + actually do a SC in the helper, and assign the result bit to + .resSC. Ugly. + */ + IRType d_ty = typeOfIRExpr(pce->sb->tyenv, data); + IRExpr* addrv = schemeEw_Atom( pce, addr ); + if (resSC != IRTemp_INVALID) { + tl_assert(typeOfIRTemp(pce->sb->tyenv, resSC) == Ity_I1); + /* viz, not something we want to shadow */ + /* also, throw out all store-conditional cases that + we can't handle */ + if (pce->gWordTy == Ity_I32 && d_ty != Ity_I32) + return False; + if (pce->gWordTy == Ity_I64 && d_ty != Ity_I32 && d_ty != Ity_I64) + return False; + } + if (pce->gWordTy == Ity_I32) { + /* ------ 32 bit host/guest (cough, cough) ------ */ + switch (d_ty) { + /* Integer word case */ + case Ity_I32: { + IRExpr* datav = schemeEw_Atom( pce, data ); + if (resSC == IRTemp_INVALID) { + /* "normal" store */ + gen_dirty_v_WWWW( pce, + &check_store4_P, "check_store4_P", + addr, addrv, data, datav ); + } else { + /* store-conditional; need to snarf the success bit */ + IRTemp resSC32 + = gen_dirty_W_WWWW( pce, + &check_store4C_P, + "check_store4C_P", + addr, addrv, data, datav ); + /* presumably resSC32 will really be Ity_I32. In + any case we'll get jumped by the IR sanity + checker if it's not, when it sees the + following statement. */ + assign( 'I', pce, resSC, unop(Iop_32to1, mkexpr(resSC32)) ); + } + break; + } + /* Integer subword cases */ + case Ity_I16: + gen_dirty_v_WWW( pce, + &check_store2, "check_store2", + addr, addrv, + uwiden_to_host_word( pce, data )); + break; + case Ity_I8: + gen_dirty_v_WWW( pce, + &check_store1, "check_store1", + addr, addrv, + uwiden_to_host_word( pce, data )); + break; + /* 64-bit float. Pass store data in 2 32-bit pieces. */ + case Ity_F64: { + IRAtom* d64 = assignNew( 'I', pce, Ity_I64, + unop(Iop_ReinterpF64asI64, data) ); + IRAtom* dLo32 = assignNew( 'I', pce, Ity_I32, + unop(Iop_64to32, d64) ); + IRAtom* dHi32 = assignNew( 'I', pce, Ity_I32, + unop(Iop_64HIto32, d64) ); + gen_dirty_v_WWWW( pce, + &check_store8_ms4B_ls4B, + "check_store8_ms4B_ls4B", + addr, addrv, dHi32, dLo32 ); + break; + } + /* 32-bit float. We can just use _store4, but need + to futz with the argument type. */ + case Ity_F32: { + IRAtom* i32 = assignNew( 'I', pce, Ity_I32, + unop(Iop_ReinterpF32asI32, + data ) ); + gen_dirty_v_WWW( pce, + &check_store4, + "check_store4", + addr, addrv, i32 ); + break; + } + /* 64-bit int. Pass store data in 2 32-bit pieces. */ + case Ity_I64: { + IRAtom* dLo32 = assignNew( 'I', pce, Ity_I32, + unop(Iop_64to32, data) ); + IRAtom* dHi32 = assignNew( 'I', pce, Ity_I32, + unop(Iop_64HIto32, data) ); + gen_dirty_v_WWWW( pce, + &check_store8_ms4B_ls4B, + "check_store8_ms4B_ls4B", + addr, addrv, dHi32, dLo32 ); + break; + } + /* 128-bit vector. Pass store data in 4 32-bit pieces. + This is all very ugly and inefficient, but it is + hard to better without considerably complicating the + store-handling schemes. */ + case Ity_V128: { + IRAtom* dHi64 = assignNew( 'I', pce, Ity_I64, + unop(Iop_V128HIto64, data) ); + IRAtom* dLo64 = assignNew( 'I', pce, Ity_I64, + unop(Iop_V128to64, data) ); + IRAtom* w3 = assignNew( 'I', pce, Ity_I32, + unop(Iop_64HIto32, dHi64) ); + IRAtom* w2 = assignNew( 'I', pce, Ity_I32, + unop(Iop_64to32, dHi64) ); + IRAtom* w1 = assignNew( 'I', pce, Ity_I32, + unop(Iop_64HIto32, dLo64) ); + IRAtom* w0 = assignNew( 'I', pce, Ity_I32, + unop(Iop_64to32, dLo64) ); + gen_dirty_v_6W( pce, + &check_store16_ms4B_4B_4B_ls4B, + "check_store16_ms4B_4B_4B_ls4B", + addr, addrv, w3, w2, w1, w0 ); + break; + } + default: + ppIRType(d_ty); tl_assert(0); + } + } else { + /* ------ 64 bit host/guest (cough, cough) ------ */ + switch (d_ty) { + /* Integer word case */ + case Ity_I64: { + IRExpr* datav = schemeEw_Atom( pce, data ); + if (resSC == IRTemp_INVALID) { + /* "normal" store */ + gen_dirty_v_WWWW( pce, + &check_store8_P, "check_store8_P", + addr, addrv, data, datav ); + } else { + IRTemp resSC64 + = gen_dirty_W_WWWW( pce, + &check_store8C_P, + "check_store8C_P", + addr, addrv, data, datav ); + assign( 'I', pce, resSC, unop(Iop_64to1, mkexpr(resSC64)) ); + } + break; + } + /* Integer subword cases */ + case Ity_I32: + if (resSC == IRTemp_INVALID) { + /* "normal" store */ + gen_dirty_v_WWW( pce, + &check_store4, "check_store4", + addr, addrv, + uwiden_to_host_word( pce, data )); + } else { + /* store-conditional; need to snarf the success bit */ + IRTemp resSC64 + = gen_dirty_W_WWW( pce, + &check_store4C, + "check_store4C", + addr, addrv, + uwiden_to_host_word( pce, data )); + assign( 'I', pce, resSC, unop(Iop_64to1, mkexpr(resSC64)) ); + } + break; + case Ity_I16: + gen_dirty_v_WWW( pce, + &check_store2, "check_store2", + addr, addrv, + uwiden_to_host_word( pce, data )); + break; + case Ity_I8: + gen_dirty_v_WWW( pce, + &check_store1, "check_store1", + addr, addrv, + uwiden_to_host_word( pce, data )); + break; + /* 128-bit vector. Pass store data in 2 64-bit pieces. */ + case Ity_V128: { + IRAtom* dHi64 = assignNew( 'I', pce, Ity_I64, + unop(Iop_V128HIto64, data) ); + IRAtom* dLo64 = assignNew( 'I', pce, Ity_I64, + unop(Iop_V128to64, data) ); + gen_dirty_v_WWWW( pce, + &check_store16_ms8B_ls8B, + "check_store16_ms8B_ls8B", + addr, addrv, dHi64, dLo64 ); + break; + } + /* 64-bit float. */ + case Ity_F64: { + IRAtom* dI = assignNew( 'I', pce, Ity_I64, + unop(Iop_ReinterpF64asI64, + data ) ); + gen_dirty_v_WWW( pce, + &check_store8_all8B, + "check_store8_all8B", + addr, addrv, dI ); + break; + } + /* 32-bit float. We can just use _store4, but need + to futz with the argument type. */ + case Ity_F32: { + IRAtom* i32 = assignNew( 'I', pce, Ity_I32, + unop(Iop_ReinterpF32asI32, + data ) ); + IRAtom* i64 = assignNew( 'I', pce, Ity_I64, + unop(Iop_32Uto64, + i32 ) ); + gen_dirty_v_WWW( pce, + &check_store4, + "check_store4", + addr, addrv, i64 ); + break; + } + default: + ppIRType(d_ty); tl_assert(0); + } + } + /* And don't copy the original, since the helper does the store. + Ick. */ + return True; /* store was successfully instrumented */ +} + + +/* schemeS helper for doing loads, pulled out into a function because + it needs to handle both normal loads and load-linked's. +*/ +static void schemeS_load ( PCEnv* pce, IRExpr* addr, IRType e_ty, IRTemp dstv ) +{ + HChar* h_nm = NULL; + void* h_fn = NULL; + IRExpr* addrv = NULL; + if (e_ty == pce->gWordTy) { + tl_assert(dstv != IRTemp_INVALID); + } else { + tl_assert(dstv == IRTemp_INVALID); + } + if (pce->gWordTy == Ity_I32) { + /* 32 bit host/guest (cough, cough) */ + switch (e_ty) { + /* Ity_I32: helper returns shadow value. */ + case Ity_I32: h_fn = &check_load4_P; + h_nm = "check_load4_P"; break; + /* all others: helper does not return a shadow + value. */ + case Ity_V128: h_fn = &check_load16; + h_nm = "check_load16"; break; + case Ity_I64: + case Ity_F64: h_fn = &check_load8; + h_nm = "check_load8"; break; + case Ity_F32: h_fn = &check_load4; + h_nm = "check_load4"; break; + case Ity_I16: h_fn = &check_load2; + h_nm = "check_load2"; break; + case Ity_I8: h_fn = &check_load1; + h_nm = "check_load1"; break; + default: ppIRType(e_ty); tl_assert(0); + } + addrv = schemeEw_Atom( pce, addr ); + if (e_ty == Ity_I32) { + assign( 'I', pce, dstv, + mkexpr( gen_dirty_W_WW( pce, h_fn, h_nm, + addr, addrv )) ); + } else { + gen_dirty_v_WW( pce, NULL, h_fn, h_nm, addr, addrv ); + } + } else { + /* 64 bit host/guest (cough, cough) */ + switch (e_ty) { + /* Ity_I64: helper returns shadow value. */ + case Ity_I64: h_fn = &check_load8_P; + h_nm = "check_load8_P"; break; + /* all others: helper does not return a shadow + value. */ + case Ity_V128: h_fn = &check_load16; + h_nm = "check_load16"; break; + case Ity_F64: h_fn = &check_load8; + h_nm = "check_load8"; break; + case Ity_F32: + case Ity_I32: h_fn = &check_load4; + h_nm = "check_load4"; break; + case Ity_I16: h_fn = &check_load2; + h_nm = "check_load2"; break; + case Ity_I8: h_fn = &check_load1; + h_nm = "check_load1"; break; + default: ppIRType(e_ty); tl_assert(0); + } + addrv = schemeEw_Atom( pce, addr ); + if (e_ty == Ity_I64) { + assign( 'I', pce, dstv, + mkexpr( gen_dirty_W_WW( pce, h_fn, h_nm, + addr, addrv )) ); + } else { + gen_dirty_v_WW( pce, NULL, h_fn, h_nm, addr, addrv ); + } + } +} + + /* Generate into 'pce', instrumentation for 'st'. Also copy 'st' itself into 'pce' (the caller does not do so). This is somewhat complex and relies heavily on the assumption that the incoming IR @@ -4571,6 +4888,29 @@ static void schemeS ( PCEnv* pce, IRStmt* st ) break; } + case Ist_LLSC: { + if (st->Ist.LLSC.storedata == NULL) { + /* LL */ + IRTemp dst = st->Ist.LLSC.result; + IRType dataTy = typeOfIRTemp(pce->sb->tyenv, dst); + Bool isWord = dataTy == pce->gWordTy; + IRTemp dstv = isWord ? newShadowTmp( pce, dst ) + : IRTemp_INVALID; + schemeS_load( pce, st->Ist.LLSC.addr, dataTy, dstv ); + /* copy the original -- must happen after the helper call */ + stmt( 'C', pce, st ); + } else { + /* SC */ + schemeS_store( pce, + st->Ist.LLSC.storedata, + st->Ist.LLSC.addr, + st->Ist.LLSC.result ); + /* Don't copy the original, since the helper does the + store itself. */ + } + break; + } + case Ist_Dirty: { Int i; IRDirty* di; @@ -4702,244 +5042,15 @@ static void schemeS ( PCEnv* pce, IRStmt* st ) } /* case Ist_Put */ case Ist_Store: { - /* We have: STle(addr) = data - if data is int-word sized, do - check_store4(addr, addr#, data, data#) - for all other stores - check_store{1,2}(addr, addr#, data) - - The helper actually *does* the store, so that it can do - the post-hoc ugly hack of inspecting and "improving" the - shadow data after the store, in the case where it isn't an - aligned word store. - - Only word-sized values are shadowed. If this is a - store-conditional, .resSC will denote a non-word-typed - temp, and so we don't need to shadow it. Assert about the - type, tho. However, since we're not re-emitting the - original IRStmt_Store, but rather doing it as part of the - helper function, we need to actually do a SC in the - helper, and assign the result bit to .resSC. Ugly. - */ - IRExpr* data = st->Ist.Store.data; - IRExpr* addr = st->Ist.Store.addr; - IRType d_ty = typeOfIRExpr(pce->sb->tyenv, data); - IRExpr* addrv = schemeEw_Atom( pce, addr ); - IRTemp resSC = st->Ist.Store.resSC; - if (resSC != IRTemp_INVALID) { - tl_assert(typeOfIRTemp(pce->sb->tyenv, resSC) == Ity_I1); - /* viz, not something we want to shadow */ - /* also, throw out all store-conditional cases that - we can't handle */ - if (pce->gWordTy == Ity_I32 && d_ty != Ity_I32) - goto unhandled; - if (pce->gWordTy == Ity_I64 && d_ty != Ity_I32 && d_ty != Ity_I64) - goto unhandled; - } - if (pce->gWordTy == Ity_I32) { - /* ------ 32 bit host/guest (cough, cough) ------ */ - switch (d_ty) { - /* Integer word case */ - case Ity_I32: { - IRExpr* datav = schemeEw_Atom( pce, data ); - if (resSC == IRTemp_INVALID) { - /* "normal" store */ - gen_dirty_v_WWWW( pce, - &check_store4_P, "check_store4_P", - addr, addrv, data, datav ); - } else { - /* store-conditional; need to snarf the success bit */ - IRTemp resSC32 - = gen_dirty_W_WWWW( pce, - &check_store4C_P, - "check_store4C_P", - addr, addrv, data, datav ); - /* presumably resSC32 will really be Ity_I32. In - any case we'll get jumped by the IR sanity - checker if it's not, when it sees the - following statement. */ - assign( 'I', pce, resSC, unop(Iop_32to1, mkexpr(resSC32)) ); - } - break; - } - /* Integer subword cases */ - case Ity_I16: - gen_dirty_v_WWW( pce, - &check_store2, "check_store2", - addr, addrv, - uwiden_to_host_word( pce, data )); - break; - case Ity_I8: - gen_dirty_v_WWW( pce, - &check_store1, "check_store1", - addr, addrv, - uwiden_to_host_word( pce, data )); - break; - /* 64-bit float. Pass store data in 2 32-bit pieces. */ - case Ity_F64: { - IRAtom* d64 = assignNew( 'I', pce, Ity_I64, - unop(Iop_ReinterpF64asI64, data) ); - IRAtom* dLo32 = assignNew( 'I', pce, Ity_I32, - unop(Iop_64to32, d64) ); - IRAtom* dHi32 = assignNew( 'I', pce, Ity_I32, - unop(Iop_64HIto32, d64) ); - gen_dirty_v_WWWW( pce, - &check_store8_ms4B_ls4B, - "check_store8_ms4B_ls4B", - addr, addrv, dHi32, dLo32 ); - break; - } - /* 32-bit float. We can just use _store4, but need - to futz with the argument type. */ - case Ity_F32: { - IRAtom* i32 = assignNew( 'I', pce, Ity_I32, - unop(Iop_ReinterpF32asI32, - data ) ); - gen_dirty_v_WWW( pce, - &check_store4, - "check_store4", - addr, addrv, i32 ); - break; - } - /* 64-bit int. Pass store data in 2 32-bit pieces. */ - case Ity_I64: { - IRAtom* dLo32 = assignNew( 'I', pce, Ity_I32, - unop(Iop_64to32, data) ); - IRAtom* dHi32 = assignNew( 'I', pce, Ity_I32, - unop(Iop_64HIto32, data) ); - gen_dirty_v_WWWW( pce, - &check_store8_ms4B_ls4B, - "check_store8_ms4B_ls4B", - addr, addrv, dHi32, dLo32 ); - break; - } - - /* 128-bit vector. Pass store data in 4 32-bit pieces. - This is all very ugly and inefficient, but it is - hard to better without considerably complicating the - store-handling schemes. */ - case Ity_V128: { - IRAtom* dHi64 = assignNew( 'I', pce, Ity_I64, - unop(Iop_V128HIto64, data) ); - IRAtom* dLo64 = assignNew( 'I', pce, Ity_I64, - unop(Iop_V128to64, data) ); - IRAtom* w3 = assignNew( 'I', pce, Ity_I32, - unop(Iop_64HIto32, dHi64) ); - IRAtom* w2 = assignNew( 'I', pce, Ity_I32, - unop(Iop_64to32, dHi64) ); - IRAtom* w1 = assignNew( 'I', pce, Ity_I32, - unop(Iop_64HIto32, dLo64) ); - IRAtom* w0 = assignNew( 'I', pce, Ity_I32, - unop(Iop_64to32, dLo64) ); - gen_dirty_v_6W( pce, - &check_store16_ms4B_4B_4B_ls4B, - "check_store16_ms4B_4B_4B_ls4B", - addr, addrv, w3, w2, w1, w0 ); - break; - } - - - default: - ppIRType(d_ty); tl_assert(0); - } - } else { - /* ------ 64 bit host/guest (cough, cough) ------ */ - switch (d_ty) { - /* Integer word case */ - case Ity_I64: { - IRExpr* datav = schemeEw_Atom( pce, data ); - if (resSC == IRTemp_INVALID) { - /* "normal" store */ - gen_dirty_v_WWWW( pce, - &check_store8_P, "check_store8_P", - addr, addrv, data, datav ); - } else { - IRTemp resSC64 - = gen_dirty_W_WWWW( pce, - &check_store8C_P, - "check_store8C_P", - addr, addrv, data, datav ); - assign( 'I', pce, resSC, unop(Iop_64to1, mkexpr(resSC64)) ); - } - break; - } - /* Integer subword cases */ - case Ity_I32: - if (resSC == IRTemp_INVALID) { - /* "normal" store */ - gen_dirty_v_WWW( pce, - &check_store4, "check_store4", - addr, addrv, - uwiden_to_host_word( pce, data )); - } else { - /* store-conditional; need to snarf the success bit */ - IRTemp resSC64 - = gen_dirty_W_WWW( pce, - &check_store4C, - "check_store4C", - addr, addrv, - uwiden_to_host_word( pce, data )); - assign( 'I', pce, resSC, unop(Iop_64to1, mkexpr(resSC64)) ); - } - break; - case Ity_I16: - gen_dirty_v_WWW( pce, - &check_store2, "check_store2", - addr, addrv, - uwiden_to_host_word( pce, data )); - break; - case Ity_I8: - gen_dirty_v_WWW( pce, - &check_store1, "check_store1", - addr, addrv, - uwiden_to_host_word( pce, data )); - break; - /* 128-bit vector. Pass store data in 2 64-bit pieces. */ - case Ity_V128: { - IRAtom* dHi64 = assignNew( 'I', pce, Ity_I64, - unop(Iop_V128HIto64, data) ); - IRAtom* dLo64 = assignNew( 'I', pce, Ity_I64, - unop(Iop_V128to64, data) ); - gen_dirty_v_WWWW( pce, - &check_store16_ms8B_ls8B, - "check_store16_ms8B_ls8B", - addr, addrv, dHi64, dLo64 ); - break; - } - /* 64-bit float. */ - case Ity_F64: { - IRAtom* dI = assignNew( 'I', pce, Ity_I64, - unop(Iop_ReinterpF64asI64, - data ) ); - gen_dirty_v_WWW( pce, - &check_store8_all8B, - "check_store8_all8B", - addr, addrv, dI ); - break; - } - /* 32-bit float. We can just use _store4, but need - to futz with the argument type. */ - case Ity_F32: { - IRAtom* i32 = assignNew( 'I', pce, Ity_I32, - unop(Iop_ReinterpF32asI32, - data ) ); - IRAtom* i64 = assignNew( 'I', pce, Ity_I64, - unop(Iop_32Uto64, - i32 ) ); - gen_dirty_v_WWW( pce, - &check_store4, - "check_store4", - addr, addrv, i64 ); - break; - } - default: - ppIRType(d_ty); tl_assert(0); - } - } - /* And don't copy the original, since the helper does the - store. Ick. */ + Bool ok = schemeS_store( pce, + st->Ist.Store.data, + st->Ist.Store.addr, + IRTemp_INVALID/*not a SC*/ ); + if (!ok) goto unhandled; + /* Don't copy the original, since the helper does the store + itself. */ break; - } /* case Ist_Store */ + } case Ist_WrTmp: { /* This is the only place we have to deal with the full @@ -4992,69 +5103,7 @@ static void schemeS ( PCEnv* pce, IRStmt* st ) } case Iex_Load: { - IRExpr* addr = e->Iex.Load.addr; - HChar* h_nm = NULL; - void* h_fn = NULL; - IRExpr* addrv = NULL; - if (pce->gWordTy == Ity_I32) { - /* 32 bit host/guest (cough, cough) */ - switch (e_ty) { - /* Ity_I32: helper returns shadow value. */ - case Ity_I32: h_fn = &check_load4_P; - h_nm = "check_load4_P"; break; - /* all others: helper does not return a shadow - value. */ - case Ity_V128: h_fn = &check_load16; - h_nm = "check_load16"; break; - case Ity_I64: - case Ity_F64: h_fn = &check_load8; - h_nm = "check_load8"; break; - case Ity_F32: h_fn = &check_load4; - h_nm = "check_load4"; break; - case Ity_I16: h_fn = &check_load2; - h_nm = "check_load2"; break; - case Ity_I8: h_fn = &check_load1; - h_nm = "check_load1"; break; - default: ppIRType(e_ty); tl_assert(0); - } - addrv = schemeEw_Atom( pce, addr ); - if (e_ty == Ity_I32) { - assign( 'I', pce, dstv, - mkexpr( gen_dirty_W_WW( pce, h_fn, h_nm, - addr, addrv )) ); - } else { - gen_dirty_v_WW( pce, NULL, h_fn, h_nm, addr, addrv ); - } - } else { - /* 64 bit host/guest (cough, cough) */ - switch (e_ty) { - /* Ity_I64: helper returns shadow value. */ - case Ity_I64: h_fn = &check_load8_P; - h_nm = "check_load8_P"; break; - /* all others: helper does not return a shadow - value. */ - case Ity_V128: h_fn = &check_load16; - h_nm = "check_load16"; break; - case Ity_F64: h_fn = &check_load8; - h_nm = "check_load8"; break; - case Ity_F32: - case Ity_I32: h_fn = &check_load4; - h_nm = "check_load4"; break; - case Ity_I16: h_fn = &check_load2; - h_nm = "check_load2"; break; - case Ity_I8: h_fn = &check_load1; - h_nm = "check_load1"; break; - default: ppIRType(e_ty); tl_assert(0); - } - addrv = schemeEw_Atom( pce, addr ); - if (e_ty == Ity_I64) { - assign( 'I', pce, dstv, - mkexpr( gen_dirty_W_WW( pce, h_fn, h_nm, - addr, addrv )) ); - } else { - gen_dirty_v_WW( pce, NULL, h_fn, h_nm, addr, addrv ); - } - } + schemeS_load( pce, e->Iex.Load.addr, e_ty, dstv ); /* copy the original -- must happen after the helper call */ stmt( 'C', pce, st ); break; diff --git a/helgrind/hg_main.c b/helgrind/hg_main.c index 31cec56a..d2f34b68 100644 --- a/helgrind/hg_main.c +++ b/helgrind/hg_main.c @@ -4008,19 +4008,41 @@ IRSB* hg_instrument ( VgCallbackClosure* closure, break; } - case Ist_Store: - /* It seems we pretend that store-conditionals don't - exist, viz, just ignore them ... */ - if (st->Ist.Store.resSC == IRTemp_INVALID) { + case Ist_LLSC: { + /* We pretend store-conditionals don't exist, viz, ignore + them. Whereas load-linked's are treated the same as + normal loads. */ + IRType dataTy; + if (st->Ist.LLSC.storedata == NULL) { + /* LL */ + dataTy = typeOfIRTemp(bbIn->tyenv, st->Ist.LLSC.result); if (!inLDSO) { - instrument_mem_access( - bbOut, - st->Ist.Store.addr, - sizeofIRType(typeOfIRExpr(bbIn->tyenv, st->Ist.Store.data)), - True/*isStore*/, + instrument_mem_access( + bbOut, + st->Ist.LLSC.addr, + sizeofIRType(dataTy), + False/*!isStore*/, sizeofIRType(hWordTy) ); } + } else { + /* SC */ + /*ignore */ + } + break; + } + + case Ist_Store: + /* It seems we pretend that store-conditionals don't + exist, viz, just ignore them ... */ + if (!inLDSO) { + instrument_mem_access( + bbOut, + st->Ist.Store.addr, + sizeofIRType(typeOfIRExpr(bbIn->tyenv, st->Ist.Store.data)), + True/*isStore*/, + sizeofIRType(hWordTy) + ); } break; diff --git a/lackey/lk_main.c b/lackey/lk_main.c index d2110b6e..39e5bbde 100644 --- a/lackey/lk_main.c +++ b/lackey/lk_main.c @@ -790,17 +790,50 @@ IRSB* lk_instrument ( VgCallbackClosure* closure, was introduced, since prior to that point, the Vex front ends would translate a lock-prefixed instruction into a (normal) read followed by a (normal) write. */ + Int dataSize; + IRType dataTy; + IRCAS* cas = st->Ist.CAS.details; + tl_assert(cas->addr != NULL); + tl_assert(cas->dataLo != NULL); + dataTy = typeOfIRExpr(tyenv, cas->dataLo); + dataSize = sizeofIRType(dataTy); + if (cas->dataHi != NULL) + dataSize *= 2; /* since it's a doubleword-CAS */ if (clo_trace_mem) { - Int dataSize; - IRCAS* cas = st->Ist.CAS.details; - tl_assert(cas->addr != NULL); - tl_assert(cas->dataLo != NULL); - dataSize = sizeofIRType(typeOfIRExpr(tyenv, cas->dataLo)); - if (cas->dataHi != NULL) - dataSize *= 2; /* since it's a doubleword-CAS */ addEvent_Dr( sbOut, cas->addr, dataSize ); addEvent_Dw( sbOut, cas->addr, dataSize ); } + if (clo_detailed_counts) { + instrument_detail( sbOut, OpLoad, dataTy ); + if (cas->dataHi != NULL) /* dcas */ + instrument_detail( sbOut, OpLoad, dataTy ); + instrument_detail( sbOut, OpStore, dataTy ); + if (cas->dataHi != NULL) /* dcas */ + instrument_detail( sbOut, OpStore, dataTy ); + } + addStmtToIRSB( sbOut, st ); + break; + } + + case Ist_LLSC: { + IRType dataTy; + if (st->Ist.LLSC.storedata == NULL) { + /* LL */ + dataTy = typeOfIRTemp(tyenv, st->Ist.LLSC.result); + if (clo_trace_mem) + addEvent_Dr( sbOut, st->Ist.LLSC.addr, + sizeofIRType(dataTy) ); + if (clo_detailed_counts) + instrument_detail( sbOut, OpLoad, dataTy ); + } else { + /* SC */ + dataTy = typeOfIRExpr(tyenv, st->Ist.LLSC.storedata); + if (clo_trace_mem) + addEvent_Dw( sbOut, st->Ist.LLSC.addr, + sizeofIRType(dataTy) ); + if (clo_detailed_counts) + instrument_detail( sbOut, OpStore, dataTy ); + } addStmtToIRSB( sbOut, st ); break; } @@ -821,7 +854,8 @@ IRSB* lk_instrument ( VgCallbackClosure* closure, mkIRExprVec_0() ); else di = unsafeIRDirty_0_N( 0, "add_one_inverted_Jcc", - VG_(fnptr_to_fnentry)( &add_one_inverted_Jcc ), + VG_(fnptr_to_fnentry)( + &add_one_inverted_Jcc ), mkIRExprVec_0() ); addStmtToIRSB( sbOut, IRStmt_Dirty(di) ); diff --git a/massif/ms_main.c b/massif/ms_main.c index ecfb31c9..bcb13650 100644 --- a/massif/ms_main.c +++ b/massif/ms_main.c @@ -1903,14 +1903,12 @@ static void add_counter_update(IRSB* sbOut, Int n) IRTemp t2 = newIRTemp(sbOut->tyenv, Ity_I64); IRExpr* counter_addr = mkIRExpr_HWord( (HWord)&guest_instrs_executed ); - IRStmt* st1 = IRStmt_WrTmp(t1, IRExpr_Load(False/*!isLL*/, - END, Ity_I64, counter_addr)); + IRStmt* st1 = IRStmt_WrTmp(t1, IRExpr_Load(END, Ity_I64, counter_addr)); IRStmt* st2 = IRStmt_WrTmp(t2, IRExpr_Binop(Iop_Add64, IRExpr_RdTmp(t1), IRExpr_Const(IRConst_U64(n)))); - IRStmt* st3 = IRStmt_Store(END, IRTemp_INVALID/*"not store-conditional"*/, - counter_addr, IRExpr_RdTmp(t2)); + IRStmt* st3 = IRStmt_Store(END, counter_addr, IRExpr_RdTmp(t2)); addStmtToIRSB( sbOut, st1 ); addStmtToIRSB( sbOut, st2 ); diff --git a/memcheck/mc_translate.c b/memcheck/mc_translate.c index de570ab3..1509398b 100644 --- a/memcheck/mc_translate.c +++ b/memcheck/mc_translate.c @@ -3862,6 +3862,68 @@ static void do_shadow_CAS_double ( MCEnv* mce, IRCAS* cas ) } +/* ------ Dealing with LL/SC (not difficult) ------ */ + +static void do_shadow_LLSC ( MCEnv* mce, + IREndness stEnd, + IRTemp stResult, + IRExpr* stAddr, + IRExpr* stStoredata ) +{ + /* In short: treat a load-linked like a normal load followed by an + assignment of the loaded (shadow) data to the result temporary. + Treat a store-conditional like a normal store, and mark the + result temporary as defined. */ + IRType resTy = typeOfIRTemp(mce->sb->tyenv, stResult); + IRTemp resTmp = findShadowTmpV(mce, stResult); + + tl_assert(isIRAtom(stAddr)); + if (stStoredata) + tl_assert(isIRAtom(stStoredata)); + + if (stStoredata == NULL) { + /* Load Linked */ + /* Just treat this as a normal load, followed by an assignment of + the value to .result. */ + /* Stay sane */ + tl_assert(resTy == Ity_I64 || resTy == Ity_I32 + || resTy == Ity_I16 || resTy == Ity_I8); + assign( 'V', mce, resTmp, + expr2vbits_Load( + mce, stEnd, resTy, stAddr, 0/*addr bias*/)); + } else { + /* Store Conditional */ + /* Stay sane */ + IRType dataTy = typeOfIRExpr(mce->sb->tyenv, + stStoredata); + tl_assert(dataTy == Ity_I64 || dataTy == Ity_I32 + || dataTy == Ity_I16 || dataTy == Ity_I8); + do_shadow_Store( mce, stEnd, + stAddr, 0/* addr bias */, + stStoredata, + NULL /* shadow data */, + NULL/*guard*/ ); + /* This is a store conditional, so it writes to .result a value + indicating whether or not the store succeeded. Just claim + this value is always defined. In the PowerPC interpretation + of store-conditional, definedness of the success indication + depends on whether the address of the store matches the + reservation address. But we can't tell that here (and + anyway, we're not being PowerPC-specific). At least we are + guaranteed that the definedness of the store address, and its + addressibility, will be checked as per normal. So it seems + pretty safe to just say that the success indication is always + defined. + + In schemeS, for origin tracking, we must correspondingly set + a no-origin value for the origin shadow of .result. + */ + tl_assert(resTy == Ity_I1); + assign( 'V', mce, resTmp, definedOfType(resTy) ); + } +} + + /*------------------------------------------------------------*/ /*--- Memcheck main ---*/ /*------------------------------------------------------------*/ @@ -3979,6 +4041,11 @@ static Bool checkForBogusLiterals ( /*FLAT*/ IRStmt* st ) || isBogusAtom(cas->expdLo) || (cas->dataHi ? isBogusAtom(cas->dataHi) : False) || isBogusAtom(cas->dataLo); + case Ist_LLSC: + return isBogusAtom(st->Ist.LLSC.addr) + || (st->Ist.LLSC.storedata + ? isBogusAtom(st->Ist.LLSC.storedata) + : False); default: unhandled: ppIRStmt(st); @@ -4182,32 +4249,6 @@ IRSB* MC_(instrument) ( VgCallbackClosure* closure, st->Ist.Store.data, NULL /* shadow data */, NULL/*guard*/ ); - /* If this is a store conditional, it writes to .resSC a - value indicating whether or not the store succeeded. - Just claim this value is always defined. In the - PowerPC interpretation of store-conditional, - definedness of the success indication depends on - whether the address of the store matches the - reservation address. But we can't tell that here (and - anyway, we're not being PowerPC-specific). At least we - are guarantted that the definedness of the store - address, and its addressibility, will be checked as per - normal. So it seems pretty safe to just say that the - success indication is always defined. - - In schemeS, for origin tracking, we must - correspondingly set a no-origin value for the origin - shadow of resSC. - */ - if (st->Ist.Store.resSC != IRTemp_INVALID) { - assign( 'V', &mce, - findShadowTmpV(&mce, st->Ist.Store.resSC), - definedOfType( - shadowTypeV( - typeOfIRTemp(mce.sb->tyenv, - st->Ist.Store.resSC) - ))); - } break; case Ist_Exit: @@ -4241,6 +4282,14 @@ IRSB* MC_(instrument) ( VgCallbackClosure* closure, does it all. */ break; + case Ist_LLSC: + do_shadow_LLSC( &mce, + st->Ist.LLSC.end, + st->Ist.LLSC.result, + st->Ist.LLSC.addr, + st->Ist.LLSC.storedata ); + break; + default: VG_(printf)("\n"); ppIRStmt(st); @@ -4597,6 +4646,7 @@ static IRAtom* zWidenFrom32 ( MCEnv* mce, IRType dstTy, IRAtom* e ) { tl_assert(0); } + static IRAtom* schemeE ( MCEnv* mce, IRExpr* e ) { tl_assert(MC_(clo_mc_level) == 3); @@ -4732,6 +4782,7 @@ static IRAtom* schemeE ( MCEnv* mce, IRExpr* e ) } } + static void do_origins_Dirty ( MCEnv* mce, IRDirty* d ) { // This is a hacked version of do_shadow_Dirty @@ -4888,6 +4939,26 @@ static void do_origins_Dirty ( MCEnv* mce, IRDirty* d ) } } + +static void do_origins_Store ( MCEnv* mce, + IREndness stEnd, + IRExpr* stAddr, + IRExpr* stData ) +{ + Int dszB; + IRAtom* dataB; + /* assert that the B value for the address is already available + (somewhere), since the call to schemeE will want to see it. + XXXX how does this actually ensure that?? */ + tl_assert(isIRAtom(stAddr)); + tl_assert(isIRAtom(stData)); + dszB = sizeofIRType( typeOfIRExpr(mce->sb->tyenv, stData ) ); + dataB = schemeE( mce, stData ); + gen_store_b( mce, dszB, stAddr, 0/*offset*/, dataB, + NULL/*guard*/ ); +} + + static void schemeS ( MCEnv* mce, IRStmt* st ) { tl_assert(MC_(clo_mc_level) == 3); @@ -4928,30 +4999,47 @@ static void schemeS ( MCEnv* mce, IRStmt* st ) st->Ist.PutI.bias, t4 )); break; } + case Ist_Dirty: do_origins_Dirty( mce, st->Ist.Dirty.details ); break; - case Ist_Store: { - Int dszB; - IRAtom* dataB; - /* assert that the B value for the address is already - available (somewhere) */ - tl_assert(isIRAtom(st->Ist.Store.addr)); - dszB = sizeofIRType( - typeOfIRExpr(mce->sb->tyenv, st->Ist.Store.data )); - dataB = schemeE( mce, st->Ist.Store.data ); - gen_store_b( mce, dszB, st->Ist.Store.addr, 0/*offset*/, dataB, - NULL/*guard*/ ); - /* For the rationale behind this, see comments at the place - where the V-shadow for .resSC is constructed, in the main - loop in MC_(instrument). In short, wee regard .resSc as - always-defined. */ - if (st->Ist.Store.resSC != IRTemp_INVALID) { - assign( 'B', mce, findShadowTmpB(mce, st->Ist.Store.resSC), - mkU32(0) ); + + case Ist_Store: + do_origins_Store( mce, st->Ist.Store.end, + st->Ist.Store.addr, + st->Ist.Store.data ); + break; + + case Ist_LLSC: { + /* In short: treat a load-linked like a normal load followed + by an assignment of the loaded (shadow) data the result + temporary. Treat a store-conditional like a normal store, + and mark the result temporary as defined. */ + if (st->Ist.LLSC.storedata == NULL) { + /* Load Linked */ + IRType resTy + = typeOfIRTemp(mce->sb->tyenv, st->Ist.LLSC.result); + IRExpr* vanillaLoad + = IRExpr_Load(st->Ist.LLSC.end, resTy, st->Ist.LLSC.addr); + tl_assert(resTy == Ity_I64 || resTy == Ity_I32 + || resTy == Ity_I16 || resTy == Ity_I8); + assign( 'B', mce, findShadowTmpB(mce, st->Ist.LLSC.result), + schemeE(mce, vanillaLoad)); + } else { + /* Store conditional */ + do_origins_Store( mce, st->Ist.LLSC.end, + st->Ist.LLSC.addr, + st->Ist.LLSC.storedata ); + /* For the rationale behind this, see comments at the + place where the V-shadow for .result is constructed, in + do_shadow_LLSC. In short, we regard .result as + always-defined. */ + assign( 'B', mce, findShadowTmpB(mce, st->Ist.LLSC.result), + mkU32(0) ); } break; } + case Ist_Put: { Int b_offset = MC_(get_otrack_shadow_offset)( @@ -4965,15 +5053,18 @@ static void schemeS ( MCEnv* mce, IRStmt* st ) } break; } + case Ist_WrTmp: assign( 'B', mce, findShadowTmpB(mce, st->Ist.WrTmp.tmp), schemeE(mce, st->Ist.WrTmp.data) ); break; + case Ist_MBE: case Ist_NoOp: case Ist_Exit: case Ist_IMark: break; + default: VG_(printf)("mc_translate.c: schemeS: unhandled: "); ppIRStmt(st); -- cgit v1.2.3