Some questions on reentrancy, __DYNAMIC_REENT__ and _impure_ptr

Some questions on reentrancy, __DYNAMIC_REENT__ and _impure_ptr

[Nick]

Hi,

I've been trying to enable reentrancy of newlib on a home brew kernel 
for the x86 platform and have some questions on how various pieces all 
fits together.

Implemented __getreent () to return a private copy of struct reent, and 
also hard coded __DYNAMIC_REENT__ and GETREENT_PROVIDED in sys/config.h 
to rule out any issue of passing in via build CFLAGS or the CFLAGS in 
configure.host. Things including errno seem to work but not totally 
making sense.

As many library functions are still accessing the reent structure using 
_impure_ptr instead of calling my __getreent () function, for example, 
the CHECK_INIT (_REENT, fp) at the beginning of __swsetup_r (struct 
_reent *ptr, register FILE * fp).

Questions:

1. Are the library functions expected to still use _impure_ptr instead 
of calling __getreent () when both __DYNAMIC_REENT__ and 
GETREENT_PROVIDED are hard coded in sys/config.h?

If so, how do they provide reentrancy? Since _impure_ptr is a global 
pointer visible to all threads and threads can easily step on each 
other's toes trying to change fields in the reent structure pointed to 
by _impure_ptr concurrently.

If not, what other MACROs or changes should I make so that all the 
library functions all use __getreent () instead of _impure_ptr? Is it 
okay to set _impure_ptr to a bad value such as NULL in this case, in 
order to catch any unintended access?

2. in the documentation on https://sourceware.org/newlib/, the following 
is mentioned as needed for syscalls stubs to return errno:

#include <errno.h>
#undef errno
extern int errno;

If I do include this part, all the syscalls stubs seem to do when they 
assign values to errno is setting the global int errno; inside reent.c. 
As user code built against the library don’t read out that integer but 
instead calls __(), errno set by syscall stubs can't be read out by user 
code.

If on the other hand I don’t include this part before my syscall stubs, 
the errno set by them do seem to work as they also set the copy in reent 
structures. What might I have missed here?

3. There were some old discussions about manually changing _impure_ptr 
at each context switch. But I’m wondering about the validity of such a 
method since it seems like a really clumsy maneuver for kernel code at 
CPL0 to reach into user space belonging to different binaries to change 
a global pointer. What's more, if manually changing _impure_ptr at each 
context switch is needed, then what would be the purpose of 
__DYNAMIC_REENT__, GETREENT_PROVIDED and implementing a __getreent () to 
get a thread local version?

4. Is _global_impure_ptr thread safe? It is a bit concerning as it seems 
to be pointing to the same copy of impure_data that some libraries calls 
would access, and even if I try to change _impure_ptr at each context 
switch, some threads might still be accessing _global_impure_ptr 
concurrently?

5. There were also old discussions about having to provide mutex for 
malloc, is this still the case for newer versions of newlib like 4.10?

Thanks!
Nick

Re: Some questions on reentrancy, __DYNAMIC_REENT__ and _impure_ptr

[Dave Nadler]

On 4/1/2021 12:58 AM, Nick wrote:
> Hi,
>
> I've been trying to enable reentrancy of newlib on a home brew kernel 
> for the x86 platform and have some questions on how various pieces all 
> fits together.

Oy, we can never have enough kernels ;-)

I'm not familiar with all the possible permutations.
In FreeRTOS, the scheduler simply switches _impure_ptr before each 
context switch.
This is perfectly thread safe given:
- the read/write of this ptr is atomic (true on the architectures I 
know), and
- no ISR use of anything in the RTL requiring this (ie no malloc, 
strtok, etc. in ISR)
Here's the code from FreeRTOS:

         #if ( configUSE_NEWLIB_REENTRANT == 1 )
         {
             /* Switch Newlib's _impure_ptr variable to point to the _reent
             structure specific to this task.
             See the third party link 
http://www.nadler.com/embedded/newlibAndFreeRTOS.html
             for additional information. */
             _impure_ptr = &( pxCurrentTCB->xNewLib_reent );
         }
         #endif /* configUSE_NEWLIB_REENTRANT */

I hope that clears up all your questions below!
Best Regards, Dave



