Search code examples
creturnintelicc

Return statement does not get executed in c


So, I have a curious case and can't quite figure out what I've done wrong. Here's the scenario:

I have written a creator function that should return a pointer to a function. To fill the structure with data, I read in a text file. Depending on what text file I use as input, the error either occurs or it doesn't occur. (The error occurs for a text file with ~4000 lines and not for a file with ~200 if that makes a difference). The strange thing is that the code executes until right before the return statement. But it then does not return but just hangs. No errors, no compiler warnings (intel compiler). I wonder if anyone has experienced something similar or has an idea what could be going wrong.

The code below is simplified to illustrate the problem. The actual code is somewhat more complex since I'm using Schreiner's Approach to play with objects in C.

struct Somestruct {
    int A;
    int B;
    int C;
}

static void *Somestruct_ctor(void *_self) {
    struct Somestruct *self = _self;
    fillDataFromFile(self);
    printf("This line gets executed.\n");
    return self; // <- this one doesn't
}

int main(int argc, char *argv[]) {
    void *myObject;

    myObject = Somestruct_ctor(myObject);
    printf("The code does NOT get until here\n");
    return 0;
}

Solution

  • void * myObject; is uninitialized, and not pointing at valid storage. Reading its value (to pass it as an arg by value to Somestruct_ctor(myObject)) is undefined behaviour.

    The fact that your code doesn't always crash tells us that in ICC's code-gen it happens to be pointing somewhere valid, probably somewhere on the stack. With a larger file, we presumably get a buffer overflow that overwrites a local variable and/or a return address and ends up in an infinite loop. It's pretty amazing that this managed to still not crash given that it happened by accident. (In the x86-64 asm from ICC with optimization disabled, it just loads some uninitialized stack memory as an arg for Somestruct_ctor.)

    Or maybe its a pointer to a stdio data structure, left over from init of stdio before main. Perhaps having fillDataFromFile scribble all over the data that FILE *stdout points to (for example) left it in a "locked" state, so your single thread is stuck waiting for something else to unlock a mutex. If you know asm, it might be interesting to single-step the infinite loop or "deadlock" inside printf and see exactly what happened.


    If you compile with gcc -O3, the compiler zeros a register as an arg for fillDataFromFile (after inlining Somestruct_ctor), so it's passing a NULL pointer. That would presumably crash always, assuming the function dereferences the pointer.

    clang chooses to leave rdi (the first arg-passing register in the x86-64 System V calling conventino) uninitialized, so it still holds argc when main calls fillDataFromFile. That would also reliably crash.


    You forgot to enable compiler warnings.

    All the major x86 compilers (gcc, clang, MSVC, ICC) have warnings for this, but they aren't on by default in all compilers (only in MSVC). Probably because there can be cases where the compiler isn't sure about a var being uninitialized if there's some conditional stuff. In this case it's 100% certain that it's definitely used uninitialized, but if the init and the use were inside different if() blocks, the compiler might not be able to prove that the use only happened if the init happened.

    With clang and gcc, you should usually use -Wall and silence all the warnings.

    With ICC, -diag-enable:warn is closer to gcc -Wall. (ICC's -Wall doesn't enable this very important warning. Don't be fooled into thinking you've enabled all important warnings with icc -Wall.)

     # from icc -diag-enable:warn on your code
    <source>(21): warning #592: variable "myObject" is used before its value is set
        myObject = Somestruct_ctor(myObject);
                                   ^
    

    how to turn on icc/icpc warnings? has some info. It ways that icc's -Wall is very minimalisitc compared to gcc's. So maybe -Wall -Wextra would be useful with icc. It recommends -w2 or -w3 as potentially-useful warning levels.


    Clang usually has the nicest warnings, in this case:

    <source>:21:30: warning: variable 'myObject' is uninitialized when used here [-Wuninitialized]
      myObject = Somestruct_ctor(myObject);
                                 ^~~~~~~~
    <source>:19:18: note: initialize the variable 'myObject' to silence this warning
      void * myObject;
                     ^
                      = NULL
    1 warning generated.
    

    I got the above outputs by compiling your source on the Godbolt compiler explorer (after fixing the syntax errors: missing semicolon after the struct, and the capitalization of the Struct keyword.) -xc tells the C++ compilers on Godbolt to compile as C.

    It turns out that you don't need to enable optimization for icc and gcc to notice this error. Some warnings only work with optimization enabled, where the compiler does more analysis of program logic and can notice more, but they track uninitialized even at -O0.


    constructor code that would make more sense:

    // C
    int main(void){
      struct Somestruct myObject;     // automatic storage for the object value
      Somestruct_ctor(&myObject);     // pass a pointer to that storage
    }
    

    The object needs to live somewhere. We can get space for it with automatic (a local), static (a static local, or a global), or dynamic storage (malloc).

    Automatic storage + calling the constructor is equivalent to C++ like this, if struct Somestruct has a C++ default constructor declared in the struct/class definition.

    // C++
    int main(void){
      Somestruct myObject;     // calls the default constructor, if there is one
      // destructor will be called at some point when myObject goes out of scope
    }