cpu, mem: Changing AtomicOpFunctor* for unique_ptr<AtomicOpFunctor>

This change is based on modify the way we move the AtomicOpFunctor*
through gem5 in order to mantain proper ownership of the object and
ensuring its destruction when it is no longer used.

Doing that we fix at the same time a memory leak in Request.hh
where we were assigning a new AtomicOpFunctor* without destroying the
previous one.

This change creates a new type AtomicOpFunctor_ptr as a
std::unique_ptr<AtomicOpFunctor> and move its ownership as needed. Except
for its only usage when AtomicOpFunc() is called.

Change-Id: Ic516f9d8217cb1ae1f0a19500e5da0336da9fd4f
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/20919
Reviewed-by: Andreas Sandberg <andreas.sandberg@arm.com>
Maintainer: Andreas Sandberg <andreas.sandberg@arm.com>
Tested-by: kokoro <noreply+kokoro@google.com>

1 files changed, 12 insertions, 18 deletions

diff --git a/src/mem/request.hh b/src/mem/request.hh
index 324ae382e..50944932e 100644
--- a/src/mem/request.hh
+++ b/src/mem/request.hh
@@ -389,7 +389,7 @@ class Request
     InstSeqNum _reqInstSeqNum;
 
     /** A pointer to an atomic operation */
-    AtomicOpFunctor *atomicOpFunctor;
+    AtomicOpFunctorPtr atomicOpFunctor;
 
   public:
 
@@ -470,9 +470,9 @@ class Request
 
     Request(uint64_t asid, Addr vaddr, unsigned size, Flags flags,
             MasterID mid, Addr pc, ContextID cid,
-            AtomicOpFunctor *atomic_op)
+            AtomicOpFunctorPtr atomic_op)
     {
-        setVirt(asid, vaddr, size, flags, mid, pc, atomic_op);
+        setVirt(asid, vaddr, size, flags, mid, pc, std::move(atomic_op));
         setContext(cid);
     }
 
@@ -489,19 +489,13 @@ class Request
           translateDelta(other.translateDelta),
           accessDelta(other.accessDelta), depth(other.depth)
     {
-        if (other.atomicOpFunctor)
-            atomicOpFunctor = (other.atomicOpFunctor)->clone();
-        else
-            atomicOpFunctor = nullptr;
-    }
 
-    ~Request()
-    {
-        if (hasAtomicOpFunctor()) {
-            delete atomicOpFunctor;
-        }
+        atomicOpFunctor.reset(other.atomicOpFunctor ?
+                                other.atomicOpFunctor->clone() : nullptr);
     }
 
+    ~Request() {}
+
     /**
      * Set up Context numbers.
      */
@@ -533,7 +527,7 @@ class Request
      */
     void
     setVirt(uint64_t asid, Addr vaddr, unsigned size, Flags flags,
-            MasterID mid, Addr pc, AtomicOpFunctor *amo_op = nullptr)
+            MasterID mid, Addr pc, AtomicOpFunctorPtr amo_op = nullptr)
     {
         _asid = asid;
         _vaddr = vaddr;
@@ -549,7 +543,7 @@ class Request
         depth = 0;
         accessDelta = 0;
         translateDelta = 0;
-        atomicOpFunctor = amo_op;
+        atomicOpFunctor = std::move(amo_op);
     }
 
     /**
@@ -669,14 +663,14 @@ class Request
     bool
     hasAtomicOpFunctor()
     {
-        return atomicOpFunctor != NULL;
+        return (bool)atomicOpFunctor;
     }
 
     AtomicOpFunctor *
     getAtomicOpFunctor()
     {
-        assert(atomicOpFunctor != NULL);
-        return atomicOpFunctor;
+        assert(atomicOpFunctor);
+        return atomicOpFunctor.get();
     }
 
     /** Accessor for flags. */