> Implemented __getreent () to return a private copy of struct reent, 
> and also hard coded __DYNAMIC_REENT__ and GETREENT_PROVIDED in 
> sys/config.h to rule out any issue of passing in via build CFLAGS or 
> the CFLAGS in configure.host. Things including errno seem to work but 
> not totally making sense.
>
> As many library functions are still accessing the reent structure 
> using _impure_ptr instead of calling my __getreent () function, for 
> example, the CHECK_INIT (_REENT, fp) at the beginning of __swsetup_r 
> (struct _reent *ptr, register FILE * fp).
>
> Questions:
>
> 1. Are the library functions expected to still use _impure_ptr instead 
> of calling __getreent () when both __DYNAMIC_REENT__ and 
> GETREENT_PROVIDED are hard coded in sys/config.h?
>
> If so, how do they provide reentrancy? Since _impure_ptr is a global 
> pointer visible to all threads and threads can easily step on each 
> other's toes trying to change fields in the reent structure pointed to 
> by _impure_ptr concurrently.
>
> If not, what other MACROs or changes should I make so that all the 
> library functions all use __getreent () instead of _impure_ptr? Is it 
> okay to set _impure_ptr to a bad value such as NULL in this case, in 
> order to catch any unintended access?
>
> 2. in the documentation on https://sourceware.org/newlib/, the 
> following is mentioned as needed for syscalls stubs to return errno:
>
> #include <errno.h>
> #undef errno
> extern int errno;
>
> If I do include this part, all the syscalls stubs seem to do when they 
> assign values to errno is setting the global int errno; inside 
> reent.c. As user code built against the library don’t read out that 
> integer but instead calls __(), errno set by syscall stubs can't be 
> read out by user code.
>
> If on the other hand I don’t include this part before my syscall 
> stubs, the errno set by them do seem to work as they also set the copy 
> in reent structures. What might I have missed here?
>
> 3. There were some old discussions about manually changing _impure_ptr 
> at each context switch. But I’m wondering about the validity of such a 
> method since it seems like a really clumsy maneuver for kernel code at 
> CPL0 to reach into user space belonging to different binaries to 
> change a global pointer. What's more, if manually changing _impure_ptr 
> at each context switch is needed, then what would be the purpose of 
> __DYNAMIC_REENT__, GETREENT_PROVIDED and implementing a __getreent () 
> to get a thread local version?
>
> 4. Is _global_impure_ptr thread safe? It is a bit concerning as it 
> seems to be pointing to the same copy of impure_data that some 
> libraries calls would access, and even if I try to change _impure_ptr 
> at each context switch, some threads might still be accessing 
> _global_impure_ptr concurrently?
>
> 5. There were also old discussions about having to provide mutex for 
> malloc, is this still the case for newer versions of newlib like 4.10?
>
> Thanks!
> Nick


-- 
Dave Nadler, USA East Coast voice (978) 263-0097, drn@nadler.com, Skype
  Dave.Nadler1

Re: Some questions on reentrancy, __DYNAMIC_REENT__ and _impure_ptr

[Nick]

Thanks Dave, haha we can never have enough of anything =)

It is a bit more challenging for me as AFAIK FreeRTOS is statically 
linked so it can easily see and change the _impure_ptr pointer. But in 
my case, the kernel is a standalone binary which loads other programs 
(that are linked with newlib, elf format) at runtime, so it either has 
to parse for that pointer during load, or require special arrangement in 
crt0 to report the pointer's location during process init.

It's feasible, but I'm hoping to get the __DYNAMIC_REENT__ method to 
work as it seems quite a bit cleaner.

Nick

2021-04-01 10:48 に Dave Nadler さんは書きました:
> On 4/1/2021 12:58 AM, Nick wrote:
> 
>> Hi,
>> 
>> I've been trying to enable reentrancy of newlib on a home brew
>> kernel for the x86 platform and have some questions on how various
>> pieces all fits together.
> 
> Oy, we can never have enough kernels ;-)
> 
> I'm not familiar with all the possible permutations.
> In FreeRTOS, the scheduler simply switches _impure_ptr before each
> context switch.
> This is perfectly thread safe given:
> - the read/write of this ptr is atomic (true on the architectures I
> know), and
> - no ISR use of anything in the RTL requiring this (ie no malloc,
> strtok, etc. in ISR)
> Here's the code from FreeRTOS:
> 
>         #if ( configUSE_NEWLIB_REENTRANT == 1 )
>         {
>             /* Switch Newlib's _impure_ptr variable to point to the
> _reent
>             structure specific to this task.
>             See the third party link
> http://www.nadler.com/embedded/newlibAndFreeRTOS.html
>             for additional information. */
>             _impure_ptr = &( pxCurrentTCB->xNewLib_reent );
>         }
>         #endif /* configUSE_NEWLIB_REENTRANT */
> 
> I hope that clears up all your questions below!
> Best Regards, Dave
> 
>> Implemented __getreent () to return a private copy of struct reent,
>> and also hard coded __DYNAMIC_REENT__ and GETREENT_PROVIDED in
>> sys/config.h to rule out any issue of passing in via build CFLAGS or
>> the CFLAGS in configure.host. Things including errno seem to work
>> but not totally making sense.
>> 
>> As many library functions are still accessing the reent structure
>> using _impure_ptr instead of calling my __getreent () function, for
>> example, the CHECK_INIT (_REENT, fp) at the beginning of __swsetup_r
>> (struct _reent *ptr, register FILE * fp).
>> 
>> Questions:
>> 
>> 1. Are the library functions expected to still use _impure_ptr
>> instead of calling __getreent () when both __DYNAMIC_REENT__ and
>> GETREENT_PROVIDED are hard coded in sys/config.h?
>> 
>> If so, how do they provide reentrancy? Since _impure_ptr is a global
>> pointer visible to all threads and threads can easily step on each
>> other's toes trying to change fields in the reent structure pointed
>> to by _impure_ptr concurrently.
>> 
>> If not, what other MACROs or changes should I make so that all the
>> library functions all use __getreent () instead of _impure_ptr? Is
>> it okay to set _impure_ptr to a bad value such as NULL in this case,
>> in order to catch any unintended access?
>> 
>> 2. in the documentation on https://sourceware.org/newlib/, the
>> following is mentioned as needed for syscalls stubs to return errno:
>> 
>> 
>> #include <errno.h>
>> #undef errno
>> extern int errno;
>> 
>> If I do include this part, all the syscalls stubs seem to do when
>> they assign values to errno is setting the global int errno; inside
>> reent.c. As user code built against the library don’t read out
>> that integer but instead calls __(), errno set by syscall stubs
>> can't be read out by user code.
>> 
>> If on the other hand I don’t include this part before my syscall
>> stubs, the errno set by them do seem to work as they also set the
>> copy in reent structures. What might I have missed here?
>> 
>> 3. There were some old discussions about manually changing
>> _impure_ptr at each context switch. But I’m wondering about the
>> validity of such a method since it seems like a really clumsy
>> maneuver for kernel code at CPL0 to reach into user space belonging
>> to different binaries to change a global pointer. What's more, if
>> manually changing _impure_ptr at each context switch is needed, then
>> what would be the purpose of __DYNAMIC_REENT__, GETREENT_PROVIDED
>> and implementing a __getreent () to get a thread local version?
>> 
>> 4. Is _global_impure_ptr thread safe? It is a bit concerning as it
>> seems to be pointing to the same copy of impure_data that some
>> libraries calls would access, and even if I try to change
>> _impure_ptr at each context switch, some threads might still be
>> accessing _global_impure_ptr concurrently?
>> 
>> 5. There were also old discussions about having to provide mutex for
>> malloc, is this still the case for newer versions of newlib like
>> 4.10?
>> 
>> Thanks!
>> Nick
> 
> --
> Dave Nadler, USA East Coast voice (978) 263-0097, drn@nadler.com,
> Skype
>  Dave.Nadler1

