Access Violation

[mackenziejgoodwin]

I have been trying to track down this segmentation fault and I just can't seem to find it. I feel bad pasting so much code but I have been at this for upwards of 4 hours and still haven't found the problem. I have tracked it down to a point where I know that the fault always happens on the last element of the split (last string created when freed causes a fault)

>>>>>>>[New Thread 3768.0x1a8c]
warning: HEAP[Parser.exe]: 
warning: Invalid address specified to RtlFreeHeap( 007D0000, 007D1A48 )

Program received signal SIGTRAP, Trace/breakpoint trap.
0x772102d8 in ntdll!RtlpNtSetValueKey () from C:\WINDOWS\SYSTEM32\ntdll.dll

    int length = 0;

    str string = strNew((char *) "asd as ww");

    string = strAppend(string, (char *) " asd", 4);

    str* array = strSplit(string, (char) 32, length);

    for(int i = 0; i < length; i++) {
        strFree(array[i]);
    }

    strFree(string);

String library

typedef char *str;

struct __attribute__ ((__packed__)) strstruct {
    int length;
    int allocated;
    volatile char buff[];
};

using namespace std;

static inline size_t getLength(const str string) {
    return (size_t) (((struct strstruct*)(string)) - sizeof(struct strstruct))->length;
}

static inline void setLength(const str string, int newLength) {
    (((struct strstruct*)(string)) - sizeof(struct strstruct))->length = newLength;
}

static inline size_t getAllocated(const str string) {
    return (size_t) (((struct strstruct*)(string)) - sizeof(struct strstruct))->allocated;
}

static inline void setAllocated(const str string, int newAllocated) {
    (((struct strstruct*)(string)) - sizeof(struct strstruct))->allocated = newAllocated;
}

#define STRLENGTH (sizeof(strstruct))

str strNew(char *init) {
    return strNewLength(init, strLength(init));
}

str strNewLength(char *init, int length) {
    void *pointer = malloc(STRLENGTH + length + 1);
//    memset(pointer, 0, STRLENGTH + length + 1);

    str string = ((str) pointer) + STRLENGTH;

    setLength(string, length);
    setAllocated(string, length);

    if (init != NULL)
        memcpy(string, init, (size_t) length);

    string[length] = '\0';

    return string;
}

str strSubString(str string, int startIndex, int endIndex) {
    if (endIndex == -1 || startIndex == -1)
        return NULL;
    return strNewLength(string + startIndex, endIndex - startIndex);
}

int strIndexOf(str string, char divisor, int index) {
    for (int i = 0; i < getLength(string); i++) {
        if (string[i] == divisor) {
            if (index == 0) {
                return i;
            }
            --index;
        }
    }
    return -1;
}

int strNumberFound(str string, char divisor) {
    int num = 0;
    int length = getLength(string);
    for (int i = 0; i < length; ++i) {
        cout << string[i] << endl;
        if (string[i] == divisor)
            ++num;
    }
    return num;
}

str strAppend(str original, char *other, int otherLength) {
    int length = getLength(original);
    int allocated = getAllocated(original);
    if (allocated < length + otherLength) {
        void *pointer = malloc(STRLENGTH + length + otherLength + 1);
        str string = (char *) pointer + STRLENGTH;
        memcpy(string, original, (size_t) length);
        memcpy(string + length, other, (size_t) otherLength);
        string[length + otherLength] = '\0';
        setLength(string, length + otherLength);
        strFree(original);
        return string;
    } else {
        memcpy(original + length, other, (size_t) otherLength);
        setLength(original, length + otherLength);
        original[length + otherLength] = '\0';
        return original;
    }
}

str *strSplit(str string, char divisor, int &length) {
    int numFound = strNumberFound(string, divisor);
    str* array = new str[numFound + 1];
    int previousEnd = 0;
    length = 0;
    int startIndex;
    int endIndex;
    for (int i = 0; i < numFound + 1; ++i) {
        startIndex = previousEnd + (i == 0 ? 0 : 1);
        endIndex = strIndexOf(string, divisor, i);
        array[i] = strSubString(string, startIndex, endIndex == -1 ? getLength(string) : (size_t) endIndex);
        previousEnd = endIndex;
        ++length;
    }
    return array;
}

void strFree(str string) {
    if (string == NULL)
        return;

    //memoryHexDump(string - 2, 10);

//    cout << "String length " << getLength(string) << endl;
//    for(int i = 0; i < getLength(string) + 1; i++) {
//        cout << (int)(string[i]) << "|";
//    }

    free(string - STRLENGTH);
    string = nullptr;
}

[Chervil]

pasting so much code

