Skip to content

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

Merged
Qubitium merged 1 commit into
mainfrom
fix/second-safety-sweep
Aug 24, 2026
Merged

Fix wrong-result and UB bugs found by second memory/thread-safety sweep#109
Qubitium merged 1 commit into
mainfrom
fix/second-safety-sweep

Conversation

@Qubitium

Copy link
Copy Markdown
Contributor

Summary

Follow-up to #108. After that PR merged, a clean re-audit of main was run: a fresh-eyes review of pcre2.c, a deep pass over the helper files that had lighter coverage the first time, a second pass over the Python layer, and an adversarial review of the #108 diff itself — which found no regressions (the exclusive-ownership context cache, FindIter locking, GC wiring, and allocator changes all held up under source scrutiny and multi-threaded stress on 3.14/3.14t). The sweep did surface the following remaining bugs, now fixed. The first three are user-visible wrong-result bugs reproducible from the public API today.

Wrong results / undefined behavior

Bug Symptom Fix
utf8_index_to_offset chunk-boundary error (util.c) The 8-byte starter scan could return an offset pointing inside a multi-byte character when a chunk boundary splits it near the string tail. pcre.compile('.').search('𐍈a𐍈b', 3) → span (3,3), .group() raises UnicodeDecodeError (re gives (3,4)); str subjects always run PCRE2_NO_UTF_CHECK, so PCRE2 also received mid-codepoint offsets — documented UB Tail scan now always skips trailing continuation bytes
Interpreter fallback keyed on the pattern-global jit flag (Pattern_execute) Any call that skips JIT locally while pattern->jit_enabled stays set ran neither engine: rc stayed 0 and the uninitialized match_data became a Match with garbage offsets (segfault on .group()). Latent until the partial-range JIT skip below armed it — caught by this PR's own regression tests Per-call jit_produced_result flag decides the fallback
First-literal prescan vs. inline caselessness (?i:abc), ((?i)abc) report a first code unit but their caselessness is invisible to pattern_info, so pcre.compile('(?i:abc)').search('ABC')None Prescan accepts both cases of an ASCII letter (still a pure filter; PCRE2 only reports a non-ASCII first unit when all case variants share it)
JIT bypasses UTF offset validation pcre2_jit_match performs none of the checks the module relies on for partial bytes ranges: mid-character pos on a UTF bytes subject returned None under JIT where the interpreter raises PcreErrorBadutfoffset — and executed JIT code on malformed boundaries Partial ranges of UTF bytes subjects take the interpreter path (execute, findall, finditer)
PCRE2_NO_UTF_CHECK smuggling via options= User-supplied flag reached pcre2_match unmasked; with a mid-character pos that is documented UB triggerable from Python Masked out of caller options everywhere; the module re-adds it only for validated ranges

Free-threading / liveness

  • Stop-the-world stalls (GIL=0): free-threaded builds never detached the thread state around PCRE2 calls, so a thread in a long pcre2_match (huge subject, catastrophic backtracking) blocked every gc.collect() in the process until the match finished. Large calls now detach exactly like GIL builds (same 256 KiB threshold).
  • Match_expand borrowed-ref UAF: the expand_match_template helper was fetched with PyDict_GetItemString and INCREF'd afterwards — a concurrent rebind (monkeypatch/reload) could free it in the window. Now a strong reference via PyObject_GetAttrString. Same class as the error.c fix in Fix memory and thread-safety issues for free-threaded (GIL=0) Python #108.
  • jit serial lock latch: importlib.reload with PYPCRE_FORCE_JIT_LOCK newly set could materialize jit_serial_lock while a thread was between jit_guard_acquire (saw NULL, took nothing) and jit_guard_release (sees the lock, releases it) — permanently breaking JIT serialization. jit_support_initialize and pattern_cache_initialize (whose mode flip had the analogous problem) are now latched to first init.
  • Pattern_substitute guard completeness: pcre2_substitute executes JIT code even after a JIT_BADOPTION downgrade cleared the module's flag; the guard is now also keyed on PCRE2_INFO_JITSIZE.