Re: Some questions on reentrancy, __DYNAMIC_REENT__ and _impure_ptr

[Dave Nadler]

Thanks Jeff also for answering the mutex question I missed.
One other issue I should have mentioned that becomes more important daily:
If there are multiple processors the simple _impure_ptr mechanism won't 
work...
Good luck and do let us know what you come up with,
Best Regards, Dave

On 4/1/2021 11:39 PM, Nick wrote:
> Thanks Dave, haha we can never have enough of anything =)
>
> It is a bit more challenging for me as AFAIK FreeRTOS is statically 
> linked so it can easily see and change the _impure_ptr pointer. But in 
> my case, the kernel is a standalone binary which loads other programs 
> (that are linked with newlib, elf format) at runtime, so it either has 
> to parse for that pointer during load, or require special arrangement 
> in crt0 to report the pointer's location during process init.
>
> It's feasible, but I'm hoping to get the __DYNAMIC_REENT__ method to 
> work as it seems quite a bit cleaner.
>
> Nick
>
> 2021-04-01 10:48 に Dave Nadler さんは書きました:
>> On 4/1/2021 12:58 AM, Nick wrote:
>>
>>> Hi,
>>>
>>> I've been trying to enable reentrancy of newlib on a home brew
>>> kernel for the x86 platform and have some questions on how various
>>> pieces all fits together.
>>
>> Oy, we can never have enough kernels ;-)
>>
>> I'm not familiar with all the possible permutations.
>> In FreeRTOS, the scheduler simply switches _impure_ptr before each
>> context switch.
>> This is perfectly thread safe given:
>> - the read/write of this ptr is atomic (true on the architectures I
>> know), and
>> - no ISR use of anything in the RTL requiring this (ie no malloc,
>> strtok, etc. in ISR)
>> Here's the code from FreeRTOS:
>>
>>         #if ( configUSE_NEWLIB_REENTRANT == 1 )
>>         {
>>             /* Switch Newlib's _impure_ptr variable to point to the
>> _reent
>>             structure specific to this task.
>>             See the third party link
>> http://www.nadler.com/embedded/newlibAndFreeRTOS.html
>>             for additional information. */
>>             _impure_ptr = &( pxCurrentTCB->xNewLib_reent );
>>         }
>>         #endif /* configUSE_NEWLIB_REENTRANT */
>>
>> I hope that clears up all your questions below!
>> Best Regards, Dave
>>
>>> Implemented __getreent () to return a private copy of struct reent,
>>> and also hard coded __DYNAMIC_REENT__ and GETREENT_PROVIDED in
>>> sys/config.h to rule out any issue of passing in via build CFLAGS or
>>> the CFLAGS in configure.host. Things including errno seem to work
>>> but not totally making sense.
>>>
>>> As many library functions are still accessing the reent structure
>>> using _impure_ptr instead of calling my __getreent () function, for
>>> example, the CHECK_INIT (_REENT, fp) at the beginning of __swsetup_r
>>> (struct _reent *ptr, register FILE * fp).
>>>
>>> Questions:
>>>
>>> 1. Are the library functions expected to still use _impure_ptr
>>> instead of calling __getreent () when both __DYNAMIC_REENT__ and
>>> GETREENT_PROVIDED are hard coded in sys/config.h?
>>>
>>> If so, how do they provide reentrancy? Since _impure_ptr is a global
>>> pointer visible to all threads and threads can easily step on each
>>> other's toes trying to change fields in the reent structure pointed
>>> to by _impure_ptr concurrently.
>>>
>>> If not, what other MACROs or changes should I make so that all the
>>> library functions all use __getreent () instead of _impure_ptr? Is
>>> it okay to set _impure_ptr to a bad value such as NULL in this case,
>>> in order to catch any unintended access?
>>>
>>> 2. in the documentation on https://sourceware.org/newlib/, the
>>> following is mentioned as needed for syscalls stubs to return errno:
>>>
>>>
>>> #include <errno.h>
>>> #undef errno
>>> extern int errno;
>>>
>>> If I do include this part, all the syscalls stubs seem to do when
>>> they assign values to errno is setting the global int errno; inside
>>> reent.c. As user code built against the library don’t read out
>>> that integer but instead calls __(), errno set by syscall stubs
>>> can't be read out by user code.
>>>
>>> If on the other hand I don’t include this part before my syscall
>>> stubs, the errno set by them do seem to work as they also set the
>>> copy in reent structures. What might I have missed here?
>>>
>>> 3. There were some old discussions about manually changing
>>> _impure_ptr at each context switch. But I’m wondering about the
>>> validity of such a method since it seems like a really clumsy
>>> maneuver for kernel code at CPL0 to reach into user space belonging
>>> to different binaries to change a global pointer. What's more, if
>>> manually changing _impure_ptr at each context switch is needed, then
>>> what would be the purpose of __DYNAMIC_REENT__, GETREENT_PROVIDED
>>> and implementing a __getreent () to get a thread local version?
>>>
>>> 4. Is _global_impure_ptr thread safe? It is a bit concerning as it
>>> seems to be pointing to the same copy of impure_data that some
>>> libraries calls would access, and even if I try to change
>>> _impure_ptr at each context switch, some threads might still be
>>> accessing _global_impure_ptr concurrently?
>>>
>>> 5. There were also old discussions about having to provide mutex for
>>> malloc, is this still the case for newer versions of newlib like
>>> 4.10?
>>>
>>> Thanks!
>>> Nick


