feat(Core): Added ABORT() macro to prevent the usage of ASSERT(false)…

… as a quick hack to crash the core misusing assert (#2273)
azerothcore · Sep 26, 2019 · 854b426 · 854b426
1 parent 58f3cfe
commit 854b426
Show file tree

Hide file tree

Showing 28 changed files with 127 additions and 74 deletions.
diff --git a/src/common/Collision/Management/MMapManager.cpp b/src/common/Collision/Management/MMapManager.cpp
@@ -228,7 +228,7 @@ namespace MMAP
             // if the grid is later reloaded, dtNavMesh::addTile will return error but no extra memory is used
             // we cannot recover from this error - assert out
             sLog->outError("MMAP:unloadMap: Could not unload %03u%02i%02i.mmtile from navmesh", mapId, x, y);
-            ASSERT(false);
+            ABORT();
         }
         else
         {

diff --git a/src/common/Debugging/Errors.cpp b/src/common/Debugging/Errors.cpp
@@ -6,26 +6,45 @@
 
 #include "Errors.h"
 
-#include <ace/Stack_Trace.h>
-#include <ace/OS_NS_unistd.h>
+#include <thread>
 #include <cstdlib>
 
 namespace Trinity {
 
 void Assert(char const* file, int line, char const* function, char const* message)
 {
-    ACE_Stack_Trace st;
-    fprintf(stderr, "\n%s:%i in %s ASSERTION FAILED:\n  %s\n%s\n",
-            file, line, function, message, st.c_str());
+    fprintf(stderr, "\n%s:%i in %s ASSERTION FAILED:\n  %s\n",
+            file, line, function, message);
     *((volatile int*)NULL) = 0;
     exit(1);
 }
 
-void Fatal(char const* file, int line, char const* function, char const* message)
+void Assert(char const* file, int line, char const* function, char const* message, char const* format, ...)
 {
-    fprintf(stderr, "\n%s:%i in %s FATAL ERROR:\n  %s\n",
-                   file, line, function, message);
-    ACE_OS::sleep(10);
+    va_list args;
+    va_start(args, format);
+
+    fprintf(stderr, "\n%s:%i in %s ASSERTION FAILED:\n  %s ", file, line, function, message);
+    vfprintf(stderr, format, args);
+    fprintf(stderr, "\n");
+    fflush(stderr);
+
+    va_end(args);
+    *((volatile int*)NULL) = 0;
+    exit(1);
+}
+
+void Fatal(char const* file, int line, char const* function, char const* message, ...)
+{
+    va_list args;
+    va_start(args, message);
+
+    fprintf(stderr, "\n%s:%i in %s FATAL ERROR:\n  ", file, line, function);
+    vfprintf(stderr, message, args);
+    fprintf(stderr, "\n");
+    fflush(stderr);
+
+    std::this_thread::sleep_for(std::chrono::seconds(10));
     *((volatile int*)NULL) = 0;
     exit(1);
 }
@@ -44,4 +63,20 @@ void Warning(char const* file, int line, char const* function, char const* messa
                    file, line, function, message);
 }
 
+void Abort(char const* file, int line, char const* function)
+{
+    fprintf(stderr, "\n%s:%i in %s ABORTED.\n",
+                   file, line, function);
+    *((volatile int*)NULL) = 0;
+    exit(1);
+}
+
+void AbortHandler(int /*sigval*/)
+{
+    // nothing useful to log here, no way to pass args
+    *((volatile int*)NULL) = 0;
+    exit(1);
+}
+
+
 } // namespace Trinity
