|
From: <sv...@va...> - 2009-06-02 08:26:08
|
Author: sewardj
Date: 2009-06-02 09:25:59 +0100 (Tue, 02 Jun 2009)
New Revision: 10209
Log:
Change tool instrumentation routines to track VEX branches/DCAS
changes in r1898 (add support for LL/SC and get rid of various prior
kludges).
It also adds to coregrind the ability to synthesise a SIGBUS, since
ppc requires (or, at least, results in) SIGBUS to be thrown in the
case where the address to l{w,d}arx and st{w,d}cx. is not naturally
aligned.
Modified:
branches/DCAS/callgrind/main.c
branches/DCAS/coregrind/m_scheduler/scheduler.c
branches/DCAS/coregrind/m_signals.c
branches/DCAS/coregrind/pub_core_signals.h
branches/DCAS/drd/drd_load_store.c
branches/DCAS/exp-ptrcheck/h_main.c
branches/DCAS/helgrind/hg_main.c
branches/DCAS/massif/ms_main.c
branches/DCAS/memcheck/mc_machine.c
branches/DCAS/memcheck/mc_translate.c
Modified: branches/DCAS/callgrind/main.c
===================================================================
--- branches/DCAS/callgrind/main.c 2009-06-02 08:24:20 UTC (rev 10208)
+++ branches/DCAS/callgrind/main.c 2009-06-02 08:25:59 UTC (rev 10209)
@@ -483,8 +483,14 @@
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 )),
Modified: branches/DCAS/coregrind/m_scheduler/scheduler.c
===================================================================
--- branches/DCAS/coregrind/m_scheduler/scheduler.c 2009-06-02 08:24:20 UTC (rev 10208)
+++ branches/DCAS/coregrind/m_scheduler/scheduler.c 2009-06-02 08:25:59 UTC (rev 10209)
@@ -614,22 +614,6 @@
trc = 0;
dispatch_ctr_SAVED = VG_(dispatch_ctr);
-# if defined(VGA_ppc32) || defined(VGA_ppc64)
- /* This is necessary due to the hacky way vex models reservations
- on ppc. It's really quite incorrect for each thread to have its
- own reservation flag/address, since it's really something that
- all threads share (that's the whole point). But having shared
- guest state is something we can't model with Vex. However, as
- per PaulM's 2.4.0ppc, the reservation is modelled using a
- reservation flag which is cleared at each context switch. So it
- is indeed possible to get away with a per thread-reservation if
- the thread's reservation is cleared before running it.
- */
- /* Clear any existing reservation that this thread might have made
- last time it was running. */
- VG_(threads)[tid].arch.vex.guest_RESVN = 0;
-# endif
-
# if defined(VGP_ppc32_aix5) || defined(VGP_ppc64_aix5)
/* On AIX, we need to get a plausible value for SPRG3 for this
thread, since it's used I think as a thread-state pointer. It
@@ -1102,6 +1086,10 @@
VG_(synth_fault)(tid);
break;
+ case VEX_TRC_JMP_SIGBUS:
+ VG_(synth_sigbus)(tid);
+ break;
+
case VEX_TRC_JMP_NODECODE:
VG_(message)(Vg_UserMsg,
"valgrind: Unrecognised instruction at address %#lx.",
Modified: branches/DCAS/coregrind/m_signals.c
===================================================================
--- branches/DCAS/coregrind/m_signals.c 2009-06-02 08:24:20 UTC (rev 10208)
+++ branches/DCAS/coregrind/m_signals.c 2009-06-02 08:25:59 UTC (rev 10209)
@@ -1635,6 +1635,27 @@
deliver_signal(tid, &info, NULL);
}
+// Synthesise a SIGBUS.
+void VG_(synth_sigbus)(ThreadId tid)
+{
+ vki_siginfo_t info;
+
+ vg_assert(VG_(threads)[tid].status == VgTs_Runnable);
+
+ VG_(memset)(&info, 0, sizeof(info));
+ info.si_signo = VKI_SIGBUS;
+ /* There are several meanings to SIGBUS (as per POSIX, presumably),
+ but the most widely understood is "invalid address alignment",
+ so let's use that. */
+ info.si_code = VKI_BUS_ADRALN;
+ /* If we knew the invalid address in question, we could put it
+ in .si_addr. Oh well. */
+ /* info.VKI_SIGINFO_si_addr = (void*)addr; */
+
+ resume_scheduler(tid);
+ deliver_signal(tid, &info, NULL);
+}
+
// Synthesise a SIGTRAP.
void VG_(synth_sigtrap)(ThreadId tid)
{
Modified: branches/DCAS/coregrind/pub_core_signals.h
===================================================================
--- branches/DCAS/coregrind/pub_core_signals.h 2009-06-02 08:24:20 UTC (rev 10208)
+++ branches/DCAS/coregrind/pub_core_signals.h 2009-06-02 08:25:59 UTC (rev 10209)
@@ -73,6 +73,7 @@
extern void VG_(synth_fault_perms) (ThreadId tid, Addr addr);
extern void VG_(synth_sigill) (ThreadId tid, Addr addr);
extern void VG_(synth_sigtrap) (ThreadId tid);
+extern void VG_(synth_sigbus) (ThreadId tid);
/* Extend the stack to cover addr, if possible */
extern Bool VG_(extend_stack)(Addr addr, UInt maxsize);
Modified: branches/DCAS/drd/drd_load_store.c
===================================================================
--- branches/DCAS/drd/drd_load_store.c 2009-06-02 08:24:20 UTC (rev 10208)
+++ branches/DCAS/drd/drd_load_store.c 2009-06-02 08:25:59 UTC (rev 10209)
@@ -449,7 +449,6 @@
IRSB* bb;
IRExpr** argv;
Bool instrument = True;
- Bool bus_locked = False;
/* Set up BB */
bb = emptyIRSB();
@@ -483,16 +482,6 @@
{
case Imbe_Fence:
break; /* not interesting */
- case Imbe_BusLock:
- case Imbe_SnoopedStoreBegin:
- tl_assert(! bus_locked);
- bus_locked = True;
- break;
- case Imbe_BusUnlock:
- case Imbe_SnoopedStoreEnd:
- tl_assert(bus_locked);
- bus_locked = False;
- break;
default:
tl_assert(0);
}
@@ -500,7 +489,8 @@
break;
case Ist_Store:
- if (instrument && ! bus_locked)
+ if (instrument && /* ignore stores resulting from st{d,w}cx. */
+ st->Ist.Store.resSC == IRTemp_INVALID)
{
instrument_store(bb,
st->Ist.Store.addr,
@@ -546,8 +536,7 @@
argv);
addStmtToIRSB(bb, IRStmt_Dirty(di));
}
- if ((mFx == Ifx_Write || mFx == Ifx_Modify)
- && ! bus_locked)
+ if (mFx == Ifx_Write || mFx == Ifx_Modify)
{
di = unsafeIRDirty_0_N(
/*regparms*/2,
@@ -570,8 +559,6 @@
}
}
- tl_assert(! bus_locked);
-
return bb;
}
Modified: branches/DCAS/exp-ptrcheck/h_main.c
===================================================================
--- branches/DCAS/exp-ptrcheck/h_main.c 2009-06-02 08:24:20 UTC (rev 10208)
+++ branches/DCAS/exp-ptrcheck/h_main.c 2009-06-02 08:25:59 UTC (rev 10209)
@@ -1536,7 +1536,6 @@
if (o == GOF(CTR) && is4) goto exactly1;
if (o == GOF(CIA) && is4) goto none;
if (o == GOF(IP_AT_SYSCALL) && is4) goto none;
- if (o == GOF(RESVN) && is4) goto none;
if (o == GOF(TISTART) && is4) goto none;
if (o == GOF(TILEN) && is4) goto none;
if (o == GOF(REDIR_SP) && is4) goto none;
@@ -1700,7 +1699,6 @@
if (o == GOF(CTR) && is8) goto exactly1;
if (o == GOF(CIA) && is8) goto none;
if (o == GOF(IP_AT_SYSCALL) && is8) goto none;
- if (o == GOF(RESVN) && is8) goto none;
if (o == GOF(TISTART) && is8) goto none;
if (o == GOF(TILEN) && is8) goto none;
if (o == GOF(REDIR_SP) && is8) goto none;
@@ -4223,11 +4221,25 @@
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.
+
+ JRS 1 June 09: urr, this totally breaks with
+ store-conditional, since there's no platform-independent
+ way for the helper to do that and extract the success bit.
+ Ick.
*/
IRExpr* data = st->Ist.Store.data;
IRExpr* addr = st->Ist.Store.addr;
IRType d_ty = typeOfIRExpr(pce->bb->tyenv, data);
IRExpr* addrv = schemeEw_Atom( pce, addr );
+ if (st->Ist.Store.resSC != IRTemp_INVALID) {
+ tl_assert(typeOfIRTemp(pce->bb->tyenv, st->Ist.Store.resSC)
+ == Ity_I1); /* viz, not something we want to shadow */
+ }
if (pce->gWordTy == Ity_I32) {
/* ------ 32 bit host/guest (cough, cough) ------ */
switch (d_ty) {
Modified: branches/DCAS/helgrind/hg_main.c
===================================================================
--- branches/DCAS/helgrind/hg_main.c 2009-06-02 08:24:20 UTC (rev 10208)
+++ branches/DCAS/helgrind/hg_main.c 2009-06-02 08:25:59 UTC (rev 10209)
@@ -3601,40 +3601,6 @@
}
-//static void instrument_memory_bus_event ( IRSB* bbOut, IRMBusEvent event )
-//{
-// switch (event) {
-// case Imbe_SnoopedStoreBegin:
-// case Imbe_SnoopedStoreEnd:
-// /* These arise from ppc stwcx. insns. They should perhaps be
-// handled better. */
-// break;
-// case Imbe_Fence:
-// break; /* not interesting */
-// case Imbe_BusLock:
-// case Imbe_BusUnlock:
-// addStmtToIRSB(
-// bbOut,
-// IRStmt_Dirty(
-// unsafeIRDirty_0_N(
-// 0/*regparms*/,
-// event == Imbe_BusLock ? "evh__bus_lock"
-// : "evh__bus_unlock",
-// VG_(fnptr_to_fnentry)(
-// event == Imbe_BusLock ? &evh__bus_lock
-// : &evh__bus_unlock
-// ),
-// mkIRExprVec_0()
-// )
-// )
-// );
-// break;
-// default:
-// tl_assert(0);
-// }
-//}
-
-
static
IRSB* hg_instrument ( VgCallbackClosure* closure,
IRSB* bbIn,
@@ -3644,7 +3610,6 @@
{
Int i;
IRSB* bbOut;
- Bool isSnoopedStore = False;
Addr64 cia; /* address of current insn */
IRStmt* st;
@@ -3697,22 +3662,6 @@
switch (st->Ist.MBE.event) {
case Imbe_Fence:
break; /* not interesting */
- /* Imbe_Bus{Lock,Unlock} arise from x86/amd64 LOCK
- prefixed instructions. */
- case Imbe_BusLock:
- break;
- case Imbe_BusUnlock:
- break;
- /* Imbe_SnoopedStore{Begin,End} arise from ppc
- stwcx. instructions. */
- case Imbe_SnoopedStoreBegin:
- tl_assert(isSnoopedStore == False);
- isSnoopedStore = True;
- break;
- case Imbe_SnoopedStoreEnd:
- tl_assert(isSnoopedStore == True);
- isSnoopedStore = False;
- break;
default:
goto unhandled;
}
@@ -3721,8 +3670,7 @@
case Ist_CAS: {
/* Atomic read-modify-write cycle. Just pretend it's a
read. */
- IRCAS* cas = st->Ist.CAS.details;
- tl_assert(!isSnoopedStore);
+ IRCAS* cas = st->Ist.CAS.details;
/* FIXME: handle DCAS ! */
if (cas->oldHi != IRTemp_INVALID || cas->expdHi || cas->dataHi)
goto unhandled;
@@ -3737,7 +3685,9 @@
}
case Ist_Store:
- if (!isSnoopedStore)
+ /* It seems we pretend that store-conditionals don't
+ exist, viz, just ignore them ... */
+ if (st->Ist.Store.resSC == IRTemp_INVALID)
instrument_mem_access(
bbOut,
st->Ist.Store.addr,
@@ -3745,9 +3695,11 @@
True/*isStore*/,
sizeofIRType(hWordTy)
);
- break;
+ break;
case Ist_WrTmp: {
+ /* ... whereas here we don't care whether a load is a
+ vanilla one or a load-linked. */
IRExpr* data = st->Ist.WrTmp.data;
if (data->tag == Iex_Load) {
instrument_mem_access(
@@ -3776,11 +3728,6 @@
sizeofIRType(hWordTy)
);
}
- /* This isn't really correct. Really the
- instrumentation should be only added when
- !isSnoopedStore, just like with
- Ist_Store. Still, I don't think this is
- particularly important. */
if (d->mFx == Ifx_Write || d->mFx == Ifx_Modify) {
instrument_mem_access(
bbOut, d->mAddr, dataSize, True/*isStore*/,
Modified: branches/DCAS/massif/ms_main.c
===================================================================
--- branches/DCAS/massif/ms_main.c 2009-06-02 08:24:20 UTC (rev 10208)
+++ branches/DCAS/massif/ms_main.c 2009-06-02 08:25:59 UTC (rev 10209)
@@ -1871,12 +1871,14 @@
IRTemp t2 = newIRTemp(sbOut->tyenv, Ity_I64);
IRExpr* counter_addr = mkIRExpr_HWord( (HWord)&guest_instrs_executed );
- IRStmt* st1 = IRStmt_WrTmp(t1, IRExpr_Load(END, Ity_I64, counter_addr));
+ IRStmt* st1 = IRStmt_WrTmp(t1, IRExpr_Load(False/*!isLL*/,
+ 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, counter_addr, IRExpr_RdTmp(t2));
+ IRStmt* st3 = IRStmt_Store(END, IRTemp_INVALID/*"not store-conditional"*/,
+ counter_addr, IRExpr_RdTmp(t2));
addStmtToIRSB( sbOut, st1 );
addStmtToIRSB( sbOut, st2 );
Modified: branches/DCAS/memcheck/mc_machine.c
===================================================================
--- branches/DCAS/memcheck/mc_machine.c 2009-06-02 08:24:20 UTC (rev 10208)
+++ branches/DCAS/memcheck/mc_machine.c 2009-06-02 08:25:59 UTC (rev 10209)
@@ -182,7 +182,6 @@
if (o == GOF(CIA) && sz == 8) return -1;
if (o == GOF(IP_AT_SYSCALL) && sz == 8) return -1; /* slot unused */
- if (o == GOF(RESVN) && sz == 8) return -1;
if (o == GOF(FPROUND) && sz == 4) return -1;
if (o == GOF(EMWARN) && sz == 4) return -1;
if (o == GOF(TISTART) && sz == 8) return -1;
@@ -341,7 +340,6 @@
if (o == GOF(CIA) && sz == 4) return -1;
if (o == GOF(IP_AT_SYSCALL) && sz == 4) return -1; /* slot unused */
- if (o == GOF(RESVN) && sz == 4) return -1;
if (o == GOF(FPROUND) && sz == 4) return -1;
if (o == GOF(VRSAVE) && sz == 4) return -1;
if (o == GOF(EMWARN) && sz == 4) return -1;
Modified: branches/DCAS/memcheck/mc_translate.c
===================================================================
--- branches/DCAS/memcheck/mc_translate.c 2009-06-02 08:24:20 UTC (rev 10208)
+++ branches/DCAS/memcheck/mc_translate.c 2009-06-02 08:25:59 UTC (rev 10209)
@@ -4179,6 +4179,32 @@
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:
@@ -4899,6 +4925,14 @@
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) );
+ }
break;
}
case Ist_Put: {
|