Trinitycore: [Crash] - Spell::SelectImplicitAreaTargets

[Jildor]

Description:
No idea about this crash :(
Related issues: #10468 #7046

Crashlog: https://gist.github.com/Jildor/4ce6e104d6b7a02bfafef2f0923e93ce
Old crashlog (rev about December 2015): https://gist.github.com/Jildor/15100253e3acddde17a065dae5a046fe

Branch(es): 3.3.5

TC rev. hash/commit: 445c5a00e8

TDB version: TDB335.63

Operating system: Debian

[Ovahlord]

Happens when you use a AOE spell and use a target filter that erases all targets from the list. When you don't check for a empty target list then, you will get nullpointers for some reason and you will crash.

[Shauren]

That explanation can't be more wrong - if you erase all targets from list then that code in Spell::SelectImplicitAreaTargets will never execute - https://github.com/TrinityCore/TrinityCore/blob/d23682331b6584aa276a06f1473f2b239c96ea16/src/server/game/Spells/Spell.cpp#L1263

[Jildor]

@Shauren I will try adding:

diff --git a/src/server/game/Spells/Spell.cpp b/src/server/game/Spells/Spell.cpp
index 4818a224b2..94a5d0da53 100644
--- a/src/server/game/Spells/Spell.cpp
+++ b/src/server/game/Spells/Spell.cpp
@@ -7807,6 +7807,12 @@ void Spell::CallScriptObjectAreaTargetSelectHandlers(std::list<WorldObject*>& ta
             if (hookItr->IsEffectAffected(m_spellInfo, effIndex) && targetType.GetTarget() == hookItr->GetTarget())
                 hookItr->Call(*scritr, targets);

+        // DEBUG CODE START
+        for (std::list<WorldObject*>::const_iterator itr = targets.begin(); itr != targets.end(); ++itr)
+            if (!*itr)
+                TC_LOG_ERROR("spells", "Script %s attached to spell %u pushed a NULL target!\n", (*scritr)->_GetScriptName()->c_str(), m_spellInfo->Id);
+        // DEBUG CODE END
+
         (*scritr)->_FinishScriptCall();
     }
 }

Like your comment: https://github.com/TrinityCore/TrinityCore/issues/7046#issuecomment-11645302
This can show the crash error?

[Shauren]

if the script pushes NULL as that code tries to detect then yes, it should show something

[Shauren]

I have a request - please use the gdb script supplied in contrib/debugger or at least alter your own reporting tool to use these 4 commands found here https://github.com/TrinityCore/TrinityCore/blob/6d2bc7abf48a7e64b04b42b082ebabecd0c5dce6/contrib/debugger/crashreport.gdb#L9-L15

Info from the info registers and x/32i $pc would give me more info about what is wrong with itr

[Jildor]

Oks, thx @Shauren
I will try it
Only a doubt, adding this can affect the performance or hight diff?

[Shauren]

no, these are gdb commands executed after a crash

[zorix]

Targets list is probably invalid targets = std::list = {[0] = 0xc334f700c2386e00}
You can see that upper 16 bits are not zeros but currently CPUs use up to 48 bits (look on other pointers in crashlog like this 0x7275a2cb6ae0)
If you split this pointer into two numbers you will get:
0xc334f700 and 0xc2386e00
After converting it to float you will get:
-180.965 and -46.1074

Possible issue:
https://github.com/TrinityCore/TrinityCore/blob/a0a158b5b851db7e2c16819ec89e913d914a3aba/src/server/scripts/EasternKingdoms/BaradinHold/boss_occuthar.cpp#L238-L247
Here if after remove_if all targets are removed.

https://github.com/TrinityCore/TrinityCore/blob/a0a158b5b851db7e2c16819ec89e913d914a3aba/src/server/scripts/Outland/Auchindoun/SethekkHalls/boss_talon_king_ikiss.cpp#L179-L184
And here if spell didn't had any target.

Edit:
https://github.com/TrinityCore/TrinityCore/blob/b453e124231a90321fe79fbf3a62acdcfa54a691/src/server/scripts/Kalimdor/HallsOfOrigination/boss_temple_guardian_anhuur.cpp#L288-L300
Here if after remove_if and remove all targets are removed or only 1 left.

[Faq]

@Jildor any news here?

[Jildor]

By the moment no, sorry 😄

[Jildor]

here new crashlog: https://gist.github.com/Jildor/c2071e8c5d333d9b0ac0ab691e9f47dc

But the log error not appears :( (https://github.com/TrinityCore/TrinityCore/issues/20317#issuecomment-329126493)

[ccrs]

https://github.com/TrinityCore/TrinityCore/blob/3.3.5/src/server/game/Spells/Spell.cpp#L1185

can that actually turn to 0?

[Jildor]

@Shauren Could you explain to me how you have seen in the crashlog that the problem is in boss_talon_king_ikiss.cpp

I would like to learn 😄

[Shauren]

If you look closely at the commit messge - I only did ref this issue, not closes

Anyway, here goes slightly longer explanation

First, extracting info from the crash log, listing the important pieces

• targets = std::list = {[0] = 0x438d896cc23a8e24} - this tells me that target list is not empty AFTER executing all script hooks and has garbage in it
• effIndex=effIndex@entry=EFFECT_0 - targets was corrupted in a script hook bound to first effect
• grid_x = 31, grid_y = 32 - ingame location (but we don't know the map)

Based on that, grep the code for this pattern SpellObjectAreaTargetSelectFn.*EFFECT_0, on 3.3.5 branch this gave me ~60 results and I had to manually check all of them
Out of these places only 1 (spell_talon_king_ikiss_blink) looked like it could cause a crash
This is the non-obvious part Trinity::Containers::SelectRandomContainerElement must not be called on empty containers (since its basically dereferencing *list.end() iterator)

Finally ingame test - go to sethekk halls and .gps at talon king ikiss, got grid 32,32 which is "close enough"

[jackpoz]

@Shauren maybe we can close this now ?