CTD when conquering city

M

molodets

Chieftain
Joined
Aug 19, 2010
Messages
34
Hi, i found a CTD when taking a city. Instructions how to reproduce:

Start a multi-player game, load attached save "Augustus BC-3550", select "Romans"
Conquer Hattusas (in the lower right of the visible map; one defender remaining)
-> instant CTD (no new turn required)

Attached: Savegame, logs and minidump

View attachment Augustus BC-3550.zip
View attachment Logs.zip

On a related note: is there any documentation on how to set up for dll debugging? I'm a c++ developer and would like to help finding the reason for crashes like this one.
 
molodets said:
Hi, i found a CTD when taking a city. Instructions how to reproduce:

Start a multi-player game, load attached save "Augustus BC-3550", select "Romans"
Conquer Hattusas (in the lower right of the visible map; one defender remaining)
-> instant CTD (no new turn required)

Attached: Savegame, logs and minidump

View attachment 361031
View attachment 361032

On a related note: is there any documentation on how to set up for dll debugging? I'm a c++ developer and would like to help finding the reason for crashes like this one.
Click to expand...

I don't have the PDB for this build (it was one of the TB builds where he was unable to upload the PDB), so I cannot interpret the minidump. I tried the save, but for me when I attacked, two atl atl appeared to add to the defense (militia I presume - think this is a random chance), and when I killed those the city was taken without any issues.

In regard to setting up to debug:

1) First set up to build - start from this base guide: http://modiki.civfanatics.com/index.php/How_to_Install_the_SDK. Once you have those components you can then use any version of visual studio as your actual editing/debugging environment (I use VS2010 and VS2008 mostly). In the C2C source you'll find project files and also the file 'makefilePaths', which you'll need to edit to point to the paths at which you installed the SDK toolkit and the 2003 Visual Studio Express (it must be THAT compiler that actually performs the build, but the makefile takes care of that)

2) Build the 'final release' version and copy the resulting DLL and PDB to your mod assets folder

3) Run up BTS, attach the debugger to the running image and do whatever it is that causes the crash. That should drop you into the debugger with source-level debugging.

4) If it reproduces then try it with the DEBUG build instead, as the final release version lacks debugging info, and you won't be able to examine variables, or step through code reliably. Note however that the DEBUG version is ORDERS OF MAGNITUDE slower, so switch to it only after verifying reproducibility

Also any crash without a debugger attached should generate a minidump - copy this to a folder that also contains the PDB matching the build that crashed and then double- click to open - it will give you source-level context for the crash (but no variable or memory info), provided it occurred in the DLL.
 
Ok, I think you may be able to figure this out... looks like a null reference pass where there shouldn't be one but I can't be sure exactly what ends up being the problem.

The call stack:
Code: 
 	CvGameCoreDLL.dll!CvUnitInfo::isHiddenNationality()  Line 10418	C++
 	CvGameCoreDLL.dll!CvUnit::getVisualOwner(TeamTypes eForTeam=-1)  Line 19786 + 0xb bytes	C++
 	CvGameCoreDLL.dll!CvPlot::plotMinimapColor()  Line 12954 + 0x9 bytes	C++
 	CvGameCoreDLL.dll!CvPlot::updateMinimapColor()  Line 1381 + 0x54 bytes	C++
 	CvGameCoreDLL.dll!CvPlot::setRevealedOwner(TeamTypes eTeam=17, PlayerTypes eNewValue=17)  Line 10808	C++
 	CvGameCoreDLL.dll!CvPlot::updateRevealedOwner(TeamTypes eTeam=17)  Line 10861 + 0x10 bytes	C++
 	CvGameCoreDLL.dll!CvPlot::changeVisibilityCount(TeamTypes eTeam=17, int iChange=1, InvisibleTypes eSeeInvisible=-1, bool bUpdatePlotGroups=false)  Line 10576 + 0x8 bytes	C++
 	CvGameCoreDLL.dll!CvPlot::changeAdjacentSight(TeamTypes eTeam=0, int iRange=2, bool bIncrement=true, CvUnit * pUnit=0x00000000, bool bUpdatePlotGroups=false)  Line 2527 + 0x27 bytes	C++
