Fixing the GCC 4.4 bug "warning: attempt to free a non-heap object"

GCC 4.4 has a bug. It tries to track if references to "free()" are really allocated. However it fails to track variables correctly. This is extremely annoying, because for example Debian requires to compile with -Werror, such that this bogus warning makes packets fail even if they correct.

Presented here is a general way to escape from this bug. The method is to wrap the call to free() into a function which is only called pointer in question into a deallocator which is only called when the pointer was allocated. The other way (doing it with an if()) fails (see idiom1full.c).

This bug neither applies to GCC4.3 nor GCC4.5. GCC4.3 does not track variables which are free()d, while (rumours tell me that) GCC4.5 corrcetly tracks the origin of a value, such that it can detect the cases in the examples here correctly.

However as seen in the fix, it is not really a drawback. Instead circumventing the GCC4.4 bug will improve the usability and quality of your code. Well, perhaps it's one of those rare healthy bugs, then.

  1. Fixing the GCC 4.4 bug "warning: attempt to free a non-heap object"
    1. Introduction
    2. A general programming pattern
    3. The fix
    4. The improved next iteration
    5. Last step in evolution
    6. Conclusion
    7. Full sources
      1. diffs
        1. pattern1.c to pattern1fix.c
        2. pattern1fix.c to pattern1full.c
        3. pattern1full.c to pattern1last.c
        4. pattern1.c to pattern1last.c
      2. pattern1.c
      3. pattern1fix.c
      4. pattern2full.c
      5. pattern2last.c
    8. Erratas

Introduction

The archive of the sources shown below can be freely used and distributed as it is CLLed code.

When compiling the sources in the archive you will see this
~/gcc4.4-bug-pattern$ gcc --version
gcc (Debian 4.4.5-8) 4.4.5
Copyright (C) 2010 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

~/gcc4.4-bug-pattern$ make
cc -Wall -O3    pattern1.c   -o pattern1
pattern1.c: In function 'main':
pattern1.c:70: warning: attempt to free a non-heap object 'staticnull'
cc -Wall -O3    pattern1fix.c   -o pattern1fix
cc -Wall -O3    pattern1full.c   -o pattern1full
pattern1full.c: In function 'main':
pattern1full.c:72: warning: attempt to free a non-heap object 'staticnull'
cc -Wall -O3    pattern1last.c   -o pattern1last

~/gcc4.4-bug-pattern$ make clean
rm -f *.o pattern1 pattern1fix pattern1full pattern1last *~

A general programming pattern

There is a general programming pattern found in trainloads of programs. That is some structure which can be pre-allocated or dynamically-allocated at the same time. The idea is that "static" variables are 0-initalized, and so can be used as some global buffers, while internally you can use the same thing dynamically for temporary data.

Here is the excepts of such a source (full source see below). It does nothing, but you will see the pattern:

struct idiom1
  {
     struct idiom1      *next;
     int                allocated;
     int                fill, size;
     char               *buffer;        /* always allocated     */
  };
static struct idiom1 *
idiom1_new(void)
{
  struct idiom1 *ptr;

  ptr   = alloc0(sizeof *ptr);
  ptr->allocated = 1;
  return ptr;
}
static void
idiom1_free(struct idiom1 *ptr)
{
  if (ptr->buffer)
    free(ptr->buffer);
  ptr->buffer   = 0;    /* paranoia     */
  if (ptr->allocated)
    free(ptr);  /* GCC4.4.5-8: idiom1.c:66: warning: attempt to free a non-heap object 'staticnull' */
}
int
main(int argc, char **argv)
{
  static struct idiom1  staticnull;
  struct idiom1 *dynalloc, *staticptr;

  staticptr     = idiom1_init(&staticnull);
  dynalloc      = idiom1_init(NULL);

  /* do something with &staticnull or dynalloc  */

  idiom1_free(staticptr);
  idiom1_free(dynalloc);

  return 0;
}

