c99 goto past initialization

Question

While debugging a crash, I came across this issue in some code:

int func()
{
    char *p1 = malloc(...);
    if (p1 == NULL)
        goto err_exit;

    char *p2 = malloc(...);
    if (p2 == NULL)
        goto err_exit;

    ...

err_exit:
    free(p2);
    free(p1);

    return -1;
}

The problem occurs when the first malloc fails. Because we jump across the initialization of p2, it contains random data and the call to free(p2) can crash.

I would expect/hope that this would be treated the same way as in C++ where the compiler does not allow a goto to jump across an initialization.

My question: is jumping across an initialization allowed by the standard or is this a bug in gcc's implementation of c99?

Answer 1

This is not a bug in gcc. A jump is just a jump in C. There is no special logic applied. The issue is that you are not initializing your pointers to NULL first. If you were to do that then you free call would be free(NULL) which would not crash. Start the function with char *p1 = NULL, *p2 = NULL; and all will be well.

Answer 2

You can ask gcc to warn you when you jump over a variable definition by using -Wjump-misses-init and then you can use -Werror to force the users to deal with it. This warning is included in -Wc++-compat so the gcc developers are aware that the code behaves differently in C versus C++.

You could also change the code slightly:

int func()
{
    char *p1 = malloc(...);
    if (p1 == NULL)
        goto err_exit_1;

    char *p2 = malloc(...);
    if (p2 == NULL)
        goto err_exit_2;

    ...

err_exit_2:
    free(p2);
err_exit_1:
    free(p1);

    return -1;
}

... and just keep pairing labels with initialized variables. You'll have the same problem with calling many other functions with unitialized variables, free just happens to be a more obvious one.

Answer 3

Using gotos is not a smart idea, and you've just found one reason why. You should call an error handling function for each individual error.

Answer 4

A jump like that is indeed allowed by the standard, so this is not a bug in GCC. The standard lists this situation as a suggested warning in Annex I.

The only restriction imposed on jumps in C99 with regard to scope is that it is illegal to jump into scope of a variable of variably modified type, like a VLA

int main() {
  int n = 5;
  goto label; // <- ERROR: illegal jump
  int a[n];
label:;
}

In other words, it is not correct to say that "a jump is just a jump in C". Jumps are somewhat restricted when it comes to entering variable scope, albeit not as strictly as in C++. The situation you describe is not one of the restricted ones.

Answer 5

if i compile this code with -O2 flag

gcc -Wall -std=c99 -O2 jump.c

i've got warning:

jump.c: In function ‘func’:
jump.c:10: warning: ‘p2’ may be used uninitialised in this function

and no warning without optimization

Answer 6

As AndreyT says, jumping over the initialisation is allowed by C99. You can fix your logic by using seperate labels for the two failures:

int func()
{
    char *p1 = malloc(...);
    if (p1 == NULL)
        goto err_exit_p1;

    char *p2 = malloc(...);
    if (p2 == NULL)
        goto err_exit;

    ...

err_exit:
    free(p2);
err_exit_p1:
    free(p1);

    return -1;
}

This is a standard pattern - "early errors" cause a jump to a later part of the error exit code.

Answer 7

Hmm, it's not because the new standard allows for variable declarations anywhere that it's always a good idea to use it. In your case I would do like we did it in classic C.

int func()
{
char *p1 = NULL;    /* So we have a definite value */
char *p2 = NULL;

  p1 = malloc(...);
  if(!p1)
    goto err_exit;

  p2 = malloc(...);
  if(!p2)
    goto err_exit;

  ...

  err_exit:
    free(p2);
    free(p1);

  return -1;
}