Skip to content

Commit 8d6eb30

Browse files
committed
Improve crash fix from 20a14e8
1 parent 2580ae3 commit 8d6eb30

File tree

1 file changed

+59
-24
lines changed

1 file changed

+59
-24
lines changed

Client/game_sa/CRenderWareSA.TextureReplacing.cpp

Lines changed: 59 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -359,25 +359,54 @@ namespace
359359
return pTexture && SharedUtil::IsReadablePointer(pTexture, sizeof(*pTexture));
360360
}
361361

362-
[[nodiscard]] size_t GetTextureNameLength(const RwTexture* const pTexture)
362+
// Safe wrapper for RwTexDictionaryFindNamedTexture that validates TXD structure
363+
// Prevents crash at 0x003F3A17
364+
[[nodiscard]] RwTexture* SafeFindNamedTexture(RwTexDictionary* pTxd, const char* name)
363365
{
364-
if (!pTexture)
365-
return 0;
366+
if (!pTxd || !name)
367+
return nullptr;
368+
369+
if (!IsValidTexDictionary(pTxd))
370+
return nullptr;
366371

372+
RwListEntry* firstNode = pTxd->textures.root.next;
373+
374+
if (!firstNode)
375+
return nullptr;
376+
377+
// Empty list check (valid case - no textures in TXD)
378+
if (firstNode == &pTxd->textures.root)
379+
return nullptr;
380+
381+
// Validate first texture using container_of pattern
382+
// SA will iterate the list and dereference textures, so we need to ensure at least the first one is valid
383+
RwTexture* firstTexture = reinterpret_cast<RwTexture*>(reinterpret_cast<char*>(firstNode) - offsetof(RwTexture, TXDList));
384+
if (!IsValidTexturePtr(firstTexture))
385+
return nullptr;
386+
387+
// Structure validated, safe to call SA native function
388+
return RwTexDictionaryFindNamedTexture(pTxd, name);
389+
}
390+
391+
// Fast path: caller guarantees pTexture is valid (already validated with IsValidTexturePtr)
392+
[[nodiscard]] __forceinline std::string_view GetTextureNameViewUnsafe(const RwTexture* const pTexture) noexcept
393+
{
367394
const char* p = pTexture->name;
368395
const char* end = p + RW_TEXTURE_NAME_LENGTH;
369396
while (p < end && *p != '\0')
370397
++p;
371-
return static_cast<size_t>(p - pTexture->name);
398+
399+
const size_t length = static_cast<size_t>(p - pTexture->name);
400+
return std::string_view(pTexture->name, length);
372401
}
373402

403+
// Safe path: validates pointer before dereferencing
374404
[[nodiscard]] std::string_view GetTextureNameView(const RwTexture* const pTexture)
375405
{
376-
if (!pTexture)
406+
if (!IsValidTexturePtr(pTexture))
377407
return {};
378408

379-
const size_t length = GetTextureNameLength(pTexture);
380-
return std::string_view(pTexture->name, length);
409+
return GetTextureNameViewUnsafe(pTexture);
381410
}
382411

383412
TextureIndex::Name ExtractTextureName(const RwTexture* pTexture)
@@ -413,7 +442,7 @@ namespace
413442
for (auto& record : info.originalTextures)
414443
{
415444
if (!IsValidTexturePtr(record.texture) && !record.name.Empty())
416-
record.texture = RwTexDictionaryFindNamedTexture(pTxd, record.name.CStr());
445+
record.texture = SafeFindNamedTexture(pTxd, record.name.CStr());
417446
}
418447
}
419448

@@ -441,7 +470,7 @@ namespace
441470

442471
auto& record = info.originalTextures[idx];
443472
if (!IsValidTexturePtr(record.texture) && canReload && !record.name.Empty())
444-
record.texture = RwTexDictionaryFindNamedTexture(pTxd, record.name.CStr());
473+
record.texture = SafeFindNamedTexture(pTxd, record.name.CStr());
445474

446475
return IsValidTexturePtr(record.texture) ? record.texture : nullptr;
447476
};
@@ -469,7 +498,7 @@ namespace
469498
if (lookupName.Empty())
470499
return nullptr;
471500

472-
RwTexture* pFound = RwTexDictionaryFindNamedTexture(pTxd, lookupName.CStr());
501+
RwTexture* pFound = SafeFindNamedTexture(pTxd, lookupName.CStr());
473502
if (!pFound)
474503
return nullptr;
475504

@@ -529,7 +558,7 @@ namespace
529558
continue;
530559

531560
RwTexture* pOriginalTexture = (idx < perTxdInfo.replacedOriginals.size()) ? perTxdInfo.replacedOriginals[idx] : nullptr;
532-
const std::string_view replacementName = GetTextureNameView(pReplacementTexture);
561+
const std::string_view replacementName = GetTextureNameViewUnsafe(pReplacementTexture); // Already validated above
533562
RwTexture* pResolvedOriginal = ResolveOriginalTexture(info, pTxd, pOriginalTexture, replacementName);
534563

535564
if (pResolvedOriginal)
@@ -670,7 +699,11 @@ namespace
670699
// TXD was unloaded - try to reload it
671700
guard.info->pTxd = CTxdStore_GetTxd(usTxdId);
672701

673-
// If still invalid, the cached entry is stale and needs rebuild
702+
// Refresh cached texture pointers after TXD reload
703+
// This prevents stale texture pointers from causing crashes
704+
RefreshOriginalTextureCache(*guard.info, guard.info->pTxd);
705+
706+
// If still invalid after refresh, the cached entry is stale and needs rebuild
674707
if (!IsValidTexDictionary(guard.info->pTxd))
675708
{
676709
// Invalidate the entry - caller should handle this
@@ -746,7 +779,7 @@ namespace
746779
CRenderWareSA::GetTxdTextures(originals, pTxd);
747780

748781
// If texture enumeration failed
749-
if (std::empty(originals))
782+
if (originals.empty())
750783
{
751784
ms_ModelTexturesInfoMap.erase(insertIt);
752785
guard.info = nullptr;
@@ -783,7 +816,7 @@ bool CRenderWareSA::ModelInfoTXDLoadTextures(SReplacementTextures* pReplacementT
783816
return false;
784817

785818
// Are we already loaded?
786-
if (!std::empty(pReplacementTextures->ownedTextures))
819+
if (!pReplacementTextures->ownedTextures.empty())
787820
return false;
788821

789822
// Try to load it
@@ -916,7 +949,8 @@ bool CRenderWareSA::ModelInfoTXDAddTextures(SReplacementTextures* pReplacementTe
916949
bool cloneFailure = false;
917950
for (RwTexture* pSourceTexture : pReplacementTextures->textures)
918951
{
919-
if (!pSourceTexture)
952+
// Validate texture pointer before dereferencing its fields
953+
if (!IsValidTexturePtr(pSourceTexture))
920954
{
921955
cloneFailure = true;
922956
break;
@@ -975,15 +1009,16 @@ bool CRenderWareSA::ModelInfoTXDAddTextures(SReplacementTextures* pReplacementTe
9751009
dassert(bHasValidTxd);
9761010
for (RwTexture* pNewTexture : pPerTxdInfo->usingTextures)
9771011
{
978-
const std::string_view replacementName = GetTextureNameView(pNewTexture);
1012+
// All textures in usingTextures were validated during preparation phase
1013+
const std::string_view replacementName = GetTextureNameViewUnsafe(pNewTexture);
9791014

9801015
RwTexture* pExistingTexture = LookupOriginalTexture(*pInfo, replacementName);
981-
if (!pExistingTexture && bHasValidTxd && !std::empty(replacementName))
1016+
if (!pExistingTexture && bHasValidTxd && !replacementName.empty())
9821017
{
9831018
// Create null-terminated name for safe call
9841019
TextureIndex::Name safeName;
9851020
safeName.Assign(replacementName);
986-
pExistingTexture = RwTexDictionaryFindNamedTexture(pInfo->pTxd, safeName.CStr());
1021+
pExistingTexture = SafeFindNamedTexture(pInfo->pTxd, safeName.CStr());
9871022
}
9881023

9891024
if (pExistingTexture && bHasValidTxd)
@@ -1024,7 +1059,7 @@ bool CRenderWareSA::ModelInfoTXDAddTextures(SReplacementTextures* pReplacementTe
10241059
pInfo->usedByReplacements.push_back(pReplacementTextures);
10251060

10261061
ValidateUsageTracking(*pReplacementTextures);
1027-
return !std::empty(pPerTxdInfo->usingTextures);
1062+
return !pPerTxdInfo->usingTextures.empty();
10281063
}
10291064

10301065
////////////////////////////////////////////////////////////////
@@ -1040,7 +1075,7 @@ void CRenderWareSA::ModelInfoTXDRemoveTextures(SReplacementTextures* pReplacemen
10401075
if (!pReplacementTextures)
10411076
return;
10421077

1043-
std::scoped_lock guard(ms_ModelTexturesInfoMutex);
1078+
std::lock_guard<std::recursive_mutex> guard(ms_ModelTexturesInfoMutex);
10441079
// For each using txd
10451080
for (std::size_t i = 0; i < pReplacementTextures->perTxdList.size(); ++i)
10461081
{
@@ -1103,7 +1138,7 @@ void CRenderWareSA::ModelInfoTXDRemoveTextures(SReplacementTextures* pReplacemen
11031138
continue;
11041139

11051140
// Use the already-safe record.name instead of pOriginalTexture->name
1106-
if (!record.name.Empty() && !RwTexDictionaryFindNamedTexture(pTxd, record.name.CStr()))
1141+
if (!record.name.Empty() && !SafeFindNamedTexture(pTxd, record.name.CStr()))
11071142
{
11081143
RwTexture* const pAddedTexture = RwTexDictionaryAddTexture(pTxd, pOriginalTexture);
11091144
dassert(pAddedTexture == pOriginalTexture);
@@ -1115,7 +1150,7 @@ void CRenderWareSA::ModelInfoTXDRemoveTextures(SReplacementTextures* pReplacemen
11151150
{
11161151
ListRemove(pInfo->usedByReplacements, pReplacementTextures);
11171152

1118-
if (std::empty(pInfo->usedByReplacements))
1153+
if (pInfo->usedByReplacements.empty())
11191154
{
11201155
CTxdStore_RemoveRef(pInfo->usTxdId);
11211156
ms_ModelTexturesInfoMap.erase(usTxdId);
@@ -1149,11 +1184,11 @@ void CRenderWareSA::ModelInfoTXDRemoveTextures(SReplacementTextures* pReplacemen
11491184
/////////////////////////////////////////////////////////////
11501185
void CRenderWareSA::ModelInfoTXDCleanupOrphanedEntries()
11511186
{
1152-
std::scoped_lock lock(ms_ModelTexturesInfoMutex);
1187+
std::lock_guard<std::recursive_mutex> lock(ms_ModelTexturesInfoMutex);
11531188

11541189
for (auto it = ms_ModelTexturesInfoMap.begin(); it != ms_ModelTexturesInfoMap.end();)
11551190
{
1156-
if (std::empty(it->second.usedByReplacements))
1191+
if (it->second.usedByReplacements.empty())
11571192
{
11581193
CTxdStore_RemoveRef(it->first);
11591194
it = ms_ModelTexturesInfoMap.erase(it);

0 commit comments

Comments
 (0)