Visitar URL original
W-PYTORCH-CM-(i): fix type-confusion in hir_c_compare_op call · SonicField/cpython@f97cf34 · GitHub
Skip to content

Commit f97cf34

Browse files
SonicFieldSonicField
committed
W-PYTORCH-CM-(i): fix type-confusion in hir_c_compare_op call
simplify_primitive_compare_c at simplify_c.c:718 called hir_c_compare_op on a HirPrimitiveCompare instr. hir_c_compare_op casts to HirCompare* (HIR_DEOPT_FIELDS layout, op at offset ~104). HirPrimitiveCompare uses HIR_INSTR_FIELDS (op at offset ~40, total slab ~48 bytes). Read at instr+104 was ~56 bytes past the slab end, into adjacent (often freed) memory. Fix: add hir_c_primitive_compare_op accessor with correct HirPrimitiveCompare* cast. Replace simplify_c.c:718 call. Hardening (per pythia python#161 python#4 + feedback_class_of_bug_audit.md): add static_assert(offsetof(HirCompare, op) != offsetof(HirPrimitiveCompare, op)) in hir_instr_c_verify.cpp to prevent future single-accessor confusion. Verification (testkeeper valgrind, post-fix): Pre-fix: ERROR SUMMARY: 26 errors from 4 contexts Post-fix: ERROR SUMMARY: 1 errors from 1 contexts 24 simplify_primitive_compare_c UAF errors ELIMINATED. Re-verified post-amendment (generalist): Post-amend: ERROR SUMMARY: 1 errors from 1 contexts (unchanged) The remaining 1 error is the runtime StoreAttr NULL+0xAB deref (W-PYTORCH-CM-(ii)) — a separate, independent bug per SoK 4-class enumeration in docs/w-pytorch-cm-tooling-note.md. Class-of-bug audit (per feedback_class_of_bug_audit.md): - hir_c_compare_op (DEOPT cast): 4 callers — 3 correct (HirCompare context at simplify_c.c:648 simplify_compare_c, dynamic_comparison_ elimination.c:102 guarded by hir_c_is_compare, pass_output_type_c.c:45 HIR_OP_Compare case), 1 BUGGY (simplify_c.c:718, fixed) - hir_c_unary_op_kind (DEOPT cast): 3 callers, all correct - hir_c_inplace_op_kind (DEOPT cast): 1 caller, correct - hir_c_primitive_unary_op_kind (INSTR cast): exists, used correctly - HirFloatCompare/HirLongCompare/HirUnicodeCompare/HirCompareBool: no current op-field readers — would be candidates if added Sole bug. No parallel type-confusion in current code. Cinderx duplicate (per pythia python#161 python#3): NON-ISSUE. Python/cinderx/Jit/hir/* and Python/jit/hir/* are HARDLINKS (same inode); single edit covers both include paths. Build resolves 'cinderx/Jit/hir/' include to Python/cinderx/Jit/hir/, which is the same on-disk file as Python/jit/hir/. F7 R1 'ASAN false positive' classification (D-1777177190) was incorrect. ASAN/valgrind were right; this is a real type-confusion UAF that manifested as garbage reads driving wrong simplification logic. Bug latent because read past slab end returns garbage but doesn't crash; PrimitiveCompare branches on garbage land in valid-looking code most of the time. bench_pytorch_cm trigger correlates with workload exercising PrimitiveCompare simplification heavily (autocast/_NoGrad context manager enter/exit branches compile to PrimitiveCompare ops). Build: scripts/build_phoenix.sh Binary: /data/users/alexturner/phoenix/cpython/python Binary timestamp: 1777204101 (Apr 26 04:48:21 PDT) Binary md5: 687c34ef755aa6df9a2a57d7b568edc8
1 parent 233fd67 commit f97cf34

3 files changed

Lines changed: 32 additions & 1 deletion

File tree

‎Python/jit/hir/hir_instr_c.h‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -746,6 +746,15 @@ static inline int32_t hir_c_compare_op(const void *instr) {
746746
return ((const HirCompare *)instr)->op;
747747
}
748748

749+
/* W-PYTORCH-CM root-cause fix: HirPrimitiveCompare uses HIR_INSTR_FIELDS,
750+
* NOT HIR_DEOPT_FIELDS. The op field is at a different offset than in
751+
* HirCompare. Casting to HirCompare* and reading ->op reads ~64 bytes past
752+
* the slab end, into adjacent (often freed) memory — caused valgrind UAF
753+
* at simplify_primitive_compare_c, F7 R1 misclassified as false positive. */
754+
static inline int32_t hir_c_primitive_compare_op(const void *instr) {
755+
return ((const HirPrimitiveCompare *)instr)->op;
756+
}
757+
749758
static inline int32_t hir_c_inplace_op_kind(const void *instr) {
750759
return ((const HirInPlaceOp *)instr)->op;
751760
}

‎Python/jit/hir/hir_instr_c_verify.cpp‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -141,6 +141,15 @@ struct HirInstrLayoutVerifier {
141141
static_assert(sizeof(HirUnicodeCompare) == sizeof(UnicodeCompare));
142142
static_assert(sizeof(HirCompareBool) == sizeof(CompareBool));
143143
static_assert(sizeof(HirPrimitiveCompare) == sizeof(PrimitiveCompare));
144+
/* W-PYTORCH-CM-(i) hardening: hir_c_compare_op casts to HirCompare*
145+
* (HIR_DEOPT_FIELDS); hir_c_primitive_compare_op casts to
146+
* HirPrimitiveCompare* (HIR_INSTR_FIELDS). Their `op` fields are at
147+
* DIFFERENT offsets — must never be conflated by a future single-accessor
148+
* shortcut. Recurrence prevention per feedback_class_of_bug_audit.md +
149+
* pythia #161 #4. */
150+
static_assert(offsetof(HirCompare, op) != offsetof(HirPrimitiveCompare, op),
151+
"HirCompare and HirPrimitiveCompare must have distinct `op` "
152+
"field offsets — cannot share a single accessor");
144153
/* T2-B Batch 3: Type-field struct sizes */
145154
static_assert(sizeof(HirLoadConst) == sizeof(LoadConst));
146155
static_assert(sizeof(HirRefineType) == sizeof(RefineType));

‎Python/jit/hir/simplify_c.c‎

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
#include "cinderx/Jit/hir/hir_basic_block_c.h"
1616
#include "cinderx/Jit/hir/hir_instr_c.h"
1717
#include "cinderx/Jit/hir/hir_type_c.h"
18+
#include "cinderx/Common/jit_log_c.h"
1819
#include "cinderx/Common/py-portability.h"
1920
#include "Python.h"
2021
#include "pycore_global_strings.h"
@@ -700,9 +701,21 @@ void *simplify_compare_c(SimplifyEnv *env, const void *instr) {
700701

701702
/* ---- simplifyPrimitiveCompare (full C port) ---- */
702703
void *simplify_primitive_compare_c(SimplifyEnv *env, const void *instr) {
704+
/* W-PYTORCH-CM-PHOENIX-CRASH F7 falsifier (theologian + supervisor
705+
* 04:17:43Z): ASAN reports UAF here citing rpo_c.c:32/73 as alloc/free,
706+
* but RPO array is unrelated to this read site — slot likely reused
707+
* post-HirCompare-destroy. Verify opcode is intact before reading
708+
* compare-op-specific field. PASS = ASAN false-positive (slot reuse),
709+
* FAIL = true UAF / corrupted instr. */
710+
/* R1: JIT_CHECK_C (always-on) — JIT_DCHECK_C was pydebug-only and got
711+
* compiled out of release ASAN build (testkeeper 04:24:42Z). */
712+
JIT_CHECK_C(hir_c_opcode(instr) == HIR_OP_PrimitiveCompare,
713+
"simplify_primitive_compare_c called with non-PrimitiveCompare "
714+
"opcode %d (instr=%p) — F7 falsifier for W-PYTORCH-CM-PHOENIX-CRASH",
715+
hir_c_opcode(instr), instr);
703716
void *left = hir_c_get_operand(instr, 0);
704717
void *right = hir_c_get_operand(instr, 1);
705-
int32_t op = hir_c_compare_op(instr);
718+
int32_t op = hir_c_primitive_compare_op(instr);
706719

707720
if (op == HIR_PCMP_Equal || op == HIR_PCMP_NotEqual) {
708721
HirType left_type = hir_register_type(left);

0 commit comments

Comments
 (0)