Move getopt() variables into TLS

getopt() in the FLAT build environment is not thread safe.  This is because global variables that are process-specific in Unix are truly global in the FLAT build.  Moving the getopt() variables into TLS resolves this issue.

No side-effects are expected other than to getopt()

Tested with sim:nsh
This commit is contained in:
Gregory Nutt 2021-03-24 11:34:47 -06:00 committed by Xiang Xiao
parent 22eebf3edf
commit 2ccc0da0c7
18 changed files with 504 additions and 128 deletions

View File

@ -0,0 +1,89 @@
/****************************************************************************
* include/nuttx/lib/getopt.h
*
* Licensed to the Apache Software Foundation (ASF) under one or more
* contributor license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright ownership. The
* ASF licenses this file to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance with the
* License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
* WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
* License for the specific language governing permissions and limitations
* under the License.
*
****************************************************************************/
#ifndef __INCLUDE_NUTTX_LIB_GETOPT_H
#define __INCLUDE_NUTTX_LIB_GETOPT_H
/****************************************************************************
* Included Files
****************************************************************************/
#include <nuttx/config.h>
#include <stdbool.h>
/****************************************************************************
* Pre-processor Definitions
****************************************************************************/
#define GETOPT_INIITIALIZER() = \
{ \
NULL, \
0, \
1, \
'?' \
NULL, \
false \
}
/****************************************************************************
* Public Types
****************************************************************************/
/* This structure encapsulates all variables associated with getopt(). */
struct getopt_s
{
/* Part of the implementation of the public getopt() interface */
FAR char *go_optarg; /* Optional argument following option */
int go_opterr; /* Print error message */
int go_optind; /* Index into argv */
int go_optopt; /* unrecognized option character */
/* Internal getopt() state */
FAR char *go_optptr; /* Current parsing location */
bool go_binitialized; /* true: getopt() has been initialized */
};
/****************************************************************************
* Public Data
****************************************************************************/
#undef EXTERN
#if defined(__cplusplus)
#define EXTERN extern "C"
extern "C"
{
#else
#define EXTERN extern
#endif
/****************************************************************************
* Public Function Prototypes
****************************************************************************/
#undef EXTERN
#if defined(__cplusplus)
}
#endif
#endif /* __INCLUDE_NUTTX_LIB_GETOPT_H */

View File

@ -0,0 +1,76 @@
/****************************************************************************
* include/nuttx/lib/libvars.h
*
* Licensed to the Apache Software Foundation (ASF) under one or more
* contributor license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright ownership. The
* ASF licenses this file to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance with the
* License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
* WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
* License for the specific language governing permissions and limitations
* under the License.
*
****************************************************************************/
#ifndef __INCLUDE_NUTTX_LIB_LIBVARS_H
#define __INCLUDE_NUTTX_LIB_LIBVARS_H
/****************************************************************************
* Included Files
****************************************************************************/
#include <nuttx/config.h>
#include <stdbool.h>
#include <nuttx/lib/getopt.h>
/****************************************************************************
* Public Types
****************************************************************************/
#ifndef CONFIG_BUILD_KERNEL
/* This structure encapsulates all task-specific variables needed by the C
* Library. This structure is retained at the beginning of the main thread
* of and is accessed via a reference stored in the TLS of all threads in
* the task group.
*
* NOTE: task-specific variables are not needed in the KERNEL build. In
* that build mode, all global variables are inherently process-specific.
*/
struct libvars_s
{
struct getopt_s lv_getopt; /* Globals used by getopt() */
};
#endif
/****************************************************************************
* Public Data
****************************************************************************/
#undef EXTERN
#if defined(__cplusplus)
#define EXTERN extern "C"
extern "C"
{
#else
#define EXTERN extern
#endif
/****************************************************************************
* Public Function Prototypes
****************************************************************************/
#undef EXTERN
#if defined(__cplusplus)
}
#endif
#endif /* __INCLUDE_NUTTX_LIB_LIBVARS_H */

