3

I've been trying to solve this for days now, and can't get it to work. I'm getting a access violation in the method "orphan_all" in a std::string destructor that's called from a compiler-generated POD struct, that contains some std::string.

struct SaveData
{
    SaveData()
    {
        MusicStage = GameState::MusicStage;
        MusicSubStage= GameState::MusicSubStage;

        PlotStage = GameState::PlotStage;
        PlotSubStage = GameState::PlotSubStage;

        GameStage = GameState::GameStage;
        GameSubStage = GameState::GameSubStage;

        PlayerLife = 100.0f;
        PlayerSuitEnergy = 100.0f;

        CurrentPower = 0;
        PlayerPos = XMFLOAT3(0,0,0);
        CurrentGun = 0;
        Guns = 0;
        ModsL1 = 0;
        ModsL2 = 0;
        ModsL3 = 0;
        ModsL4 = 0;
        CurrentBulletMod = (uint)BulletMod::NoMod;
        ElectricModMult = 1.0;
        ExplosiveModMult = 1.0;
        CorrosiveModMult = 1.0;
    }

    string MusicStage;
    string MusicSubStage;
    string PlotStage;
    string PlotSubStage;
    string GameStage;
    string GameSubStage;

    float PlayerLife;
    float PlayerSuitEnergy;
    uint CurrentPower;
    XMFLOAT3 PlayerPos;

    uint CurrentGun;
    uint CurrentBulletMod;

    float ElectricModMult;
    float ExplosiveModMult;
    float CorrosiveModMult;

    uint Guns;
    uint ModsL1;
    uint ModsL2;
    uint ModsL3;
    uint ModsL4;
};

struct FileData
{
    uint64 Hash;
    uint Version;
    SaveData Data;
};

That's the structure. When the destructor of that object is called, here:

HRESULT SavesIO::LoadGameFile(const std::string& FileName,SaveData& Data)
{
    ifstream file;
    file.open(FileName,ios::binary);
    if(file.is_open())
    {
        FileData fdata;
        file.read((char*)&fdata,sizeof(FileData));
        if(fdata.Hash != GameHash)
        {
            cout << "Corrupt Savegame : " << FileName << endl;
            return CheckHR(HR_Fail);
        }
        if(fdata.Version > CurrentVersion)
        {
            cout << "Savegame version is greater than game version : " << FileName << endl;
            return CheckHR(HR_Fail);
        }
        Data = fdata.Data;
        return HR_Correct;
    }

    cout << "Savegame : " << FileName << "not found" << endl;
    return CheckHR(HR_Invalid_Arg);
}

A access violation happends inside "orphan_all",that is called from the destructor of the strings inside "Data" inside "fdata", and it says locations like "0xdddddddd" or "0xFEEEFEEE", so by some reason it appears to call some deleted data. I checked for heap corruption using HeapValidate() and _CrtCheckMemory() and everything seems to be fine. If I compile in release, the problem goes away. Anyone has any idea? My system is Windows 8 Pro x64, using Visual Studio Express 2012, compiling with the v110 toolset.

EDIT: I'm writing the data like this:

void SavesIO::SaveGameFile(SaveData Data,const std::string& FileName)
{
    ofstream file;
    file.open(FileName,ios::binary);

    FileData fdata;
    fdata.Hash = GameHash;
    fdata.Version = CurrentVersion;
    fdata.Data = Data;
    file.write((char*)&fdata,sizeof(FileData));

    file.close();
}
Santiago Pacheco
  • 197
  • 1
  • 13
  • 2
    You are (de-)serializing complex structures as though they were POD's. The line `file.read((char*)&fdata,...)` is going to cause you trouble. You are essentially reading back pointers (contained in your `std::string`s for example) that happened to be valid when you wrote them to a file. – IInspectable Sep 01 '13 at 17:51
  • what would then be a good idea to serialize/deserialize that structure? Could that be the cause of the later crash? – Santiago Pacheco Sep 01 '13 at 17:56
  • It **is** the cause of undefined behavior, a crash being one notable observable result. Storing strings in a file format usually boils down to writing the length followed by the individual characters. Both writing and reading requires code. Using C++ streams and `operator>>` might be a solution, but it's not going to be your best bet if you are going to share safegames between different bitness (32 and 64) or machines with different endianness (little endian and big endian). – IInspectable Sep 01 '13 at 18:05

