public inbox for newlib@sourceware.org
 help / color / mirror / Atom feed
* Malloc: unusable area at the end of the heap section
@ 2022-06-06 21:17 Jerome Leroux
  2022-06-07  6:17 ` Torbjorn SVENSSON
  0 siblings, 1 reply; 8+ messages in thread
From: Jerome Leroux @ 2022-06-06 21:17 UTC (permalink / raw)
  To: newlib

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=4191266#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/830a9b707caa5e343b6ffce7fcb2d3ca97e3259c/newlib/libc/stdlib/_mallocr.c#L198) in 
"malloc_extend_top" (probably here 
https://github.com/bminor/newlib/blob/830a9b707caa5e343b6ffce7fcb2d3ca97e3259c/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/4a3d0a5a5d829c05868a34658eb45731dbb5112b
- 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


^ permalink raw reply	[flat|nested] 8+ messages in thread

* RE: Malloc: unusable area at the end of the heap section
  2022-06-06 21:17 Malloc: unusable area at the end of the heap section Jerome Leroux
@ 2022-06-07  6:17 ` Torbjorn SVENSSON
  2022-06-07 16:03   ` Jeff Johnston
  0 siblings, 1 reply; 8+ messages in thread
From: Torbjorn SVENSSON @ 2022-06-07  6:17 UTC (permalink / raw)
  To: Jerome Leroux, newlib

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


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: Malloc: unusable area at the end of the heap section
  2022-06-07  6:17 ` Torbjorn SVENSSON
@ 2022-06-07 16:03   ` Jeff Johnston
  2022-06-07 16:38     ` Torbjorn SVENSSON
  2022-09-14 20:52     ` Jeff Johnston
  0 siblings, 2 replies; 8+ messages in thread
From: Jeff Johnston @ 2022-06-07 16:03 UTC (permalink / raw)
  To: Torbjorn SVENSSON; +Cc: Jerome Leroux, newlib

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
>
>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* RE: Malloc: unusable area at the end of the heap section
  2022-06-07 16:03   ` Jeff Johnston
@ 2022-06-07 16:38     ` Torbjorn SVENSSON
  2022-09-14 20:52     ` Jeff Johnston
  1 sibling, 0 replies; 8+ messages in thread
From: Torbjorn SVENSSON @ 2022-06-07 16:38 UTC (permalink / raw)
  To: Jeff Johnston; +Cc: Jerome Leroux, newlib

Hello Jeff,

Unfortunately, I’m not so familiar with autotools and I’m quite choked for the next few weeks.
If you have time, and knowledge of how to do your suggestion, feel free to improve the patch.

Do you know if the newlib/libc/unix directory is built for targets like bare-metal arm?

Kind regards,
Torbjörn

From: Jeff Johnston <jjohnstn@redhat.com>
Sent: den 7 juni 2022 18:04
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

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<mailto: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<mailto:st.com@sourceware.org>> On Behalf Of Jerome
> Leroux
> Sent: den 6 juni 2022 23:18
> To: newlib@sourceware.org<mailto: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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: Malloc: unusable area at the end of the heap section
  2022-06-07 16:03   ` Jeff Johnston
  2022-06-07 16:38     ` Torbjorn SVENSSON
@ 2022-09-14 20:52     ` Jeff Johnston
  2022-09-16 20:13       ` Jeff Johnston
  1 sibling, 1 reply; 8+ messages in thread
From: Jeff Johnston @ 2022-09-14 20:52 UTC (permalink / raw)
  To: Torbjorn SVENSSON; +Cc: Jerome Leroux, newlib