View File

@ -38,6 +38,7 @@
#include <nuttx/clock.h>
#include <nuttx/irq.h>
#include <nuttx/wdog.h>
#include <nuttx/lib/libvars.h>
#include <nuttx/mm/shm.h>
#include <nuttx/fs/fs.h>
#include <nuttx/net/net.h>
@ -529,6 +530,12 @@ struct task_group_s
tls_ndxset_t tg_tlsset; /* Set of TLS data indexes allocated */
#endif
/* Task-specific Data *********************************************************/
#ifndef CONFIG_BUILD_KERNEL
FAR struct libvars_s *tg_libvars; /* C library global variables */
#endif
/* POSIX Signal Control Fields ************************************************/
sq_queue_t tg_sigactionq; /* List of actions for signals */

View File

@ -78,6 +78,7 @@ struct tls_info_s
#if CONFIG_TLS_NELEM > 0
uintptr_t tl_elem[CONFIG_TLS_NELEM]; /* TLS elements */
#endif
FAR struct libvars_s *tl_libvars; /* Task-specific C library data */
int tl_errno; /* Per-thread error number */
};

View File

@ -260,6 +260,13 @@
#define getdtablesize(f) ((int)sysconf(_SC_OPEN_MAX))
#define getpagesize(f) ((int)sysconf(_SC_PAGESIZE))
/* Accessor functions associated with getopt(). */
#define optarg (*(getoptargp()))
#define opterr (*(getopterrp()))
#define optind (*(getoptindp()))
#define optopt (*(getoptoptp()))
/****************************************************************************
* Public Data
****************************************************************************/
@ -273,23 +280,6 @@ extern "C"
#define EXTERN extern
#endif
/* Used by getopt (obviously NOT thread safe!). These variables cannot be
* accessed directly by an external NXFLAT module. In that case, accessor
* functions must be used.
*/
#ifndef __NXFLAT__
EXTERN FAR char *optarg; /* Optional argument following option */
EXTERN int opterr; /* Print error message */
EXTERN int optind; /* Index into argv */
EXTERN int optopt; /* Unrecognized option character */
#else
# define optarg (*(getoptargp()))
# define opterr (*(getopterrp()))
# define optind (*(getoptindp()))
# define optopt (*(getoptoptp()))
#endif
/****************************************************************************
* Public Function Prototypes
****************************************************************************/
@ -372,10 +362,7 @@ void swab(FAR const void *src, FAR void *dest, ssize_t nbytes);
int getopt(int argc, FAR char * const argv[], FAR const char *optstring);
/* Accessor functions intended for use only by external NXFLAT
* modules. The global variables optarg, optind, and optopt cannot
* be referenced directly from external modules.
*/
/* Accessor functions associated with getopt(). */
FAR char **getoptargp(void); /* Optional argument following option */
FAR int *getopterrp(void); /* Print error message */

View File

@ -21,8 +21,9 @@
# Add the unistd C files to the build
CSRCS += lib_access.c lib_daemon.c lib_swab.c lib_pathconf.c lib_sysconf.c
CSRCS += lib_getopt.c lib_getoptargp.c lib_getopterrp.c lib_getoptindp.c
CSRCS += lib_getoptoptp.c lib_alarm.c lib_fstatvfs.c lib_statvfs.c lib_sleep.c
CSRCS += lib_getopt.c lib_getoptvars.c lib_getoptargp.c lib_getopterrp.c
CSRCS += lib_getoptindp.c lib_getoptoptp.c
CSRCS += lib_alarm.c lib_fstatvfs.c lib_statvfs.c lib_sleep.c
CSRCS += lib_usleep.c lib_seteuid.c lib_setegid.c lib_geteuid.c lib_getegid.c
CSRCS += lib_setreuid.c lib_setregid.c
CSRCS += lib_getrusage.c lib_utimes.c