3 Answers3

3

It seems that _ITERATOR_DEBUG_LEVEL is to blame for the debug mode only crashes.

I am not opposing to your solution. On the contrary it is the best thing to do. However, the following is GoodToKnow:

Either #define _ITERATOR_DEBUG_LEVEL 0 or (even better) set it in the precompiler defines in your project.

This will stop STL for firing exceptions... Reference: https://msdn.microsoft.com/en-us/library/hh697468.aspx

Make sure that all your nested/dependant projects are compiled with the same option, or: _iterator_debug_level value '0' doesn't match value '2'

The default value of this define is 2.

PS Looks like MS are still compiling their in-house developed STL, even in toolset v120

CodeManX
  • 11,159
  • 5
  • 49
  • 70
Sterge
  • 69
  • 5
1

I got the AccessViolation with the callstack into xmemory std::string destructor because I was passing an string from an assembly compiled in Debug into another assembly compiled in Release.

Slack Groverglow
  • 846
  • 7
  • 25
0

It was what IInspectable said, but I can't mark it as an answer because it was a comment. My write and my read function now look like this:

void SavesIO::SaveGameFile(SaveData Data,const std::string& FileName)
{
    ofstream file;
    file.open(FileName,ios::binary);

    FileData fdata;
    fdata.Hash = GameHash;
    fdata.Version = CurrentVersion;

    fdata.PlayerLife = Data.PlayerLife;
    fdata.PlayerSuitEnergy = Data.PlayerSuitEnergy;
    fdata.CurrentPower = Data.CurrentPower;
    fdata.PlayerPos = Data.PlayerPos;

    fdata.CurrentGun = Data.CurrentGun;
    fdata.CurrentBulletMod = Data.CurrentBulletMod;

    fdata.ElectricModMult = Data.ElectricModMult;
    fdata.ExplosiveModMult = Data.ExplosiveModMult;
    fdata.CorrosiveModMult = Data.CorrosiveModMult;

    fdata.Guns = Data.Guns;
    fdata.ModsL1 = Data.ModsL1;
    fdata.ModsL2 = Data.ModsL2;
    fdata.ModsL3 = Data.ModsL3;
    fdata.ModsL4 = Data.ModsL4;

    file.write((char*)&fdata,sizeof(FileData));

    int size = Data.MusicStage.length();
    file.write((char*)&size,sizeof(int));
    file.write(Data.MusicStage.c_str(),Data.MusicStage.length()+1);

    size = Data.MusicSubStage.length();
    file.write((char*)&size,sizeof(int));
    file.write(Data.MusicSubStage.c_str(),Data.MusicSubStage.length()+1);

    size = Data.PlotStage.length();
    file.write((char*)&size,sizeof(int));
    file.write(Data.PlotStage.c_str(),Data.PlotStage.length()+1);

    size = Data.PlotSubStage.length();
    file.write((char*)&size,sizeof(int));
    file.write(Data.PlotSubStage.c_str(),Data.PlotSubStage.length()+1);

    size = Data.GameStage.length();
    file.write((char*)&size,sizeof(int));
    file.write(Data.GameStage.c_str(),Data.GameStage.length()+1);

    size = Data.GameSubStage.length();
    file.write((char*)&size,sizeof(int));
    file.write(Data.GameSubStage.c_str(),Data.GameSubStage.length()+1);

    file.close();
}

and my read:

