Skip to content

Commit 5103e88

Browse files
committed
Replace unsafe pointer cast with PyMutex in PyCode
Add `source_path: PyMutex<&'static PyStrInterned>` field to `PyCode` for interior mutability, replacing the UB-inducing `#[allow(invalid_reference_casting)]` + `write_volatile` pattern in `update_code_filenames`. Update all read sites across the codebase to use the new `source_path()` accessor method.
1 parent d1c19be commit 5103e88

File tree

10 files changed

+33
-26
lines changed

10 files changed

+33
-26
lines changed

crates/stdlib/src/faulthandler.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -252,7 +252,7 @@ mod decl {
252252
/// Dump a single frame's info to fd (signal-safe), reading live data.
253253
#[cfg(any(unix, windows))]
254254
fn dump_frame_from_raw(fd: i32, frame: &Frame) {
255-
let filename = frame.code.source_path.as_str();
255+
let filename = frame.code.source_path().as_str();
256256
let funcname = frame.code.obj_name.as_str();
257257
let lasti = frame.lasti();
258258
let lineno = if lasti == 0 {
@@ -328,7 +328,7 @@ mod decl {
328328
#[cfg(any(unix, windows))]
329329
fn dump_frame_from_ref(fd: i32, frame: &crate::vm::PyRef<Frame>) {
330330
let funcname = frame.code.obj_name.as_str();
331-
let filename = frame.code.source_path.as_str();
331+
let filename = frame.code.source_path().as_str();
332332
let lineno = if frame.lasti() == 0 {
333333
frame.code.first_line_number.map(|n| n.get()).unwrap_or(1) as u32
334334
} else {

crates/vm/src/builtins/code.rs

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ use alloc::fmt;
1515
use core::{borrow::Borrow, ops::Deref};
1616
use malachite_bigint::BigInt;
1717
use num_traits::Zero;
18+
use rustpython_common::lock::PyMutex;
1819
use rustpython_compiler_core::{OneIndexed, bytecode::CodeUnits, bytecode::PyCodeLocationInfoKind};
1920

2021
/// State for iterating through code address ranges
@@ -323,6 +324,7 @@ impl<B: AsRef<[u8]>> IntoCodeObject for frozen::FrozenCodeObject<B> {
323324
#[pyclass(module = false, name = "code")]
324325
pub struct PyCode {
325326
pub code: CodeObject,
327+
source_path: PyMutex<&'static PyStrInterned>,
326328
}
327329

328330
impl Deref for PyCode {
@@ -333,8 +335,20 @@ impl Deref for PyCode {
333335
}
334336

335337
impl PyCode {
336-
pub const fn new(code: CodeObject) -> Self {
337-
Self { code }
338+
pub fn new(code: CodeObject) -> Self {
339+
let sp = code.source_path;
340+
Self {
341+
code,
342+
source_path: PyMutex::new(sp),
343+
}
344+
}
345+
346+
pub fn source_path(&self) -> &'static PyStrInterned {
347+
*self.source_path.lock()
348+
}
349+
350+
pub fn set_source_path(&self, new: &'static PyStrInterned) {
351+
*self.source_path.lock() = new;
338352
}
339353
pub fn from_pyc_path(path: &std::path::Path, vm: &VirtualMachine) -> PyResult<PyRef<Self>> {
340354
let name = match path.file_stem() {
@@ -397,7 +411,7 @@ impl Representable for PyCode {
397411
"<code object {} at {:#x} file {:?}, line {}>",
398412
code.obj_name,
399413
zelf.get_id(),
400-
code.source_path.as_str(),
414+
zelf.source_path().as_str(),
401415
code.first_line_number.map_or(-1, |n| n.get() as i32)
402416
))
403417
}
@@ -572,7 +586,7 @@ impl PyCode {
572586

573587
#[pygetset]
574588
pub fn co_filename(&self) -> PyStrRef {
575-
self.code.source_path.to_owned()
589+
self.source_path().to_owned()
576590
}
577591

578592
#[pygetset]
@@ -906,7 +920,7 @@ impl PyCode {
906920

907921
let source_path = match co_filename {
908922
OptionalArg::Present(source_path) => source_path,
909-
OptionalArg::Missing => self.code.source_path.to_owned(),
923+
OptionalArg::Missing => self.source_path().to_owned(),
910924
};
911925

912926
let first_line_number = match co_firstlineno {

crates/vm/src/builtins/traceback.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -131,7 +131,7 @@ impl serde::Serialize for PyTraceback {
131131
let mut struc = s.serialize_struct("PyTraceback", 3)?;
132132
struc.serialize_field("name", self.frame.code.obj_name.as_str())?;
133133
struc.serialize_field("lineno", &self.lineno.get())?;
134-
struc.serialize_field("filename", self.frame.code.source_path.as_str())?;
134+
struc.serialize_field("filename", self.frame.code.source_path().as_str())?;
135135
struc.end()
136136
}
137137
}

crates/vm/src/exceptions.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -383,7 +383,7 @@ fn write_traceback_entry<W: Write>(
383383
output: &mut W,
384384
tb_entry: &Py<PyTraceback>,
385385
) -> Result<(), W::Error> {
386-
let filename = tb_entry.frame.code.source_path.as_str();
386+
let filename = tb_entry.frame.code.source_path().as_str();
387387
writeln!(
388388
output,
389389
r##" File "{}", line {}, in {}"##,

crates/vm/src/frame.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3223,7 +3223,7 @@ impl ExecutingFrame<'_> {
32233223
self.lasti(),
32243224
self.code.obj_name,
32253225
op_name,
3226-
self.code.source_path
3226+
self.code.source_path()
32273227
);
32283228
}
32293229
self.state.stack.drain(stack_len - count..).map(|obj| {

crates/vm/src/import.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -168,7 +168,7 @@ pub fn import_code_obj(
168168
if set_file_attr {
169169
attrs.set_item(
170170
identifier!(vm, __file__),
171-
code_obj.source_path.to_object(),
171+
code_obj.source_path().to_object(),
172172
vm,
173173
)?;
174174
}
@@ -195,7 +195,7 @@ fn remove_importlib_frames_inner(
195195
return (None, false);
196196
};
197197

198-
let file_name = traceback.frame.code.source_path.as_str();
198+
let file_name = traceback.frame.code.source_path().as_str();
199199

200200
let (inner_tb, mut now_in_importlib) =
201201
remove_importlib_frames_inner(vm, traceback.next.lock().clone(), always_trim);

crates/vm/src/stdlib/imp.rs

Lines changed: 4 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -229,7 +229,7 @@ mod _imp {
229229

230230
#[pyfunction]
231231
fn _fix_co_filename(code: PyRef<PyCode>, path: PyStrRef, vm: &VirtualMachine) {
232-
let old_name = code.code.source_path;
232+
let old_name = code.source_path();
233233
let new_name = vm.ctx.intern_str(path.as_str());
234234
super::update_code_filenames(&code, old_name, new_name);
235235
}
@@ -288,18 +288,11 @@ fn update_code_filenames(
288288
old_name: &'static PyStrInterned,
289289
new_name: &'static PyStrInterned,
290290
) {
291-
if !core::ptr::eq(code.code.source_path, old_name)
292-
&& code.code.source_path.as_str() != old_name.as_str()
293-
{
291+
let current = code.source_path();
292+
if !core::ptr::eq(current, old_name) && current.as_str() != old_name.as_str() {
294293
return;
295294
}
296-
// SAFETY: called during import before the code object is shared.
297-
// Mutates co_filename in place.
298-
#[allow(invalid_reference_casting)]
299-
unsafe {
300-
let source_path_ptr = &code.code.source_path as *const _ as *mut &'static PyStrInterned;
301-
core::ptr::write_volatile(source_path_ptr, new_name);
302-
}
295+
code.set_source_path(new_name);
303296
for constant in code.code.constants.iter() {
304297
let obj: &crate::PyObject = constant.borrow();
305298
if let Some(inner_code) = obj.downcast_ref::<PyCode>() {

crates/vm/src/stdlib/io.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4971,7 +4971,7 @@ mod _io {
49714971
&& let Some(frame) = vm.current_frame()
49724972
&& let Some(stdlib_dir) = vm.state.config.paths.stdlib_dir.as_deref()
49734973
{
4974-
let path = frame.code.source_path.as_str();
4974+
let path = frame.code.source_path().as_str();
49754975
if !path.starts_with(stdlib_dir) {
49764976
stacklevel = stacklevel.saturating_sub(1);
49774977
}

crates/vm/src/vm/context.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -658,7 +658,7 @@ impl Context {
658658

659659
pub fn new_code(&self, code: impl code::IntoCodeObject) -> PyRef<PyCode> {
660660
let code = code.into_code_object(self);
661-
PyRef::new_ref(PyCode { code }, self.types.code_type.to_owned(), None)
661+
PyRef::new_ref(PyCode::new(code), self.types.code_type.to_owned(), None)
662662
}
663663
}
664664

crates/vm/src/warn.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -578,7 +578,7 @@ fn setup_context(
578578
}
579579

580580
let (globals, filename, lineno) = if let Some(f) = f {
581-
(f.globals.clone(), f.code.source_path, f.f_lineno())
581+
(f.globals.clone(), f.code.source_path(), f.f_lineno())
582582
} else if let Some(frame) = vm.current_frame() {
583583
// We have a frame but it wasn't found during stack walking
584584
(frame.globals.clone(), vm.ctx.intern_str("<sys>"), 1)

0 commit comments

Comments
 (0)