X Tutup
Skip to content
Merged
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
12 changes: 7 additions & 5 deletions crates/codegen/src/compile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1277,7 +1277,7 @@ impl Compiler {
context: OpArgMarker::marker(),
}
.into(),
arg: OpArg::new(u32::from(bytecode::ResumeType::AtFuncStart)),
arg: OpArg::new(oparg::ResumeLocation::AtFuncStart.into()),
target: BlockIdx::NULL,
location,
end_location,
Expand Down Expand Up @@ -7200,9 +7200,9 @@ impl Compiler {
self,
Instruction::Resume {
context: if is_await {
bytecode::ResumeType::AfterAwait
oparg::ResumeContext::from(oparg::ResumeLocation::AfterAwait)
} else {
bytecode::ResumeType::AfterYieldFrom
oparg::ResumeContext::from(oparg::ResumeLocation::AfterYieldFrom)
}
}
);
Expand Down Expand Up @@ -7374,7 +7374,7 @@ impl Compiler {
emit!(
self,
Instruction::Resume {
context: bytecode::ResumeType::AfterYield
context: oparg::ResumeContext::from(oparg::ResumeLocation::AfterYield)
}
);
}
Expand Down Expand Up @@ -7596,7 +7596,9 @@ impl Compiler {
emit!(
compiler,
Instruction::Resume {
context: bytecode::ResumeType::AfterYield
context: oparg::ResumeContext::from(
oparg::ResumeLocation::AfterYield
)
}
);
emit!(compiler, Instruction::PopTop);
Expand Down
29 changes: 14 additions & 15 deletions crates/codegen/src/ir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -363,7 +363,7 @@ impl CodeInfo {
}
}