diff --git a/src/common/Debugging/Errors.h b/src/common/Debugging/Errors.h
@@ -13,26 +13,44 @@ namespace Trinity
 {
 
     DECLSPEC_NORETURN void Assert(char const* file, int line, char const* function, char const* message) ATTR_NORETURN;
+    DECLSPEC_NORETURN void Assert(char const* file, int line, char const* function, char const* message, char const* format, ...) ATTR_NORETURN ATTR_PRINTF(5, 6);
 
-    DECLSPEC_NORETURN void Fatal(char const* file, int line, char const* function, char const* message) ATTR_NORETURN;
+    DECLSPEC_NORETURN void Fatal(char const* file, int line, char const* function, char const* message, ...) ATTR_NORETURN ATTR_PRINTF(4, 5);
 
     DECLSPEC_NORETURN void Error(char const* file, int line, char const* function, char const* message) ATTR_NORETURN;
 
+    DECLSPEC_NORETURN void Abort(char const* file, int line, char const* function) ATTR_NORETURN;
+
     void Warning(char const* file, int line, char const* function, char const* message);
 
+    DECLSPEC_NORETURN void AbortHandler(int sigval) ATTR_NORETURN;
+
 } // namespace Trinity
 
-#define WPAssert(cond) do { if (!(cond)) Trinity::Assert(__FILE__, __LINE__, __FUNCTION__, #cond); } while (0)
-#define WPFatal(cond, msg) do { if (!(cond)) Trinity::Fatal(__FILE__, __LINE__, __FUNCTION__, (msg)); } while (0)
-#define WPError(cond, msg) do { if (!(cond)) Trinity::Error(__FILE__, __LINE__, __FUNCTION__, (msg)); } while (0)
-#define WPWarning(cond, msg) do { if (!(cond)) Trinity::Warning(__FILE__, __LINE__, __FUNCTION__, (msg)); } while (0)
+#if COMPILER == COMPILER_MICROSOFT
+#define ASSERT_BEGIN __pragma(warning(push)) __pragma(warning(disable: 4127))
+#define ASSERT_END __pragma(warning(pop))
+#else
+#define ASSERT_BEGIN
+#define ASSERT_END
+#endif
+
+#define WPAssert(cond, ...) ASSERT_BEGIN do { if (!(cond)) Trinity::Assert(__FILE__, __LINE__, __FUNCTION__, #cond, ##__VA_ARGS__); } while(0) ASSERT_END
+#define WPFatal(cond, ...) ASSERT_BEGIN do { if (!(cond)) Trinity::Fatal(__FILE__, __LINE__, __FUNCTION__, ##__VA_ARGS__); } while(0) ASSERT_END
+#define WPError(cond, msg) ASSERT_BEGIN do { if (!(cond)) Trinity::Error(__FILE__, __LINE__, __FUNCTION__, (msg)); } while(0) ASSERT_END
+#define WPWarning(cond, msg) ASSERT_BEGIN do { if (!(cond)) Trinity::Warning(__FILE__, __LINE__, __FUNCTION__, (msg)); } while(0) ASSERT_END
+#define WPAbort() ASSERT_BEGIN do { Trinity::Abort(__FILE__, __LINE__, __FUNCTION__); } while(0) ASSERT_END
 
 #define ASSERT WPAssert
+#define ABORT WPAbort
 
-template <typename T> inline T* ASSERT_NOTNULL(T* pointer)
+template <typename T>
+inline T* ASSERT_NOTNULL_IMPL(T* pointer, char const* expr)
 {
-    ASSERT(pointer);
+    ASSERT(pointer, "%s", expr);
     return pointer;
 }
 
-#endif
+#define ASSERT_NOTNULL(pointer) ASSERT_NOTNULL_IMPL(pointer, #pointer)
+
+#endif
diff --git a/src/common/Utilities/Util.h b/src/common/Utilities/Util.h
@@ -542,7 +542,7 @@ bool CompareValues(ComparisionType type, T val1, T val2)
             return val1 <= val2;
         default:
             // incorrect parameter
-            ASSERT(false);
+            ABORT();
             return false;
     }
 }

