[PATCH] Reclaim _REENT_MP_P5S in _reclaim_reent

[PATCH] Reclaim _REENT_MP_P5S in _reclaim_reent

[chrisj]

From: Chris Johns <chrisj@rtems.org>

The change fixes a memory leak in threads that clean up using
_reclaim_reent.

RTEMS: Closes #4967
---
 newlib/libc/reent/reent.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/newlib/libc/reent/reent.c b/newlib/libc/reent/reent.c
index db80ca06e..6bb271b9a 100644
--- a/newlib/libc/reent/reent.c
+++ b/newlib/libc/reent/reent.c
@@ -59,6 +59,17 @@ _reclaim_reent (struct _reent *ptr)
 	}
       if (_REENT_MP_RESULT(ptr))
 	_free_r (ptr, _REENT_MP_RESULT(ptr));
+      if (_REENT_MP_P5S(ptr))
+        {
+         struct _Bigint *thisone, *nextone;
+         nextone = _REENT_MP_P5S(ptr);
+         while (nextone)
+           {
+             thisone = nextone;
+             nextone = nextone->_next;
+             _free_r (ptr, thisone);
+           }
+       }
 #ifdef _REENT_SMALL
       }
 #endif
-- 
2.37.1

Re: [PATCH] Reclaim _REENT_MP_P5S in _reclaim_reent

[Corinna Vinschen]

On Nov 16 12:12, chrisj@rtems.org wrote:
> From: Chris Johns <chrisj@rtems.org>
> 
> The change fixes a memory leak in threads that clean up using
> _reclaim_reent.
> 
> RTEMS: Closes #4967
> ---
>  newlib/libc/reent/reent.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/newlib/libc/reent/reent.c b/newlib/libc/reent/reent.c
> index db80ca06e..6bb271b9a 100644
> --- a/newlib/libc/reent/reent.c
> +++ b/newlib/libc/reent/reent.c
> @@ -59,6 +59,17 @@ _reclaim_reent (struct _reent *ptr)
>  	}
>        if (_REENT_MP_RESULT(ptr))
>  	_free_r (ptr, _REENT_MP_RESULT(ptr));
> +      if (_REENT_MP_P5S(ptr))
> +        {
> +         struct _Bigint *thisone, *nextone;
> +         nextone = _REENT_MP_P5S(ptr);
> +         while (nextone)
> +           {
> +             thisone = nextone;
> +             nextone = nextone->_next;
> +             _free_r (ptr, thisone);
> +           }
> +       }

The p5s data is allocated using Balloc, so the pointers are in freelist
and should already be free'd at this point, starting at line 42.

Please explain what happens and why free'ing the freelist is not
sufficient in this case, preferredly as part of the commit message.


Thanks,
Corinna

Re: [PATCH] Reclaim _REENT_MP_P5S in _reclaim_reent

[Chris Johns]

On 16/11/2023 8:15 pm, Corinna Vinschen wrote:
> On Nov 16 12:12, chrisj@rtems.org wrote:
>> From: Chris Johns <chrisj@rtems.org>
>>
>> The change fixes a memory leak in threads that clean up using
>> _reclaim_reent.
>>
>> RTEMS: Closes #4967
>> ---
>>  newlib/libc/reent/reent.c | 11 +++++++++++
>>  1 file changed, 11 insertions(+)
>>
>> diff --git a/newlib/libc/reent/reent.c b/newlib/libc/reent/reent.c
>> index db80ca06e..6bb271b9a 100644
>> --- a/newlib/libc/reent/reent.c
>> +++ b/newlib/libc/reent/reent.c
>> @@ -59,6 +59,17 @@ _reclaim_reent (struct _reent *ptr)
>>  	}
>>        if (_REENT_MP_RESULT(ptr))
>>  	_free_r (ptr, _REENT_MP_RESULT(ptr));
>> +      if (_REENT_MP_P5S(ptr))
>> +        {
>> +         struct _Bigint *thisone, *nextone;
>> +         nextone = _REENT_MP_P5S(ptr);
>> +         while (nextone)
>> +           {
>> +             thisone = nextone;
>> +             nextone = nextone->_next;
>> +             _free_r (ptr, thisone);
>> +           }
>> +       }
> 
> The p5s data is allocated using Balloc, so the pointers are in freelist
> and should already be free'd at this point, starting at line 42.

Yes however P5S blocks allocated by Balloc via i2b are not passed to Bfree.

> Please explain what happens and why free'ing the freelist is not
> sufficient in this case, preferredly as part of the commit message.

Yes good idea. How about:

The _REENT_MP_P5S blocks are allocated using Balloc via i2b and linked in the
pow5mult call. As a result these blocks are not on the freelist managed by the
Bfree call. This change fixes a memory leak in threads that clean up using
_reclaim_reent.

Chris

Re: [PATCH] Reclaim _REENT_MP_P5S in _reclaim_reent

[Corinna Vinschen]

On Nov 17 08:17, Chris Johns wrote:
> On 16/11/2023 8:15 pm, Corinna Vinschen wrote:
> > On Nov 16 12:12, chrisj@rtems.org wrote:
> >> From: Chris Johns <chrisj@rtems.org>
> >>
> >> The change fixes a memory leak in threads that clean up using
> >> _reclaim_reent.
> >>
> >> RTEMS: Closes #4967
> >> ---
> >>  newlib/libc/reent/reent.c | 11 +++++++++++
> >>  1 file changed, 11 insertions(+)
> >>
> >> diff --git a/newlib/libc/reent/reent.c b/newlib/libc/reent/reent.c
> >> index db80ca06e..6bb271b9a 100644
> >> --- a/newlib/libc/reent/reent.c
> >> +++ b/newlib/libc/reent/reent.c
> >> @@ -59,6 +59,17 @@ _reclaim_reent (struct _reent *ptr)
> >>  	}
> >>        if (_REENT_MP_RESULT(ptr))
> >>  	_free_r (ptr, _REENT_MP_RESULT(ptr));
> >> +      if (_REENT_MP_P5S(ptr))
> >> +        {
> >> +         struct _Bigint *thisone, *nextone;
> >> +         nextone = _REENT_MP_P5S(ptr);
> >> +         while (nextone)
> >> +           {
> >> +             thisone = nextone;
> >> +             nextone = nextone->_next;
> >> +             _free_r (ptr, thisone);
> >> +           }
> >> +       }
> > 
> > The p5s data is allocated using Balloc, so the pointers are in freelist
> > and should already be free'd at this point, starting at line 42.
> 
> Yes however P5S blocks allocated by Balloc via i2b are not passed to Bfree.

Aaah, right, it really never does.

> > Please explain what happens and why free'ing the freelist is not
> > sufficient in this case, preferredly as part of the commit message.
> 
> Yes good idea. How about:
> 
> The _REENT_MP_P5S blocks are allocated using Balloc via i2b and linked in the
> pow5mult call. As a result these blocks are not on the freelist managed by the
> Bfree call. This change fixes a memory leak in threads that clean up using
> _reclaim_reent.

Yeah, sounds good.  Please resend with this commit message.


Thanks,
Corinna