Author Topic: Outpost2 crashes when checking victory conditions page after loading saved game  (Read 7432 times)

Offline Vagabond

  • Global Moderator
  • Hero Member
  • *****
  • Posts: 1015
I'm working on saving the victory conditions for a colony map using version 3.1 of the SDK. The victory condition shows up fine on the mission objectives page when the map is first initialized. Once the map is saved and then reloaded, checking the victory conditions screen causes Outpost 2 to crash and gives me the message below.

Unhandled exception at 0x004C68A2 in Outpost2.exe: 0xC0000005: Access violation reading location 0x6FEC3254.

Below is my test code. I pulled everything out of main.cpp and used very standard values for everything to try and isolate the problem. Not sure where the mistake is though. Willing to post the full solution somewhere if it would help someone looking at it.

Code: [Select]
#define WIN32_LEAN_AND_MEAN
#include <windows.h>
#include "Outpost2DLL\Outpost2DLL.h"
#include "OP2Helper\OP2Helper.h"

char MapName[] = "on4_01.map";
char LevelDesc[] = "Hooville Victory Test";
char TechtreeName[] = "MULTITEK.TXT";
SDescBlock DescBlock = { Colony, 1, 12, 0 };

struct ScriptGlobal
{
    Trigger VictoryTrig;
};

ScriptGlobal saveData;

BOOL APIENTRY DllMain(HINSTANCE hinstDLL, DWORD fdwReason, LPVOID lpvReserved)
{
    if (fdwReason == DLL_PROCESS_ATTACH)
    {
        DisableThreadLibraryCalls(hinstDLL);
    }

    return TRUE;
}

int InitProc()
{
    saveData.VictoryTrig = CreateCountTrigger(true, true, 0, mapEvacuationModule, mapNone, 1, cmpGreaterEqual, "NoResponseToTrigger");
    CreateVictoryCondition(true, true, saveData.VictoryTrig, "Evacuate 200 colonists to the starship.");

    return 1; // return 1 if OK; 0 on failure
}

void AIProc() { }

void __cdecl GetSaveRegions(struct BufferDesc &bufDesc)
{
    bufDesc.bufferStart = &saveData; // Pointer to a buffer that needs to be saved
    bufDesc.length = sizeof(saveData); // sizeof(buffer)
}

int StatusProc()
{
    return 0; // must return 0
}

SCRIPT_API void NoResponseToTrigger() { }

Offline Sirbomber

  • Hero Member
  • *****
  • Posts: 3238
This is an issue that's cropped up a lot.  Supposedly it had something to do with CodeBlocks (maybe some compiler setting CodeBlocks turned on by default?).  If you're using CodeBlocks, try switching to MSVC.  Otherwise, you'll need to talk to someone more familiar with compiler setup than myself (or who remembers the actual issue, which was probably discussed on IRC, so any record of the conversation is probably long-gone or buried in several megabytes worth of chat logs).
"As usual, colonist opinion is split between those who think the plague is a good idea, and those who are dying from it." - Outpost Evening Star

Outpost 2 Coding 101 Tutorials

Offline Hooman

  • Administrator
  • Hero Member
  • *****
  • Posts: 4955
The crash address is deep within string processing code, such as sprintf, vsprintf, vsnprintf. A stack trace could provide more information about what is calling the string processing code, and figure out what is being passed that is causing the problem.

Is this code example complete enough to compile and reproduce the bug?

Posting the compiled DLL, and instructions to reproduce the issue could help solve it. If Sirbomber is right about it being compiler or compiler setting specific, then the compiled code would be important here.

CodeBlocks is from what I understand just an editor. If you have MSVC installed, it might be calling the MSVC compiler. It can call other compilers though, so I don't know for sure what is going on there. The name mangling, as used by Outpost 2 in the exported symbol names, is usually compiler specific, so that would imply MSVC is being used. Mind you, MSVC is popular enough that it's name mangling may have been replicated.


You can shorten your code by using some of the macros from the SDK. You can check Outpost2DLL/RequiredExports.h for details. Macros like ExportLevelDetails(levelDesc, mapName, techTreeName, missionType, numPlayers), and ExportSaveLoadData(globalVarStructName) can cut down on a lot of boilerplate code. The function StatusProc is not actually used, so you can remove it. It was present in all the built in levels, and so got included in the SDK. We found out later it doesn't need to be there.