let mut block_to_offset = vec![Label::new(0); blocks.len()];
let mut block_to_offset = vec![Label::from_u32(0); blocks.len()];
// block_to_index: maps block idx to instruction index (for exception table)
// This is the index into the final instructions array, including EXTENDED_ARG and CACHE
let mut block_to_index = vec![0u32; blocks.len()];
Expand All @@ -372,7 +372,7 @@ impl CodeInfo {
loop {
let mut num_instructions = 0;
for (idx, block) in iter_blocks(&blocks) {
block_to_offset[idx.idx()] = Label::new(num_instructions as u32);
block_to_offset[idx.idx()] = Label::from_u32(num_instructions as u32);
// block_to_index uses the same value as block_to_offset but as u32
// because lasti in frame.rs is the index into instructions array
// and instructions array index == byte offset (each instruction is 1 CodeUnit)
Expand Down Expand Up @@ -2188,20 +2188,19 @@ pub(crate) fn label_exception_targets(blocks: &mut [Block]) {
}

// Set RESUME DEPTH1 flag based on last yield's except depth
if matches!(
blocks[bi].instructions[i].instr.real(),
Some(Instruction::Resume { .. })
) {
const RESUME_AT_FUNC_START: u32 = 0;
const RESUME_OPARG_LOCATION_MASK: u32 = 0x3;
const RESUME_OPARG_DEPTH1_MASK: u32 = 0x4;

if (u32::from(arg) & RESUME_OPARG_LOCATION_MASK) != RESUME_AT_FUNC_START {
if last_yield_except_depth == 1 {
blocks[bi].instructions[i].arg =
OpArg::new(u32::from(arg) | RESUME_OPARG_DEPTH1_MASK);
if let Some(Instruction::Resume { context }) =
blocks[bi].instructions[i].instr.real()
{
let location = context.get(arg).location();
match location {
oparg::ResumeLocation::AtFuncStart => {}
_ => {
if last_yield_except_depth == 1 {
blocks[bi].instructions[i].arg =
OpArg::new(oparg::ResumeContext::new(location, true).as_u32());
}
last_yield_except_depth = -1;
}
last_yield_except_depth = -1;
}
}

Expand Down
4 changes: 2 additions & 2 deletions crates/compiler-core/src/bytecode.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ pub use crate::bytecode::{
BinaryOperator, BuildSliceArgCount, CommonConstant, ComparisonOperator, ConvertValueOparg,
IntrinsicFunction1, IntrinsicFunction2, Invert, Label, LoadAttr, LoadSuperAttr,
MakeFunctionFlag, MakeFunctionFlags, NameIdx, OpArg, OpArgByte, OpArgState, OpArgType,
RaiseKind, ResumeType, SpecialMethod, UnpackExArgs,
RaiseKind, SpecialMethod, UnpackExArgs,
},
};

Expand Down Expand Up @@ -1041,7 +1041,7 @@ impl<C: Constant> CodeObject<C> {
}

// arrow and offset
let arrow = if label_targets.contains(&Label::new(offset as u32)) {
let arrow = if label_targets.contains(&Label::from_u32(offset as u32)) {
">>"
} else {
" "
Expand Down
2 changes: 1 addition & 1 deletion crates/compiler-core/src/bytecode/instruction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -304,7 +304,7 @@ pub enum Instruction {
} = 120,
// CPython 3.14 RESUME (128)
Resume {
context: Arg<oparg::ResumeType>,
context: Arg<oparg::ResumeContext>,
} = 128,
// CPython 3.14 specialized opcodes (129-211)
BinaryOpAddFloat = 129, // Placeholder
Expand Down
162 changes: 109 additions & 53 deletions crates/compiler-core/src/bytecode/oparg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -276,48 +276,6 @@ impl fmt::Display for ConvertValueOparg {
}
}

/// Resume type for the RESUME instruction
#[derive(Copy, Clone, Debug, Hash, PartialEq, Eq)]
pub enum ResumeType {
AtFuncStart,
AfterYield,
AfterYieldFrom,
AfterAwait,
Other(u32),
}

impl From<u32> for ResumeType {
fn from(value: u32) -> Self {
match value {
0 => Self::AtFuncStart,
5 => Self::AfterYield,
2 => Self::AfterYieldFrom,
3 => Self::AfterAwait,
_ => Self::Other(value),
}
}
}

impl From<ResumeType> for u32 {
fn from(typ: ResumeType) -> Self {
match typ {
ResumeType::AtFuncStart => 0,
ResumeType::AfterYield => 5,
ResumeType::AfterYieldFrom => 2,
ResumeType::AfterAwait => 3,
ResumeType::Other(v) => v,
}
}
}

impl core::fmt::Display for ResumeType {
fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
u32::from(*self).fmt(f)
}
}

impl OpArgType for ResumeType {}

pub type NameIdx = u32;

impl OpArgType for u32 {}
Expand Down Expand Up @@ -756,14 +714,8 @@ macro_rules! newtype_oparg {
impl $name {
/// Creates a new [`$name`] instance.
#[must_use]
pub const fn new(value: u32) -> Self {
Self(value)
}

/// Alias to [`$name::new`].
#[must_use]
pub const fn from_u32(value: u32) -> Self {
Self::new(value)
Self(value)
}

/// Returns the oparg as a `u32` value.
Expand Down Expand Up @@ -843,15 +795,119 @@ newtype_oparg!(
pub struct Label(u32)
);

newtype_oparg!(
/// Context for [`Instruction::Resume`].
///
/// The oparg consists of two parts:
/// 1. [`ResumeContext::location`]: Indicates where the instruction occurs.
/// 2. [`ResumeContext::is_exception_depth1`]: Is the instruction is at except-depth 1.
#[derive(Clone, Copy)]
#[repr(transparent)]
pub struct ResumeContext(u32)
);
Comment on lines +798 to +807
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

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '\n[newtype_oparg! conversions]\n'
sed -n '714,758p' crates/compiler-core/src/bytecode/oparg.rs

printf '\n[ResumeContext definition]\n'
sed -n '798,894p' crates/compiler-core/src/bytecode/oparg.rs

printf '\n[explicit TryFrom<u32> for ResumeContext]\n'
rg -nC2 'impl\s+TryFrom<u32>\s+for\s+ResumeContext' crates/compiler-core/src/bytecode/oparg.rs || true

printf '\n[generic typed-oparg decode sites]\n'
rg -nC3 '\bOpArgType\b|TryInto::try_into|::try_from\(' crates/compiler-core/src

Repository: RustPython/RustPython

Length of output: 19038


Add explicit validation to ResumeContext::try_from(u32) for unknown bits.

Because ResumeContext is declared through newtype_oparg!, it gets impl From<u32>, and Rust supplies an infallible blanket TryFrom<u32>. This means invalid RESUME opargs with bits outside LOCATION_MASK (0x3) | DEPTH1_MASK (0x4) are silently accepted and later masked by location() / is_exception_depth1() instead of rejected as invalid bytecode. Implement an explicit TryFrom<u32> that validates the known bits.

Also fix the intra-doc links in the docstring: Context::location and Context::is_exception_depth1 should reference ResumeContext::location and ResumeContext::is_exception_depth1.

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

In `@crates/compiler-core/src/bytecode/oparg.rs` around lines 798 - 807, Update
the ResumeContext documentation links to point to ResumeContext::location and
ResumeContext::is_exception_depth1, and add an explicit TryFrom<u32> impl for
ResumeContext that rejects any u32 with bits outside the allowed mask
(LOCATION_MASK | DEPTH1_MASK); specifically, implement
ResumeContext::try_from(value: u32) to check (value & !ALLOWED_MASK) == 0 and
return Err for non-zero unknown bits, otherwise construct ResumeContext(value)
(replacing the implicit infallible conversion created by newtype_oparg!), so
invalid RESUME opargs are rejected rather than silently masked.


impl ResumeContext {
/// [CPython `RESUME_OPARG_LOCATION_MASK`](https://github.com/python/cpython/blob/v3.14.3/Include/internal/pycore_opcode_utils.h#L84)
pub const LOCATION_MASK: u32 = 0x3;

/// [CPython `RESUME_OPARG_DEPTH1_MASK`](https://github.com/python/cpython/blob/v3.14.3/Include/internal/pycore_opcode_utils.h#L85)
pub const DEPTH1_MASK: u32 = 0x4;

#[must_use]
pub const fn new(location: ResumeLocation, is_exception_depth1: bool) -> Self {
let value = if is_exception_depth1 {
Self::DEPTH1_MASK
} else {
0
};

Self::from_u32(location.as_u32() | value)
}

/// Resume location is determined by [`Self::LOCATION_MASK`].
#[must_use]
pub fn location(&self) -> ResumeLocation {
// SAFETY: The mask should return a value that is in range.
unsafe { ResumeLocation::try_from(self.as_u32() & Self::LOCATION_MASK).unwrap_unchecked() }
}

/// True if the bit at [`Self::DEPTH1_MASK`] is on.
#[must_use]
pub const fn is_exception_depth1(&self) -> bool {
(self.as_u32() & Self::DEPTH1_MASK) != 0
}
}

#[derive(Copy, Clone)]
pub enum ResumeLocation {
/// At the start of a function, which is neither a generator, coroutine nor an async generator.
AtFuncStart,
/// After a `yield` expression.
AfterYield,
/// After a `yield from` expression.
AfterYieldFrom,
/// After an `await` expression.
AfterAwait,
}

impl From<ResumeLocation> for ResumeContext {
fn from(location: ResumeLocation) -> Self {
Self::new(location, false)
}
}

impl TryFrom<u32> for ResumeLocation {
type Error = MarshalError;

fn try_from(value: u32) -> Result<Self, Self::Error> {
Ok(match value {
0 => Self::AtFuncStart,
1 => Self::AfterYield,
2 => Self::AfterYieldFrom,
3 => Self::AfterAwait,
_ => return Err(Self::Error::InvalidBytecode),
})
}
}

impl ResumeLocation {
#[must_use]
pub const fn as_u8(&self) -> u8 {
match self {
Self::AtFuncStart => 0,
Self::AfterYield => 1,
Self::AfterYieldFrom => 2,
Self::AfterAwait => 3,
}
}

#[must_use]
pub const fn as_u32(&self) -> u32 {
self.as_u8() as u32
}
}

impl From<ResumeLocation> for u8 {
fn from(location: ResumeLocation) -> Self {
location.as_u8()
}
}

impl From<ResumeLocation> for u32 {
fn from(location: ResumeLocation) -> Self {
location.as_u32()
}
}

impl VarNums {
#[must_use]
pub const fn idx_1(self) -> VarNum {
VarNum::new(self.0 >> 4)
VarNum::from_u32(self.0 >> 4)
}

#[must_use]
pub const fn idx_2(self) -> VarNum {
VarNum::new(self.0 & 15)
VarNum::from_u32(self.0 & 15)
}

#[must_use]
Expand Down Expand Up @@ -887,7 +943,7 @@ impl LoadAttrBuilder {
#[must_use]
pub const fn build(self) -> LoadAttr {
let value = (self.name_idx << 1) | (self.is_method as u32);
LoadAttr::new(value)
LoadAttr::from_u32(value)
}

#[must_use]
Expand Down Expand Up @@ -937,7 +993,7 @@ impl LoadSuperAttrBuilder {
pub const fn build(self) -> LoadSuperAttr {
let value =
(self.name_idx << 2) | ((self.has_class as u32) << 1) | (self.is_load_method as u32);
LoadSuperAttr::new(value)
LoadSuperAttr::from_u32(value)
}

#[must_use]
Expand Down
6 changes: 3 additions & 3 deletions crates/jit/src/instructions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ impl<'a, 'b> FunctionCompiler<'a, 'b> {
let target = after
.checked_add(u32::from(arg))
.ok_or(JitCompileError::BadBytecode)?;
Ok(Label::new(target))
Ok(Label::from_u32(target))
}

fn jump_target_backward(
Expand All @@ -177,7 +177,7 @@ impl<'a, 'b> FunctionCompiler<'a, 'b> {
let target = after
.checked_sub(u32::from(arg))
.ok_or(JitCompileError::BadBytecode)?;
Ok(Label::new(target))
Ok(Label::from_u32(target))
}

fn instruction_target(
Expand Down Expand Up @@ -232,7 +232,7 @@ impl<'a, 'b> FunctionCompiler<'a, 'b> {
let mut in_unreachable_code = false;

for (offset, &raw_instr) in clean_instructions.iter().enumerate() {
let label = Label::new(offset as u32);
let label = Label::from_u32(offset as u32);
let (instruction, arg) = arg_state.get(raw_instr);

// If this is a label that some earlier jump can target,
Expand Down
Loading
Loading
X Tutup