Re: Some questions on reentrancy, __DYNAMIC_REENT__ and _impure_ptr

On Thu, Apr 1, 2021 at 12:58 AM Nick <[email protected]> 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.