Project

General

Profile

Actions
Copy link

Feature #13842

open

thread-local errno is all you need

Added by Joshua M. Clulow over 2 years ago. Updated over 1 year ago.

Status:
New
Priority:
Normal
Assignee:
Joshua M. Clulow
Category:
lib - userland libraries
Start date:
Due date:
% Done:

0%

Estimated time:
Difficulty:
Medium
Tags:
Gerrit CR:
External Bug:

Description

Many modern C/C++ programs use threads. The mechanics of errno in threaded programs is slightly more complex than in the pre-thread era: while errno must appear to C source as a global variable, it must in reality have a per-thread value.

In /usr/include/errno.h, we define an errno macro which is implemented in terms of a function, ___errno(), which returns a pointer to the per-thread version of errno that software should be using:

#if defined(_REENTRANT) || defined(_TS_ERRNO) || _POSIX_C_SOURCE - 0 >= 199506L
extern int *___errno();
#define errno (*(___errno()))
#else
extern int errno;
/* ANSI C++ requires that errno be a macro */
#if __cplusplus >= 199711L
#define errno errno
#endif
#endif  /* defined(_REENTRANT) || defined(_TS_ERRNO) */

By default, neither _REENTRANT nor _TS_ERRNO are defined in the compilation environment, so it is unfortunately very easy for a program that uses threads to be miscompiled to use the global errno. This can result in failures with apparently spurious "Error 0" conditions in messages when software attempts to use, e.g., strerror().

In the file lib/libc/port/threads/thr.c, in libc_init(), we set the per-thread location for errno to the classical global errno (which we cannot remove for binary compatibility reasons):

        self->ul_errnop = &errno;

In all other threads, we set it to a location in the thread ulwp_t structure:

        ulwp->ul_errnop = &ulwp->ul_errno;

While the threads page in the manual does correctly direct people to the use of -D_REENTRANT for multi-threaded programs, it seems that at a minimum we could stop requiring it to get the thread-local errno. We could replace the entire definition in the header with an unconditional:

extern int *___errno();
#define errno (*(___errno()))

___errno() would continue to return the location of the global errno for the first thread, as it does today, and this change would have no impact on the ABI. It is unlikely that any correct C/C++ program would stop building as a result of this change.

Actions
Copy link
 #1

Updated by Joshua M. Clulow over 2 years ago

  • Description updated (diff)
Actions
Copy link
 #2

Updated by Joshua M. Clulow over 2 years ago

  • Description updated (diff)
Actions
Copy link
 #4

Updated by Joshua M. Clulow over 2 years ago

In addition to errno, there is a similar global t_errno variable defined in other headers (tiuser.h, xti.h, etc) that we should likely fix at the same time and in the same way:

#if defined(_REENTRANT) || defined(_TS_ERRNO) || \
        _POSIX_C_SOURCE - 0 >= 199506L
extern int      *__t_errno(void);
#define t_errno (*(__t_errno()))
#else
extern int t_errno;
#endif  /* defined(_REENTRANT) || defined(_TS_ERRNO) */
Actions
Copy link
 #5

Updated by Rich Lowe over 1 year ago

What we want here is to break everything under TS_ERRNO out and make it the default.

Unfortunately, there are other errno that are just under _REENTRANT.
I think what we want from this change is for anything errno-like to be thread-safe by default, regardless of whether it was protected by TS_ERRNO (such as errno and t_errno), or _REENTRANT (such as _h_errno, _nderror, etc.)

Actions
Copy link
 #6

Updated by Joshua M. Clulow over 1 year ago

I've been doing a bit of a survey of places where we're looking at _REENTRANT but not TS_ERRNO. These bits are relatively clear cut:

  • uts/common/rpc/rpcsec_defs.h
    • defines rpc_gss_err as *__rpc_gss_err() and should do so unconditionally
    • rpc_gss_err doesn't seem to be mentioned in a manual page
  • uts/common/rpc/clnt.h
    • defines rpc_createerr as *__rpc_createerr() and should do so unconditionally
    • defines rpc_callerr as *__rpc_callerr() and should do so unconditionally
    • probably need to update rpc_clnt_create(3NSL) which calls rpc_createerr out as a global and mentions that the macroness is conditional right now
  • head/time.h
    • defines getdate_err as _getdate_err_addr() and should do so unconditionally
    • some rework of getdate(3C) likely required
  • head/netdir.h
    • defines _nderror to be *__nderror() and should do so unconditionally
    • this one is ostensibly totally private

This is a bit more sketchy and I don't yet know the full scope of it:

  • head/netdb.h
    • defines h_errno as *__h_errno() but apparently only if H_ERRNO_IS_FUNCTION is defined, and I don't see that anywhere at all?!
    • the h_errno global is mentioned in a few places in netdb.h(3HEAD) and particularly in endhostent(3XNET) where it is explicitly called out as being global and in opposition to the reentrant versions which use an errno pointer in their signatures
    • maybe we should leave this one alone?

This stuff is seemingly not errno-related, but definitely requires investigation as part of broader _REENTRANT cleanup:

  • head/resolv.h
  • head/libgen.h
  • lib/libgen/inc/regexpr.h
  • lib/libgen/common/reg_compile.c
  • lib/libgen/common/reg_step.c
  • lib/libgen/common/bufsplit.c
  • lib/libgen/common/pathfind.c
  • lib/libgen/common/bgets.c

Finally, there is the small matter of the standalone environment for KMDB. In three libraries, we currently engage in shenanigans to undefine TS_ERRNO when compiling for the standalone environment so that we use the stock errno.h:

  • define ___errno() in cmd/mdb/common/libstand/errno.c as a wrapper around a global errno so that we can use the regular headers without engaging in #ifdef _STANDALONE special casery
Actions
Copy link
 #7

Updated by Joshua M. Clulow over 1 year ago

  • Assignee set to Joshua M. Clulow
Actions
Copy link

Also available in: Atom PDF