Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion GeneralsMD/Code/GameEngine/Include/GameLogic/Object.h
Original file line number Diff line number Diff line change
Expand Up @@ -761,7 +761,7 @@ class Object : public Thing, public Snapshot

Object* m_containedBy; /**< an object can only be contained by at most one
other object, this is that object (if present) */
ObjectID m_xferContainedByID; ///< xfer uses IDs to store pointers and looks them up after
ObjectID m_containedByID; ///< ID of the object we're contained by; only to be used when m_containedBy cannot be used
UnsignedInt m_containedByFrame; ///< frame we were contained by m_containedBy

Real m_constructionPercent; ///< for objects being built ... this is the amount completed (0.0 to 100.0)
Expand Down
64 changes: 55 additions & 9 deletions GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Object.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,7 @@ Object::Object( const ThingTemplate *tt, const ObjectStatusMaskType &objectStatu
m_physics(nullptr),
m_geometryInfo(tt->getTemplateGeometryInfo()),
m_containedBy(nullptr),
m_xferContainedByID(INVALID_ID),
m_containedByID(INVALID_ID),
m_containedByFrame(0),
m_behaviors(nullptr),
m_body(nullptr),
Expand Down Expand Up @@ -691,6 +691,22 @@ void Object::onContainedBy( Object *containedBy )
m_containedBy = containedBy;
m_containedByFrame = TheGameLogic->getFrame();

#if RETAIL_COMPATIBLE_CRC
// TheSuperHackers @info Set INVALID_ID if the container object was destroyed
// to indicate that the pointer will become a dangling pointer in the next frame.
if (containedBy && !containedBy->isDestroyed())
Comment thread
xezon marked this conversation as resolved.
{
m_containedByID = containedBy->getID();
}
else
{
m_containedByID = INVALID_ID;
Comment thread
xezon marked this conversation as resolved.
}
#else
DEBUG_ASSERTCRASH(containedBy == nullptr || !containedBy->isDestroyed(),
("Object::onContainedBy - Adding into a destroyed container"));
#endif
Comment thread
Caball009 marked this conversation as resolved.

handlePartitionCellMaintenance(); // which should unlook me now that I am contained

}
Expand All @@ -704,6 +720,10 @@ void Object::onRemovedFrom( Object *removedFrom )
m_containedBy = nullptr;
m_containedByFrame = 0;

#if RETAIL_COMPATIBLE_CRC
m_containedByID = INVALID_ID;
#endif

handlePartitionCellMaintenance(); // get a clean look, now that I am outdoors, again

}
Expand Down Expand Up @@ -756,9 +776,33 @@ void Object::onDestroy()
{

// This is the old cleanUpContain safeguard. Say goodbye so they don't try to look us up.
if( m_containedBy && m_containedBy->getContain() )
if (m_containedBy)
Comment thread
Caball009 marked this conversation as resolved.
{
m_containedBy->getContain()->removeFromContain( this );
#if RETAIL_COMPATIBLE_CRC
if (m_containedByID == INVALID_ID)
{
// TheSuperHackers @bugfix Caball009 25/05/2026 Due to a potential use-after-free bug that cannot be fixed
// with retail compatibility, the 'contained by' pointer of this object may point to an already destroyed object.
// Avoid removing this object from the contain list, because it could crash the game,
// as the begin / end iterator for STLPort and MSVC std::list implementations depends on dynamically allocated memory.
DEBUG_CRASH(("container object must be valid; this looks like use-after-free"));
}
else
{
DEBUG_ASSERTCRASH(TheGameLogic->findObjectByID(m_containedByID) == m_containedBy,
("contained by pointer is out of sync with contained by ID"));

if (ContainModuleInterface* contain = m_containedBy->getContain())
{
contain->removeFromContain(this);
}
}
#else
if (ContainModuleInterface* contain = m_containedBy->getContain())
{
contain->removeFromContain(this);
}
#endif
}

//
Expand Down Expand Up @@ -4246,16 +4290,18 @@ void Object::xfer( Xfer *xfer )
// No, the contain module is just going to friend_ reach in and set this for us.
// Containers more complicated than Open (like Tunnel) can't do that. Our variable,
// our responsibility.
#if !RETAIL_COMPATIBLE_CRC
// TheSuperHackers @tweak Contained by ID is already set with retail compatibility; don't overwrite it.
if( xfer->getXferMode() == XFER_SAVE )
Comment thread
xezon marked this conversation as resolved.
{
if( m_containedBy != nullptr )
m_xferContainedByID = m_containedBy->getID();
m_containedByID = m_containedBy->getID();
else
m_xferContainedByID = INVALID_ID;
m_containedByID = INVALID_ID;
}
Comment thread
greptile-apps[bot] marked this conversation as resolved.
#endif


xfer->xferObjectID( &m_xferContainedByID );
xfer->xferObjectID( &m_containedByID );
}

// contained by frame
Expand Down Expand Up @@ -4480,8 +4526,8 @@ void Object::xfer( Xfer *xfer )
//-------------------------------------------------------------------------------------------------
void Object::loadPostProcess()
{
if( m_xferContainedByID != INVALID_ID )
m_containedBy = TheGameLogic->findObjectByID(m_xferContainedByID);
if( m_containedByID != INVALID_ID )
m_containedBy = TheGameLogic->findObjectByID(m_containedByID);
else
m_containedBy = nullptr;

Expand Down
Loading