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.

5 files changed, 19 insertions, 18 deletions

diff --git a/src/mem/probes/base.hh b/src/mem/probes/base.hh
index 7a0a7ea05..ccb5ca749 100644
--- a/src/mem/probes/base.hh
+++ b/src/mem/probes/base.hh
@@ -43,8 +43,7 @@
 #include <memory>
 #include <vector>
 
-#include "mem/packet.hh"
-#include "sim/probe/probe.hh"
+#include "sim/probe/mem.hh"
 #include "sim/sim_object.hh"
 
 struct BaseMemProbeParams;
@@ -72,10 +71,10 @@ class BaseMemProbe : public SimObject
     /**
      * Callback to analyse intercepted Packets.
      */
-    virtual void handleRequest(const PacketPtr &pkt) = 0;
+    virtual void handleRequest(const ProbePoints::PacketInfo &pkt_info) = 0;
 
   private:
-    class PacketListener : public ProbeListenerArgBase<PacketPtr>
+    class PacketListener : public ProbeListenerArgBase<ProbePoints::PacketInfo>
     {
       public:
         PacketListener(BaseMemProbe &_parent,
@@ -83,8 +82,8 @@ class BaseMemProbe : public SimObject
             : ProbeListenerArgBase(pm, name),
               parent(_parent) {}
 
-        void notify(const PacketPtr &pkt) M5_ATTR_OVERRIDE {
-            parent.handleRequest(pkt);
+        void notify(const ProbePoints::PacketInfo &pkt_info) M5_ATTR_OVERRIDE {
+            parent.handleRequest(pkt_info);
         }
 
       protected:
diff --git a/src/mem/probes/mem_trace.cc b/src/mem/probes/mem_trace.cc
index 6bf6f7f77..121c6de48 100644
--- a/src/mem/probes/mem_trace.cc
+++ b/src/mem/probes/mem_trace.cc
@@ -93,15 +93,15 @@ MemTraceProbe::closeStreams()
 }
 
 void
-MemTraceProbe::handleRequest(const PacketPtr &pkt)
+MemTraceProbe::handleRequest(const ProbePoints::PacketInfo &pkt_info)
 {
     ProtoMessage::Packet pkt_msg;
 
     pkt_msg.set_tick(curTick());
-    pkt_msg.set_cmd(pkt->cmdToIndex());
-    pkt_msg.set_flags(pkt->req->getFlags());
-    pkt_msg.set_addr(pkt->getAddr());
-    pkt_msg.set_size(pkt->getSize());
+    pkt_msg.set_cmd(pkt_info.cmd.toInt());
+    pkt_msg.set_flags(pkt_info.flags);
+    pkt_msg.set_addr(pkt_info.addr);
+    pkt_msg.set_size(pkt_info.size);
 
     traceStream->write(pkt_msg);
 }
diff --git a/src/mem/probes/mem_trace.hh b/src/mem/probes/mem_trace.hh
index 677b8ae32..51f272812 100644
--- a/src/mem/probes/mem_trace.hh
+++ b/src/mem/probes/mem_trace.hh
@@ -52,7 +52,8 @@ class MemTraceProbe : public BaseMemProbe
     MemTraceProbe(MemTraceProbeParams *params);
 
   protected:
-    void handleRequest(const PacketPtr &pkt) M5_ATTR_OVERRIDE;
+    void handleRequest(const ProbePoints::PacketInfo &pkt_info) \
+        M5_ATTR_OVERRIDE;
 
     /**
      * Callback to flush and close all open output streams on exit. If
diff --git a/src/mem/probes/stack_dist.cc b/src/mem/probes/stack_dist.cc
index c742cae7b..a447f49e5 100644
--- a/src/mem/probes/stack_dist.cc
+++ b/src/mem/probes/stack_dist.cc
@@ -94,15 +94,15 @@ StackDistProbe::regStats()
 }
 
 void
-StackDistProbe::handleRequest(const PacketPtr &pkt)
+StackDistProbe::handleRequest(const ProbePoints::PacketInfo &pkt_info)
 {
     // only capturing read and write requests (which allocate in the
     // cache)
-    if (!pkt->isRead() && !pkt->isWrite())
+    if (!pkt_info.cmd.isRead() && !pkt_info.cmd.isWrite())
         return;
 
     // Align the address to a cache line size
-    const Addr aligned_addr(roundDown(pkt->getAddr(), lineSize));
+    const Addr aligned_addr(roundDown(pkt_info.addr, lineSize));
 
     // Calculate the stack distance
     const uint64_t sd(calc.calcStackDistAndUpdate(aligned_addr).first);
@@ -113,7 +113,7 @@ StackDistProbe::handleRequest(const PacketPtr &pkt)
 
     // Sample the stack distance of the address in linear bins
     if (!disableLinearHists) {
-        if (pkt->isRead())
+        if (pkt_info.cmd.isRead())
             readLinearHist.sample(sd);
         else
             writeLinearHist.sample(sd);
@@ -123,7 +123,7 @@ StackDistProbe::handleRequest(const PacketPtr &pkt)
         int sd_lg2 = sd == 0 ? 1 : floorLog2(sd);
 
         // Sample the stack distance of the address in log bins
-        if (pkt->isRead())
+        if (pkt_info.cmd.isRead())
             readLogHist.sample(sd_lg2);
         else
             writeLogHist.sample(sd_lg2);
diff --git a/src/mem/probes/stack_dist.hh b/src/mem/probes/stack_dist.hh
index 210800894..8374672da 100644
--- a/src/mem/probes/stack_dist.hh
+++ b/src/mem/probes/stack_dist.hh
@@ -55,7 +55,8 @@ class StackDistProbe : public BaseMemProbe
     void regStats() M5_ATTR_OVERRIDE;
 
   protected:
-    void handleRequest(const PacketPtr &pkt) M5_ATTR_OVERRIDE;
+    void handleRequest(const ProbePoints::PacketInfo &pkt_info) \
+        M5_ATTR_OVERRIDE;
 
   protected:
     // Cache line size to simulate