Thanks for the trace... one problem with the earlier trace is that I was reading it backwards. I'll take a look at this (along with the earlier emails) later today (I hope) and see if I can come up with an adequate solution. Nothing personal, but I'm pretty sure the patch you sent out isn't it :-).
Steve On Fri, Mar 19, 2010 at 10:08 AM, Beckmann, Brad <[email protected]> wrote: > For the TraceData bug, below is the complete set of call stacks flagged by > Valgrind. I do have a patch that fixes this problem, but it is fairly > heavyweight do to the fact that I did not want to redesign timing.cc. I also > didn't send it out before because it doesn't compile with the O3CPU model. > If someone wants to pick up this patch and make it work with the O3 model, > that would be great. I'll send it out for you all to take a look at. > > Brad > > > ==27646== Invalid write of size 8 > ==27646== at 0x50819C: Trace::InstRecord::setData(unsigned long) > (/tmp/bbeckman/m5/build/ALPHA_SE_MOESI_hammer/sim/insttracer.hh:113) > ==27646== by 0x832EE8: AlphaISAInst::Stq::initiateAcc(TimingSimpleCPU*, > Trace::InstRecord*) const (/tmp/bbeckman/m5/build/ALPHA_SE_MOE > SI_hammer/arch/alpha/timing_simple_cpu_exec.cc:1955) > ==27646== by 0x80C56C: TimingSimpleCPU::completeIfetch(Packet*) > (/tmp/bbeckman/m5/build/ALPHA_SE_MOESI_hammer/cpu/simple/timing.cc:769) > ==27646== by 0x80CAAB: TimingSimpleCPU::IcachePort::recvTiming(Packet*) > (/tmp/bbeckman/m5/build/ALPHA_SE_MOESI_hammer/cpu/simple/timing.cc:832) > ==27646== by 0x42F8AB: Port::sendTiming(Packet*) > (/tmp/bbeckman/m5/build/ALPHA_SE_MOESI_hammer/mem/port.hh:186) > ==27646== by 0x576756: Bus::recvTiming(Packet*) > (/tmp/bbeckman/m5/build/ALPHA_SE_MOESI_hammer/mem/bus.cc:243) > ==27646== by 0x57EDA7: Bus::BusPort::recvTiming(Packet*) > (/tmp/bbeckman/m5/build/ALPHA_SE_MOESI_hammer/mem/bus.hh:89) > ==27646== by 0x42F8AB: Port::sendTiming(Packet*) > (/tmp/bbeckman/m5/build/ALPHA_SE_MOESI_hammer/mem/port.hh:186) > ==27646== by 0x847881: SimpleTimingPort::sendDeferredPacket() > (/tmp/bbeckman/m5/build/ALPHA_SE_MOESI_hammer/mem/tport.cc:150) > ==27646== by 0x847F6B: SimpleTimingPort::processSendEvent() > (/tmp/bbeckman/m5/build/ALPHA_SE_MOESI_hammer/mem/tport.cc:191) > ==27646== by 0x849131: EventWrapper<SimpleTimingPort, > &(SimpleTimingPort::processSendEvent())>::process() > (/tmp/bbeckman/m5/build/ALPH > A_SE_MOESI_hammer/sim/eventq.hh:468) > ==27646== by 0x5F74A6: EventQueue::serviceOne() > (/tmp/bbeckman/m5/build/ALPHA_SE_MOESI_hammer/sim/eventq.cc:202) > ==27646== Address 0x5d99c08 is 72 bytes inside a block of size 120 free'd > ==27646== at 0x4A1A19C: operator delete(void*) > (m_replacemalloc/vg_replace_malloc.c:342) > ==27646== by 0x5FD04A: Trace::ExeTracerRecord::~ExeTracerRecord() > (/tmp/bbeckman/m5/build/ALPHA_SE_MOESI_hammer/cpu/exetrace.hh:47) > ==27646== by 0x80CDA5: > TimingSimpleCPU::translationFault(RefCountingPtr<FaultBase>) > (/tmp/bbeckman/m5/build/ALPHA_SE_MOESI_hammer/cpu/simple/timing.cc:376) > ==27646== by 0x80D39B: > TimingSimpleCPU::sendData(RefCountingPtr<FaultBase>, Request*, unsigned > char*, unsigned long*, bool) > (/tmp/bbeckman/m5/build/ALPHA_SE_MOESI_hammer/cpu/simple/timing.cc:281) > ==27646== by 0x8134EC: > TimingSimpleCPU::DataTranslation::finish(RefCountingPtr<FaultBase>, Request*, > ThreadContext*, BaseTLB::Mode) (/ > tmp/bbeckman/m5/build/ALPHA_SE_MOESI_hammer/cpu/simple/timing.hh:139) > ==27646== by 0x843209: AlphaISA::TLB::translateTiming(Request*, > ThreadContext*, BaseTLB::Translation*, BaseTLB::Mode) (/tmp/bbeckman/m > 5/build/ALPHA_SE_MOESI_hammer/arch/alpha/tlb.cc:603) > ==27646== by 0x810B60: RefCountingPtr<FaultBase> > TimingSimpleCPU::write<unsigned long>(unsigned long, unsigned long, unsigned, > unsigne > d long*) > (/tmp/bbeckman/m5/build/ALPHA_SE_MOESI_hammer/cpu/simple/timing.cc:599) > ==27646== by 0x832E7E: AlphaISAInst::Stq::initiateAcc(TimingSimpleCPU*, > Trace::InstRecord*) const (/tmp/bbeckman/m5/build/ALPHA_SE_MOE > SI_hammer/arch/alpha/timing_simple_cpu_exec.cc:1953) > ==27646== by 0x80C56C: TimingSimpleCPU::completeIfetch(Packet*) > (/tmp/bbeckman/m5/build/ALPHA_SE_MOESI_hammer/cpu/simple/timing.cc:769) > ==27646== by 0x80CAAB: TimingSimpleCPU::IcachePort::recvTiming(Packet*) > (/tmp/bbeckman/m5/build/ALPHA_SE_MOESI_hammer/cpu/simple/timing.cc:832) > ==27646== by 0x42F8AB: Port::sendTiming(Packet*) > (/tmp/bbeckman/m5/build/ALPHA_SE_MOESI_hammer/mem/port.hh:186) > ==27646== by 0x576756: Bus::recvTiming(Packet*) > (/tmp/bbeckman/m5/build/ALPHA_SE_MOESI_hammer/mem/bus.cc:243) > ==27646== > > -----Original Message----- > From: [email protected] [mailto:[email protected]] On Behalf Of > Steve Reinhardt > Sent: Thursday, March 18, 2010 9:52 PM > To: M5 Developer List > Subject: Re: [m5-dev] [PATCH 07 of 31] m5: Fixed request read bug flagged by > Valgrind > > Hmm, now I'm wondering if perhaps the other bug you ran into that you > attributed to the static vs dynamic data allocation is just another > manifestation of the same thing; do you think that's possible? > Perhaps we can take a closer look tomorrow. > > Steve > > On Thu, Mar 18, 2010 at 9:41 PM, Steve Reinhardt <[email protected]> wrote: >> This stack is kind of funky... I'm not sure why Stq::initiateAcc would >> call into TimingSimpleCPU::completeIfetch. >> >> I went back and looked at that ExecEnable bug discussion though, and I >> recall that problem... sad to see that after all the discussion we had >> almost a year ago the bug is still not fixed (as far as I can tell). >> This patch you made here does look like it could be addressing the >> same or a similar issue. >> >> Steve >> >> On Thu, Mar 18, 2010 at 6:56 PM, Beckmann, Brad <[email protected]> >> wrote: >>> I don't have exact Valgrind stack anymore, but below is the approximate >>> call stack (it is actually a subset of another call stack that I wasn't >>> able to completely solve...more on that later). Basically if I remember >>> correctly, the request is deleted within sendData. >>> >>> TimingSimpleCPU::sendData(RefCountingPtr<FaultBase>, Request*, unsigned >>> char*, unsigned long*, bool) ==27646== by >>> TimingSimpleCPU::DataTranslation::finish(RefCountingPtr<FaultBase>, >>> Request*, ThreadContext*, BaseTLB::Mode) (/ >>> tmp/bbeckman/m5/build/ALPHA_SE_MOESI_hammer/cpu/simple/timing.hh:139) >>> AlphaISA::TLB::translateTiming(Request*, ThreadContext*, >>> BaseTLB::Translation*, BaseTLB::Mode) (/tmp/bbeckman/m >>> 5/build/ALPHA_SE_MOESI_hammer/arch/alpha/tlb.cc:603) >>> RefCountingPtr<FaultBase> TimingSimpleCPU::write<unsigned long>(unsigned >>> long, unsigned long, unsigned, unsigne >>> d long*) >>> (/tmp/bbeckman/m5/build/ALPHA_SE_MOESI_hammer/cpu/simple/timing.cc:599) >>> AlphaISAInst::Stq::initiateAcc(TimingSimpleCPU*, Trace::InstRecord*) const >>> (/tmp/bbeckman/m5/build/ALPHA_SE_MOE >>> SI_hammer/arch/alpha/timing_simple_cpu_exec.cc:1953) >>> TimingSimpleCPU::completeIfetch(Packet*) >>> (/tmp/bbeckman/m5/build/ALPHA_SE_MOESI_hammer/cpu/simple/timing.cc:769) >>> TimingSimpleCPU::IcachePort::recvTiming(Packet*) >>> (/tmp/bbeckman/m5/build/ALPHA_SE_MOESI_hammer/cpu/simple/timing.cc:832) >>> Port::sendTiming(Packet*) >>> (/tmp/bbeckman/m5/build/ALPHA_SE_MOESI_hammer/mem/port.hh:186) >>> Bus::recvTiming(Packet*) >>> (/tmp/bbeckman/m5/build/ALPHA_SE_MOESI_hammer/mem/bus.cc:243) >>> >>> >>> Now the bug I wasn't able to completely fix involved the bug identified in >>> the old thread: "Memory corruption in m5 dev repository whenusing >>> --trace-flags="ExecEnable"". That bug as a similar problem where the trace >>> data is deleted within the write function and then referenced later in >>> initiateAcc, however it is even more confusing because it deals with two >>> different pointers to traceData. I have a patch that has a heavyweight fix >>> for that bug, but I didn't send it out for review because I couldn't figure >>> out how to get it to compile with the O3 cpu model. If you're interested, >>> I can send that out as well. >>> >>> Brad >>> >>> >>> -----Original Message----- >>> From: [email protected] [mailto:[email protected]] On Behalf >>> Of Steve Reinhardt >>> Sent: Thursday, March 18, 2010 4:24 PM >>> To: M5 Developer List >>> Subject: Re: [m5-dev] [PATCH 07 of 31] m5: Fixed request read bug flagged >>> by Valgrind >>> >>> It seems very odd that translateTiming would delete the request >>> object... can you point out where that happens? I couldn't find it in >>> the alpha or x86 tlb code. >>> >>> Thanks, >>> >>> Steve >>> >>> On Thu, Mar 18, 2010 at 3:46 PM, Brad Beckmann <[email protected]> >>> wrote: >>>> # HG changeset patch >>>> # User Brad Beckmann <[email protected]> >>>> # Date 1268941825 25200 >>>> # Node ID c2db9da78da715a22d76e92e0cabbced82dcef9f >>>> # Parent d657b9a0875113dbb453986391661fb3c7fa669c >>>> m5: Fixed request read bug flagged by Valgrind >>>> >>>> Previously the recording of an uncached read occurred after the request was >>>> possibly deleted within the translateTiming function. >>>> >>>> diff --git a/src/cpu/simple/timing.cc b/src/cpu/simple/timing.cc >>>> --- a/src/cpu/simple/timing.cc >>>> +++ b/src/cpu/simple/timing.cc >>>> @@ -432,6 +432,10 @@ >>>> Addr split_addr = roundDown(addr + data_size - 1, block_size); >>>> assert(split_addr <= addr || split_addr - addr < block_size); >>>> >>>> + // This will need a new way to tell if it's hooked up to a cache or >>>> not. >>>> + if (req->isUncacheable()) >>>> + recordEvent("Uncached Write"); >>>> + >>>> _status = DTBWaitResponse; >>>> if (split_addr > addr) { >>>> RequestPtr req1, req2; >>>> @@ -461,10 +465,6 @@ >>>> traceData->setAddr(addr); >>>> } >>>> >>>> - // This will need a new way to tell if it has a dcache attached. >>>> - if (req->isUncacheable()) >>>> - recordEvent("Uncached Read"); >>>> - >>>> return NoFault; >>>> } >>>> >>>> @@ -510,7 +510,6 @@ >>>> return read(addr, *(uint32_t*)&data, flags); >>>> } >>>> >>>> - >>>> template<> >>>> Fault >>>> TimingSimpleCPU::read(Addr addr, int32_t &data, unsigned flags) >>>> @@ -555,6 +554,10 @@ >>>> Addr split_addr = roundDown(addr + data_size - 1, block_size); >>>> assert(split_addr <= addr || split_addr - addr < block_size); >>>> >>>> + // This will need a new way to tell if it's hooked up to a cache or >>>> not. >>>> + if (req->isUncacheable()) >>>> + recordEvent("Uncached Write"); >>>> + >>>> T *dataP = new T; >>>> *dataP = TheISA::htog(data); >>>> _status = DTBWaitResponse; >>>> @@ -586,10 +589,6 @@ >>>> traceData->setData(data); >>>> } >>>> >>>> - // This will need a new way to tell if it's hooked up to a cache or >>>> not. >>>> - if (req->isUncacheable()) >>>> - recordEvent("Uncached Write"); >>>> - >>>> // If the write needs to have a fault on the access, consider calling >>>> // changeStatus() and changing it to "bad addr write" or something. >>>> return NoFault; >>>> >>>> _______________________________________________ >>>> m5-dev mailing list >>>> [email protected] >>>> http://m5sim.org/mailman/listinfo/m5-dev >>>> >>> _______________________________________________ >>> m5-dev mailing list >>> [email protected] >>> http://m5sim.org/mailman/listinfo/m5-dev >>> >>> >>> _______________________________________________ >>> m5-dev mailing list >>> [email protected] >>> http://m5sim.org/mailman/listinfo/m5-dev >>> >> > _______________________________________________ > m5-dev mailing list > [email protected] > http://m5sim.org/mailman/listinfo/m5-dev > > > _______________________________________________ > m5-dev mailing list > [email protected] > http://m5sim.org/mailman/listinfo/m5-dev > _______________________________________________ m5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/m5-dev
