This patch adds further comments to nano-mallocr.c, to more comprehensively
explain how padding works in the malloc_chunk structure.
It was originally discussed in the following thread:
https://sourceware.org/ml/newlib/2017/msg00031.html
2017-01-13 Joe Seymour <joe.s@somniumtech.com>
newlib/
* libc/stdlib/nano-mallocr.c (malloc_chunk, get_chunk_from_ptr)
(nano_malloc): Add comments.
As described in nano-mallocr.c, chunks of heap are represented in memory
as a size (of type long), followed by some optional padding containing a
negative offset to size, followed by the data area.
get_chunk_from_ptr is responsible for taking a pointer to the data area
(as returned by malloc) and finding the start of the chunk. It does this
by assuming there is no padding and trying to read the size, if the size
is negative then it uses that as an offset to find the true size.
Crucially, it reads the padding area as a long.
nano_malloc is responsible for populating the optional padding area. It
does so by casting a pointer to an (int *) and writing the negative
offset into it.
This means that padding is being written as an int but read as a long.
On msp430 an int is 2 bytes, while a long is 4 bytes. This means that 2
bytes are written to the padding, but 4 bytes are read from it: it has
only been partially initialised.
nano_malloc is the default malloc implementation for msp430.
This patch changes the cast from (int *) to (long *). The change to
nano_malloc has has been observed to fix a TI Energia project that
had been malfunctioning because malloc was returning invalid addresses.
The change to nano_memalign is based entirely on code inspection.
I've built and tested as follows:
Configured (gcc+newlib) with: --target=msp430-elf --enable-languages=c
gcc testsuite variations:
msp430-sim/-mcpu=msp430
msp430-sim/-mcpu=msp430x
msp430-sim/-mcpu=msp430x/-mlarge/-mdata-region=either/-mcode-region=either
msp430-sim/-mhwmult=none
msp430-sim/-mhwmult=f5series
My testing has shown no regressions, however I don't know if the gcc
testsuite provides sufficient coverage for this patch?
I don't have write access, so if this patch is acceptable after review,
I would appreciate it if someone would commit it for me.
Thanks,
2017-01-XX Joe Seymour <joe.s@somniumtech.com>
newlib/
* libc/stdlib/nano-mallocr.c (nano_malloc): Fix incorrect cast.
(nano_memalign): Likewise.
Non-reentrant system calls version implies both MISSING_SYSCALL_NAMES
and REENTRANT_SYSCALL_PROVIDED macros to be defined.
Being coupled with --enable-newlib-nano-malloc knob it breaks the build:
bash-4.3$ ../newlib-2.3.0.20160104/configure CC_FOR_TARGET=gcc
AR_FOR_TARGET=ar RANLIB_FOR_TARGET=ranlib CFLAGS_FOR_TARGET="-m32
-DMISSING_SYSCALL_NAMES -DREENTRANT_SYSCALLS_PROVIDED"
--target=i386-elf --enable-newlib-nano-malloc && make
<...omitted output...>
../../../../../../newlib-2.3.0.20160104/newlib/libc/stdlib/nano-mallocr.c:
In function ‘_mallinfo_r’:
../../../../../../newlib-2.3.0.20160104/newlib/libc/stdlib/nano-mallocr.c:489:35:
error: macro "_sbrk_r" requires 2 arguments, but only 1 given
sbrk_now = _sbrk_r(RCALL 0);
^
../../../../../../newlib-2.3.0.20160104/newlib/libc/stdlib/nano-mallocr.c:489:20:
error: ‘_sbrk_r’ undeclared (first use in this function)
sbrk_now = _sbrk_r(RCALL 0);
^
../../../../../../newlib-2.3.0.20160104/newlib/libc/stdlib/nano-mallocr.c:489:20:
note: each undeclared identifier is reported only once for each
function it appears in
Makefile:1512: recipe for target 'lib_a-nano-mallinfor.o' failed
make[8]: *** [lib_a-nano-mallinfor.o] Error 1
In case of non-reentrant system calls _sbrk_r became a macro with TWO
args (defined in reent.h):
#define _sbrk_r(__reent, __incr) sbrk(__incr)
But in our case only one argument is present. (RCALL 0) is considered
as a single argument despite RCALL itself is a macro:)
So intermediate one-arg macro will be enough to expand args before
final _sbrk_r expansion:
#define _SBRK_R(X) _sbrk_r(X)
Here is a patch:
* libc/stdlib/nano-mallocr.c (RONECALL): New parameter macro.
(sbrk_start): New variable.
(current_mallinfo, free_list): Forward declaration of data.
(chunk): Define size as long type.
(mallinfo): Synchronize with latest malloc.h.
(nano_*): Forward declaration of functions.
(get_chunk_from_ptr): Defined to static inline.
(nano_mallinfo): Implement this function.
(nano_malloc_stats): Implement this function.
Jaap de Wolff