View File

@ -28,27 +28,16 @@
#include <getopt.h>
#include <string.h>
#include "unistd.h"
/****************************************************************************
* Pre-processor Definitions
****************************************************************************/
/****************************************************************************
* Public Data
****************************************************************************/
FAR char *optarg; /* Optional argument following option */
int opterr = 0; /* Print error message */
int optind = 1; /* Index into argv */
int optopt = '?'; /* unrecognized option character */
/****************************************************************************
* Private Data
****************************************************************************/
static FAR char *g_optptr = NULL;
static FAR char * const *g_argv = NULL;
static int g_argc = 0;
static bool g_binitialized = false;
#undef optarg
#undef opterr
#undef optind
#undef optopt
/****************************************************************************
* Public Functions
@ -103,18 +92,12 @@ static bool g_binitialized = false;
int getopt(int argc, FAR char * const argv[], FAR const char *optstring)
{
/* Were new argc or argv passed in? This detects misuse of getopt() by
* applications that break out of the getopt() loop before getop() returns
* -1.
*/
/* Get thread-specific getopt() variables */
if (argc != g_argc || argv != g_argv)
FAR struct getopt_s *go = getoptvars();
if (go == NULL)
{
/* Yes, clear the internal state */
g_binitialized = false;
g_argc = argc;
g_argv = argv;
return '?';
}
/* Verify input parameters. */
@ -128,13 +111,13 @@ int getopt(int argc, FAR char * const argv[], FAR const char *optstring)
* the program, optind must be reset to some value <= 1.
*/
if (optind < 1 || !g_binitialized)
if (go->go_optind < 1 || !go->go_binitialized)
{
optarg = NULL;
optind = 1; /* Skip over the program name */
optopt = '?';
g_optptr = NULL; /* Start at the beginning of the first argument */
g_binitialized = true; /* Now we are initialized */
go->go_optarg = NULL;
go->go_optind = 1; /* Skip over the program name */
go->go_optopt = '?';
go->go_optptr = NULL; /* Start at the beginning of the first argument */
go->go_binitialized = true; /* Now we are initialized */
}
/* If the first character of opstring s ':', then ':' is in the event
@ -148,34 +131,34 @@ int getopt(int argc, FAR char * const argv[], FAR const char *optstring)
}
/* Are we resuming in the middle, or at the end of a string of
* arguments? g_optptr == NULL means that we are started at the
* beginning of argv[optind]; *g_optptr == \0 means that we are
* arguments? optptr == NULL means that we are started at the
* beginning of argv[optind]; *optptr == \0 means that we are
* starting at the beginning of optind+1
*/
while (!g_optptr || !*g_optptr)
while (!go->go_optptr || !*go->go_optptr)
{
/* We need to start at the beginning of the next argv. Check if we
* need to increment optind
*/
if (g_optptr)
if (go->go_optptr)
{
/* Yes.. Increment it and check for the case where where we
* have processed everything in the argv[] array.
*/
optind++;
go->go_optind++;
}
/* Check for the end of the argument list */
g_optptr = argv[optind];
if (!g_optptr)
go->go_optptr = argv[go->go_optind];
if (!go->go_optptr)
{
/* There are no more arguments, we are finished */
g_binitialized = false;
go->go_binitialized = false;
return ERROR;
}
@ -183,47 +166,47 @@ int getopt(int argc, FAR char * const argv[], FAR const char *optstring)
* the first character must be '-'
*/
if (*g_optptr != '-')
if (*go->go_optptr != '-')
{
/* The argument does not start with '-', we are finished */
g_binitialized = false;
go->go_binitialized = false;
return ERROR;
}
/* Skip over the '-' */
g_optptr++;
go->go_optptr++;
}
/* Special case handling of "-" and "-:" */
if (!*g_optptr)
if (!*go->go_optptr)
{
optopt = '\0'; /* We'll fix up g_optptr the next time we are called */
go->go_optopt = '\0'; /* We'll fix up optptr the next time we are called */
return '?';
}
/* Handle the case of "-:" */
if (*g_optptr == ':')
if (*go->go_optptr == ':')
{
optopt = ':';
g_optptr++;
go->go_optopt = ':';
go->go_optptr++;
return '?';
}
/* g_optptr now points at the next option and it is not something
/* go->go_optptr now points at the next option and it is not something
* crazy. check if the option is in the list of valid options.
*/
optchar = strchr(optstring, *g_optptr);
optchar = strchr(optstring, *go->go_optptr);
if (!optchar)
{
/* No this character is not in the list of valid options */
optopt = *g_optptr;
g_optptr++;
go->go_optopt = *go->go_optptr;
go->go_optptr++;
return '?';
}
@ -235,7 +218,7 @@ int getopt(int argc, FAR char * const argv[], FAR const char *optstring)
{
/* No, no arguments. Just return the character that we found */
g_optptr++;
go->go_optptr++;
return *optchar;
}
@ -243,40 +226,40 @@ int getopt(int argc, FAR char * const argv[], FAR const char *optstring)
* immediately after the command in this same argument?
*/
if (g_optptr[1] != '\0')
if (go->go_optptr[1] != '\0')
{
/* Yes, return a pointer into the current argument */
optarg = &g_optptr[1];
optind++;
g_optptr = NULL;
go->go_optarg = &go->go_optptr[1];
go->go_optind++;
go->go_optptr = NULL;
return *optchar;
}
/* No.. is the optional argument the next argument in argv[] ? */
if (argv[optind + 1] && *argv[optind + 1] != '-')
if (argv[go->go_optind + 1] && *argv[go->go_optind + 1] != '-')
{
/* Yes.. return that */
optarg = argv[optind + 1];
optind += 2;
g_optptr = NULL;
go->go_optarg = argv[go->go_optind + 1];
go->go_optind += 2;
go->go_optptr = NULL;
return *optchar;
}
/* No argument was supplied */
g_optptr = NULL;
optarg = NULL;
optopt = *optchar;
optind++;
go->go_optptr = NULL;
go->go_optarg = NULL;
go->go_optopt = *optchar;
go->go_optind++;
return noarg_ret;
}
/* Restore the initial, uninitialized state */
g_binitialized = false;
go->go_binitialized = false;
return ERROR;
}