The DllMain method can also be omitted, as a default implementation should be provided. That also eliminates the need to include windows.h.

Code: [Select]
#include "Outpost2DLL\Outpost2DLL.h"
#include "OP2Helper\OP2Helper.h"

ExportLevelDetails("Hooville Victory Test", "on4_01.map", "MULTITEK.TXT", Colony, 1);

struct ScriptGlobal
{
    Trigger VictoryTrig;
} scriptGlobal;
ExportSaveLoadData(scriptGlobal);

Export int InitProc()
{
...
}

Export void AIProc() {}
Export void NoResponseToTrigger() {}
« Last Edit: January 27, 2016, 03:43:02 AM by Hooman »

Offline leeor_net

  • Administrator
  • Hero Member
  • *****
  • Posts: 2352
  • OPHD Lead Developer
    • LairWorks Entertainment
He mentions SDK 3.1 -- no such beast so I assume 2.1 which comes with the Visual Studio solution pretty much ready to go. So I'm assuming this.

Offline Vagabond

  • Global Moderator
  • Hero Member
  • *****
  • Posts: 1015
Hey everyone,

I meant SDK 2.1. Not sure what I was thinking when typing 3.1.

Hooman, thank you for the tips on reducing the boiler plate code. I went ahead and made the suggested changes and recompiled the solution. I checked it by loading it into Outpost 2, saving the scenario, and then loading the save, just in case something changed. It still crashes when going to the mission objectives tab. I put the exception and stack trace at the bottom of this post.

I'm using Microsoft Visual Studio 2015 and the built in compiler. I didn't change any project or solution level settings from the SDK 2.1 example project Except:

  • Changed the Project Target Name to cTest (Under Project -> OP2Script Properties -> General)
  • Changed the Output File to .\Release\cTest.dll (Under Project -> OP2Script -> Linker)

Attached to this post is the Compiled DLL throwing the error. Also attached is a ZIP file containing the full Visual Studio Solution that was used to compile the code. Except the ZIP file is missing the file OP2Script.sdf so it is small enough to attach to the forum post. If the SDF file is important, let me know and we can work something out through an email or something else to transfer it. I think SDF is a Sequel Server Compact Database file, but not sure what its purpose is in the SDK.

Is there anything preventing us from making the following updates to the sample project included in the SDK?
  • Remove the function StatusProc
  • Using the function ExportLevelDetails to set scenario properties instead of the separate variables
  • Remove the GetSaveRegions and make a comment to use ExportSaveLoadData if data persistence is required for the scenario
  • Remove #include <windows.h> and the function DllMain

Exception:
Unhandled exception at 0x004C68A2 in Outpost2.exe: 0xC0000005: Access violation reading location 0x10963254.

Stack Trace
>   Outpost2.exe!004c68a2()   Unknown
    [Frames below may be incorrect and/or missing, no symbols loaded for Outpost2.exe]   
    Outpost2.exe!0040a605()   Unknown
    Outpost2.exe!004317de()   Unknown
    [External Code]   
    Outpost2.exe!00430d83()   Unknown
    Outpost2.exe!00430e7f()   Unknown
    Outpost2.exe!00465494()   Unknown
    Outpost2.exe!004653ca()   Unknown
    [External Code]   
    Outpost2.exe!00431206()   Unknown
    Outpost2.exe!0040a99d()   Unknown
    Outpost2.exe!0049d244()   Unknown
    Outpost2.exe!0045d1de()   Unknown
    Outpost2.exe!0045d1f7()   Unknown
    Outpost2.exe!0045c5c3()   Unknown
    Outpost2.exe!0040acfe()   Unknown
    [External Code]   
    Outpost2.exe!00499f46()   Unknown
    [External Code]   
    Outpost2.exe!0040a605()   Unknown
    Outpost2.exe!004317de()   Unknown
    Outpost2.exe!0045c98d()   Unknown
    Outpost2.exe!00431303()   Unknown
    [External Code]   
    Outpost2.exe!004866b7()   Unknown
    Outpost2.exe!004a88a0()   Unknown
    Outpost2.exe!004c425b()   Unknown
    [External Code]   

Offline Hooman

  • Administrator
  • Hero Member
  • *****
  • Posts: 4955
You have a pretty standard setup, so things should just work. The script is very minimal with nothing obviously wrong. This looks very much like an SDK or project setting bug. Or possibly a bug in Outpost 2, but I've never noticed it before for the built in levels, so probably not.