-- 
Dave Nadler, USA East Coast voice (978) 263-0097, drn@nadler.com, Skype
  Dave.Nadler1

Re: Fw: Some questions on reentrancy, __DYNAMIC_REENT__ and _impure_ptr

[C Howland]

>
>
>
> ------------------------------
> *From:* Newlib <newlib-bounces@sourceware.org> on behalf of Dave Nadler <
> drn@nadler.com>
> *Sent:* Friday, April 2, 2021 7:34 AM
> *To:* Nick <cl26@nicolachel.net>
> *Cc:* newlib@sourceware.org <newlib@sourceware.org>
> *Subject:* Re: Some questions on reentrancy, __DYNAMIC_REENT__ and
> _impure_ptr
>
>
> Thanks Jeff also for answering the mutex question I missed.
> One other issue I should have mentioned that becomes more important daily:
> If there are multiple processors the simple _impure_ptr mechanism won't
> work...
>
Well, if you ignore the fact you have multiple processors and do nothing
about it, then, sure, you'll have problems.  But if you manage it (e.g.
treating each processor as its own thread, or as independent of each other,
etc.), then multiple processors can work fine.  (I have such systems
working.)
Craig

Re: Fw: Some questions on reentrancy, __DYNAMIC_REENT__ and _impure_ptr

[Dave Nadler]

Sorry if I was not clear: If you use one memory location for _impure_ptr,
and you have two processors running two threads, this does not work.
Instead an alternate mechanism (ie thread-local storage for this ptr) is 
needed,
to support each concurrent thread with a separate reentrancy structure.
Hope I was clear this time!
Thanks,
Best Regards, Dve

On 4/2/2021 12:51 PM, C Howland via Newlib wrote:
>> ------------------------------
>> *From:* Newlib <newlib-bounces@sourceware.org> on behalf of Dave Nadler <
>> drn@nadler.com>
>> *Sent:* Friday, April 2, 2021 7:34 AM
>> *To:* Nick <cl26@nicolachel.net>
>> *Cc:* newlib@sourceware.org <newlib@sourceware.org>
>> *Subject:* Re: Some questions on reentrancy, __DYNAMIC_REENT__ and
>> _impure_ptr
>>
>>
>> Thanks Jeff also for answering the mutex question I missed.
>> One other issue I should have mentioned that becomes more important daily:
>> If there are multiple processors the simple _impure_ptr mechanism won't
>> work...
>>
> Well, if you ignore the fact you have multiple processors and do nothing
> about it, then, sure, you'll have problems.  But if you manage it (e.g.
> treating each processor as its own thread, or as independent of each other,
> etc.), then multiple processors can work fine.  (I have such systems
> working.)
> Craig


-- 
Dave Nadler, USA East Coast voice (978) 263-0097, drn@nadler.com, Skype
  Dave.Nadler1

Re: Fw: Some questions on reentrancy, __DYNAMIC_REENT__ and _impure_ptr

[Joel Sherrill]

On Fri, Apr 2, 2021 at 1:35 PM Dave Nadler <drn@nadler.com> wrote:

> Sorry if I was not clear: If you use one memory location for _impure_ptr,
> and you have two processors running two threads, this does not work.
> Instead an alternate mechanism (ie thread-local storage for this ptr) is
> needed,
> to support each concurrent thread with a separate reentrancy structure.
> Hope I was clear this time!
>