View File

@ -26,17 +26,7 @@
#include <unistd.h>
/****************************************************************************
* Pre-processor Definitions
****************************************************************************/
/****************************************************************************
* Public Data
****************************************************************************/
/****************************************************************************
* Private Data
****************************************************************************/
#include "unistd.h"
/****************************************************************************
* Public Functions
@ -53,5 +43,6 @@
FAR char **getoptargp(void)
{
return &optarg;
FAR struct getopt_s *go = getoptvars();
return &go->go_optarg;
}

View File

@ -26,6 +26,8 @@
#include <unistd.h>
#include "unistd.h"
/****************************************************************************
* Public Functions
****************************************************************************/
@ -41,5 +43,6 @@
FAR int *getopterrp(void)
{
return &opterr;
FAR struct getopt_s *go = getoptvars();
return &go->go_opterr;
}

View File

@ -26,17 +26,7 @@
#include <unistd.h>
/****************************************************************************
* Pre-processor Definitions
****************************************************************************/
/****************************************************************************
* Public Data
****************************************************************************/
/****************************************************************************
* Private Data
****************************************************************************/
#include "unistd.h"
/****************************************************************************
* Public Functions
@ -53,5 +43,6 @@
int *getoptindp(void)
{
return &optind;
FAR struct getopt_s *go = getoptvars();
return &go->go_optind;
}

View File

@ -26,17 +26,7 @@
#include <unistd.h>
/****************************************************************************
* Pre-processor Definitions
****************************************************************************/
/****************************************************************************
* Public Data
****************************************************************************/
/****************************************************************************
* Private Data
****************************************************************************/
#include "unistd.h"
/****************************************************************************
* Public Functions
@ -53,5 +43,6 @@
int *getoptoptp(void)
{
return &optopt;
FAR struct getopt_s *go = getoptvars();
return &go->go_optopt;
}

View File

@ -0,0 +1,83 @@
/****************************************************************************
* libs/libc/unistd/lib_getoptvars.c
*
* Licensed to the Apache Software Foundation (ASF) under one or more
* contributor license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright ownership. The
* ASF licenses this file to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance with the
* License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
* WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
* License for the specific language governing permissions and limitations
* under the License.
*
****************************************************************************/
/****************************************************************************
* Included Files
****************************************************************************/
#include <nuttx/config.h>
#include <unistd.h>
#include <assert.h>
#include <nuttx/tls.h>
#include <nuttx/lib/libvars.h>
#include <arch/tls.h>
#include "unistd.h"
#include "libc.h"
/****************************************************************************
* Private Data
****************************************************************************/
#ifdef CONFIG_BUILD_KERNEL
/* Data is naturally process-specific in the KERNEL build so no special
* access to process-specific global data is needed.
*/
FAR struct getopt_s g_getopt_vars =
{
NULL,
0,
1,
'?',
NULL,
false
};
#endif
/****************************************************************************
* Public Functions
****************************************************************************/
/****************************************************************************
* Name: getoptvars
*
* Description:
* Returns a pointer to to the thread-specific getopt() data.
*
****************************************************************************/
FAR struct getopt_s *getoptvars(void)
{
#ifndef CONFIG_BUILD_KERNEL
FAR struct tls_info_s *tlsinfo;
/* Get the structure of getopt() variables using the key. */
tlsinfo = up_tls_info();
DEBUGASSERT(tlsinfo != NULL && tlsinfo->tl_libvars != NULL);
return &tlsinfo->tl_libvars->lv_getopt;
#else
return &g_getopt_vars;
#endif
}

