CODEWITHSUNDEEP
c++variablesstack
MGZero
MGZero

Reputation: 5982

Stack around variable corrupt, not sure what the problem is

Problem solved, thank you all for the help

I've got a bit of a problem here it's not something that's blowing my program up, but it's just bothering me that I can't fix it. I have a function reading in some data from a file, at the end of the execution, the stack around variable longGarbage is corrupted. I've looked around a bit and found that a possible cause is writing to invalid memory. I cleaned up some memory leaks that I had and the problem still persists. What's confusing me is that it happens when the function finishes executing, so it appears to be happening when the variable goes out of scope. Here's the code...

CHCF::CHCF(std::string fileName)
: PAKID("HVST84838672")
{
FILE * archive = fopen(fileName.c_str(), "rb");
std::string strGarbage = "";
unsigned int intGarbage = 0;
unsigned long longGarbage = 0;
unsigned char * data = 0;
char charGarbage = '0';

if (!archive)
{
    fclose (archive);
    return;
}

for (int i = 0; i < 12; i++)
{
    fread(&charGarbage, 1, 1, archive);
    strGarbage += charGarbage;
}


if (strGarbage != PAKID)
{
    fclose(archive);
    throw "Incorrect archive format";
}
strGarbage = "";

fread(&_gameID, sizeof(_gameID),1,archive);
fread(&_fileCount, sizeof(_fileCount),1,archive);

for (int i = 0; i < _fileCount; i++)
{
    fread(&longGarbage, 8,1,archive); //file offset

    fread(&intGarbage, 4, 1, archive);//fileName

    for (int i = 0; i < intGarbage; i++)
    {
        fread(&charGarbage, 1, 1, archive);
        strGarbage += charGarbage;
    }

    fread(&longGarbage, 8, 1, archive); //fileSize

    fread(&intGarbage, 4, 1, archive); //fileType

    data = new unsigned char[longGarbage];

    for (long i = 0; i < longGarbage; i++)
    {
        fread(&charGarbage, 1, 1, archive);
        data[i] = charGarbage;
    }

    switch ((FILETYPES)intGarbage)
    {
    case MAP:
        _maps.append(strGarbage, new CFileData(strGarbage, FILETYPES::MAP, data, longGarbage));
        break;

    default:
        break;
    }

    delete [] data;
    data = 0;
    strGarbage.clear();
    longGarbage = 0;

}
fclose(archive);
} //error happens here

Here is the CFileData constructor:

CFileData::CFileData(std::string fileName, FILETYPES type, unsigned char *data, long fileSize)
{
_fileName = fileName;
_type = type;
_data = new unsigned char[fileSize];

for (int i = 0; i < fileSize; i++)
    _data[i] = data[i];
}

Upvotes: 3

Views: 2066

Answers (4)

invalidsyntax
invalidsyntax

Reputation: 650

It seems from the other comments and the information provided that the issue is on the C++ side you should use either __int64 for a windows environment, or int64_t for cross platform.

Upvotes: 0

Frank Boyne
Frank Boyne

Reputation: 4580

What's happening is that something is writing to memory around or over the location of longGarbage which is causing the corruption.

You don't say what development environment you are using. One way to diagnose this would be to set a breakpoint that triggers when a specific memory location changes. Choose a memory location that overlaps the area of corruption and wait for it to trigger unexpectedly.

Another way to diagnose this would be to examine code that changes memory around or over longGarbage. That could be almost anything of course but likely candidates are modifications to 'data', modifications to 'intGarbage' and modifications to 'longGarbage' itself.

We can narrow things down even further because we can (usually) be fairly sure the assignment operator itself is safe. Code like data = new... isn't likely to be the culprit so really we need to focus on memory changes that involve taking the address of 'data', 'intGarbage' or 'longGarbage'. In particular memory changes that change more bytes than they should.

Several others have already pointed out that a long is probably not eight bytes in length. If you pass the wrong length to fread, the extra bytes retrieved have to go somewhere.

Upvotes: 1

Andr&#233; Caron
Andr&#233; Caron

Reputation: 45304

You are using a lot of magic numbers for data sizes, so I would check that first. In particular, I doubt that sizeof(unsigned long)==8 and sizeof(unsigned in)==4 in all possible circumstances. Refer to your compiler's documentation, but you should still be wary, as this is very likely to change from one compiler/platform to another.

Check for these bits:

fread(&longGarbage, 8,1,archive); //file offset

You also might want to use C++ <iostream> library instead of the C FILE* stuff for reading. It would allow for a much shorter version because you wouldn't need to close the file 3 times.

Upvotes: 0

Billy ONeal
Billy ONeal

Reputation: 106609

  1. Might I suggest std::vector instead of calling new and delete manually? Your code is not exception safe -- you leak if an exception is thrown.

  2. fread(&longGarbage, 8, 1, archive); //fileSize Are you sure sizeof(long) is 8? I suspect it's 4. I believe on Linux boxes sometimes it's 8, but most everywhere else sizeof(long) is 4, and sizeof(long long) is 8.

  3. What about any constructors on members of this class? They can corrupt the stack too.

Upvotes: 2

Related Questions