I understood it before but RTEMS has SMP support and we dealt with
this issue a few years ago so I had background. You want to make sure
the _impure_ptr is "thread local" whatever that means in the OS
adaptation. And you should be careful to implement the locking support
used in some parts of newlib.

--joel


> Thanks,
> Best Regards, Dve
>
> On 4/2/2021 12:51 PM, C Howland via Newlib wrote:
> >> ------------------------------
> >> *From:* Newlib <newlib-bounces@sourceware.org> on behalf of Dave
> Nadler <
> >> drn@nadler.com>
> >> *Sent:* Friday, April 2, 2021 7:34 AM
> >> *To:* Nick <cl26@nicolachel.net>
> >> *Cc:* newlib@sourceware.org <newlib@sourceware.org>
> >> *Subject:* Re: Some questions on reentrancy, __DYNAMIC_REENT__ and
> >> _impure_ptr
> >>
> >>
> >> Thanks Jeff also for answering the mutex question I missed.
> >> One other issue I should have mentioned that becomes more important
> daily:
> >> If there are multiple processors the simple _impure_ptr mechanism won't
> >> work...
> >>
> > Well, if you ignore the fact you have multiple processors and do nothing
> > about it, then, sure, you'll have problems.  But if you manage it (e.g.
> > treating each processor as its own thread, or as independent of each
> other,
> > etc.), then multiple processors can work fine.  (I have such systems
> > working.)
> > Craig
>
>
> --
> Dave Nadler, USA East Coast voice (978) 263-0097, drn@nadler.com, Skype
>   Dave.Nadler1
>
>

Re: Fw: Some questions on reentrancy, __DYNAMIC_REENT__ and _impure_ptr

[Jeff Johnston]

Hi Dave,

Yes, you can't use a single _impure_ptr for multiple threads.  You need to
either switch the value between threads or use __getreent() to get thread
local storage.

-- Jeff J.

On Fri, Apr 2, 2021 at 2:36 PM Dave Nadler <drn@nadler.com> wrote:

> Sorry if I was not clear: If you use one memory location for _impure_ptr,
> and you have two processors running two threads, this does not work.
> Instead an alternate mechanism (ie thread-local storage for this ptr) is
> needed,
> to support each concurrent thread with a separate reentrancy structure.
> Hope I was clear this time!
> Thanks,
> Best Regards, Dve
>
> On 4/2/2021 12:51 PM, C Howland via Newlib wrote:
> >> ------------------------------
> >> *From:* Newlib <newlib-bounces@sourceware.org> on behalf of Dave
> Nadler <
> >> drn@nadler.com>
> >> *Sent:* Friday, April 2, 2021 7:34 AM
> >> *To:* Nick <cl26@nicolachel.net>
> >> *Cc:* newlib@sourceware.org <newlib@sourceware.org>
> >> *Subject:* Re: Some questions on reentrancy, __DYNAMIC_REENT__ and
> >> _impure_ptr
> >>
> >>
> >> Thanks Jeff also for answering the mutex question I missed.
> >> One other issue I should have mentioned that becomes more important
> daily:
> >> If there are multiple processors the simple _impure_ptr mechanism won't
> >> work...
> >>
> > Well, if you ignore the fact you have multiple processors and do nothing
> > about it, then, sure, you'll have problems.  But if you manage it (e.g.
> > treating each processor as its own thread, or as independent of each
> other,
> > etc.), then multiple processors can work fine.  (I have such systems
> > working.)
> > Craig
>
>
> --
> Dave Nadler, USA East Coast voice (978) 263-0097, drn@nadler.com, Skype
>   Dave.Nadler1
>
>

Re: Some questions on reentrancy, __DYNAMIC_REENT__ and _impure_ptr

[Nick]

Thanks Dave for bringing this up, you are totally right swapping 
_impure_ptr won't work for SMP.

Turns out that my build script wasn't clearing the build dir before each 
build, thus allowing objects compiled with different configs (different 
implementations of _REENT) to be linked together and caused all the 
strangeness.

After fixing that, __DYNAMIC_REENT__ worked as expected and I can now 
also set _impure_ptr to NULL in crt0 without issues.

The mutex/lock part seems a bit more straightforward than reent. 
Multiple threads from the same process can malloc/free in a loop without 
causing any obvious damage to each other now, although I probably need 
to test more to be sure that the heap itself is still healthy.

Thanks Jeff and Dave for helping out!

Nick

