From d40ed0f82614fdc5bf70b6dadeb5658e08cd6d9f Mon Sep 17 00:00:00 2001
From: Gabriel Busnot <gabriel.busnot@arteris.com>
Date: Thu, 2 Feb 2023 08:31:07 +0000
Subject: [PATCH] mem: Deprecate RequestPort and ResponsePort owner ref member

The reference can be bound to an invalid object (*nullptr) in
situations where no proper owner SimObject can be provided to the port
constructor. This rightfully triggers a UBSAN warning.

Also, these two classes do not make use of the owner reference member
themselves and expose it as a protected member reference to
subclasses. This desing has several drawbacks: requires the reference
to owner to travel the class hierarchy up and down, loosing its true
static type in the process ; non-private member variable should not be
part of the API of such fundamental classes, if only for
maintainability ; a reference bound from a nullable pointer is a lying
API as it hides the optional aspect of ownership.

Note that the reference to invalid object can't be properly fixed until
the complete removal of the owner reference. This patch lays the path
toward that fix.

Change-Id: I8b42bc57d7826656726f7708492c43366f20633a
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/67551
Reviewed-by: Bobby Bruce <bbruce@ucdavis.edu>
Tested-by: kokoro <noreply+kokoro@google.com>
Maintainer: Bobby Bruce <bbruce@ucdavis.edu>
Reviewed-by: Jason Lowe-Power <power.jg@gmail.com>
Maintainer: Jason Lowe-Power <power.jg@gmail.com>
---
 src/mem/port.cc | 47 +++++++++++++++++++++++++++++++++++++++++------
 src/mem/port.hh | 18 +++++++++++++-----
 2 files changed, 54 insertions(+), 11 deletions(-)

diff --git a/src/mem/port.cc b/src/mem/port.cc
index 18793d487b..e36323fb74 100644
--- a/src/mem/port.cc
+++ b/src/mem/port.cc
@@ -120,9 +120,23 @@ DefaultResponsePort defaultResponsePort;
 /**
  * Request port
  */
-RequestPort::RequestPort(const std::string& name, SimObject* _owner,
-    PortID _id) : Port(name, _id), _responsePort(&defaultResponsePort),
-    owner(*_owner)
+[[deprecated]]
+RequestPort::RequestPort(const std::string& name,
+                         SimObject* _owner,
+                         PortID _id):
+    Port(name, _id), _responsePort(&defaultResponsePort), owner{*_owner}
+{
+}
+
+/*** FIXME:
+ * The owner reference member is going through a deprecation path. In the
+ * meantime, it must be initialized but no valid reference is available here.
+ * Using 1 instead of nullptr prevents warning upon dereference. It should be
+ * OK until definitive removal of owner.
+ */
+RequestPort::RequestPort(const std::string& name, PortID _id) :
+    Port(name, _id), _responsePort(&defaultResponsePort),
+    owner{*reinterpret_cast<SimObject*>(1)}
 {
 }
 
@@ -175,9 +189,30 @@ RequestPort::printAddr(Addr a)
 /**
  * Response port
  */
-ResponsePort::ResponsePort(const std::string& name, SimObject* _owner,
-    PortID id) : Port(name, id), _requestPort(&defaultRequestPort),
-    defaultBackdoorWarned(false), owner(*_owner)
+
+[[deprecated]]
+ResponsePort::ResponsePort(const std::string& name,
+                           SimObject* _owner,
+                           PortID _id):
+    Port(name, _id),
+    _requestPort(&defaultRequestPort),
+    defaultBackdoorWarned(false),
+    owner{*_owner}
+{
+}
+
+
+/*** FIXME:
+ * The owner reference member is going through a deprecation path. In the
+ * meantime, it must be initialized but no valid reference is available here.
+ * Using 1 instead of nullptr prevents warning upon dereference. It should be
+ * OK until definitive removal of owner.
+ */
+ResponsePort::ResponsePort(const std::string& name, PortID id) :
+    Port(name, id),
+    _requestPort(&defaultRequestPort),
+    defaultBackdoorWarned(false),
+    owner{*reinterpret_cast<SimObject*>(1)}
 {
 }
 
diff --git a/src/mem/port.hh b/src/mem/port.hh
index fb0f4b8812..0d61787f62 100644
--- a/src/mem/port.hh
+++ b/src/mem/port.hh
@@ -86,8 +86,13 @@ class RequestPort: public Port, public AtomicRequestProtocol,
     SimObject &owner;
 
   public:
+    [[deprecated("RequestPort ownership is deprecated. "
+                 "Owner should now be registered in derived classes.")]]
     RequestPort(const std::string& name, SimObject* _owner,
-               PortID id=InvalidPortID);
+                PortID id=InvalidPortID);
+
+    RequestPort(const std::string& name, PortID id=InvalidPortID);
+
     virtual ~RequestPort();
 
     /**
@@ -266,9 +271,7 @@ class RequestPort: public Port, public AtomicRequestProtocol,
 class [[deprecated]] MasterPort : public RequestPort
 {
   public:
-    MasterPort(const std::string& name, SimObject* _owner,
-               PortID id=InvalidPortID) : RequestPort(name, _owner, id)
-               {}
+    using RequestPort::RequestPort;
 };
 
 /**
@@ -294,8 +297,13 @@ class ResponsePort : public Port, public AtomicResponseProtocol,
     SimObject& owner;
 
   public:
+    [[deprecated("ResponsePort ownership is deprecated. "
+                 "Owner should now be registered in derived classes.")]]
     ResponsePort(const std::string& name, SimObject* _owner,
-              PortID id=InvalidPortID);
+                 PortID id=InvalidPortID);
+
+    ResponsePort(const std::string& name, PortID id=InvalidPortID);
+
     virtual ~ResponsePort();
 
     /**