[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: regexp_exec thread-unsafe
From: |
Paul Eggert |
Subject: |
Re: regexp_exec thread-unsafe |
Date: |
Sun, 19 May 2013 14:30:55 -0700 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130510 Thunderbird/17.0.6 |
On 05/14/2013 02:21 PM, Ludovic Courtès wrote:
> How should that be fixed? Shouldn’t __libc_lock_unlock & co. be rebased
> on top of pthread_mutex_t?
Yes, thanks, that sounds right. I pushed the following patch into gnulib;
could you please see whether it fixes the problem for you? It'd be
helpful to try it on non-glibc systems too, if possible.
---
ChangeLog | 19 +++++++++++++++++++
lib/regcomp.c | 11 ++++++++---
lib/regex_internal.h | 23 +++++++++++++++++------
lib/regexec.c | 4 ----
modules/regex | 1 +
5 files changed, 45 insertions(+), 13 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index 08042c5..35b6dd2 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,24 @@
2013-05-19 Paul Eggert <address@hidden>
+ regex: fix dfa race in multithreaded uses
+ Problem reported by Ludovic Courtès in
+ <http://lists.gnu.org/archive/html/bug-gnulib/2013-05/msg00058.html>.
+ * lib/regex_internal.h (lock_define, lock_init, lock_fini):
+ New macros. All uses of __libc_lock_define, __libc_lock_init
+ changed to use the first two of these.
+ (__libc_lock_lock, __libc_lock_unlock): New macros, for
+ non-glibc platforms.
+ (struct re_dfa_t): Define the lock unconditionally.
+ * lib/regexec.c (regexec, re_search_stub): Remove some now-incorrect
+ '#ifdef _LIBC"s.
+ * modules/regex (Depends-on): Add pthread, if we use the
+ included regex.
+
+ * lib/regcomp.c: Do actions that are not needed for glibc,
+ but may be needed elsewhere.
+ (regfree, re_compile_internal): Destroy the lock.
+ (re_compile_internal): Check for lock-initialization failure.
+
malloca: port to compilers that reject size-zero arrays
This fixes a bug introduced in my previous patch.
* lib/malloca.c (struct preliminary_header): Use an int
diff --git a/lib/regcomp.c b/lib/regcomp.c
index 4a8485e..5344381 100644
--- a/lib/regcomp.c
+++ b/lib/regcomp.c
@@ -663,7 +663,10 @@ regfree (preg)
{
re_dfa_t *dfa = preg->buffer;
if (BE (dfa != NULL, 1))
- free_dfa_content (dfa);
+ {
+ lock_fini (dfa->lock);
+ free_dfa_content (dfa);
+ }
preg->buffer = NULL;
preg->allocated = 0;
@@ -784,6 +787,8 @@ re_compile_internal (regex_t *preg, const char * pattern,
size_t length,
preg->used = sizeof (re_dfa_t);
err = init_dfa (dfa, length);
+ if (BE (err == REG_NOERROR && lock_init (dfa->lock) != 0, 0))
+ err = REG_ESPACE;
if (BE (err != REG_NOERROR, 0))
{
free_dfa_content (dfa);
@@ -797,8 +802,6 @@ re_compile_internal (regex_t *preg, const char * pattern,
size_t length,
strncpy (dfa->re_str, pattern, length + 1);
#endif
- __libc_lock_init (dfa->lock);
-
err = re_string_construct (®exp, pattern, length, preg->translate,
(syntax & RE_ICASE) != 0, dfa);
if (BE (err != REG_NOERROR, 0))
@@ -806,6 +809,7 @@ re_compile_internal (regex_t *preg, const char * pattern,
size_t length,
re_compile_internal_free_return:
free_workarea_compile (preg);
re_string_destruct (®exp);
+ lock_fini (dfa->lock);
free_dfa_content (dfa);
preg->buffer = NULL;
preg->allocated = 0;
@@ -838,6 +842,7 @@ re_compile_internal (regex_t *preg, const char * pattern,
size_t length,
if (BE (err != REG_NOERROR, 0))
{
+ lock_fini (dfa->lock);
free_dfa_content (dfa);
preg->buffer = NULL;
preg->allocated = 0;
diff --git a/lib/regex_internal.h b/lib/regex_internal.h
index 439444c..63a9979 100644
--- a/lib/regex_internal.h
+++ b/lib/regex_internal.h
@@ -32,12 +32,25 @@
#include <wctype.h>
#include <stdbool.h>
#include <stdint.h>
+
#if defined _LIBC
# include <bits/libc-lock.h>
+#endif
+/* Use __libc_lock_define (, NAME) if the library defines the macro,
+ and if the compiler is known to support empty macro arguments. */
+#if (defined __libc_lock_define \
+ && ((defined __GNUC__ && !defined __STRICT_ANSI__) \
+ || (defined __STDC_VERSION__ && 199901L <= __STDC_VERSION__)))
+# define lock_define(name) __libc_lock_define (, name)
+# define lock_init(lock) (__libc_lock_init (lock), 0)
+# define lock_fini(lock) 0
#else
-# define __libc_lock_init(NAME) do { } while (0)
-# define __libc_lock_lock(NAME) do { } while (0)
-# define __libc_lock_unlock(NAME) do { } while (0)
+# include <pthread.h>
+# define lock_define(name) pthread_mutex_t name;
+# define lock_init(lock) pthread_mutex_init (&(lock), 0)
+# define lock_fini(lock) pthread_mutex_destroy (&(lock))
+# define __libc_lock_lock(lock) pthread_mutex_lock (&(lock))
+# define __libc_lock_unlock(lock) pthread_mutex_unlock (&(lock))
#endif
/* In case that the system doesn't have isblank(). */
@@ -698,9 +711,7 @@ struct re_dfa_t
#ifdef DEBUG
char* re_str;
#endif
-#ifdef _LIBC
- __libc_lock_define (, lock)
-#endif
+ lock_define (lock)
};
#define re_node_set_init_empty(set) memset (set, '\0', sizeof (re_node_set))
diff --git a/lib/regexec.c b/lib/regexec.c
index 09c3eec..114287e 100644
--- a/lib/regexec.c
+++ b/lib/regexec.c
@@ -228,9 +228,7 @@ regexec (preg, string, nmatch, pmatch, eflags)
{
reg_errcode_t err;
Idx start, length;
-#ifdef _LIBC
re_dfa_t *dfa = preg->buffer;
-#endif
if (eflags & ~(REG_NOTBOL | REG_NOTEOL | REG_STARTEND))
return REG_BADPAT;
@@ -421,9 +419,7 @@ re_search_stub (struct re_pattern_buffer *bufp,
Idx nregs;
regoff_t rval;
int eflags = 0;
-#ifdef _LIBC
re_dfa_t *dfa = bufp->buffer;
-#endif
Idx last_start = start + range;
/* Check for out-of-range. */
diff --git a/modules/regex b/modules/regex
index 8f5eda0..2dbb777 100644
--- a/modules/regex
+++ b/modules/regex
@@ -24,6 +24,7 @@ memmove [test $ac_use_included_regex = yes]
mbrtowc [test $ac_use_included_regex = yes]
mbsinit [test $ac_use_included_regex = yes]
nl_langinfo [test $ac_use_included_regex = yes]
+pthread [test $ac_use_included_regex = yes]
stdbool [test $ac_use_included_regex = yes]
stdint [test $ac_use_included_regex = yes]
wchar [test $ac_use_included_regex = yes]
--
1.7.11.7