2021-04-02 07:34 に Dave Nadler さんは書きました:
> Thanks Jeff also for answering the mutex question I missed.
> One other issue I should have mentioned that becomes more important
> daily:
> If there are multiple processors the simple _impure_ptr mechanism
> won't work...
> Good luck and do let us know what you come up with,
> Best Regards, Dave
> 
> On 4/1/2021 11:39 PM, Nick wrote:
> 
>> Thanks Dave, haha we can never have enough of anything =)
>> 
>> It is a bit more challenging for me as AFAIK FreeRTOS is statically
>> linked so it can easily see and change the _impure_ptr pointer. But
>> in my case, the kernel is a standalone binary which loads other
>> programs (that are linked with newlib, elf format) at runtime, so it
>> either has to parse for that pointer during load, or require special
>> arrangement in crt0 to report the pointer's location during process
>> init.
>> 
>> It's feasible, but I'm hoping to get the __DYNAMIC_REENT__ method to
>> work as it seems quite a bit cleaner.
>> 
>> Nick
>> 
>> 2021-04-01 10:48 に Dave Nadler さんは書きました:
>> On 4/1/2021 12:58 AM, Nick wrote:
>> 
>> Hi,
>> 
>> I've been trying to enable reentrancy of newlib on a home brew
>> kernel for the x86 platform and have some questions on how various
>> pieces all fits together.
>> 
>> Oy, we can never have enough kernels ;-)
>> 
>> I'm not familiar with all the possible permutations.
>> In FreeRTOS, the scheduler simply switches _impure_ptr before each
>> context switch.
>> This is perfectly thread safe given:
>> - the read/write of this ptr is atomic (true on the architectures I
>> know), and
>> - no ISR use of anything in the RTL requiring this (ie no malloc,
>> strtok, etc. in ISR)
>> Here's the code from FreeRTOS:
>> 
>> #if ( configUSE_NEWLIB_REENTRANT == 1 )
>> {
>> /* Switch Newlib's _impure_ptr variable to point to the
>> _reent
>> structure specific to this task.
>> See the third party link
>> http://www.nadler.com/embedded/newlibAndFreeRTOS.html
>> for additional information. */
>> _impure_ptr = &( pxCurrentTCB->xNewLib_reent );
>> }
>> #endif /* configUSE_NEWLIB_REENTRANT */
>> 
>> I hope that clears up all your questions below!
>> Best Regards, Dave
>> 
>> Implemented __getreent () to return a private copy of struct reent,
>> and also hard coded __DYNAMIC_REENT__ and GETREENT_PROVIDED in
>> sys/config.h to rule out any issue of passing in via build CFLAGS or
>> 
>> the CFLAGS in configure.host. Things including errno seem to work
>> but not totally making sense.
>> 
>> As many library functions are still accessing the reent structure
>> using _impure_ptr instead of calling my __getreent () function, for
>> example, the CHECK_INIT (_REENT, fp) at the beginning of __swsetup_r
>> 
>> (struct _reent *ptr, register FILE * fp).
>> 
>> Questions:
>> 
>> 1. Are the library functions expected to still use _impure_ptr
>> instead of calling __getreent () when both __DYNAMIC_REENT__ and
>> GETREENT_PROVIDED are hard coded in sys/config.h?
>> 
>> If so, how do they provide reentrancy? Since _impure_ptr is a global
>> 
>> pointer visible to all threads and threads can easily step on each
>> other's toes trying to change fields in the reent structure pointed
>> to by _impure_ptr concurrently.
>> 
>> If not, what other MACROs or changes should I make so that all the
>> library functions all use __getreent () instead of _impure_ptr? Is
>> it okay to set _impure_ptr to a bad value such as NULL in this case,
>> 
>> in order to catch any unintended access?
>> 
>> 2. in the documentation on https://sourceware.org/newlib/, the
>> following is mentioned as needed for syscalls stubs to return errno:
>> 
>> 
>> #include <errno.h>
>> #undef errno
>> extern int errno;
>> 
>> If I do include this part, all the syscalls stubs seem to do when
>> they assign values to errno is setting the global int errno; inside
>> reent.c. As user code built against the library don’t read out
>> that integer but instead calls __(), errno set by syscall stubs
>> can't be read out by user code.
>> 
>> If on the other hand I don’t include this part before my syscall
>> stubs, the errno set by them do seem to work as they also set the
>> copy in reent structures. What might I have missed here?
>> 
>> 3. There were some old discussions about manually changing
>> _impure_ptr at each context switch. But I’m wondering about the
>> validity of such a method since it seems like a really clumsy
>> maneuver for kernel code at CPL0 to reach into user space belonging
>> to different binaries to change a global pointer. What's more, if
>> manually changing _impure_ptr at each context switch is needed, then
>> 
>> what would be the purpose of __DYNAMIC_REENT__, GETREENT_PROVIDED
>> and implementing a __getreent () to get a thread local version?
>> 
>> 4. Is _global_impure_ptr thread safe? It is a bit concerning as it
>> seems to be pointing to the same copy of impure_data that some
>> libraries calls would access, and even if I try to change
>> _impure_ptr at each context switch, some threads might still be
>> accessing _global_impure_ptr concurrently?
>> 
>> 5. There were also old discussions about having to provide mutex for
>> 
>> malloc, is this still the case for newer versions of newlib like
>> 4.10?
>> 
>> Thanks!
>> Nick
> 
> --
> Dave Nadler, USA East Coast voice (978) 263-0097, drn@nadler.com,
> Skype
>  Dave.Nadler1

Re: Some questions on reentrancy, __DYNAMIC_REENT__ and _impure_ptr

[Jeff Johnston]

On Thu, Apr 1, 2021 at 12:58 AM Nick <cl26@nicolachel.net> wrote:

