derek/gem5
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

cpu: fix exec tracing memory corruption bug

Browse Source 
Accessing traceData (to call setAddress() and/or setData())
after initiating a timing translation was causing crashes,
since a failed translation could delete the traceData
object before returning.

It turns out that there was never a need to access traceData
after initiating the translation, as the traced data was
always available earlier; this ordering was merely
historical.  Furthermore, traceData->setAddress() and
traceData->setData() were being called both from the CPU
model and the ISA definition, often redundantly.

This patch standardizes all setAddress and setData calls
for memory instructions to be in the CPU models and not
in the ISA definition.  It also moves those calls above
the translation calls to eliminate the crashes.
This commit is contained in:
Steve Reinhardt
2010-03-23 08:50:57 -07:00
parent d484e1b334
commit 4d77ea7a57
11 changed files with 43 additions and 52 deletions
Download Patch File Download Diff File Expand all files Collapse all files

6
src/arch/alpha/isa/mem.isa
View File

@@ -275,7 +275,6 @@ def template StoreExecute {{
if (fault == NoFault) {
fault = xc->write((uint%(mem_acc_size)d_t&)Mem, EA,
memAccessFlags, NULL);
if (traceData) { traceData->setData(Mem); }
}
if (fault == NoFault) {
@@ -310,7 +309,6 @@ def template StoreCondExecute {{
if (fault == NoFault) {
fault = xc->write((uint%(mem_acc_size)d_t&)Mem, EA,
memAccessFlags, &write_result);
if (traceData) { traceData->setData(Mem); }
}
if (fault == NoFault) {
@@ -344,7 +342,6 @@ def template StoreInitiateAcc {{
if (fault == NoFault) {
fault = xc->write((uint%(mem_acc_size)d_t&)Mem, EA,
memAccessFlags, NULL);
if (traceData) { traceData->setData(Mem); }
}
return fault;
@@ -478,9 +475,6 @@ def LoadStoreBase(name, Name, ea_code, memacc_code, mem_flags, inst_flags,
mem_flags = makeList(mem_flags)
inst_flags = makeList(inst_flags)
# add hook to get effective addresses into execution trace output.
ea_code += '\nif (traceData) { traceData->setAddr(EA); }\n'
# Some CPU models execute the memory operation as an atomic unit,
# while others want to separate them into an effective address
# computation and a memory access operation. As a result, we need

2
src/arch/arm/isa/formats/mem.isa
View File

@@ -172,7 +172,6 @@ def template StoreExecute {{
if (fault == NoFault) {
fault = xc->write((uint%(mem_acc_size)d_t&)Mem, EA,
memAccessFlags, NULL);
if (traceData) { traceData->setData(Mem); }
}
if (fault == NoFault) {
@@ -204,7 +203,6 @@ def template StoreInitiateAcc {{
if (fault == NoFault) {
fault = xc->write((uint%(mem_acc_size)d_t&)Mem, EA,
memAccessFlags, NULL);
if (traceData) { traceData->setData(Mem); }
}
// Need to write back any potential address register update

8
src/arch/mips/isa/formats/mem.isa
View File

@@ -305,7 +305,6 @@ def template StoreExecute {{
if (fault == NoFault) {
fault = xc->write((uint%(mem_acc_size)d_t&)Mem, EA,
memAccessFlags, NULL);
if (traceData) { traceData->setData(Mem); }
}
if (fault == NoFault) {
@@ -342,7 +341,6 @@ def template StoreFPExecute {{
if (fault == NoFault) {
fault = xc->write((uint%(mem_acc_size)d_t&)Mem, EA,
memAccessFlags, NULL);
if (traceData) { traceData->setData(Mem); }
}
if (fault == NoFault) {
@@ -377,7 +375,6 @@ def template StoreCondExecute {{
if (fault == NoFault) {
fault = xc->write((uint%(mem_acc_size)d_t&)Mem, EA,
memAccessFlags, &write_result);
if (traceData) { traceData->setData(Mem); }
}
if (fault == NoFault) {
@@ -411,7 +408,6 @@ def template StoreInitiateAcc {{
if (fault == NoFault) {
fault = xc->write((uint%(mem_acc_size)d_t&)Mem, EA,
memAccessFlags, NULL);
if (traceData) { traceData->setData(Mem); }
}
return fault;
@@ -435,8 +431,6 @@ def template StoreCompleteAcc {{
if (fault == NoFault) {
%(op_wb)s;
if (traceData) { traceData->setData(getMemData(xc, pkt)); }
}
return fault;
@@ -459,8 +453,6 @@ def template StoreCompleteAcc {{
if (fault == NoFault) {
%(op_wb)s;
if (traceData) { traceData->setData(getMemData(xc, pkt)); }
}
return fault;

3
src/arch/mips/isa/formats/util.isa
View File

@@ -38,9 +38,6 @@ def LoadStoreBase(name, Name, ea_code, memacc_code, mem_flags, inst_flags,
mem_flags = makeList(mem_flags)
inst_flags = makeList(inst_flags)
# add hook to get effective addresses into execution trace output.
ea_code += '\nif (traceData) { traceData->setAddr(EA); }\n'
# Some CPU models execute the memory operation as an atomic unit,
# while others want to separate them into an effective address
# computation and a memory access operation. As a result, we need

2
src/arch/power/isa/formats/mem.isa
View File

@@ -166,7 +166,6 @@ def template StoreExecute {{
if (fault == NoFault) {
fault = xc->write((uint%(mem_acc_size)d_t&)Mem, EA,
memAccessFlags, NULL);
if (traceData) { traceData->setData(Mem); }
}
if (fault == NoFault) {
@@ -196,7 +195,6 @@ def template StoreInitiateAcc {{
if (fault == NoFault) {
fault = xc->write((uint%(mem_acc_size)d_t&)Mem, EA,
memAccessFlags, NULL);
if (traceData) { traceData->setData(Mem); }
}
// Need to write back any potential address register update

3
src/arch/power/isa/formats/util.isa
View File

@@ -97,9 +97,6 @@ def LoadStoreBase(name, Name, ea_code, memacc_code, mem_flags, inst_flags,
mem_flags = makeList(mem_flags)
inst_flags = makeList(inst_flags)
# add hook to get effective addresses into execution trace output.
ea_code += '\nif (traceData) { traceData->setAddr(EA); }\n'
# Generate InstObjParams for the memory access.
iop = InstObjParams(name, Name, base_class,
{'ea_code': ea_code,

9
src/cpu/inorder/resources/cache_unit.cc
View File

@@ -443,6 +443,10 @@ CacheUnit::read(DynInstPtr inst, Addr addr, T &data, unsigned flags)
//The size of the data we're trying to read.
int dataSize = sizeof(T);
if (inst->traceData) {
inst->traceData->setAddr(addr);
}
if (inst->split2ndAccess) {
dataSize = inst->split2ndSize;
cache_req->splitAccess = true;
@@ -541,6 +545,11 @@ CacheUnit::write(DynInstPtr inst, T data, Addr addr, unsigned flags,
//The size of the data we're trying to read.
int dataSize = sizeof(T);
if (inst->traceData) {
inst->traceData->setAddr(addr);
inst->traceData->setData(data);
}
if (inst->split2ndAccess) {
dataSize = inst->split2ndSize;
cache_req->splitAccess = true;

7
src/cpu/simple/atomic.cc
View File

@@ -451,6 +451,7 @@ AtomicSimpleCPU::write(T data, Addr addr, unsigned flags, uint64_t *res)
if (traceData) {
traceData->setAddr(addr);
traceData->setData(data);
}
//The block size of our peer.
@@ -530,12 +531,6 @@ AtomicSimpleCPU::write(T data, Addr addr, unsigned flags, uint64_t *res)
//stop now.
if (fault != NoFault || secondAddr <= addr)
{
// If the write needs to have a fault on the access, consider
// calling changeStatus() and changing it to "bad addr write"
// or something.
if (traceData) {
traceData->setData(gtoh(data));
}
if (req->isLocked() && fault == NoFault) {
assert(locked);
locked = false;

21
src/cpu/simple/base.cc
View File

@@ -205,6 +205,27 @@ change_thread_state(ThreadID tid, int activate, int priority)
{
}
void
BaseSimpleCPU::prefetch(Addr addr, unsigned flags)
{
if (traceData) {
traceData->setAddr(addr);
}
// need to do this...
}
void
BaseSimpleCPU::writeHint(Addr addr, int size, unsigned flags)
{
if (traceData) {
traceData->setAddr(addr);
}
// need to do this...
}
Fault
BaseSimpleCPU::copySrcTranslate(Addr src)
{

12
src/cpu/simple/base.hh
View File

@@ -232,16 +232,8 @@ class BaseSimpleCPU : public BaseCPU
Addr getEA() { panic("BaseSimpleCPU::getEA() not implemented\n");
M5_DUMMY_RETURN}
void prefetch(Addr addr, unsigned flags)
{
// need to do this...
}
void writeHint(Addr addr, int size, unsigned flags)
{
// need to do this...
}
void prefetch(Addr addr, unsigned flags);
void writeHint(Addr addr, int size, unsigned flags);
Fault copySrcTranslate(Addr src);

22
src/cpu/simple/timing.cc
View File

@@ -426,6 +426,10 @@ TimingSimpleCPU::read(Addr addr, T &data, unsigned flags)
int data_size = sizeof(T);
BaseTLB::Mode mode = BaseTLB::Read;
if (traceData) {
traceData->setAddr(addr);
}
RequestPtr req = new Request(asid, addr, data_size,
flags, pc, _cpuId, tid);
@@ -460,11 +464,6 @@ TimingSimpleCPU::read(Addr addr, T &data, unsigned flags)
thread->dtb->translateTiming(req, tc, translation, mode);
}
if (traceData) {
traceData->setData(data);
traceData->setAddr(addr);
}
return NoFault;
}
@@ -548,6 +547,11 @@ TimingSimpleCPU::write(T data, Addr addr, unsigned flags, uint64_t *res)
int data_size = sizeof(T);
BaseTLB::Mode mode = BaseTLB::Write;
if (traceData) {
traceData->setAddr(addr);
traceData->setData(data);
}
RequestPtr req = new Request(asid, addr, data_size,
flags, pc, _cpuId, tid);
@@ -584,13 +588,7 @@ TimingSimpleCPU::write(T data, Addr addr, unsigned flags, uint64_t *res)
thread->dtb->translateTiming(req, tc, translation, mode);
}
if (traceData) {
traceData->setAddr(req->getVaddr());
traceData->setData(data);
}
// If the write needs to have a fault on the access, consider calling
// changeStatus() and changing it to "bad addr write" or something.
// Translation faults will be returned via finishTranslation()
return NoFault;
}