Skip to content
Merged

marshal #7467

Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 1 addition & 19 deletions Lib/test/test_marshal.py
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,6 @@ def test_ints(self):
self.helper(expected)
n = n >> 1

@unittest.expectedFailure # TODO: RUSTPYTHON
def test_int64(self):
# Simulate int marshaling with TYPE_INT64.
maxint64 = (1 << 63) - 1
Expand Down Expand Up @@ -141,7 +140,6 @@ def test_different_filenames(self):
self.assertEqual(co1.co_filename, "f1")
self.assertEqual(co2.co_filename, "f2")

@unittest.expectedFailure # TODO: RUSTPYTHON; TypeError: Unexpected keyword argument allow_code
def test_no_allow_code(self):
data = {'a': [({0},)]}
dump = marshal.dumps(data, allow_code=False)
Expand Down Expand Up @@ -234,14 +232,12 @@ def test_bytearray(self):
new = marshal.loads(marshal.dumps(b))
self.assertEqual(type(new), bytes)

@unittest.expectedFailure # TODO: RUSTPYTHON
def test_memoryview(self):
b = memoryview(b"abc")
self.helper(b)
new = marshal.loads(marshal.dumps(b))
self.assertEqual(type(new), bytes)

@unittest.expectedFailure # TODO: RUSTPYTHON
def test_array(self):
a = array.array('B', b"abc")
new = marshal.loads(marshal.dumps(a))
Expand Down Expand Up @@ -274,7 +270,6 @@ def test_fuzz(self):
except Exception:
pass

@unittest.expectedFailure # TODO: RUSTPYTHON
def test_loads_recursion(self):
def run_tests(N, check):
# (((...None...),),)
Expand All @@ -295,7 +290,7 @@ def check(s):
run_tests(2**20, check)

@unittest.skipIf(support.is_android, "TODO: RUSTPYTHON; segfault")
@unittest.expectedFailure # TODO: RUSTPYTHON; segfault
@unittest.skipIf(os.name == 'nt', "TODO: RUSTPYTHON; write depth limit is 2000 not 1000")
def test_recursion_limit(self):
# Create a deeply nested structure.
head = last = []
Expand Down Expand Up @@ -324,7 +319,6 @@ def test_recursion_limit(self):
last.append([0])
self.assertRaises(ValueError, marshal.dumps, head)

@unittest.expectedFailure # TODO: RUSTPYTHON
def test_exact_type_match(self):
# Former bug:
# >>> class Int(int): pass
Expand All @@ -348,7 +342,6 @@ def test_invalid_longs(self):
invalid_string = b'l\x02\x00\x00\x00\x00\x00\x00\x00'
self.assertRaises(ValueError, marshal.loads, invalid_string)

@unittest.expectedFailure # TODO: RUSTPYTHON
def test_multiple_dumps_and_loads(self):
# Issue 12291: marshal.load() should be callable multiple times
# with interleaved data written by non-marshal code
Expand Down Expand Up @@ -532,66 +525,56 @@ def helper3(self, rsample, recursive=False, simple=False):
else:
self.assertGreaterEqual(len(s2), len(s3))

@unittest.expectedFailure # TODO: RUSTPYTHON
def testInt(self):
intobj = 123321
self.helper(intobj)
self.helper3(intobj, simple=True)

@unittest.expectedFailure # TODO: RUSTPYTHON
def testFloat(self):
floatobj = 1.2345
self.helper(floatobj)
self.helper3(floatobj)

@unittest.expectedFailure # TODO: RUSTPYTHON
def testStr(self):
strobj = "abcde"*3
self.helper(strobj)
self.helper3(strobj)

@unittest.expectedFailure # TODO: RUSTPYTHON
def testBytes(self):
bytesobj = b"abcde"*3
self.helper(bytesobj)
self.helper3(bytesobj)

@unittest.expectedFailure # TODO: RUSTPYTHON
def testList(self):
for obj in self.keys:
listobj = [obj, obj]
self.helper(listobj)
self.helper3(listobj)