> Hi,
>
> I've been trying to enable reentrancy of newlib on a home brew kernel
> for the x86 platform and have some questions on how various pieces all
> fits together.
>
> Implemented __getreent () to return a private copy of struct reent, and
> also hard coded __DYNAMIC_REENT__ and GETREENT_PROVIDED in sys/config.h
> to rule out any issue of passing in via build CFLAGS or the CFLAGS in
> configure.host. Things including errno seem to work but not totally
> making sense.
>
> As many library functions are still accessing the reent structure using
> _impure_ptr instead of calling my __getreent () function, for example,
> the CHECK_INIT (_REENT, fp) at the beginning of __swsetup_r (struct
> _reent *ptr, register FILE * fp).
>
> Questions:
>
> 1. Are the library functions expected to still use _impure_ptr instead
> of calling __getreent () when both __DYNAMIC_REENT__ and
> GETREENT_PROVIDED are hard coded in sys/config.h?
>
>
No, the dynamic reent system redefines the _REENT macro in sys/reent.h to
call
__getreent().  Something is going wrong in your build.  You can try
compiling a file with -dD to see what is defined by the preprocessor to see
what went wrong.  If you have specified --enable-newlib-multithread=no or
--disable-newlib-multithread, then the dynamic reent system will not be
used.

If so, how do they provide reentrancy? Since _impure_ptr is a global
> pointer visible to all threads and threads can easily step on each
> other's toes trying to change fields in the reent structure pointed to
> by _impure_ptr concurrently.
>
>
If not, what other MACROs or changes should I make so that all the
> library functions all use __getreent () instead of _impure_ptr? Is it
> okay to set _impure_ptr to a bad value such as NULL in this case, in
> order to catch any unintended access?
>
>
Note there is a global impure ptr reference used for std I/O which is shared
between threads.

2. in the documentation on https://sourceware.org/newlib/, the following
> is mentioned as needed for syscalls stubs to return errno:
>
> #include <errno.h>
> #undef errno
> extern int errno;
>
> If I do include this part, all the syscalls stubs seem to do when they
> assign values to errno is setting the global int errno; inside reent.c.
> As user code built against the library don’t read out that integer but
> instead calls __(), errno set by syscall stubs can't be read out by user
> code.
>
> If on the other hand I don’t include this part before my syscall stubs,
> the errno set by them do seem to work as they also set the copy in reent
> structures. What might I have missed here?
>
>
Look at lib/include/reent.h for explanation of reentrancy and syscalls.
You need to specify true
_r versions of the syscalls that place the errno result into the reentrancy
structure.  Otherwise,
you can have a collision in writing to the global errno value before it
gets transferred unless
you use a lock mechanism to ensure syscalls don't happen at the same time.


3. There were some old discussions about manually changing _impure_ptr
> at each context switch. But I’m wondering about the validity of such a
> method since it seems like a really clumsy maneuver for kernel code at
> CPL0 to reach into user space belonging to different binaries to change
> a global pointer. What's more, if manually changing _impure_ptr at each
> context switch is needed, then what would be the purpose of
> __DYNAMIC_REENT__, GETREENT_PROVIDED and implementing a __getreent () to
> get a thread local version?
>
>
See Dave's response on that.

4. Is _global_impure_ptr thread safe? It is a bit concerning as it seems
> to be pointing to the same copy of impure_data that some libraries calls
> would access, and even if I try to change _impure_ptr at each context
> switch, some threads might still be accessing _global_impure_ptr
> concurrently?
>
>
There are locks that are performed for various I/O functions.  You will
need to provide
the low-level lock support ... i.e. provide a lock.c implementation (see
misc/lock.c, libc/include/sys/lock.h).


> 5. There were also old discussions about having to provide mutex for
> malloc, is this still the case for newer versions of newlib like 4.10?
>
>
There is a mallock lock system which again uses the low-level lock.c
implementation which you need to provide
as part of 4.

Thanks!
> Nick
>
>

-- Jeff J.

Re: Some questions on reentrancy, __DYNAMIC_REENT__ and _impure_ptr

[Nick]

Thanks Jeff!

Have some follow up questions, esp. about _REENT as it seems to take on 
different expansion results depending on whether I try to print it 
during build or not, details below.

2021-04-01 12:26 に Jeff Johnston さんは書きました:
> On Thu, Apr 1, 2021 at 12:58 AM Nick <cl26@nicolachel.net> wrote:
> 
>> Hi,
>> 
>> I've been trying to enable reentrancy of newlib on a home brew
>> kernel
>> for the x86 platform and have some questions on how various pieces
>> all
>> fits together.
>> 
>> Implemented __getreent () to return a private copy of struct reent,
>> and
>> also hard coded __DYNAMIC_REENT__ and GETREENT_PROVIDED in
>> sys/config.h
>> to rule out any issue of passing in via build CFLAGS or the CFLAGS
>> in
>> configure.host. Things including errno seem to work but not totally
>> making sense.
>> 
>> As many library functions are still accessing the reent structure
>> using
>> _impure_ptr instead of calling my __getreent () function, for
>> example,
>> the CHECK_INIT (_REENT, fp) at the beginning of __swsetup_r (struct
>> _reent *ptr, register FILE * fp).
>> 
>> Questions:
>> 
>> 1. Are the library functions expected to still use _impure_ptr
>> instead
>> of calling __getreent () when both __DYNAMIC_REENT__ and
>> GETREENT_PROVIDED are hard coded in sys/config.h?
> 
> No, the dynamic reent system redefines the _REENT macro in sys/reent.h
> to call
> __getreent().  Something is going wrong in your build.  You can try
> compiling a file with -dD to see what is defined by the preprocessor
> to see
> what went wrong.  If you have specified --enable-newlib-multithread=no
> or
> --disable-newlib-multithread, then the dynamic reent system will not
> be used.


