runtime error : return value gets lost

[zsteve]

Hi everyone
I'm working on this project : https://github.com/zsteve/zebra-zmachine-interpreter
which is supposed to be an interpreter for z-machine story files, but I'm still working on it.
Anyway, the error I'm having is within ztext.cpp and ZDictionary's unit test main.cpp.
zChartoDictionaryZCharString() is supposed to return a pointer to a string/array of zwords.
It is creating/calculating that array properly, but is strangely failing to return it properly...
the error occurs at line 499 of ztext.cpp :
return (outWord);
Right at this point, outWord points to an array of zwords : {15 25, 28 ab} allocated by new[]
Once it returns to line 42 in https://github.com/zsteve/zebra-zmachine-interpreter/blob/master/zdictionary/zdictionary/main.cpp ,
the returned zword* now points to {50 00, 00 00}, which shouldn't happen.

I spent an hour debugging the entire thing until I figured out that this was where it went wrong.
So my question is : what causes this problem in my program?
I suspect it's either unmatched new[]/delete[] (could this cause it?) or the std::vector objects in zChartoDictionaryZCharString().
Here is the code :

in main.cpp (/zdictionary/zdictionary/main.cpp) from line 40:

 
    // error occurs here :
    // at this point : tmp == {15 25 28 ab}
    zword* tmp=zChartoDictionaryZCharString(tmp_);

in ztext.cpp (/ztext/ztext/ztext.cpp) from line 502:

zword* zChartoDictionaryZCharString(zword* zstring)
{
// converts a normal zchar string to a dictionary zchar string
// returns a zword* to either a 2 or 3 zword array.
    std::vector<zchar> vector1=expandZChars(zstring);
    std::vector<zchar> vector2(0);
    if(zVersion<=3){
        // for versions 1 to 3, dictionary word length is 4 bytes (6 zchars)
        int done=0;
        for(int i=0; i<vector1.size() && i<6; i++, done++)
        {
            vector2.push_back(vector1[i]);
        }
        if(done<6)
        {
            for(int i=done; i<6; i++)
            {
                vector2.push_back(5);
            }
        }
        // now we need to pack them to 2x zwords.
        zword* outData=new zword[2];
        zchar* zcharArray=new zchar[vector2.size()];
        for(int i=0; i<vector2.size(); i++) zcharArray[i]=vector2[i];
        outData[0]=endianize(packZChars(zcharArray));
        outData[1]=endianize(packZChars(zcharArray+3)|128);
        delete[] zcharArray;
        vector1.clear();
        vector2.clear();
        return outData;
    }else if(zVersion>3){
        // for versions 4 and up, dictionary word length is 6 bytes (9 zchars)
        int done=0;
        for(int i=0; i<vector1.size() && i<9; i++, done++)
        {
            vector2.push_back(vector1[i]);
        }
        if(done<9)
        {
            for(int i=done; i<9; i++)
            {
                vector2.push_back(5);
            }
        }
        // now we need to pack them to 3x zwords.
        zword* outData=new zword[3];
        zchar* zcharArray=new zchar[vector2.size()];
        for(int i=0; i<vector2.size(); i++) zcharArray[i]=vector2[i];
        outData[0]=packZChars(zcharArray);
        outData[1]=packZChars(zcharArray+3);
        outData[2]=packZChars(zcharArray+6);
        outData[2]|=128;
        for(int i=0; i<3; i++) outData[i]=endianize(outData[i]);
        delete[] zcharArray;
        return outData;
    }
}

Any help on fixing this would be greatly appreciated.
Thanks!

[cire]

The usual culprit for such things is memory corruption, which is generally caused by doing something to memory you don't own. If this snippet of code is any indication of how you normally handle memory management, that wouldn't be terribly surprising.

[zsteve]

ok, I have got a test program to throw me the same error as well :
http://pastebin.com/MDN4Qa5x
exactly the same problem I am having in ZDictionary/ZText

short* test()
{
    std::vector<short> tV(0);
    std::vector<short> tV2=randVect();
    if(someCondition==1){
        short *tmp=new short[2];
        cout << "test() says : " << tmp << endl;
        return tmp;
    }else if(someCondition==2){
        short *tmp=new short[4];
        cout << "test() says : " << tmp << endl;
        return tmp;
    }
}

this piece of code allocates 2 vectors ( I think 1 vector makes the same problem too),
checks a condition, and returns two different values based on the value of the condition,
new short[2] or new short[4].
I think it's probably something to do with the vector's destructor, as EAX holds the wrong value
when it's at the last brace of test()...

so during runtime, I have
test() says : some value here
main() says : some other value here

@cire : And could you please tell me what I'm doing wrong with my memory management? I'd like to know so I can avoid it because it generally is how I write my code :P

[ne555]

> what I'm doing wrong with my memory management?
you are doing it.

> I have got a test program to throw me the same error as well :
¿are you sure? http://ideone.com/xvA8go

> I'm working on this project : https://github.com/zsteve/zebra-zmachine-interpreter
don't commit the build results to git.
Also, ¿don't you read the warnings?


http://www.eelis.net/iso-c++/testcase.xhtml (point 6)

[cire]

As ne555 points out, your "test case" doesn't reproduce the problem in my local installations of GCC or MSVC++. The code is small enough to reason about that it is obvious it shouldn't reproduce the problem.

Using manual memory management increases the opportunity for errors in your code. Your function returns an array of variable length, but how does the caller know which size was allocated? Why not, instead, return a vector whose size can be queried by the caller? Why do you create temporary dynamically allocated arrays simply to copy the values from a vector that are already there and accessible via the vector? The dynamic array is superfluous.

At least, if you insist on using dynamic arrays, also use smart pointers to manage the lifetime of the arrays, and you should probably return a smart pointer from the function as well if you insist on not using a container.

[zsteve]

http://www.onlinecompiler.net/ compiling here does apparently reproduce the problem
I download the exe and I get:
http://0x4c.phatcode.net/img/error.bmp
only thing I added was a getch() so I could take a screenshot...
I'm using Code::Blocks/GCC on Windows.

It is apparently caused (I think) by std::vector's destructor being called at the last closing brace of test(), as the code runs fine (on my pc) If I rewrite test() as

short* test()
{
    if(someCondition==1){
    std::vector<short> tV(0);
    std::vector<short> tV2=randVect();
        short *tmp=new short[2];
        cout << "test() says : " << tmp << endl;
        return tmp;
    }else if(someCondition==2){
    std::vector<short> tV(0);
    std::vector<short> tV2=randVect();
        short *tmp=new short[4];
        cout << "test() says : " << tmp << endl;
        return tmp;
    }
}

[cire]

What version of GCC are you using?

[zsteve]

I'm feeling rather foolish... :P
Problem disappeared after I added a return 0 after the if block, even though it would never be executed.
Looks like some compiler specific thing as MSVC has no problem
... so problem solved

[cire]

That shouldn't be important so long as someCondition was either 1 or 2. If it was ever anything else, of course, you run into undefined behavior.