@unittest.expectedFailure # TODO: RUSTPYTHON
def testTuple(self):
for obj in self.keys:
tupleobj = (obj, obj)
self.helper(tupleobj)
self.helper3(tupleobj)

@unittest.expectedFailure # TODO: RUSTPYTHON
def testSet(self):
for obj in self.keys:
setobj = {(obj, 1), (obj, 2)}
self.helper(setobj)
self.helper3(setobj)

@unittest.expectedFailure # TODO: RUSTPYTHON
def testFrozenSet(self):
for obj in self.keys:
frozensetobj = frozenset({(obj, 1), (obj, 2)})
self.helper(frozensetobj)
self.helper3(frozensetobj)

@unittest.expectedFailure # TODO: RUSTPYTHON
def testDict(self):
for obj in self.keys:
dictobj = {"hello": obj, "goodbye": obj, obj: "hello"}
self.helper(dictobj)
self.helper3(dictobj)

@unittest.expectedFailure # TODO: RUSTPYTHON
def testModule(self):
with open(__file__, "rb") as f:
code = f.read()
Expand Down Expand Up @@ -651,7 +634,6 @@ def testNoIntern(self):
self.assertNotEqual(id(s2), id(s))

class SliceTestCase(unittest.TestCase, HelperMixin):
@unittest.expectedFailure # TODO: RUSTPYTHON; NotImplementedError: TODO: not implemented yet or marshal unsupported type
def test_slice(self):
for obj in (
slice(None), slice(1), slice(1, 2), slice(1, 2, 3),
Expand Down
18 changes: 13 additions & 5 deletions crates/compiler-core/src/bytecode.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@

use crate::{
marshal::MarshalError,
varint::{read_varint, read_varint_with_start, write_varint, write_varint_with_start},
varint::{read_varint, read_varint_with_start, write_varint_be, write_varint_with_start},
{OneIndexed, SourceLocation},
};
use alloc::{borrow::ToOwned, boxed::Box, collections::BTreeSet, fmt, string::String, vec::Vec};
Expand Down Expand Up @@ -71,9 +71,9 @@ pub fn encode_exception_table(entries: &[ExceptionTableEntry]) -> alloc::boxed::
let depth_lasti = ((entry.depth as u32) << 1) | (entry.push_lasti as u32);

write_varint_with_start(&mut data, entry.start);
write_varint(&mut data, size);
write_varint(&mut data, entry.target);
write_varint(&mut data, depth_lasti);
write_varint_be(&mut data, size);
write_varint_be(&mut data, entry.target);
write_varint_be(&mut data, depth_lasti);
}
data.into_boxed_slice()
}
Expand Down Expand Up @@ -204,7 +204,7 @@ impl PyCodeLocationInfoKind {
}
}

