Page 1 of 1

A bug in malloc() etc.

Posted: Mon Oct 25, 2004 7:15 am
by raipsu
Hi!

I noticed that there are something wrong with malloc() etc. functions. Seems that sometimes free() does not free the memory.

I wrote this little program to demonstrate it:

Code: Select all

#include <stdio.h>
#include <malloc.h>

int main &#40;&#41; &#123;
  int iter = 0;

  realloc&#40;malloc&#40;10&#41;, 8&#41;; /* 1 */
  realloc&#40;malloc&#40;10&#41;, 8&#41;; /* 2 */
  realloc&#40;realloc&#40;malloc&#40;10&#41;, 20&#41;, 10&#41;;

  for &#40;;;&#41; &#123;
    char *ptr;
    ptr = malloc&#40;2048*1024&#41;;  /* This can be smaller also */
    printf &#40;"%d&#58; %p\n", iter++, ptr&#41;;
    if &#40;ptr == NULL&#41; &#123;
      printf &#40;"malloc&#40;&#41; failed!\n"&#41;;
      break;
    &#125;
    free&#40;ptr&#41;;
  &#125;

  return 0;
&#125;
I'm using a toolchain built a few days ago with oopo's toolchain.sh script.

I'll compile it like this:

ee-gcc -DPS2_EE -O3 -G0 -Wall -I$PS2SDK/common/include -I$PS2SDK/ee/include -c main.c
ee-gcc -nostartfiles -L$PS2SDK/ee/lib -T$PS2SDK/ee/startup/linkfile -o test.elf $PS2SDK/ee/startup/crt0.o main.o -lc -lkernel -lsyscall -lc

And then run it:

# ps2client execee host:test.elf
loadelf: fname host:test.elf secname all
Input ELF format filename = host:test.elf
0 00100000 000040b8 .
Loaded, host:test.elf
start address 0x100008
gp address 00000000
0: 1079296
1: 3176464
2: 5273632
3: 7370800
4: 9467968
5: 11565136
6: 13662304
7: 15759472
8: 17856640
9: 19953808
10: 22050976
11: 24148144
12: 26245312
13: 28342480
14: 30439648
15: 0
malloc() failed!

After 15 malloc() and free()'s it has filled up the whole memory and malloc() returns NULL. According my testings, realloc() is needed to make free() behave like this. If you'll remove the lines commented with 1 and 2, malloc() and free() works normally and the loop does never end. Any ideas how to fix it? :)

Posted: Mon Oct 25, 2004 9:08 am
by mrbrown
I thought %p was supposed to print a pointer, which is represented as a hex number. Why does it print out a decimal number?

(Sorry not to answer the original question).

Posted: Mon Oct 25, 2004 10:09 am
by pixel
Fixed. Was a stupid realloc() bug: http://cvs.ps2dev.org/ps2sdk/ee/libc/sr ... 1.3&r2=1.4

and since your realloc was shrinking the memory, the realloc function was munching its pointers located after the malloc()ed zone.


I also (uglily) fixed the %p part of xprintf.c. Still doesn't behave as it should though, since it should be translated into "0x%08X" and not "%X". Ho well...

Posted: Tue Nov 02, 2004 12:15 am
by raipsu
Thanks, the realloc() is working fine now.

Now memalign() is broken. :( Replace the malloc() and realloc() lines in start of previous test program with malloc(2); memalign(64, 10); and free() stops working again..

Posted: Tue Nov 02, 2004 1:30 am
by raipsu
memalign() was broken because it did not update __alloc_heap_head and __alloc_heap_tail. This should fix it:

Code: Select all

*** ../d/ps2sdk/ee/libc/src/alloc.c     Mon Oct 11 03&#58;44&#58;59 2004
--- ee/libc/src/alloc.c Mon Nov  1 17&#58;22&#58;39 2004
***************
*** 206,211 ****
--- 206,212 ----
  &#123;
        heap_mem_header_t new_mem;
        heap_mem_header_t *cur_mem;
+       heap_mem_header_t *old_mem;
        void *ptr = NULL;

        if &#40;align <= DEFAULT_ALIGNMENT&#41;
        void *ptr = NULL;

        if &#40;align <= DEFAULT_ALIGNMENT&#41;
***************
*** 226,231 ****
--- 227,234 ----
        /* Otherwise, align the pointer and fixup our hearder accordingly.  */
        ptr = &#40;void *&#41;ALIGN&#40;&#40;u32&#41;ptr, align&#41;;

+       old_mem = cur_mem;
+
        /* Copy the heap_mem_header_t locally, before repositioning &#40;to make
           sure we don't overwrite ourselves.  */
        memcpy&#40;&new_mem, cur_mem, sizeof&#40;heap_mem_header_t&#41;&#41;;
***************
*** 237,242 ****
--- 240,251 ----
        if &#40;cur_mem->next&#41;
                cur_mem->next->prev = cur_mem;

+       if &#40;__alloc_heap_head == old_mem&#41;
+               __alloc_heap_head = cur_mem;
+
+       if &#40;__alloc_heap_tail == old_mem&#41;
+               __alloc_heap_tail = cur_mem;
+
        cur_mem->ptr = ptr;
        return ptr;
  &#125;

Posted: Tue Nov 02, 2004 3:33 am
by pixel
Thanks -- commited.


Also, did you notice I changed realloc() code so it is a bit smarter ? (that is, won't re-malloc() if there is room in memory whatsoever).

I didn't do any extensive check however, so, could you be kind enough to run some of your previous code and check if things go right ? Thanks.