[-- Attachment #1: Type: text/plain, Size: 6233 bytes --]

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
>>
>>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: Malloc: unusable area at the end of the heap section
  2022-09-14 20:52     ` Jeff Johnston
@ 2022-09-16 20:13       ` Jeff Johnston
  2022-09-17  9:01         ` Torbjorn SVENSSON
  0 siblings, 1 reply; 8+ messages in thread
From: Jeff Johnston @ 2022-09-16 20:13 UTC (permalink / raw)
  To: Torbjorn SVENSSON; +Cc: Jerome Leroux, newlib


[-- Attachment #1.1: Type: text/plain, Size: 6703 bytes --]

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
>>>
>>>

[-- Attachment #2: 0001-Implement-sysconf-for-Arm.patch --]
[-- Type: text/x-patch, Size: 3643 bytes --]

From aae3831e6334eed571e8c2eb04295e53cf24f024 Mon Sep 17 00:00:00 2001
From: Jeff Johnston <jjohnstn@redhat.com>
Date: Fri, 16 Sep 2022 16:04:21 -0400
Subject: [PATCH] Implement sysconf for Arm

- add support for using sysconf to get page size in _mallocr.c via
  HAVE_SYSCONF_PAGESIZE flag set in configure.host
- set flag in configure.host for arm and add a default sysconf implementation
  in libc/sys/arm that returns the page size
- the default implementation can be overridden outside newlib to allow a
  different page size to improve malloc on devices with a small footprint
  without needing to rebuild newlib
- this patch is based on a contribution from Torbjorn Svensson and
  Niklas Dahlquist (https://ecos.sourceware.org/ml/newlib/current/017616.html)
---
 newlib/configure.host            |  2 ++
 newlib/libc/stdlib/_mallocr.c    |  2 ++
 newlib/libc/sys/arm/Makefile.inc |  2 +-
 newlib/libc/sys/arm/sysconf.c    | 30 ++++++++++++++++++++++++++++++
 4 files changed, 35 insertions(+), 1 deletion(-)
 create mode 100644 newlib/libc/sys/arm/sysconf.c

diff --git a/newlib/configure.host b/newlib/configure.host
index 98ce07d..32d1436 100644
--- a/newlib/configure.host
+++ b/newlib/configure.host
@@ -628,6 +628,7 @@ newlib_cflags="${newlib_cflags} -DCLOCK_PROVIDED -DMALLOC_PROVIDED -DEXIT_PROVID
 	;;
   arm*-*-pe)
 	syscall_dir=syscalls
+	newlib_cflags="${newlib_cflags} -DHAVE_SYSCONF_PAGESIZE"
 	;;
   arm*-*-*)
 	syscall_dir=syscalls
@@ -642,6 +643,7 @@ newlib_cflags="${newlib_cflags} -DCLOCK_PROVIDED -DMALLOC_PROVIDED -DEXIT_PROVID
 #         newlib_cflags="${newlib_cflags} -DARM_RDP_MONITOR"
 	  newlib_cflags="${newlib_cflags} -DARM_RDI_MONITOR"
 	fi
+	newlib_cflags="${newlib_cflags} -DHAVE_SYSCONF_PAGESIZE"
 	;;
   avr*)
 	newlib_cflags="${newlib_cflags} -DNO_EXEC -DSMALL_MEMORY -DMISSING_SYSCALL_NAMES"
diff --git a/newlib/libc/stdlib/_mallocr.c b/newlib/libc/stdlib/_mallocr.c
index 4b53997..1997b6d 100644
--- a/newlib/libc/stdlib/_mallocr.c
+++ b/newlib/libc/stdlib/_mallocr.c
@@ -320,12 +320,14 @@ extern "C" {
 #endif
 
 #ifndef _WIN32
+#ifndef HAVE_SYSCONF_PAGESIZE
 #ifdef SMALL_MEMORY
 #define malloc_getpagesize (128)
 #else
 #define malloc_getpagesize (4096)
 #endif
 #endif
+#endif
 
 #if __STD_C
 extern void __malloc_lock(struct _reent *);
diff --git a/newlib/libc/sys/arm/Makefile.inc b/newlib/libc/sys/arm/Makefile.inc
index 490a963..0122956 100644
--- a/newlib/libc/sys/arm/Makefile.inc
+++ b/newlib/libc/sys/arm/Makefile.inc
@@ -1,6 +1,6 @@
 AM_CPPFLAGS_%C% = -I$(srcdir)/libc/machine/arm
 
-libc_a_SOURCES += %D%/access.c %D%/aeabi_atexit.c
+libc_a_SOURCES += %D%/access.c %D%/aeabi_atexit.c %D%/sysconf.c
 if MAY_SUPPLY_SYSCALLS
 libc_a_SOURCES += %D%/libcfunc.c %D%/trap.S %D%/syscalls.c
 endif
diff --git a/newlib/libc/sys/arm/sysconf.c b/newlib/libc/sys/arm/sysconf.c
new file mode 100644
index 0000000..dbed7d7
--- /dev/null
+++ b/newlib/libc/sys/arm/sysconf.c
@@ -0,0 +1,30 @@
+/* libc/sys/arm/sysconf.c - The sysconf function */
+
+/* Copyright 2020, STMicroelectronics
+ *
+ * All rights reserved.
+ *
+ * Redistribution, modification, and use in source and binary forms is permitted
+ * provided that the above copyright notice and following paragraph are
+ * duplicated in all such forms.
+ *
+ * This file is distributed WITHOUT ANY WARRANTY; without even the implied
+ * warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
+ */
+
+#include <unistd.h>
+#include <errno.h>
+
+long sysconf(int name)
+{
+  switch (name)
+  {
+  case _SC_PAGESIZE:
+    return 4096;
+
+  default:
+    errno = EINVAL;
+    return -1;
+  }
+  return -1; /* Can't get here */
+}
-- 
1.8.3.1


^ permalink raw reply	[flat|nested] 8+ messages in thread

* RE: Malloc: unusable area at the end of the heap section
  2022-09-16 20:13       ` Jeff Johnston
@ 2022-09-17  9:01         ` Torbjorn SVENSSON
  2022-09-19 14:48           ` Jeff Johnston
  0 siblings, 1 reply; 8+ messages in thread
From: Torbjorn SVENSSON @ 2022-09-17  9:01 UTC (permalink / raw)
  To: Jeff Johnston; +Cc: Jerome Leroux, newlib, Niklas DAHLQUIST

[-- Attachment #1: Type: text/plain, Size: 7029 bytes --]

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<mailto: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<mailto: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<mailto: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<mailto:st.com@sourceware.org>> On Behalf Of Jerome
> Leroux
> Sent: den 6 juni 2022 23:18
> To: newlib@sourceware.org<mailto: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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: Malloc: unusable area at the end of the heap section
  2022-09-17  9:01         ` Torbjorn SVENSSON
@ 2022-09-19 14:48           ` Jeff Johnston
  0 siblings, 0 replies; 8+ messages in thread
From: Jeff Johnston @ 2022-09-19 14:48 UTC (permalink / raw)
  To: Torbjorn SVENSSON; +Cc: Jerome Leroux, newlib, Niklas DAHLQUIST

[-- Attachment #1: Type: text/plain, Size: 7638 bytes --]

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
>
>

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2022-09-19 14:49 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-06-06 21:17 Malloc: unusable area at the end of the heap section Jerome Leroux
2022-06-07  6:17 ` Torbjorn SVENSSON
2022-06-07 16:03   ` Jeff Johnston
2022-06-07 16:38     ` Torbjorn SVENSSON
2022-09-14 20:52     ` Jeff Johnston
2022-09-16 20:13       ` Jeff Johnston
2022-09-17  9:01         ` Torbjorn SVENSSON
2022-09-19 14:48           ` Jeff Johnston

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).