Which part of this C++ program accesses memory is not belonging to me?

Question

unsigned char* readBMP(char* filename) {
    int i;
    FILE* f = fopen(filename, "rb");
    unsigned char info[54];
    fread(info, sizeof(unsigned char), 54, f); // read the 54-byte header

    // extract image height and width from header
    int width = *(int*)&info[18];
    int height = *(int*)&info[22];

    int size = 3 * width * height;
    unsigned char* data = new unsigned char[size]; // allocate 3 bytes per pixel
    fread(data, sizeof(unsigned char), size, f); // read the rest of the data at once
    fclose(f);

    for(i = 0; i < size; i += 3) {
            unsigned char tmp = data[i];
            data[i] = data[i+2];
            data[i+2] = tmp;
    }

    return data;
}

int main() {
    char* filename = "image.bmp";
    unsigned char* originalImageData = readBMP(filename);
    for (int i=0;i<sizeof(originalImageData);i++) {
        cout << originalImageData[i];
    }

    return 0;
}

When running the program it results in:

Segmentation fault (core dumped)

By the way, is there a way to locate this kind of runtime error without using some IDE? (This is a CMake project, so I don't think gcc -g and gdb would work.)

`sizeof(originalImageData)` -- This does not do what you think it does. — PaulMcKenzie, Dec 19 '19 at 10:37
Try a debugger or just adding some prints to see how far it gets before crashing. The fact you don't check the `fopen` result is scary... — John3136, Dec 19 '19 at 10:38
This project is a CMake project, so actually I compiled it using cmake and make — TaihouKai, Dec 19 '19 at 10:40
`int width = *(int*)&info[18];` breaks strict aliaising rule (and doesn't handle aliasing neither anyway). And so it is pedantically UB. — Jarod42, Dec 19 '19 at 10:40
Maybe you should be focused on *why* you are getting segmentation faults. The problems with the code are elementary issues (such as misuse/misunderstanding of `sizeof`, dangerous C-style casting, etc.). — PaulMcKenzie, Dec 19 '19 at 10:46
`char* filename = "image.bmp";` is not a valid line in C++. Read [this](https://stackoverflow.com/questions/57842756/why-should-i-always-enable-compiler-warnings). — n. m. could be an AI, Dec 19 '19 at 10:49

score 4 · Accepted Answer · answered Dec 19 '19 at 10:43

Most likely, it's this line:

fread(info, sizeof(unsigned char), 54, f); // read the 54-byte header

If the fopen fails, which you don't check, f will be NULL. And NULL definitely doesn't point to memory that belongs to you.

You have two other issues as well:

int width = *(int*)&info[18];
int height = *(int*)&info[22];

This won't work. Nobody stored an int there, so you can't read one. It can crash on some architectures and the results will depend on non-portable details such as sizeof(int). Do the conversions correctly.

Also:

unsigned char* originalImageData = readBMP(filename);
for (int i=0;i<sizeof(originalImageData);i++) {
    cout << originalImageData[i];
}

Since originalImageData is a pointer, sizeof(originalImageData) is the size of that pointer. That's not what you want. Why do you care how big the pointer is?

score 0 · Answer 2 · answered Dec 19 '19 at 10:56

First things first: This is not a C++ program. Throwing a plain C program to a C++ compiler does not make it C++.

Your second question: Segfaults can be located by mainly two techniques:

a debugger. GDB if you are on gcc/clang, this is a commandline tool.
put cout statements throughout your code, so you see how far it runs before the segfault. It's a bit tedious - but it works.

Your initial question is difficult to answer without running it through a compiler/debugger. (Hint: it's not the purpose of stackoverflow to have other users debug your code)

There are a few obvious issues with the code where you take assumptions like the file length or the existence of an integer at a specific place (info[18]). What if that integer reads not the correct value? You will have vastly wrong values for data and the size of your image. @DavidSchwartz has already provided quite a good answer.

There is a saying in C++:

when you have new in your code, you might have a memory leak - when you have delete in your code you know that you have a memory leak.

Which part of this C++ program accesses memory is not belonging to me?

2 Answers2