You are right! The build has somehow gone wrong. I added #error in the 
_impure_ptr branch in sys/reent.h like below:

#if defined(__DYNAMIC_REENT__) && !defined(__SINGLE_THREAD__)
#ifndef __getreent
   struct _reent * __getreent (void);
#endif
# define _REENT (__getreent())
#else /* __SINGLE_THREAD__ || !__DYNAMIC_REENT__ */

#error __getreent() is not being used!

# define _REENT _impure_ptr
#endif /* __SINGLE_THREAD__ || !__DYNAMIC_REENT__ */

I also broke the stub __getreent () implementation in getreent.c just in 
case it was unexpectedly pulled in instead of my implementation.

However, _REENT would still expand into to _impure_ptr. as observed by 
objdump the resultant binaries, without triggering either of the above.

What's more, if I add a #pragma message right above the _REENT inside 
library source files to print out what it would expand into, the message 
say that it has expanded to __getreent () as expected, and it really 
does, thus making that specific reference to _REENT working as expected. 
But all other _REENTs scattered throughout the library source still 
expended to _impure_ptr and fails at run time. This is despite the 
"#error __getreent() is not being used!" directive inside sys/reent.h 
never catching anything.

Now I'm quite confused and hopefully it wasn't due to today's date :(

Also --enable-newlib-multithread=no wasn't specified, and specifying 
--enable-newlib-multithread=yes to the configure script does not change 
the behavior either.

Any suggestions on what might be the issue here or where to look for it?

>> If so, how do they provide reentrancy? Since _impure_ptr is a global
>> 
>> pointer visible to all threads and threads can easily step on each
>> other's toes trying to change fields in the reent structure pointed
>> to
>> by _impure_ptr concurrently.
> 
>> If not, what other MACROs or changes should I make so that all the
>> library functions all use __getreent () instead of _impure_ptr? Is
>> it
>> okay to set _impure_ptr to a bad value such as NULL in this case, in
>> 
>> order to catch any unintended access?
> 
> Note there is a global impure ptr reference used for std I/O which is
> shared
> between threads.
> 
>> 2. in the documentation on https://sourceware.org/newlib/, the
>> following
>> is mentioned as needed for syscalls stubs to return errno:
>> 
>> #include <errno.h>
>> #undef errno
>> extern int errno;
>> 
>> If I do include this part, all the syscalls stubs seem to do when
>> they
>> assign values to errno is setting the global int errno; inside
>> reent.c.
>> As user code built against the library don’t read out that integer
>> but
>> instead calls __(), errno set by syscall stubs can't be read out by
>> user
>> code.
>> 
>> If on the other hand I don’t include this part before my syscall
>> stubs,
>> the errno set by them do seem to work as they also set the copy in
>> reent
>> structures. What might I have missed here?
> 
> Look at lib/include/reent.h for explanation of reentrancy and
> syscalls.  You need to specify true
> 
> _r versions of the syscalls that place the errno result into the
> reentrancy structure.  Otherwise,
> you can have a collision in writing to the global errno value before
> it gets transferred unless
> you use a lock mechanism to ensure syscalls don't happen at the same
> time.

I thought if I use errno inside syscall stubs, it is a MACRO rather than 
a global and expands to (*__errno()) which is already thread safe, and 
also the same value/address user code on the same thread can read out 
later?

"specify true _r versions" meaning provide syscall stubs that ends with 
_r instead?

>> 3. There were some old discussions about manually changing
>> _impure_ptr
>> at each context switch. But I’m wondering about the validity of
>> such a
>> method since it seems like a really clumsy maneuver for kernel code
>> at
>> CPL0 to reach into user space belonging to different binaries to
>> change
>> a global pointer. What's more, if manually changing _impure_ptr at
>> each
>> context switch is needed, then what would be the purpose of
>> __DYNAMIC_REENT__, GETREENT_PROVIDED and implementing a __getreent
>> () to
>> get a thread local version?
> 
> See Dave's response on that.
> 
>> 4. Is _global_impure_ptr thread safe? It is a bit concerning as it
>> seems
>> to be pointing to the same copy of impure_data that some libraries
>> calls
>> would access, and even if I try to change _impure_ptr at each
>> context
>> switch, some threads might still be accessing _global_impure_ptr
>> concurrently?
> 
> There are locks that are performed for various I/O functions.  You
> will need to provide
> the low-level lock support ... i.e. provide a lock.c implementation
> (see misc/lock.c, libc/include/sys/lock.h).
> 
>> 5. There were also old discussions about having to provide mutex for
>> 
>> malloc, is this still the case for newer versions of newlib like
>> 4.10?
> 
> There is a mallock lock system which again uses the low-level lock.c
> implementation which you need to provide
> as part of 4.

Makes sense, I'll add a lock.

>> Thanks!
>> Nick
> 
> -- Jeff J.

Thanks again,
Nick