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>
This commit is contained in:
Gabriel Busnot
2023-02-02 08:31:07 +00:00
committed by Gabriel B.
parent c1b1a702f9
commit d40ed0f826
2 changed files with 54 additions and 11 deletions

View File

@@ -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)}
{
}

View File

@@ -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();
/**