The attached DLL doesn't load for me, but I was running the game under Wine. I get the "Could not initialize game" popup box. I believe this is related to the CRT dependencies your DLL has. They depend on the runtime for the most recent version of Visual Studio. I quite possibly don't have those installed. I can try checking it under Windows later.

Interestingly, when I tried to load the DLL in a debugger (under Wine), it loaded, and then promptly unloaded. I'd never seen that happen before. I was originally trying to check for required level exports. They all seem to exist, but dependencies, maybe not.

Not all files in the project folder are important. Some of them are auto generated, often when you compile your project. The source is scanned for identifiers (variable names, class names, struct names, template names, etc.) and stored in a database. That information can later be used for things like code completion. If you were to delete cache files and then recompile, the cached info would be regenerated.

Quote
Is there anything preventing us from making the following updates to the sample project included in the SDK?
  • Remove the function StatusProc
  • Using the function ExportLevelDetails to set scenario properties instead of the separate variables
  • Remove the GetSaveRegions and make a comment to use ExportSaveLoadData if data persistence is required for the scenario
  • Remove #include <windows.h> and the function DllMain
The SVN version of the SDK already has those changes.

Removing "#include <windows.h?" also means you can remove "#define WIN32_LEAN_AND_MEAN".


As for the stack trace:
Quote
[Frames below may be incorrect and/or missing, no symbols loaded for Outpost2.exe]   
That seems to be where the interesting information should have been. The function below that is basically the window proc for a user interface control. It probably handled clicking on the button to show the mission objectives list.

Offline Vagabond

  • Global Moderator
  • Hero Member
  • *****
  • Posts: 1015
Hooman, thank you for looking at it.

In regards to not working with Wine, I read up on how to change the Target Framework and Platform Toolset in Visual Studio 2015 from this article on MSDN: https://msdn.microsoft.com/en-us/library/ff770576.aspx.

I have Visual Studio 2013 installed, so I already have the Visual Studio 2013 toolsets available. I guess one could just download the toolsets for earlier versions of Visual Studio as needed and still use the newer IDE.

So, I changed the Project Target Platform Toolset to Visual Studio 2013 - Windows XP (v120_xp). (Under Project -> OP2Script Properties -> General) The setting I originally used to build the DLL was Visual Studio 2015 (v140). The other options were Visual Studio 2015 - Windows XP (v140_xp) and Visual Studio 2013  (v120).

The interesting part is that the DLL shrank from 14.5 KB to 7.0 KB when switching to Visual Studio 2013 - Windows XP (v120_xp). When building the dll with the XP version of Visual Studio 2015 toolset, the file size remained at 14.5 KB. The 2013 toolset DLL works with my copy of Outpost 2, but still has same mission objective crash. I'm testing it on a Windows 7 64 bit machine.

I have never used Wine, but I would be curious if the DLL built with the 2013 toolset was compatible with it.

Attached is a PNG screenshot of the general properties tab of the solution. Some available properties available using the Visual Studio 2015 toolset disappear when selecting Visual Studio 2013 toolset.