HRESULT SavesIO::LoadGameFile(const std::string& FileName,SaveData& Data)
{
    ifstream file;
    file.open(FileName,ios::binary);
    if(file.is_open())
    {
        FileData fdata;
        file.read((char*)&fdata,sizeof(FileData));
        if(fdata.Hash != GameHash)
        {
            cout << "Corrupt Savegame : " << FileName << endl;
            return CheckHR(HR_Fail);
        }
        if(fdata.Version > CurrentVersion)
        {
            cout << "Savegame version is greater than game version : " << FileName << endl;
            return CheckHR(HR_Fail);
        }

        Data.PlayerLife = fdata.PlayerLife;
        Data.PlayerSuitEnergy = fdata.PlayerSuitEnergy;
        Data.CurrentPower = fdata.CurrentPower;
        Data.PlayerPos = fdata.PlayerPos;

        Data.CurrentGun = fdata.CurrentGun;
        Data.CurrentBulletMod = fdata.CurrentBulletMod;

        Data.ElectricModMult = fdata.ElectricModMult;
        Data.ExplosiveModMult = fdata.ExplosiveModMult;
        Data.CorrosiveModMult = fdata.CorrosiveModMult;

        Data.Guns = fdata.Guns;
        Data.ModsL1 = fdata.ModsL1;
        Data.ModsL2 = fdata.ModsL2;
        Data.ModsL3 = fdata.ModsL3;
        Data.ModsL4 = fdata.ModsL4;

        int size = 0;
        file.read((char*)&size,sizeof(int));
        char* tmp = new char[size+1];
        file.read(tmp,size+1);
        Data.MusicStage = tmp;
        delete tmp;

        size = 0;
        file.read((char*)&size,sizeof(int));
        tmp = new char[size+1];
        file.read(tmp,size+1);
        Data.MusicSubStage = tmp;
        delete tmp;

        size = 0;
        file.read((char*)&size,sizeof(int));
        tmp = new char[size+1];
        file.read(tmp,size+1);
        Data.PlotStage = tmp;
        delete tmp;

        size = 0;
        file.read((char*)&size,sizeof(int));
        tmp = new char[size+1];
        file.read(tmp,size+1);
        Data.PlotSubStage = tmp;
        delete tmp;
        return HR_Correct;

        size = 0;
        file.read((char*)&size,sizeof(int));
        tmp = new char[size+1];
        file.read(tmp,size+1);
        Data.GameStage = tmp;
        delete tmp;
        return HR_Correct;


        size = 0;
        file.read((char*)&size,sizeof(int));
        tmp = new char[size+1];
        file.read(tmp,size+1);
        Data.PlotSubStage = tmp;
        delete tmp;
        return HR_Correct;

        size = 0;
        file.read((char*)&size,sizeof(int));
        tmp = new char[size+1];
        file.read(tmp,size+1);
        Data.GameSubStage = tmp;
        delete tmp;
        return HR_Correct;
    }

    cout << "Savegame : " << FileName << "not found" << endl;
    return CheckHR(HR_Invalid_Arg);
}

I have one last question though. What problems can I have if an user tries to transfer his saves from one 32 bit computer to a 64 bits one? Endianess is probably not an issue, as the game is windows-only. Thanks anyway, you saved me a LOT of time.

Santiago Pacheco
  • 197
  • 1
  • 13
  • 1
    Issues you should be prepared for when dealing with different bitness: If you dump data as binary structures, make sure the structures have the same memory layout. In general make sure that binary layout and supported ranges are the same; `DWORD_PTR` for example will have different sizes. As an alternative you might consider a file format that is bitness agnostic (like XML). – IInspectable Sep 01 '13 at 19:59
  • Thanks, i'll consider that. I was trying to keep it binary, but might put it in XML if it's necessary – Santiago Pacheco Sep 01 '13 at 20:14
  • XML is not necessarily better, and has a number of gotchas as well. Writing string data requires escaping of special characters (like `>`) and possibly re-encoding UTF-16 to UTF-8, streaming floating point numbers either involves rounding (with rounding errors) or conversion to a serializable form (a hex string of the binary data, for example). XML will be a valuable tool during debugging though. You can read and modify it using any text editor. This also makes cheating easy, so additional security measures would have to be implemented. – IInspectable Sep 01 '13 at 20:22