Re: [Qemu-devel] [PATCH] [RFC] qemu_malloc dynamic checking

[Blue Swirl]

On 4/15/09, Kevin Wolf <address@hidden> wrote:
> Am Mittwoch, 15. April 2009 20:23 schrieb Blue Swirl:
>
> > This patch adds simple checks for qemu_malloc/free/realloc. With the
>  > check enabled, i386-softmmu crashes. Sparc32, Sparc64, PPC32 and PPC64
>  > system emulators are fine.
>
>
> Generally I like the idea of running this kind of tests occasionally. I'm
>  wondering though if we really need additional code for it, or if using
>  external tools like valgrind could do the same without additional code and
>  with the extra bonus of actually telling you who the culprit was.

The check examines whether the memory allocated with qemu_malloc and
friends is freed with plain free (or malloc'ed memory freed with
qemu_free). We are not checking if the memory is corrupted that
Valgrind checks for. Maybe semantic patch stuff might be able to catch
this type of errors.

>  > Index: qemu/qemu-malloc.c
>  > ===================================================================
>  > --- qemu.orig/qemu-malloc.c     2009-04-15 17:16:44.000000000 +0000
>  > +++ qemu/qemu-malloc.c  2009-04-15 18:09:49.000000000 +0000
>  > @@ -24,6 +24,11 @@
>  >  #include "qemu-common.h"
>  >  #include <stdlib.h>
>  >
>  > +#define DEBUG_MALLOC
>  > +
>  > +/* Canary must not break malloc alignment */
>  > +static const char canary[] = "QEMU_MALLOC1234";
>  > +
>  >  static void *oom_check(void *ptr)
>  >  {
>  >      if (ptr == NULL)
>  > @@ -38,20 +43,58 @@
>  >
>  >  void qemu_free(void *ptr)
>  >  {
>  > +#ifdef DEBUG_MALLOC
>  > +    fprintf(stderr, "qemu_free: 0x%p\n", ptr);
>  > +    if (ptr) {
>  > +        ptr = (void *)((unsigned long)ptr - sizeof(canary));
>  > +        if (memcmp(ptr, canary, sizeof(canary)) != 0) {
>  > +            exit(0);
>
>  I'm used to the convention that an exit code of 0 means success. abort() 
> might
>  be the right thing here.

Right.