Malloc: unusable area at the end of the heap section

Mon Sep 19 14:48:46 GMT 2022

Hi Torbjorn,

Adding the small memory logic is trivial so I'll add it and then push the
change.

Thanks,

-- Jeff J.

On Sat, Sep 17, 2022 at 5:01 AM Torbjorn SVENSSON <torbjorn.svensson@st.com>
wrote:

> Hello,
>
>
>
> The patch looks good to me.
>
> Thank you Jeff for adding the HAVE_SYSCONF_PAGESIZE logic!
>
>
>
>
>
> One minor question.
>
> When HAVE_SYSCONF_PAGESIZE is not set, there would be 2 possible values
> (128 or 4096). Would it make sense to also have that ifdef part in the
> sysconf function? I know that my original patch did not include this, but
> maybe it would be beneficial to have sysconf(_SC_PAGESIZE) return 128 when
> SMALL_MEMORY is defined and sysconf is not overridden by the application?
>
>
>
> Kind regards,
>
> Torbjörn
>
>
>
>
>
> ST Restricted
>
> *From:* Jeff Johnston <jjohnstn@redhat.com>
> *Sent:* den 16 september 2022 22:14
> *To:* Torbjorn SVENSSON <torbjorn.svensson@st.com>
> *Cc:* Jerome Leroux <jerome.leroux@microej.com>; newlib@sourceware.org
> *Subject:* Re: Malloc: unusable area at the end of the heap section
>
>
>
> Ok,
>
>
>
> I am attaching the modified patch after discussing with Torbjorn about the
> licensing.  Torbjorn, please
>
> review and let me know if there are changes you would like, otherwise, I
> will push.
>
>
>
> -- Jeff J.
>
>
>
>
>
> On Wed, Sep 14, 2022 at 4:52 PM Jeff Johnston <jjohnstn@redhat.com> wrote:
>
> Hi Torbjorn,
>
>
>
> I took a look at what would be needed to make this more generic.  I have a
> patch almost ready, but
>
> the one issue is that your libc/sys/arm/sysconf.c file does not have a
> license.  Could you please append a
>
> newer version of the file with a license and I will complete the patch for
> you to review?
>
>
>
> I decided to go with a simple HAVE_xxxx macro rather than a configure
> option so any platform can
>
> simply set the flag in configure.host.
>
>
>
> -- Jeff J.
>
>
>
> On Tue, Jun 7, 2022 at 12:03 PM Jeff Johnston <jjohnstn@redhat.com> wrote:
>
> Hi Torbjorn,
>
>
>
> I think it would be useful.  Do you want to modify the patch to be more
> generic?  I am thinking of a var set in configure.host that sets a compile
> flag at the end such as _USE_SYSCONF_FOR_PAGESIZE.  Then,
>
> the default sysconf.c can be put in the newlib/libc/unix directory.  It is
> then a straight-forward exercise to add this as an enablement configuration
> option.  What do you think?
>
>
>
> -- Jeff J.
>
>
>
> On Tue, Jun 7, 2022 at 2:18 AM Torbjorn SVENSSON via Newlib <
> newlib@sourceware.org> wrote:
>
> Hello,
>
> A while back, I provided a patch[1] to newlib that would allow the
> application to override the pagesize for malloc, but the patch got stalled.
> Maybe this would be a good time to take another look at the patch and see
> if it would actually fix the generic newlib usage or if it's still
> something that is only applicable for small embedded targets.
>
> [1] https://ecos.sourceware.org/ml/newlib/current/017616.html
>
> Kind regards,
> Torbjörn
>
> > -----Original Message-----
> > From: Newlib <newlib-
> > bounces+torbjorn.svensson=st.com@sourceware.org> On Behalf Of Jerome
> > Leroux
> > Sent: den 6 juni 2022 23:18
> > To: newlib@sourceware.org
> > Subject: Malloc: unusable area at the end of the heap section
> >
> > Hello Newlib developers,
> >
> > I am a user of Newlib in a project that runs on an NXP MCU.
> > I am using MCUXpressoIDE_11.3.0_5180_prc3, which comes with GCC “arm-
> > none-eabi-gcc.exe (GNU Arm Embedded Toolchain
> > 9-2020-q2-update) 9.3.1 20200408 (release)” and Newlib 3.3.0.
> >
> > I have identified an issue in malloc, and I think the problem is still
> present in
> > the latest version of Newlib. I could
> > not see any changes in the incriminated code since Newlib 3.3.0.
> >
> > I noticed this issue only in the standard malloc implementation and not
> in the
> > nano-malloc version.
> >
> > Here is a description of the problem:
> > The allocator splits the heap into pages. When a page is full, it
> increases the
> > heap size by reserving a new page in the
> > heap section. When reserving a new page, the allocator keeps the page end
> > address aligned with malloc_getpagesize, which
> > is set to 4096 by default. If there is not enough space to reserve the
> full page,
> > the allocation fails even if there is
> > enough space in the heap to allocate the chunk of memory.
> > Because the issue is related to the heap end address and how the linker
> > positions the heap, the same sequence of
> > allocations may lead to different results (failure or success) depending
> on the
> > location of the heap, even if the heap
> > size is constant. Typically, adding a new C global variable can shift
> the start
> > address of the heap section and cause a
> > malloc error.
> >
> > For example, with a heap section of 4096 bytes (0x1000 bytes):
> > If the heap section address is 0x20100-0x21100, during the
> initialization, the
> > page end address is set to 0x21000
> > (aligned on 4096). We will be able to allocate until the address
> 0x21000. After
> > that, the allocator will try to reserve
> > a new page, but it will fail because it won’t be able to reserve a 4096
> bytes
> > page from 0x21000 to 0x22000. The
> > following allocations will fail. The usable heap size is 3840 bytes
> (0x21000 -
> > 0x20100) instead of 4096.
> > If the heap section address is 0x20F00-0x21F00 (same size), with the same
> > scenario, the usable heap size is 256 bytes
> > (0x21000 - 0x20F00).
> > Here are two examples of heap configurations:
> > https://gist.github.com/jerome-
> > leroux/759159fbd3e7bb5e189dbceb04636914?permalink_comment_id=4191
> > 266#gistcomment-4191266
> >
> > I did not dig into the implementation so much. From my understanding, the
> > problem comes from the usage of
> > "malloc_getpagesize" (see
> > https://github.com/bminor/newlib/blob/830a9b707caa5e343b6ffce7fcb2d3c
> > a97e3259c/newlib/libc/stdlib/_mallocr.c#L198) in
> > "malloc_extend_top" (probably here
> > https://github.com/bminor/newlib/blob/830a9b707caa5e343b6ffce7fcb2d3c
> > a97e3259c/newlib/libc/stdlib/_mallocr.c#L2166).
> > I can understand it makes sense to keep the pages aligned when running
> in a
> > system that implements virtual memory.
> > Still, on an MCU, the heap is just a contiguous chunk of memory
> allocated at
> > link time. Furthermore, the heap size is
> > usually pretty small (a few kilobytes), so potentially wasting 4 KB of
> memory
> > is unacceptable. Using the default
> > implementation of "sbrk" documented at
> > https://sourceware.org/newlib/libc.html#index-sbrk will lead to the
> > problem.
> >
> > I have written a simple example that demonstrates the issue (see
> > https://gist.github.com/jerome-
> > leroux/759159fbd3e7bb5e189dbceb04636914 ). To reproduce the problem,
> > define the macros
> > HEAP_SECTION_START_SYMBOL and HEAP_SECTION_END_SYMBOL, which
> > are specific to your environment. Then call the function
> > "test_malloc()".
> >
> > I tried to find someone with the same issue, but I couldn’t. The related
> > commits/discussions I found are:
> > -
> > https://github.com/bminor/newlib/commit/4a3d0a5a5d829c05868a34658eb
> > 45731dbb5112b
> > - https://stackoverflow.com/questions/39088598/malloc-in-newlib-does-it-
> > waste-memory-after-one-big-failure-allocation
> >
> > Can anyone confirm what I have noticed?
> >
> > Thanks.
> >
> > --
> > Jerome Leroux
>
>