From 0fe554b3d116d01a9b4f01dcfdff9bfac23e423b Mon Sep 17 00:00:00 2001 From: Kawe Mazidjatari <48657826+Mauler125@users.noreply.github.com> Date: Thu, 4 Aug 2022 00:21:48 +0200 Subject: [PATCH] Fix crash cases when setting ConVar string using SetValue Added IsMaterialThreadSetAllowed and QueueMaterialThreadSetValue checks. --- r5dev/engine/server/sv_rcon.cpp | 11 ++-- r5dev/public/include/iconvar.h | 1 + r5dev/tier1/IConVar.cpp | 110 ++++++++++++++++++++------------ r5dev/tier1/IConVar.h | 18 +++--- r5dev/tier1/cvar.cpp | 24 +++++++ r5dev/tier1/cvar.h | 5 ++ 6 files changed, 115 insertions(+), 54 deletions(-) diff --git a/r5dev/engine/server/sv_rcon.cpp b/r5dev/engine/server/sv_rcon.cpp index e2ec24e4..26d3ddfb 100644 --- a/r5dev/engine/server/sv_rcon.cpp +++ b/r5dev/engine/server/sv_rcon.cpp @@ -483,12 +483,15 @@ void CRConServer::ProcessMessage(const cl_rcon::request& cl_request) //----------------------------------------------------------------------------- void CRConServer::Execute(const cl_rcon::request& cl_request, bool bConVar) const { - ConVar* pConVar = g_pCVar->FindVar(cl_request.requestbuf().c_str()); - if (pConVar) // Set value without running the callback. + if (bConVar) { - pConVar->SetValue(cl_request.requestval().c_str()); + ConVar* pConVar = g_pCVar->FindVar(cl_request.requestbuf().c_str()); + if (pConVar) // Set value without running the callback. + { + pConVar->SetValue(cl_request.requestval().c_str()); + } } - else if (!bConVar) // Execute command with "". + else // Execute command with "". { Cbuf_AddText(Cbuf_GetCurrentPlayer(), cl_request.requestbuf().c_str(), cmd_source_t::kCommandSrcCode); Cbuf_Execute(); diff --git a/r5dev/public/include/iconvar.h b/r5dev/public/include/iconvar.h index a214c440..49c6f68d 100644 --- a/r5dev/public/include/iconvar.h +++ b/r5dev/public/include/iconvar.h @@ -56,6 +56,7 @@ class CCommand; #define FCVAR_SERVER_CANNOT_QUERY (1<<29) // If this is set, then the server is not allowed to query this cvar's value (via IServerPluginHelpers::StartQueryCvarValue). #define FCVAR_CLIENTCMD_CAN_EXECUTE (1<<30) // IVEngineClient::ClientCmd is allowed to execute this command. +#define FCVAR_MATERIAL_THREAD_MASK ( FCVAR_RELOAD_MATERIALS | FCVAR_RELOAD_TEXTURES | FCVAR_MATERIAL_SYSTEM_THREAD ) /* class ConVar : ConCommandBase, IConVar; [MI] (#classinformer) dq offset ? ? _R4ConVar@@6B@; const ConVar::`RTTI Complete Object Locator' diff --git a/r5dev/tier1/IConVar.cpp b/r5dev/tier1/IConVar.cpp index 1348dae6..e00f3cac 100644 --- a/r5dev/tier1/IConVar.cpp +++ b/r5dev/tier1/IConVar.cpp @@ -279,7 +279,7 @@ void ConVar::PurgeHostNames(void) const { if (ConVar* pCVar = g_pCVar->FindVar(pszHostNames[i])) { - pCVar->ChangeStringValue("0.0.0.0"); + pCVar->SetValue("0.0.0.0"); } } } @@ -512,14 +512,17 @@ void ConVar::SetValue(Color value) //----------------------------------------------------------------------------- void ConVar::InternalSetValue(const char* pszValue) { - if (strcmp(this->m_pParent->m_Value.m_pszString, pszValue) == 0) + if (IsFlagSet(this, FCVAR_MATERIAL_THREAD_MASK)) { - return; + if (g_pCVar && !g_pCVar->IsMaterialThreadSetAllowed()) + { + g_pCVar->QueueMaterialThreadSetValue(this, pszValue); + return; + } } - this->m_pParent->m_Value.m_pszString = pszValue; - char szTempValue[32]{}; - const char* pszNewValue{}; + char szTempValue[32]; + const char* pszNewValue; // Only valid for root convars. assert(m_pParent == this); @@ -533,7 +536,9 @@ void ConVar::InternalSetValue(const char* pszValue) if (!SetColorFromString(pszValue)) { // Not a color, do the standard thing - float flNewValue = static_cast(atof(pszValue)); + double dblValue = atof(pszValue); // Use double to avoid 24-bit restriction on integers and allow storing timestamps or dates in convars + float flNewValue = static_cast(dblValue); + if (!IsFinite(flNewValue)) { Warning(eDLL_T::ENGINE, "Warning: ConVar '%s' = '%s' is infinite, clamping value.\n", GetBaseName(), pszValue); @@ -566,6 +571,15 @@ void ConVar::InternalSetIntValue(int nValue) if (nValue == m_Value.m_nValue) return; + if (IsFlagSet(this, FCVAR_MATERIAL_THREAD_MASK)) + { + if (g_pCVar && !g_pCVar->IsMaterialThreadSetAllowed()) + { + g_pCVar->QueueMaterialThreadSetValue(this, nValue); + return; + } + } + assert(m_pParent == this); // Only valid for root convars. float fValue = static_cast(nValue); @@ -595,6 +609,15 @@ void ConVar::InternalSetFloatValue(float flValue) if (flValue == m_Value.m_fValue) return; + if (IsFlagSet(this, FCVAR_MATERIAL_THREAD_MASK)) + { + if (g_pCVar && !g_pCVar->IsMaterialThreadSetAllowed()) + { + g_pCVar->QueueMaterialThreadSetValue(this, flValue); + return; + } + } + assert(m_pParent == this); // Only valid for root convars. // Check bounds @@ -742,52 +765,57 @@ bool ConVar::SetColorFromString(const char* pszValue) //----------------------------------------------------------------------------- void ConVar::ChangeStringValue(const char* pszTempVal) { - ConVar_ChangeStringValue(this, pszTempVal); + assert(!(m_nFlags & FCVAR_NEVER_AS_STRING)); - //assert(!(m_nFlags & FCVAR_NEVER_AS_STRING)); + char* pszOldValue = reinterpret_cast(_malloca(m_Value.m_iStringLength)); + if (pszOldValue != nullptr) + { + memcpy(pszOldValue, m_Value.m_pszString, m_Value.m_iStringLength); + } - //char* pszOldValue = reinterpret_cast(_malloca(m_Value.m_iStringLength)); - //if (pszOldValue != nullptr) - //{ - // memcpy(pszOldValue, m_Value.m_pszString, m_Value.m_iStringLength); - //} + if (pszTempVal) + { + size_t len = strlen(pszTempVal) + 1; + if (len > m_Value.m_iStringLength) + { + if (m_Value.m_pszString) + { + MemAllocSingleton()->Free(m_Value.m_pszString); + } - //if (pszTempVal) - //{ - // size_t len = strlen(pszTempVal) + 1; + m_Value.m_pszString = MemAllocSingleton()->Alloc(len); + m_Value.m_iStringLength = len; + } + else if (!m_Value.m_pszString) + { + m_Value.m_pszString = MemAllocSingleton()->Alloc(len); + m_Value.m_iStringLength = len; + } + memmove(m_Value.m_pszString, pszTempVal, len); - // if (len > m_Value.m_iStringLength) - // { - // if (m_Value.m_pszString) - // { - // MemAllocSingleton()->Free(m_Value.m_pszString); - // } + /***** + !FIXME: + Respawn put additional code here which + seems to itterate over a 64bit integer + to call the callback several times (as many times as m_iCallbackCount?). + ******/ + } + else + { + m_Value.m_pszString = nullptr; + } - // m_Value.m_pszString = MemAllocSingleton()->Alloc(len); - // m_Value.m_iStringLength = len; - // } - // else if (!m_Value.m_pszString) - // { - // m_Value.m_pszString = MemAllocSingleton()->Alloc(len); - // m_Value.m_iStringLength = len; - // } - // memmove(const_cast(m_Value.m_pszString), pszTempVal, len); - //} - //else - //{ - // m_Value.m_pszString = nullptr; - //} - - //pszOldValue = nullptr; + pszOldValue = nullptr; } //----------------------------------------------------------------------------- -// Purpose: changes the ConVar string value (only use if the new string is equal or lower than this->m_iStringLength). +// Purpose: changes the ConVar string value (this is faster than ChangeStringValue, +// only use if the new string is equal or lower than this->m_iStringLength). // Input : *pszTempVal - flOldValue //----------------------------------------------------------------------------- void ConVar::ChangeStringValueUnsafe(const char* pszNewValue) { - m_Value.m_pszString = pszNewValue; + m_Value.m_pszString = const_cast(pszNewValue); } //----------------------------------------------------------------------------- diff --git a/r5dev/tier1/IConVar.h b/r5dev/tier1/IConVar.h index 22f0f272..2e0c946a 100644 --- a/r5dev/tier1/IConVar.h +++ b/r5dev/tier1/IConVar.h @@ -70,17 +70,17 @@ public: struct CVValue_t { - const char* m_pszString; - size_t m_iStringLength; - float m_fValue; - int m_nValue; + char* m_pszString; + size_t m_iStringLength; + float m_fValue; + int m_nValue; }; struct CVCallback_t { - void** m_ppCallback; - int64_t m_iFlags; - char m_Pad[8]; - int64_t m_iTimesChanged; + void** m_ppCallback; + int64_t m_iFlags; + char m_Pad[8]; + int64_t m_iCallbackCount; }; IConVar* m_pIConVarVFTable{}; //0x0040 @@ -91,7 +91,7 @@ public: float m_fMinVal {}; //0x0074 bool m_bHasMax {}; //0x0078 float m_fMaxVal {}; //0x007C - CVCallback_t m_Callback {}; //0x0080 + CVCallback_t m_Callback {}; //0x0080 // <-- !FIXME: 'CUtlVector< FnChangeCallback_t > m_fnChangeCallbacks;' }; //Size: 0x00A0 /* ==== ICONVAR ========================================================================================================================================================= */ diff --git a/r5dev/tier1/cvar.cpp b/r5dev/tier1/cvar.cpp index 7f1943d3..99f3496e 100644 --- a/r5dev/tier1/cvar.cpp +++ b/r5dev/tier1/cvar.cpp @@ -243,5 +243,29 @@ unordered_map CCVar::DumpToMap(void) return allConVars; } +//----------------------------------------------------------------------------- +// Purpose: deal with queued material system ConVars +//----------------------------------------------------------------------------- +bool CCVar::IsMaterialThreadSetAllowed(void) +{ + const int index = 280; + return CallVFunc(index, this); +} +void CCVar::QueueMaterialThreadSetValue(ConVar* pConVar, float flValue) +{ + const int index = 288; + CallVFunc(index, this, pConVar, flValue); +} +void CCVar::QueueMaterialThreadSetValue(ConVar* pConVar, int nValue) +{ + const int index = 296; + CallVFunc(index, this, pConVar, nValue); +} +void CCVar::QueueMaterialThreadSetValue(ConVar* pConVar, const char* pValue) +{ + const int index = 304; + CallVFunc(index, this, pConVar, pValue); +} + /////////////////////////////////////////////////////////////////////////////// CCVar* g_pCVar = nullptr; diff --git a/r5dev/tier1/cvar.h b/r5dev/tier1/cvar.h index 1cbd4e10..ca34e9f9 100644 --- a/r5dev/tier1/cvar.h +++ b/r5dev/tier1/cvar.h @@ -182,6 +182,11 @@ public: ConCommand* FindCommand(const char* pszCommandName); CCVarIteratorInternal* FactoryInternalIterator(void); unordered_map DumpToMap(void); + + bool IsMaterialThreadSetAllowed(void); + void QueueMaterialThreadSetValue(ConVar* pConVar, float flValue); + void QueueMaterialThreadSetValue(ConVar* pConVar, int nValue); + void QueueMaterialThreadSetValue(ConVar* pConVar, const char* pValue); }; ///////////////////////////////////////////////////////////////////////////////