Re: Weird behavior of mallocr() function

[Jeff Johnston <jjohnstn at redhat dot com>]

On 03/01/10 05:52 AM, Denis Micolani wrote:
> Hi,
> I'm working on some embedded stuff, and for some reason I needed to
> get malloc() to work, newlib's release is the 1.18.0.
> I've wrote this simple function as stub (newlib is compiled to be reentrant):
>
> caddr_t _sbrk_r (struct _reent *r, int incr)
> {
>      static unsigned char* _heap_current_end =&__heap_start;
>
>      if (&__heap_start + incr>  &__heap_end )
>      {
>          errno = ENOMEM;
>          return (caddr_t) -1;
>      }
>      unsigned char* _old_heap_end = _heap_current_end;
>      _heap_current_end += incr;
>
>      return (caddr_t) _old_heap_end;
> }
>
> The constant addresses are taken from the linker script as above
> (these are correct and accessible):
>
> .heap           0x00200a9c      0x404 load address 0x000017b0
>                  0x00200aa0                . = ALIGN (0x20)
>   *fill*         0x00200a9c        0x4 00
>                  0x00200aa0                __heap_start = .
>                  0x00200ea0                . = (. + 0x400)
>   *fill*         0x00200aa0      0x400 00
>                  0x00200ea0                . = ALIGN (0x20)
>                  0x00200ea0                __heap_end = .
>
>
> Anyway malloc() will ALWAYS return a NULL ptr (no matter how much I'm
> asking for, could even be 1 byte). This is the debugging output (sorry
> about the volume of it):
>
> (gdb) continue
> Continuing.
>
> Breakpoint 1, _sbrk_r (r=0x20000c, incr=32) at syscalls.c:16
> 16              if (&__heap_start + incr>  &__heap_end )
> (gdb) step
> 21              unsigned char* _old_heap_end = _heap_current_end;
> (gdb) step
> 22              _heap_current_end += incr;
> (gdb) step
> 24              return (caddr_t) _old_heap_end;
> (gdb) step
> 25      }
> (gdb) step
> malloc_extend_top (reent_ptr=0x20000c, bytes=<value optimized out>)
>      at ../../../../../newlib-1.18.0/newlib/libc/stdlib/mallocr.c:2163
> 2163    ../../../../../newlib-1.18.0/newlib/libc/stdlib/mallocr.c: No such file
> or directory.
>          in ../../../../../newlib-1.18.0/newlib/libc/stdlib/mallocr.c
> (gdb) step
> 2160    in ../../../../../newlib-1.18.0/newlib/libc/stdlib/mallocr.c
> (gdb) step
> 2163    in ../../../../../newlib-1.18.0/newlib/libc/stdlib/mallocr.c
> (gdb) step
> 2146    in ../../../../../newlib-1.18.0/newlib/libc/stdlib/mallocr.c
> (gdb) step
> 2163    in ../../../../../newlib-1.18.0/newlib/libc/stdlib/mallocr.c
> (gdb) step
> 2167    in ../../../../../newlib-1.18.0/newlib/libc/stdlib/mallocr.c
> (gdb) step
> 2169    in ../../../../../newlib-1.18.0/newlib/libc/stdlib/mallocr.c
> (gdb) step
> 2167    in ../../../../../newlib-1.18.0/newlib/libc/stdlib/mallocr.c
> (gdb) step
> 2169    in ../../../../../newlib-1.18.0/newlib/libc/stdlib/mallocr.c
> (gdb) step
> 2178    in ../../../../../newlib-1.18.0/newlib/libc/stdlib/mallocr.c
> (gdb) step
> 2181    in ../../../../../newlib-1.18.0/newlib/libc/stdlib/mallocr.c
> (gdb) step
> 2179    in ../../../../../newlib-1.18.0/newlib/libc/stdlib/mallocr.c
> (gdb) step
> 2181    in ../../../../../newlib-1.18.0/newlib/libc/stdlib/mallocr.c
> (gdb) step
> 2179    in ../../../../../newlib-1.18.0/newlib/libc/stdlib/mallocr.c
> (gdb) step
> 2181    in ../../../../../newlib-1.18.0/newlib/libc/stdlib/mallocr.c
> (gdb) step
> 2185    in ../../../../../newlib-1.18.0/newlib/libc/stdlib/mallocr.c
> (gdb) step
> 2187    in ../../../../../newlib-1.18.0/newlib/libc/stdlib/mallocr.c
> (gdb) step
> 2188    in ../../../../../newlib-1.18.0/newlib/libc/stdlib/mallocr.c
> (gdb) step
> 2194    in ../../../../../newlib-1.18.0/newlib/libc/stdlib/mallocr.c
> (gdb) step
> 2185    in ../../../../../newlib-1.18.0/newlib/libc/stdlib/mallocr.c
> (gdb) step
> 2188    in ../../../../../newlib-1.18.0/newlib/libc/stdlib/mallocr.c
> (gdb) step
> 2194    in ../../../../../newlib-1.18.0/newlib/libc/stdlib/mallocr.c
> (gdb) step
> 2197    in ../../../../../newlib-1.18.0/newlib/libc/stdlib/mallocr.c
> (gdb) step
>
> Breakpoint 1, _sbrk_r (r=0x20000c, incr=1344) at syscalls.c:16
> 16              if (&__heap_start + incr>  &__heap_end )
> (gdb) step
> 18                      errno = ENOMEM;
>
>
> As you can see, there is a first request for memory, with incr =32,
> I've investigated and verified that my function (_sbrk_r()) returns
> 0x00200aa0 as address, that is the label __heap_start, and so I guess
> it's correct.
> Anyway mallocr() will make ANOTHER request to _sbrk_r, with incr=1344
> (those are bytes, what a huge number).
> I've checked the mallocr() source code and afaik this further call to
> the function _sbrk_r() has something to do with some disalignement,
> but I couldn't figure it out in its entirety.
>
> Could someone with some more experience help me about this?

For starters, your linker script is restricting the heap to be 0x400 
bytes (1024).  That is less that 1344 so your return code makes sense. 
Malloc creates bins to reuse for small allocations so if you are really 
this constrained for space, you won't be able to use the malloc logic in 
newlib and will have to override it.

As well, your current code isn't going to work.  You are adding incr to 
heap_start and comparing this to heap_end (i.e. neither changes).  I'm 
pretty sure you meant to use heap_current_end in your if statement.  As 
well, if you are going to have a reentrant version, you might as well 
set errno properly using the reentrant struct that is input.  See 
newlib/libc/include/sys/errno.h for the __errno_r macro.

Most sbrk implementations use the logic found in 
libgloss/libnosys/sbrk.c which has the heap and stack going in opposite 
directions and allocates a lump sum for the two in the linker script. 
This allows peaks for either the stack or heap as long as the total is 
within limits.  You might want to check out this design before 
implementing your own.

Regards,

-- Jeff J.