From 2191a9d0c8582cbebe8e4e270e2bd6ea77a0f3d5 Mon Sep 17 00:00:00 2001 From: dot Date: Mon, 12 Oct 2026 21:13:56 +0900 Subject: [PATCH] Fix 3.15 script validation and exception cleanup contracts Recognize common None loads when checking script returns. Keep the public cache tag version-derived while retaining the private DA16 format boundary. Release the thread-slot borrow before exception decrefs can finalize an interpreter. Assisted-by: OpenAI Codex:model identifier unavailable --- crates/vm/src/version.rs | 5 +- crates/vm/src/vm/crossinterp.rs | 58 ++++++------------- crates/vm/src/vm/thread.rs | 13 +++-- .../snippets/private_315_cache_contract.py | 4 +- 4 files changed, 31 insertions(+), 49 deletions(-) diff --git a/crates/vm/src/version.rs b/crates/vm/src/version.rs index a2c6f42de..7c24b23d8 100644 --- a/crates/vm/src/version.rs +++ b/crates/vm/src/version.rs @@ -69,9 +69,10 @@ pub const RUSTPYTHON_VERSION: &str = const { }; // Private, non-distributable reconstruction ABI. This is not CPython's magic. -// Change token and tag together for every incompatible development format. +// Change the token for every incompatible development format. The public +// cache tag identifies the implementation/version; magic rejects stale ABIs. pub const PYC_MAGIC_NUMBER: u16 = 0xDA16; -pub const PYC_CACHE_TAG: &str = "rustpython-315-devabi2"; +pub const PYC_CACHE_TAG: &str = concat!("rustpython-", env!("MAJOR_CPY"), env!("MINOR_CPY")); // CPython format: magic_number | ('\r' << 16) | ('\n' << 24) // This protects against text-mode file reads diff --git a/crates/vm/src/vm/crossinterp.rs b/crates/vm/src/vm/crossinterp.rs index a72d34ea9..6e215e378 100644 --- a/crates/vm/src/vm/crossinterp.rs +++ b/crates/vm/src/vm/crossinterp.rs @@ -15,7 +15,7 @@ use crate::{ }, bytecode::{ BorrowedConstant, CodeFlags, Constant, Instruction, - oparg::{OpArg, OpArgState}, + oparg::{CommonConstant, OpArg, OpArgState}, }, protocol::PyBuffer, }; @@ -828,47 +828,25 @@ pub(crate) fn code_returns_only_none(code: &Py) -> bool { if !is_pure_function(code) { return false; } - let units = &code.instructions; - let Some(last) = units.len().checked_sub(1) else { - return true; - }; - // The last instruction either returns or raises. We can take advantage - // of that for a quick exit. - let final_op = units.read_op(last).deoptimize(); - - // Look up None in co_consts. - let Some(none_index) = code - .constants - .iter() - .position(|c| matches!(c.borrow_constant(), BorrowedConstant::None)) - else { - // None wasn't there, which means there was no implicit return, - // "return", or "return None". - - // That means there must be an explicit return (non-None), or it only - // raises. - if matches!(final_op, Instruction::ReturnValue) { - // It was an explicit return (non-None). + // Final 3.15 bytecode loads None through LOAD_COMMON_CONSTANT, so it + // need not appear in co_consts. Walk decoded operands rather than looking + // one raw code unit back, which can lose EXTENDED_ARG bits. + let mut previous_loads_none = false; + for (_, op, arg) in walk_instructions(code) { + if matches!(op, Instruction::ExtendedArg) { + continue; + } + if matches!(op, Instruction::ReturnValue) && !previous_loads_none { return false; } - // It must end with a raise then. We still have to walk the bytecode - // to see if there's any explicit return (non-None). - return !walk_instructions(code).any(|(_, op, _)| matches!(op, Instruction::ReturnValue)); - }; - // Walk the bytecode, looking for RETURN_VALUE. - for (at, op, _) in walk_instructions(code) { - if !matches!(op, Instruction::ReturnValue) { - continue; - } - // Ignore it if it returns None. - if let Some(prev) = at.checked_sub(1) - && matches!(units.read_op(prev).deoptimize(), Instruction::LoadConst { .. }) - // We don't worry about EXTENDED_ARG for now. - && usize::from(u8::from(units.read_arg(prev))) == none_index - { - continue; - } - return false; + previous_loads_none = match op { + Instruction::LoadCommonConstant { idx } => idx.get(arg) == CommonConstant::None, + Instruction::LoadConst { consti } => code + .constants + .get(consti.get(arg) as usize) + .is_some_and(|value| matches!(value.borrow_constant(), BorrowedConstant::None)), + _ => false, + }; } true } diff --git a/crates/vm/src/vm/thread.rs b/crates/vm/src/vm/thread.rs index e1bc40c44..3c21fe4f4 100644 --- a/crates/vm/src/vm/thread.rs +++ b/crates/vm/src/vm/thread.rs @@ -1094,13 +1094,14 @@ pub fn get_current_frame() -> *const InterpreterFrame { /// Called from push_exception/pop_exception/set_exception. #[cfg(feature = "threading")] pub fn update_thread_exception(exc: Option) { - CURRENT_THREAD_SLOT.with(|slot| { - if let Some(s) = slot.borrow().as_ref() { - // SAFETY: Called only from the owning thread. The old ref is dropped - // here on the owning thread, which is safe. - let _old = unsafe { s.exception.swap(exc) }; - } + // Release the TLS borrow before dropping either exception: traceback + // locals can finalize another interpreter and re-enter this thread slot. + let slot = current_thread_slot(); + let old = slot.and_then(|s| { + // SAFETY: Called only from the owning thread. + unsafe { s.exception.swap(exc) } }); + drop(old); } /// Collect all threads' current exceptions for sys._current_exceptions(). diff --git a/extra_tests/snippets/private_315_cache_contract.py b/extra_tests/snippets/private_315_cache_contract.py index 334fd9426..558fe124f 100644 --- a/extra_tests/snippets/private_315_cache_contract.py +++ b/extra_tests/snippets/private_315_cache_contract.py @@ -11,7 +11,9 @@ from pathlib import Path if sys.implementation.name == "rustpython": assert external.MAGIC_NUMBER == bytes((0x16, 0xDA, 13, 10)) - assert sys.implementation.cache_tag == "rustpython-315-devabi2" + assert sys.implementation.cache_tag == ( + f"{sys.implementation.name}-{sys.version_info.major}{sys.version_info.minor}" + ) with tempfile.TemporaryDirectory() as directory: root = Path(directory)