[GitHub] [incubator-nuttx] xiaoxiang781216 edited a comment on issue #986: Userspace errno

[GitBox <gi...@apache.org>]

xiaoxiang781216 edited a comment on issue #986:
URL: https://github.com/apache/incubator-nuttx/issues/986#issuecomment-625306304


   > ## TLS interfaces
   > TLS is a non-standard, but more general interface. It differs from pthread-specific data only in that is semantics are general; the semantics of the pthread-specific data interfaces are focused on pthreads. But they really should shared the same common underly logic.
   
   But we already use pthread API across the code base which is only portable C API we can use for the  thread creation. I don't think there is any problem that TLS inteface focus on pthreads. The tls_* exist just because we can't move phread TLS storage to the start of the stack because the old implmentation require the very huge stack alignment which isn't suitable for FLAT/PROTECTED build. Since we remove the alignment requirement from the new TLS implmentation, we can move all per thread data(include errno and pthread TLS) into the stack. I prefer that pthread TLS API is implemented directly on top of sched_get_stackinfo and remove all private tls_* API.
   
   > 
   > Currently, there are two TLS interfaces:
   > 
   > ```
   > uintptr_t tls_get_element(int elem);
   > void tls_set_element(int elem, uintptr_t value);
   > ```
   > 
   > I propose that we extend these TLS interfaces so that they can coexit with the pthread interfaces. Linux (actual GLIBC) does not provide a good mechanism; it relies on a storage class that is specific to ELF binaries. That is not useful in an embedded system where no ELF information is present.
   > 
   > [This is a case of the bad decision to let specific tools environment drive a design. Good designs should be independent of tools].
   > 
   > Instead, I think we should adapt from the Windows TLS interfaces which are directly anlagous to the POSIC pthread-specific data interfaces. Here is such an adaption (following NuttX coding standards. See https://en.wikipedia.org/wiki/Thread-local_storage#Windows_implementation):
   > 
   > ```
   > int tls_alloc(void);
   > FAR void *tls_get_value(int tlsindex);
   > bool tls_set_value(int tlsindex, uinptr_t tlsvalue);
   > bool tls_free(int tlsindex);
   > ```
   > 
   > The Windows prototypes can be found from links on this page: https://docs.microsoft.com/en-us/windows/win32/api/processthreadsapi/nf-processthreadsapi-tlsalloc
   >
   
   Why we can't directly use pthread TLS API here? I don't see there is any benefit to define a new set of interface and forward pthread API to them, which just:
   1.Increase the code size
   2.Increase the document effort
   3.Make people confusion which API he/she should use
   4.The mix usage will spread in the code base and inroduce the inconsistence
   
   > ## Accessing the errno from kernel space.
   > We should never access the errno from kernel space. This is especially dangerous from kernel space because we can't be certain which user address environment is in effect or which stack is being used (if we were to use separate kernel mode stacks in the future as Linux does ... for security reasons).
   > 
   > Most OS interfaces have two parts:
   > 
   > 1. A user callable function, say `osapi()` that sets the errno value is necessary (and may implement cancellation points) and
   > 2. An internal OS version which might then be `nx_osapi()` which does not modify the errno value.
   > 
   
   This design isn't perfect, most people don't understand the difference between osapi and nx_osapi. Even he/she know the difference but often forget in coding. I have fixed more than thousound of this type of bug(e.g. driver call osapi) and find that the same issue appear in the new code.
   
   > Ideally, all of the user callable OS interfaces (the `osapi()`) should be moved to the location in libs/libc the system call (`sycall/`) should be redirected to `nx_osapi()`. A complication to doing this is that the OS interfaces which are also cancellation points call special internal interfaces, `enter_cancellation_point()` and `leave_cancellation_point()` that are not available from user space. So some addition partitioning would be need to accomplish this.
   > 
   
   Ideally, we just need to:
   1.Implement osapi which return error code instead modifing errno
   Then the difference can be done by the tool automatically:
   2.STUB_xxx can add cancel point for us
   3.The PROXY_xxx can modify errno for us
   For FLAT build, since we don't separate userspace and kernel space, some hack need to be taken:
   1.Redefine osapi to nx_osapi when __KERNEL__ is defined
   2.Rename osapi to nx_osapi by toolchain like simulator
   Maybe we can find better method after more dissussing. But at least we definitely can find a way to do this thing automatically.
   
   > ## Code References
   > Things to look at:
   > 
   > ```
   > include/errno.h        - Defines current errno access
   > include/nuttx/tls.h    - Defines the tls_info_s structure
   > include/nuttx/sched.h  - The errno and pthread-specific data
   > sched/errno/*          - The old errno access logic.  Move to libs/libc/errno or tls
   > libs/libc/tls          - The new TLS and possible errno access logic
   > sched/pthread          -  pthread_key*.c, pthread_*specific.c - Move to libs/libc/pthread
   > ```
   
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org