CS:GO Server Crash w/ fuzzy map names & self-compiled SM

[sneak-it]

Help us help you

• I have checked that my issue doesn't exist yet.
• I have tried my absolute best to reduce the problem-space and have provided the absolute smallest test-case possible.
• I can always reproduce the issue with the provided description below.

Environment

• Operating System version: Windows Server 2008 R2
• Game/AppID (with version if applicable): CS:GO/730 (1.36.5.7)
• Current SourceMod version: 1.9.0.6260
• Current SourceMod snapshot: 1.10.6353
• Current Metamod: Source snapshot: 1.11.0.1116

• I have updated SourceMod to the latest version and it still happens.
• I have updated SourceMod to the latest snapshot and it still happens.
• I have updated SourceMM to the latest snapshot and it still happens.

Description

When using a self-compiled version of SourceMod locally on Windows, every version since early October (first noticed, possibly earlier) has caused instant crashes when attempting to use fuzzy map names. Retail SourceMod versions do not have this issue. I'm compiling against up to date versions of hl2sdk and metamod, also building "retail" versions of SM under the master and 1.9-dev branches with no changes. There's likely some relevant information I'm missing here - I'll gladly fill in any necessary blanks!

@jason-e and @Headline have noticed the same issue and might have some more relevant information that I'm missing here. I believe this happens in both Windows and Linux builds.

Problematic Code (or Steps to Reproduce)

Compile latest SourceMod 1.9 or 1.10 locally for Windows, place on server, /nominate and instant crash.

Logs

https://crash.limetech.org/p3kreav6ojxu

[psychonic]

What version of the VS toolchain are you compiling with? This could be an ABI compatibility issue.

[KyleSanderson]

Does this still crash without PTaH?

[sneak-it]

@psychonic commented on Oct 26, 2018, 4:16 PM EDT:

What version of the VS toolchain are you compiling with? This could be an ABI compatibility issue.

MSVC Version 1915

@KyleSanderson commented on Oct 26, 2018, 4:18 PM EDT:

Does this still crash without PTaH?

It crashes without PTaH.

[e54385991]

Windows Server 2016
MSVC Version 1915 same problem

https://crash.limetech.org/cm3lcmopvy7s

[psychonic]

Does the same issue occur using the VS 2015 toolset?

[e54385991]

1 . 1 0 . 0 . 6 3 2 9
When I build the old version, No more crashes

[asherkin]

This is crashing in the CUtlVector's CUtlString's destructor, as the game is allocating memory for the strings then we're freeing it cross-CRT :(

It's gross, but this works:

diff --git a/core/HalfLife2.cpp b/core/HalfLife2.cpp
index 9e2bad5b..d9b8b1c2 100644
--- a/core/HalfLife2.cpp
+++ b/core/HalfLife2.cpp
@@ -1245,9 +1245,10 @@ SMFindMapResult CHalfLife2::FindMap(const char *pMapName, char *pFoundMap, size_

        static size_t helperCmdLen = strlen(pHelperCmd->GetName());

-       CUtlVector<CUtlString> results;
-       pHelperCmd->AutoCompleteSuggest(pMapName, results);
-       if (results.Count() == 0)
+       // Intentionally leak this, it has allocation issues. (#910)
+       CUtlVector<CUtlString> *results = new CUtlVector<CUtlString>();
+       pHelperCmd->AutoCompleteSuggest(pMapName, *results);
+       if (results->Count() == 0)
                return SMFindMapResult::NotFound;

        // Results come back as you'd see in autocomplete. (ie. "changelevel fullmapnamehere"),
@@ -1255,14 +1256,14 @@ SMFindMapResult CHalfLife2::FindMap(const char *pMapName, char *pFoundMap, size_

        // Like the engine, we're only going to deal with the first match.

-       bool bExactMatch = Q_strcmp(pMapName, &results[0][helperCmdLen + 1]) == 0;
+       bool bExactMatch = Q_strcmp(pMapName, &(*results)[0][helperCmdLen + 1]) == 0;
        if (bExactMatch)
        {
                return SMFindMapResult::Found;
        }
        else
        {
-               ke::SafeStrcpy(pFoundMap, nMapNameMax, &results[0][helperCmdLen + 1]);
+               ke::SafeStrcpy(pFoundMap, nMapNameMax, &(*results)[0][helperCmdLen + 1]);
                return SMFindMapResult::FuzzyMatch;
        }

If we wanted to avoid the leak, we'd need to locate a dtor for CUtlVector<CUtlString> or CUtlString and invoke it manually.

[psychonic]

If we wanted to avoid the leak, we'd need to locate a dtor for CUtlVector or CUtlString and invoke it manually.

Or manually iterate, removing from vec and calling g_pMemAlloc->Free() on the strings.

[KyleSanderson]

Is this still used on the next frame? Even if we stored the ptrs on nextlevel they wouldn't be used. Isn't the global memory allocator gone? This was one of the last things Alfred pulled iirc.

[psychonic]

Isn't the global memory allocator gone?

It was removed on POSIX builds, on some engines, and then brought back to them on some (maybe all). This issue is Windows-specific though.