Copy link

Copy Markdown

Member

methane requested a review from vstinner February 13, 2017 17:38

Carreau reviewed Feb 14, 2017

View reviewed changes

methane force-pushed the deprecate-evalcall branch from df67bdc to 89c7440 Compare February 14, 2017 14:32

methane force-pushed the deprecate-evalcall branch from 89c7440 to 76737a6 Compare February 14, 2017 14:41

@@            Coverage Diff            @@
##             master      #75   +/-   ##
=========================================
  Coverage          ?   82.38%           
=========================================
  Files             ?     1428           
  Lines             ?   351138           
  Branches          ?        0           
=========================================
  Hits              ?   289281           
  Misses            ?    61857           
  Partials          ?        0

methane force-pushed the deprecate-evalcall branch 2 times, most recently from 04111c1 to 7b1dd6e Compare February 14, 2017 15:13

ncoghlan previously requested changes Feb 19, 2017

View reviewed changes

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

methane dismissed ncoghlan’s stale review February 20, 2017 01:18

vstinner requested changes Feb 28, 2017

View reviewed changes

methane force-pushed the deprecate-evalcall branch 2 times, most recently from 3272dba to e17b495 Compare March 1, 2017 09:26

methane force-pushed the deprecate-evalcall branch from e17b495 to 4a3c0d5 Compare March 1, 2017 09:32

methane deleted the deprecate-evalcall branch March 14, 2017 09:01

Co-authored-by: Jules <[email protected]>

Stay at eol when moving up/down

Prevents accidental shallow copy of raw MemoryIndirect* in value_.indirect,
which would create double-free risk. The copy constructor deliberately skips
value_ (safe); this ensures assignment doesn't silently shallow-copy it.

Per gatekeeper recommendation (20:34) and Pythia assessment python#75.

C++ destroys members in reverse declaration order. With cfg before env,
env destructs first — freeing Registers while cfg still holds
Instructions with FrameState Register* pointers into env memory.

Swap: env now declared before cfg, so env outlives cfg during
destruction. FrameState's borrowed Register* pointers remain valid
throughout cfg teardown.

Found by Pythia assessment python#75. Currently safe by coincidence
(~FrameState only frees the buffer, never dereferences), but any
future dereference during destruction would be use-after-free.

ARM64 pydebug shutdown SEGV root-cause: GlobalCacheKey holds raw
PyDictObject* pointers to globals/builtins. When globals is freed
during Py_FinalizeEx (interpreter_clear → PyDict_Clear), the cache
entry remains in map_ + watch_map_[builtins]. Subsequent
PyDict_EVENT_CLEARED on builtins triggers updateCache, which derefs
cache.key().globals → use-after-free. Pydebug poison fill (0xdd...)
makes the UAF a hard SEGV; x86_64 release reads stale-but-readable
type pointer and silently passes the JIT_CHECK.

Two surgical guards address the FINALIZE-SPECIFIC manifestation:

  Part A — Python/jit_support/phoenix_init.cpp
    phoenix_dict_watcher early-returns when Py_IsFinalizing(). Cache
    state is about to be destroyed in phoenix_free; processing events
    on tearing-down dicts is moot. Pattern matches pyjit.cpp:1642
    funcDestroyed precedent.

  Part B — Python/jit/global_cache.cpp
    GlobalCacheManager::clear() short-circuits when Py_IsFinalizing().
    Avoids two failure modes:
      1. Ci_Watchers_UnwatchDict (PyDict_Unwatch) reads dict internals
         on freed memory → UAF under pydebug
      2. ci_watcher_state_fini ran first in phoenix_free → watcher_id
         is -1 → JIT_CHECK 'Invalid dict watcher ID -1' abort
         (observed in earlier gate logs, regression historically)

SCOPE HONESTY: this is FINALIZE-SPECIFIC protection, NOT full
root-cause for the raw-pointer-aliasing class. Mid-execution module
unload (refcount→0 on globals while cache holds raw pointer) remains
unaddressed. Filed as W27 GlobalCacheKey raw-pointer lifecycle
re-architecture (docs/w27-globalcachekey-lifecycle.md, Tier 6+ scope).
The 314d0f2 'weak reference semantics' comment was FALSIFIED — no
mechanism actually provided weak-reference semantics; comments updated
to name the actual mechanisms (Py_IsFinalizing guard, finalize-only
protection).

Diagnostic evidence (per Debug-First protocol):
  lldb backtrace on ARM64 pydebug push 59 falsifier:
    PyType_HasFeature(type=0xdddddddddddddddd) object.h:966  <-- SEGV
    hasOnlyUnicodeKeys()                       dict.h:52
    GlobalCacheManager::updateCache()          global_cache.cpp:325
    GlobalCacheManager::notifyDictClear()      global_cache.cpp:144
    phoenix_dict_watcher()                     phoenix_init.cpp:58
    PyDict_Clear → interpreter_clear → Py_FinalizeEx

Test plan post-commit:
  testkeeper ARM64 pydebug --clean rebuild + falsifier rerun
  (test_phoenix_jit_inline_except_closure under timeout+redirect)
  + push 58 baseline regression check (8b7b935 still passes).

testkeeper x86_64 compile-only PASS verified pre-commit at
binary timestamp 1776874355 (chat L1872 build verification).

Authorization chain: supervisor option (i) at chat L1862, theologian
LEVEL-1 ratification at chat L1861. Pythia python#75 python#1 (trigger isolation)
gates downstream emitCallExceptionHandler queue, NOT this fix.

Per pythia python#76 [chat 16:45Z] + supervisor revision [chat 16:45Z python#3]:
filed-not-owned workstreams accrue weed-debt ('a field marked for
fallow grows weeds named next year'). W27 was filed at bf8982b
without ownership, falsification test, or revisit-trigger.

Added §1 frontmatter:
  - Owner: theologian (design), generalist (implementation)
  - Schedule: entry-trigger = Tier 5 ZERO C++ complete + no active
    push-class blocker; exit by start of cinderx Tier 6 cleanup
  - Falsification test reference: §8

Added §8 falsification test sketch:
  Lib/test/test_phoenix_w27_module_unload_uaf.py
  - SKIPPED until W27 work begins
  - Reproducer: register cache entry → del sys.modules → gc.collect →
    fire builtins event → expect SEGV pre-W27 / clean post-W27
  - Acceptance: reproduces SEGV pre-W27 + passes post-W27, OR W27
    scope re-opens if reproducer is harder to trigger than predicted
  - Discipline: 'falsification-first' — test lands before fix code

Added §7 cross-references:
  - Push 58 ARM64-pydebug coredump finding from testkeeper [chat
    16:45Z]: 8b7b935 baseline ALSO produces shutdown coredump
    on the same falsifier workload (just non-deterministic vs push
    59's deterministic SEGV) — independent confirmation that this
    is pre-existing latent (pythia python#75 b1 outcome) not push-59-port-
    shape-specific
  - W27 ownership update authority chain

This commit IS supervisor's pythia python#76 python#3 corrective action — turning
'filed' into 'owned, scheduled, testable'. Pattern reusable: same
audit will be applied to W23 (verifier hardening), W25 (typed bridges),
W26 (build-state hardening) per supervisor [chat 16:45Z python#3] deferral.