Skip to content

Fix wrong-result and UB bugs found by second memory/thread-safety sweep #109

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Qubitium merged 1 commit into from
Aug 24, 2026
Merged

Fix wrong-result and UB bugs found by second memory/thread-safety sweep #109

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions pcre/threads.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,10 @@ def _pool_for_target_locked(
def ensure_thread_pool(max_workers: int | None = None) -> ThreadPoolExecutor:
"""Return the shared executor, creating or resizing it if required."""

# Resolve the (possibly subprocess-backed) CPU probe before taking the
# process-wide pool lock; probing under the lock would stall every other
# thread touching the pool behind a fork/exec.
_performance_cpu_total()
with _THREAD_POOL_LOCK:
target = _determine_worker_count(
max_workers if max_workers is not None else _THREAD_POOL_WORKERS
Expand Down Expand Up @@ -158,6 +162,7 @@ def _thread_pool_submission(
"""

old_pool: ThreadPoolExecutor | None = None
_performance_cpu_total() # probe outside the lock (see ensure_thread_pool)
try:
with _THREAD_POOL_LOCK:
target = _determine_worker_count(
Expand Down Expand Up @@ -230,6 +235,7 @@ def get_thread_pool_size() -> int:
if snapshot is not None and snapshot == _THREAD_POOL_WORKERS:
return snapshot

_performance_cpu_total() # probe outside the lock (see ensure_thread_pool)
with _THREAD_POOL_LOCK:
if _THREAD_POOL_WORKERS is None:
_THREAD_POOL_WORKERS = _determine_worker_count(None)
Expand Down
29 changes: 29 additions & 0 deletions pcre_ext/atomic_compat.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,11 @@ static __forceinline int ac_cas_ptr (void * volatile *p, void **e, void *d) {
/* These work on *plain* volatile T* (no _Atomic required). */
/* --------------------------------------------------------------------- */

/* On 32-bit Windows size_t IS unsigned int (== uint32_t): listing both in
* one _Generic is a duplicate-association constraint violation, so the
* size_t branches exist only where size_t is a distinct 64-bit type. */
#if SIZE_MAX > 0xFFFFFFFFu

/* Load */
#define atomic_compat_load(ptr) \
_Generic((ptr), \
Expand All @@ -245,6 +250,30 @@ static __forceinline int ac_cas_ptr (void * volatile *p, void **e, void *d) {
/* pointer */ default: ac_store_ptr((void * volatile *)(ptr), (void*)(value)) \
)

#else /* 32-bit size_t: the uint32_t associations already cover it */

/* Load */
#define atomic_compat_load(ptr) \
_Generic((ptr), \
/* signed */ volatile int32_t *: ac_load_i32((volatile int32_t*)(ptr)), \
int32_t *: ac_load_i32((volatile int32_t*)(ptr)), \
/* unsigned */ volatile uint32_t *: ac_load_u32((volatile uint32_t*)(ptr)), \
uint32_t *: ac_load_u32((volatile uint32_t*)(ptr)), \
/* pointer */ default: ac_load_ptr((void * volatile *)(ptr)) \
)

/* Store */
#define atomic_compat_store(ptr, value) \
_Generic((ptr), \
/* signed */ volatile int32_t *: ac_store_i32((volatile int32_t*)(ptr), (int32_t)(value)), \
int32_t *: ac_store_i32((volatile int32_t*)(ptr), (int32_t)(value)), \
/* unsigned */ volatile uint32_t *: ac_store_u32((volatile uint32_t*)(ptr), (uint32_t)(value)), \
uint32_t *: ac_store_u32((volatile uint32_t*)(ptr), (uint32_t)(value)), \
/* pointer */ default: ac_store_ptr((void * volatile *)(ptr), (void*)(value)) \
)

#endif /* SIZE_MAX > 0xFFFFFFFFu */

/* Exchange */
#define atomic_compat_exchange(ptr, value) \
_Generic((ptr), \
Expand Down
4 changes: 3 additions & 1 deletion pcre_ext/cache.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -562,7 +562,9 @@ cache_initialize(int global_mode)

cache_strategy_set(global_mode ? CACHE_STRATEGY_GLOBAL : CACHE_STRATEGY_THREAD_LOCAL);
cache_strategy_set_locked(0);
atomic_store_explicit(&context_cache_enabled, 1, memory_order_release);
/* context_cache_enabled is NOT reset here: module_exec has already
* applied the PYPCRE_DISABLE_CONTEXT_CACHE environment toggle, and
* storing 1 unconditionally would silently clobber it. */

global_match_cache_clear();
global_jit_cache_clear();
Expand Down
161 changes: 116 additions & 45 deletions pcre_ext/pcre2.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,19 +74,21 @@ coerce_uint32_argument(PyObject *value, const char *name, uint32_t *out)
#define PCRE2_GIL_RELEASE_THRESHOLD 262144ULL
#define PCRE_PATTERN_CACHE_INPUT_LIMIT (64 * 1024)

#if defined(Py_GIL_DISABLED)
#define PCRE2_CALL_RELEASE_GIL(call) \
do { \
rc = (call); \
} while (0)
#else
/*
* Detach the thread state on both GIL and free-threaded builds. On GIL
* builds this releases the GIL; on free-threaded builds it marks the thread
* as safe for a stop-the-world pause β€” without it, a thread inside a long
* pcre2_match (huge subject, catastrophic backtracking) blocks every
* gc.collect() in the process for the duration of the match. The wrapped
* calls are pure PCRE2 with exclusively-owned arguments, so running them
* detached is safe.
*/
#define PCRE2_CALL_RELEASE_GIL(call) \
do { \
PyThreadState *_save = PyEval_SaveThread(); \
rc = (call); \
PyEval_RestoreThread(_save); \
} while (0)
#endif

#define PCRE2_CALL_MAYBE_RELEASE_GIL(call, length) \
do { \
Expand All @@ -97,15 +99,6 @@ coerce_uint32_argument(PyObject *value, const char *name, uint32_t *out)
} \
} while (0)

#if defined(Py_GIL_DISABLED)
#define PCRE2_JIT_CALL_MAYBE_RELEASE_GIL(call, length) \
do { \
(void)(length); \
jit_guard_acquire(); \
rc = (call); \
jit_guard_release(); \
} while (0)
#else
#define PCRE2_JIT_CALL_MAYBE_RELEASE_GIL(call, length) \
do { \
if ((length) > PCRE2_GIL_RELEASE_THRESHOLD) { \
Expand All @@ -120,7 +113,6 @@ coerce_uint32_argument(PyObject *value, const char *name, uint32_t *out)
jit_guard_release(); \
} \
} while (0)
#endif

static inline pcre2_match_data *
pattern_match_data_acquire(PatternObject *pattern, int *from_pattern_cache)
Expand Down Expand Up @@ -2020,14 +2012,10 @@ Match_expand(MatchObject *self, PyObject *template_obj)
return NULL;
}

/* The helper is a module function, so a direct dictionary lookup avoids
attribute lookup machinery on every expand() call while retaining the
module's normal import/refcount lifetime. */
PyObject *helper = PyDict_GetItemString(
PyModule_GetDict(module),
"expand_match_template"
);
Py_XINCREF(helper);
/* GetAttr returns a strong reference. A borrowed dict lookup here could
be freed by a concurrent rebind of the module attribute (monkeypatch,
reload) before the INCREF on free-threaded builds. */
PyObject *helper = PyObject_GetAttrString(module, "expand_match_template");
Py_DECREF(module);
if (helper == NULL) {
return NULL;
Expand Down Expand Up @@ -2452,6 +2440,13 @@ FindIter_iternext_unlocked(FindIterObject *self)
}

int use_jit = pattern_jit_get(self->pattern) && !self->retry_nonempty;
if (use_jit && self->subject_is_bytes &&
(self->pattern->compile_options & PCRE2_UTF) != 0 &&
!(self->base_options & PCRE2_NO_UTF_CHECK)) {
/* Partial range of a UTF bytes subject: only the interpreter path
* validates pos/endpos character boundaries (see Pattern_execute). */
use_jit = 0;
}
if (use_jit) {
if (self->match_context == NULL) {
self->match_context = pcre2_match_context_create(NULL);
Expand Down Expand Up @@ -2765,7 +2760,10 @@ Pattern_create_finditer(PatternObject *pattern,
iter->resolved_end = 0;
iter->resolved_end_byte = 0;
iter->has_endpos = 0;
iter->base_options = options;
/* Strip caller-supplied PCRE2_NO_UTF_CHECK; it is re-added below only
* for ranges this module has validated (mid-character offsets under the
* flag are undefined behavior in PCRE2). */
iter->base_options = options & ~(uint32_t)PCRE2_NO_UTF_CHECK;
iter->exhausted = 0;
iter->match_data = NULL;
iter->match_context = NULL;
Expand Down Expand Up @@ -3272,14 +3270,32 @@ Pattern_execute(PatternObject *self, PyObject *subject_obj, Py_ssize_t pos,
return NULL;
}

/*
* The first-literal prescan must stay conservative for ASCII letters:
* PCRE2 reports a first code unit for patterns whose first character is
* caseless via a scoped inline group β€” (?i:a)b, ((?i)a)b β€” and exposes no
* pattern_info for its internal FIRSTCASELESS bit, so `first_literal_
* caseless` cannot see those. Accepting either case of a letter keeps
* the filter sound (a false pass just falls through to pcre2_match).
* Non-ASCII lead bytes are safe as-is: PCRE2 only reports a single first
* code unit when every case variant shares it.
*/
if (mode == EXEC_MODE_SEARCH && self->has_first_literal) {
if (byte_start >= byte_end) {
Py_DECREF(utf8_owner);
Py_RETURN_NONE;
}
const unsigned char *scan_start = (const unsigned char *)(buffer + byte_start);
size_t span = (size_t)(byte_end - byte_start);
if (memchr(scan_start, (unsigned char)self->first_literal, span) == NULL) {
unsigned char lit = (unsigned char)self->first_literal;
unsigned char folded = (unsigned char)(lit | 0x20u);
if (folded >= 'a' && folded <= 'z') {
if (memchr(scan_start, folded, span) == NULL &&
memchr(scan_start, (unsigned char)(folded ^ 0x20u), span) == NULL) {
Py_DECREF(utf8_owner);
Py_RETURN_NONE;
}
} else if (memchr(scan_start, lit, span) == NULL) {
Py_DECREF(utf8_owner);
Py_RETURN_NONE;
}
Expand All @@ -3292,15 +3308,26 @@ Pattern_execute(PatternObject *self, PyObject *subject_obj, Py_ssize_t pos,
Py_RETURN_NONE;
}
unsigned char leading = (unsigned char)buffer[byte_start];
if (leading != (unsigned char)self->first_literal) {
unsigned char lit = (unsigned char)self->first_literal;
unsigned char folded = (unsigned char)(lit | 0x20u);
int mismatch;
if (folded >= 'a' && folded <= 'z') {
mismatch = ((unsigned char)(leading | 0x20u) != folded);
} else {
mismatch = (leading != lit);
}
if (mismatch) {
Py_DECREF(utf8_owner);
Py_RETURN_NONE;
}
}

PCRE2_SIZE offset_limit = (PCRE2_SIZE)byte_end;

uint32_t match_options = options;
/* Never trust a caller-supplied PCRE2_NO_UTF_CHECK: with a mid-character
* pos/endpos it makes pcre2_match undefined behavior. The flag is
* re-added below exactly when this module has validated the range. */
uint32_t match_options = options & ~(uint32_t)PCRE2_NO_UTF_CHECK;
if (mode == EXEC_MODE_MATCH) {
match_options |= PCRE2_ANCHORED;
} else if (mode == EXEC_MODE_FULLMATCH) {
Expand All @@ -3327,7 +3354,24 @@ Pattern_execute(PatternObject *self, PyObject *subject_obj, Py_ssize_t pos,

int rc = 0;
int attempt_jit = pattern_jit_get(self);
/* pcre2_jit_match skips every UTF validity check that pcre2_match
* performs. For a partial range of a UTF bytes subject we deliberately
* leave PCRE2_NO_UTF_CHECK unset so PCRE2 rejects mid-character
* pos/endpos; that contract only holds on the interpreter path, so take
* it for those calls (a mid-character offset into JIT-compiled code is
* undefined behavior and returns silently wrong results). */
if (attempt_jit && subject_is_bytes &&
(self->compile_options & PCRE2_UTF) != 0 &&
!(match_options & PCRE2_NO_UTF_CHECK)) {
attempt_jit = 0;
}
int jit_endanchor_uncertain = 0;
/* Whether the JIT call above delivered a definitive result for THIS call.
* The interpreter fallback must not consult the pattern-global jit flag:
* when this call skips JIT (or JIT reports BADOPTION) while the global
* flag is still set, neither engine would run and the uninitialized
* match_data (rc == 0) would be turned into a garbage Match. */
int jit_produced_result = 0;
pcre2_match_context *match_context = NULL;
int match_context_from_pattern = 0;
int match_context_used_offset_limit = 0;
Expand Down Expand Up @@ -3426,7 +3470,10 @@ Pattern_execute(PatternObject *self, PyObject *subject_obj, Py_ssize_t pos,
Py_DECREF(utf8_owner);
raise_pcre_error("jit_match", rc, error_offset);
return NULL;
} else if (jit_anchor_fixup_needed() && rc >= 0 &&
} else {
jit_produced_result = 1;
}
if (jit_produced_result && jit_anchor_fixup_needed() && rc >= 0 &&
(mode == EXEC_MODE_MATCH || mode == EXEC_MODE_FULLMATCH)) {
/*
* Some PCRE2 builds' pcre2_jit_match() silently ignore
Expand All @@ -3443,7 +3490,7 @@ Pattern_execute(PatternObject *self, PyObject *subject_obj, Py_ssize_t pos,
}
}

if (!pattern_jit_get(self) || jit_endanchor_uncertain) {
if (!jit_produced_result || jit_endanchor_uncertain) {
/*
* For the fullmatch JIT fallback, truncate the interpreter re-run
* to the requested endpos (offset_limit). This guarantees that
Expand Down Expand Up @@ -3847,9 +3894,17 @@ Pattern_findall(PatternObject *self,
pcre2_jit_stack_assign(match_context, NULL, jit_stack);
}

uint32_t match_options = options;
/* Strip caller-supplied PCRE2_NO_UTF_CHECK (re-added below only for
* module-validated ranges); the internal PCRE2_USE_OFFSET_LIMIT bit
* OR'd in above survives the mask. */
uint32_t match_options = options & ~(uint32_t)PCRE2_NO_UTF_CHECK;
if (!subject_is_bytes || (byte_start == 0 && byte_end == subject_length_bytes)) {
match_options |= PCRE2_NO_UTF_CHECK;
} else if (attempt_jit && (self->compile_options & PCRE2_UTF) != 0) {
/* Partial range of a UTF bytes subject: only the interpreter
* validates pos/endpos boundaries, so do not run JIT-compiled code
* on a potentially mid-character offset (documented UB). */
attempt_jit = 0;
}

result = PyList_New(0);
Expand Down Expand Up @@ -4323,13 +4378,18 @@ Pattern_substitute(PatternObject *self,
goto error;
}

/* pcre2_substitute executes JIT-compiled code whenever the pattern's
* code block carries a JIT translation β€” even after this module's
* jit_enabled flag was cleared by a JIT_BADOPTION fallback β€” so key the
* serialization guard (PYPCRE_FORCE_JIT_LOCK platforms) on the compiled
* code itself, not just on whether we assigned a jit stack. */
size_t code_jit_size = 0;
(void)pcre2_pattern_info(self->code, PCRE2_INFO_JITSIZE, &code_jit_size);
int guard_jit = (jit_stack != NULL) || code_jit_size != 0;

for (int attempts = 0; attempts < 5; ++attempts) {
limit_state.stopped = 0;
int rc;
/* pcre2_substitute executes JIT-compiled code when the pattern is
* JIT-enabled, so it needs the same serialization guard as every
* other JIT execution site (PYPCRE_FORCE_JIT_LOCK platforms). */
int guard_jit = (jit_stack != NULL);
if (guard_jit) {
jit_guard_acquire();
}
Expand Down Expand Up @@ -6121,13 +6181,19 @@ module_exec(PyObject *module)
int force_jit_lock = 0;
int first_init = !atomic_load_explicit(&module_fully_initialized, memory_order_acquire);

force_lock_env = Py_GETENV("PYPCRE_FORCE_JIT_LOCK");
if (force_lock_env == NULL) {
force_lock_env = Py_GETENV("PCRE2_FORCE_JIT_LOCK");
}
force_jit_lock = env_flag_is_true(force_lock_env);
if (jit_support_initialize(force_jit_lock) < 0) {
goto error_jit_support;
if (first_init) {
/* The jit serial lock must not materialize (or change) after threads
* have observed its absence: jit_guard_release would release a lock
* that jit_guard_acquire never took. Latch the decision at first
* init; a re-exec keeps the existing configuration. */
force_lock_env = Py_GETENV("PYPCRE_FORCE_JIT_LOCK");
if (force_lock_env == NULL) {
force_lock_env = Py_GETENV("PCRE2_FORCE_JIT_LOCK");
}
force_jit_lock = env_flag_is_true(force_lock_env);
if (jit_support_initialize(force_jit_lock) < 0) {
goto error_jit_support;
}
}

if (first_init) {
Expand All @@ -6145,8 +6211,13 @@ module_exec(PyObject *module)
pattern_cache_env = Py_GETENV("PCRE2_CACHE_PATTERN_GLOBAL");
}
pattern_cache_global = env_flag_is_true(pattern_cache_env);
if (pattern_cache_initialize(pattern_cache_global) < 0) {
goto error_pattern_cache;
if (first_init) {
/* A re-exec with a changed env var must not flip the pattern-cache
* mode: threads may hold references into the published global map,
* and cache.c's strategy (latched above) would disagree with it. */
if (pattern_cache_initialize(pattern_cache_global) < 0) {
goto error_pattern_cache;
}
}

if (PyType_Ready(&PatternType) < 0) {
Expand Down
11 changes: 8 additions & 3 deletions pcre_ext/string_helpers.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -528,13 +528,18 @@ module_translate_unicode_escapes(PyObject *Py_UNUSED(module), PyObject *arg)
}
if (valid) {
if (codepoint > 0x10FFFFu) {
/* PyErr_Format does not support the dynamic
* `%.*s` precision specifier (it raises
* SystemError), so copy the digits out first. */
char hex_digits[9];
memcpy(hex_digits, run_end + 1, (size_t)hex_len);
hex_digits[hex_len] = '\0';
PyMem_Free(buffer);
PyErr_Format(
PcreError,
"Unicode escape \\%c%.*s exceeds 0x10FFFF",
"Unicode escape \\%c%s exceeds 0x10FFFF",
*run_end,
hex_len,
run_end + 1
hex_digits
);
return NULL;
}
Expand Down
Loading