CODEWITHSUNDEEP
cgccgcc-warning
santileortiz
santileortiz

Reputation: 333

Is gcc's optimizer doing something weird here?

I have a program that triggers -Wmaybe-uninitialized while using any kind of optimizations (-O1, -O2, -O3) the following is the smallest code I could make that reproduces this behavior.

#include <stdio.h>
#include <stdlib.h>
#include <inttypes.h>

struct result {
    int res;
};

struct graph {
    int vertices[5];
};

// In reality this is a backtracking search.
void computational_search (struct graph *g, struct result *out) {
    out->res = 0;
    int i;
    for (i=0; i<5; i++) {
        out->res += g->vertices[i];
    }
}

// In reality queries a database of geometrical graphs.
void next_graph (struct graph *g)
{
    int i;
    for (i=0; i<5; i++) {
        g->vertices[i] += rand();
    }
}

enum format_t {
    FMT = 1,
    FMT_1 = 2
};

int main()
{
    int val = rand()%10;
    int num_graphs = 5;

    struct graph g;
    struct result res;

    uint64_t *count;
    if (val & FMT) {
        count = malloc(sizeof(uint64_t)*num_graphs);
    }

    int i;
    for (i=0; i<num_graphs; i++) {
        next_graph (&g);
        computational_search (&g, &res);
        if (val & FMT) {
            count[i] = res.res; /* ERROR HERE */
        }
    }
    return 0;
}

I'm aware there is an execution path where count is uninitialized, but it's not being used there. Can the optimizer be doing something that may use count uninitialized?.

Compilig with gcc -Wall -O1 test.c outputs this on gcc 5.4.0:

test.c: In function ‘main’:
test.c:43:15: warning: ‘count’ may be used uninitialized in this function [-Wmaybe-uninitialized]
     uint64_t *count;
               ^

The same happens for -O2 and -O3, but not for -O0.

Upvotes: 2

Views: 241

Answers (2)

Mikhail Maltsev
Mikhail Maltsev

Reputation: 1678

Let me say it a bit bluntly: -Wmaybe-uninitialized sucks. What GCC actually tries to do is first perform a lot of optimizing transformations and then check that every use of a variable is preceded by an initialization (hoping that the optimizer is smart enough to get all impossible paths eliminated). IIRC it also has a couple of simple heuristics. With such an approach you should expect false positives.

For example at -O3 the compiler should unswitch the loop in main and transform it into something like this:

if (val & FMT) {
    for (i=0; i<num_graphs; i++) {
        next_graph (&g);
        computational_search (&g, &res);
        count[i] = res.res;
    }
} else {
    for (i=0; i<num_graphs; i++) {
        next_graph (&g);
        computational_search (&g, &res);
    }
}

Then it should notice that we already have a val & FMT check and merge the corresponding branches:

if (val & FMT) {
    count = malloc(sizeof(uint64_t)*num_graphs);
    for (i=0; i<num_graphs; i++) {
        next_graph (&g);
        computational_search (&g, &res);
        count[i] = res.res;
    }
} else {
    for (i=0; i<num_graphs; i++) {
        next_graph (&g);
        computational_search (&g, &res);
    }
}

The path involving an uninitialized use is gone. Now, back to the reality: unswitching does not happen (for some reason). If you add -fno-inline (note: it's just a test, I'm not proposing it as a solution), it does happen and everything works as I described earlier (no warning). IMHO, this warning is very brittle and unpredictable.

Can the optimizer be doing something that may use count uninitialized?.

No, unless it has a bug.

Upvotes: 1

old_timer
old_timer

Reputation: 71546

The compiler appears correct. It is not noticing that you have the same if statement for both use cases, so the latter is treated separately from the former it is seen as a declare a pointer with no memory behind it then use it as if it has memory behind it. 

uint64_t *count;
...
        if (val & FMT) {
            count[i] = res.res; /* ERROR HERE */
        }

gcc is not as smart as you think it is or should be.

EDIT

for many years gcc has not complained about this

unsigned int fun ( unsigned int x )
{
    switch(x&3)
    {
        case 0: return(x);
        case 1: return(x<<1);
        case 2: return(x>>1);
        case 3: return(x+1);
    }
    return(3);
}

But used to complain if you didnt have a return after the switch. Insisted on you putting in dead code that cannot be reached, then complained when you did. No win situation.

Just tried with 5.4.0 against x86 and it doesnt complain either way. Tried with 7.2.0 against arm and it doesnt complain either way.

Happy to see they removed at least one issue, but would like to see both.

There are other optimization bugs that I have found and expect to find more in the future. You are welcome to try to file yours as a bug and see what they say.

Upvotes: 2

Related Questions