>From 5dbb9992fe9c872b3fb57b79a8a74161d05ea50b Mon Sep 17 00:00:00 2001 From: Bruno Haible Date: Tue, 17 Dec 2019 14:00:59 +0100 Subject: [PATCH 2/3] nl_langinfo: Fix multithread-safety bug on mingw and MSVC. * lib/nl_langinfo.c (ctype_codeset, rpl_nl_langinfo): Use a stack-allocated buffer to assemble each result and different static buffers to return it. * tests/test-nl_langinfo-mt.c: New file. * modules/nl_langinfo-tests (Files): Add it. (Depends-on): Add thread, nanosleep. (Makefile.am): Build test-nl_langinfo-mt test. --- ChangeLog | 11 ++ lib/nl_langinfo.c | 101 +++++++++++++------ modules/nl_langinfo-tests | 8 +- tests/test-nl_langinfo-mt.c | 238 ++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 325 insertions(+), 33 deletions(-) create mode 100644 tests/test-nl_langinfo-mt.c diff --git a/ChangeLog b/ChangeLog index 9027da4..bb6b22b 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,16 @@ 2019-12-17 Bruno Haible + nl_langinfo: Fix multithread-safety bug on mingw and MSVC. + * lib/nl_langinfo.c (ctype_codeset, rpl_nl_langinfo): Use a + stack-allocated buffer to assemble each result and different static + buffers to return it. + * tests/test-nl_langinfo-mt.c: New file. + * modules/nl_langinfo-tests (Files): Add it. + (Depends-on): Add thread, nanosleep. + (Makefile.am): Build test-nl_langinfo-mt test. + +2019-12-17 Bruno Haible + langinfo: Document more details. * doc/posix-headers/langinfo.texi: List platform details. * doc/posix-functions/nl_langinfo.texi: Likewise. diff --git a/lib/nl_langinfo.c b/lib/nl_langinfo.c index 76579a8..2c4d3a4 100644 --- a/lib/nl_langinfo.c +++ b/lib/nl_langinfo.c @@ -28,13 +28,25 @@ # include #endif +/* nl_langinfo() must be multithread-safe. To achieve this without using + thread-local storage: + 1. We use a specific static buffer for each possible argument. + So that different threads can call nl_langinfo with different arguments, + without interfering. + 2. We use a simple strcpy or memcpy to fill this static buffer. Filling it + through, for example, strcpy + strcat would not be guaranteed to leave + the buffer's contents intact if another thread is currently accessing + it. If necessary, the contents is first assembled in a stack-allocated + buffer. */ + #if !REPLACE_NL_LANGINFO || GNULIB_defined_CODESET /* Return the codeset of the current locale, if this is easily deducible. Otherwise, return "". */ static char * ctype_codeset (void) { - static char buf[2 + 10 + 1]; + static char result[2 + 10 + 1]; + char buf[2 + 10 + 1]; char const *locale = setlocale (LC_CTYPE, NULL); char *codeset = buf; size_t codesetlen; @@ -79,11 +91,16 @@ ctype_codeset (void) /* For a locale name such as "French_France.65001", in Windows 10, setlocale now returns "French_France.utf8" instead. */ if (strcmp (buf + 2, "65001") == 0 || strcmp (buf + 2, "utf8") == 0) - return "UTF-8"; + return (char *) "UTF-8"; else - return memcpy (buf, "CP", 2); + { + memcpy (buf, "CP", 2); + strcpy (result, buf); + return result; + } # else - return codeset; + strcpy (result, codeset); + return result; #endif } #endif @@ -175,7 +192,7 @@ rpl_nl_langinfo (nl_item item) char * nl_langinfo (nl_item item) { - static char nlbuf[100]; + char buf[100]; struct tm tmm = { 0 }; switch (item) @@ -215,14 +232,22 @@ nl_langinfo (nl_item item) case T_FMT_AMPM: return (char *) "%I:%M:%S %p"; case AM_STR: - if (!strftime (nlbuf, sizeof nlbuf, "%p", &tmm)) - return (char *) "AM"; - return nlbuf; + { + static char result[80]; + if (!strftime (buf, sizeof result, "%p", &tmm)) + return (char *) "AM"; + strcpy (result, buf); + return result; + } case PM_STR: - tmm.tm_hour = 12; - if (!strftime (nlbuf, sizeof nlbuf, "%p", &tmm)) - return (char *) "PM"; - return nlbuf; + { + static char result[80]; + tmm.tm_hour = 12; + if (!strftime (buf, sizeof result, "%p", &tmm)) + return (char *) "PM"; + strcpy (result, buf); + return result; + } case DAY_1: case DAY_2: case DAY_3: @@ -231,14 +256,16 @@ nl_langinfo (nl_item item) case DAY_6: case DAY_7: { + static char result[7][50]; static char const days[][sizeof "Wednesday"] = { "Sunday", "Monday", "Tuesday", "Wednesday", "Thursday", "Friday", "Saturday" }; tmm.tm_wday = item - DAY_1; - if (!strftime (nlbuf, sizeof nlbuf, "%A", &tmm)) + if (!strftime (buf, sizeof result[0], "%A", &tmm)) return (char *) days[item - DAY_1]; - return nlbuf; + strcpy (result[item - DAY_1], buf); + return result[item - DAY_1]; } case ABDAY_1: case ABDAY_2: @@ -248,13 +275,15 @@ nl_langinfo (nl_item item) case ABDAY_6: case ABDAY_7: { + static char result[7][30]; static char const abdays[][sizeof "Sun"] = { "Sun", "Mon", "Tue", "Wed", "Thu", "Fri", "Sat" }; tmm.tm_wday = item - ABDAY_1; - if (!strftime (nlbuf, sizeof nlbuf, "%a", &tmm)) + if (!strftime (buf, sizeof result[0], "%a", &tmm)) return (char *) abdays[item - ABDAY_1]; - return nlbuf; + strcpy (result[item - ABDAY_1], buf); + return result[item - ABDAY_1]; } { static char const months[][sizeof "September"] = { @@ -273,10 +302,14 @@ nl_langinfo (nl_item item) case MON_10: case MON_11: case MON_12: - tmm.tm_mon = item - MON_1; - if (!strftime (nlbuf, sizeof nlbuf, "%B", &tmm)) - return (char *) months[item - MON_1]; - return nlbuf; + { + static char result[12][50]; + tmm.tm_mon = item - MON_1; + if (!strftime (buf, sizeof result[0], "%B", &tmm)) + return (char *) months[item - MON_1]; + strcpy (result[item - MON_1], buf); + return result[item - MON_1]; + } case ALTMON_1: case ALTMON_2: case ALTMON_3: @@ -289,15 +322,19 @@ nl_langinfo (nl_item item) case ALTMON_10: case ALTMON_11: case ALTMON_12: - tmm.tm_mon = item - ALTMON_1; - /* The platforms without nl_langinfo() don't support strftime with %OB. - We don't even need to try. */ - #if 0 - if (!strftime (nlbuf, sizeof nlbuf, "%OB", &tmm)) - #endif - if (!strftime (nlbuf, sizeof nlbuf, "%B", &tmm)) - return (char *) months[item - ALTMON_1]; - return nlbuf; + { + static char result[12][50]; + tmm.tm_mon = item - ALTMON_1; + /* The platforms without nl_langinfo() don't support strftime with + %OB. We don't even need to try. */ + #if 0 + if (!strftime (buf, sizeof result[0], "%OB", &tmm)) + #endif + if (!strftime (buf, sizeof result[0], "%B", &tmm)) + return (char *) months[item - ALTMON_1]; + strcpy (result[item - ALTMON_1], buf); + return result[item - ALTMON_1]; + } } case ABMON_1: case ABMON_2: @@ -312,14 +349,16 @@ nl_langinfo (nl_item item) case ABMON_11: case ABMON_12: { + static char result[12][30]; static char const abmonths[][sizeof "Jan"] = { "Jan", "Feb", "Mar", "Apr", "May", "Jun", "Jul", "Sep", "Oct", "Nov", "Dec" }; tmm.tm_mon = item - ABMON_1; - if (!strftime (nlbuf, sizeof nlbuf, "%b", &tmm)) + if (!strftime (buf, sizeof result[0], "%b", &tmm)) return (char *) abmonths[item - ABMON_1]; - return nlbuf; + strcpy (result[item - ABMON_1], buf); + return result[item - ABMON_1]; } case ERA: return (char *) ""; diff --git a/modules/nl_langinfo-tests b/modules/nl_langinfo-tests index 2d3fda0..db0a57d 100644 --- a/modules/nl_langinfo-tests +++ b/modules/nl_langinfo-tests @@ -1,6 +1,7 @@ Files: tests/test-nl_langinfo.sh tests/test-nl_langinfo.c +tests/test-nl_langinfo-mt.c tests/signature.h tests/macros.h m4/locale-fr.m4 @@ -8,12 +9,15 @@ m4/locale-fr.m4 Depends-on: c-strcase setlocale +thread +nanosleep configure.ac: gt_LOCALE_FR gt_LOCALE_FR_UTF8 Makefile.am: -TESTS += test-nl_langinfo.sh +TESTS += test-nl_langinfo.sh test-nl_langinfo-mt TESTS_ENVIRONMENT += LOCALE_FR='@LOCALE_FR@' LOCALE_FR_UTF8='@LOCALE_FR_UTF8@' -check_PROGRAMS += test-nl_langinfo +check_PROGRAMS += test-nl_langinfo test-nl_langinfo-mt +test_nl_langinfo_mt_LDADD = $(LDADD) $(LIBMULTITHREAD) $(LIB_NANOSLEEP) diff --git a/tests/test-nl_langinfo-mt.c b/tests/test-nl_langinfo-mt.c new file mode 100644 index 0000000..66bcbf6 --- /dev/null +++ b/tests/test-nl_langinfo-mt.c @@ -0,0 +1,238 @@ +/* Multithread-safety test for nl_langinfo(). + Copyright (C) 2019 Free Software Foundation, Inc. + + This program is free software: you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 3 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program. If not, see . */ + +/* Written by Bruno Haible , 2019. */ + +#include + +/* Specification. */ +#include + +#include +#include +#include +#include +#include + +#include "glthread/thread.h" + + +/* Some common locale names. */ + +#if defined _WIN32 && !defined __CYGWIN__ +# define ENGLISH "English_United States" +# define FRENCH "French_France" +# define GERMAN "German_Germany" +# define ENCODING ".1252" +#else +# define ENGLISH "en_US" +# define FRENCH "fr_FR" +# define GERMAN "de_DE" +# if defined __sgi +# define ENCODING ".ISO8859-15" +# elif defined __hpux +# define ENCODING ".utf8" +# else +# define ENCODING ".UTF-8" +# endif +#endif + +static const char LOCALE1[] = ENGLISH ENCODING; +static const char LOCALE2[] = FRENCH ENCODING; +static const char LOCALE3[] = GERMAN ENCODING; + +static char *expected1; + +static void * +thread1_func (void *arg) +{ + for (;;) + { + const char *value = nl_langinfo (CODESET); + if (strcmp (expected1, value) != 0) + { + fprintf (stderr, "thread1 disturbed by threadN!\n"); fflush (stderr); + abort (); + } + } + + /*NOTREACHED*/ + return NULL; +} + +static char *expected2; + +static void * +thread2_func (void *arg) +{ + for (;;) + { + const char *value = nl_langinfo (PM_STR); + if (strcmp (expected2, value) != 0) + { + fprintf (stderr, "thread2 disturbed by threadN!\n"); fflush (stderr); + abort (); + } + } + + /*NOTREACHED*/ + return NULL; +} + +static char *expected3; + +static void * +thread3_func (void *arg) +{ + for (;;) + { + const char *value = nl_langinfo (DAY_2); + if (strcmp (expected3, value) != 0) + { + fprintf (stderr, "thread3 disturbed by threadN!\n"); fflush (stderr); + abort (); + } + } + + /*NOTREACHED*/ + return NULL; +} + +static char *expected4; + +static void * +thread4_func (void *arg) +{ + for (;;) + { + const char *value = nl_langinfo (ALTMON_2); + if (strcmp (expected4, value) != 0) + { + fprintf (stderr, "thread4 disturbed by threadN!\n"); fflush (stderr); + abort (); + } + } + + /*NOTREACHED*/ + return NULL; +} + +static char *expected5; + +static void * +thread5_func (void *arg) +{ + for (;;) + { + const char *value = nl_langinfo (CRNCYSTR); + if (strcmp (expected5, value) != 0) + { + fprintf (stderr, "thread5 disturbed by threadN!\n"); fflush (stderr); + abort (); + } + } + + /*NOTREACHED*/ + return NULL; +} + +static char *expected6; + +static void * +thread6_func (void *arg) +{ + for (;;) + { + const char *value = nl_langinfo (RADIXCHAR); + if (strcmp (expected6, value) != 0) + { + fprintf (stderr, "thread6 disturbed by threadN!\n"); fflush (stderr); + abort (); + } + } + + /*NOTREACHED*/ + return NULL; +} + +static void * +threadN_func (void *arg) +{ + for (;;) + { + nl_langinfo (CODESET); /* LC_CTYPE */ /* locale charmap */ + nl_langinfo (AM_STR); /* LC_TIME */ /* locale -k am_pm */ + nl_langinfo (PM_STR); /* LC_TIME */ /* locale -k am_pm */ + nl_langinfo (DAY_2); /* LC_TIME */ /* locale -k day */ + nl_langinfo (DAY_5); /* LC_TIME */ /* locale -k day */ + nl_langinfo (ALTMON_2); /* LC_TIME */ /* locale -k alt_mon */ + nl_langinfo (ALTMON_9); /* LC_TIME */ /* locale -k alt_mon */ + nl_langinfo (CRNCYSTR); /* LC_MONETARY */ /* locale -k currency_symbol */ + nl_langinfo (RADIXCHAR); /* LC_NUMERIC */ /* locale -k decimal_point */ + nl_langinfo (THOUSEP); /* LC_NUMERIC */ /* locale -k thousands_sep */ + } + + /*NOTREACHED*/ + return NULL; +} + +int +main (int argc, char *argv[]) +{ + if (setlocale (LC_ALL, LOCALE1) == NULL) + { + fprintf (stderr, "Skipping test: LOCALE1 not recognized\n"); + return 77; + } + if (setlocale (LC_MONETARY, LOCALE2) == NULL) + { + fprintf (stderr, "Skipping test: LOCALE2 not recognized\n"); + return 77; + } + if (setlocale (LC_NUMERIC, LOCALE3) == NULL) + { + fprintf (stderr, "Skipping test: LOCALE3 not recognized\n"); + return 77; + } + + expected1 = strdup (nl_langinfo (CODESET)); + expected2 = strdup (nl_langinfo (PM_STR)); + expected3 = strdup (nl_langinfo (DAY_2)); + expected4 = strdup (nl_langinfo (ALTMON_2)); + expected5 = strdup (nl_langinfo (CRNCYSTR)); + expected6 = strdup (nl_langinfo (RADIXCHAR)); + + /* Create the checker threads. */ + gl_thread_create (thread1_func, NULL); + gl_thread_create (thread2_func, NULL); + gl_thread_create (thread3_func, NULL); + gl_thread_create (thread4_func, NULL); + gl_thread_create (thread5_func, NULL); + gl_thread_create (thread6_func, NULL); + /* Create the disturber thread. */ + gl_thread_create (threadN_func, NULL); + + /* Let them run for 2 seconds. */ + { + struct timespec duration; + duration.tv_sec = (argc > 1 ? atoi (argv[1]) : 2); + duration.tv_nsec = 0; + + nanosleep (&duration, NULL); + } + + return 0; +} -- 2.7.4