Making that leads to following annoying warning:
cc -Wall -O3    pattern1.c   -o pattern1
pattern1.c: In function 'main':
pattern1.c:70: warning: attempt to free a non-heap object 'staticnull'

The fix

The problematic part is, that GCC4.4 does not understand, that ptr->allocated only is true if ptr was malloc()ed. So the fix involves to change the flag into a pointer, which is non-NULL when it is malloc()ed and NULL when it is static. Apparently GCC groks this:

struct idiom1
  {
     struct idiom1      *next;
     void *             allocated;      /* changed to pointer to free() */
     int                fill, size;
     char               *buffer;        /* always allocated     */
  };
static void
idiom1_free(struct idiom1 *ptr)
{
  if (ptr->buffer)
    free(ptr->buffer);
  ptr->buffer   = 0;    /* paranoia     */
  if (ptr->allocated)
    free(ptr->allocated);       /* GCC4.4.5-8: no warning anymore, however there's more */
}

This compiles cleanly with GCC 4.4.

The improved next iteration

Well, programmers are not just interested in doing things right, they want to know everything is right. Now that we have a pointer it would be wise to check if the pointer is the right one to free(), so we test if ptr==ptr->allocated. But GCC 4.4 does not want this!

static void
idiom1_free(struct idiom1 *ptr)
{
  if (ptr->buffer)
    free(ptr->buffer);
  ptr->buffer   = 0;    /* paranoia     */
  if (ptr->allocated)
    {
      if (ptr->allocated==ptr)
        free(ptr->allocated);   /* GCC4.4.5-8: idiom1full.c:68: warning: attempt to free a non-heap object 'staticnull' */
    }
}

However we are now not informed if something is wrong. Doing so is not easy, as usually it is deeply in the lib. Using ASSERT() does not work for production code either, so we come up with another idea:

If the pointer points elsewhere, we use it as a deallocator. This way the pointer is exdecuted, and if it points somewhere badly we will nearly always see a SIGSEV etc. Adding this is easy, just 2 lines of code, the routine now looks like:

static void
idiom1_free(struct idiom1 *ptr)
{
  if (ptr->buffer)
    free(ptr->buffer);
  ptr->buffer   = 0;    /* paranoia     */
  if (ptr->allocated)
    {
      if (ptr->allocated==ptr)
        free(ptr->allocated);   /* GCC4.4.5-8: idiom1full.c:68: warning: attempt to free a non-heap object 'staticnull' */
      else
        (((void(*)(struct idiom1 *))ptr->allocated)(ptr));      /* call deallocator     */
    }
}

However compiling that we now get:
cc -Wall -O3    pattern1full.c   -o pattern1full
pattern1full.c: In function 'main':
pattern1full.c:72: warning: attempt to free a non-heap object 'staticnull'