>	CvGameCoreDLL.dll!CvPlot::setOwner(PlayerTypes eNewValue=0, bool bCheckUnits=false, bool bUpdatePlotGroup=false)  Line 7694	C++
 	CvGameCoreDLL.dll!CvCity::init(int iID=81952, PlayerTypes eOwner=17, int iX=43, int iY=58, bool bBumpUnits=false, bool bUpdatePlotGroups=false)  Line 328	C++
 	CvGameCoreDLL.dll!CvPlayer::initCity(int iX=43, int iY=58, bool bBumpUnits=false, bool bUpdatePlotGroups=false)  Line 3103	C++
 	CvGameCoreDLL.dll!CvPlayer::acquireCity(CvCity * pOldCity=0x5a9910a0, bool bConquest=true, bool bTrade=false, bool bUpdatePlotGroups=true)  Line 3643 + 0x22 bytes	C++
 	CvGameCoreDLL.dll!CvUnit::setXY(int iX=43, int iY=58, bool bGroup=true, bool bUpdate=true, bool bShow=true, bool bCheckPlotVisible=true)  Line 16468	C++
 	CvGameCoreDLL.dll!CvUnit::move(CvPlot * pPlot=0x51d5084c, bool bShow=true)  Line 6424	C++
 	CvGameCoreDLL.dll!CvSelectionGroup::groupMove(CvPlot * pPlot=0x51d5084c, bool bCombat=true, CvUnit * pCombatUnit=0x620d1f88, bool bEndMove=false)  Line 5336	C++
 	CvGameCoreDLL.dll!CvUnit::updateCombat(bool bQuick=false)  Line 4100	C++
 	CvGameCoreDLL.dll!CvSelectionGroup::updateTimers()  Line 540	C++
 	CvGameCoreDLL.dll!CvPlayer::updateTimers()  Line 6750 + 0x5 bytes	C++
 	CvGameCoreDLL.dll!CvGame::update()  Line 2656 + 0x1f bytes	C++

It stops on the return here:
Code: 
bool CvUnitInfo::isHiddenNationality() const
{
	return m_bHiddenNationality;
}
This function was called by:
Code: 
PlayerTypes CvUnit::getVisualOwner(TeamTypes eForTeam) const
{
	if (NO_TEAM == eForTeam)
	{
		eForTeam = GC.getGameINLINE().getActiveTeam();
	}

	if (getTeam() != eForTeam && eForTeam != BARBARIAN_TEAM)
	{
		if (m_pUnitInfo->isHiddenNationality())<------Here!
		{
			if (!plot()->isCity(true, getTeam()))
			{
				return BARBARIAN_PLAYER;
			}
		}
	}

	return getOwnerINLINE();
}
Oddly, it looks like even by that point eTeam still registers as -1.

So that means 2 things are amiss here. That eTeam is still -1 indicating there's no active team or we have something like a stack overflow on that team. This may just be an artifact of the minidump not showing the correct value of the eTeam at this point in the processing.

The other part is... why are we calling to the base unit definition here? Shouldn't we be calling to a Unit.cpp function? I suppose we don't yet have the ability for promos and ccs to adjust hidden nationality (which reminds me... that would be hella easy to make with what I know now and its something I always wanted to do for the mod!)

So I'm wondering... is it possible that we're evaluating this on a non-unit with a non-player (team)??? Perhaps one that just got eliminated and is still awaiting its final code death?

We're calling to that function from here:
Code: 
ColorTypes CvPlot::plotMinimapColor()
{
	CvUnit* pCenterUnit;

	if (GC.getGameINLINE().getActivePlayer() != NO_PLAYER)
	{
		CvCity* pCity;

		pCity = getPlotCity();

		if ((pCity != NULL) && pCity->isRevealed(GC.getGameINLINE().getActiveTeam(), true))
		{
			return (ColorTypes)GC.getInfoTypeForString("COLOR_WHITE");
		}

		if (isActiveVisible(true))
		{
			pCenterUnit = getDebugCenterUnit();

			if (pCenterUnit != NULL)
			{
				return ((ColorTypes)(GC.getPlayerColorInfo(GET_PLAYER(pCenterUnit->getVisualOwner()).getPlayerColor()).getColorTypePrimary()));<--- Here!
			}
		}

		if ((getRevealedOwner(GC.getGameINLINE().getActiveTeam(), true) != NO_PLAYER) && !isRevealedBarbarian())
		{
			return ((ColorTypes)(GC.getPlayerColorInfo(GET_PLAYER(getRevealedOwner(GC.getGameINLINE().getActiveTeam(), true)).getPlayerColor()).getColorTypePrimary()));
		}
	}

	return (ColorTypes)GC.getInfoTypeForString("COLOR_CLEAR");
}
Looks like we're protected from evaluating a non-unit here... (or are we? Should it not be NO_UNIT rather than NULL?)