66
libs/libc/unistd/unistd.h Normal file
View File

@ -0,0 +1,66 @@
/****************************************************************************
* libs/libc/unistd/unistd.h
*
* Licensed to the Apache Software Foundation (ASF) under one or more
* contributor license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright ownership. The
* ASF licenses this file to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance with the
* License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
* WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
* License for the specific language governing permissions and limitations
* under the License.
*
****************************************************************************/
#ifndef __LIBC_UNISTD_UNISTD_H
#define __LIBC_UNISTD_UNISTD_H
/****************************************************************************
* Included Files
****************************************************************************/
#include <nuttx/config.h>
#include <stdbool.h>
#include <nuttx/lib/getopt.h>
/****************************************************************************
* Public Data
****************************************************************************/
#undef EXTERN
#if defined(__cplusplus)
#define EXTERN extern "C"
extern "C"
{
#else
#define EXTERN extern
#endif
/****************************************************************************
* Public Function Prototypes
****************************************************************************/
/****************************************************************************
* Name: getoptvars
*
* Description:
* Returns a pointer to to the thread-specific getopt() data.
*
****************************************************************************/
FAR struct getopt_s *getoptvars(void);
#undef EXTERN
#if defined(__cplusplus)
}
#endif
#endif /* __LIBC_UNISTD_UNISTD_H */

View File

@ -57,6 +57,10 @@ ifneq ($(CONFIG_TLS_NELEM),0)
CSRCS += group_tlsalloc.c group_tlsfree.c
endif
ifndef CONFIG_BUILD_KERNEL
CSRCS += group_taskdata.c
endif
# Include group build support
DEPPATH += --dep-path group

View File

