| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
There was a problem hiding this comment.
did you ment Use PyObject_Call() it seem like this patch remove PyObject_CallObject ... Autocompleter got a bit overzealous ?
Sorry, something went wrong.
There was a problem hiding this comment.
This patch deprecates PyEval_CallObject, not PyObject_CallObject.
Sorry, something went wrong.
There was a problem hiding this comment.
Oh, sorry, I got confused with the unified diff and above comment, the proximity of PyObject_Call() and PyObject_CallObject() made me think you had an Object too much in the comment.
Sorry, something went wrong.
There was a problem hiding this comment.
There are so many similar call APIs.
That's why I think we should deprecate undocumented and less used APIs.
Sorry, something went wrong.
Codecov Report
@@ Coverage Diff @@
## master #75 +/- ##
=========================================
Coverage ? 82.38%
=========================================
Files ? 1428
Lines ? 351138
Branches ? 0
=========================================
Hits ? 289281
Misses ? 61857
Partials ? 0Continue to review full report at Codecov.
|
Sorry, something went wrong.
|
Sorry but I'm confused by the PR title and the commit messages. They don't describe exactly what I see in the code. Can you please update them? |
Sorry, something went wrong.
|
I updated pull request title and body. |
Sorry, something went wrong.
There was a problem hiding this comment.
For the PyEval_*/PyObject_* functions that are now duplicates, it would be useful to have block comments explaining the equivalence, and the fact the redundant definitions are being kept around for backwards compatibility reasons.
Sorry, something went wrong.
|
@Haypo Would you review this again? |
Sorry, something went wrong.
There was a problem hiding this comment.
I don't understand why you change the code in this commit, and 2 commits later reverts this change. You should practice git rebase -i HEAD~4 to merge the two commits maybe?
Sorry, something went wrong.
There was a problem hiding this comment.
Please update the commit message which doesn't make sense compared to the modified code.
This commit fixes a bug, no? It makes sure that a TypeError is raised if kwargs is not a dictionary, right? Please explain it in the commit message.
Sorry, something went wrong.
There was a problem hiding this comment.
I remove this bugfix for now and update #87 instead.
Sorry, something went wrong.
There was a problem hiding this comment.
done. I created split issue for #87.
Sorry, something went wrong.
There was a problem hiding this comment.
maybe use an else here?
Sorry, something went wrong.
There was a problem hiding this comment.
commit message: this change also optimizes PyEval_CallFunction() and PyEval_CallMethod() to avoid temporary tuple when possible.
Sorry, something went wrong.
There was a problem hiding this comment.
Main purpose is reducing maintenance cost rather than optimization.
There are very few callers in the world.
I hope compiler can remove duplicated code too, but I'm not sure.
Sorry, something went wrong.
There was a problem hiding this comment.
I don't understand the "are documented" part, maybe remove it?
I suggest to be more explicit:
" PyEval_CallObjectWithKeywords() is kept for backward compatibility: PyObject_Call(), PyObject_CallFunction() and PyObject_CallMethod() are recommended to call a callable object."
Sorry, something went wrong.
There was a problem hiding this comment.
done
Sorry, something went wrong.
|
I'm not confortable yet with GitHub to review commit messages :-/ It's not easy to comment the commit message: I also suggest to add "bpo-29548" to commit messages (at least to one commit message). |
Sorry, something went wrong.
I agree. I like Gerrit for this.
Final commit message is written when pushing "Squash and merge" button. I hope it works as poor man's Gerrit for a while, until we have new tool to build Misc/NEWS. |
Sorry, something went wrong.
PyEval_Call* APIs are not documented and they doesn't respect PY_SSIZE_T_CLEAN. So add comment block to recommend PyObject_Call* APIs where they are declared. This commit also changes PyEval_CallMethod and PyEval_CallFunction implementation same to PyObject_CallMethod and PyObject_CallFunction to reduce future maintenance cost. Optimization to avoid temporary tuple are copied too. PyEval_CallFunction(callable, "i", (int)i) now calls callable(i) instead of raising TypeError. But accepting this edge case is backward compatible.
|
I squashed all commit and modified commit message. |
Sorry, something went wrong.
Co-authored-by: Jules <julia.poo.poo.poo@gmail.com>
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.
| Back | FazBrowse Home | New Git URL |
PyEval_Call* APIs are not documented and they doesn't respect PY_SSIZE_T_CLEAN.
So add comment block to recommend them where PyEval_Call APIs are declared.
This commit also changes PyEval_CallMethod and PyEval_CallFunction implementation
same to PyObject_CallMethod and PyObject_CallFunction.
PyEval_CallFunction(callable, "i", (int)i) now calls callable(i) instead of raising TypeError.
I expect this allows compiler to share some code between PyEval_CallFunction and PyObject_CallFunction. But I'm not sure.
[bpo-29548]