From b400cf145499b8d0cda654938f5fc658bf929df9 Mon Sep 17 00:00:00 2001
From: Christoph Oelckers <c.oelckers@zdoom.fake>
Date: Fri, 30 Sep 2016 10:50:41 +0200
Subject: [PATCH] - added an integrity check to the SNDINFO parser to detect
 and eliminate recursive links. Normally these would crash the sound code
 later. - allow recursive linking of $random definitions (as long as they do
 not link back, see above.) - fixed the sound precaching which did not handle
 $alias inside $random. Normally this went undetected but in cases where the
 random sound index was the same as a sound index in the current link chain
 this could hang the function.

---
 src/s_advsound.cpp | 103 +++++++++++++++++++++++++++++++++++++++++++--
 src/s_sound.cpp    |  15 ++++---
 2 files changed, 107 insertions(+), 11 deletions(-)

diff --git a/src/s_advsound.cpp b/src/s_advsound.cpp
index b073070fd..c1c19aabf 100644
--- a/src/s_advsound.cpp
+++ b/src/s_advsound.cpp
@@ -51,6 +51,7 @@
 #include "i_system.h"
 #include "d_player.h"
 #include "serializer.h"
+#include "v_text.h"
 
 // MACROS ------------------------------------------------------------------
 
@@ -326,6 +327,99 @@ void S_HashSounds ()
 	}
 }
 
+//==========================================================================
+//
+// S_CheckIntegrity
+//
+// Scans the entire sound list and looks for recursive definitions.
+//==========================================================================
+
+static bool S_CheckSound(sfxinfo_t *startsfx, sfxinfo_t *sfx, TArray<sfxinfo_t *> &chain)
+{
+	sfxinfo_t *me = sfx;
+	bool success = true;
+	unsigned siz = chain.Size();
+
+	if (sfx->bPlayerReserve)
+	{
+		return true;
+	}
+
+	// There is a bad link in here, but let's report it only for the sound that contains the broken definition.
+	// Once that sound has been disabled this one will work again.
+	if (chain.Find(sfx) < chain.Size())
+	{
+		return true;
+	}
+	chain.Push(sfx);
+
+	if (me->bRandomHeader)
+	{
+		const FRandomSoundList *list = &S_rnd[me->link];
+		for (int i = 0; i < list->NumSounds; ++i)
+		{
+			auto rsfx = &S_sfx[list->Sounds[i]];
+			if (rsfx == startsfx)
+			{
+				Printf(TEXTCOLOR_RED "recursive sound $random found for %s:\n", startsfx->name);
+				success = false;
+				for (unsigned i = 1; i<chain.Size(); i++)
+				{
+					Printf(TEXTCOLOR_ORANGE "    -> %s\n", chain[i]->name);
+				}
+			}
+			else
+			{
+				success &= S_CheckSound(startsfx, rsfx, chain);
+			}
+		}
+	}
+	else if (me->link != sfxinfo_t::NO_LINK)
+	{
+		me = &S_sfx[me->link];
+		if (me == startsfx)
+		{
+			Printf(TEXTCOLOR_RED "recursive sound $alias found for %s:\n", startsfx->name);
+			success = false;
+			for (unsigned i = 1; i<chain.Size(); i++)
+			{
+				Printf(TEXTCOLOR_ORANGE "    -> %s\n", chain[i]->name);
+			}
+			chain.Resize(siz);
+		}
+		else
+		{
+			success &= S_CheckSound(startsfx, me, chain);
+		}
+	}
+	chain.Pop();
+	return success;
+}
+
+void S_CheckIntegrity()
+{
+	TArray<sfxinfo_t *> chain;
+	TArray<bool> broken;
+
+	broken.Resize(S_sfx.Size());
+	memset(&broken[0], 0, sizeof(bool)*S_sfx.Size());
+	for (unsigned i = 0; i < S_sfx.Size(); i++)
+	{
+		auto &sfx = S_sfx[i];
+		broken[i] = !S_CheckSound(&sfx, &sfx, chain);
+	}
+	for (unsigned i = 0; i < S_sfx.Size(); i++)
+	{
+		if (broken[i])
+		{
+			auto &sfx = S_sfx[i];
+			Printf(TEXTCOLOR_RED "Sound %s has been disabled\n", sfx.name);
+			sfx.bRandomHeader = false;
+			sfx.link = 0;	// link to the empty sound.
+		}
+	}
+}
+
 //==========================================================================
 //
 // S_PickReplacement
@@ -334,13 +428,12 @@ void S_HashSounds ()
 // is not the head of a random list, then the sound passed is returned.
 //==========================================================================
 
-int S_PickReplacement (int refid)
+int S_PickReplacement(int refid)
 {
-	if (S_sfx[refid].bRandomHeader)
+	while (S_sfx[refid].bRandomHeader)
 	{
 		const FRandomSoundList *list = &S_rnd[S_sfx[refid].link];
-
-		return list->Sounds[pr_randsound() % list->NumSounds];
+		refid = list->Sounds[pr_randsound() % list->NumSounds];
 	}
 	return refid;
 }
@@ -941,6 +1034,7 @@ void S_ParseSndInfo (bool redefine)
 	S_ShrinkPlayerSoundLists ();
 
 	sfx_empty = Wads.CheckNumForName ("dsempty", ns_sounds);
+	S_CheckIntegrity();
 }
 
 //==========================================================================
@@ -961,6 +1055,7 @@ void S_AddLocalSndInfo(int lump)
 	}
 
 	S_ShrinkPlayerSoundLists ();
+	S_CheckIntegrity();
 }
 
 //==========================================================================
diff --git a/src/s_sound.cpp b/src/s_sound.cpp
index 06df0dbad..524b12175 100644
--- a/src/s_sound.cpp
+++ b/src/s_sound.cpp
@@ -522,18 +522,19 @@ void S_CacheSound (sfxinfo_t *sfx)
 		{
 			return;
 		}
-		else if (sfx->bRandomHeader)
+		sfxinfo_t *orig = sfx;
+		while (!sfx->bRandomHeader && sfx->link != sfxinfo_t::NO_LINK)
 		{
-			S_CacheRandomSound (sfx);
+			sfx = &S_sfx[sfx->link];
+		}
+		if (sfx->bRandomHeader)
+		{
+			S_CacheRandomSound(sfx);
 		}
 		else
 		{
-			while (sfx->link != sfxinfo_t::NO_LINK)
-			{
-				sfx = &S_sfx[sfx->link];
-			}
+			S_LoadSound(sfx);
 			sfx->bUsed = true;
-			S_LoadSound (sfx);
 		}
 	}
 }