Gabriel B. has uploaded this change for review. ( https://gem5-review.googlesource.com/c/public/gem5/+/67551?usp=email )

Change subject: mem: Deprecate RequestPort and ResponsePort owner ref member
......................................................................

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
---
M src/mem/port.cc
M src/mem/port.hh
2 files changed, 79 insertions(+), 11 deletions(-)



diff --git a/src/mem/port.cc b/src/mem/port.cc
index 18793d4..36a32f9 100644
--- a/src/mem/port.cc
+++ b/src/mem/port.cc
@@ -120,9 +120,22 @@
 /**
  * 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.
+ * Let's trigger a big dirty warning until definitive owner member removal.
+ */
+RequestPort::RequestPort(const std::string& name, PortID _id) :
+    Port(name, _id), _responsePort(&defaultResponsePort),
+    owner{*static_cast<SimObject*>(nullptr)}
 {
 }

@@ -175,9 +188,28 @@
 /**
  * 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.
+ * Let's trigger a big dirty warning until definitive owner member removal.
+ */
+ResponsePort::ResponsePort(const std::string& name, PortID id) :
+    Port(name, id),
+    _requestPort(&defaultRequestPort),
+    defaultBackdoorWarned(false),
+    owner{*static_cast<SimObject*>(nullptr)}
 {
 }

diff --git a/src/mem/port.hh b/src/mem/port.hh
index fb0f4b8..c16eb1d 100644
--- a/src/mem/port.hh
+++ b/src/mem/port.hh
@@ -86,8 +86,13 @@
     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,9 @@
 class [[deprecated]] MasterPort : public RequestPort
 {
   public:
-    MasterPort(const std::string& name, SimObject* _owner,
-               PortID id=InvalidPortID) : RequestPort(name, _owner, id)
-               {}
+    MasterPort(const std::string& name, PortID id=InvalidPortID) :
+        RequestPort(name, id)
+    {}
 };

 /**
@@ -294,8 +299,13 @@
     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();

     /**

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/67551?usp=email To unsubscribe, or for help writing mail filters, visit https://gem5-review.googlesource.com/settings

Gerrit-Project: public/gem5
Gerrit-Branch: develop
Gerrit-Change-Id: I8b42bc57d7826656726f7708492c43366f20633a
Gerrit-Change-Number: 67551
Gerrit-PatchSet: 1
Gerrit-Owner: Gabriel B. <gabriel.bus...@arteris.com>
Gerrit-MessageType: newchange
_______________________________________________
gem5-dev mailing list -- gem5-dev@gem5.org
To unsubscribe send an email to gem5-dev-le...@gem5.org

Reply via email to