I attached DLLs built using 3 of the toolset options. The files are each labeled with which toolset was used. I believe you have to drop the filename to 7 chars or less for Outpost 2 to recognize it (I'm sure most of you know this, but in just in case).

I pulled the SDK off of Redmine here http://redmine.outpostuniverse.org/projects/op2-sdk/files. It looks like the templates used in the Redmine release match the templates in the SVN under tag SDK-V2 and not the trunk, which is newer than the tag.

Is there a way to pull more information into the stack trace or a way to 'load symbols for Outpost2.exe?'. I'm guessing since we do not have the source code no, but this is something I've never attempted.

Offline Vagabond

  • Global Moderator
  • Hero Member
  • *****
  • Posts: 1015
I still have problems with the victory conditions screen crashing the game whenever I compile a scenario DLL.

Today, when testing a scenario, I saved and then loaded the scenario. Clicking on the Mission Objectives page didn't crash, but instead showed indecipherable characters. See the attached screenshot.

After this, I tried reloading the same saved scenario, and it crashes as usual.

Also, I noticed the Mission 0.50 suffers from this same mission objectives page crash.


Offline Hooman

  • Administrator
  • Hero Member
  • *****
  • Posts: 4955
I started looking into this today. It seems the pointer to the mission objective string on the internal trigger class is being corrupted. The pointer itself appears to have been modified to point to invalid memory, rather than the character data being corrupted.

This string is accessed in the user interface processing code for lists. The generic list code, which controls things like victory condition lists, factory build lists, etc., makes a virtual function call to retrieve a string for a given list entry. The code specific to victory condition lists determines if the objective is satisfied, retrieves the mission objective string from the internal trigger class, and then copies the string to an output buffer with a color code prefix added to the string, depending on whether or not the objective is satisfied. This prefixing of the color code uses the snprintf function which attempts to access the mission object string, and causes a crash due to the invalid pointer.

Next up is to figure out where and why the mission objective string pointer is being corrupted.

Offline Hooman

  • Administrator
  • Hero Member
  • *****
  • Posts: 4955
Figured it out. The pointer is fine. The DLL uses ASLR (Address Space Layout Randomization). You'll need to turn this off for Outpost 2 level DLLs.

Project -> Properties -> Configuration Properties -> Linker -> Advanced -> Randomize Base Address: No

When the level is loaded, the DLL is (re)loaded. The /DYNAMICBASE option enables ASLR, which causes the DLL to be loaded at a different random address each time. This is meant to mitigate certain exploits that make use of knowledge of code and data addresses. By rebasing the DLL, it will cause certain exploits with hardcoded addresses to fail (or at best work probabilistically). Unfortunately, by loading the DLL to a new address, any existing pointers into the DLLs address space will be invalidated, and Outpost 2 makes some assumptions that it can save and reload pointers that point into the DLLs address space. In particular, it makes this assumption for the victory condition strings.

This setting only exists on newer versions of Windows, and so is only present in newer versions of Visual Studio. For security reasons, the default for this option is on. By upgrading to a newer version of Visual Studio, this option got enabled. This also explains why people using CodeBlocks ran into this issue, since it will likely have used an updated compiler, have set its own project settings, and likely enabled the option too.

Offline Vagabond

  • Global Moderator
  • Hero Member
  • *****
  • Posts: 1015
Hooman,

Thank you for finding the fix! I just tested and the mission objectives page is working properly after loading! I will test a little more this evening by actually finishing all mission objectives after saving and make sure the scenario completes successfully.


While discussing security settings, I've been setting  /SAFESEH:NO on the project Linker advanced properties tab to get new dlls to compile. The default in Visual Studio 2015 is /SAFESEH:YES, which prevents building the code since some of the referenced source code is not compatible with SAFESEH. The error presented in Visual Studio is easy to follow and correct, unlike the Victory Conditions one.

https://msdn.microsoft.com/en-us/library/9a89h429.aspx
http://www.accuvant.com/blog/old-meets-new-microsoft-windows-safeseh-incompatibility

Offline Hooman

  • Administrator
  • Hero Member
  • *****
  • Posts: 4955
Interesting linked articles, particularly about the bug in the implementation.

I wouldn't worry about turning off the SAFESEH option. There are other security issues with Outpost 2 code, and having extra checks on exception handlers should be the least of your worries. Plus, as stated in the MSDN, the information must be added at compile time, and there is no (reasonable) way to add it to already compiled code, so it wouldn't work with code from Outpost2.exe.

At least the SAFESEH thing is a compile time check, where the compiler can tell you what's wrong. The ASLR setting had no compile time check (how on earth could it check what Outpost2.exe was doing anyway), and so you don't get an error until runtime. Further, there's no reasonable way of catching such a runtime error and producing a reasonable error message.


A side note to the ASLR problem: Don't save and load pointers to disk. It's a bad idea all around. When a process restarts, or a module reloads, and especially if code is updated, there's no guarantee things will be in the same place. Without code upgrades, it could have worked for globals (and statics) without ASLR, and you might get lucky with local variables, but you're unlikely to get lucky with the free store. ASLR breaks all of that, on purpose.
« Last Edit: March 08, 2016, 11:12:30 PM by Hooman »

Offline Arklon

  • Administrator
  • Hero Member
  • *****
  • Posts: 1269
A side note to the ASLR problem: Don't save and load pointers to disk. It's a bad idea all around. When a process restarts, or a module reloads, and especially if code is updated, there's no guarantee things will be in the same place. Without code upgrades, it could have worked for globals (and statics) without ASLR, and you might get lucky with local variables, but you're unlikely to get lucky with the free store. ASLR breaks all of that, on purpose.
ASLR wasn't a thing back in 1997, and many .exe files (but not .dll's) back then even had their base relocations, which are needed for ASLR, stripped to reduce filesize, so I can see why they wouldn't have necessarily written their code to work with it safely.
The whole base relocation issue is why I never really liked how Forced Exports worked and looked into injecting a new EAT at runtime instead.
« Last Edit: March 08, 2016, 11:43:28 PM by Arklon »