Apparently GCC4.4 sees that ptr->allocated in the call to free() is aliased to ptr (wow! That's quite a piece of work for an optimizer to spot that!), and now the bug hits again.

Last step in evolution

Now having a deallocator it is more clean to always call it. This gets rid of the "if (ptr==ptr->allocated)", and removing a conditional always is a good thing. It reduces complexity and can improve runtime as well.

So we introduce a deallocator and thus re-introduce the general OO pattern which comes free with C++:

struct idiom1
  {
     struct idiom1      *next;
     void               (*dealloc)(struct idiom1 *);    /* changed to deallocator only  */
     int                fill, size;
     char               *buffer;        /* always allocated     */
  };
static void
idiom1_dealloc(struct idiom1 *ptr)
{
  free(ptr);    /* GCC 4.4.5-8: apparently it does grok that this function is never called on static pointers   */
}
static struct idiom1 *
idiom1_new(void)
{
  struct idiom1 *ptr;

  ptr   = alloc0(sizeof *ptr);
  ptr->dealloc  = idiom1_dealloc;
  return ptr;
}
static void
idiom1_free(struct idiom1 *ptr)
{
  if (ptr->buffer)
    free(ptr->buffer);
  ptr->buffer   = 0;    /* paranoia     */
  if (ptr->dealloc)
    ptr->dealloc(ptr);  /* now using deallocator        */
}

This compiles cleanly again.

Conclusion

Now the source has been cleaned up and is more useful, too. The GCC4.4 bug might be quite annoying, but by just changing a little bit of the pattern we improve the code while circumventing the bug.

So the bug is not that bad, it is just an annoyance we can live with.

Full sources

The archive of the sources shown below can be freely used and distributed as it is CLLed code.

Note that the sources shown here differ from the archive, because my CMS is not TAB aware, so I had to replace TABs with spaces.

diffs

pattern1.c to pattern1fix.c

The first (naive) fix includes to use another suitable pointer to free() such that GCC 4.4 is able (or no more able) to track the origin:

--- pattern1.c  2011-02-11 19:54:19.000000000 +0100
+++ pattern1fix.c       2011-02-11 19:54:22.000000000 +0100
@@ -9,7 +9,7 @@
 struct idiom1
   {
      struct idiom1     *next;
-     int               allocated;
+     void *            allocated;      /* changed to pointer to free() */
      int               fill, size;
      char              *buffer;        /* always allocated     */
   };
@@ -43,7 +43,7 @@
   struct idiom1 *ptr;

   ptr  = alloc0(sizeof *ptr);
-  ptr->allocated = 1;
+  ptr->allocated = ptr;
   return ptr;
 }

@@ -67,7 +67,7 @@
     free(ptr->buffer);
   ptr->buffer  = 0;    /* paranoia     */
   if (ptr->allocated)
-    free(ptr); /* GCC4.4.5-8: idiom1.c:66: warning: attempt to free a non-heap object 'staticnull' */
+    free(ptr->allocated);      /* GCC4.4.5-8: no warning anymore, however there's more */
 }

 int

pattern1fix.c to pattern1full.c

Now that you have a pointer you probably want to check if the ->allocated pointer still points to the right memory area (ptr==ptr->allocated) but this fails. We here also introduce (not neccessary to show the bug) the idea, to call the other pointer as deallocator, such that derived structures are possible now:

--- pattern1fix.c       2011-02-11 19:54:22.000000000 +0100
+++ pattern1full.c      2011-02-11 19:54:24.000000000 +0100
@@ -67,7 +67,12 @@
     free(ptr->buffer);
   ptr->buffer  = 0;    /* paranoia     */
   if (ptr->allocated)
-    free(ptr->allocated);      /* GCC4.4.5-8: no warning anymore, however there's more */
+    {
+      if (ptr->allocated==ptr)
+        free(ptr->allocated);  /* GCC4.4.5-8: idiom1full.c:68: warning: attempt to free a non-heap object 'staticnull' */
+      else
+        (((void(*)(struct idiom1 *))ptr->allocated)(ptr));     /* call deallocator     */
+    }
 }

 int

pattern1full.c to pattern1last.c

Now we introduce a real deallocator, which is far more clean than the hack above. The deallocator now is usable by aggregated structures as well which usually is a good thing. So this way GCC4.4 urges us to clean up the code and improve it's usability, which can be seen a good thing:

--- pattern1full.c      2011-02-11 19:54:24.000000000 +0100
+++ pattern1last.c      2011-02-11 20:29:50.000000000 +0100
@@ -9,7 +9,7 @@
 struct idiom1
   {
      struct idiom1     *next;
-     void *            allocated;      /* changed to pointer to free() */
+     void              (*dealloc)(struct idiom1 *);    /* changed to deallocator only  */
      int               fill, size;
      char              *buffer;        /* always allocated     */
   };
@@ -37,13 +37,19 @@
   return ptr;
 }

+static void
+idiom1_dealloc(struct idiom1 *ptr)
+{
+  free(ptr);   /* GCC 4.4.5-8: apparently it does grok that this function is never called on static pointers   */
+}
+
 static struct idiom1 *
 idiom1_new(void)
 {
   struct idiom1 *ptr;

   ptr  = alloc0(sizeof *ptr);
-  ptr->allocated = ptr;
+  ptr->dealloc = idiom1_dealloc;
   return ptr;
 }

