Gabriel B. has submitted this change. (
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
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/67551
Reviewed-by: Bobby Bruce <bbr...@ucdavis.edu>
Tested-by: kokoro <noreply+kok...@google.com>
Maintainer: Bobby Bruce <bbr...@ucdavis.edu>
Reviewed-by: Jason Lowe-Power <power...@gmail.com>
Maintainer: Jason Lowe-Power <power...@gmail.com>
---
M src/mem/port.cc
M src/mem/port.hh
2 files changed, 86 insertions(+), 11 deletions(-)
Approvals:
Jason Lowe-Power: Looks good to me, but someone else must approve; Looks
good to me, approved
kokoro: Regressions pass
Bobby Bruce: Looks good to me, approved; Looks good to me, approved
diff --git a/src/mem/port.cc b/src/mem/port.cc
index 18793d4..e36323f 100644
--- a/src/mem/port.cc
+++ b/src/mem/port.cc
@@ -120,9 +120,23 @@
/**
* 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 @@
/**
* 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 fb0f4b8..0d61787 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,7 @@
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 @@
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: 5
Gerrit-Owner: Gabriel B. <gabriel.bus...@arteris.com>
Gerrit-Reviewer: Andreas Sandberg <andreas.sandb...@arm.com>
Gerrit-Reviewer: Bobby Bruce <bbr...@ucdavis.edu>
Gerrit-Reviewer: Daniel Carvalho <oda...@yahoo.com.br>
Gerrit-Reviewer: Gabe Black <gabe.bl...@gmail.com>
Gerrit-Reviewer: Gabriel B. <gabriel.bus...@arteris.com>
Gerrit-Reviewer: Giacomo Travaglini <giacomo.travagl...@arm.com>
Gerrit-Reviewer: Jason Lowe-Power <ja...@lowepower.com>
Gerrit-Reviewer: Jason Lowe-Power <power...@gmail.com>
Gerrit-Reviewer: Nikos Nikoleris <nikos.nikole...@arm.com>
Gerrit-Reviewer: kokoro <noreply+kok...@google.com>
Gerrit-MessageType: merged
_______________________________________________
gem5-dev mailing list -- gem5-dev@gem5.org
To unsubscribe send an email to gem5-dev-le...@gem5.org