pub trait Constant: Sized {
pub trait Constant: Sized + Clone {
type Name: AsRef<str>;

/// Transforms the given Constant to a BorrowedConstant
Expand Down Expand Up @@ -567,6 +567,14 @@ impl Deref for CodeUnits {
}

impl CodeUnits {
/// Disable adaptive specialization by setting all counters to unreachable.
/// Used for CPython-compiled bytecode where specialization may not be safe.
pub fn disable_specialization(&self) {
for counter in self.adaptive_counters.iter() {
counter.store(UNREACHABLE_BACKOFF, Ordering::Relaxed);
}
}
Comment on lines +570 to +576
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

disable_specialization() is undone by the next quicken().

This only writes UNREACHABLE_BACKOFF into adaptive_counters, but CodeUnits::quicken() later reinitializes every cacheable site unconditionally. A code object marked non-specializable before first execution will still get warmup counters at RESUME.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@crates/compiler-core/src/bytecode.rs` around lines 570 - 576, The current
disable_specialization() only writes UNREACHABLE_BACKOFF into adaptive_counters
but CodeUnits::quicken() later unconditionally reinitializes cacheable sites,
undoing the disable; update the implementation so that quicken() respects the
non-specializable state: either add a persistent flag on the code object (e.g.,
non_specializable) set by disable_specialization() and check that flag in
CodeUnits::quicken() before initializing warmup counters at RESUME, or have
quicken() inspect each adaptive counter and skip reinitialization when the
counter already equals UNREACHABLE_BACKOFF; ensure you reference
disable_specialization, adaptive_counters, UNREACHABLE_BACKOFF, and
CodeUnits::quicken() in your changes so the disable persists across quicken().


/// Replace the opcode at `index` in-place without changing the arg byte.
/// Uses atomic Release store to ensure prior cache writes are visible
/// to threads that subsequently read the new opcode with Acquire.
Expand Down
73 changes: 60 additions & 13 deletions crates/compiler-core/src/bytecode/oparg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -382,6 +382,10 @@ oparg_enum!(
);

bitflagset::bitflag! {
/// `SET_FUNCTION_ATTRIBUTE` flags.
/// Bitmask: Defaults=0x01, KwOnly=0x02, Annotations=0x04,
/// Closure=0x08, TypeParams=0x10, Annotate=0x20.
/// Stored as bit position (0-5) by `bitflag!` macro.
#[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)]
#[repr(u8)]
pub enum MakeFunctionFlag {
Expand Down Expand Up @@ -426,20 +430,63 @@ impl From<MakeFunctionFlag> for u32 {

impl OpArgType for MakeFunctionFlag {}

oparg_enum!(
/// The possible comparison operators.
#[derive(Debug, Copy, Clone, PartialEq, Eq)]
pub enum ComparisonOperator {
// be intentional with bits so that we can do eval_ord with just a bitwise and
// bits: | Equal | Greater | Less |
Less = 0b001,
Greater = 0b010,
NotEqual = 0b011,
Equal = 0b100,
LessOrEqual = 0b101,
GreaterOrEqual = 0b110,
/// `COMPARE_OP` arg is `(cmp_index << 5) | mask`. Only the upper
/// 3 bits identify the comparison; the lower 5 bits are an inline
/// cache mask for adaptive specialization.
#[derive(Debug, Copy, Clone, PartialEq, Eq)]
pub enum ComparisonOperator {
Less,
LessOrEqual,
Equal,
NotEqual,
Greater,
GreaterOrEqual,
}

impl TryFrom<u8> for ComparisonOperator {
type Error = MarshalError;
fn try_from(value: u8) -> Result<Self, Self::Error> {
Self::try_from(value as u32)
}
);
}

impl TryFrom<u32> for ComparisonOperator {
type Error = MarshalError;
/// Decode from `COMPARE_OP` arg: `(cmp_index << 5) | mask`.
fn try_from(value: u32) -> Result<Self, Self::Error> {
match value >> 5 {
0 => Ok(Self::Less),
1 => Ok(Self::LessOrEqual),
2 => Ok(Self::Equal),
3 => Ok(Self::NotEqual),
4 => Ok(Self::Greater),
5 => Ok(Self::GreaterOrEqual),
_ => Err(MarshalError::InvalidBytecode),
}
}
}

impl From<ComparisonOperator> for u8 {
/// Encode as `cmp_index << 5` (mask bits zero).
fn from(value: ComparisonOperator) -> Self {
match value {
ComparisonOperator::Less => 0,
ComparisonOperator::LessOrEqual => 1 << 5,
ComparisonOperator::Equal => 2 << 5,
ComparisonOperator::NotEqual => 3 << 5,
ComparisonOperator::Greater => 4 << 5,
ComparisonOperator::GreaterOrEqual => 5 << 5,
}
}
}

impl From<ComparisonOperator> for u32 {
fn from(value: ComparisonOperator) -> Self {
Self::from(u8::from(value))
}
}

impl OpArgType for ComparisonOperator {}

oparg_enum!(
/// The possible Binary operators
Expand Down
Loading
Loading