diff --git a/src/server/game/Battlegrounds/Battleground.cpp b/src/server/game/Battlegrounds/Battleground.cpp
@@ -1168,7 +1168,7 @@ void Battleground::Init()
     if (m_BgInvitedPlayers[TEAM_ALLIANCE] > 0 || m_BgInvitedPlayers[TEAM_HORDE] > 0)
     {
         sLog->outError("Battleground::Reset: one of the counters is not 0 (alliance: %u, horde: %u) for BG (map: %u, instance id: %u)!", m_BgInvitedPlayers[TEAM_ALLIANCE], m_BgInvitedPlayers[TEAM_HORDE], m_MapId, m_InstanceID);
-        ASSERT(false);
+        ABORT();
     }
 
     m_BgInvitedPlayers[TEAM_ALLIANCE] = 0;

diff --git a/src/server/game/Battlegrounds/BattlegroundMgr.cpp b/src/server/game/Battlegrounds/BattlegroundMgr.cpp
@@ -1024,7 +1024,7 @@ void RandomBattlegroundSystem::Update(uint32 diff)
             case BATTLEGROUND_EY: m_SwitchTimer = 40*IN_MILLISECONDS; break; // max 15 per team
             case BATTLEGROUND_AB: m_SwitchTimer = 40*IN_MILLISECONDS; break; // max 15 per team
             case BATTLEGROUND_SA: m_SwitchTimer = 40*IN_MILLISECONDS; break; // max 15 per team
-            default: ASSERT(false); break;
+            default: ABORT(); break;
         }
     }
     else

diff --git a/src/server/game/Battlegrounds/BattlegroundQueue.cpp b/src/server/game/Battlegrounds/BattlegroundQueue.cpp
@@ -265,7 +265,7 @@ void BattlegroundQueue::RemovePlayer(uint64 guid, bool sentToBg, uint32 playerQu
     QueuedPlayersMap::iterator itr = m_QueuedPlayers.find(guid);
     if (itr == m_QueuedPlayers.end())
     {
-        ASSERT(false);
+        ABORT();
         return;
     }
 
@@ -286,7 +286,7 @@ void BattlegroundQueue::RemovePlayer(uint64 guid, bool sentToBg, uint32 playerQu
     //player can't be in queue without group, but just in case
     if (group_itr == m_QueuedGroups[_bracketId][_groupType].end())
     {
-        ASSERT(false);
+        ABORT();
         return;
     }
 

diff --git a/src/server/game/Battlegrounds/Zones/BattlegroundAV.cpp b/src/server/game/Battlegrounds/Zones/BattlegroundAV.cpp
@@ -842,7 +842,7 @@ BG_AV_Nodes BattlegroundAV::GetNodeThroughObject(uint32 object)
     if (object == BG_AV_OBJECT_FLAG_N_SNOWFALL_GRAVE)
         return BG_AV_NODES_SNOWFALL_GRAVE;
     sLog->outError("BattlegroundAV: ERROR! GetPlace got a wrong object :(");
-    ASSERT(false);
+    ABORT();
     return BG_AV_Nodes(0);
 }
 
@@ -882,7 +882,7 @@ uint32 BattlegroundAV::GetObjectThroughNode(BG_AV_Nodes node)
     else if (m_Nodes[node].OwnerId == TEAM_NEUTRAL)
         return BG_AV_OBJECT_FLAG_N_SNOWFALL_GRAVE;
     sLog->outError("BattlegroundAV: Error! GetPlaceNode couldn't resolve node %i", node);
-    ASSERT(false);
+    ABORT();
     return 0;
 }
 
