A bug in malloc() etc.

Discuss the development of software, tools, libraries and anything else that helps make ps2dev happen.

Moderators: cheriff, Herben

Post Reply
raipsu
Posts: 6
Joined: Mon Oct 25, 2004 7:03 am

A bug in malloc() etc.

Post 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? :)
mrbrown
Site Admin
Posts: 1537
Joined: Sat Jan 17, 2004 11:24 am

Post 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).
"He was warned..."
pixel
Posts: 791
Joined: Fri Jan 30, 2004 11:43 pm

Post 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...
pixel: A mischievous magical spirit associated with screen displays. The computer industry has frequently borrowed from mythology. Witness the sprites in computer graphics, the demons in artificial intelligence and the trolls in the marketing department.
raipsu
Posts: 6
Joined: Mon Oct 25, 2004 7:03 am

Post 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..
raipsu
Posts: 6
Joined: Mon Oct 25, 2004 7:03 am

Post 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;
pixel
Posts: 791
Joined: Fri Jan 30, 2004 11:43 pm

Post 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.
pixel: A mischievous magical spirit associated with screen displays. The computer industry has frequently borrowed from mythology. Witness the sprites in computer graphics, the demons in artificial intelligence and the trolls in the marketing department.
Post Reply