Skip to content

Commit 8139b4e

Browse files
Pablo GarciaPablo Garcia
authored andcommitted
Fix panic in code.replace() with non-interned strings
code.replace() called as_interned_str().unwrap() on its string arguments, which panics when the caller passes a string that has not been interned. modulefinder's replace_paths_in_code() builds a fresh co_filename, so any use of ModuleFinder(replace_paths=...) aborted the interpreter. Intern the incoming strings instead, and raise TypeError rather than panicking when a non-string appears in co_names/co_varnames/ co_cellvars/co_freevars. Unskips test_modulefinder.test_replace_paths.
1 parent 1819677 commit 8139b4e

2 files changed

Lines changed: 19 additions & 21 deletions

File tree

Lib/test/test_modulefinder.py

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -390,8 +390,6 @@ def test_bytecode(self):
390390
os.remove(source_path)
391391
self._do_test(bytecode_test)
392392

393-
# TODO: RUSTPYTHON; panics at code.rs with 'called Option::unwrap() on a None value'
394-
@unittest.skip("TODO: RUSTPYTHON; panics in co_filename replacement")
395393
def test_replace_paths(self):
396394
old_path = os.path.join(self.test_dir, 'a', 'module.py')
397395
new_path = os.path.join(self.test_dir, 'a', 'spam.py')

crates/vm/src/builtins/code.rs

Lines changed: 19 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1366,19 +1366,25 @@ impl PyCode {
13661366
OptionalArg::Missing => self.code.instructions.clone(),
13671367
};
13681368

1369+
let intern_all = |objs: Vec<PyObjectRef>, field: &str| -> PyResult<Box<[_]>> {
1370+
objs.into_iter()
1371+
.map(|o| {
1372+
let s = o.downcast_ref::<super::pystr::PyStr>().ok_or_else(|| {
1373+
vm.new_type_error(format!("{field} must be a tuple of strings"))
1374+
})?;
1375+
Ok(vm.ctx.intern_str(s.as_wtf8()))
1376+
})
1377+
.collect::<PyResult<Vec<_>>>()
1378+
.map(Vec::into_boxed_slice)
1379+
};
1380+
13691381
let cellvars = match co_cellvars {
1370-
OptionalArg::Present(cellvars) => cellvars
1371-
.into_iter()
1372-
.map(|o| o.as_interned_str(vm).unwrap())
1373-
.collect(),
1382+
OptionalArg::Present(cellvars) => intern_all(cellvars, "co_cellvars")?,
13741383
OptionalArg::Missing => self.code.cellvars.clone(),
13751384
};
13761385

13771386
let freevars = match co_freevars {
1378-
OptionalArg::Present(freevars) => freevars
1379-
.into_iter()
1380-
.map(|o| o.as_interned_str(vm).unwrap())
1381-
.collect(),
1387+
OptionalArg::Present(freevars) => intern_all(freevars, "co_freevars")?,
13821388
OptionalArg::Missing => self.code.freevars.clone(),
13831389
};
13841390

@@ -1411,25 +1417,19 @@ impl PyCode {
14111417
posonlyarg_count,
14121418
arg_count,
14131419
kwonlyarg_count,
1414-
source_path: source_path.as_object().as_interned_str(vm).unwrap(),
1420+
source_path: vm.ctx.intern_str(source_path.as_wtf8()),
14151421
first_line_number,
1416-
obj_name: obj_name.as_object().as_interned_str(vm).unwrap(),
1417-
qualname: qualname.as_object().as_interned_str(vm).unwrap(),
1422+
obj_name: vm.ctx.intern_str(obj_name.as_wtf8()),
1423+
qualname: vm.ctx.intern_str(qualname.as_wtf8()),
14181424

14191425
max_stackdepth,
14201426
instructions,
14211427
// FIXME: invalid locations. Actually locations is a duplication of linetable.
14221428
// It can be removed once we move every other code to use linetable only.
14231429
locations: self.code.locations.clone(),
14241430
constants: constants.into_iter().map(Literal).collect(),
1425-
names: names
1426-
.into_iter()
1427-
.map(|o| o.as_interned_str(vm).unwrap())
1428-
.collect(),
1429-
varnames: varnames
1430-
.into_iter()
1431-
.map(|o| o.as_interned_str(vm).unwrap())
1432-
.collect(),
1431+
names: intern_all(names, "co_names")?,
1432+
varnames: intern_all(varnames, "co_varnames")?,
14331433
cellvars,
14341434
freevars,
14351435
localspluskinds: self.code.localspluskinds.clone(),

0 commit comments

Comments
 (0)