@@ -1464,22 +1464,22 @@ void BattlegroundAV::AssaultNode(BG_AV_Nodes node, TeamId teamId)
     if (m_Nodes[node].TotalOwnerId == teamId)
     {
         sLog->outCrash("Assaulting team is TotalOwner of node");
-        ASSERT(false);
+        ABORT();
     }
     if (m_Nodes[node].OwnerId == teamId)
     {
         sLog->outCrash("Assaulting team is owner of node");
-        ASSERT(false);
+        ABORT();
     }
     if (m_Nodes[node].State == POINT_DESTROYED)
     {
         sLog->outCrash("Destroyed node is being assaulted");
-        ASSERT(false);
+        ABORT();
     }
     if (m_Nodes[node].State == POINT_ASSAULTED && m_Nodes[node].TotalOwnerId != TEAM_NEUTRAL) //only assault an assaulted node if no totalowner exists
     {
         sLog->outCrash("Assault on an not assaulted node with total owner");
-        ASSERT(false);
+        ABORT();
     }
     //the timer gets another time, if the previous owner was 0 == Neutral
     m_Nodes[node].Timer      = (m_Nodes[node].PrevOwnerId != TEAM_NEUTRAL)? BG_AV_CAPTIME : BG_AV_SNOWFALL_FIRSTCAP;

diff --git a/src/server/game/Battlegrounds/Zones/BattlegroundSA.cpp b/src/server/game/Battlegrounds/Zones/BattlegroundSA.cpp
@@ -825,7 +825,7 @@ bool BattlegroundSA::CanInteractWithObject(uint32 objectId)
                 return false;
             break;
         default:
-            ASSERT(false);
+            ABORT();
             break;
     }
 
@@ -988,7 +988,7 @@ void BattlegroundSA::CaptureGraveyard(BG_SA_Graveyards i, Player *Source)
                 SendWarningToAll(LANG_BG_SA_H_GY_SOUTH);
             break;
         default:
-            ASSERT(false);
+            ABORT();
             break;
     };
 }

diff --git a/src/server/game/Entities/Creature/TemporarySummon.cpp b/src/server/game/Entities/Creature/TemporarySummon.cpp
@@ -389,7 +389,7 @@ void Puppet::InitSummon()
         else
         {
             sLog->outMisc("Puppet::InitSummon (B1)");
-            //ASSERT(false); // ZOMG!
+            //ABORT(); // ZOMG!
         }
     }
 }

diff --git a/src/server/game/Entities/GameObject/GameObject.h b/src/server/game/Entities/GameObject/GameObject.h
@@ -700,7 +700,7 @@ class GameObject : public WorldObject, public GridObject<GameObject>, public Mov
             // Owner already found and different than expected owner - remove object from old owner
             if (owner && GetOwnerGUID() && GetOwnerGUID() != owner)
             {
-                ASSERT(false);
+                ABORT();
             }
             m_spawnedByDefault = false;                     // all object with owner is despawned after delay
             SetUInt64Value(OBJECT_FIELD_CREATED_BY, owner);

diff --git a/src/server/game/Entities/Item/Item.cpp b/src/server/game/Entities/Item/Item.cpp
@@ -1036,7 +1036,7 @@ Item* Item::CreateItem(uint32 item, uint32 count, Player const* player)
             delete pItem;
     }
     else
-        ASSERT(false);
+        ABORT();
     return NULL;
 }
 

diff --git a/src/server/game/Entities/Object/Object.cpp b/src/server/game/Entities/Object/Object.cpp
@@ -92,7 +92,7 @@ WorldObject::~WorldObject()
         if (GetTypeId() == TYPEID_CORPSE)
         {
             sLog->outCrash("Object::~Object Corpse guid=" UI64FMTD ", type=%d, entry=%u deleted but still in map!!", GetGUID(), ((Corpse*)this)->GetType(), GetEntry());
-            ASSERT(false);
+            ABORT();
         }
         ResetMap();
     }
@@ -105,14 +105,14 @@ Object::~Object()
         sLog->outCrash("Object::~Object - guid=" UI64FMTD ", typeid=%d, entry=%u deleted but still in world!!", GetGUID(), GetTypeId(), GetEntry());
         if (isType(TYPEMASK_ITEM))
             sLog->outCrash("Item slot %u", ((Item*)this)->GetSlot());