@@ -66,13 +72,8 @@
   if (ptr->buffer)
     free(ptr->buffer);
   ptr->buffer  = 0;    /* paranoia     */
-  if (ptr->allocated)
-    {
-      if (ptr->allocated==ptr)
-        free(ptr->allocated);  /* GCC4.4.5-8: idiom1full.c:68: warning: attempt to free a non-heap object 'staticnull' */
-      else
-        (((void(*)(struct idiom1 *))ptr->allocated)(ptr));     /* call deallocator     */
-    }
+  if (ptr->dealloc)
+    ptr->dealloc(ptr); /* now using deallocator        */
 }

 int

pattern1.c to pattern1last.c

Here the complete diff as an evolution pattern. As you can see it is not that complex:

--- pattern1.c  2011-02-11 19:54:19.000000000 +0100
+++ pattern1last.c      2011-02-11 20:29:50.000000000 +0100
@@ -9,7 +9,7 @@
 struct idiom1
   {
      struct idiom1     *next;
-     int               allocated;
+     void              (*dealloc)(struct idiom1 *);    /* changed to deallocator only  */
      int               fill, size;
      char              *buffer;        /* always allocated     */
   };
@@ -37,13 +37,19 @@
   return ptr;
 }

+static void
+idiom1_dealloc(struct idiom1 *ptr)
+{
+  free(ptr);   /* GCC 4.4.5-8: apparently it does grok that this function is never called on static pointers   */
+}
+
 static struct idiom1 *
 idiom1_new(void)
 {
   struct idiom1 *ptr;

   ptr  = alloc0(sizeof *ptr);
-  ptr->allocated = 1;
+  ptr->dealloc = idiom1_dealloc;
   return ptr;
 }

@@ -66,8 +72,8 @@
   if (ptr->buffer)
     free(ptr->buffer);
   ptr->buffer  = 0;    /* paranoia     */
-  if (ptr->allocated)
-    free(ptr); /* GCC4.4.5-8: idiom1.c:66: warning: attempt to free a non-heap object 'staticnull' */
+  if (ptr->dealloc)
+    ptr->dealloc(ptr); /* now using deallocator        */
 }

 int

pattern1.c

This is the common programming pattern: Some structure (idiom1) may be statically initialized or dynamically allocated. GCC 4.4 warns here incorrectly, that the static variable may be free()d without being malloc()ed, even that it is never allocated and thus the free() is never called. This also probably means, that GCC 4.4 here incorrectly does not detect that free() is not reachable with allocated variables and thus does not optimize the code (as it can be left away):

/* This Works is placed under the terms of the Copyright Less License,
 * see file COPYRIGHT.CLL.  USE AT OWN RISK, ABSOLUTELY NO WARRANTY.
 */

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

struct idiom1
  {
     struct idiom1      *next;
     int                allocated;
     int                fill, size;
     char               *buffer;        /* always allocated     */
  };

static void *
alloc(size_t len)
{
  void  *ptr;

  if ((ptr=malloc(len))==0)
    {
      perror("malloc");
      exit(1);
    }
  return ptr;
}

static void *
alloc0(size_t len)
{
  void  *ptr;

  ptr   = alloc(len);
  memset(ptr, 0, len);
  return ptr;
}

static struct idiom1 *
idiom1_new(void)
{
  struct idiom1 *ptr;

  ptr   = alloc0(sizeof *ptr);
  ptr->allocated = 1;
  return ptr;
}

static struct idiom1 *
idiom1_init(struct idiom1 *ptr)
{
  if (!ptr)
    ptr = idiom1_new();
  ptr->fill     = 0;
  if (!ptr->size)
    ptr->size   = BUFSIZ;
  if (!ptr->buffer)
    ptr->buffer = alloc(ptr->size);
  return ptr;
}