Offline Vagabond

  • Global Moderator
  • Hero Member
  • *****
  • Posts: 1015
So, I played with saving/loading scenarios more with ASLR off. It looks like the issue is only half fixed (unless I'm doing something wrong, which is possible).

After compiling a scenario DLL with ASLR off, when I save and then load the scenario without exiting Outpost 2, mission objectives and being able to win the scenario works perfect.

However, if I save the game, then exit Outpost 2 completely and then reopen Outpost 2 and reload the save, the bug still exists and my Mission Objectives page either crashes Outpost 2 or shows random characters from the game. At one time my mission objectives were reading off the TEASER/DESCRIPTION for Stickyfoam research, and other times it was boxes and Greek letters. Also, I cannot win the scenario, even if I manage to meet the mission objectives if the mission objectives text is garbled.

Hooman, have you tried exiting Outpost 2 between saving and loading the scenario? I am curious if this is reproduceable on another machine, or just me.

Offline Arklon

  • Administrator
  • Hero Member
  • *****
  • Posts: 1269
It's quite likely that the base load address of your mission DLL conflicts with the load address of another DLL (such as op2ext.dll or NetFixV3.dll), which causes it to be relocated. I think I set my mission DLLs to have a base address of 0x11000000, for reference. If yours is already set to that, try a different one.
« Last Edit: March 09, 2016, 06:02:05 PM by Arklon »

Offline Vagabond

  • Global Moderator
  • Hero Member
  • *****
  • Posts: 1015
Arklon & Hooman,

Thank you, Seems to be working correctly now! I had left the base address blank/default so far, but 0x11000000 worked.

I read a little from MSDN about the base load address, and it looks like by setting a value, you are telling the operating system what you want, but if it is already taken, then the operating system will assign you wherever it wants. So if another DLL takes 0x11000000, then my scenario DLL will break again.

What made you chose 0x11000000 and what are the odds that another pre-loaded DLL will take that address in the future?

I've never had to make so many property changes within a project before. I'll try and write up a list of all the required settings so it is concise and in one place.

Good news is, with both your help I've pulled the last known bug out of the colony builder scenario I was making. Also managed to get the Mission Briefing to show up like I wanted, so I should be able to push a compiled copy soon but should do some more testing first.

From https://msdn.microsoft.com/en-us/library/f7f5138s.aspx:
Quote
The /BASE option sets a base address for the program, overriding the default location for an .exe file (at 0x400000) or a DLL (at 0x10000000). The operating system first attempts to load a program at its specified or default base address. If sufficient space is not available there, the system relocates the program.

Offline Arklon

  • Administrator
  • Hero Member
  • *****
  • Posts: 1269
What made you chose 0x11000000 and what are the odds that another pre-loaded DLL will take that address in the future?
That's what the stock OP2 mission DLLs just happen to have as their base address. Mod DLLs/etc. should be set to have different base addresses so missions can load at their preferred address.

Offline Hooman

  • Administrator
  • Hero Member
  • *****
  • Posts: 4955
Arklon seems to have nailed it there.

If you strip the relocations from the DLL, you should get a load error if a relocation would have been required. A stripped module can't be relocated. That should produce a consistent error message rather than a weird crash from Outpost 2.


Weird assumption bugs aside, here's some largely obsolete optimization information:
Specifying a base for a DLL can optimize the load time of the DLL. When a preferred load address is taken, the DLL must be relocated, which requires patching addresses in the image using the relocation table. This takes time, slowing loading of the DLL. By leaving a compiler default, DLLs will tend to conflict with their preferred memory address, causing relocations upon loading. By setting it yourself, using knowledge of what DLLs you use, where they load, and their size, you can ensure DLLs are spread out in memory and don't require relocation. There are also some memory optimizations possible when a DLL is loaded by more than one process. The read-only sections (such as code, or read-only data) can be shared between processes. If a DLL is relocated though, those sections will need to be written to have relocations applied, which will require a copy to be made. Of course this info is largely obsolete now because ASLR is the new default, and enabling ASLR forces a relocation.