From 459ef42c41d24117d1adfb2a088c0f4a835601fe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Garillot?= Date: Tue, 7 Jul 2026 11:37:22 -0400 Subject: [PATCH 1/9] analysis: add bounded advice-taint analysis support --- codegen/masm/src/emit/binary.rs | 10 ++ codegen/masm/src/emit/unary.rs | 1 + codegen/masm/src/lower/lowering.rs | 7 +- dialects/arith/src/builders.rs | 15 +- dialects/arith/src/ops/binary.rs | 24 ++- .../hir/src/analyses/advice_taint/lattice.rs | 139 ++++++++++++++++++ dialects/hir/src/analyses/advice_taint/mod.rs | 90 ++++++++++-- .../hir/src/analyses/advice_taint/sinks.rs | 23 ++- dialects/hir/src/ops/cast.rs | 16 +- hir-analysis/src/config.rs | 16 ++ hir-analysis/src/solver.rs | 98 +++++++++++- hir-analysis/src/sparse/forward.rs | 8 +- hir/src/dialects/builtin/ops/cast.rs | 21 ++- 13 files changed, 446 insertions(+), 22 deletions(-) diff --git a/codegen/masm/src/emit/binary.rs b/codegen/masm/src/emit/binary.rs index 1fdf1f4a9..bf14e11d5 100644 --- a/codegen/masm/src/emit/binary.rs +++ b/codegen/masm/src/emit/binary.rs @@ -870,6 +870,16 @@ impl OpEmitter<'_> { self.push(ty); } + pub fn exp_u32_exponent(&mut self, span: SourceSpan) { + let rhs = self.pop().expect("operand stack is empty"); + let lhs = self.pop().expect("operand stack is empty"); + let ty = lhs.ty(); + assert_eq!(ty, Type::Felt, "expected exp.u32 base to be felt"); + assert_eq!(rhs.ty(), Type::Felt, "expected exp.u32 exponent to be felt"); + self.emit(masm::Instruction::ExpBitLength(32), span); + self.push(ty); + } + #[allow(unused)] pub fn exp_imm(&mut self, imm: Immediate, span: SourceSpan) { let lhs = self.pop().expect("operand stack is empty"); diff --git a/codegen/masm/src/emit/unary.rs b/codegen/masm/src/emit/unary.rs index 585ed144b..acf15f338 100644 --- a/codegen/masm/src/emit/unary.rs +++ b/codegen/masm/src/emit/unary.rs @@ -306,6 +306,7 @@ impl OpEmitter<'_> { self.felt_to_int(dst_bits, span); } // u32 + (Type::U32, Type::Felt) => (), (Type::U32, Type::I64 | Type::U64 | Type::I128) => self.zext_int32(dst_bits, span), (Type::U32, Type::I32) => self.assert_i32(span), (Type::U32, Type::U16 | Type::U8 | Type::I1) => { diff --git a/codegen/masm/src/lower/lowering.rs b/codegen/masm/src/lower/lowering.rs index a083df5d6..5e37f3462 100644 --- a/codegen/masm/src/lower/lowering.rs +++ b/codegen/masm/src/lower/lowering.rs @@ -682,7 +682,12 @@ impl HirLowering for arith::MulOverflowing { impl HirLowering for arith::Exp { fn emit(&self, emitter: &mut BlockEmitter<'_>) -> Result<(), Report> { - emitter.inst_emitter(self.as_operation()).exp(self.span()); + let mut emitter = emitter.inst_emitter(self.as_operation()); + if *self.get_exponent_must_be_u32() { + emitter.exp_u32_exponent(self.span()); + } else { + emitter.exp(self.span()); + } Ok(()) } } diff --git a/dialects/arith/src/builders.rs b/dialects/arith/src/builders.rs index fc627d0a1..56219badf 100644 --- a/dialects/arith/src/builders.rs +++ b/dialects/arith/src/builders.rs @@ -359,11 +359,24 @@ pub trait ArithOpBuilder<'f, B: ?Sized + Builder> { /// Exponentiation fn exp(&mut self, lhs: ValueRef, rhs: ValueRef, span: SourceSpan) -> Result { - let op_builder = self.builder_mut().create::(span); + let op_builder = self.builder_mut().create::(span); let op = op_builder(lhs, rhs)?; Ok(op.borrow().result().as_value_ref()) } + /// Exponentiation whose exponent operand must be u32-range constrained. + fn exp_u32_exponent( + &mut self, + lhs: ValueRef, + rhs: ValueRef, + span: SourceSpan, + ) -> Result { + let op_builder = + self.builder_mut().create::(span); + let op = op_builder(lhs, rhs, true)?; + Ok(op.borrow().result().as_value_ref()) + } + /// Compute 2^n fn pow2(&mut self, n: ValueRef, span: SourceSpan) -> Result { let op_builder = self.builder_mut().create::(span); diff --git a/dialects/arith/src/ops/binary.rs b/dialects/arith/src/ops/binary.rs index c37d1cc75..8e7b373bc 100644 --- a/dialects/arith/src/ops/binary.rs +++ b/dialects/arith/src/ops/binary.rs @@ -2,7 +2,7 @@ use alloc::rc::Rc; use midenc_hir::{ derive::{EffectOpInterface, OpParser, OpPrinter, operation}, - dialects::builtin::attributes::OverflowAttr, + dialects::builtin::attributes::{BoolAttr, OverflowAttr}, effects::*, traits::*, *, @@ -184,19 +184,39 @@ infer_return_ty_for_binary_op!(MulOverflowing, overflowed: Type::I1); #[operation( dialect = ArithDialect, traits(BinaryOp, SameTypeOperands, SameOperandsAndResultType), - implements(InferTypeOpInterface, MemoryEffectOpInterface, OpPrinter) + implements( + InferTypeOpInterface, + MemoryEffectOpInterface, + OperandRangeRequirementOpInterface, + OpPrinter + ) )] pub struct Exp { #[operand] lhs: IntFelt, #[operand] rhs: IntFelt, + #[attr] + #[default] + exponent_must_be_u32: BoolAttr, #[result] result: IntFelt, } infer_return_ty_for_binary_op!(Exp); +impl OperandRangeRequirementOpInterface for Exp { + fn operand_range_requirement(&self, operand_index: usize) -> OperandRangeRequirement { + if operand_index == 1 && *self.get_exponent_must_be_u32() { + OperandRangeRequirement::Required(ValueRangeConstraint::Type(Type::U32)) + } else { + default_operand_range_requirement(self.as_operation(), operand_index) + .map(OperandRangeRequirement::Required) + .unwrap_or(OperandRangeRequirement::None) + } + } +} + /// Unsigned integer division, traps on division by zero #[derive(EffectOpInterface, OpPrinter, OpParser)] #[operation( diff --git a/dialects/hir/src/analyses/advice_taint/lattice.rs b/dialects/hir/src/analyses/advice_taint/lattice.rs index 4a36f1230..4b7c9104e 100644 --- a/dialects/hir/src/analyses/advice_taint/lattice.rs +++ b/dialects/hir/src/analyses/advice_taint/lattice.rs @@ -156,6 +156,7 @@ impl LatticeLike for AdviceTaintValue { } const MAX_CALL_CONTEXT_DEPTH: usize = 4; +const MAX_CALL_CONTEXTS: usize = 32; type CallContext = SmallVec<[CallContextFrame; MAX_CALL_CONTEXT_DEPTH]>; pub(super) type AdviceTaintSparseLattice = Lattice; @@ -225,6 +226,24 @@ impl ContextualAdviceTaintValue { origins.into_iter() } + pub(super) fn call_context_spans_containing_origin( + &self, + origin: AdviceTaintOrigin, + ) -> Vec { + let mut spans = Vec::new(); + for (context, taint) in self.contexts.iter() { + if !taint.contains_origin(origin) { + continue; + } + for frame in context { + if !spans.contains(&frame.span) { + spans.push(frame.span); + } + } + } + spans + } + pub fn mark_reported(&self) -> Self { Self { contexts: self @@ -300,10 +319,26 @@ impl ContextualAdviceTaintValue { context: CallContext, taint: AdviceTaintValue, ) { + if context.is_empty() && !taint.is_clean() && !contexts.is_empty() { + let collapsed = + contexts.values().fold(taint, |acc, taint| LatticeLike::join(&acc, taint)); + contexts.clear(); + contexts.insert(CallContext::new(), collapsed); + return; + } + if let Some(empty) = contexts.get_mut(&CallContext::new()) + && !empty.is_clean() + { + *empty = LatticeLike::join(empty, &taint); + return; + } contexts .entry(context) .and_modify(|current| *current = LatticeLike::join(current, &taint)) .or_insert(taint); + canonicalize_clean_contexts(contexts); + remove_redundant_clean_contexts(contexts); + collapse_contexts_if_needed(contexts); } } @@ -342,6 +377,35 @@ fn push_call_context(context: &CallContext, frame: CallContextFrame) -> CallCont pushed } +fn collapse_contexts_if_needed(contexts: &mut BTreeMap) { + if contexts.len() <= MAX_CALL_CONTEXTS { + return; + } + if contexts.values().all(AdviceTaintValue::is_clean) { + return; + } + + let collapsed = contexts + .values() + .fold(AdviceTaintValue::clean(), |acc, taint| LatticeLike::join(&acc, taint)); + contexts.clear(); + contexts.insert(CallContext::new(), collapsed); +} + +fn remove_redundant_clean_contexts(contexts: &mut BTreeMap) { + if contexts.len() <= 1 { + return; + } + contexts.retain(|_, taint| !taint.is_clean()); +} + +fn canonicalize_clean_contexts(contexts: &mut BTreeMap) { + if contexts.values().all(AdviceTaintValue::is_clean) { + contexts.clear(); + contexts.insert(CallContext::new(), AdviceTaintValue::clean()); + } +} + #[derive(Debug, Copy, Clone, Eq, PartialEq)] enum OriginState { Unreported, @@ -385,3 +449,78 @@ pub(super) fn value_taint(value: ValueRef, solver: &DataFlowSolver) -> Contextua .map(|state| state.value().clone()) .unwrap_or_default() } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn contextual_taint_collapses_when_call_context_cap_is_exceeded() { + let raw = ContextualAdviceTaintValue::raw(SourceSpan::UNKNOWN); + let mut joined = ContextualAdviceTaintValue::clean(); + + for id in 0..=MAX_CALL_CONTEXTS { + let frame = CallContextFrame { + id, + span: SourceSpan::UNKNOWN, + }; + joined = LatticeLike::join(&joined, &raw.enter_call(frame)); + } + + assert_eq!(joined.contexts.len(), 1); + assert!(joined.contexts.contains_key(&CallContext::new())); + assert!(joined.has_unreported_origin()); + } + + #[test] + fn contextual_taint_keeps_distinct_call_contexts_below_cap() { + let raw = ContextualAdviceTaintValue::raw(SourceSpan::UNKNOWN); + let mut joined = ContextualAdviceTaintValue::clean(); + + for id in 0..MAX_CALL_CONTEXTS { + let frame = CallContextFrame { + id, + span: SourceSpan::UNKNOWN, + }; + joined = LatticeLike::join(&joined, &raw.enter_call(frame)); + } + + assert_eq!(joined.contexts.len(), MAX_CALL_CONTEXTS); + assert!(!joined.contexts.contains_key(&CallContext::new())); + } + + #[test] + fn collapsed_contextual_taint_absorbs_precise_contexts() { + let raw = ContextualAdviceTaintValue::raw(SourceSpan::UNKNOWN); + let mut precise = ContextualAdviceTaintValue::clean(); + + for id in 0..MAX_CALL_CONTEXTS { + let frame = CallContextFrame { + id, + span: SourceSpan::UNKNOWN, + }; + precise = LatticeLike::join(&precise, &raw.enter_call(frame)); + } + + let collapsed = ContextualAdviceTaintValue::raw(SourceSpan::UNKNOWN); + let joined = LatticeLike::join(&collapsed, &precise); + + assert_eq!(joined, collapsed); + } + + #[test] + fn clean_contextual_taint_stays_canonical_across_call_contexts() { + let clean = ContextualAdviceTaintValue::clean(); + let mut joined = ContextualAdviceTaintValue::clean(); + + for id in 0..MAX_CALL_CONTEXTS { + let frame = CallContextFrame { + id, + span: SourceSpan::UNKNOWN, + }; + joined = LatticeLike::join(&joined, &clean.enter_call(frame)); + } + + assert_eq!(joined, ContextualAdviceTaintValue::clean()); + } +} diff --git a/dialects/hir/src/analyses/advice_taint/mod.rs b/dialects/hir/src/analyses/advice_taint/mod.rs index aaca4d4ec..d3fc7aeff 100644 --- a/dialects/hir/src/analyses/advice_taint/mod.rs +++ b/dialects/hir/src/analyses/advice_taint/mod.rs @@ -5,7 +5,11 @@ mod propagation; mod sinks; mod storage; -use alloc::{rc::Rc, vec::Vec}; +use alloc::{ + rc::Rc, + string::{String, ToString}, + vec::Vec, +}; use core::any::Any; use midenc_hir::{ @@ -44,6 +48,11 @@ pub struct AdviceTaintAnalysis { external_call_findings: Vec, } +pub struct AdviceTaintAnalysisResult { + pub analysis: AdviceTaintAnalysis, + pub incomplete_reason: Option, +} + impl AdviceTaintAnalysis { pub fn findings(&self) -> &[AdviceTaintFinding] { &self.findings @@ -86,6 +95,68 @@ impl AdviceTaintAnalysis { pub fn solver(&self) -> &DataFlowSolver { &self.solver } + + pub fn analyze_with_config( + &mut self, + op: &Operation, + analysis_manager: AnalysisManager, + config: DataFlowConfig, + ) -> Result<(), Report> { + self.run_solver_with_config(op, analysis_manager, config)?; + self.collect_analysis_results(op); + Ok(()) + } + + fn run_solver_with_config( + &mut self, + op: &Operation, + analysis_manager: AnalysisManager, + config: DataFlowConfig, + ) -> Result<(), Report> { + self.solver = DataFlowSolver::new(config); + self.solver.load::(); + self.solver.load::(); + self.solver.initialize_and_run(op, analysis_manager) + } + + fn collect_analysis_results(&mut self, op: &Operation) { + self.findings = collect_findings(op, &self.solver); + self.exit_findings = collect_exit_findings(op, &self.solver); + self.external_call_findings = collect_external_call_findings(op, &self.solver); + } + + pub fn run_with_config( + op: &Operation, + analysis_manager: AnalysisManager, + config: DataFlowConfig, + ) -> Result { + let mut analysis = Self::default(); + analysis.analyze_with_config(op, analysis_manager, config)?; + Ok(analysis) + } + + pub fn run_with_config_allow_partial( + op: &Operation, + analysis_manager: AnalysisManager, + config: DataFlowConfig, + ) -> Result { + let mut analysis = Self::default(); + let incomplete_reason = match analysis.run_solver_with_config(op, analysis_manager, config) + { + Ok(()) => None, + Err(err) if is_dataflow_budget_exhaustion(&err) => Some(err.to_string()), + Err(err) => return Err(err), + }; + analysis.collect_analysis_results(op); + Ok(AdviceTaintAnalysisResult { + analysis, + incomplete_reason, + }) + } +} + +fn is_dataflow_budget_exhaustion(err: &Report) -> bool { + err.to_string().contains("dataflow solver exceeded worklist iteration budget") } impl Analysis for AdviceTaintAnalysis { @@ -110,14 +181,7 @@ impl Analysis for AdviceTaintAnalysis { ) -> Result<(), Report> { let mut config = DataFlowConfig::new(); config.set_interprocedural(true); - self.solver = DataFlowSolver::new(config); - self.solver.load::(); - self.solver.load::(); - self.solver.initialize_and_run(op, analysis_manager)?; - self.findings = collect_findings(op, &self.solver); - self.exit_findings = collect_exit_findings(op, &self.solver); - self.external_call_findings = collect_external_call_findings(op, &self.solver); - Ok(()) + self.analyze_with_config(op, analysis_manager, config) } fn invalidate(&self, _preserved_analyses: &mut PreservedAnalyses) -> bool { @@ -149,12 +213,16 @@ fn collect_findings(op: &Operation, solver: &DataFlowSolver) -> Vec OperandRangeRequirement { + // Casts may refine their result via `CheckedCastOpInterface`, but they are + // not themselves semantic consumers of a range-constrained value. + OperandRangeRequirement::None + } +} + impl CheckedCastOpInterface for Cast { fn checked_cast_refinement(&self, result: ValueRef) -> Option { let cast_result = self.result().as_value_ref(); diff --git a/hir-analysis/src/config.rs b/hir-analysis/src/config.rs index c53ad7158..e18682f0a 100644 --- a/hir-analysis/src/config.rs +++ b/hir-analysis/src/config.rs @@ -3,6 +3,8 @@ pub struct DataFlowConfig { /// Indicates whether the solver should operation interprocedurally interprocedural: bool, + /// Optional limit on queued analysis visits while solving to fixpoint. + max_worklist_iterations: Option, } impl DataFlowConfig { @@ -17,6 +19,11 @@ impl DataFlowConfig { self.interprocedural } + #[inline(always)] + pub const fn max_worklist_iterations(&self) -> Option { + self.max_worklist_iterations + } + /// Set whether the solver should operate interprocedurally, i.e. enter the callee body when /// available. /// @@ -26,4 +33,13 @@ impl DataFlowConfig { self.interprocedural = yes; self } + + /// Set a maximum number of queued analysis visits while solving to fixpoint. + /// + /// This is intended for lint and diagnostic callers that need a bounded analysis result instead + /// of an unbounded run on large or currently unsupported IR graphs. + pub fn set_max_worklist_iterations(&mut self, max: Option) -> &mut Self { + self.max_worklist_iterations = max; + self + } } diff --git a/hir-analysis/src/solver.rs b/hir-analysis/src/solver.rs index a9c586490..7ea22bd28 100644 --- a/hir-analysis/src/solver.rs +++ b/hir-analysis/src/solver.rs @@ -1,10 +1,10 @@ mod allocator; -use alloc::{collections::VecDeque, rc::Rc}; +use alloc::{collections::VecDeque, format, rc::Rc, string::ToString, vec::Vec}; use core::{any::TypeId, cell::RefCell, ptr::NonNull}; use midenc_hir::{ - EntityRef, FxHashMap, Operation, ProgramPoint, Report, SmallVec, hashbrown, + EntityRef, FxHashMap, Operation, ProgramPoint, Report, SmallVec, dialects::builtin, hashbrown, pass::AnalysisManager, }; @@ -307,6 +307,7 @@ impl DataFlowSolver { log::debug!(target: "dataflow:solver", "running queued dataflow analyses to fixpoint.."); // Run the analysis until fixpoint + let mut iterations = 0usize; while let Some(QueuedAnalysis { point, mut analysis, @@ -314,6 +315,32 @@ impl DataFlowSolver { let mut worklist = self.worklist.borrow_mut(); worklist.pop_front() } { + if let Some(max) = self.config.max_worklist_iterations() + && iterations >= max + { + let remaining = self.worklist.borrow().len() + 1; + let analysis_name = unsafe { analysis.as_ref().debug_name() }; + let owner = describe_program_point_owner(&point); + let queue_summary = summarize_queued_analyses( + core::iter::once(analysis_name).chain( + self.worklist + .borrow() + .iter() + .map(|queued| unsafe { queued.analysis.as_ref().debug_name() }), + ), + ); + let owner_summary = summarize_queued_program_point_owners( + core::iter::once(&point) + .chain(self.worklist.borrow().iter().map(|queued| &queued.point)), + ); + return Err(Report::msg(format!( + "dataflow solver exceeded worklist iteration budget of {max}; next analysis \ + would run {analysis_name} at {point}{owner}, with {remaining} queued item(s) \ + remaining; queued analyses: {queue_summary}; queued functions: \ + {owner_summary}", + ))); + } + iterations += 1; self.current_analysis = Some(analysis); unsafe { let analysis = analysis.as_mut(); @@ -556,6 +583,73 @@ impl DataFlowSolver { } } +fn summarize_queued_analyses( + names: impl IntoIterator, +) -> alloc::string::String { + let mut counts = Vec::<(&'static str, usize)>::new(); + for name in names { + if let Some((_, count)) = counts.iter_mut().find(|(existing, _)| *existing == name) { + *count += 1; + } else { + counts.push((name, 1)); + } + } + counts.sort_by(|(lhs_name, lhs_count), (rhs_name, rhs_count)| { + rhs_count.cmp(lhs_count).then_with(|| lhs_name.cmp(rhs_name)) + }); + counts + .into_iter() + .take(5) + .map(|(name, count)| format!("{name}={count}")) + .collect::>() + .join(", ") +} + +fn describe_program_point_owner(point: &ProgramPoint) -> alloc::string::String { + describe_program_point_owner_name(point) + .map(|name| format!(" in function '{name}'")) + .unwrap_or_default() +} + +fn describe_program_point_owner_name(point: &ProgramPoint) -> Option { + let Some(op_ref) = point.operation() else { + return None; + }; + let op = op_ref.borrow(); + if let Some(function) = op.downcast_ref::() { + return Some(function.get_name().as_str().to_string()); + } + op.nearest_parent_op::().map(|function| { + let function = function.borrow(); + function.get_name().as_str().to_string() + }) +} + +fn summarize_queued_program_point_owners<'a>( + points: impl IntoIterator, +) -> alloc::string::String { + let mut counts = Vec::<(alloc::string::String, usize)>::new(); + for name in points + .into_iter() + .map(|point| describe_program_point_owner_name(point).unwrap_or_else(|| "".into())) + { + if let Some((_, count)) = counts.iter_mut().find(|(existing, _)| *existing == name) { + *count += 1; + } else { + counts.push((name, 1)); + } + } + counts.sort_by(|(lhs_name, lhs_count), (rhs_name, rhs_count)| { + rhs_count.cmp(lhs_count).then_with(|| lhs_name.cmp(rhs_name)) + }); + counts + .into_iter() + .take(5) + .map(|(name, count)| format!("{name}={count}")) + .collect::>() + .join(", ") +} + /// Represents an analysis that has derived facts at a specific program point from the state of /// another analysis, that has since changed. As a result, the dependent analysis must be re-applied /// at that program point to determine if the state changes have any effect on the state of its diff --git a/hir-analysis/src/sparse/forward.rs b/hir-analysis/src/sparse/forward.rs index fa89fdcf7..0ea38eba8 100644 --- a/hir-analysis/src/sparse/forward.rs +++ b/hir-analysis/src/sparse/forward.rs @@ -288,8 +288,14 @@ where for predecessor in predecessors.known_predecessors() { let inputs = predecessors.successor_inputs(predecessor); let mut operand_lattices = SmallVec::<[_; 4]>::with_capacity(inputs.len()); + let mut required_inputs = SmallVec::<[ValueRef; 4]>::new(); for operand in inputs.iter().copied() { - let operand_lattice = get_lattice_element_for::(current_point, operand, solver); + let operand_lattice = if required_inputs.contains(&operand) { + get_lattice_element::(operand, solver) + } else { + required_inputs.push(operand); + get_lattice_element_for::(current_point, operand, solver) + }; operand_lattices.push(operand_lattice); } analysis.visit_call_control_flow_transfer( diff --git a/hir/src/dialects/builtin/ops/cast.rs b/hir/src/dialects/builtin/ops/cast.rs index 9638d55ce..8af918eec 100644 --- a/hir/src/dialects/builtin/ops/cast.rs +++ b/hir/src/dialects/builtin/ops/cast.rs @@ -3,14 +3,22 @@ use crate::{ derive::{EffectOpInterface, OpParser, OpPrinter, operation}, dialects::builtin::{BuiltinDialect, attributes::TypeAttr}, effects::MemoryEffectOpInterface, - traits::{AnyType, InferTypeOpInterface, UnaryOp}, + traits::{ + AnyType, InferTypeOpInterface, OperandRangeRequirement, OperandRangeRequirementOpInterface, + UnaryOp, + }, }; #[derive(EffectOpInterface, OpPrinter, OpParser)] #[operation( dialect = BuiltinDialect, traits(UnaryOp), - implements(InferTypeOpInterface, MemoryEffectOpInterface, OpPrinter) + implements( + InferTypeOpInterface, + MemoryEffectOpInterface, + OperandRangeRequirementOpInterface, + OpPrinter + ) )] pub struct UnrealizedConversionCast { #[operand] @@ -28,3 +36,12 @@ impl InferTypeOpInterface for UnrealizedConversionCast { Ok(()) } } + +impl OperandRangeRequirementOpInterface for UnrealizedConversionCast { + fn operand_range_requirement(&self, _operand_index: usize) -> OperandRangeRequirement { + // Unrealized casts bridge representations while conversion/legalization is in progress. + // The operation that semantically consumes a constrained value should carry the range + // requirement. + OperandRangeRequirement::None + } +} From 410344667355e3da81b8215c8c712220d386a1cb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Garillot?= Date: Tue, 7 Jul 2026 11:37:39 -0400 Subject: [PATCH 2/9] frontend-masm: add MASM lint-mode project lifting --- Cargo.lock | 1 + frontend/masm/Cargo.toml | 1 + frontend/masm/src/infer.rs | 84 +- frontend/masm/src/lib.rs | 145 +++- frontend/masm/src/lift.rs | 538 ++++++++++-- frontend/masm/src/semantics.rs | 5 +- frontend/masm/src/tests.rs | 1487 +++++++++++++++++++++++++++++--- frontend/masm/tests/e2e.rs | 26 + 8 files changed, 2066 insertions(+), 221 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 037a6b60c..db91f1ac9 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3770,6 +3770,7 @@ dependencies = [ "midenc-dialect-hir", "midenc-dialect-scf", "midenc-hir", + "midenc-hir-analysis", "midenc-session", "rustc-hash", ] diff --git a/frontend/masm/Cargo.toml b/frontend/masm/Cargo.toml index 9bec49ae8..293bf0670 100644 --- a/frontend/masm/Cargo.toml +++ b/frontend/masm/Cargo.toml @@ -33,4 +33,5 @@ miden-core-lib = { workspace = true, features = ["std"] } miden-package-registry.workspace = true miden-processor = { workspace = true, features = ["std"] } midenc-codegen-masm.workspace = true +midenc-hir-analysis.workspace = true midenc-session.workspace = true diff --git a/frontend/masm/src/infer.rs b/frontend/masm/src/infer.rs index f24ea8c13..e6196a1f1 100644 --- a/frontend/masm/src/infer.rs +++ b/frontend/masm/src/infer.rs @@ -6,7 +6,7 @@ use miden_assembly::{ }; use miden_assembly_syntax::{ ast::{Block, Immediate, Instruction, InvocationTarget, Op, Procedure, SymbolResolution}, - debuginfo::{SourceManager, SourceSpan}, + debuginfo::{SourceManager, SourceSpan, Spanned}, parser::{IntValue, PushValue, WordValue}, }; use midenc_hir::{ @@ -40,6 +40,67 @@ pub(crate) fn infer_signature( Ok(Signature::with_convention(context, CallConv::Fast, params, results)) } +pub(crate) fn validate_declared_signature( + gid: GlobalItemIndex, + procedure: &Procedure, + context: &Rc, + linker: &Linker, + signatures: &FxHashMap, + signature: &Signature, +) -> Result<()> { + let mut state = InferState::new(gid, context, linker, signatures); + state.stack = signature + .params() + .iter() + .rev() + .map(|param| AbstractValue::typed(param.ty.clone(), procedure.span())) + .collect(); + state.infer_block(procedure.body())?; + + if !state.inputs.is_empty() { + return Err(Report::msg(format!( + "declared signature for '{}::{}' requires {} undeclared input value(s); stack \ + underflow at {}", + linker[gid.module].path(), + procedure.name(), + state.inputs.len(), + state.format_span(procedure.span()) + ))); + } + + let expected_results = signature.results().len(); + if state.stack.len() < expected_results { + return Err(Report::msg(format!( + "declared signature for '{}::{}' expects {} result value(s), but the body leaves only \ + {}; stack underflow at {}", + linker[gid.module].path(), + procedure.name(), + expected_results, + state.stack.len(), + state.format_span(procedure.span()) + ))); + } + + for result in signature.results() { + let value = state + .stack + .pop() + .expect("declared signature result count was checked before popping"); + value.constrain(result.ty.clone(), procedure.span()); + } + + if !state.stack.is_empty() { + return Err(Report::msg(format!( + "declared signature for '{}::{}' leaves {} extra value(s) on the stack", + linker[gid.module].path(), + procedure.name(), + state.stack.len() + ))); + } + + Ok(()) +} + #[derive(Clone)] struct AbstractValue(Rc>); @@ -316,6 +377,12 @@ impl<'a> InferState<'a> { self.push(Type::Felt); Ok(()) } + ExpBitLength(32) => { + self.pop_with_type(Type::U32, span)?; + self.pop_with_type(Type::Felt, span)?; + self.push(Type::Felt); + Ok(()) + } Not => { self.pop_with_type(Type::I1, span)?; self.push(Type::I1); @@ -639,7 +706,8 @@ impl<'a> InferState<'a> { if then_state.stack.len() != else_state.stack.len() { return Err(Report::msg(format!( - "if branches leave different inferred stack depths at {span:?}: then={}, else={}", + "if branches leave different inferred stack depths at {}: then={}, else={}", + self.format_span(span), then_state.stack.len(), else_state.stack.len() ))); @@ -673,8 +741,9 @@ impl<'a> InferState<'a> { let expected = inputs.len() + base_stack.len() + 1; if body_state.stack.len() != expected { return Err(Report::msg(format!( - "while body must leave {expected} inferred value(s) for the next iteration at \ - {span:?}, but left {}", + "while body must leave {expected} inferred value(s) for the next iteration at {}, \ + but left {}", + self.format_span(span), body_state.stack.len() ))); } @@ -700,6 +769,13 @@ impl<'a> InferState<'a> { Ok(()) } + fn format_span(&self, span: SourceSpan) -> String { + match self.source_manager.file_line_col(span) { + Ok(location) => format!("{location}"), + Err(_) => format!("{span:?}"), + } + } + fn ext2_binary(&mut self, span: SourceSpan) -> Result<()> { for _ in 0..4 { self.pop_with_type(Type::Felt, span)?; diff --git a/frontend/masm/src/lib.rs b/frontend/masm/src/lib.rs index 4eff6961a..788b3788b 100644 --- a/frontend/masm/src/lib.rs +++ b/frontend/masm/src/lib.rs @@ -14,7 +14,7 @@ use std::{collections::BTreeMap, path::Path, rc::Rc, sync::Arc}; use miden_assembly::{ProjectSourceInputs, ast::ModuleKind}; use miden_assembly_syntax::{ ast::{self, Module}, - debuginfo::{SourceLanguage, SourceManager, Uri}, + debuginfo::{SourceLanguage, SourceManager, SourceSpan, Uri}, parser::read_modules_from_root, }; use midenc_hir::{Context, FunctionType, Report, Type, dialects::builtin}; @@ -58,6 +58,16 @@ pub struct DisassembledWorld { /// This is retained as a convenience for single-module callers and existing analyses which /// operate on a module root. Multi-module callers should prefer walking `world`. pub module: builtin::ModuleRef, + /// Procedures omitted from the HIR world in lint mode. + pub skipped_procedures: Vec, +} + +/// A MASM procedure skipped while building a lintable HIR world. +#[derive(Debug, Clone)] +pub struct SkippedProcedure { + pub path: Arc, + pub span: SourceSpan, + pub reason: String, } /// Disassemble a MASM file into an HIR world. @@ -74,7 +84,23 @@ pub fn disassemble_file( let target = project::ProjectTargetInput::new(ProjectSourceInputs { root, support }, Default::default()); - lift::lift_project_target(target, config, context) + lift::lift_project_target(target, &lift::LiftConfig::strict(config), context) +} + +/// Disassemble a MASM file for linting, skipping procedures that cannot be lifted. +pub fn disassemble_file_for_lint( + path: impl AsRef, + config: &DisassemblerConfig, + context: Rc, +) -> Result { + let path = path.as_ref(); + let source_manager = context.source_manager(); + let warnings_as_errors = context.session().options.diagnostics.warnings.warnings_as_errors(); + let (root, support) = + read_modules_from_root(path, None, None, source_manager, warnings_as_errors)?; + let target = + project::ProjectTargetInput::new(ProjectSourceInputs { root, support }, Default::default()); + lift::lift_project_target(target, &lift::LiftConfig::lint(config), context) } /// Disassemble a MASM file into an HIR world, using externally-provided procedure signatures for @@ -97,7 +123,7 @@ pub fn disassemble_file_with_external_signatures( ..Default::default() }, ); - lift::lift_project_target(target, config, context) + lift::lift_project_target(target, &lift::LiftConfig::strict(config), context) } /// Disassemble a MASM source string into an HIR world. @@ -115,7 +141,25 @@ pub fn disassemble_source( }, ExternalMetadata::default(), ); - lift::lift_project_target(target, config, context) + lift::lift_project_target(target, &lift::LiftConfig::strict(config), context) +} + +/// Disassemble a MASM source string for linting, skipping procedures that cannot be lifted. +pub fn disassemble_source_for_lint( + source: impl Into, + module_path: impl AsRef, + config: &DisassemblerConfig, + context: Rc, +) -> Result { + let root = parse_source_with_module_path(source, module_path, context.clone())?; + let target = project::ProjectTargetInput::new( + ProjectSourceInputs { + root, + support: Default::default(), + }, + ExternalMetadata::default(), + ); + lift::lift_project_target(target, &lift::LiftConfig::lint(config), context) } /// Disassemble a MASM source string into an HIR world, using externally-provided procedure @@ -193,7 +237,7 @@ pub fn disassemble_source_with_external_signatures( }; target.kernel = kernel; target.dependency_modules.extend(modules.into_values()); - lift::lift_project_target(target, config, context) + lift::lift_project_target(target, &lift::LiftConfig::strict(config), context) } /// Disassemble already-parsed `sources`, discovering external procedure signatures from @@ -221,7 +265,42 @@ pub fn disassemble_project_target_with_sources( &context, )?; let inputs = project::ProjectTargetInput::new(sources, external_metadata); - lift::lift_project_target(inputs, config, context) + lift::lift_project_target(inputs, &lift::LiftConfig::strict(config), context) +} + +/// Disassemble a project target for linting, skipping procedures that cannot be lifted. +pub fn disassemble_project_target_for_lint( + project: &miden_project::Project, + target: Option<&str>, + sources: Option, + config: &DisassemblerConfig, + context: Rc, +) -> Result { + let inputs = if let Some(sources) = sources { + let metadata = project::collect_dependency_metadata(project, &context)?; + project::ProjectTargetInput::new(sources, metadata) + } else { + project::resolve_project_target(project, target, &context)? + }; + lift::lift_project_target(inputs, &lift::LiftConfig::lint(config), context) +} + +/// Disassemble pre-resolved project target inputs. +pub fn disassemble_project_target_input( + inputs: project::ProjectTargetInput, + config: &DisassemblerConfig, + context: Rc, +) -> Result { + lift::lift_project_target(inputs, &lift::LiftConfig::strict(config), context) +} + +/// Disassemble pre-resolved project target inputs for linting. +pub fn disassemble_project_target_input_for_lint( + inputs: project::ProjectTargetInput, + config: &DisassemblerConfig, + context: Rc, +) -> Result { + lift::lift_project_target(inputs, &lift::LiftConfig::lint(config), context) } /// Disassemble a target from a `miden-project.toml` package manifest. @@ -236,7 +315,22 @@ pub fn disassemble_project_target_from_path( target, &context, )?; - lift::lift_project_target(target, config, context) + lift::lift_project_target(target, &lift::LiftConfig::strict(config), context) +} + +/// Disassemble a target from a `miden-project.toml` package manifest for linting. +pub fn disassemble_project_target_from_path_for_lint( + manifest_path: impl AsRef, + target: Option<&str>, + config: &DisassemblerConfig, + context: Rc, +) -> Result { + let target = project::resolve_project_target_from_manifest_path( + manifest_path.as_ref(), + target, + &context, + )?; + lift::lift_project_target(target, &lift::LiftConfig::lint(config), context) } /// Disassemble a target from a `miden-project.toml` package manifest, using a precomputed @@ -254,7 +348,24 @@ pub fn disassemble_project_target_with_dependency_graph( dependency_graph, &context, )?; - lift::lift_project_target(target, config, context) + lift::lift_project_target(target, &lift::LiftConfig::strict(config), context) +} + +/// Disassemble a target from a manifest and precomputed dependency graph for linting. +pub fn disassemble_project_target_with_dependency_graph_for_lint( + manifest_path: impl AsRef, + target: Option<&str>, + dependency_graph: &miden_project::ProjectDependencyGraph, + config: &DisassemblerConfig, + context: Rc, +) -> Result { + let target = project::resolve_project_target_from_manifest_path_with_dependency_graph( + manifest_path.as_ref(), + target, + dependency_graph, + &context, + )?; + lift::lift_project_target(target, &lift::LiftConfig::lint(config), context) } /// Disassemble a parsed MASM AST module into HIR. @@ -270,7 +381,23 @@ pub fn disassemble_module( }, ExternalMetadata::default(), ); - lift::lift_project_target(target, config, context) + lift::lift_project_target(target, &lift::LiftConfig::strict(config), context) +} + +/// Disassemble a parsed MASM AST module for linting, skipping procedures that cannot be lifted. +pub fn disassemble_module_for_lint( + root: Box, + config: &DisassemblerConfig, + context: Rc, +) -> Result { + let target = project::ProjectTargetInput::new( + ProjectSourceInputs { + root, + support: Default::default(), + }, + ExternalMetadata::default(), + ); + lift::lift_project_target(target, &lift::LiftConfig::lint(config), context) } fn parse_source_with_module_path( diff --git a/frontend/masm/src/lift.rs b/frontend/masm/src/lift.rs index e2c92db63..1f72bf573 100644 --- a/frontend/masm/src/lift.rs +++ b/frontend/masm/src/lift.rs @@ -32,15 +32,40 @@ use rustc_hash::FxHashMap; use crate::{ DisassembledWorld, DisassemblerConfig, ExternalSignatureMap, ExternalTypeMap, Result, + SkippedProcedure, events::{system_event_id, system_event_read_count}, infer, project, semantics::{self, InstructionSemantics}, stack as masm_stack, }; +const LINT_ESTIMATED_HIR_OP_LIMIT: usize = 900; +const LINT_SIGNATURE_VALUE_LIMIT: usize = 8; + +pub(crate) struct LiftConfig { + infer_missing_signatures: bool, + lint: bool, +} + +impl LiftConfig { + pub(crate) fn strict(config: &DisassemblerConfig) -> Self { + Self { + infer_missing_signatures: config.infer_missing_signatures, + lint: false, + } + } + + pub(crate) fn lint(config: &DisassemblerConfig) -> Self { + Self { + infer_missing_signatures: config.infer_missing_signatures, + lint: true, + } + } +} + pub(crate) fn lift_project_target( target: project::ProjectTargetInput, - config: &DisassemblerConfig, + config: &LiftConfig, context: Rc, ) -> Result { let project::ProjectTargetInput { @@ -72,7 +97,7 @@ fn lift_modules( kernel: Option>, packages: Vec>, extra_modules: Vec>, - config: &DisassemblerConfig, + config: &LiftConfig, external_signatures: ExternalSignatureMap, external_types: ExternalTypeMap, context: Rc, @@ -124,7 +149,11 @@ fn lift_modules( } } } - let module_indices = linker.link([root], support)?; + let (module_indices, initial_skips) = if config.lint { + link_modules_for_lint(&mut linker, root, support)? + } else { + (linker.link([root], support)?, Vec::new()) + }; let root_index = module_indices[0]; let mut registry = ModuleRegistry::new( @@ -132,6 +161,7 @@ fn lift_modules( module_indices, external_signatures, external_types, + initial_skips, context.clone(), ); registry.infer_missing_signatures(config)?; @@ -148,13 +178,105 @@ fn lift_modules( registry.lift_bodies()?; let module = registry.modules[&root_index]; + let skipped_procedures = registry.skipped_procedures(); Ok(DisassembledWorld { context, world, module, + skipped_procedures, }) } +fn link_modules_for_lint( + linker: &mut Linker, + root: Box, + support: Vec>, +) -> Result<(Vec, Vec<(GlobalItemIndex, SourceSpan, String)>)> { + let mut module_indices = linker.link_modules([root])?; + module_indices.extend(linker.link_modules(support)?); + + let mut skipped = BTreeMap::::new(); + loop { + let mut progress = false; + for module_index in 0..linker.modules().len() { + let module_index = ModuleIndex::new(module_index); + for (item_index, item) in linker[module_index].symbols().enumerate() { + let gid = module_index + ast::ItemIndex::new(item_index); + if skipped.contains_key(&gid) { + continue; + } + let SymbolItem::Procedure(procedure) = item.item() else { + continue; + }; + let procedure = procedure.borrow(); + let mut reason = None; + for invoke in procedure.invoked() { + let resolution = SymbolResolutionContext { + span: invoke.span(), + module: module_index, + kind: Some(invoke.kind), + }; + match linker.resolve_invoke_target(&resolution, &invoke.target) { + Ok(SymbolResolution::Exact { gid: callee, .. }) => { + if let Some((_, callee_reason)) = skipped.get(&callee) { + let callee_path = linker[callee.module] + .path() + .join(linker[callee].name()); + reason = Some(format!( + "depends on skipped procedure '{callee_path}': {callee_reason}" + )); + break; + } + } + Ok(_) => {} + Err(err) => { + reason = Some(format!( + "failed to resolve invocation during signature metadata pre-scan: \ + {err}; external signature metadata is missing" + )); + break; + } + } + } + if let Some(reason) = reason { + skipped.insert(gid, (procedure.span(), reason)); + progress = true; + } + } + } + if !progress { + break; + } + } + + for gid in skipped.keys().copied() { + let SymbolItem::Procedure(procedure) = linker[gid].item() else { + continue; + }; + let mut procedure = procedure.borrow_mut(); + let span = procedure.span(); + let signature = procedure.signature().cloned(); + let mut stub = Procedure::new( + span, + procedure.visibility(), + procedure.name().clone(), + procedure.num_locals(), + Block::new(span, Vec::new()), + ); + stub.set_syscall(procedure.is_syscall()); + if let Some(signature) = signature { + stub.set_signature(signature); + } + *procedure = Box::new(stub); + } + + linker.link(core::iter::empty(), core::iter::empty())?; + Ok(( + module_indices, + skipped.into_iter().map(|(gid, (span, reason))| (gid, span, reason)).collect(), + )) +} + struct ModuleRegistry { context: Rc, linker: Box, @@ -168,6 +290,7 @@ struct ModuleRegistry { external_signatures: FxHashMap, Signature>, #[allow(unused)] referenced_external_signatures: FxHashMap, Signature>, + skipped_procedures: FxHashMap, } impl ModuleRegistry { @@ -176,6 +299,7 @@ impl ModuleRegistry { top_level_modules: Vec, external_signatures: ExternalSignatureMap, external_types: ExternalTypeMap, + initial_skips: Vec<(GlobalItemIndex, SourceSpan, String)>, context: Rc, ) -> Self { let external_signatures = external_signatures @@ -184,7 +308,7 @@ impl ModuleRegistry { (path, Signature::with_convention(&context, ty.abi, ty.params, ty.results)) }) .collect::>(); - Self { + let mut registry = Self { context, linker, top_level_modules, @@ -195,97 +319,181 @@ impl ModuleRegistry { external_types, external_signatures, referenced_external_signatures: FxHashMap::default(), + skipped_procedures: FxHashMap::default(), + }; + for (gid, span, reason) in initial_skips { + registry.skip_item(gid, span, reason); } + registry } - fn infer_missing_signatures(&mut self, config: &DisassemblerConfig) -> Result<()> { + fn infer_missing_signatures(&mut self, config: &LiftConfig) -> Result<()> { let mut visited = FxHashSet::default(); - for root in self.top_level_modules.iter().copied() { + for root in self.top_level_modules.clone() { let root_path = self.linker[root].path().clone(); - for (index, item) in self.linker[root].symbols().enumerate() { - let gid = root + ast::ItemIndex::new(index); - if !item.is_procedure() { - continue; - } - let path = root_path.join(item.name()); - let toposort = self.linker.topological_sort_from_root(gid).map_err(|cycle| { - let iter = cycle.into_node_ids(); - let mut nodes = Vec::with_capacity(iter.len()); - for node in iter { - let module = self.linker[node.module].path(); - let proc = self.linker[node].name(); - nodes.push(format!("{}", module.join(proc))); + let roots = self.linker[root] + .symbols() + .enumerate() + .filter_map(|(index, item)| { + item.is_procedure().then_some(root + ast::ItemIndex::new(index)) + }) + .collect::>(); + for gid in roots { + let path = root_path.join(self.linker[gid].name()); + let toposort = match self.linker.topological_sort_from_root(gid) { + Ok(toposort) => toposort, + Err(cycle) => { + let cycle = cycle.into_node_ids().collect::>(); + let mut nodes = Vec::with_capacity(cycle.len()); + for node in cycle.iter().copied() { + let module = self.linker[node.module].path(); + let proc = self.linker[node].name(); + nodes.push(format!("{}", module.join(proc))); + } + let reason = format!( + "found cycle in call graph rooted at '{path}' involving: {}", + DisplayValues::new(nodes.iter()) + ); + if config.lint { + for node in cycle { + self.skip_item(node, SourceSpan::UNKNOWN, reason.clone()); + } + self.skip_item(gid, SourceSpan::UNKNOWN, reason); + continue; + } + return Err(Report::msg(reason)); } - Report::msg(format!( - "found cycle in call graph rooted at '{path}' involving: {}", - DisplayValues::new(nodes.iter()) - )) - })?; + }; + for gid in toposort.iter().rev().copied() { - if !visited.insert(gid) { + if !visited.insert(gid) || self.skipped_procedures.contains_key(&gid) { continue; } - let item = self.linker[gid].item(); - match item { + match self.linker[gid].item() { SymbolItem::Procedure(p) => { - let p = p.borrow(); - let signature = self.linker.resolve_signature(gid)?; - match signature { - Some(sig) => { - self.signatures.insert( - gid, - Signature::with_convention( + let (span, signature) = { + let p = p.borrow(); + let span = p.span(); + let signature = (|| { + if config.lint { + for invoke in p.invoked() { + let resolution = SymbolResolutionContext { + span: invoke.span(), + module: gid.module, + kind: Some(invoke.kind), + }; + if let SymbolResolution::Exact { gid: callee, .. } = + self.linker.resolve_invoke_target( + &resolution, + &invoke.target, + )? + && let Some(skipped) = + self.skipped_procedures.get(&callee) + { + return Err(Report::msg(format!( + "depends on skipped procedure '{}': {}", + skipped.path, skipped.reason + ))); + } + } + if let Some((inst, span)) = + first_non_liftable_instruction(p.body()) + { + return Err(Report::msg(format!( + "MASM instruction {inst:?} is not supported during \ + disassembly at {span:?}" + ))); + } + let count = estimated_hir_operation_count(p.body()); + if count > LINT_ESTIMATED_HIR_OP_LIMIT { + return Err(Report::msg(format!( + "procedure '{}' is estimated to expand to at least \ + {count} HIR operation(s), exceeding the lint \ + analysis limit of {LINT_ESTIMATED_HIR_OP_LIMIT}", + self.item_path(gid) + ))); + } + } + + let signature = match self.linker.resolve_signature(gid)? { + Some(sig) => Signature::with_convention( &self.context, sig.abi, sig.params.iter().cloned(), sig.results.iter().cloned(), ), - ); - } - None if !config.infer_missing_signatures => { - let path = self.linker[gid.module].path(); - return Err(Report::msg(format!( - "procedure '{}' is missing a signature", - path.join(p.name().as_str()) - ))); - } - None => { - let signature = infer::infer_signature( - gid, - &p, - &self.context, - &self.linker, - &self.signatures, - )?; + None if !config.infer_missing_signatures => { + return Err(Report::msg(format!( + "procedure '{}' is missing a signature", + self.item_path(gid) + ))); + } + None => infer::infer_signature( + gid, + &p, + &self.context, + &self.linker, + &self.signatures, + )?, + }; + + if config.lint { + infer::validate_declared_signature( + gid, + &p, + &self.context, + &self.linker, + &self.signatures, + &signature, + )?; + validate_lint_signature(&self.item_path(gid), &signature)?; + } + + Ok(signature) + })(); + (span, signature) + }; + match signature { + Ok(signature) => { self.signatures.insert(gid, signature); } + Err(err) if config.lint => { + self.skip_item(gid, span, err.to_string()); + } + Err(err) => return Err(err), } } SymbolItem::Compiled(ItemInfo::Procedure(p)) => { - match p.signature.as_deref() { - Some(sig) => { - self.signatures.insert( - gid, - Signature::with_convention( - &self.context, - sig.abi, - sig.params.iter().cloned(), - sig.results.iter().cloned(), - ), - ); + let signature = match p.signature.as_deref() { + Some(sig) => Ok(Signature::with_convention( + &self.context, + sig.abi, + sig.params.iter().cloned(), + sig.results.iter().cloned(), + )), + None => Err(Report::msg(format!( + "compiled procedure '{}' is missing a signature", + self.item_path(gid) + ))), + } + .and_then(|signature| { + if config.lint { + validate_lint_signature(&self.item_path(gid), &signature)?; } - None => { - let path = self.linker[gid.module].path(); - return Err(Report::msg(format!( - "compiled procedure '{}' is missing a signature", - path.join(p.name.as_str()) - ))); + Ok(signature) + }); + match signature { + Ok(signature) => { + self.signatures.insert(gid, signature); } + Err(err) if config.lint => { + self.skip_item(gid, SourceSpan::UNKNOWN, err.to_string()); + } + Err(err) => return Err(err), } } - SymbolItem::Constant(_) | SymbolItem::Type(_) | SymbolItem::Compiled(_) => { - } + SymbolItem::Constant(_) | SymbolItem::Type(_) | SymbolItem::Compiled(_) => {} } } } @@ -294,6 +502,29 @@ impl ModuleRegistry { Ok(()) } + fn item_path(&self, gid: GlobalItemIndex) -> Arc { + self.linker[gid.module] + .path() + .join(self.linker[gid].name()) + .into_boxed_path() + .into() + } + + fn skip_item(&mut self, gid: GlobalItemIndex, span: SourceSpan, reason: String) { + let path = self.item_path(gid); + self.skipped_procedures.entry(gid).or_insert(SkippedProcedure { + path, + span, + reason, + }); + } + + fn skipped_procedures(&self) -> Vec { + let mut skipped = self.skipped_procedures.values().cloned().collect::>(); + skipped.sort_by(|lhs, rhs| lhs.path.as_str().cmp(rhs.path.as_str())); + skipped + } + fn declare_modules(&mut self, world: midenc_hir::dialects::builtin::WorldRef) -> Result<()> { self.world = Some(world); let mut world_builder = WorldBuilder::new(world); @@ -388,6 +619,106 @@ impl ModuleRegistry { } } +fn validate_lint_signature(path: &ast::Path, signature: &Signature) -> Result<()> { + if signature.params().len() > u8::MAX as usize { + return Err(Report::msg(format!( + "procedure '{path}' has {} parameter(s), exceeding the HIR operand limit of {}", + signature.params().len(), + u8::MAX + ))); + } + if signature.results().len() > u8::MAX as usize { + return Err(Report::msg(format!( + "procedure '{path}' returns {} value(s), exceeding the HIR operand limit of {}", + signature.results().len(), + u8::MAX + ))); + } + if signature.params().len() > LINT_SIGNATURE_VALUE_LIMIT { + return Err(Report::msg(format!( + "procedure '{path}' has {} parameter(s), exceeding the lint analysis signature limit \ + of {LINT_SIGNATURE_VALUE_LIMIT}", + signature.params().len() + ))); + } + if signature.results().len() > LINT_SIGNATURE_VALUE_LIMIT { + return Err(Report::msg(format!( + "procedure '{path}' returns {} value(s), exceeding the lint analysis signature limit \ + of {LINT_SIGNATURE_VALUE_LIMIT}", + signature.results().len() + ))); + } + Ok(()) +} + +fn first_non_liftable_instruction(block: &Block) -> Option<(&Instruction, SourceSpan)> { + for op in block.iter() { + match op { + Op::Inst(inst) + if semantics::instruction_semantics(inst.inner()) + != InstructionSemantics::LiftAndInfer => + { + return Some((inst.inner(), inst.span())); + } + Op::Inst(_) => {} + Op::If { + then_blk, else_blk, .. + } => { + if let Some(unsupported) = first_non_liftable_instruction(then_blk) { + return Some(unsupported); + } + if let Some(unsupported) = first_non_liftable_instruction(else_blk) { + return Some(unsupported); + } + } + Op::While { body, .. } | Op::DoWhile { body, .. } | Op::Repeat { body, .. } => { + if let Some(unsupported) = first_non_liftable_instruction(body) { + return Some(unsupported); + } + } + } + } + None +} + +fn estimated_hir_operation_count(block: &Block) -> usize { + estimated_block_hir_operation_count(block, LINT_ESTIMATED_HIR_OP_LIMIT.saturating_add(1)) +} + +fn estimated_block_hir_operation_count(block: &Block, cap: usize) -> usize { + let mut total = 0usize; + for op in block.iter() { + total = total.saturating_add(estimated_op_hir_operation_count(op, cap)); + if total >= cap { + return cap; + } + } + total +} + +fn estimated_op_hir_operation_count(op: &Op, cap: usize) -> usize { + match op { + Op::Inst(_) => 4, + Op::If { + then_blk, else_blk, .. + } => 1usize + .saturating_add(estimated_block_hir_operation_count(then_blk, cap)) + .saturating_add(estimated_block_hir_operation_count(else_blk, cap)) + .min(cap), + Op::While { body, .. } | Op::DoWhile { body, .. } => { + 1usize.saturating_add(estimated_block_hir_operation_count(body, cap)).min(cap) + } + Op::Repeat { count, body, .. } => match count { + Immediate::Value(count) => estimated_block_hir_operation_count(body, cap) + .saturating_mul(count.into_inner() as usize) + .min(cap), + Immediate::Constant(_) => { + 1usize.saturating_add(estimated_block_hir_operation_count(body, cap)).min(cap) + } + }, + } +} + fn masm_module_symbol_path(path: &ast::Path) -> SymbolPath { let path = path.as_str().strip_prefix("::").unwrap_or(path.as_str()); SymbolPath::from_masm_module_id(path) @@ -451,6 +782,15 @@ impl<'a> ProcedureLifter<'a> { self.stack.len() ))); } + if results.len() > u8::MAX as usize { + return Err(Report::msg(format!( + "procedure '{}::{}' returns {} value(s), exceeding the HIR operand limit of {}", + self.registry.linker[self.item.module].path(), + self.procedure.name(), + results.len(), + u8::MAX + ))); + } builder.ret(results, self.procedure.span())?; Ok(()) } @@ -1131,6 +1471,16 @@ impl<'a> ProcedureLifter<'a> { Exp => self.binary_with_type(builder, Type::Felt, span, |builder, lhs, rhs, span| { builder.exp(lhs, rhs, span) }), + ExpBitLength(32) => { + let exponent = self.pop(span)?; + let base = self.pop(span)?; + let base = self.cast(builder, base.value, Type::Felt, span)?; + let exponent = self.cast(builder, exponent.value, Type::U32, span)?; + let exponent = self.cast(builder, exponent, Type::Felt, span)?; + let result = builder.exp_u32_exponent(base, exponent, span)?; + self.push_value(result, span); + Ok(()) + } ExpImm(value) => { self.felt_binary_imm(builder, value, span, |builder, lhs, rhs, span| { builder.exp(lhs, rhs, span) @@ -2521,9 +2871,7 @@ impl<'a> ProcedureLifter<'a> { } fn pop(&mut self, span: SourceSpan) -> Result { - self.stack - .pop() - .ok_or_else(|| Report::msg(format!("stack underflow at {span:?}"))) + self.stack.pop().ok_or_else(|| self.stack_underflow(span)) } fn pop_binary(&mut self, span: SourceSpan) -> Result<(StackValue, StackValue)> { @@ -2540,15 +2888,15 @@ impl<'a> ProcedureLifter<'a> { } fn dup(&mut self, depth: usize, span: SourceSpan) -> Result<()> { - masm_stack::dup(&mut self.stack, depth).ok_or_else(|| stack_underflow(span)) + masm_stack::dup(&mut self.stack, depth).ok_or_else(|| self.stack_underflow(span)) } fn dup_word(&mut self, depth: usize, span: SourceSpan) -> Result<()> { - masm_stack::dup_word(&mut self.stack, depth).ok_or_else(|| stack_underflow(span)) + masm_stack::dup_word(&mut self.stack, depth).ok_or_else(|| self.stack_underflow(span)) } fn swap(&mut self, depth: usize, span: SourceSpan) -> Result<()> { - masm_stack::swap(&mut self.stack, depth).ok_or_else(|| stack_underflow(span)) + masm_stack::swap(&mut self.stack, depth).ok_or_else(|| self.stack_underflow(span)) } fn swap_word(&mut self, depth: usize, span: SourceSpan) -> Result<()> { @@ -2561,11 +2909,11 @@ impl<'a> ProcedureLifter<'a> { fn swap_chunks(&mut self, chunk_len: usize, depth: usize, span: SourceSpan) -> Result<()> { masm_stack::swap_chunks(&mut self.stack, chunk_len, depth) - .ok_or_else(|| stack_underflow(span)) + .ok_or_else(|| self.stack_underflow(span)) } fn movup(&mut self, depth: usize, span: SourceSpan) -> Result<()> { - masm_stack::movup(&mut self.stack, depth).ok_or_else(|| stack_underflow(span)) + masm_stack::movup(&mut self.stack, depth).ok_or_else(|| self.stack_underflow(span)) } fn movup_word(&mut self, depth: usize, span: SourceSpan) -> Result<()> { @@ -2579,11 +2927,11 @@ impl<'a> ProcedureLifter<'a> { span: SourceSpan, ) -> Result<()> { masm_stack::move_chunk_to_top(&mut self.stack, chunk_len, depth) - .ok_or_else(|| stack_underflow(span)) + .ok_or_else(|| self.stack_underflow(span)) } fn movdn(&mut self, depth: usize, span: SourceSpan) -> Result<()> { - masm_stack::movdn(&mut self.stack, depth).ok_or_else(|| stack_underflow(span)) + masm_stack::movdn(&mut self.stack, depth).ok_or_else(|| self.stack_underflow(span)) } fn movdn_word(&mut self, depth: usize, span: SourceSpan) -> Result<()> { @@ -2597,15 +2945,15 @@ impl<'a> ProcedureLifter<'a> { span: SourceSpan, ) -> Result<()> { masm_stack::move_top_chunk_down(&mut self.stack, chunk_len, depth) - .ok_or_else(|| stack_underflow(span)) + .ok_or_else(|| self.stack_underflow(span)) } fn reverse_word(&mut self, span: SourceSpan) -> Result<()> { - masm_stack::reverse_n(&mut self.stack, 4).ok_or_else(|| stack_underflow(span)) + masm_stack::reverse_n(&mut self.stack, 4).ok_or_else(|| self.stack_underflow(span)) } fn reverse_double_word(&mut self, span: SourceSpan) -> Result<()> { - masm_stack::reverse_n(&mut self.stack, 8).ok_or_else(|| stack_underflow(span)) + masm_stack::reverse_n(&mut self.stack, 8).ok_or_else(|| self.stack_underflow(span)) } fn pop_word(&mut self, span: SourceSpan) -> Result> { @@ -2649,16 +2997,32 @@ impl<'a> ProcedureLifter<'a> { } fn pop_chunk(&mut self, chunk_len: usize, span: SourceSpan) -> Result> { - masm_stack::pop_chunk(&mut self.stack, chunk_len).ok_or_else(|| stack_underflow(span)) + masm_stack::pop_chunk(&mut self.stack, chunk_len).ok_or_else(|| self.stack_underflow(span)) } fn require_depth(&self, depth: usize, span: SourceSpan) -> Result<()> { if self.stack.len() <= depth { - Err(stack_underflow(span)) + Err(self.stack_underflow(span)) } else { Ok(()) } } + + fn stack_underflow(&self, span: SourceSpan) -> Report { + Report::msg(format!( + "stack underflow in '{}::{}' at {}", + self.registry.linker[self.item.module].path(), + self.procedure.name(), + self.format_span(span) + )) + } + + fn format_span(&self, span: SourceSpan) -> String { + match self.registry.context.session().source_manager.file_line_col(span) { + Ok(location) => format!("{location}"), + Err(_) => format!("{span:?}"), + } + } } #[derive(Clone, Copy, Debug, Eq, PartialEq)] @@ -2751,10 +3115,6 @@ fn unsupported_instruction(inst: &Instruction, span: SourceSpan) -> Result<()> { ))) } -fn stack_underflow(span: SourceSpan) -> miden_assembly_syntax::diagnostics::Report { - Report::msg(format!("stack underflow at {span:?}")) -} - fn immediate_u32(immediate: &Immediate) -> Result { match immediate { Immediate::Value(value) => Ok(value.into_inner()), diff --git a/frontend/masm/src/semantics.rs b/frontend/masm/src/semantics.rs index 08809e756..9c14068c1 100644 --- a/frontend/masm/src/semantics.rs +++ b/frontend/masm/src/semantics.rs @@ -25,7 +25,7 @@ macro_rules! define_instruction_semantics { ) => { #[cfg(test)] pub(crate) const LIFT_AND_INFER_INSTRUCTION_VARIANT_COUNT: usize = - count_instruction_patterns!($($lift_and_infer),*); + count_instruction_patterns!($($lift_and_infer),*) + 1; #[cfg(test)] pub(crate) const INFER_ONLY_INSTRUCTION_VARIANT_COUNT: usize = count_instruction_patterns!($($infer_only),*); @@ -35,6 +35,8 @@ macro_rules! define_instruction_semantics { pub(crate) fn instruction_semantics(instruction: &Instruction) -> InstructionSemantics { match instruction { + Instruction::ExpBitLength(32) => InstructionSemantics::LiftAndInfer, + Instruction::ExpBitLength(_) => InstructionSemantics::Unsupported, $($lift_and_infer => InstructionSemantics::LiftAndInfer,)* $($infer_only => InstructionSemantics::InferOnly,)* $($unsupported => InstructionSemantics::Unsupported,)* @@ -287,7 +289,6 @@ define_instruction_semantics! { Instruction::ProcRef(_), ], unsupported: [ - Instruction::ExpBitLength(_), Instruction::DynExec, Instruction::DynCall, ], diff --git a/frontend/masm/src/tests.rs b/frontend/masm/src/tests.rs index 0ee5cfc38..1bcee3427 100644 --- a/frontend/masm/src/tests.rs +++ b/frontend/masm/src/tests.rs @@ -8,15 +8,18 @@ use std::{ }; use miden_assembly::{ - Assembler, + Assembler, ProjectSourceInputs, ast::ItemIndex, linker::{Linker, SymbolItem}, }; -use miden_assembly_syntax::ast::{self, Instruction}; +use miden_assembly_syntax::{ + ast::{self, Instruction}, + debuginfo::SourceSpan, +}; use miden_package_registry::{ NoPackageStore, PackageId, PackageRecord, PackageRegistry, PackageVersions, Version, }; -use miden_project::ProjectDependencyGraphBuilder; +use miden_project::{Project, ProjectDependencyGraphBuilder}; use midenc_dialect_arith as arith; use midenc_dialect_cf as cf; use midenc_dialect_hir::{ @@ -38,6 +41,7 @@ use midenc_hir::{ effects::AdviceEffect, pass::AnalysisManager, }; +use midenc_hir_analysis::DataFlowConfig; use super::*; use crate::semantics::{ @@ -175,6 +179,7 @@ end "test", &DisassemblerConfig { infer_missing_signatures: true, + ..Default::default() }, context, ); @@ -347,6 +352,7 @@ fn supported_instruction_matrix_lifts() { felt_instruction_case("incr", 1, 1, "add.1"), felt_instruction_case("pow2", 1, 1, "pow2"), felt_instruction_case("exp", 2, 1, "exp"), + instruction_case("exp_u32", &["felt", "u32"], &["felt"], "exp.u32"), felt_instruction_case("exp_imm", 1, 1, "exp.2"), instruction_case("not", &["i1"], &["i1"], "not"), instruction_case("and", &["i1", "i1"], &["i1"], "and"), @@ -950,7 +956,7 @@ fn unsupported_instruction_matrix_reports_diagnostics() { let cases = [ unsupported_instruction_case("dynexec", 0, "dynexec"), unsupported_instruction_case("dyncall", 0, "dyncall"), - unsupported_instruction_case("exp_bit_length", 2, "exp.u32"), + unsupported_instruction_case("exp_u8", 0, "exp.u8"), ]; for case in &cases { @@ -960,9 +966,9 @@ fn unsupported_instruction_matrix_reports_diagnostics() { #[test] fn instruction_inventory_classifies_all_masm_instruction_variants() { - assert_eq!(LIFT_AND_INFER_INSTRUCTION_VARIANT_COUNT, 237); + assert_eq!(LIFT_AND_INFER_INSTRUCTION_VARIANT_COUNT, 238); assert_eq!(INFER_ONLY_INSTRUCTION_VARIANT_COUNT, 1); - assert_eq!(UNSUPPORTED_INSTRUCTION_VARIANT_COUNT, 3); + assert_eq!(UNSUPPORTED_INSTRUCTION_VARIANT_COUNT, 2); assert_eq!( LIFT_AND_INFER_INSTRUCTION_VARIANT_COUNT + INFER_ONLY_INSTRUCTION_VARIANT_COUNT @@ -976,6 +982,14 @@ fn instruction_inventory_classifies_all_masm_instruction_variants() { )), InstructionSemantics::InferOnly ); + assert_eq!( + instruction_semantics(&Instruction::ExpBitLength(32)), + InstructionSemantics::LiftAndInfer + ); + assert_eq!( + instruction_semantics(&Instruction::ExpBitLength(8)), + InstructionSemantics::Unsupported + ); } #[test] @@ -990,6 +1004,7 @@ end "test", &DisassemblerConfig { infer_missing_signatures: true, + ..Default::default() }, context, )?; @@ -1019,6 +1034,7 @@ end "test", &DisassemblerConfig { infer_missing_signatures: true, + ..Default::default() }, context, )?; @@ -1045,6 +1061,7 @@ end "test", &DisassemblerConfig { infer_missing_signatures: true, + ..Default::default() }, context, )?; @@ -1078,6 +1095,7 @@ end "test", &DisassemblerConfig { infer_missing_signatures: true, + ..Default::default() }, context, )?; @@ -1124,6 +1142,7 @@ end "test", &DisassemblerConfig { infer_missing_signatures: true, + ..Default::default() }, context, )?; @@ -1167,6 +1186,7 @@ end "test", &DisassemblerConfig { infer_missing_signatures: true, + ..Default::default() }, context, )?; @@ -1205,6 +1225,7 @@ end "test", &DisassemblerConfig { infer_missing_signatures: true, + ..Default::default() }, context, )?; @@ -1253,6 +1274,7 @@ end "test", &DisassemblerConfig { infer_missing_signatures: true, + ..Default::default() }, context, )?; @@ -1284,6 +1306,7 @@ end "test", &DisassemblerConfig { infer_missing_signatures: true, + ..Default::default() }, context, )?; @@ -1312,6 +1335,7 @@ end "test", &DisassemblerConfig { infer_missing_signatures: true, + ..Default::default() }, context, )?; @@ -1354,6 +1378,7 @@ end "test", &DisassemblerConfig { infer_missing_signatures: true, + ..Default::default() }, context, )?; @@ -1462,6 +1487,7 @@ end "test", &DisassemblerConfig { infer_missing_signatures: true, + ..Default::default() }, context, )?; @@ -1497,6 +1523,7 @@ end "test", &DisassemblerConfig { infer_missing_signatures: true, + ..Default::default() }, context, )?; @@ -1525,6 +1552,7 @@ end "test", &DisassemblerConfig { infer_missing_signatures: true, + ..Default::default() }, context, )?; @@ -1554,6 +1582,7 @@ end "test", &DisassemblerConfig { infer_missing_signatures: true, + ..Default::default() }, context, )?; @@ -1580,6 +1609,7 @@ end "test", &DisassemblerConfig { infer_missing_signatures: true, + ..Default::default() }, context, )?; @@ -1624,6 +1654,7 @@ end "test", &DisassemblerConfig { infer_missing_signatures: true, + ..Default::default() }, context, )?; @@ -1669,6 +1700,7 @@ end "test", &DisassemblerConfig { infer_missing_signatures: true, + ..Default::default() }, context, )?; @@ -1700,6 +1732,7 @@ end "test", &DisassemblerConfig { infer_missing_signatures: true, + ..Default::default() }, context, )?; @@ -1757,6 +1790,7 @@ end "test", &DisassemblerConfig { infer_missing_signatures: true, + ..Default::default() }, context, )?; @@ -1816,6 +1850,7 @@ end "test", &DisassemblerConfig { infer_missing_signatures: true, + ..Default::default() }, context, )?; @@ -1871,6 +1906,7 @@ end "test", &DisassemblerConfig { infer_missing_signatures: true, + ..Default::default() }, context, )?; @@ -1926,6 +1962,7 @@ end "test", &DisassemblerConfig { infer_missing_signatures: true, + ..Default::default() }, context, )?; @@ -2021,6 +2058,7 @@ end "test", &DisassemblerConfig { infer_missing_signatures: true, + ..Default::default() }, &external_signatures, context, @@ -2133,6 +2171,7 @@ end "test", &DisassemblerConfig { infer_missing_signatures: true, + ..Default::default() }, &external_signatures, context, @@ -2168,10 +2207,68 @@ end Ok(()) } +#[test] +fn known_signature_array_alias_preserves_first_class_stack_value() -> Result<()> { + let context = Rc::new(Context::default()); + let output = disassemble_source( + r#" +pub type Caller = [felt; 4] + +pub proc get_caller() -> Caller + caller +end +"#, + "test", + &DisassemblerConfig::default(), + context, + )?; + + let signature = find_function(output.module, "get_caller").borrow().get_signature().clone(); + assert_eq!(signature.params().len(), 0); + assert_eq!(signature.results().len(), 1); + assert_eq!(signature.results()[0].ty, Type::from(ArrayType::new(Type::Felt, 4))); + + Ok(()) +} + Ok(()) +} + +#[test] +fn external_hir_array_signature_preserves_first_class_stack_value() -> Result<()> { + let context = Rc::new(Context::default()); + let array = Type::from(ArrayType::new(Type::Felt, 4)); + let mut external_signatures = ExternalSignatureMap::new(); + external_signatures + .insert(ast::Path::new("::dep::api::callee").into(), masm_signature([], [array.clone()])); + + let output = disassemble_source_with_external_signatures( + r#" +pub type Caller = [felt; 4] + +pub proc entry() -> Caller + exec.::dep::api::callee +end +"#, + "test", + &DisassemblerConfig::default(), + &external_signatures, + context, + )?; + + let dep_api = find_world_module(output.world, "dep::api"); + let callee = find_function(dep_api, "callee"); + let signature = callee.borrow().get_signature().clone(); + assert_eq!(signature.params().len(), 0); + assert_eq!(signature.results().len(), 1); + assert_eq!(signature.results()[0].ty, array); + + Ok(()) +} #[test] -fn project_disassembly_uses_source_dependency_signatures() -> Result<()> { - let (root, app_dir) = write_source_dependency_project("midenc_frontend_masm_source_dep"); +fn project_disassembly_uses_source_dependency_module_index_metadata() -> Result<()> { + let (root, app_dir) = + write_source_dependency_module_index_project("midenc_frontend_masm_source_dep_index"); let context = Rc::new(Context::default()); let output = disassemble_project_target_from_path( @@ -2224,6 +2321,212 @@ fn project_disassembly_loads_support_modules_into_world_tree() -> Result<()> { Ok(()) } +#[test] +fn project_disassembly_accepts_module_index_roots() -> Result<()> { + let (root, app_dir) = write_module_index_project("midenc_frontend_masm_module_index"); + + let context = Rc::new(Context::default()); + let output = disassemble_project_target_from_path( + app_dir.join("miden-project.toml"), + None, + &DisassemblerConfig::default(), + context, + )?; + + let child = find_world_module(output.world, "app::child"); + let leaf = find_world_module(output.world, "app::child::leaf"); + let child_function = find_function(child, "double"); + let signature = child_function.borrow().get_signature().clone(); + assert_eq!(signature.params().len(), 1); + assert_eq!(signature.params()[0].ty, Type::Felt); + assert_eq!(signature.results().len(), 1); + assert_eq!(signature.results()[0].ty, Type::Felt); + + let function = find_function(leaf, "inc"); + let signature = function.borrow().get_signature().clone(); + assert_eq!(signature.params().len(), 1); + assert_eq!(signature.params()[0].ty, Type::Felt); + assert_eq!(signature.results().len(), 1); + assert_eq!(signature.results()[0].ty, Type::Felt); + assert!(child.borrow().get(SymbolName::intern("leaf")).is_some()); + + let entry_block = child_function.borrow().entry_block(); + let entry_block = entry_block.borrow(); + let mut callee = None; + for op in entry_block.body().iter() { + if let Some(op) = op.as_trait::() { + callee = Some(op.resolve().expect("re-exported leaf callee should resolve")); + break; + } + } + let callee = callee.expect("child::double should call through the public re-export"); + assert_eq!(callee.borrow().name().as_str(), "inc"); + + let _ = fs::remove_dir_all(root); + + Ok(()) +} + +#[test] +fn project_disassembly_accepts_private_child_module_declaration() -> Result<()> { + let (root, app_dir) = + write_private_child_module_index_project("midenc_frontend_masm_private_child_index"); + + let context = Rc::new(Context::default()); + let output = disassemble_project_target_from_path( + app_dir.join("miden-project.toml"), + None, + &DisassemblerConfig::default(), + context, + )?; + + let function = find_function(output.module, "call_secret"); + assert_eq!(top_level_op_count::(function), 1); + + let _ = fs::remove_dir_all(root); + + Ok(()) +} + +#[test] +fn lint_project_disassembly_with_preparsed_sources_does_not_read_target_path() -> Result<()> { + let root = temp_project_dir("midenc_frontend_masm_lint_preparsed_index"); + let app_dir = root.join("app"); + fs::create_dir_all(&app_dir).unwrap(); + fs::write( + app_dir.join("miden-project.toml"), + r#"[package] +name = "app" +version = "0.0.1" + +[lib] +path = "missing.masm" +"#, + ) + .unwrap(); + let context = Rc::new(Context::default()); + let manifest_path = app_dir.join("miden-project.toml"); + let project = Project::load(&manifest_path, &context.session().source_manager)?; + let app = parse_test_module_with_path( + r#" +pub proc entry() -> felt + push.7 +end +"#, + "app", + &context, + )?; + let output = disassemble_project_target_for_lint( + &project, + None, + Some(ProjectSourceInputs { + root: app, + support: vec![], + }), + &DisassemblerConfig::default(), + context, + )?; + + let _ = find_function(output.module, "entry"); + assert!(output.skipped_procedures.is_empty()); + + let _ = fs::remove_dir_all(root); + + Ok(()) +} + +#[test] +fn lint_project_disassembly_with_resolved_input_keeps_module_index_metadata() -> Result<()> { + let (root, app_dir) = + write_private_child_module_index_project("midenc_frontend_masm_lint_resolved_index"); + + let context = Rc::new(Context::default()); + let manifest_path = app_dir.join("miden-project.toml"); + let project = Project::load(&manifest_path, &context.session().source_manager)?; + let target = project::resolve_project_target(&project, None, &context)?; + let output = + disassemble_project_target_input_for_lint(target, &DisassemblerConfig::default(), context)?; + + let function = find_function(output.module, "call_secret"); + assert_eq!(top_level_op_count::(function), 1); + assert!(output.skipped_procedures.is_empty()); + + let _ = fs::remove_dir_all(root); + + Ok(()) +} + +#[test] +fn project_disassembly_does_not_fallback_to_undeclared_child_procedure() -> Result<()> { + let (root, app_dir) = + write_undeclared_child_module_index_project("midenc_frontend_masm_undeclared_child_index"); + + let context = Rc::new(Context::default()); + let err = match disassemble_project_target_from_path( + app_dir.join("miden-project.toml"), + None, + &DisassemblerConfig::default(), + context, + ) { + Ok(_) => panic!("undeclared child procedure should not resolve through fallback"), + Err(err) => err, + }; + + let message = err.to_string(); + assert!(message.contains("leaf::secret")); + assert!(message.contains("failed to resolve") || message.contains("could not resolve")); + + let _ = fs::remove_dir_all(root); + + Ok(()) +} + +#[test] +fn project_disassembly_does_not_strip_invalid_module_declarations() -> Result<()> { + let root = temp_project_dir("midenc_frontend_masm_invalid_module_declaration"); + let app_dir = root.join("app"); + fs::create_dir_all(&app_dir).unwrap(); + fs::write( + app_dir.join("miden-project.toml"), + r#"[package] +name = "app" +version = "0.0.1" + +[lib] +path = "mod.masm" +"#, + ) + .unwrap(); + fs::write( + app_dir.join("mod.masm"), + r#" +mod child:: + +pub proc entry() -> felt + push.1 +end +"#, + ) + .unwrap(); + + let context = Rc::new(Context::default()); + let err = match disassemble_project_target_from_path( + app_dir.join("miden-project.toml"), + None, + &DisassemblerConfig::default(), + context, + ) { + Ok(_) => panic!("invalid module declaration should be reported by the parser"), + Err(err) => err, + }; + + assert!(err.to_string().contains("invalid syntax")); + + let _ = fs::remove_dir_all(root); + + Ok(()) +} + #[test] fn project_dependency_graph_resolves_imported_external_type_metadata() -> Result<()> { let (root, app_dir) = @@ -2711,6 +3014,7 @@ end "test", &DisassemblerConfig { infer_missing_signatures: true, + ..Default::default() }, context, ) { @@ -2899,6 +3203,24 @@ end Ok(()) } +#[test] +fn advice_taint_reports_raw_advice_used_by_exp_u32_exponent() -> Result<()> { + let findings = advice_taint_findings_for_source( + r#" +pub proc entry() -> felt + push.3 + adv_push + exp.u32 +end +"#, + )?; + + assert_eq!(sink_names(&findings), ["arith.exp"]); + assert_eq!(findings[0].function.map(|name| name.as_str()), Some("entry")); + + Ok(()) +} + #[test] fn advice_taint_diagnostics_include_actionable_context() -> Result<()> { let context = Rc::new(Context::default()); @@ -3737,24 +4059,18 @@ end } #[test] -fn advice_taint_keeps_nested_passthrough_call_results_call_site_specific() -> Result<()> { +fn advice_taint_handles_duplicate_call_return_values() -> Result<()> { let context = Rc::new(Context::default()); let output = disassemble_source( r#" -proc passthrough(value: felt) -> felt - nop -end - -proc outer(value: felt) -> felt - exec.passthrough +proc duplicate(value: u32) -> (u32, u32) + dup end pub proc entry(rhs: u32) -> u32 adv_push - exec.outer + exec.duplicate drop - push.0 - exec.outer u32wrapping_add end "#, @@ -3764,48 +4080,162 @@ end )?; let findings = advice_taint_findings(output.module)?; - assert!(findings.is_empty(), "{findings:#?}"); + assert_eq!(sink_names(&findings), ["arith.add"]); Ok(()) } #[test] -fn advice_taint_does_not_taint_external_result_without_advice_effects() -> Result<()> { +fn advice_taint_reports_worklist_budget_exhaustion() -> Result<()> { let context = Rc::new(Context::default()); - let mut external_signatures = ExternalSignatureMap::new(); - external_signatures - .insert(ast::Path::new("::dep::source").into(), masm_signature([], [Type::Felt])); - - let output = disassemble_source_with_external_signatures( + let output = disassemble_source( r#" +proc duplicate(value: u32) -> (u32, u32) + dup +end + pub proc entry(rhs: u32) -> u32 - exec.::dep::source + adv_push + exec.duplicate + drop u32wrapping_add end "#, "test", &DisassemblerConfig::default(), - &external_signatures, context, )?; - let findings = advice_taint_findings(output.module)?; - assert!(findings.is_empty(), "{findings:#?}"); + let analysis_manager = AnalysisManager::new(output.module.as_operation_ref(), None); + let mut config = DataFlowConfig::new(); + config.set_interprocedural(true).set_max_worklist_iterations(Some(0)); + let module = output.module.borrow(); + let err = + match AdviceTaintAnalysis::run_with_config(module.as_operation(), analysis_manager, config) + { + Ok(_) => panic!("expected advice taint analysis to report worklist budget exhaustion"), + Err(err) => err, + }; - let diagnostics = advice_taint_diagnostics(output.module)?; - assert!(diagnostics.is_empty(), "{diagnostics:#?}"); + let err = err.to_string(); + assert!(err.contains("dataflow solver exceeded worklist iteration budget")); + assert!(err.contains("queued analyses:")); Ok(()) } #[test] -fn advice_taint_does_not_taint_external_result_for_non_read_advice_effects() -> Result<()> { +fn advice_taint_partial_run_reports_budget_exhaustion() -> Result<()> { let context = Rc::new(Context::default()); - let mut external_signatures = ExternalSignatureMap::new(); - external_signatures - .insert(ast::Path::new("::dep::source").into(), masm_signature([], [Type::Felt])); - - let output = disassemble_source_with_external_signatures( + let output = disassemble_source( + r#" +proc duplicate(value: u32) -> (u32, u32) + dup +end + +pub proc entry(rhs: u32) -> u32 + adv_push + exec.duplicate + drop + u32wrapping_add +end +"#, + "test", + &DisassemblerConfig::default(), + context, + )?; + + let analysis_manager = AnalysisManager::new(output.module.as_operation_ref(), None); + let mut config = DataFlowConfig::new(); + config.set_interprocedural(true).set_max_worklist_iterations(Some(0)); + let module = output.module.borrow(); + let result = AdviceTaintAnalysis::run_with_config_allow_partial( + module.as_operation(), + analysis_manager, + config, + )?; + + let reason = result + .incomplete_reason + .expect("expected partial advice taint analysis to report budget exhaustion"); + assert!(reason.contains("dataflow solver exceeded worklist iteration budget")); + assert!(reason.contains("queued analyses:")); + let source_manager = output.world.borrow().as_operation().context().source_manager(); + let _ = result.analysis.diagnostics(&source_manager); + + Ok(()) +} + +#[test] +fn advice_taint_keeps_nested_passthrough_call_results_call_site_specific() -> Result<()> { + let context = Rc::new(Context::default()); + let output = disassemble_source( + r#" +proc passthrough(value: felt) -> felt + nop +end + +proc outer(value: felt) -> felt + exec.passthrough +end + +pub proc entry(rhs: u32) -> u32 + adv_push + exec.outer + drop + push.0 + exec.outer + u32wrapping_add +end +"#, + "test", + &DisassemblerConfig::default(), + context, + )?; + + let findings = advice_taint_findings(output.module)?; + assert!(findings.is_empty(), "{findings:#?}"); + + Ok(()) +} + +#[test] +fn advice_taint_does_not_taint_external_result_without_advice_effects() -> Result<()> { + let context = Rc::new(Context::default()); + let mut external_signatures = ExternalSignatureMap::new(); + external_signatures + .insert(ast::Path::new("::dep::source").into(), masm_signature([], [Type::Felt])); + + let output = disassemble_source_with_external_signatures( + r#" +pub proc entry(rhs: u32) -> u32 + exec.::dep::source + u32wrapping_add +end +"#, + "test", + &DisassemblerConfig::default(), + &external_signatures, + context, + )?; + + let findings = advice_taint_findings(output.module)?; + assert!(findings.is_empty(), "{findings:#?}"); + + let diagnostics = advice_taint_diagnostics(output.module)?; + assert!(diagnostics.is_empty(), "{diagnostics:#?}"); + + Ok(()) +} + +#[test] +fn advice_taint_does_not_taint_external_result_for_non_read_advice_effects() -> Result<()> { + let context = Rc::new(Context::default()); + let mut external_signatures = ExternalSignatureMap::new(); + external_signatures + .insert(ast::Path::new("::dep::source").into(), masm_signature([], [Type::Felt])); + + let output = disassemble_source_with_external_signatures( r#" pub proc entry(rhs: u32) -> u32 exec.::dep::source @@ -3954,6 +4384,7 @@ end "test", &DisassemblerConfig { infer_missing_signatures: true, + ..Default::default() }, &external_signatures, context, @@ -3995,6 +4426,7 @@ end "test", &DisassemblerConfig { infer_missing_signatures: true, + ..Default::default() }, &external_signatures, context, @@ -4085,6 +4517,34 @@ end Ok(()) } +#[test] +fn advice_taint_deduplicates_public_return_origins_for_same_result() -> Result<()> { + let context = Rc::new(Context::default()); + let output = disassemble_source( + r#" +pub proc entry() -> felt + adv_push + adv_push + add +end +"#, + "test", + &DisassemblerConfig::default(), + context, + )?; + + let exits = advice_taint_exit_findings(output.module)?; + assert_eq!(exits.len(), 1, "{exits:#?}"); + assert_eq!(exits[0].function.as_str(), "entry"); + assert_eq!(exits[0].result_index, 0); + assert_eq!(exits[0].origin.kind, AdviceTaintOriginKind::Advice); + + let diagnostics = advice_taint_diagnostics(output.module)?; + assert_eq!(diagnostics.len(), 1, "{diagnostics:#?}"); + + Ok(()) +} + #[test] fn advice_taint_does_not_report_private_function_returning_raw_advice() -> Result<()> { let context = Rc::new(Context::default()); @@ -4733,6 +5193,7 @@ end "test", &DisassemblerConfig { infer_missing_signatures: true, + ..Default::default() }, context, ); @@ -4770,107 +5231,671 @@ end } #[test] -fn rejects_mutual_recursion() -> Result<()> { +fn strict_disassembly_still_errors_on_lint_branch_depth_fixture() { let context = Rc::new(Context::default()); - let Err(err) = disassemble_source( + let result = disassemble_source( r#" -pub proc a() -> felt - exec.b -end - -pub proc b() -> felt - exec.a +pub proc bad + push.1 + if.true + push.1 + else + push.1 + push.2 + end end "#, "test", - &DisassemblerConfig::default(), + &DisassemblerConfig { + infer_missing_signatures: true, + ..Default::default() + }, context, - ) else { - panic!("expected disassembly with mututal recursion to fail"); + ); + let err = match result { + Ok(_) => panic!("expected strict disassembly to reject mismatched branch stack depths"), + Err(err) => err, }; - assert!(err.to_string().contains("found a cycle in the call graph")); - Ok(()) + assert!(err.to_string().contains("if branches leave different inferred stack depths")); } -struct InstructionCase { - name: String, - locals: usize, - params: Vec<&'static str>, - results: Vec<&'static str>, - body: String, -} +#[test] +fn lint_disassembly_skips_branch_depth_mismatch_procedure() -> Result<()> { + let context = Rc::new(Context::default()); + let output = disassemble_source_for_lint( + r#" +pub proc good + push.1 +end -fn felt_instruction_case( - name: impl Into, - params: usize, - results: usize, - body: impl Into, -) -> InstructionCase { - instruction_case(name, &felt_types(params), &felt_types(results), body) -} +pub proc bad + push.1 + if.true + push.1 + else + push.1 + push.2 + end +end +"#, + "test", + &DisassemblerConfig { + infer_missing_signatures: true, + }, + context, + )?; -fn instruction_case( - name: impl Into, - params: &[&'static str], - results: &[&'static str], - body: impl Into, -) -> InstructionCase { - instruction_case_with_locals(name, 0, params, results, body) -} + let _ = find_function(output.module, "good"); + assert!(!module_has_function(output.module, "bad")); + assert_eq!(output.skipped_procedures.len(), 1); + let skipped = &output.skipped_procedures[0]; + assert_eq!(skipped.path.as_str(), "::test::bad"); + assert_ne!(skipped.span, SourceSpan::UNKNOWN); + assert!(skipped.reason.contains("if branches leave different inferred stack depths")); -fn instruction_case_with_locals( - name: impl Into, - locals: usize, - params: &[&'static str], - results: &[&'static str], - body: impl Into, -) -> InstructionCase { - InstructionCase { - name: name.into(), - locals, - params: params.to_vec(), - results: results.to_vec(), - body: body.into(), - } + Ok(()) } -fn unsupported_instruction_case( - name: impl Into, - locals: usize, - body: impl Into, -) -> InstructionCase { - instruction_case_with_locals(name, locals, &[], &[], body) -} +#[test] +fn lint_disassembly_skips_known_signature_stack_shape_mismatch() -> Result<()> { + let context = Rc::new(Context::default()); + let output = disassemble_source_for_lint( + r#" +pub proc good() -> felt + push.1 +end -fn parse_test_module( - source: &str, - context: &Rc, -) -> Result> { - miden_assembly::ModuleParser::new(Some(ast::ModuleKind::Library)).parse_str( - Some(ast::Path::new("test")), - source, - context.source_manager(), - ) -} +pub proc bad(cond: i1) -> felt + if.true + push.1 + else + push.1 + push.2 + end +end +"#, + "test", + &DisassemblerConfig { + ..Default::default() + }, + context, + )?; -fn felt_types(count: usize) -> Vec<&'static str> { - vec!["felt"; count] -} + let _ = find_function(output.module, "good"); + assert!(!module_has_function(output.module, "bad")); + assert_eq!(output.skipped_procedures.len(), 1); + let skipped = &output.skipped_procedures[0]; + assert_eq!(skipped.path.as_str(), "::test::bad"); + assert!(skipped.reason.contains("if branches leave different inferred stack depths")); -fn felt_word_select_params() -> Vec<&'static str> { - let mut params = Vec::with_capacity(9); - params.push("i1"); - params.extend(felt_types(8)); - params + Ok(()) } -fn u32_types(count: usize) -> Vec<&'static str> { - vec!["u32"; count] -} +#[test] +fn lint_single_module_skips_missing_external_after_core_metadata_prescan() -> Result<()> { + let context = Rc::new(Context::default()); + let output = disassemble_source_for_lint( + r#" +pub proc good() -> felt + push.1 +end -fn felt_memory_pointer_type() -> Type { - Type::from(PointerType::new_with_address_space(Type::Felt, AddressSpace::Element)) +pub proc bad(value: felt) -> felt + exec.::dep::missing +end +"#, + "test", + &DisassemblerConfig { + ..Default::default() + }, + context, + )?; + + let _ = find_function(output.module, "good"); + assert!(!module_has_function(output.module, "bad")); + assert_eq!(output.skipped_procedures.len(), 1); + let skipped = &output.skipped_procedures[0]; + assert_eq!(skipped.path.as_str(), "::test::bad"); + assert!(skipped.reason.contains("::dep::missing")); + assert!(skipped.reason.contains("signature metadata is missing")); + + Ok(()) +} + +#[test] +fn lint_disassembly_skips_unresolved_relative_invoke_during_signature_prescan() -> Result<()> { + let context = Rc::new(Context::default()); + let output = disassemble_source_for_lint( + r#" +pub proc good() -> felt + push.1 +end + +pub proc bad(value: felt) -> felt + exec.missing::callee +end +"#, + "test", + &DisassemblerConfig { + ..Default::default() + }, + context, + )?; + + let _ = find_function(output.module, "good"); + assert!(!module_has_function(output.module, "bad")); + assert_eq!(output.skipped_procedures.len(), 1); + let skipped = &output.skipped_procedures[0]; + assert_eq!(skipped.path.as_str(), "::test::bad"); + assert!(skipped.reason.contains("signature metadata pre-scan")); + assert!(skipped.reason.contains("missing::callee")); + + Ok(()) +} + +#[test] +fn lint_disassembly_skips_declared_signature_body_arity_drift() -> Result<()> { + let context = Rc::new(Context::default()); + let output = disassemble_source_for_lint( + r#" +pub proc good() -> felt + push.1 +end + +pub proc bad() -> felt + push.1 + push.2 +end +"#, + "test", + &DisassemblerConfig { + ..Default::default() + }, + context, + )?; + + let _ = find_function(output.module, "good"); + assert!(!module_has_function(output.module, "bad")); + assert_eq!(output.skipped_procedures.len(), 1); + let skipped = &output.skipped_procedures[0]; + assert_eq!(skipped.path.as_str(), "::test::bad"); + assert!(skipped.reason.contains("declared signature")); + assert!(skipped.reason.contains("extra value")); + + Ok(()) +} + +#[test] +fn lint_disassembly_keeps_declared_pass_through_signature() -> Result<()> { + let context = Rc::new(Context::default()); + let output = disassemble_source_for_lint( + r#" +pub proc id(value: felt) -> felt + nop +end +"#, + "test", + &DisassemblerConfig { + ..Default::default() + }, + context, + )?; + + let signature = find_function(output.module, "id").borrow().get_signature().clone(); + assert_eq!(signature.params().len(), 1); + assert_eq!(signature.params()[0].ty, Type::Felt); + assert_eq!(signature.results().len(), 1); + assert_eq!(signature.results()[0].ty, Type::Felt); + assert!(output.skipped_procedures.is_empty()); + + Ok(()) +} + +#[test] +fn lint_disassembly_skips_callers_of_skipped_procedures() -> Result<()> { + let context = Rc::new(Context::default()); + let output = disassemble_source_for_lint( + r#" +pub proc good + push.1 +end + +pub proc caller + exec.bad +end + +pub proc bad + push.1 + if.true + push.1 + else + push.1 + push.2 + end +end +"#, + "test", + &DisassemblerConfig { + infer_missing_signatures: true, + }, + context, + )?; + + let _ = find_function(output.module, "good"); + assert!(!module_has_function(output.module, "bad")); + assert!(!module_has_function(output.module, "caller")); + + let bad = output + .skipped_procedures + .iter() + .find(|skipped| skipped.path.as_str() == "::test::bad") + .expect("expected bad procedure to be skipped"); + assert!(bad.reason.contains("if branches leave different inferred stack depths")); + + let caller = output + .skipped_procedures + .iter() + .find(|skipped| skipped.path.as_str() == "::test::caller") + .expect("expected caller procedure to be skipped"); + assert!(caller.reason.contains("depends on skipped procedure '::test::bad'")); + + Ok(()) +} + +#[test] +fn lint_advice_taint_reports_findings_when_other_procedures_are_skipped() -> Result<()> { + let context = Rc::new(Context::default()); + let output = disassemble_source_for_lint( + r#" +pub proc entry() -> u32 + adv_push + push.1 + u32wrapping_add +end + +pub proc bad + push.1 + if.true + push.1 + else + push.1 + push.2 + end +end +"#, + "test", + &DisassemblerConfig { + infer_missing_signatures: true, + }, + context, + )?; + + assert!(module_has_function(output.module, "entry")); + assert!(!module_has_function(output.module, "bad")); + assert_eq!(output.skipped_procedures.len(), 1); + assert_eq!(output.skipped_procedures[0].path.as_str(), "::test::bad"); + + let findings = advice_taint_findings(output.module)?; + assert_eq!(sink_names(&findings), ["arith.add"]); + assert_eq!(findings[0].function.map(|name| name.as_str()), Some("entry")); + + Ok(()) +} + +#[test] +fn lint_disassembly_skips_unsupported_dynamic_invocation() -> Result<()> { + let context = Rc::new(Context::default()); + let output = disassemble_source_for_lint( + r#" +pub proc good() -> felt + push.1 +end + +pub proc dynamic(value: felt) -> felt + dynexec +end +"#, + "test", + &DisassemblerConfig { + ..Default::default() + }, + context, + )?; + + let _ = find_function(output.module, "good"); + assert!(!module_has_function(output.module, "dynamic")); + assert_eq!(output.skipped_procedures.len(), 1); + let skipped = &output.skipped_procedures[0]; + assert_eq!(skipped.path.as_str(), "::test::dynamic"); + assert_ne!(skipped.span, SourceSpan::UNKNOWN); + assert!(skipped.reason.contains("DynExec")); + assert!(skipped.reason.contains("not supported during disassembly")); + + Ok(()) +} + +#[test] +fn lint_disassembly_skips_unsupported_exp_bit_length() -> Result<()> { + let context = Rc::new(Context::default()); + let output = disassemble_source_for_lint( + r#" +pub proc good() -> felt + push.1 +end + +pub proc exp_u8(base: felt, exponent: u32) -> felt + exp.u8 +end +"#, + "test", + &DisassemblerConfig { + ..Default::default() + }, + context, + )?; + + let _ = find_function(output.module, "good"); + assert!(!module_has_function(output.module, "exp_u8")); + assert_eq!(output.skipped_procedures.len(), 1); + let skipped = &output.skipped_procedures[0]; + assert_eq!(skipped.path.as_str(), "::test::exp_u8"); + assert_ne!(skipped.span, SourceSpan::UNKNOWN); + assert!(skipped.reason.contains("ExpBitLength(8)")); + assert!(skipped.reason.contains("not supported during disassembly")); + + Ok(()) +} + +#[test] +fn lint_disassembly_skips_infer_only_proc_ref() -> Result<()> { + let context = Rc::new(Context::default()); + let output = disassemble_source_for_lint( + r#" +pub proc good() -> felt + push.1 +end + +proc target() + nop +end + +pub proc capture() -> [felt; 4] + procref.target +end +"#, + "test", + &DisassemblerConfig { + ..Default::default() + }, + context, + )?; + + let _ = find_function(output.module, "good"); + assert!(!module_has_function(output.module, "capture")); + let skipped = output + .skipped_procedures + .iter() + .find(|skipped| skipped.path.as_str() == "::test::capture") + .expect("expected procref procedure to be skipped"); + assert!(skipped.reason.contains("ProcRef")); + assert!(skipped.reason.contains("not supported during disassembly")); + + Ok(()) +} + +#[test] +fn lint_disassembly_skips_oversized_inferred_signatures() -> Result<()> { + let context = Rc::new(Context::default()); + let pushes = std::iter::repeat_n(" padw", 64).collect::>().join("\n"); + let source = format!( + r#" +pub proc good() -> felt + push.1 +end + +pub proc caller() -> felt + exec.huge +end + +pub proc huge +{pushes} +end +"# + ); + + let output = disassemble_source_for_lint( + &source, + "test", + &DisassemblerConfig { + infer_missing_signatures: true, + }, + context, + )?; + + let _ = find_function(output.module, "good"); + assert!(!module_has_function(output.module, "huge")); + assert!(!module_has_function(output.module, "caller")); + + let huge = output + .skipped_procedures + .iter() + .find(|skipped| skipped.path.as_str() == "::test::huge") + .expect("expected huge procedure to be skipped"); + assert!(huge.reason.contains("returns 256 value(s)")); + assert!(huge.reason.contains("HIR operand limit")); + + let caller = output + .skipped_procedures + .iter() + .find(|skipped| skipped.path.as_str() == "::test::caller") + .expect("expected caller procedure to be skipped"); + assert!( + caller.reason.contains("depends on skipped procedure '::test::huge'") + || caller.reason.contains("declared signature") + ); + + Ok(()) +} + +#[test] +fn lint_disassembly_skips_oversized_expanded_bodies() -> Result<()> { + let context = Rc::new(Context::default()); + let output = disassemble_source_for_lint( + r#" +pub proc good(value: felt) -> felt + add.1 +end + +pub proc caller(value: felt) -> felt + exec.huge +end + +pub proc huge(value: felt) -> felt + repeat.801 + add.1 + end +end +"#, + "test", + &DisassemblerConfig { + ..Default::default() + }, + context, + )?; + + let _ = find_function(output.module, "good"); + assert!(!module_has_function(output.module, "huge")); + assert!(!module_has_function(output.module, "caller")); + + let huge = output + .skipped_procedures + .iter() + .find(|skipped| skipped.path.as_str() == "::test::huge") + .expect("expected huge procedure to be skipped"); + assert!(huge.reason.contains("estimated to expand to at least 901 HIR operation")); + assert!(huge.reason.contains("lint analysis limit")); + + let caller = output + .skipped_procedures + .iter() + .find(|skipped| skipped.path.as_str() == "::test::caller") + .expect("expected caller procedure to be skipped"); + assert!(caller.reason.contains("depends on skipped procedure '::test::huge'")); + + Ok(()) +} + +#[test] +fn lint_disassembly_skips_wide_signatures() -> Result<()> { + let context = Rc::new(Context::default()); + let output = disassemble_source_for_lint( + r#" +pub proc good() -> felt + push.1 +end + +pub proc wide( + v0: felt, v1: felt, v2: felt, v3: felt, v4: felt, + v5: felt, v6: felt, v7: felt, v8: felt +) -> felt + drop + dropw + dropw + push.1 +end +"#, + "test", + &DisassemblerConfig { + ..Default::default() + }, + context, + )?; + + let _ = find_function(output.module, "good"); + assert!(!module_has_function(output.module, "wide")); + let wide = output + .skipped_procedures + .iter() + .find(|skipped| skipped.path.as_str() == "::test::wide") + .expect("expected wide procedure to be skipped"); + assert!(wide.reason.contains("has 9 parameter(s)")); + assert!(wide.reason.contains("signature limit of 8")); + + Ok(()) +} + + +#[test] +fn rejects_mutual_recursion() -> Result<()> { + let context = Rc::new(Context::default()); + let Err(err) = disassemble_source( + r#" +pub proc a() -> felt + exec.b +end + +pub proc b() -> felt + exec.a +end +"#, + "test", + &DisassemblerConfig::default(), + context, + ) else { + panic!("expected disassembly with mututal recursion to fail"); + }; + + assert!(err.to_string().contains("found a cycle in the call graph")); + Ok(()) +} + +struct InstructionCase { + name: String, + locals: usize, + params: Vec<&'static str>, + results: Vec<&'static str>, + body: String, +} + +fn felt_instruction_case( + name: impl Into, + params: usize, + results: usize, + body: impl Into, +) -> InstructionCase { + instruction_case(name, &felt_types(params), &felt_types(results), body) +} + +fn instruction_case( + name: impl Into, + params: &[&'static str], + results: &[&'static str], + body: impl Into, +) -> InstructionCase { + instruction_case_with_locals(name, 0, params, results, body) +} + +fn instruction_case_with_locals( + name: impl Into, + locals: usize, + params: &[&'static str], + results: &[&'static str], + body: impl Into, +) -> InstructionCase { + InstructionCase { + name: name.into(), + locals, + params: params.to_vec(), + results: results.to_vec(), + body: body.into(), + } +} + +fn unsupported_instruction_case( + name: impl Into, + locals: usize, + body: impl Into, +) -> InstructionCase { + instruction_case_with_locals(name, locals, &[], &[], body) +} + +fn parse_test_module( + source: &str, + context: &Rc, +) -> Result> { + parse_test_module_with_path(source, "test", context) +} + +fn parse_test_module_with_path( + source: &str, + path: &str, + context: &Rc, +) -> Result> { + miden_assembly::ModuleParser::new(Some(ast::ModuleKind::Library)).parse_str( + Some(ast::Path::new(path)), + source, + context.source_manager(), + ) +} + +fn felt_types(count: usize) -> Vec<&'static str> { + vec!["felt"; count] +} + +fn felt_word_select_params() -> Vec<&'static str> { + let mut params = Vec::with_capacity(9); + params.push("i1"); + params.extend(felt_types(8)); + params +} + +fn u32_types(count: usize) -> Vec<&'static str> { + vec!["u32"; count] +} + +fn felt_memory_pointer_type() -> Type { + Type::from(PointerType::new_with_address_space(Type::Felt, AddressSpace::Element)) } fn assert_instruction_case_lifts(case: &InstructionCase) { @@ -5028,6 +6053,17 @@ fn find_function(module: builtin::ModuleRef, name: &str) -> builtin::FunctionRef panic!("expected function '{name}'"); } +fn module_has_function(module: builtin::ModuleRef, name: &str) -> bool { + if module.borrow().get(SymbolName::intern(name)).is_some() { + return true; + } + + module.borrow().body().entry().body().iter().any(|op| { + op.downcast_ref::() + .is_some_and(|function| function.get_name().as_str() == name) + }) +} + fn find_world_module(world: builtin::WorldRef, module_path: &str) -> builtin::ModuleRef { let path = SymbolPath::from_masm_module_id(module_path); let symbol = world @@ -5229,6 +6265,74 @@ dep = { path = "../dep" } (root, app_dir) } +fn write_source_dependency_module_index_project( + prefix: &str, +) -> (std::path::PathBuf, std::path::PathBuf) { + let root = temp_project_dir(prefix); + let app_dir = root.join("app"); + let dep_dir = root.join("dep"); + fs::create_dir_all(&app_dir).unwrap(); + fs::create_dir_all(&dep_dir).unwrap(); + + fs::write( + dep_dir.join("miden-project.toml"), + r#"[package] +name = "dep" +version = "0.0.1" + +[lib] +path = "mod.masm" +"#, + ) + .unwrap(); + fs::write( + dep_dir.join("mod.masm"), + r#" +pub mod child + +pub proc entry(a: felt) -> felt + exec.child::leaf +end +"#, + ) + .unwrap(); + fs::write( + dep_dir.join("child.masm"), + r#" +pub proc leaf(a: felt) -> felt + add.1 +end +"#, + ) + .unwrap(); + + fs::write( + app_dir.join("miden-project.toml"), + r#"[package] +name = "app" +version = "0.0.1" + +[lib] +path = "main.masm" + +[dependencies] +dep = { path = "../dep" } +"#, + ) + .unwrap(); + fs::write( + app_dir.join("main.masm"), + r#" +pub proc entry(a: felt) -> felt + exec.::dep::entry +end +"#, + ) + .unwrap(); + + (root, app_dir) +} + fn write_multi_module_project(prefix: &str) -> (std::path::PathBuf, std::path::PathBuf) { let root = temp_project_dir(prefix); let app_dir = root.join("app"); @@ -5277,6 +6381,155 @@ end (root, app_dir) } +fn write_module_index_project(prefix: &str) -> (std::path::PathBuf, std::path::PathBuf) { + let root = temp_project_dir(prefix); + let app_dir = root.join("app"); + let child_dir = app_dir.join("child"); + fs::create_dir_all(&child_dir).unwrap(); + + fs::write( + app_dir.join("miden-project.toml"), + r#"[package] +name = "app" +version = "0.0.1" + +[lib] +path = "mod.masm" +"#, + ) + .unwrap(); + fs::write( + app_dir.join("mod.masm"), + r#" +#! This root is only a module index. + +pub mod child +"#, + ) + .unwrap(); + fs::write( + child_dir.join("mod.masm"), + r#" +pub mod leaf +pub use {inc} from self::leaf + +pub proc double(a: felt) -> felt + exec.inc +end +"#, + ) + .unwrap(); + fs::write( + child_dir.join("leaf.masm"), + r#" +pub proc inc(a: felt) -> felt + add.1 +end +"#, + ) + .unwrap(); + + (root, app_dir) +} + +fn write_private_child_module_index_project( + prefix: &str, +) -> (std::path::PathBuf, std::path::PathBuf) { + let root = temp_project_dir(prefix); + let app_dir = root.join("app"); + let child_dir = app_dir.join("child"); + fs::create_dir_all(&child_dir).unwrap(); + + fs::write( + app_dir.join("miden-project.toml"), + r#"[package] +name = "app" +version = "0.0.1" + +[lib] +path = "mod.masm" +"#, + ) + .unwrap(); + fs::write( + app_dir.join("mod.masm"), + r#" +mod child + +pub proc call_secret() -> felt + exec.child::leaf::secret +end +"#, + ) + .unwrap(); + fs::write( + child_dir.join("mod.masm"), + r#" +pub mod leaf +"#, + ) + .unwrap(); + fs::write( + child_dir.join("leaf.masm"), + r#" +pub proc secret() -> felt + push.1 +end +"#, + ) + .unwrap(); + + (root, app_dir) +} + +fn write_undeclared_child_module_index_project( + prefix: &str, +) -> (std::path::PathBuf, std::path::PathBuf) { + let root = temp_project_dir(prefix); + let app_dir = root.join("app"); + let child_dir = app_dir.join("child"); + fs::create_dir_all(&child_dir).unwrap(); + + fs::write( + app_dir.join("miden-project.toml"), + r#"[package] +name = "app" +version = "0.0.1" + +[lib] +path = "mod.masm" +"#, + ) + .unwrap(); + fs::write( + app_dir.join("mod.masm"), + r#" +pub proc call_secret() -> felt + exec.child::leaf::secret +end +"#, + ) + .unwrap(); + fs::write( + child_dir.join("mod.masm"), + r#" +pub mod leaf +"#, + ) + .unwrap(); + fs::write( + child_dir.join("leaf.masm"), + r#" +pub proc secret() -> felt + push.1 +end +"#, + ) + .unwrap(); + + (root, app_dir) +} + fn write_imported_type_dependency_project( prefix: &str, include_type_dependency: bool, diff --git a/frontend/masm/tests/e2e.rs b/frontend/masm/tests/e2e.rs index f52d94c52..5846b4e6b 100644 --- a/frontend/masm/tests/e2e.rs +++ b/frontend/masm/tests/e2e.rs @@ -68,6 +68,19 @@ end ); } +#[test] +fn e2e_roundtrip_exp_u32() { + let source = r#" +pub proc entry(base: felt, exponent: u32) -> felt + exp.u32 +end +"#; + assert_roundtrip_outputs(source, &[3, 5], 1); + + let emitted = render_roundtripped_masm(source, e2e_context()); + assert!(emitted.contains("exp.u32"), "{emitted}"); +} + #[test] fn e2e_roundtrip_word_immediate_order() { assert_roundtrip_outputs( @@ -314,6 +327,19 @@ end .expect("round-tripped MASM program should assemble") } +fn render_roundtripped_masm(source: &str, context: Rc) -> String { + let disassembled = + disassemble_source(source, "test", &DisassemblerConfig::default(), context.clone()) + .expect("MASM should disassemble to HIR"); + let analysis_manager = AnalysisManager::new(disassembled.world.as_operation_ref(), None); + disassembled + .world + .borrow() + .to_masm_component(analysis_manager) + .expect("HIR should lower back to MASM") + .to_string() +} + fn execute_program( program: &Package, inputs: &[Felt], From 0527b61fd3bf3914643fa6e899837b1ab40ef4bb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Garillot?= Date: Tue, 7 Jul 2026 11:52:27 -0400 Subject: [PATCH 3/9] frontend-masm: trim MASM lint support helpers --- .../hir/src/analyses/advice_taint/lattice.rs | 31 ++++-------- frontend/masm/src/lib.rs | 26 ---------- frontend/masm/src/lift.rs | 50 ++++++++++--------- hir-analysis/src/solver.rs | 35 +++++-------- 4 files changed, 50 insertions(+), 92 deletions(-) diff --git a/dialects/hir/src/analyses/advice_taint/lattice.rs b/dialects/hir/src/analyses/advice_taint/lattice.rs index 4b7c9104e..4ec4a761f 100644 --- a/dialects/hir/src/analyses/advice_taint/lattice.rs +++ b/dialects/hir/src/analyses/advice_taint/lattice.rs @@ -336,9 +336,7 @@ impl ContextualAdviceTaintValue { .entry(context) .and_modify(|current| *current = LatticeLike::join(current, &taint)) .or_insert(taint); - canonicalize_clean_contexts(contexts); - remove_redundant_clean_contexts(contexts); - collapse_contexts_if_needed(contexts); + normalize_contexts(contexts); } } @@ -377,11 +375,18 @@ fn push_call_context(context: &CallContext, frame: CallContextFrame) -> CallCont pushed } -fn collapse_contexts_if_needed(contexts: &mut BTreeMap) { - if contexts.len() <= MAX_CALL_CONTEXTS { +fn normalize_contexts(contexts: &mut BTreeMap) { + if contexts.values().all(AdviceTaintValue::is_clean) { + contexts.clear(); + contexts.insert(CallContext::new(), AdviceTaintValue::clean()); return; } - if contexts.values().all(AdviceTaintValue::is_clean) { + + if contexts.len() > 1 { + contexts.retain(|_, taint| !taint.is_clean()); + } + + if contexts.len() <= MAX_CALL_CONTEXTS { return; } @@ -392,20 +397,6 @@ fn collapse_contexts_if_needed(contexts: &mut BTreeMap) { - if contexts.len() <= 1 { - return; - } - contexts.retain(|_, taint| !taint.is_clean()); -} - -fn canonicalize_clean_contexts(contexts: &mut BTreeMap) { - if contexts.values().all(AdviceTaintValue::is_clean) { - contexts.clear(); - contexts.insert(CallContext::new(), AdviceTaintValue::clean()); - } -} - #[derive(Debug, Copy, Clone, Eq, PartialEq)] enum OriginState { Unreported, diff --git a/frontend/masm/src/lib.rs b/frontend/masm/src/lib.rs index 788b3788b..7c7515609 100644 --- a/frontend/masm/src/lib.rs +++ b/frontend/masm/src/lib.rs @@ -285,15 +285,6 @@ pub fn disassemble_project_target_for_lint( lift::lift_project_target(inputs, &lift::LiftConfig::lint(config), context) } -/// Disassemble pre-resolved project target inputs. -pub fn disassemble_project_target_input( - inputs: project::ProjectTargetInput, - config: &DisassemblerConfig, - context: Rc, -) -> Result { - lift::lift_project_target(inputs, &lift::LiftConfig::strict(config), context) -} - /// Disassemble pre-resolved project target inputs for linting. pub fn disassemble_project_target_input_for_lint( inputs: project::ProjectTargetInput, @@ -351,23 +342,6 @@ pub fn disassemble_project_target_with_dependency_graph( lift::lift_project_target(target, &lift::LiftConfig::strict(config), context) } -/// Disassemble a target from a manifest and precomputed dependency graph for linting. -pub fn disassemble_project_target_with_dependency_graph_for_lint( - manifest_path: impl AsRef, - target: Option<&str>, - dependency_graph: &miden_project::ProjectDependencyGraph, - config: &DisassemblerConfig, - context: Rc, -) -> Result { - let target = project::resolve_project_target_from_manifest_path_with_dependency_graph( - manifest_path.as_ref(), - target, - dependency_graph, - &context, - )?; - lift::lift_project_target(target, &lift::LiftConfig::lint(config), context) -} - /// Disassemble a parsed MASM AST module into HIR. pub fn disassemble_module( root: Box, diff --git a/frontend/masm/src/lift.rs b/frontend/masm/src/lift.rs index 1f72bf573..e3b74a9dc 100644 --- a/frontend/masm/src/lift.rs +++ b/frontend/masm/src/lift.rs @@ -620,32 +620,36 @@ impl ModuleRegistry { } fn validate_lint_signature(path: &ast::Path, signature: &Signature) -> Result<()> { - if signature.params().len() > u8::MAX as usize { - return Err(Report::msg(format!( - "procedure '{path}' has {} parameter(s), exceeding the HIR operand limit of {}", + let checks = [ + (signature.params().len(), u8::MAX as usize, "has", "parameter", "HIR operand"), + ( + signature.results().len(), + u8::MAX as usize, + "returns", + "value", + "HIR operand", + ), + ( signature.params().len(), - u8::MAX - ))); - } - if signature.results().len() > u8::MAX as usize { - return Err(Report::msg(format!( - "procedure '{path}' returns {} value(s), exceeding the HIR operand limit of {}", + LINT_SIGNATURE_VALUE_LIMIT, + "has", + "parameter", + "lint analysis signature", + ), + ( signature.results().len(), - u8::MAX - ))); - } - if signature.params().len() > LINT_SIGNATURE_VALUE_LIMIT { - return Err(Report::msg(format!( - "procedure '{path}' has {} parameter(s), exceeding the lint analysis signature limit \ - of {LINT_SIGNATURE_VALUE_LIMIT}", - signature.params().len() - ))); - } - if signature.results().len() > LINT_SIGNATURE_VALUE_LIMIT { + LINT_SIGNATURE_VALUE_LIMIT, + "returns", + "value", + "lint analysis signature", + ), + ]; + if let Some((count, limit, verb, noun, limit_name)) = + checks.into_iter().find(|(count, limit, ..)| count > limit) + { return Err(Report::msg(format!( - "procedure '{path}' returns {} value(s), exceeding the lint analysis signature limit \ - of {LINT_SIGNATURE_VALUE_LIMIT}", - signature.results().len() + "procedure '{path}' {verb} {count} {noun}(s), exceeding the {limit_name} limit of \ + {limit}" ))); } Ok(()) diff --git a/hir-analysis/src/solver.rs b/hir-analysis/src/solver.rs index 7ea22bd28..23f48e79c 100644 --- a/hir-analysis/src/solver.rs +++ b/hir-analysis/src/solver.rs @@ -586,23 +586,7 @@ impl DataFlowSolver { fn summarize_queued_analyses( names: impl IntoIterator, ) -> alloc::string::String { - let mut counts = Vec::<(&'static str, usize)>::new(); - for name in names { - if let Some((_, count)) = counts.iter_mut().find(|(existing, _)| *existing == name) { - *count += 1; - } else { - counts.push((name, 1)); - } - } - counts.sort_by(|(lhs_name, lhs_count), (rhs_name, rhs_count)| { - rhs_count.cmp(lhs_count).then_with(|| lhs_name.cmp(rhs_name)) - }); - counts - .into_iter() - .take(5) - .map(|(name, count)| format!("{name}={count}")) - .collect::>() - .join(", ") + summarize_top_counts(names.into_iter().map(ToString::to_string)) } fn describe_program_point_owner(point: &ProgramPoint) -> alloc::string::String { @@ -627,16 +611,21 @@ fn describe_program_point_owner_name(point: &ProgramPoint) -> Option( points: impl IntoIterator, +) -> alloc::string::String { + summarize_top_counts(points.into_iter().map(|point| { + describe_program_point_owner_name(point).unwrap_or_else(|| "".into()) + })) +} + +fn summarize_top_counts( + items: impl IntoIterator, ) -> alloc::string::String { let mut counts = Vec::<(alloc::string::String, usize)>::new(); - for name in points - .into_iter() - .map(|point| describe_program_point_owner_name(point).unwrap_or_else(|| "".into())) - { - if let Some((_, count)) = counts.iter_mut().find(|(existing, _)| *existing == name) { + for item in items { + if let Some((_, count)) = counts.iter_mut().find(|(existing, _)| *existing == item) { *count += 1; } else { - counts.push((name, 1)); + counts.push((item, 1)); } } counts.sort_by(|(lhs_name, lhs_count), (rhs_name, rhs_count)| { From 46186550ac071a756f0ace92c0ed3365d3174810 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Garillot?= Date: Tue, 7 Jul 2026 12:08:28 -0400 Subject: [PATCH 4/9] frontend-masm: fix MASM lint branch clippy warnings --- frontend/masm/src/tests.rs | 73 ++++++-------------------------------- hir-analysis/src/solver.rs | 4 +-- 2 files changed, 11 insertions(+), 66 deletions(-) diff --git a/frontend/masm/src/tests.rs b/frontend/masm/src/tests.rs index 1bcee3427..f913a483e 100644 --- a/frontend/masm/src/tests.rs +++ b/frontend/masm/src/tests.rs @@ -179,7 +179,6 @@ end "test", &DisassemblerConfig { infer_missing_signatures: true, - ..Default::default() }, context, ); @@ -1004,7 +1003,6 @@ end "test", &DisassemblerConfig { infer_missing_signatures: true, - ..Default::default() }, context, )?; @@ -1034,7 +1032,6 @@ end "test", &DisassemblerConfig { infer_missing_signatures: true, - ..Default::default() }, context, )?; @@ -1061,7 +1058,6 @@ end "test", &DisassemblerConfig { infer_missing_signatures: true, - ..Default::default() }, context, )?; @@ -1095,7 +1091,6 @@ end "test", &DisassemblerConfig { infer_missing_signatures: true, - ..Default::default() }, context, )?; @@ -1142,7 +1137,6 @@ end "test", &DisassemblerConfig { infer_missing_signatures: true, - ..Default::default() }, context, )?; @@ -1186,7 +1180,6 @@ end "test", &DisassemblerConfig { infer_missing_signatures: true, - ..Default::default() }, context, )?; @@ -1225,7 +1218,6 @@ end "test", &DisassemblerConfig { infer_missing_signatures: true, - ..Default::default() }, context, )?; @@ -1274,7 +1266,6 @@ end "test", &DisassemblerConfig { infer_missing_signatures: true, - ..Default::default() }, context, )?; @@ -1306,7 +1297,6 @@ end "test", &DisassemblerConfig { infer_missing_signatures: true, - ..Default::default() }, context, )?; @@ -1335,7 +1325,6 @@ end "test", &DisassemblerConfig { infer_missing_signatures: true, - ..Default::default() }, context, )?; @@ -1378,7 +1367,6 @@ end "test", &DisassemblerConfig { infer_missing_signatures: true, - ..Default::default() }, context, )?; @@ -1487,7 +1475,6 @@ end "test", &DisassemblerConfig { infer_missing_signatures: true, - ..Default::default() }, context, )?; @@ -1523,7 +1510,6 @@ end "test", &DisassemblerConfig { infer_missing_signatures: true, - ..Default::default() }, context, )?; @@ -1552,7 +1538,6 @@ end "test", &DisassemblerConfig { infer_missing_signatures: true, - ..Default::default() }, context, )?; @@ -1582,7 +1567,6 @@ end "test", &DisassemblerConfig { infer_missing_signatures: true, - ..Default::default() }, context, )?; @@ -1609,7 +1593,6 @@ end "test", &DisassemblerConfig { infer_missing_signatures: true, - ..Default::default() }, context, )?; @@ -1654,7 +1637,6 @@ end "test", &DisassemblerConfig { infer_missing_signatures: true, - ..Default::default() }, context, )?; @@ -1700,7 +1682,6 @@ end "test", &DisassemblerConfig { infer_missing_signatures: true, - ..Default::default() }, context, )?; @@ -1732,7 +1713,6 @@ end "test", &DisassemblerConfig { infer_missing_signatures: true, - ..Default::default() }, context, )?; @@ -1790,7 +1770,6 @@ end "test", &DisassemblerConfig { infer_missing_signatures: true, - ..Default::default() }, context, )?; @@ -1850,7 +1829,6 @@ end "test", &DisassemblerConfig { infer_missing_signatures: true, - ..Default::default() }, context, )?; @@ -1906,7 +1884,6 @@ end "test", &DisassemblerConfig { infer_missing_signatures: true, - ..Default::default() }, context, )?; @@ -1962,7 +1939,6 @@ end "test", &DisassemblerConfig { infer_missing_signatures: true, - ..Default::default() }, context, )?; @@ -2058,7 +2034,6 @@ end "test", &DisassemblerConfig { infer_missing_signatures: true, - ..Default::default() }, &external_signatures, context, @@ -2171,7 +2146,6 @@ end "test", &DisassemblerConfig { infer_missing_signatures: true, - ..Default::default() }, &external_signatures, context, @@ -2230,8 +2204,6 @@ end Ok(()) } - Ok(()) -} #[test] fn external_hir_array_signature_preserves_first_class_stack_value() -> Result<()> { @@ -3014,7 +2986,6 @@ end "test", &DisassemblerConfig { infer_missing_signatures: true, - ..Default::default() }, context, ) { @@ -4384,7 +4355,6 @@ end "test", &DisassemblerConfig { infer_missing_signatures: true, - ..Default::default() }, &external_signatures, context, @@ -4426,7 +4396,6 @@ end "test", &DisassemblerConfig { infer_missing_signatures: true, - ..Default::default() }, &external_signatures, context, @@ -5193,7 +5162,6 @@ end "test", &DisassemblerConfig { infer_missing_signatures: true, - ..Default::default() }, context, ); @@ -5248,7 +5216,6 @@ end "test", &DisassemblerConfig { infer_missing_signatures: true, - ..Default::default() }, context, ); @@ -5316,9 +5283,7 @@ pub proc bad(cond: i1) -> felt end "#, "test", - &DisassemblerConfig { - ..Default::default() - }, + &DisassemblerConfig::default(), context, )?; @@ -5346,9 +5311,7 @@ pub proc bad(value: felt) -> felt end "#, "test", - &DisassemblerConfig { - ..Default::default() - }, + &DisassemblerConfig::default(), context, )?; @@ -5377,9 +5340,7 @@ pub proc bad(value: felt) -> felt end "#, "test", - &DisassemblerConfig { - ..Default::default() - }, + &DisassemblerConfig::default(), context, )?; @@ -5409,9 +5370,7 @@ pub proc bad() -> felt end "#, "test", - &DisassemblerConfig { - ..Default::default() - }, + &DisassemblerConfig::default(), context, )?; @@ -5436,9 +5395,7 @@ pub proc id(value: felt) -> felt end "#, "test", - &DisassemblerConfig { - ..Default::default() - }, + &DisassemblerConfig::default(), context, )?; @@ -5557,9 +5514,7 @@ pub proc dynamic(value: felt) -> felt end "#, "test", - &DisassemblerConfig { - ..Default::default() - }, + &DisassemblerConfig::default(), context, )?; @@ -5589,9 +5544,7 @@ pub proc exp_u8(base: felt, exponent: u32) -> felt end "#, "test", - &DisassemblerConfig { - ..Default::default() - }, + &DisassemblerConfig::default(), context, )?; @@ -5625,9 +5578,7 @@ pub proc capture() -> [felt; 4] end "#, "test", - &DisassemblerConfig { - ..Default::default() - }, + &DisassemblerConfig::default(), context, )?; @@ -5718,9 +5669,7 @@ pub proc huge(value: felt) -> felt end "#, "test", - &DisassemblerConfig { - ..Default::default() - }, + &DisassemblerConfig::default(), context, )?; @@ -5766,9 +5715,7 @@ pub proc wide( end "#, "test", - &DisassemblerConfig { - ..Default::default() - }, + &DisassemblerConfig::default(), context, )?; diff --git a/hir-analysis/src/solver.rs b/hir-analysis/src/solver.rs index 23f48e79c..7d09f3a20 100644 --- a/hir-analysis/src/solver.rs +++ b/hir-analysis/src/solver.rs @@ -596,9 +596,7 @@ fn describe_program_point_owner(point: &ProgramPoint) -> alloc::string::String { } fn describe_program_point_owner_name(point: &ProgramPoint) -> Option { - let Some(op_ref) = point.operation() else { - return None; - }; + let op_ref = point.operation()?; let op = op_ref.borrow(); if let Some(function) = op.downcast_ref::() { return Some(function.get_name().as_str().to_string()); From 58a61a149746ef0d163023f16f10fd0148d8e74f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Garillot?= Date: Tue, 7 Jul 2026 12:46:31 -0400 Subject: [PATCH 5/9] frontend-masm: preserve MASM module index source spans --- frontend/masm/src/lift.rs | 50 +++++++++++++++++++------------------ frontend/masm/src/tests.rs | 51 +++++++++++++++++++++++++++++++++++--- 2 files changed, 73 insertions(+), 28 deletions(-) diff --git a/frontend/masm/src/lift.rs b/frontend/masm/src/lift.rs index e3b74a9dc..87c47d417 100644 --- a/frontend/masm/src/lift.rs +++ b/frontend/masm/src/lift.rs @@ -41,6 +41,7 @@ use crate::{ const LINT_ESTIMATED_HIR_OP_LIMIT: usize = 900; const LINT_SIGNATURE_VALUE_LIMIT: usize = 8; +type InitialSkip = (GlobalItemIndex, SourceSpan, String); pub(crate) struct LiftConfig { infer_missing_signatures: bool, @@ -187,11 +188,12 @@ fn lift_modules( }) } +#[allow(clippy::vec_box)] fn link_modules_for_lint( linker: &mut Linker, root: Box, support: Vec>, -) -> Result<(Vec, Vec<(GlobalItemIndex, SourceSpan, String)>)> { +) -> Result<(Vec, Vec)> { let mut module_indices = linker.link_modules([root])?; module_indices.extend(linker.link_modules(support)?); @@ -219,9 +221,8 @@ fn link_modules_for_lint( match linker.resolve_invoke_target(&resolution, &invoke.target) { Ok(SymbolResolution::Exact { gid: callee, .. }) => { if let Some((_, callee_reason)) = skipped.get(&callee) { - let callee_path = linker[callee.module] - .path() - .join(linker[callee].name()); + let callee_path = + linker[callee.module].path().join(linker[callee].name()); reason = Some(format!( "depends on skipped procedure '{callee_path}': {callee_reason}" )); @@ -267,7 +268,7 @@ fn link_modules_for_lint( if let Some(signature) = signature { stub.set_signature(signature); } - *procedure = Box::new(stub); + **procedure = stub; } linker.link(core::iter::empty(), core::iter::empty())?; @@ -299,7 +300,7 @@ impl ModuleRegistry { top_level_modules: Vec, external_signatures: ExternalSignatureMap, external_types: ExternalTypeMap, - initial_skips: Vec<(GlobalItemIndex, SourceSpan, String)>, + initial_skips: Vec, context: Rc, ) -> Self { let external_signatures = external_signatures @@ -401,16 +402,17 @@ impl ModuleRegistry { first_non_liftable_instruction(p.body()) { return Err(Report::msg(format!( - "MASM instruction {inst:?} is not supported during \ - disassembly at {span:?}" + "MASM instruction {inst:?} is not supported \ + during disassembly at {span:?}" ))); } let count = estimated_hir_operation_count(p.body()); if count > LINT_ESTIMATED_HIR_OP_LIMIT { return Err(Report::msg(format!( - "procedure '{}' is estimated to expand to at least \ - {count} HIR operation(s), exceeding the lint \ - analysis limit of {LINT_ESTIMATED_HIR_OP_LIMIT}", + "procedure '{}' is estimated to expand to at \ + least {count} HIR operation(s), exceeding the \ + lint analysis limit of \ + {LINT_ESTIMATED_HIR_OP_LIMIT}", self.item_path(gid) ))); } @@ -493,7 +495,8 @@ impl ModuleRegistry { Err(err) => return Err(err), } } - SymbolItem::Constant(_) | SymbolItem::Type(_) | SymbolItem::Compiled(_) => {} + SymbolItem::Constant(_) | SymbolItem::Type(_) | SymbolItem::Compiled(_) => { + } } } } @@ -512,11 +515,9 @@ impl ModuleRegistry { fn skip_item(&mut self, gid: GlobalItemIndex, span: SourceSpan, reason: String) { let path = self.item_path(gid); - self.skipped_procedures.entry(gid).or_insert(SkippedProcedure { - path, - span, - reason, - }); + self.skipped_procedures + .entry(gid) + .or_insert(SkippedProcedure { path, span, reason }); } fn skipped_procedures(&self) -> Vec { @@ -529,6 +530,13 @@ impl ModuleRegistry { self.world = Some(world); let mut world_builder = WorldBuilder::new(world); + for module_index in self.top_level_modules.iter().copied() { + let module = &self.linker[module_index]; + let symbol_path = masm_module_symbol_path(module.path()); + let module_ref = world_builder.declare_module_tree(&symbol_path)?; + self.modules.insert(module_index, module_ref); + } + for (gid, signature) in self.signatures.iter() { let gid = *gid; let module_ref = if let Some(module_ref) = self.modules.get(&gid.module).copied() { @@ -622,13 +630,7 @@ impl ModuleRegistry { fn validate_lint_signature(path: &ast::Path, signature: &Signature) -> Result<()> { let checks = [ (signature.params().len(), u8::MAX as usize, "has", "parameter", "HIR operand"), - ( - signature.results().len(), - u8::MAX as usize, - "returns", - "value", - "HIR operand", - ), + (signature.results().len(), u8::MAX as usize, "returns", "value", "HIR operand"), ( signature.params().len(), LINT_SIGNATURE_VALUE_LIMIT, diff --git a/frontend/masm/src/tests.rs b/frontend/masm/src/tests.rs index f913a483e..9721a0583 100644 --- a/frontend/masm/src/tests.rs +++ b/frontend/masm/src/tests.rs @@ -32,7 +32,7 @@ use midenc_dialect_hir::{ use midenc_dialect_scf as scf; use midenc_hir::{ AddressSpace, ArrayType, CallConv, CallOpInterface, FunctionType, Immediate, Op, PointerType, - SymbolName, SymbolPath, SymbolTable, Type, + Spanned, SymbolName, SymbolPath, SymbolTable, Type, diagnostics::{Report, Severity}, dialects::builtin::{ self, Function, UnrealizedConversionCast, @@ -2339,6 +2339,41 @@ fn project_disassembly_accepts_module_index_roots() -> Result<()> { Ok(()) } +#[test] +fn project_disassembly_preserves_module_index_source_spans() -> Result<()> { + let (root, app_dir) = + write_private_child_module_index_project("midenc_frontend_masm_module_index_spans"); + + let context = Rc::new(Context::default()); + let output = disassemble_project_target_from_path_for_lint( + app_dir.join("miden-project.toml"), + None, + &DisassemblerConfig::default(), + context, + )?; + + let function = find_function(output.module, "call_secret"); + let entry_block = function.borrow().entry_block(); + let op_span = { + let entry_block = entry_block.borrow(); + entry_block + .body() + .iter() + .next() + .expect("call_secret should contain a lifted operation") + .span() + }; + let source_manager = output.context.session().source_manager.clone(); + let loc = source_manager + .file_line_col(op_span) + .map_err(|err| Report::msg(err.to_string()))?; + assert!(loc.uri.to_string().ends_with("mod.masm")); + + let _ = fs::remove_dir_all(root); + + Ok(()) +} + #[test] fn project_disassembly_accepts_private_child_module_declaration() -> Result<()> { let (root, app_dir) = @@ -2446,7 +2481,12 @@ fn project_disassembly_does_not_fallback_to_undeclared_child_procedure() -> Resu let message = err.to_string(); assert!(message.contains("leaf::secret")); - assert!(message.contains("failed to resolve") || message.contains("could not resolve")); + assert!( + message.contains("failed to resolve") + || message.contains("could not resolve") + || message.contains("invalid relative item path"), + "{message}" + ); let _ = fs::remove_dir_all(root); @@ -2492,7 +2532,11 @@ end Err(err) => err, }; - assert!(err.to_string().contains("invalid syntax")); + let message = err.to_string(); + assert!( + message.contains("invalid syntax") || message.contains("syntax error"), + "{message}" + ); let _ = fs::remove_dir_all(root); @@ -5732,7 +5776,6 @@ end Ok(()) } - #[test] fn rejects_mutual_recursion() -> Result<()> { let context = Rc::new(Context::default()); From 1f9566e18996fe8ab90aa359f5c51b9a2258c3c9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Garillot?= Date: Mon, 3 Aug 2026 11:42:15 -0400 Subject: [PATCH 6/9] frontend-masm: skip unsupported do-while procedures in lint mode --- frontend/masm/src/lift.rs | 38 +++++++++++++++++--------------------- frontend/masm/src/tests.rs | 38 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 55 insertions(+), 21 deletions(-) diff --git a/frontend/masm/src/lift.rs b/frontend/masm/src/lift.rs index 87c47d417..30ba1db5d 100644 --- a/frontend/masm/src/lift.rs +++ b/frontend/masm/src/lift.rs @@ -398,14 +398,7 @@ impl ModuleRegistry { ))); } } - if let Some((inst, span)) = - first_non_liftable_instruction(p.body()) - { - return Err(Report::msg(format!( - "MASM instruction {inst:?} is not supported \ - during disassembly at {span:?}" - ))); - } + validate_lint_liftability(p.body())?; let count = estimated_hir_operation_count(p.body()); if count > LINT_ESTIMATED_HIR_OP_LIMIT { return Err(Report::msg(format!( @@ -657,34 +650,37 @@ fn validate_lint_signature(path: &ast::Path, signature: &Signature) -> Result<() Ok(()) } -fn first_non_liftable_instruction(block: &Block) -> Option<(&Instruction, SourceSpan)> { +fn validate_lint_liftability(block: &Block) -> Result<()> { for op in block.iter() { match op { Op::Inst(inst) if semantics::instruction_semantics(inst.inner()) != InstructionSemantics::LiftAndInfer => { - return Some((inst.inner(), inst.span())); + return Err(Report::msg(format!( + "MASM instruction {:?} is not supported during disassembly at {:?}", + inst.inner(), + inst.span() + ))); } Op::Inst(_) => {} Op::If { then_blk, else_blk, .. } => { - if let Some(unsupported) = first_non_liftable_instruction(then_blk) { - return Some(unsupported); - } - if let Some(unsupported) = first_non_liftable_instruction(else_blk) { - return Some(unsupported); - } + validate_lint_liftability(then_blk)?; + validate_lint_liftability(else_blk)?; } - Op::While { body, .. } | Op::DoWhile { body, .. } | Op::Repeat { body, .. } => { - if let Some(unsupported) = first_non_liftable_instruction(body) { - return Some(unsupported); - } + Op::While { body, .. } | Op::Repeat { body, .. } => { + validate_lint_liftability(body)?; + } + Op::DoWhile { span, .. } => { + return Err(Report::msg(format!( + "MASM do-while control flow is not supported during disassembly at {span:?}" + ))); } } } - None + Ok(()) } fn estimated_hir_operation_count(block: &Block) -> usize { diff --git a/frontend/masm/src/tests.rs b/frontend/masm/src/tests.rs index 9721a0583..9d2c44892 100644 --- a/frontend/masm/src/tests.rs +++ b/frontend/masm/src/tests.rs @@ -5308,6 +5308,44 @@ end Ok(()) } +#[test] +fn lint_disassembly_skips_unsupported_do_while_procedure() -> Result<()> { + let context = Rc::new(Context::default()); + let output = disassemble_source_for_lint( + r#" +pub proc good + push.1 +end + +pub proc bad + do + push.1 + dup.0 + neq.0 + while + dup.0 + lt.10 + end +end +"#, + "test", + &DisassemblerConfig { + infer_missing_signatures: true, + }, + context, + )?; + + let _ = find_function(output.module, "good"); + assert!(!module_has_function(output.module, "bad")); + assert_eq!(output.skipped_procedures.len(), 1); + let skipped = &output.skipped_procedures[0]; + assert_eq!(skipped.path.as_str(), "::test::bad"); + assert_ne!(skipped.span, SourceSpan::UNKNOWN); + assert!(skipped.reason.contains("do-while control flow is not supported")); + + Ok(()) +} + #[test] fn lint_disassembly_skips_known_signature_stack_shape_mismatch() -> Result<()> { let context = Rc::new(Context::default()); From 78e77898174e60a2be5252d539f9609450ad821d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Garillot?= Date: Mon, 3 Aug 2026 11:58:14 -0400 Subject: [PATCH 7/9] frontend-masm: use resolved dependencies for lint disassembly --- frontend/masm/src/lib.rs | 20 +++--- frontend/masm/src/tests.rs | 138 +++++++++++++++++++++++++++++++++++-- 2 files changed, 143 insertions(+), 15 deletions(-) diff --git a/frontend/masm/src/lib.rs b/frontend/masm/src/lib.rs index 7c7515609..9c37960af 100644 --- a/frontend/masm/src/lib.rs +++ b/frontend/masm/src/lib.rs @@ -268,20 +268,20 @@ pub fn disassemble_project_target_with_sources( lift::lift_project_target(inputs, &lift::LiftConfig::strict(config), context) } -/// Disassemble a project target for linting, skipping procedures that cannot be lifted. +/// Disassemble already-parsed project sources for linting, using the resolved dependency closure +/// and skipping procedures that cannot be lifted. pub fn disassemble_project_target_for_lint( - project: &miden_project::Project, - target: Option<&str>, - sources: Option, + sources: ProjectSourceInputs, + dependency_graph: &miden_project::ProjectDependencyGraph, config: &DisassemblerConfig, context: Rc, ) -> Result { - let inputs = if let Some(sources) = sources { - let metadata = project::collect_dependency_metadata(project, &context)?; - project::ProjectTargetInput::new(sources, metadata) - } else { - project::resolve_project_target(project, target, &context)? - }; + let metadata = project::collect_dependency_graph_metadata( + dependency_graph, + project::RegistryNodes::Skip, + &context, + )?; + let inputs = project::ProjectTargetInput::new(sources, metadata); lift::lift_project_target(inputs, &lift::LiftConfig::lint(config), context) } diff --git a/frontend/masm/src/tests.rs b/frontend/masm/src/tests.rs index 9d2c44892..0896c75df 100644 --- a/frontend/masm/src/tests.rs +++ b/frontend/masm/src/tests.rs @@ -2413,7 +2413,10 @@ path = "missing.masm" .unwrap(); let context = Rc::new(Context::default()); let manifest_path = app_dir.join("miden-project.toml"); - let project = Project::load(&manifest_path, &context.session().source_manager)?; + let registry = NoPackageStore; + let dependency_graph = ProjectDependencyGraphBuilder::new(®istry) + .with_source_manager(context.session().source_manager.clone()) + .build_from_path(&manifest_path)?; let app = parse_test_module_with_path( r#" pub proc entry() -> felt @@ -2424,12 +2427,11 @@ end &context, )?; let output = disassemble_project_target_for_lint( - &project, - None, - Some(ProjectSourceInputs { + ProjectSourceInputs { root: app, support: vec![], - }), + }, + &dependency_graph, &DisassemblerConfig::default(), context, )?; @@ -2442,6 +2444,49 @@ end Ok(()) } +#[test] +fn lint_project_disassembly_with_preparsed_sources_uses_transitive_dependencies() -> Result<()> { + let (root, app_dir) = + write_transitive_source_dependency_project("midenc_frontend_masm_lint_transitive_dep"); + let context = Rc::new(Context::default()); + let registry = NoPackageStore; + let dependency_graph = ProjectDependencyGraphBuilder::new(®istry) + .with_source_manager(context.session().source_manager.clone()) + .build_from_path(app_dir.join("miden-project.toml"))?; + let app = parse_test_module_with_path( + r#" +pub proc entry(a: felt) -> felt + exec.::dep::callee +end +"#, + "app", + &context, + )?; + + let output = disassemble_project_target_for_lint( + ProjectSourceInputs { + root: app, + support: vec![], + }, + &dependency_graph, + &DisassemblerConfig::default(), + context, + )?; + + let entry = find_function(output.module, "entry"); + assert_eq!(top_level_op_count::(entry), 1); + let dep = find_world_module(output.world, "dep"); + let callee = find_function(dep, "callee"); + assert_eq!(top_level_op_count::(callee), 1); + let transitive = find_world_module(output.world, "transitive"); + let _ = find_function(transitive, "callee"); + assert!(output.skipped_procedures.is_empty()); + + let _ = fs::remove_dir_all(root); + + Ok(()) +} + #[test] fn lint_project_disassembly_with_resolved_input_keeps_module_index_metadata() -> Result<()> { let (root, app_dir) = @@ -6250,6 +6295,89 @@ end ) } +fn write_transitive_source_dependency_project( + prefix: &str, +) -> (std::path::PathBuf, std::path::PathBuf) { + let root = temp_project_dir(prefix); + let app_dir = root.join("app"); + let dep_dir = root.join("dep"); + let transitive_dir = root.join("transitive"); + fs::create_dir_all(&app_dir).unwrap(); + fs::create_dir_all(&dep_dir).unwrap(); + fs::create_dir_all(&transitive_dir).unwrap(); + + fs::write( + transitive_dir.join("miden-project.toml"), + r#"[package] +name = "transitive" +version = "0.0.1" + +[lib] +path = "lib.masm" +"#, + ) + .unwrap(); + fs::write( + transitive_dir.join("lib.masm"), + r#" +pub proc callee(a: felt) -> felt + add.1 +end +"#, + ) + .unwrap(); + + fs::write( + dep_dir.join("miden-project.toml"), + r#"[package] +name = "dep" +version = "0.0.1" + +[lib] +path = "lib.masm" + +[dependencies] +transitive = { path = "../transitive" } +"#, + ) + .unwrap(); + fs::write( + dep_dir.join("lib.masm"), + r#" +pub proc callee(a: felt) -> felt + exec.::transitive::callee +end +"#, + ) + .unwrap(); + + fs::write( + app_dir.join("miden-project.toml"), + r#"[package] +name = "app" +version = "0.0.1" + +[lib] +path = "main.masm" + +[dependencies] +dep = { path = "../dep" } +"#, + ) + .unwrap(); + fs::write( + app_dir.join("main.masm"), + r#" +pub proc entry(a: felt) -> felt + exec.::dep::callee +end +"#, + ) + .unwrap(); + + (root, app_dir) +} + fn write_source_dependency_project_with_content( prefix: &str, main_masm: &str, From d1ab4441f91a2d65c3b2b0048157caf334c2b788 Mon Sep 17 00:00:00 2001 From: Paul Schoenfelder Date: Thu, 6 Aug 2026 20:11:05 -0400 Subject: [PATCH 8/9] frontend-masm: propagate lint skips with a worklist Build a reverse caller graph during lint preflight so skipped procedures propagate without repeated full-module scans. Keep root failures actionable and avoid recursively duplicating dependency reasons. --- frontend/masm/src/lift.rs | 172 ++++++++++++++++++++++++++----------- frontend/masm/src/tests.rs | 160 +++++++++++++++++++++++++++++++++- 2 files changed, 283 insertions(+), 49 deletions(-) diff --git a/frontend/masm/src/lift.rs b/frontend/masm/src/lift.rs index 30ba1db5d..786c61331 100644 --- a/frontend/masm/src/lift.rs +++ b/frontend/masm/src/lift.rs @@ -1,4 +1,8 @@ -use std::{collections::BTreeMap, rc::Rc, sync::Arc}; +use std::{ + collections::{BTreeMap, VecDeque}, + rc::Rc, + sync::Arc, +}; use miden_assembly::{ GlobalItemIndex, ModuleIndex, ProjectSourceInputs, @@ -43,6 +47,31 @@ const LINT_ESTIMATED_HIR_OP_LIMIT: usize = 900; const LINT_SIGNATURE_VALUE_LIMIT: usize = 8; type InitialSkip = (GlobalItemIndex, SourceSpan, String); +struct LintProcedurePreflight { + gid: GlobalItemIndex, + span: SourceSpan, + outcomes: Vec, +} + +enum LintInvocationOutcome { + Exact { + span: SourceSpan, + callee: GlobalItemIndex, + }, + ResolutionFailure { + span: SourceSpan, + reason: String, + }, +} + +impl LintInvocationOutcome { + fn span(&self) -> SourceSpan { + match self { + Self::Exact { span, .. } | Self::ResolutionFailure { span, .. } => *span, + } + } +} + pub(crate) struct LiftConfig { infer_missing_signatures: bool, lint: bool, @@ -197,57 +226,99 @@ fn link_modules_for_lint( let mut module_indices = linker.link_modules([root])?; module_indices.extend(linker.link_modules(support)?); - let mut skipped = BTreeMap::::new(); - loop { - let mut progress = false; - for module_index in 0..linker.modules().len() { - let module_index = ModuleIndex::new(module_index); - for (item_index, item) in linker[module_index].symbols().enumerate() { - let gid = module_index + ast::ItemIndex::new(item_index); - if skipped.contains_key(&gid) { - continue; - } - let SymbolItem::Procedure(procedure) = item.item() else { - continue; + let mut procedures = Vec::::new(); + let mut callers = FxHashMap::>::default(); + let mut skipped_gids = FxHashSet::::default(); + let mut pending = VecDeque::::new(); + + for module_index in 0..linker.modules().len() { + let module_index = ModuleIndex::new(module_index); + for (item_index, item) in linker[module_index].symbols().enumerate() { + let gid = module_index + ast::ItemIndex::new(item_index); + let SymbolItem::Procedure(procedure) = item.item() else { + continue; + }; + let procedure = procedure.borrow(); + let mut outcomes = Vec::new(); + let mut has_resolution_failure = false; + for invoke in procedure.invoked() { + let resolution = SymbolResolutionContext { + span: invoke.span(), + module: module_index, + kind: Some(invoke.kind), }; - let procedure = procedure.borrow(); - let mut reason = None; - for invoke in procedure.invoked() { - let resolution = SymbolResolutionContext { - span: invoke.span(), - module: module_index, - kind: Some(invoke.kind), - }; - match linker.resolve_invoke_target(&resolution, &invoke.target) { - Ok(SymbolResolution::Exact { gid: callee, .. }) => { - if let Some((_, callee_reason)) = skipped.get(&callee) { - let callee_path = - linker[callee.module].path().join(linker[callee].name()); - reason = Some(format!( - "depends on skipped procedure '{callee_path}': {callee_reason}" - )); - break; - } - } - Ok(_) => {} - Err(err) => { - reason = Some(format!( + match linker.resolve_invoke_target(&resolution, &invoke.target) { + Ok(SymbolResolution::Exact { gid: callee, .. }) => { + callers.entry(callee).or_default().push(gid); + outcomes.push(LintInvocationOutcome::Exact { + span: invoke.span(), + callee, + }); + } + Ok(_) => {} + Err(err) => { + has_resolution_failure = true; + outcomes.push(LintInvocationOutcome::ResolutionFailure { + span: invoke.span(), + reason: format!( "failed to resolve invocation during signature metadata pre-scan: \ {err}; external signature metadata is missing" - )); - break; - } + ), + }); } } - if let Some(reason) = reason { - skipped.insert(gid, (procedure.span(), reason)); - progress = true; - } + } + // `Procedure::invoked` is target-ordered, so restore lexical source order before + // selecting which deterministic diagnostic to report. + outcomes.sort_by_key(LintInvocationOutcome::span); + if has_resolution_failure && skipped_gids.insert(gid) { + pending.push_back(gid); + } + procedures.push(LintProcedurePreflight { + gid, + span: procedure.span(), + outcomes, + }); + } + } + + while let Some(callee) = pending.pop_front() { + let Some(callee_callers) = callers.get(&callee) else { + continue; + }; + for caller in callee_callers.iter().copied() { + if skipped_gids.insert(caller) { + pending.push_back(caller); } } - if !progress { - break; + } + + let mut skipped = BTreeMap::::new(); + for procedure in procedures { + if !skipped_gids.contains(&procedure.gid) { + continue; } + let reason = procedure + .outcomes + .iter() + .find_map(|outcome| match outcome { + LintInvocationOutcome::ResolutionFailure { reason, .. } => Some(reason.clone()), + LintInvocationOutcome::Exact { .. } => None, + }) + .or_else(|| { + procedure.outcomes.iter().find_map(|outcome| match outcome { + LintInvocationOutcome::Exact { callee, .. } + if skipped_gids.contains(callee) => + { + let callee_path = linker[callee.module].path().join(linker[*callee].name()); + Some(skipped_dependency_reason(callee_path.as_str())) + } + LintInvocationOutcome::Exact { .. } + | LintInvocationOutcome::ResolutionFailure { .. } => None, + }) + }) + .expect("a skipped procedure must have a failing invocation"); + skipped.insert(procedure.gid, (procedure.span, reason)); } for gid in skipped.keys().copied() { @@ -392,10 +463,11 @@ impl ModuleRegistry { && let Some(skipped) = self.skipped_procedures.get(&callee) { - return Err(Report::msg(format!( - "depends on skipped procedure '{}': {}", - skipped.path, skipped.reason - ))); + return Err(Report::msg( + skipped_dependency_reason( + skipped.path.as_str(), + ), + )); } } validate_lint_liftability(p.body())?; @@ -650,6 +722,10 @@ fn validate_lint_signature(path: &ast::Path, signature: &Signature) -> Result<() Ok(()) } +fn skipped_dependency_reason(path: &str) -> String { + format!("depends on skipped procedure '{path}'") +} + fn validate_lint_liftability(block: &Block) -> Result<()> { for op in block.iter() { match op { diff --git a/frontend/masm/src/tests.rs b/frontend/masm/src/tests.rs index 0896c75df..bc8d9a095 100644 --- a/frontend/masm/src/tests.rs +++ b/frontend/masm/src/tests.rs @@ -5482,6 +5482,152 @@ end Ok(()) } +#[test] +fn lint_signature_prescan_propagates_immediate_skip_reasons_through_call_chain() -> Result<()> { + let context = Rc::new(Context::default()); + let output = disassemble_source_for_lint( + r#" +pub proc good() -> felt + push.1 +end + +pub proc p0(value: felt) -> felt + exec.p1 +end + +pub proc p1(value: felt) -> felt + exec.p2 +end + +pub proc p2(value: felt) -> felt + exec.p3 + exec.p3 +end + +pub proc p3(value: felt) -> felt + exec.p4 +end + +pub proc p4(value: felt) -> felt + exec.p5 +end + +pub proc p5(value: felt) -> felt + exec.::dep::missing +end +"#, + "test", + &DisassemblerConfig::default(), + context, + )?; + + let _ = find_function(output.module, "good"); + assert_eq!(output.skipped_procedures.len(), 6); + for index in 0..5 { + let path = format!("::test::p{index}"); + let skipped = output + .skipped_procedures + .iter() + .find(|skipped| skipped.path.as_str() == path) + .unwrap_or_else(|| panic!("expected {path} to be skipped")); + assert_eq!( + skipped.reason, + format!("depends on skipped procedure '::test::p{}'", index + 1) + ); + } + let terminal = output + .skipped_procedures + .iter() + .find(|skipped| skipped.path.as_str() == "::test::p5") + .expect("expected terminal procedure to be skipped"); + assert!(terminal.reason.contains("::dep::missing")); + assert!(terminal.reason.contains("signature metadata is missing")); + + Ok(()) +} + +#[test] +fn lint_signature_prescan_uses_deterministic_skip_reason_precedence() -> Result<()> { + let context = Rc::new(Context::default()); + let output = disassemble_source_for_lint( + r#" +pub proc own_missing(value: felt) -> felt + exec.skipped_b + exec.::dep::own_missing +end + +pub proc choose_callee(value: felt) -> felt + exec.skipped_b + exec.skipped_a +end + +pub proc skipped_a(value: felt) -> felt + exec.::dep::missing_a +end + +pub proc skipped_b(value: felt) -> felt + exec.::dep::missing_b +end +"#, + "test", + &DisassemblerConfig::default(), + context, + )?; + + assert_eq!(output.skipped_procedures.len(), 4); + let own_missing = output + .skipped_procedures + .iter() + .find(|skipped| skipped.path.as_str() == "::test::own_missing") + .expect("expected own_missing to be skipped"); + assert!(own_missing.reason.contains("::dep::own_missing")); + + let choose_callee = output + .skipped_procedures + .iter() + .find(|skipped| skipped.path.as_str() == "::test::choose_callee") + .expect("expected choose_callee to be skipped"); + assert_eq!(choose_callee.reason, "depends on skipped procedure '::test::skipped_b'"); + + Ok(()) +} + +#[test] +fn lint_signature_prescan_preserves_direct_root_error_in_seeded_cycle() -> Result<()> { + let context = Rc::new(Context::default()); + let output = disassemble_source_for_lint( + r#" +pub proc cycle_a(value: felt) -> felt + exec.cycle_b +end + +pub proc cycle_b(value: felt) -> felt + exec.cycle_a + exec.::dep::cycle_root +end +"#, + "test", + &DisassemblerConfig::default(), + context, + )?; + + assert_eq!(output.skipped_procedures.len(), 2); + let cycle_a = output + .skipped_procedures + .iter() + .find(|skipped| skipped.path.as_str() == "::test::cycle_a") + .expect("expected cycle_a to be skipped"); + assert_eq!(cycle_a.reason, "depends on skipped procedure '::test::cycle_b'"); + let cycle_b = output + .skipped_procedures + .iter() + .find(|skipped| skipped.path.as_str() == "::test::cycle_b") + .expect("expected cycle_b to be skipped"); + assert!(cycle_b.reason.contains("::dep::cycle_root")); + + Ok(()) +} + #[test] fn lint_disassembly_skips_declared_signature_body_arity_drift() -> Result<()> { let context = Rc::new(Context::default()); @@ -5549,6 +5695,10 @@ pub proc caller exec.bad end +pub proc outer + exec.caller +end + pub proc bad push.1 if.true @@ -5569,6 +5719,7 @@ end let _ = find_function(output.module, "good"); assert!(!module_has_function(output.module, "bad")); assert!(!module_has_function(output.module, "caller")); + assert!(!module_has_function(output.module, "outer")); let bad = output .skipped_procedures @@ -5582,7 +5733,14 @@ end .iter() .find(|skipped| skipped.path.as_str() == "::test::caller") .expect("expected caller procedure to be skipped"); - assert!(caller.reason.contains("depends on skipped procedure '::test::bad'")); + assert_eq!(caller.reason, "depends on skipped procedure '::test::bad'"); + + let outer = output + .skipped_procedures + .iter() + .find(|skipped| skipped.path.as_str() == "::test::outer") + .expect("expected outer procedure to be skipped"); + assert_eq!(outer.reason, "depends on skipped procedure '::test::caller'"); Ok(()) } From e03651629af5d8559afc772803ada802977846ba Mon Sep 17 00:00:00 2001 From: Paul Schoenfelder Date: Thu, 6 Aug 2026 20:34:41 -0400 Subject: [PATCH 9/9] hir-analysis: bound worklist diagnostic aggregation Count queued analysis owners with a hash map before sorting the top summary so budget exhaustion remains inexpensive on large data-flow graphs. --- hir-analysis/src/solver.rs | 42 ++++++++++++++++++++++++++++++++------ 1 file changed, 36 insertions(+), 6 deletions(-) diff --git a/hir-analysis/src/solver.rs b/hir-analysis/src/solver.rs index 7d09f3a20..a1325d8a6 100644 --- a/hir-analysis/src/solver.rs +++ b/hir-analysis/src/solver.rs @@ -618,14 +618,11 @@ fn summarize_queued_program_point_owners<'a>( fn summarize_top_counts( items: impl IntoIterator, ) -> alloc::string::String { - let mut counts = Vec::<(alloc::string::String, usize)>::new(); + let mut counts = FxHashMap::::default(); for item in items { - if let Some((_, count)) = counts.iter_mut().find(|(existing, _)| *existing == item) { - *count += 1; - } else { - counts.push((item, 1)); - } + *counts.entry(item).or_insert(0) += 1; } + let mut counts = counts.into_iter().collect::>(); counts.sort_by(|(lhs_name, lhs_count), (rhs_name, rhs_count)| { rhs_count.cmp(lhs_count).then_with(|| lhs_name.cmp(rhs_name)) }); @@ -637,6 +634,39 @@ fn summarize_top_counts( .join(", ") } +#[cfg(test)] +mod tests { + use alloc::{format, string::ToString}; + + use super::summarize_top_counts; + + #[test] + fn summarize_top_counts_handles_empty_input_duplicates_and_ties() { + assert_eq!(summarize_top_counts(core::iter::empty()), ""); + + let summary = summarize_top_counts( + ["beta", "alpha", "gamma", "beta", "alpha"].into_iter().map(ToString::to_string), + ); + assert_eq!(summary, "alpha=2, beta=2, gamma=1"); + } + + #[test] + fn summarize_top_counts_breaks_the_fifth_place_tie_by_name() { + let summary = summarize_top_counts( + ["zeta", "gamma", "epsilon", "delta", "beta", "alpha", "zeta"] + .into_iter() + .map(ToString::to_string), + ); + assert_eq!(summary, "zeta=2, alpha=1, beta=1, delta=1, epsilon=1"); + } + + #[test] + fn summarize_top_counts_handles_many_unique_names() { + let summary = summarize_top_counts((0..4096).rev().map(|index| format!("item{index:04}"))); + assert_eq!(summary, "item0000=1, item0001=1, item0002=1, item0003=1, item0004=1"); + } +} + /// Represents an analysis that has derived facts at a specific program point from the state of /// another analysis, that has since changed. As a result, the dependent analysis must be re-applied /// at that program point to determine if the state changes have any effect on the state of its