mem: Add PacketInfo to be used for packet probe points

This patch fixes a use-after-delete issue in the packet probe points
by adding a PacketInfo struct to retain the key fields before passing
the packet onwards. We want to probe the packet after it is
successfully sent, but by that time the fields may be modified, and
the packet may even be deleted.

Amazingly enough the issue has gone undetected for months, and only
recently popped up in our regressions.

1 files changed, 18 insertions, 24 deletions

diff --git a/src/mem/comm_monitor.cc b/src/mem/comm_monitor.cc
index beb7fe32c..85d7be2f4 100644
--- a/src/mem/comm_monitor.cc
+++ b/src/mem/comm_monitor.cc
@@ -115,11 +115,13 @@ CommMonitor::recvFunctionalSnoop(PacketPtr pkt)
 Tick
 CommMonitor::recvAtomic(PacketPtr pkt)
 {
-    ppPktReq->notify(pkt);
+    ProbePoints::PacketInfo req_pkt_info(pkt);
+    ppPktReq->notify(req_pkt_info);
 
     const Tick delay(masterPort.sendAtomic(pkt));
     assert(pkt->isResponse());
-    ppPktResp->notify(pkt);
+    ProbePoints::PacketInfo resp_pkt_info(pkt);
+    ppPktResp->notify(resp_pkt_info);
     return delay;
 }
 
@@ -137,11 +139,10 @@ CommMonitor::recvTimingReq(PacketPtr pkt)
 
     // Store relevant fields of packet, because packet may be modified
     // or even deleted when sendTiming() is called.
+    const ProbePoints::PacketInfo pkt_info(pkt);
+
     const bool is_read = pkt->isRead();
     const bool is_write = pkt->isWrite();
-    const MemCmd cmd = pkt->cmd;
-    const unsigned size = pkt->getSize();
-    const Addr addr = pkt->getAddr();
     const bool expects_response(
         pkt->needsResponse() && !pkt->memInhibitAsserted());
 
@@ -163,14 +164,7 @@ CommMonitor::recvTimingReq(PacketPtr pkt)
     }
 
     if (successful) {
-        // The receiver might already have modified the packet. We
-        // want to give the probe access to the original packet, which
-        // means we need to fake the original packet by temporarily
-        // restoring the command.
-        const MemCmd response_cmd(pkt->cmd);
-        pkt->cmd = cmd;
-        ppPktReq->notify(pkt);
-        pkt->cmd = response_cmd;
+        ppPktReq->notify(pkt_info);
     }
 
     if (successful && is_read) {
@@ -183,12 +177,12 @@ CommMonitor::recvTimingReq(PacketPtr pkt)
 
         // Get sample of burst length
         if (!stats.disableBurstLengthHists) {
-            stats.readBurstLengthHist.sample(size);
+            stats.readBurstLengthHist.sample(pkt_info.size);
         }
 
         // Sample the masked address
         if (!stats.disableAddrDists) {
-            stats.readAddrDist.sample(addr & readAddrMask);
+            stats.readAddrDist.sample(pkt_info.addr & readAddrMask);
         }
 
         // If it needs a response increment number of outstanding read
@@ -219,18 +213,18 @@ CommMonitor::recvTimingReq(PacketPtr pkt)
         }
 
         if (!stats.disableBurstLengthHists) {
-            stats.writeBurstLengthHist.sample(size);
+            stats.writeBurstLengthHist.sample(pkt_info.size);
         }
 
         // Update the bandwidth stats on the request
         if (!stats.disableBandwidthHists) {
-            stats.writtenBytes += size;
-            stats.totalWrittenBytes += size;
+            stats.writtenBytes += pkt_info.size;
+            stats.totalWrittenBytes += pkt_info.size;
         }
 
         // Sample the masked write address
         if (!stats.disableAddrDists) {
-            stats.writeAddrDist.sample(addr & writeAddrMask);
+            stats.writeAddrDist.sample(pkt_info.addr & writeAddrMask);
         }
 
         if (!stats.disableOutstandingHists && expects_response) {
@@ -265,9 +259,10 @@ CommMonitor::recvTimingResp(PacketPtr pkt)
 
     // Store relevant fields of packet, because packet may be modified
     // or even deleted when sendTiming() is called.
+    const ProbePoints::PacketInfo pkt_info(pkt);
+
     bool is_read = pkt->isRead();
     bool is_write = pkt->isWrite();
-    unsigned size = pkt->getSize();
     Tick latency = 0;
     CommMonitorSenderState* received_state =
         dynamic_cast<CommMonitorSenderState*>(pkt->senderState);
@@ -299,8 +294,7 @@ CommMonitor::recvTimingResp(PacketPtr pkt)
     }
 
     if (successful) {
-        assert(pkt->isResponse());
-        ppPktResp->notify(pkt);
+        ppPktResp->notify(pkt_info);
     }
 
     if (successful && is_read) {
@@ -317,8 +311,8 @@ CommMonitor::recvTimingResp(PacketPtr pkt)
 
         // Update the bandwidth stats based on responses for reads
         if (!stats.disableBandwidthHists) {
-            stats.readBytes += size;
-            stats.totalReadBytes += size;
+            stats.readBytes += pkt_info.size;
+            stats.totalReadBytes += pkt_info.size;
         }
 
     } else if (successful && is_write) {