Few days ago, my colleague Paolo came to me with an interesting problem he was not able to solve. He had the same code in production and and in a unit test but the behavior he was seeing was completely different: the production code used to work fine, while the unit test crashed.

The Issue

Simplifying and removing the sensitive information, this is the failing code of the unit test. The program crashed on the second fprintf(), in the call to strlen().

// Build with: gcc -Wall -O2 -o test test.c -lglib-2.0

#include <stdio.h>
#include <string.h>

int main(void)
{
        char *s = g_strdup_printf("This is a random string with a number: %d\n", 42);
        fprintf(stderr, "Pointer is: %p\n", s);
        fprintf(stderr, "Length is: %ld\n", strlen(s));
        g_free(s);
        return 0;
}

If you are not familiar with `g_strdup_printf()`, the documentation is here.

After making a lot of (wrong) hypothesis and (useless) modifications to the code, I fixed the thing that was bothering me since the beginning but I thought it was not important: I changed the type of the pointer s from char to gchar. Guess what? The compiler threw an error and with that error, the solution became clear.

The Solution

To me, it became clear also that some kind of warnings shall never be ignored on 64 bit machines. If you are curious about the solution, we added the following line at the beginning of the file:

#include <glib.h>

Now, if you are educated enough on how C compilers work, you should already have understood everything. If you want a boring verbose explanation, keep reading.

The Explanation

Let's start from the warnings the compiler was notifying us:

warning: implicit declaration of function ‘g_strdup_printf’

warning: initialization of ‘char *’ from ‘int’ makes pointer from integer without a cast

warning: implicit declaration of function ‘g_free’

The first and the third were telling us that we didn't include the declaration to those GLib functions. Of course: the header file was not included. The build process generated an executable file because the linker has been able to find the correct shared object containing the two functions.

But the reason of the crash was in the second warning: the compiler assumes that every function that is not defined returns an integer (I mentioned it in this post).  Now, on a 32 bit system, integers and pointers have (usually) the same size: 4 bytes == 32 bits.

On 64 bit systems, integers are still 32 bits, but pointers are 64 bits. So the compiler was truncating the most significant 4 bytes of the address when assigning the result of g_strdup_printf() to the pointer s.

This resulted in the pointer being invalid and cause a crash as soon as it was referenced. However, since in C the initial value of a variable is undefined, it could also happen that s points to some valid address. In this situation, the program won't crash, but the pointer could reference anything in the program memory, leading to security issues.

Now you should have understood that:

• the compiler threw an error after using the right type (gchar *) because it could not make any assumption on it
• including the right header file solved the issue by giving the compiler all the information it needed to correctly manage the pointer

A final note: when the compiler tells you that you are making a pointer from integer without a cast, the solution is never to add a cast.

Cover image by Possessed Photography from Unsplash licensed under the Unsplash License

Gandalf meme made by me with imgflip