static void
idiom1_free(struct idiom1 *ptr)
{
  if (ptr->buffer)
    free(ptr->buffer);
  ptr->buffer   = 0;    /* paranoia     */
  if (ptr->allocated)
    free(ptr);  /* GCC4.4.5-8: idiom1.c:66: warning: attempt to free a non-heap object 'staticnull' */
}

int
main(int argc, char **argv)
{
  static struct idiom1  staticnull;
  struct idiom1 *dynalloc, *staticptr;

  staticptr     = idiom1_init(&staticnull);
  dynalloc      = idiom1_init(NULL);

  /* do something with &staticnull or dynalloc  */

  idiom1_free(staticptr);
  idiom1_free(dynalloc);

  return 0;
}

pattern1fix.c

This fixes the pattern by introducing another variable pointer which apparently either is not understood by GCC 4.4 or is understood correctly by the life-time analysis (don't know which one applies). I vote for the first thing, as pattern2full.c shows, that GCC 4.4 probaly simply does not understand this case here:

/* This Works is placed under the terms of the Copyright Less License,
 * see file COPYRIGHT.CLL.  USE AT OWN RISK, ABSOLUTELY NO WARRANTY.
 */

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

struct idiom1
  {
     struct idiom1      *next;
     void *             allocated;      /* changed to pointer to free() */
     int                fill, size;
     char               *buffer;        /* always allocated     */
  };

static void *
alloc(size_t len)
{
  void  *ptr;

  if ((ptr=malloc(len))==0)
    {
      perror("malloc");
      exit(1);
    }
  return ptr;
}

static void *
alloc0(size_t len)
{
  void  *ptr;

  ptr   = alloc(len);
  memset(ptr, 0, len);
  return ptr;
}

static struct idiom1 *
idiom1_new(void)
{
  struct idiom1 *ptr;

  ptr   = alloc0(sizeof *ptr);
  ptr->allocated = ptr;
  return ptr;
}

static struct idiom1 *
idiom1_init(struct idiom1 *ptr)
{
  if (!ptr)
    ptr = idiom1_new();
  ptr->fill     = 0;
  if (!ptr->size)
    ptr->size   = BUFSIZ;
  if (!ptr->buffer)
    ptr->buffer = alloc(ptr->size);
  return ptr;
}

static void
idiom1_free(struct idiom1 *ptr)
{
  if (ptr->buffer)
    free(ptr->buffer);
  ptr->buffer   = 0;    /* paranoia     */
  if (ptr->allocated)
    free(ptr->allocated);       /* GCC4.4.5-8: no warning anymore, however there's more */
}

int
main(int argc, char **argv)
{
  static struct idiom1  staticnull;
  struct idiom1 *dynalloc, *staticptr;

  staticptr     = idiom1_init(&staticnull);
  dynalloc      = idiom1_init(NULL);

  /* do something with &staticnull or dynalloc  */

  idiom1_free(staticptr);
  idiom1_free(dynalloc);

  return 0;
}

pattern2full.c

This is a full example which allows to introduce your own deallocator in a somewhat unclean fashion. The GCC4.4 hits again, as GCC understands the aliasing (== in the if to the free()) such that it again (incorreclty) warns that the variable may be free()d without beeing malloc()ed:

/* This Works is placed under the terms of the Copyright Less License,
 * see file COPYRIGHT.CLL.  USE AT OWN RISK, ABSOLUTELY NO WARRANTY.
 */

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

struct idiom1
  {
     struct idiom1      *next;
     void *             allocated;      /* changed to pointer to free() */
     int                fill, size;
     char               *buffer;        /* always allocated     */
  };

static void *
alloc(size_t len)
{
  void  *ptr;

  if ((ptr=malloc(len))==0)
    {
      perror("malloc");
      exit(1);
    }
  return ptr;
}

static void *
alloc0(size_t len)
{
  void  *ptr;

  ptr   = alloc(len);
  memset(ptr, 0, len);
  return ptr;
}

static struct idiom1 *
idiom1_new(void)
{
  struct idiom1 *ptr;

  ptr   = alloc0(sizeof *ptr);
  ptr->allocated = ptr;
  return ptr;
}

static struct idiom1 *
idiom1_init(struct idiom1 *ptr)
{
  if (!ptr)
    ptr = idiom1_new();
  ptr->fill     = 0;
  if (!ptr->size)
    ptr->size   = BUFSIZ;
  if (!ptr->buffer)
    ptr->buffer = alloc(ptr->size);
  return ptr;
}

static void
idiom1_free(struct idiom1 *ptr)
{
  if (ptr->buffer)
    free(ptr->buffer);
  ptr->buffer   = 0;    /* paranoia     */
  if (ptr->allocated)
    {
      if (ptr->allocated==ptr)
        free(ptr->allocated);   /* GCC4.4.5-8: idiom1full.c:68: warning: attempt to free a non-heap object 'staticnull' */
      else
        (((void(*)(struct idiom1 *))ptr->allocated)(ptr));      /* call deallocator     */
    }
}

int
main(int argc, char **argv)
{
  static struct idiom1  staticnull;
  struct idiom1 *dynalloc, *staticptr;

  staticptr     = idiom1_init(&staticnull);
  dynalloc      = idiom1_init(NULL);

  /* do something with &staticnull or dynalloc  */

  idiom1_free(staticptr);
  idiom1_free(dynalloc);

  return 0;
}

pattern2last.c

This is the last (and recommended) variant. It uses a deallocator function which is correctly tracked by GCC4.4:

/* This Works is placed under the terms of the Copyright Less License,
 * see file COPYRIGHT.CLL.  USE AT OWN RISK, ABSOLUTELY NO WARRANTY.
 */

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

struct idiom1
  {
     struct idiom1      *next;
     void               (*dealloc)(struct idiom1 *);    /* changed to deallocator only  */
     int                fill, size;
     char               *buffer;        /* always allocated     */
  };

static void *
alloc(size_t len)
{
  void  *ptr;

  if ((ptr=malloc(len))==0)
    {
      perror("malloc");
      exit(1);
    }
  return ptr;
}

static void *
alloc0(size_t len)
{
  void  *ptr;

  ptr   = alloc(len);
  memset(ptr, 0, len);
  return ptr;
}

static void
idiom1_dealloc(struct idiom1 *ptr)
{
  free(ptr);    /* GCC 4.4.5-8: apparently it does grok that this function is never called on static pointers   */
}

static struct idiom1 *
idiom1_new(void)
{
  struct idiom1 *ptr;

  ptr   = alloc0(sizeof *ptr);
  ptr->dealloc  = idiom1_dealloc;
  return ptr;
}

static struct idiom1 *
idiom1_init(struct idiom1 *ptr)
{
  if (!ptr)
    ptr = idiom1_new();
  ptr->fill     = 0;
  if (!ptr->size)
    ptr->size   = BUFSIZ;
  if (!ptr->buffer)
    ptr->buffer = alloc(ptr->size);
  return ptr;
}

static void
idiom1_free(struct idiom1 *ptr)
{
  if (ptr->buffer)
    free(ptr->buffer);
  ptr->buffer   = 0;    /* paranoia     */
  if (ptr->dealloc)
    ptr->dealloc(ptr);  /* now using deallocator        */
}

int
main(int argc, char **argv)
{
  static struct idiom1  staticnull;
  struct idiom1 *dynalloc, *staticptr;

  staticptr     = idiom1_init(&staticnull);
  dynalloc      = idiom1_init(NULL);

  /* do something with &staticnull or dynalloc  */

  idiom1_free(staticptr);
  idiom1_free(dynalloc);

  return 0;
}

Erratas

  • In the comments the names are "idiom1.c" instead of "pattern1.c". I leave the edit to the reader.
-Tino, 2011-02-11