@ -140,4 +140,10 @@ int group_setuptaskfiles(FAR struct task_tcb_s *tcb);
int group_setupstreams(FAR struct task_tcb_s *tcb);
#endif
#ifndef CONFIG_BUILD_KERNEL
/* Task specific data */
void tls_set_taskdata(FAR struct tcb_s *tcb);
#endif
#endif /* __SCHED_GROUP_GROUP_H */

View File

@ -0,0 +1,75 @@
/****************************************************************************
* sched/group/group_taskdata.c
*
* Licensed to the Apache Software Foundation (ASF) under one or more
* contributor license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright ownership. The
* ASF licenses this file to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance with the
* License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
* WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
* License for the specific language governing permissions and limitations
* under the License.
*
****************************************************************************/
/****************************************************************************
* Included Files
****************************************************************************/
#include <nuttx/config.h>
#include <sched.h>
#include <assert.h>
#include <nuttx/tls.h>
#include "group/group.h"
#ifndef CONFIG_BUILD_KERNEL
/****************************************************************************
* Public Functions
****************************************************************************/
/****************************************************************************
* Name: tls_set_taskdata
*
* Description:
* Set task-specific data pointer in TLS.
*
* Input Parameters:
* tcb - Identifies task to set TLS data
*
* Returned Value:
* None
*
****************************************************************************/
void tls_set_taskdata(FAR struct tcb_s *tcb)
{
FAR struct tls_info_s *info;
FAR struct task_group_s *group;
DEBUGASSERT(tcb != NULL && tcb->group != NULL);
group = tcb->group;
/* This currently assumes a push-down stack. The TLS data lies at the
* lowest address of the stack allocation.
*/
info = (FAR struct tls_info_s *)tcb->stack_alloc_ptr;
/* Copy the task data point buffer in the group structure into the
* thread's TLS data.
*/
info->tl_libvars = group->tg_libvars;
}
#endif /* !CONFIG_BUILD_KERNEL */

View File

@ -301,6 +301,12 @@ int pthread_create(FAR pthread_t *thread, FAR const pthread_attr_t *attr,
goto errout_with_join;
}
#ifndef CONFIG_BUILD_KERNEL
/* Save the allocated task data in TLS */
tls_set_taskdata(&ptcb->cmn);
#endif
/* Should we use the priority and scheduler specified in the pthread
* attributes? Or should we use the current thread's priority and
* scheduler?

View File

@ -32,6 +32,7 @@
#include <nuttx/arch.h>
#include <nuttx/sched.h>
#include <nuttx/lib/libvars.h>
#include "sched/sched.h"
#include "group/group.h"
@ -84,11 +85,14 @@ int nxtask_init(FAR struct task_tcb_s *tcb, const char *name, int priority,
main_t entry, FAR char * const argv[])
{
uint8_t ttype = tcb->cmn.flags & TCB_FLAG_TTYPE_MASK;
#ifndef CONFIG_BUILD_KERNEL
FAR struct task_group_s *group;
#endif
int ret;
#ifndef CONFIG_DISABLE_PTHREAD
/* Only tasks and kernel threads can be initialized in this way */
#ifndef CONFIG_DISABLE_PTHREAD
DEBUGASSERT(tcb && ttype != TCB_FLAG_TTYPE_PTHREAD);
#endif
@ -126,6 +130,18 @@ int nxtask_init(FAR struct task_tcb_s *tcb, const char *name, int priority,
goto errout_with_group;
}
#ifndef CONFIG_BUILD_KERNEL
/* Allocate a stack frame to hold task-specific data */
group = tcb->cmn.group;
group->tg_libvars = up_stack_frame(&tcb->cmn, sizeof(struct libvars_s));
DEBUGASSERT(group->tg_libvars != NULL);
/* Save the allocated task data in TLS */
tls_set_taskdata(&tcb->cmn);
#endif
/* Initialize the task control block */
ret = nxtask_setup_scheduler(tcb, priority, nxtask_start,