-        ASSERT(false);
+        ABORT();
         RemoveFromWorld();
     }
 
     if (m_objectUpdated)
     {
         sLog->outCrash("Object::~Object - guid=" UI64FMTD ", typeid=%d, entry=%u deleted but still in update list!!", GetGUID(), GetTypeId(), GetEntry());
-        ASSERT(false);
+        ABORT();
         sObjectAccessor->RemoveUpdateObject(this);
     }
 
@@ -2059,7 +2059,7 @@ void WorldObject::SetMap(Map* map)
     if (m_currMap)
     {
         sLog->outCrash("WorldObject::SetMap: obj %u new map %u %u, old map %u %u", (uint32)GetTypeId(), map->GetId(), map->GetInstanceId(), m_currMap->GetId(), m_currMap->GetInstanceId());
-        ASSERT(false);
+        ABORT();
     }
     m_currMap = map;
     m_mapId = map->GetId();

diff --git a/src/server/game/Entities/Pet/Pet.h b/src/server/game/Entities/Pet/Pet.h
@@ -180,11 +180,11 @@ class Pet : public Guardian
     private:
         void SaveToDB(uint32, uint8, uint32)                // override of Creature::SaveToDB     - must not be called
         {
-            ASSERT(false);
+            ABORT();
         }
         void DeleteFromDB()                                 // override of Creature::DeleteFromDB - must not be called
         {
-            ASSERT(false);
+            ABORT();
         }
 };
 #endif
diff --git a/src/server/game/Entities/Player/Player.cpp b/src/server/game/Entities/Player/Player.cpp
@@ -3890,7 +3890,7 @@ bool Player::_addSpell(uint32 spellId, uint8 addSpecMask, bool temporary)
                 {
                     sLog->outString("TRYING TO LEARN SPELL WITH EFFECT LEARN: %u, PLAYER: %u", spellId, GetGUIDLow());
                     return false;
-                    //ASSERT(false);
+                    //ABORT();
                 }
                 else if (const SpellInfo* learnSpell = sSpellMgr->GetSpellInfo(spellInfo->Effects[i].TriggerSpell))
                     _addSpell(learnSpell->Id, SPEC_MASK_ALL, true);
@@ -3943,7 +3943,7 @@ bool Player::_addSpell(uint32 spellId, uint8 addSpecMask, bool temporary)
         sLog->outString("TRYING TO LEARN SPELL WITH EFFECT LEARN 2: %u, PLAYER: %u", spellId, GetGUIDLow());
         m_spells.erase(spellInfo->Id); // mem leak, but should never happen
         return false;
-        //ASSERT(false);
+        //ABORT();
     }
     // pussywizard: cast passive spells (including all talents without SPELL_EFFECT_LEARN_SPELL) with additional checks
     else if (spellInfo->IsPassive() || (spellInfo->HasAttribute(SPELL_ATTR0_HIDDEN_CLIENTSIDE) && spellInfo->Stances))
@@ -20905,7 +20905,7 @@ void Player::StopCastingCharm()
         if (charm->GetCharmerGUID())
         {
             sLog->outCrash("Charmed unit has charmer guid " UI64FMTD, charm->GetCharmerGUID());
-            ASSERT(false);
+            ABORT();
         }
         else
             SetCharm(charm, false);
@@ -24521,7 +24521,7 @@ void Player::SetBattlegroundOrBattlefieldRaid(Group *group, int8 subgroup)
     if (GetGroup() && (GetGroup()->isBGGroup() || GetGroup()->isBFGroup()))
     {
         sLog->outMisc("Player::SetBattlegroundOrBattlefieldRaid - current group is %s group!", (GetGroup()->isBGGroup() ? "BG" : "BF"));
-        //ASSERT(false); // pussywizard: origanal group can never be bf/bg group
+        //ABORT(); // pussywizard: origanal group can never be bf/bg group
     }
 
     SetOriginalGroup(GetGroup(), GetSubGroup());