Smaller fixes

  • PYPCRE_DISABLE_CONTEXT_CACHE was dead: cache_initialize unconditionally re-stored context_cache_enabled = 1 right after module_exec applied the env toggle.
  • PyErr_Format doesn't support %.*s — an out-of-range \U escape raised SystemError instead of PcreError (string_helpers.c).
  • MSVC-fallback _Generic maps in atomic_compat.h listed volatile uint32_t* and volatile size_t* — identical types on 32-bit Windows, a compile error; the size_t associations are now conditional on 64-bit size_t.
  • threads.py: the macOS sysctl probe could still run under the process-wide pool lock via ensure_thread_pool / _thread_pool_submission / get_thread_pool_size (the Fix memory and thread-safety issues for free-threaded (GIL=0) Python #108 fix only covered configure_thread_pool); it's now resolved before the lock everywhere.

Verified intentional (not changed)

  • Per-thread set_cache_limit/clear_cache scoping in pcre/cache.py — flagged by the audit but explicitly asserted by test_cache_limit_thread_local_isolated as the designed semantics.
  • FindIter's PyMutex held across match-object allocation: a reentrancy probe (GC threshold 1, cyclic __del__ calling next() on the same iterator, 685 reentries) confirmed GC cannot fire inside the C iternext call, so no self-deadlock path exists.

Testing

  • CPython 3.14.7 free-threaded (sys._is_gil_enabled() == False) and 3.14.7 GIL, Linux x86-64, PCRE2 10.46.
  • Full pytest suite green on both: 838 passed + 545 subtests (FT), 833 passed + 545 subtests (GIL).
  • New differential regression script for every fix above: utf8_index_to_offset across astral/2-byte subjects at all positions vs re; inline-caseless literal patterns; JIT-vs-interpreter parity on partial UTF bytes ranges (match/search/finditer); NO_UTF_CHECK smuggling now raises PcreErrorBadutfoffset; \U00110000 raises PcreError not SystemError.
  • Multithreaded stress suite from Fix memory and thread-safety issues for free-threaded (GIL=0) Python #108 re-run on both builds: no crashes, errors, or deadlocks.

🤖 Generated with Claude Code

Follow-up to #108. A clean re-audit of the merged tree (fresh-eyes review
of pcre2.c, deep pass over the helper files, and an adversarial review of
the #108 diff itself, which found no regressions) surfaced the following.

C extension:
- util.c utf8_index_to_offset: the 8-byte chunked starter scan could stop
  with the returned offset pointing at the continuation bytes of a
  character whose starter was counted in the previous chunk (chunk
  boundary inside a multi-byte character near the string tail).  str
  subjects always run with PCRE2_NO_UTF_CHECK, so pcre2_match received a
  mid-codepoint start offset / exec length — documented undefined
  behavior.  Observable today: pcre.compile('.').search('𐍈a𐍈b', 3)
  returned span (3, 3) and .group() raised UnicodeDecodeError.  The tail
  scan now always skips trailing continuation bytes.
- Pattern_execute: the interpreter fallback ran on !pattern_jit_get(self),
  a pattern-global flag, instead of whether THIS call's JIT attempt
  produced a result.  Any call that skips JIT locally while the global
  flag stays set would run neither engine and convert the uninitialized
  match_data (rc == 0) into a Match with garbage offsets (crash on
  .group()).  Latent before; reachable once the partial-range JIT skip
  below was added.  Now tracked with a per-call flag.
- First-literal fast path: caselessness introduced by non-leading inline
  groups — (?i:abc), ((?i)abc) — is invisible to pattern_info, so the
  memchr/first-byte prescan filtered on one case only and
  pcre.compile('(?i:abc)').search('ABC') returned None.  The prescan now
  accepts both cases of an ASCII letter (still a pure filter; non-ASCII
  lead bytes are only reported by PCRE2 when shared by all case variants).
- pcre2_jit_match performs none of the UTF validity checks pcre2_match
  does, silently bypassing the module's "leave PCRE2_NO_UTF_CHECK unset so
  PCRE2 validates partial bytes ranges" invariant: a mid-character pos on
  a UTF bytes subject returned None under JIT where the interpreter raises
  PcreErrorBadutfoffset (and executed JIT code on malformed boundaries —
  documented UB).  Partial ranges of UTF bytes subjects now take the
  interpreter path (Pattern_execute, findall, finditer).
- Caller-supplied PCRE2_NO_UTF_CHECK in the options argument is now
  masked out (match/search/fullmatch, findall, finditer).  It let Python
  code trigger the same documented UB with a mid-character pos; the module
  re-adds the flag itself exactly when the range is validated.
- Free-threaded builds now detach the thread state around large PCRE2
  calls (same 256 KiB threshold as GIL builds).  Previously a thread
  inside a long pcre2_match stayed attached and stalled every
  stop-the-world pause (gc.collect() in any thread) for the duration of
  the match.
- Match_expand: the expand_match_template helper was fetched as a
  borrowed dict reference and INCREF'd afterwards — a concurrent rebind
  of the module attribute could free it in between (free-threaded UAF).
  Now fetched as a strong reference via PyObject_GetAttrString.
- Pattern_substitute: key the jit_guard on PCRE2_INFO_JITSIZE as well as
  the acquired jit stack — pcre2_substitute executes JIT-compiled code
  even after the module's jit flag was cleared by a BADOPTION downgrade.
- module_exec: latch jit_support_initialize and pattern_cache_initialize
  on first init.  A re-exec with changed env vars could materialize the
  jit serial lock mid-flight (jit_guard_release then releases a lock that
  jit_guard_acquire never took, permanently breaking JIT serialization)
  or flip the pattern-cache mode while threads hold the global map.
- cache_initialize: stop resetting context_cache_enabled — it silently
  clobbered the PYPCRE_DISABLE_CONTEXT_CACHE env toggle applied by
  module_exec just before (the knob previously had no effect).
- string_helpers.c: PyErr_Format does not support the %.*s dynamic
  precision spec — an out-of-range \U escape raised SystemError instead
  of PcreError.
- atomic_compat.h: the MSVC-fallback _Generic maps listed volatile
  uint32_t* and volatile size_t* — identical types on 32-bit Windows, a
  compile-time constraint violation.  The size_t associations now exist
  only where size_t is a distinct 64-bit type.

Python layer:
- threads.py: the macOS sysctl CPU probe could still run while holding
  the process-wide pool lock via ensure_thread_pool /
  _thread_pool_submission / get_thread_pool_size (the previous fix only
  covered configure_thread_pool); the probe is now resolved before the
  lock in all callers.

Tested on CPython 3.14.7 free-threaded (GIL=0 at runtime) and 3.14.7 GIL
builds: full pytest suite, targeted regressions for every fix above
(differential vs re where applicable), and the multithreaded stress
suite from #108.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@Qubitium
Qubitium merged commit 9c66264 into main Aug 24, 2026
19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant