SAFE_DELETE(g_Game); crashes for scenario map "Arcadia"

[elexis]

Reproduce: Start a single or multiplayergame on that map and click on exit.

It fails on EndGame() in GameSetup.cpp when doing

SAFE_DELETE(g_Game);

Error:

*** Error in `./pyrogenesis': free(): invalid next size (normal): 0x00000000095d7400 ***

[Itms]

I can't reproduce the problem (latest svn) :/

[historic_bruno]

Which compiler did you use, and are you running in a debugger? Maybe you could try running it through valgrind. That's strange that it would only happen on one map.

[leper]

I could reproduce this locally (using gcc 5.2.0). I agree that a valgrind run would be nice to have but I'm a bit busy currently. I did also get an invalid free while testing it on one of the Azure Coast maps *** Error in `binaries/system/pyrogenesis': double free or corruption (out): 0x000000000b2d5600 *** (while starting the map though).

After testing some other maps and not getting a crash I tried Arcadia again and just got a segfault (didn't have a debugger attached though).

[elexis]

On my linux system, everything before r16867 doesn't crash, everything after and including r16867 does (when exiting that map).

[mimo]

I also can reproduce it on my linux. Here is a backtrace

Program received signal SIGABRT, Aborted.
0x00007ffff34b7267 in __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:55
55      ../sysdeps/unix/sysv/linux/raise.c: Aucun fichier ou dossier de ce type.
(gdb) bt
#0  0x00007ffff34b7267 in __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:55
#1  0x00007ffff34b8eca in __GI_abort () at abort.c:89
#2  0x00007ffff34fac53 in __libc_message (do_abort=do_abort@entry=1, fmt=fmt@entry=0x7ffff36131a8 "*** Error in `%s': %s: 0x%s ***\n")
    at ../sysdeps/posix/libc_fatal.c:175
#3  0x00007ffff3502c69 in malloc_printerr (ptr=<optimized out>, str=0x7ffff3613300 "free(): invalid next size (normal)", action=1) at malloc.c:4965
#4  _int_free (av=<optimized out>, p=<optimized out>, have_lock=0) at malloc.c:3834
#5  0x00007ffff350689c in __GI___libc_free (mem=<optimized out>) at malloc.c:2950
#6  0x0000000000503290 in ~Grid (this=0x7b0df10, __in_chrg=<optimized out>) at ../../../source/simulation2/helpers/Grid.h:76
#7  CCmpPathfinder::Deinit (this=0x2fcef10) at ../../../source/simulation2/components/CCmpPathfinder.cpp:108
#8  0x00000000004a9b1a in CComponentManager::ResetState (this=this@entry=0x2a92118) at ../../../source/simulation2/system/ComponentManager.cpp:518
#9  0x00000000004a9e36 in CComponentManager::~CComponentManager (this=0x2a92118, __in_chrg=<optimized out>)
    at ../../../source/simulation2/system/ComponentManager.cpp:114
#10 0x000000000047a89b in ~CSimulation2Impl (this=0x2a920f0, __in_chrg=<optimized out>) at ../../../source/simulation2/Simulation2.cpp:85
#11 CSimulation2::~CSimulation2 (this=<optimized out>, __in_chrg=<optimized out>) at ../../../source/simulation2/Simulation2.cpp:586
#12 0x000000000062c20d in CGame::~CGame (this=0x25d2f20, __in_chrg=<optimized out>) at ../../../source/ps/Game.cpp:106
#13 0x0000000000662420 in EndGame () at ../../../source/ps/GameSetup/GameSetup.cpp:687
#14 0x00000000008bbf39 in Script_EndGame () at ../../../source/gui/scripting/ScriptFunctions.cpp:683
#15 call<void(ScriptInterface::CxPrivate*)> (fptr=<optimized out>, cx=0xddd4880) at ../../../source/scriptinterface/NativeWrapperDefns.h:55
#16 ScriptInterface::call<void, (anonymous namespace)::Script_EndGame> (cx=0xddd4880, argc=<optimized out>, vp=0xf3c748)
    at ../../../source/scriptinterface/NativeWrapperDefns.h:113

[leper]

Not entirely sure why it crashed on g_Game for elexis and myself, but when trying to reproduce this again I got the same backtrace as mimo. Running memcheck (always remember --smc-check=all to please SpiderMonkey) resulted in

==5876== Invalid read of size 2
==5876==    at 0x539412: set (Grid.h:100)
==5876==    by 0x539412: CCmpObstructionManager::RasterizeHelper(Grid<unsigned short>&, unsigned char, bool, unsigned short, CFixed<int, 2147483647, 32, 15, 16, 65536>) (CCmpObstructionManager.cpp:897)
==5876==    by 0x53A31B: CCmpObstructionManager::Rasterize(Grid<unsigned short>&, std::vector<PathfinderPassability, std::allocator<PathfinderPassability> > const&, bool) (CCmpObstructionManager.cpp:866)
==5876==    by 0x4D60E9: CCmpPathfinder::UpdateGrid() (CCmpPathfinder.cpp:520)
==5876==    by 0x4D223B: CCmpPathfinder::GetPassabilityGrid() (CCmpPathfinder.cpp:250)
==5876==    by 0x4E0FF3: CCmpTerritoryManager::CalculateCostGrid() (CCmpTerritoryManager.cpp:341)
==5876==    by 0x4E1505: CCmpTerritoryManager::CalculateTerritories() (CCmpTerritoryManager.cpp:381)
==5876==    by 0x4E2D78: CCmpTerritoryManager::GetOwner(CFixed<int, 2147483647, 32, 15, 16, 65536>, CFixed<int, 2147483647, 32, 15, 16, 65536>) (CCmpTerritoryManager.cpp:656)
==5876==    by 0x574F32: call<CFixed<int, 2147483647, 32, 15, 16, 65536>, CFixed<int, 2147483647, 32, 15, 16, 65536>, int (ICmpTerritoryManager::*)(CFixed<int, 2147483647, 32, 15, 16, 65536>, CFixed<int, 2147483647, 32, 15, 16, 65536>)> (NativeWrapperDefns.h:69)
==5876==    by 0x574F32: bool ScriptInterface::callMethod<int, CFixed<int, 2147483647, 32, 15, 16, 65536>, CFixed<int, 2147483647, 32, 15, 16, 65536>, &class_ICmpTerritoryManager, ICmpTerritoryManager, &ICmpTerritoryManager::GetOwner>(JSContext*, unsigned int, JS::Value*) (NativeWrapperDefns.h:133)
==5876==    by 0x5D09431: CallJSNative (jscntxtinlines.h:239)
==5876==    by 0x5D09431: js::Invoke(JSContext*, JS::CallArgs, js::MaybeConstruct) (Interpreter.cpp:475)
==5876==    by 0x5D0A682: Interpret(JSContext*, js::RunState&) (Interpreter.cpp:2620)
==5876==    by 0x5D1569E: js::RunScript(JSContext*, js::RunState&) (Interpreter.cpp:422)
==5876==    by 0x5D092F5: js::Invoke(JSContext*, JS::CallArgs, js::MaybeConstruct) (Interpreter.cpp:494)
==5876==  Address 0x44740438 is 824 bytes inside an unallocated block of size 1,834,720 in arena "client"
==5876==
--5876-- VALGRIND INTERNAL ERROR: Valgrind received a signal 11 (SIGSEGV) - exiting
--5876-- si_code=80;  Faulting address: 0x0;  sp: 0x8030a7de0
<snip; just valgrind crashing and telling us to fix our code>

Which pointed to Grid messing up somewhere (as did mimo's backtrace above), which lead me to enabling GRID_BOUNDS_DEBUG, which fails in T& get(int i, int j) const with i=444, j=768, m_W=768, m_H=768. This is an OOB read, for which a similar call to set will result in some heap corruption.

Relevant part of the backtrace (lower frames are just the debug_OnAssertionFailure and related functions; higher frames are reading the map, then sending an OwnershipChanged message, then a lot of SpiderMonkey)

#9  0x000000000053bba6 in get (j=768, i=444, this=0x7bb58e0) at ../../../source/simulation2/helpers/Grid.h:106
        suppress__ = 0
#10 CCmpObstructionManager::RasterizeHelper (this=this@entry=0x3046660, grid=...,
    requireMask=requireMask@entry=8 '\b', fullUpdate=fullUpdate@entry=true, appliedMask=80, clearance=...)
    at ../../../source/simulation2/components/CCmpObstructionManager.cpp:897
        i = 444
        j = 768
        __for_range = std::vector of length 11, capacity 14 = {{i0 = 444, i1 = 454, j = 765}, {i0 = 444,
            i1 = 454, j = 766}, {i0 = 444, i1 = 454, j = 764}, {i0 = 444, i1 = 454, j = 767}, {i0 = 444,
            i1 = 454, j = 763}, {i0 = 444, i1 = 454, j = 768}, {i0 = 444, i1 = 454, j = 762}, {i0 = 445,
            i1 = 453, j = 769}, {i0 = 445, i1 = 453, j = 761}, {i0 = 446, i1 = 452, j = 770}, {i0 = 446,
            i1 = 452, j = 760}}
        shape = @0x583b0f4: {entity = 308, x = {value = 29421078}, z = {value = 50104904}, u = {X = {
              value = -65494}, Y = {value = -2365}}, v = {X = {value = 2365}, Y = {value = -65494}}, hw = {
            value = 98304}, hh = {value = 98304}, flags = 15 '\017', group = 308, group2 = 0}
        square = {x = {value = 29421078}, z = {value = 50104904}, u = {X = {value = -65494}, Y = {
              value = -2365}}, v = {X = {value = 2365}, Y = {value = -65494}}, hw = {value = 98304}, hh = {
---Type <return> to continue, or q <return> to quit---
            value = 98304}}
        spans = std::vector of length 11, capacity 14 = {{i0 = 444, i1 = 454, j = 765}, {i0 = 444, i1 = 454,
            j = 766}, {i0 = 444, i1 = 454, j = 764}, {i0 = 444, i1 = 454, j = 767}, {i0 = 444, i1 = 454,
            j = 763}, {i0 = 444, i1 = 454, j = 768}, {i0 = 444, i1 = 454, j = 762}, {i0 = 445, i1 = 453,
            j = 769}, {i0 = 445, i1 = 453, j = 761}, {i0 = 446, i1 = 452, j = 770}, {i0 = 446, i1 = 452, j = 760}}
#11 0x000000000053cbdc in CCmpObstructionManager::Rasterize (this=0x3046660, grid=..., passClasses=...,
    fullUpdate=<optimized out>) at ../../../source/simulation2/components/CCmpObstructionManager.cpp:866
        __for_range = std::map with 3 elements = {[{value = 65536}] = 4, [{value = 262144}] = 80, [{
            value = 786432}] = 32}
        __profile = {<No data fields>}
        profile2__ = {m_Name = 0x9fabd3 "Rasterize"}
        pathfindingMasks = std::map with 3 elements = {[{value = 65536}] = 4, [{value = 262144}] = 80, [{
            value = 786432}] = 32}
        foundationMask = 3
#12 0x00000000004d788c in CCmpPathfinder::UpdateGrid (this=0x3049540)
    at ../../../source/simulation2/components/CCmpPathfinder.cpp:520
        __profile = {<No data fields>}
        profile2__ = {m_Name = 0x9f2622 "UpdateGrid"}
        cmpTerrain = <optimized out>
        terrainSize = <optimized out>
        cmpObstructionManager = {m = 0x3046660}
        __func__ = "UpdateGrid"
#13 0x00000000004d223c in CCmpPathfinder::GetPassabilityGrid (this=0x3049540)
    at ../../../source/simulation2/components/CCmpPathfinder.cpp:250
No locals.
#14 0x00000000004e27bf in CCmpTerritoryManager::CalculateCostGrid (this=this@entry=0x304b0b0)
    at ../../../source/simulation2/components/CCmpTerritoryManager.cpp:341
        cmpPathfinder = {m = 0x3049540}
        passClassTerritory = 8
        passClassUnrestricted = 256
        passGrid = <optimized out>
        tilesW = <optimized out>
        tilesH = <optimized out>
---Type <return> to continue, or q <return> to quit---
#15 0x00000000004e2cb6 in CCmpTerritoryManager::CalculateTerritories (this=this@entry=0x304b0b0)
    at ../../../source/simulation2/components/CCmpTerritoryManager.cpp:381
        __profile = {<No data fields>}
        tilesW = <optimized out>
        tilesH = <optimized out>
        cmpPlayerManager = <optimized out>
        influences = std::vector of length 0, capacity 8796093021325 = {
          <error reading variable influences (Cannot access memory at address 0x1)>
        influenceEntities = std::map with 140737339394006 elements<error reading variable: Cannot access memory at address 0x441f0f660b1f>
        bestWeightGrid = <optimized out>
        rootInfluenceEntities = std::vector of length 35184372085884, capacity 11287388 = {
          <error reading variable rootInfluenceEntities (Cannot access memory at address 0x0)>
        __func__ = "CalculateTerritories"
#16 0x00000000004e4c29 in CCmpTerritoryManager::GetOwner (this=0x304b0b0, x=..., z=...)
    at ../../../source/simulation2/components/CCmpTerritoryManager.cpp:656
No locals.
#17 0x00000000005778a3 in call<CFixed<int, 2147483647, 32, 15, 16, 65536>, CFixed<int, 2147483647, 32, 15, 16, 65536>, int (ICmpTerritoryManager::*)(CFixed<int, 2147483647, 32, 15, 16, 65536>, CFixed<int, 2147483647, 32, 15, 16, 65536>)> (a1=..., a0=...,
    fptr=&virtual ICmpTerritoryManager::GetOwner(CFixed<int, 2147483647, 32, 15, 16, 65536>, CFixed<int, 2147483647, 32, 15, 16, 65536>), c=0x304b0b0, rval=..., cx=0x2b0ed50)
    at ../../../source/scriptinterface/NativeWrapperDefns.h:69
No locals.

[leper]

The cause should be the <= at CCmpObstructionManager.cpp#L892​.

[Itms]

In 16944:

Handle properly static shapes that overlap the edge of the map (likely to happen with square maps).

This should fix #3364.

[Itms]

Thanks a lot for debugging. Please reopen if the fix above doesn't work.

[elexis]

Works for me, thx.