There isn't enough code. Something which can be compiled and run to re-create the problem would be best.

As it is, str seems to be an unknown user-defined type. Function strNew() is not supplied which may be the key.

[mackenziejgoodwin]

Added required stuff.

[ne555]

¿did that compile for you?
- missing headers
- missing main()
- missing strFree() declaration
- missing strNew() definition


> (char *) " asd"
there should be no need to do those castings

[mackenziejgoodwin]

I excluded the header and main because they were not important to solving this and the strFree and strNew are in there. As for the cast I do it so my ide doesn't complain to me.

[Chervil]

There was still a missing function definition strLength() so I made a guess at that.

From what I can see, there is a mistake in the way the offset is calculated in a number of functions. In function getLength(), the pointer string is first cast to type strstruct* and then the length is subtracted.

    return (size_t) (((struct strstruct*)(string)) - sizeof(struct strstruct))->length;

I believe it should be the other way round, do the subtraction first, then cast afterwards.

    return reinterpret_cast<strstruct*>(string - STRLENGTH)->length;

Here's my cut-down version of the code, I only got as far as creating and deleting a string, which was enough in the original code to generate an error. I think this fixes it. I also added const in a few places and used the C++ idiom rather than C in a few places. Inside the struct, I don't believe the buff is needed at all?

#include <cstdlib>  // malloc, free, size_t
#include <cstring>  // memcpy, strlen
#include <iostream> // cout

using std::cout;
using std::endl;

typedef char *str;

str strNew(const char *init);
void strFree(str string);
str strNewLength(const char *init, int length);
static inline size_t getAllocated(const str string);
static inline size_t getLength(const str string);

int strLength(const str);   // my deduced declaration
int strLength(const str s)  // my deduced definition
{
    return strlen(s);
}

int main()
{
    str string = strNew("asd as ww");

    cout << "string =              " << string << '\n';
    cout << "string getAllocated = " << getAllocated(string) << '\n';
    cout << "string getLength =    " << getLength(string) << '\n';

    strFree(string);
}

struct strstruct {
    int length;
    int allocated;
};


const size_t STRLENGTH = sizeof(strstruct);

static inline size_t getLength(const str string) {
//  return (size_t) (((struct strstruct*)(string)) - sizeof(struct strstruct))->length;
    return reinterpret_cast<strstruct*>(string - STRLENGTH)->length;
}

static inline void setLength(const str string, int newLength) {
//  (((struct strstruct*)(string)) - sizeof(struct strstruct))->length = newLength;
    reinterpret_cast<strstruct*>(string - STRLENGTH)->length = newLength;
}

static inline size_t getAllocated(const str string) {
//  return (size_t) (((struct strstruct*)(string)) - sizeof(struct strstruct))->allocated;
    return  reinterpret_cast<strstruct*>(string - STRLENGTH)->allocated;
}

static inline void setAllocated(const str string, int newAllocated) {
//  (((struct strstruct*)(string)) - sizeof(struct strstruct))->allocated = newAllocated;
    reinterpret_cast<strstruct*>(string - STRLENGTH)->allocated = newAllocated;
}

str strNew(const char *init) {
//    return strNewLength(init, strLength(init));
    return strNewLength(init, strlen(init));
}

str strNewLength(const char *init, int length) {
    void *pointer = malloc(STRLENGTH + length + 1);

    str string = static_cast<str>(pointer) + STRLENGTH;

    setLength(string, length);
    setAllocated(string, length);

    if (init != NULL)
        memcpy(string, init, (size_t) length);

    string[length] = '\0';

    return string;
}

void strFree(str string)
{
    if (string == nullptr)
        return;

    free(string - STRLENGTH);
    string = nullptr;
}

[ne555]

> I excluded the header and main because they were not important to solving this
you don't seem to have any idea of what is important.
your testcase ought to be self-contained. I should be able to copy-paste your code, compile, run it and get the segmentation fault.
but here, I get compile errors and have to mess with your code to add missing headers. Five more lines that you were responsible to write, and you did write them, but decided to remove them for us.

> and the strFree and strNew are in there.
strFree() declaration is still missing
I see strNew() now, apologies if it was there before.
And now, there is missing declarations for strNewLength()
Functions need to be declared prior its first use, otherwise you get a compile error.

> There was still a missing function definition strLength() so I made a guess at that.
and later we realize that the length computed was 10 times longer...


@OP: If you reach the character limit, use github to upload your code. If you need more than 1 file to reproduce your issue, upload to github.


> As for the cast I do it so my ide doesn't complain to me.
Instead of the casting you should have changed the signatures of the functions so they ask for a const char*. After all, you don't plan to modify that parameter.