I'm not sure what the problem actually is then. Perhaps m_pUnitInfo isn't quite sure which unit we're talking about? Something seems a little fishy about the way the base unit is called there.

Let me know if you need me to go back further.
 
Thanks Koshling for the info, I got it to work.

It crashes in
Code: 
PlayerTypes CvUnit::getVisualOwner(TeamTypes eForTeam) const
{
	if (NO_TEAM == eForTeam)
	{
		eForTeam = GC.getGameINLINE().getActiveTeam();
	}

	if (getTeam() != eForTeam && eForTeam != BARBARIAN_TEAM)
	{
		if (m_pUnitInfo->isHiddenNationality())
because m_pUnitInfo is NULL

I traced that back to
Code: 
ColorTypes CvPlot::plotMinimapColor()
, where getDebugCenterUnit() returns CvPlot::m_pCenterUnit which is not null, but a very odd object (all values 0). I do believe this is a galley which got destroyed during the taking of the city. Its object still lingers around causing a crash.

I fixed it for me by adding a check to
Code: 
PlayerTypes CvUnit::getVisualOwner(TeamTypes eForTeam) const
{
	if (NO_TEAM == eForTeam)
	{
		eForTeam = GC.getGameINLINE().getActiveTeam();
	}

	if (getTeam() != eForTeam && eForTeam != BARBARIAN_TEAM)
	{
		if ([COLOR="SeaGreen"]m_pUnitInfo != NULL && [/COLOR]m_pUnitInfo->isHiddenNationality())
		{
			if (!plot()->isCity(true, getTeam()))
			{
				return BARBARIAN_PLAYER;
			}
		}
	}

	return getOwnerINLINE();
}
new code in green. The extra check works here because the value of the calculation is irrelevant since it changes immediately after that(because the city has been taken).

Call stack was:
Code: 
>	CvGameCoreDLL.dll!CvUnitInfo::isHiddenNationality()  Line 10418 + 0x3 bytes	C++
 	CvGameCoreDLL.dll!CvUnit::getVisualOwner(TeamTypes eForTeam)  Line 19786 + 0xe bytes	C++
 	CvGameCoreDLL.dll!CvPlot::plotMinimapColor()  Line 12954 + 0xa bytes	C++
 	CvGameCoreDLL.dll!CvPlot::updateMinimapColor()  Line 1381 + 0x2b bytes	C++
 	CvGameCoreDLL.dll!CvPlot::setRevealedOwner(TeamTypes eTeam, PlayerTypes eNewValue)  Line 10808	C++
 	CvGameCoreDLL.dll!CvPlot::updateRevealedOwner(TeamTypes eTeam)  Line 10863	C++
 	CvGameCoreDLL.dll!CvPlot::changeVisibilityCount(TeamTypes eTeam, int iChange, InvisibleTypes eSeeInvisible, bool bUpdatePlotGroups)  Line 10578	C++
 	CvGameCoreDLL.dll!CvPlot::changeAdjacentSight(TeamTypes eTeam, int iRange, bool bIncrement, CvUnit * pUnit, bool bUpdatePlotGroups)  Line 2532	C++
 	CvGameCoreDLL.dll!CvPlot::setOwner(PlayerTypes eNewValue, bool bCheckUnits, bool bUpdatePlotGroup)  Line 7694	C++
 	CvGameCoreDLL.dll!CvCity::init(int iID, PlayerTypes eOwner, int iX, int iY, bool bBumpUnits, bool bUpdatePlotGroups)  Line 328	C++
 	CvGameCoreDLL.dll!CvPlayer::initCity(int iX, int iY, bool bBumpUnits, bool bUpdatePlotGroups)  Line 3103	C++
 	CvGameCoreDLL.dll!CvPlayer::acquireCity(CvCity * pOldCity, bool bConquest, bool bTrade, bool bUpdatePlotGroups)  Line 3643 + 0x26 bytes	C++

Since capturing/destroying sea units when conquering a city usually works, there have to be special circumstances here. I will look into acquireCity where this can happen at all.

If you want me to help out locating crash issues in the c++, i'm happy to help.



Unrelated, there are a number of asserts before that, don't know if they are important:
In any way, there are double asserts, one with and one without a message.

Code: 
void CvCity::changeBonusGoodHealth(int iChange)
{
	if (iChange != 0)
	{
		m_iBonusGoodHealth += iChange;
		FAssert(getBonusGoodHealth() >= 0); [COLOR="SeaGreen"]// <- this line is useless, does same as next line but without message[/COLOR]

		FAssertMsg(getBonusGoodHealth() >= 0, "getBonusGoodHealth is expected to be >= 0");

Call stack is
Code: 
CvGameCoreDLL.dll!CvCity::changeBonusGoodHealth(int iChange)  Line 11562 + 0x3c bytes	C++
 	CvGameCoreDLL.dll!CvCity::processBonus(BonusTypes eBonus, int iChange)  Line 6782	C++
>	CvGameCoreDLL.dll!CvCity::processNumBonusChange(BonusTypes eIndex, int iOldValue, int iNewValue)  Line 17080	C++
 	CvGameCoreDLL.dll!CvCity::changeNumBonuses(BonusTypes eIndex, int iChange)  Line 17108	C++
 	CvGameCoreDLL.dll!CvPlotGroup::changeNumBonuses(BonusTypes eBonus, int iChange)  Line 548	C++
 	CvGameCoreDLL.dll!CvPlot::updatePlotGroupBonus(bool bAdd)  Line 1877	C++
 	CvGameCoreDLL.dll!CvPlot::setOwner(PlayerTypes eNewValue, bool bCheckUnits, bool bUpdatePlotGroup)  Line 7643	C++
 	CvGameCoreDLL.dll!CvPlot::updateCulture(bool bBumpUnits, bool bUpdatePlotGroups)  Line 1172	C++
 	CvGameCoreDLL.dll!CvPlot::changeCultureRangeCities(PlayerTypes eOwnerIndex, int iRangeIndex, int iChange, bool bUpdatePlotGroups, bool bUpdateCulture)  Line 12128	C++
 	CvGameCoreDLL.dll!CvCity::setCultureLevel(CultureLevelTypes eNewValue, bool bUpdatePlotGroups)  Line 14099	C++
 	CvGameCoreDLL.dll!CvCity::kill(bool bUpdatePlotGroups, bool bUpdateCulture)  Line 1578	C++

I corrected the value manually (from -2 to 0), then up comes a FAssert(getBonusGoodHappiness() >= 0);

After that, FAssert(getBonusGoodHealth() >= 0);

They come up for a lot of goods, with values of -3 to -1

btw: i got the ctd in svn6117, and still crashes after updating to 6152. I'm debugging on 6152 now
 
Well i took to much time replying and so we have a partial double-post.

At least i found out where your non-team unit comes from:

The odd unit object is a galley which was present in the city as last unit after the last land defender died. Not sure why it wasn't deleted properly.
 
molodets said:
Well i took to much time replying and so we have a partial double-post.

At least i found out where your non-team unit comes from:

The odd unit object is a galley which was present in the city as last unit after the last land defender died. Not sure why it wasn't deleted properly.
Click to expand...

Your fix is no correct, it just happens to work because the memory of the deleted object is being NULL'd (not guaranteed). The fix needs to be one level further back - i.e. - figuring out how getDebugCenterUnit() can return an invalid unit.

I cannot see how that can happen, looking at the code, but I'll rerun your save again and see if I can reproduce the problem (I think the first time I tried it didn't due to the appearance of militia, which probably only happens occassionally)
 
Koshling said:
Your fix is no correct, it just happens to work because the memory of the deleted object is being NULL'd (not guaranteed). The fix needs to be one level further back - i.e. - figuring out how getDebugCenterUnit() can return an invalid unit.

I cannot see how that can happen, looking at the code, but I'll rerun your save again and see if I can reproduce the problem (I think the first time I tried it didn't due to the appearance of militia, which probably only happens occassionally)
Click to expand...

I know, just a quick and dirty fix to continue my game. It has to be some code calling setCenterUnit like CvPlot::updateCenterUnit. Will check again in the evening.
 
Looks like it traces back to an earlier call to updateCenterUnit.

There's a lot going on here:
Code: 
void CvPlot::updateCenterUnit()
{
	PROFILE_FUNC();

	CvUnit* pOldCenterUnit = getCenterUnit();
	CvUnit* pNewCenterUnit;

	if ( m_bInhibitCenterUnitCalculation || !shouldHaveFullGraphics() )
	{
		return;
	}
	else
	{
		pNewCenterUnit = NULL;
	}

	if (!isActiveVisible(true))
	{
		pNewCenterUnit = NULL;
	}
	else
	{
		pNewCenterUnit = getSelectedUnit();

		if (pNewCenterUnit != NULL && !pNewCenterUnit->atPlot(this))
		{
			pNewCenterUnit = NULL;
		}

		if (pNewCenterUnit == NULL)
		{
			pNewCenterUnit = getBestDefender(GC.getGameINLINE().getActivePlayer(), NO_PLAYER, NULL, false, false, true);
		}

		if (pNewCenterUnit == NULL)
		{
			pNewCenterUnit = getBestDefender(GC.getGameINLINE().getActivePlayer());
		}

		if (pNewCenterUnit == NULL)
		{
			pNewCenterUnit = getBestDefender(NO_PLAYER, GC.getGameINLINE().getActivePlayer(), gDLL->getInterfaceIFace()->getHeadSelectedUnit(), true);
		}

		if (pNewCenterUnit == NULL)
		{
			pNewCenterUnit = getBestDefender(NO_PLAYER, GC.getGameINLINE().getActivePlayer(), gDLL->getInterfaceIFace()->getHeadSelectedUnit());
		}

		if (pNewCenterUnit == NULL)
		{
			pNewCenterUnit = getBestDefender(NO_PLAYER, GC.getGameINLINE().getActivePlayer());
		}
	}

	if ( pNewCenterUnit != pOldCenterUnit )
	{
#if 0
		OutputDebugString(CvString::format("Center unit change for plot (%d,%d) from %08lx to %08lx\n", m_iX, m_iY, pOldCenterUnit, pNewCenterUnit).c_str());
		if ( pOldCenterUnit != NULL )
		{
			//	Does the old center unit still exist?
			if ( unitHere(pOldCenterUnit) )
			{
				pOldCenterUnit->reloadEntity();
			}
			else
			{
				OutputDebugString("Old center unit no longer here\n");
			}
		}
#endif

		if ( pNewCenterUnit != NULL )
		{
			pNewCenterUnit->reloadEntity(true);
		}

		setCenterUnit(pNewCenterUnit);
	}
}

Don't have time to check down ALL the rabbit trails yet.
 
Ok, I've found the problem and will be posting a fix shortly.

It was a very good catch actually - it's quite an old bug that causes stale unit pointers to be used, and could cause crashes in a variety of situations (though usually wouldn't).

I was not able to reproduce the crash, but I was able to reproduce re-use of a pointer to a deleted unit, which is the potential crash. Because I couldn't reproduce the actual crash itself I'd appreciate it if you would retry the save in your environment with my fix once it's posted as final verification.

For reference the culprit was an optimization that inhibits the updating of the center unit across operations that will update it multiple times (so that the update hit is only taken once at the end of the operation). This meant that a unit that was the cached center unit would remain so after it had been deleted until the update-inhibited section was exited. If it was accessed in that window (and your stack dump was just such an example) then crashes are possible.
 
Does not reproduce any more with svn6163. Tried with and without dll recalculation, no more crash.

Great work and thanks.
 
You must log in or register to reply here.

Similar threads

Recursive
  • Locked
New BETA Version - 4.17.8 (December 16, 2024)
5 6 7
Replies
131
Views
29K
Recursive
Recursive
A
New Version – 5.0-alpha.03
11 12 13
Replies
247
Views
34K
azum4roll
A
Recursive
  • Locked
New Version - 4.6.2 (April 19, 2024)
5 6 7
Replies
123
Views
24K
Recursive
Recursive
Recursive
  • Locked
New Version - 3.10.14 (September 26, 2023)
6 7 8
Replies
141
Views
34K
Recursive
Recursive
UncivilizedGuy
Historical Timeline Project
Replies
11
Views
4K
UncivilizedGuy
UncivilizedGuy
Back
Top Bottom