From 980b4bf70b8e3707dd0c3ae080cf8d0992882b50 Mon Sep 17 00:00:00 2001 From: Juan Munoz Date: Thu, 9 Apr 2026 15:46:20 -0300 Subject: [PATCH 1/5] fix: panic on sm mismatch --- crates/assembly/src/assembler/debuginfo.rs | 80 +++++++++++++--------- crates/assembly/src/tests.rs | 39 ++++++++++- crates/debug-types/src/source_file.rs | 15 ++-- crates/debug-types/src/source_manager.rs | 2 +- 4 files changed, 96 insertions(+), 40 deletions(-) diff --git a/crates/assembly/src/assembler/debuginfo.rs b/crates/assembly/src/assembler/debuginfo.rs index 5f64e1b52e..f7301e3dc5 100644 --- a/crates/assembly/src/assembler/debuginfo.rs +++ b/crates/assembly/src/assembler/debuginfo.rs @@ -19,7 +19,7 @@ use crate::{ types::{StructType, Type}, }, debuginfo::SourceManager, - diagnostics::Report, + diagnostics::{Report, report}, }; // DEBUG INFO SECTIONS @@ -51,39 +51,55 @@ impl DebugInfoSections { procedure: &Procedure, source_manager: &dyn SourceManager, ) -> Result<(), Report> { - if let Ok(file_line_col) = source_manager.file_line_col(*procedure.span()) { - let path_id = - self.debug_sources_section.add_string(Arc::from(file_line_col.uri.path())); - let file_id = self - .debug_sources_section - .add_file(miden_mast_package::debug_info::DebugFileInfo::new(path_id)); - let name = Arc::::from(procedure.path().as_str()); - let name_id = self.debug_functions_section.add_string(name.clone()); - let type_index = if let Some(signature) = procedure.signature() { - Some(register_debug_type( - &mut self.debug_types_section, - Some(name), - None, - &Type::Function(signature), - )?) - } else { - None - }; - let func_info = miden_mast_package::debug_info::DebugFunctionInfo::new( - name_id, - file_id, - file_line_col.line, - file_line_col.column, - ) - .with_mast_root(procedure.mast_root()); - let func_info = if let Some(type_index) = type_index { - func_info.with_type(type_index) - } else { - func_info - }; - self.debug_functions_section.add_function(func_info); + let span = *procedure.span(); + + // If the source id doesn't exist in this source manager, skip debug info + // registration (the span may be synthetic or from a deserialized module). + if source_manager.get(span.source_id()).is_err() { + return Ok(()); } + // The source id exists, so file_line_col should succeed. If it doesn't, the + // span's byte offsets are out of bounds for the file at that source id, which + // indicates the module was parsed with a different source manager. + let file_line_col = source_manager.file_line_col(span).map_err(|err| { + report!( + "source manager mismatch for procedure '{}': the module's source spans \ + are incompatible with the assembler's source manager ({err})", + procedure.path(), + ) + })?; + + let path_id = self.debug_sources_section.add_string(Arc::from(file_line_col.uri.path())); + let file_id = self + .debug_sources_section + .add_file(miden_mast_package::debug_info::DebugFileInfo::new(path_id)); + let name = Arc::::from(procedure.path().as_str()); + let name_id = self.debug_functions_section.add_string(name.clone()); + let type_index = if let Some(signature) = procedure.signature() { + Some(register_debug_type( + &mut self.debug_types_section, + Some(name), + None, + &Type::Function(signature), + )?) + } else { + None + }; + let func_info = miden_mast_package::debug_info::DebugFunctionInfo::new( + name_id, + file_id, + file_line_col.line, + file_line_col.column, + ) + .with_mast_root(procedure.mast_root()); + let func_info = if let Some(type_index) = type_index { + func_info.with_type(type_index) + } else { + func_info + }; + self.debug_functions_section.add_function(func_info); + Ok(()) } diff --git a/crates/assembly/src/tests.rs b/crates/assembly/src/tests.rs index 2c443bf035..cf44836ffa 100644 --- a/crates/assembly/src/tests.rs +++ b/crates/assembly/src/tests.rs @@ -10,7 +10,11 @@ use std::{ }; use miden_assembly_syntax::{ - MAX_REPEAT_COUNT, ast::Path, diagnostics::WrapErr, library::LibraryExport, + MAX_REPEAT_COUNT, + ast::Path, + debuginfo::{DefaultSourceManager, SourceManager}, + diagnostics::WrapErr, + library::LibraryExport, }; use miden_core::{ Felt, Word, assert_matches, @@ -5789,3 +5793,36 @@ fn test_linking_recursive_expansion_via_renamed_aliases() -> TestResult { Ok(()) } + +#[test] +fn assemble_library_with_mismatched_source_manager_returns_error() { + // Source manager A: used by the assembler. + // Parse a short module to populate SourceId(0) in this manager. + let sm_a: Arc = Arc::new(DefaultSourceManager::default()); + let _dummy = Module::parser(ModuleKind::Library) + .parse_str("lib::dummy", "pub proc dummy\n push.1\nend", sm_a.clone()) + .unwrap(); + + let assembler = Assembler::new(sm_a); + + // Source manager B: external, used to parse the module we will assemble. + // Its SourceId(0) points to a different file. + let sm_b: Arc = Arc::new(DefaultSourceManager::default()); + let long_source = "\ +# padding line to push the procedure definition past the short file length +# more padding to ensure the byte offset exceeds the boundary +# even more padding for good measure here to be absolutely sure +pub proc bar + push.1 + push.2 + add +end"; + let module = Module::parser(ModuleKind::Library) + .parse_str("lib::external", long_source, sm_b) + .unwrap(); + + let result = assembler.assemble_library([module]); + let err = result.expect_err("should fail with source manager mismatch"); + let msg = err.to_string(); + assert!(msg.contains("source manager mismatch"), "unexpected error: {msg}"); +} diff --git a/crates/debug-types/src/source_file.rs b/crates/debug-types/src/source_file.rs index c8b36a1261..809bf543a3 100644 --- a/crates/debug-types/src/source_file.rs +++ b/crates/debug-types/src/source_file.rs @@ -192,12 +192,15 @@ impl SourceFile { } /// Get a [FileLineCol] equivalent to the start of the given [SourceSpan] - pub fn location(&self, span: SourceSpan) -> FileLineCol { - assert_eq!(span.source_id(), self.id, "mismatched source ids"); + /// + /// Returns `None` if the span's source id does not match this file, or if the + /// starting byte offset is out of bounds. + pub fn location(&self, span: SourceSpan) -> Option { + if span.source_id() != self.id { + return None; + } - self.content - .location(ByteIndex(span.into_range().start)) - .expect("invalid source span: starting byte is out of bounds") + self.content.location(ByteIndex(span.into_range().start)) } } @@ -355,7 +358,7 @@ impl<'a> miette::SpanContents<'a> for ScopedSourceFileRef<'a> { let start = self.span.offset() as u32; let end = start + self.span.len() as u32; let span = SourceSpan::new(self.file.id(), start..end); - let loc = self.file.location(span); + let loc = self.file.location(span).expect("span was constructed from this file"); loc.column.to_index().to_usize() } diff --git a/crates/debug-types/src/source_manager.rs b/crates/debug-types/src/source_manager.rs index a798e52ac6..b65da264d1 100644 --- a/crates/debug-types/src/source_manager.rs +++ b/crates/debug-types/src/source_manager.rs @@ -368,7 +368,7 @@ impl DefaultSourceManagerImpl { self.files .get(span.source_id()) .ok_or(SourceManagerError::InvalidSourceId) - .map(|file| file.location(span)) + .and_then(|file| file.location(span).ok_or(SourceManagerError::InvalidBounds)) } fn location_to_span(&self, loc: Location) -> Option { From 0bd716807ae46ccef38d9ae78edd625355d6ea15 Mon Sep 17 00:00:00 2001 From: Juan Munoz Date: Fri, 10 Apr 2026 15:37:08 -0300 Subject: [PATCH 2/5] fix: false negative scenario --- crates/assembly-syntax/src/ast/module.rs | 31 ++++++++++++++++--- crates/assembly-syntax/src/sema/mod.rs | 3 +- crates/assembly/src/assembler.rs | 11 ++++--- crates/assembly/src/assembler/debuginfo.rs | 35 +++++++++++++--------- crates/assembly/src/linker/mod.rs | 1 + crates/assembly/src/linker/module.rs | 22 +++++++++++++- crates/assembly/src/tests.rs | 27 ++++------------- 7 files changed, 85 insertions(+), 45 deletions(-) diff --git a/crates/assembly-syntax/src/ast/module.rs b/crates/assembly-syntax/src/ast/module.rs index a7fd99004b..eb2ad45fc1 100644 --- a/crates/assembly-syntax/src/ast/module.rs +++ b/crates/assembly-syntax/src/ast/module.rs @@ -5,7 +5,10 @@ use miden_core::{ advice::AdviceMap, serde::{ByteReader, ByteWriter, Deserializable, DeserializationError, Serializable}, }; -use miden_debug_types::{SourceFile, SourceManager, SourceSpan, Span, Spanned}; +use miden_debug_types::{ + SourceContent, SourceFile, SourceId, SourceLanguage, SourceManager, SourceSpan, Span, Spanned, + Uri, +}; use miden_utils_diagnostics::Report; use smallvec::SmallVec; @@ -126,6 +129,11 @@ pub struct Module { pub(crate) items: Vec, /// AdviceMap that this module expects to be loaded in the host before executing. pub(crate) advice_map: AdviceMap, + /// The source file from which this module was parsed. + /// + /// This is used to verify that the source manager used during assembly is the same one that + /// was used to parse this module, via pointer identity ([`SourceManager::is_manager_of`]). + source_file: Arc, } /// Constants @@ -144,7 +152,7 @@ impl Module { impl Module { /// Creates a new [Module] with the specified `kind` and fully-qualified path, e.g. /// `std::math::u64`. - pub fn new(kind: ModuleKind, path: impl AsRef) -> Self { + pub fn new(kind: ModuleKind, path: impl AsRef, source_file: Arc) -> Self { let path = path.as_ref().to_absolute().into_owned(); Self { span: Default::default(), @@ -153,17 +161,27 @@ impl Module { kind, items: Default::default(), advice_map: Default::default(), + source_file, } } /// An alias for creating the default, but empty, `#kernel` [Module]. pub fn new_kernel() -> Self { - Self::new(ModuleKind::Kernel, Path::kernel_path()) + Self::new(ModuleKind::Kernel, Path::kernel_path(), Self::empty_source_file()) } /// An alias for creating the default, but empty, `$exec` [Module]. pub fn new_executable() -> Self { - Self::new(ModuleKind::Executable, Path::exec_path()) + Self::new(ModuleKind::Executable, Path::exec_path(), Self::empty_source_file()) + } + + /// Creates a synthetic, empty [`SourceFile`] for programmatically constructed modules + /// that have no backing source text. + fn empty_source_file() -> Arc { + Arc::new(SourceFile::from_raw_parts( + SourceId::default(), + SourceContent::new(SourceLanguage::Masm, Uri::from("synthetic"), String::new()), + )) } /// Specifies the source span in the source file in which this module was defined, that covers @@ -440,6 +458,11 @@ impl Module { &self.advice_map } + /// Returns the source file from which this module was parsed. + pub fn source_file(&self) -> &Arc { + &self.source_file + } + /// Get an iterator over the constants defined in this module. pub fn constants(&self) -> impl Iterator + '_ { self.items.iter().filter_map(|item| match item { diff --git a/crates/assembly-syntax/src/sema/mod.rs b/crates/assembly-syntax/src/sema/mod.rs index 832a88c82d..a96c4148cd 100644 --- a/crates/assembly-syntax/src/sema/mod.rs +++ b/crates/assembly-syntax/src/sema/mod.rs @@ -44,7 +44,8 @@ pub fn analyze( let mut analyzer = AnalysisContext::new(source.clone(), source_manager); analyzer.set_warnings_as_errors(warnings_as_errors); - let mut module = Box::new(Module::new(kind, path).with_span(source.source_span())); + let mut module = + Box::new(Module::new(kind, path, source.clone()).with_span(source.source_span())); let mut forms = VecDeque::from(forms); let mut enums = SmallVec::<[EnumType; 1]>::new_const(); diff --git a/crates/assembly/src/assembler.rs b/crates/assembly/src/assembler.rs index 9bb4e7b5ee..8e6c656bd5 100644 --- a/crates/assembly/src/assembler.rs +++ b/crates/assembly/src/assembler.rs @@ -920,9 +920,9 @@ impl Assembler { continue; } // Fetch procedure metadata from the graph - let (module_kind, module_path) = { + let (module_kind, module_path, module_source_file) = { let module = &self.linker[procedure_gid.module]; - (module.kind(), module.path().clone()) + (module.kind(), module.path().clone(), module.source_file().cloned()) }; match self.linker[procedure_gid].item() { SymbolItem::Procedure(proc) => { @@ -954,8 +954,11 @@ impl Assembler { // be added to the forest. // Record the debug info for this procedure - self.debug_info - .register_procedure_debug_info(&procedure, self.source_manager.as_ref())?; + self.debug_info.register_procedure_debug_info( + &procedure, + self.source_manager.as_ref(), + module_source_file.as_deref(), + )?; // Cache the compiled procedure drop(proc); diff --git a/crates/assembly/src/assembler/debuginfo.rs b/crates/assembly/src/assembler/debuginfo.rs index f7301e3dc5..1295c906f2 100644 --- a/crates/assembly/src/assembler/debuginfo.rs +++ b/crates/assembly/src/assembler/debuginfo.rs @@ -18,7 +18,7 @@ use crate::{ TypeExpr, types::{StructType, Type}, }, - debuginfo::SourceManager, + debuginfo::{SourceFile, SourceManager}, diagnostics::{Report, report}, }; @@ -50,25 +50,32 @@ impl DebugInfoSections { &mut self, procedure: &Procedure, source_manager: &dyn SourceManager, + module_source_file: Option<&SourceFile>, ) -> Result<(), Report> { let span = *procedure.span(); - // If the source id doesn't exist in this source manager, skip debug info - // registration (the span may be synthetic or from a deserialized module). - if source_manager.get(span.source_id()).is_err() { + // If no source file is available, skip debug info registration + // (the module is synthetic or was deserialized without source info). + let Some(source_file) = module_source_file else { return Ok(()); - } + }; - // The source id exists, so file_line_col should succeed. If it doesn't, the - // span's byte offsets are out of bounds for the file at that source id, which - // indicates the module was parsed with a different source manager. - let file_line_col = source_manager.file_line_col(span).map_err(|err| { - report!( - "source manager mismatch for procedure '{}': the module's source spans \ - are incompatible with the assembler's source manager ({err})", + // Verify the source file belongs to this source manager. + if !source_manager.is_manager_of(source_file) { + return Err(report!( + "source manager mismatch for procedure '{}': the module's source file \ + is not owned by the assembler's source manager", procedure.path(), - ) - })?; + )); + } + + let file_line_col = + source_manager.file_line_col(span).map_err(|err| { + report!( + "failed to resolve source location for procedure '{}': {err}", + procedure.path(), + ) + })?; let path_id = self.debug_sources_section.add_string(Arc::from(file_line_col.uri.path())); let file_id = self diff --git a/crates/assembly/src/linker/mod.rs b/crates/assembly/src/linker/mod.rs index 8962ebe95c..c57b146ef3 100644 --- a/crates/assembly/src/linker/mod.rs +++ b/crates/assembly/src/linker/mod.rs @@ -316,6 +316,7 @@ impl Linker { module.path().into(), ) .with_advice_map(module.advice_map().clone()) + .with_source_file(Some(module.source_file().clone())) .with_symbols(symbols); self.modules.push(link_module); diff --git a/crates/assembly/src/linker/module.rs b/crates/assembly/src/linker/module.rs index 75842b89c3..045fb99271 100644 --- a/crates/assembly/src/linker/module.rs +++ b/crates/assembly/src/linker/module.rs @@ -7,7 +7,7 @@ use miden_assembly_syntax::{ self, AliasTarget, ItemIndex, LocalSymbol, LocalSymbolResolver, ModuleIndex, ModuleKind, SymbolResolution, SymbolResolutionError, SymbolTable, }, - debuginfo::{SourceManager, Span, Spanned}, + debuginfo::{SourceFile, SourceManager, Span, Spanned}, }; use super::{AdviceMap, LinkStatus, Symbol, SymbolItem, SymbolResolver}; @@ -50,6 +50,12 @@ pub struct LinkModule { /// /// This is only relevant for modules parsed from MASM sources. advice_map: Option, + /// The source file from which this module was parsed (if available). + /// + /// Used to verify source manager identity via [`SourceManager::is_manager_of`] during + /// assembly, preventing silent attachment of wrong debug info when a module was parsed + /// with a different source manager than the assembler's. + source_file: Option>, } impl LinkModule { @@ -69,6 +75,7 @@ impl LinkModule { path, symbols: Vec::default(), advice_map: None, + source_file: None, } } @@ -91,6 +98,13 @@ impl LinkModule { self } + /// Specify the source file from which this module was parsed. + #[inline] + pub fn with_source_file(mut self, source_file: Option>) -> Self { + self.source_file = source_file; + self + } + /// Get the unique identifier/index of this module in the containing linker. #[inline(always)] pub fn id(&self) -> ModuleIndex { @@ -157,6 +171,12 @@ impl LinkModule { self.advice_map.as_ref() } + /// Get the source file from which this module was parsed, if available. + #[inline] + pub fn source_file(&self) -> Option<&Arc> { + self.source_file.as_ref() + } + /// Get an iterator over the symbols in this module. #[inline] pub fn symbols(&self) -> core::slice::Iter<'_, Symbol> { diff --git a/crates/assembly/src/tests.rs b/crates/assembly/src/tests.rs index cf44836ffa..2bbe4992ba 100644 --- a/crates/assembly/src/tests.rs +++ b/crates/assembly/src/tests.rs @@ -4119,7 +4119,7 @@ fn vendoring() -> TestResult { let mod1 = mod_parser .parse(PathBuf::new("test::mod1").unwrap(), source, context.source_manager()) .unwrap(); - Assembler::default().assemble_library([mod1]).unwrap() + Assembler::new(context.source_manager()).assemble_library([mod1]).unwrap() }; let lib = { @@ -4128,7 +4128,7 @@ fn vendoring() -> TestResult { .parse(PathBuf::new("test::mod2").unwrap(), source, context.source_manager()) .unwrap(); - let mut assembler = Assembler::default(); + let mut assembler = Assembler::new(context.source_manager()); assembler.link_static_library(vendor_lib)?; assembler.assemble_library([mod2]).unwrap() }; @@ -4144,7 +4144,7 @@ fn vendoring() -> TestResult { let expected_lib = { let source = source_file!(&context, "pub proc foo push.1 end"); let mod2 = mod_parser.parse("test::expected", source, context.source_manager()).unwrap(); - Assembler::default().assemble_library([mod2]).unwrap() + Assembler::new(context.source_manager()).assemble_library([mod2]).unwrap() }; // 3. Verify that the expected library (which has push.1) has AssemblyOps @@ -5101,7 +5101,7 @@ fn test_assembler_debug_info_present() { let module = parse_module!(&context, "test::foo", source); // Test: With debug mode always enabled (issue #1821), debug info should always be present - let assembler = Assembler::default(); + let assembler = Assembler::new(context.source_manager()); let library = assembler.assemble_library([module]).unwrap(); let mast_forest = library.mast_forest(); @@ -5796,29 +5796,14 @@ fn test_linking_recursive_expansion_via_renamed_aliases() -> TestResult { #[test] fn assemble_library_with_mismatched_source_manager_returns_error() { - // Source manager A: used by the assembler. - // Parse a short module to populate SourceId(0) in this manager. + // Source manager A: used by the assembler (empty — no files loaded). let sm_a: Arc = Arc::new(DefaultSourceManager::default()); - let _dummy = Module::parser(ModuleKind::Library) - .parse_str("lib::dummy", "pub proc dummy\n push.1\nend", sm_a.clone()) - .unwrap(); - let assembler = Assembler::new(sm_a); // Source manager B: external, used to parse the module we will assemble. - // Its SourceId(0) points to a different file. let sm_b: Arc = Arc::new(DefaultSourceManager::default()); - let long_source = "\ -# padding line to push the procedure definition past the short file length -# more padding to ensure the byte offset exceeds the boundary -# even more padding for good measure here to be absolutely sure -pub proc bar - push.1 - push.2 - add -end"; let module = Module::parser(ModuleKind::Library) - .parse_str("lib::external", long_source, sm_b) + .parse_str("lib::external", "pub proc bar\n push.1\n push.2\n add\nend", sm_b) .unwrap(); let result = assembler.assemble_library([module]); From 75c9b7c9a37d23465f7b27698b4a606114229aa3 Mon Sep 17 00:00:00 2001 From: Juan Munoz Date: Fri, 10 Apr 2026 16:07:52 -0300 Subject: [PATCH 3/5] docs: add changelog entry --- CHANGELOG.md | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index ca81458e01..c20d759ea4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,37 @@ # Changelog ## 0.22.1 (2026-04-07) +## v0.23.0 (TBD) + +#### Fixes +- Rejected non-syscall references to exported kernel procedures in the linker ([#2902](https://github.com/0xMiden/miden-vm/issues/2902)). +- Reverted the `MainTrace` typed row storage change that caused a large `blake3_1to1` trace-building regression ([#2949](https://github.com/0xMiden/miden-vm/pull/2949)). +#### Bug Fixes + +- Replaced unsound `ptr::read` with safe unbox in panic recovery, removing UB from potential double-drop ([#2934](https://github.com/0xMiden/miden-vm/pull/2934)). +- Reverted `InvokeKind::ProcRef` back to `InvokeKind::Exec` in `visit_mut_procref` and added an explanatory comment (#2893). +- Fixed the release dry-run publish cycle between `miden-air` and `miden-ace-codegen`, and preserved leaf-only DAG imports with explicit snapshots ([#2931](https://github.com/0xMiden/miden-vm/pull/2931)). +- Fixed silent attachment of wrong debug info when a module was parsed with a different `SourceManager` than the assembler's, by using `Arc` pointer identity (`is_manager_of`) instead of `SourceId` bounds checks ([#2987](https://github.com/0xMiden/miden-vm/pull/2987)). + +#### Changes + +- Documented sortedness precondition more prominently for sorted array operations ([#2832](https://github.com/0xMiden/miden-vm/pull/2832)). +- [BREAKING] Sync execution and proving APIs now require `SyncHost`; async `Host`, `execute`, and `prove` remain available ([#2865](https://github.com/0xMiden/miden-vm/pull/2865)). +- [BREAKING] `miden_processor::execute()` and `execute_sync()` now return `ExecutionOutput`; trace building remains explicit via `execute_trace_inputs*()` and `trace::build_trace()` ([#2865](https://github.com/0xMiden/miden-vm/pull/2865)). +- [BREAKING] Removed the deprecated `FastProcessor::execute_sync_mut()` alias; `execute_mut_sync()` is now the only sync mutable-execution entrypoint ([#2865](https://github.com/0xMiden/miden-vm/pull/2865)). +- [BREAKING] Removed the deprecated `FastProcessor::execute_for_trace_sync()` and `execute_for_trace()` wrappers; use `execute_trace_inputs_sync()` or `execute_trace_inputs()` instead ([#2865](https://github.com/0xMiden/miden-vm/pull/2865)). +- [BREAKING] Removed the deprecated unbound `TraceBuildInputs::new()` and `TraceBuildInputs::from_program()` constructors; use `execute_trace_inputs_sync()` or `execute_trace_inputs()` instead ([#2865](https://github.com/0xMiden/miden-vm/pull/2865)). +- Added `prove_from_trace_sync(...)` for proving from pre-executed trace inputs ([#2865](https://github.com/0xMiden/miden-vm/pull/2865)). +- [BREAKING] Reduced the prove-from-trace API to post-execution trace inputs: `TraceBuildInputs` no longer carries full execution output, `prove_from_trace_sync()` takes `TraceProvingInputs`, and `ProvingOptions` no longer include `ExecutionOptions` ([#2948](https://github.com/0xMiden/miden-vm/pull/2948)). +- Redesigned the hasher chiplet to use a controller/permutation split architecture with permutation calls deduplication ([#2927](https://github.com/0xMiden/miden-vm/pull/2927)). +- Documented that enum variants are module-level constants and must be unique within a module ([#2932]((https://github.com/0xMiden/miden-vm/pull/2932)). +- Refactor trace generation to row-major format ([#2937](https://github.com/0xMiden/miden-vm/pull/2937)). +- Documented non-overlap requirement for `memcopy_words`, `memcopy_elements`, and AEAD encrypt/decrypt procedures ([#2941](https://github.com/0xMiden/miden-vm/pull/2941)). +- Added chainable `Test` builders for common test setup in `miden-utils-testing` ([#2957](https://github.com/0xMiden/miden-vm/pull/2957)). +- Speed-up AUX range check trace generation by changing divisors to a flat Vec layout ([#2966](https://github.com/0xMiden/miden-vm/pull/2966)). +## 0.22.1 + +#### Enhancements - Implemented project assembly ([#2877](https://github.com/0xMiden/miden-vm/pull/2877)). - Added `FastProcessor::into_parts()` to extract advice provider, memory, and precompile transcript after step-based execution ([#2901](https://github.com/0xMiden/miden-vm/pull/2901)). From e537d12062934025c378426ab3e02e6070d33ba5 Mon Sep 17 00:00:00 2001 From: Juan Munoz Date: Mon, 13 Apr 2026 12:17:56 -0300 Subject: [PATCH 4/5] chore: adress PR comments --- crates/assembly-syntax/src/ast/module.rs | 44 +++++++++++------------- crates/assembly-syntax/src/sema/mod.rs | 2 +- crates/assembly/src/linker/errors.rs | 7 ++++ crates/assembly/src/linker/mod.rs | 11 +++++- crates/assembly/src/tests.rs | 38 ++++++++++++++++---- 5 files changed, 71 insertions(+), 31 deletions(-) diff --git a/crates/assembly-syntax/src/ast/module.rs b/crates/assembly-syntax/src/ast/module.rs index eb2ad45fc1..a441af7b68 100644 --- a/crates/assembly-syntax/src/ast/module.rs +++ b/crates/assembly-syntax/src/ast/module.rs @@ -5,10 +5,7 @@ use miden_core::{ advice::AdviceMap, serde::{ByteReader, ByteWriter, Deserializable, DeserializationError, Serializable}, }; -use miden_debug_types::{ - SourceContent, SourceFile, SourceId, SourceLanguage, SourceManager, SourceSpan, Span, Spanned, - Uri, -}; +use miden_debug_types::{SourceFile, SourceManager, SourceSpan, Span, Spanned}; use miden_utils_diagnostics::Report; use smallvec::SmallVec; @@ -129,11 +126,15 @@ pub struct Module { pub(crate) items: Vec, /// AdviceMap that this module expects to be loaded in the host before executing. pub(crate) advice_map: AdviceMap, - /// The source file from which this module was parsed. + /// The source file from which this module was parsed, if any. + /// + /// This is `None` for programmatically-constructed modules (e.g. `new_kernel()`, + /// `new_executable()`) that have no backing source text. /// - /// This is used to verify that the source manager used during assembly is the same one that - /// was used to parse this module, via pointer identity ([`SourceManager::is_manager_of`]). - source_file: Arc, + /// When present, it is used to verify that the source manager used during assembly is the + /// same one that was used to parse this module, via pointer identity + /// ([`SourceManager::is_manager_of`]). + source_file: Option>, } /// Constants @@ -152,7 +153,11 @@ impl Module { impl Module { /// Creates a new [Module] with the specified `kind` and fully-qualified path, e.g. /// `std::math::u64`. - pub fn new(kind: ModuleKind, path: impl AsRef, source_file: Arc) -> Self { + pub fn new( + kind: ModuleKind, + path: impl AsRef, + source_file: Option>, + ) -> Self { let path = path.as_ref().to_absolute().into_owned(); Self { span: Default::default(), @@ -167,21 +172,12 @@ impl Module { /// An alias for creating the default, but empty, `#kernel` [Module]. pub fn new_kernel() -> Self { - Self::new(ModuleKind::Kernel, Path::kernel_path(), Self::empty_source_file()) + Self::new(ModuleKind::Kernel, Path::kernel_path(), None) } /// An alias for creating the default, but empty, `$exec` [Module]. pub fn new_executable() -> Self { - Self::new(ModuleKind::Executable, Path::exec_path(), Self::empty_source_file()) - } - - /// Creates a synthetic, empty [`SourceFile`] for programmatically constructed modules - /// that have no backing source text. - fn empty_source_file() -> Arc { - Arc::new(SourceFile::from_raw_parts( - SourceId::default(), - SourceContent::new(SourceLanguage::Masm, Uri::from("synthetic"), String::new()), - )) + Self::new(ModuleKind::Executable, Path::exec_path(), None) } /// Specifies the source span in the source file in which this module was defined, that covers @@ -458,9 +454,11 @@ impl Module { &self.advice_map } - /// Returns the source file from which this module was parsed. - pub fn source_file(&self) -> &Arc { - &self.source_file + /// Returns the source file from which this module was parsed, if any. + /// + /// Returns `None` for programmatically-constructed modules that have no backing source text. + pub fn source_file(&self) -> Option<&Arc> { + self.source_file.as_ref() } /// Get an iterator over the constants defined in this module. diff --git a/crates/assembly-syntax/src/sema/mod.rs b/crates/assembly-syntax/src/sema/mod.rs index a96c4148cd..56355de1c8 100644 --- a/crates/assembly-syntax/src/sema/mod.rs +++ b/crates/assembly-syntax/src/sema/mod.rs @@ -45,7 +45,7 @@ pub fn analyze( analyzer.set_warnings_as_errors(warnings_as_errors); let mut module = - Box::new(Module::new(kind, path, source.clone()).with_span(source.source_span())); + Box::new(Module::new(kind, path, Some(source.clone())).with_span(source.source_span())); let mut forms = VecDeque::from(forms); let mut enums = SmallVec::<[EnumType; 1]>::new_const(); diff --git a/crates/assembly/src/linker/errors.rs b/crates/assembly/src/linker/errors.rs index c87d87a75c..3c7b600423 100644 --- a/crates/assembly/src/linker/errors.rs +++ b/crates/assembly/src/linker/errors.rs @@ -91,6 +91,13 @@ pub enum LinkerError { prev_values: Vec, new_values: Vec, }, + #[error( + "source manager mismatch: module '{path}' was parsed with a different source manager than the one used by the assembler" + )] + #[diagnostic(help( + "ensure the module is parsed with the same source manager that is passed to the assembler" + ))] + SourceManagerMismatch { path: Arc }, #[error("undefined type alias")] #[diagnostic()] UndefinedType { diff --git a/crates/assembly/src/linker/mod.rs b/crates/assembly/src/linker/mod.rs index c57b146ef3..e8d44b860b 100644 --- a/crates/assembly/src/linker/mod.rs +++ b/crates/assembly/src/linker/mod.rs @@ -277,6 +277,15 @@ impl Linker { pub fn link_module(&mut self, module: &mut Module) -> Result { log::debug!(target: "linker", "adding unprocessed module {}", module.path()); + // Reject the module early if it was parsed with a different source manager. + // Compiling a mismatched module would use the wrong source manager for span + // resolution, which can produce garbled diagnostics or panics. + if let Some(source_file) = module.source_file() + && !self.source_manager.is_manager_of(source_file) + { + return Err(LinkerError::SourceManagerMismatch { path: module.path().into() }); + } + let is_duplicate = self.find_module_index(module.path()).is_some(); if is_duplicate { return Err(LinkerError::DuplicateModule { path: module.path().into() }); @@ -316,7 +325,7 @@ impl Linker { module.path().into(), ) .with_advice_map(module.advice_map().clone()) - .with_source_file(Some(module.source_file().clone())) + .with_source_file(module.source_file().cloned()) .with_symbols(symbols); self.modules.push(link_module); diff --git a/crates/assembly/src/tests.rs b/crates/assembly/src/tests.rs index 2bbe4992ba..7563f2dee2 100644 --- a/crates/assembly/src/tests.rs +++ b/crates/assembly/src/tests.rs @@ -11,8 +11,8 @@ use std::{ use miden_assembly_syntax::{ MAX_REPEAT_COUNT, - ast::Path, - debuginfo::{DefaultSourceManager, SourceManager}, + ast::{Block, Instruction, Op, Path, Procedure, Visibility}, + debuginfo::{DefaultSourceManager, SourceManager, Span}, diagnostics::WrapErr, library::LibraryExport, }; @@ -5795,15 +5795,22 @@ fn test_linking_recursive_expansion_via_renamed_aliases() -> TestResult { } #[test] -fn assemble_library_with_mismatched_source_manager_returns_error() { - // Source manager A: used by the assembler (empty — no files loaded). +fn mismatched_source_manager_caught_before_lowering() { + // Source manager A (assembler's): pre-load a short file so it occupies SourceId(0). let sm_a: Arc = Arc::new(DefaultSourceManager::default()); + let _dummy = Module::parser(ModuleKind::Library) + .parse_str("lib::dummy", "pub proc dummy\n nop\nend", sm_a.clone()) + .unwrap(); + let assembler = Assembler::new(sm_a); - // Source manager B: external, used to parse the module we will assemble. + // Source manager B (external): parse a module whose code will trigger a compile error + // during lowering. `loc_loadw_be.2` fails because 2 is not word-aligned (must be a + // multiple of 4). The error formatting tries to resolve the instruction's source span + // via the assembler's source manager, which returns the wrong file. let sm_b: Arc = Arc::new(DefaultSourceManager::default()); let module = Module::parser(ModuleKind::Library) - .parse_str("lib::external", "pub proc bar\n push.1\n push.2\n add\nend", sm_b) + .parse_str("lib::external", "@locals(4) pub proc bar\n loc_loadw_be.2\nend", sm_b) .unwrap(); let result = assembler.assemble_library([module]); @@ -5811,3 +5818,22 @@ fn assemble_library_with_mismatched_source_manager_returns_error() { let msg = err.to_string(); assert!(msg.contains("source manager mismatch"), "unexpected error: {msg}"); } + +#[test] +fn programmatic_module_without_source_file_assembles_ok() -> TestResult { + let context = TestContext::default(); + + let mut module = Module::new(ModuleKind::Library, "lib::programmatic", None); + let nop_body = Block::new(Default::default(), vec![Op::Inst(Span::unknown(Instruction::Nop))]); + let procedure = Procedure::new( + Default::default(), + Visibility::Public, + "nop_proc".parse().unwrap(), + 0, + nop_body, + ); + module.define_procedure(procedure, context.source_manager()).unwrap(); + + context.assemble_library([alloc::boxed::Box::new(module)])?; + Ok(()) +} From 31714a0cbf6925a62ac105f268568e64417e2ab4 Mon Sep 17 00:00:00 2001 From: Juan Munoz Date: Mon, 13 Apr 2026 15:43:55 -0300 Subject: [PATCH 5/5] chore: fix changelog --- CHANGELOG.md | 31 ------------------------------- 1 file changed, 31 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c20d759ea4..ca81458e01 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,37 +1,6 @@ # Changelog ## 0.22.1 (2026-04-07) -## v0.23.0 (TBD) - -#### Fixes -- Rejected non-syscall references to exported kernel procedures in the linker ([#2902](https://github.com/0xMiden/miden-vm/issues/2902)). -- Reverted the `MainTrace` typed row storage change that caused a large `blake3_1to1` trace-building regression ([#2949](https://github.com/0xMiden/miden-vm/pull/2949)). -#### Bug Fixes - -- Replaced unsound `ptr::read` with safe unbox in panic recovery, removing UB from potential double-drop ([#2934](https://github.com/0xMiden/miden-vm/pull/2934)). -- Reverted `InvokeKind::ProcRef` back to `InvokeKind::Exec` in `visit_mut_procref` and added an explanatory comment (#2893). -- Fixed the release dry-run publish cycle between `miden-air` and `miden-ace-codegen`, and preserved leaf-only DAG imports with explicit snapshots ([#2931](https://github.com/0xMiden/miden-vm/pull/2931)). -- Fixed silent attachment of wrong debug info when a module was parsed with a different `SourceManager` than the assembler's, by using `Arc` pointer identity (`is_manager_of`) instead of `SourceId` bounds checks ([#2987](https://github.com/0xMiden/miden-vm/pull/2987)). - -#### Changes - -- Documented sortedness precondition more prominently for sorted array operations ([#2832](https://github.com/0xMiden/miden-vm/pull/2832)). -- [BREAKING] Sync execution and proving APIs now require `SyncHost`; async `Host`, `execute`, and `prove` remain available ([#2865](https://github.com/0xMiden/miden-vm/pull/2865)). -- [BREAKING] `miden_processor::execute()` and `execute_sync()` now return `ExecutionOutput`; trace building remains explicit via `execute_trace_inputs*()` and `trace::build_trace()` ([#2865](https://github.com/0xMiden/miden-vm/pull/2865)). -- [BREAKING] Removed the deprecated `FastProcessor::execute_sync_mut()` alias; `execute_mut_sync()` is now the only sync mutable-execution entrypoint ([#2865](https://github.com/0xMiden/miden-vm/pull/2865)). -- [BREAKING] Removed the deprecated `FastProcessor::execute_for_trace_sync()` and `execute_for_trace()` wrappers; use `execute_trace_inputs_sync()` or `execute_trace_inputs()` instead ([#2865](https://github.com/0xMiden/miden-vm/pull/2865)). -- [BREAKING] Removed the deprecated unbound `TraceBuildInputs::new()` and `TraceBuildInputs::from_program()` constructors; use `execute_trace_inputs_sync()` or `execute_trace_inputs()` instead ([#2865](https://github.com/0xMiden/miden-vm/pull/2865)). -- Added `prove_from_trace_sync(...)` for proving from pre-executed trace inputs ([#2865](https://github.com/0xMiden/miden-vm/pull/2865)). -- [BREAKING] Reduced the prove-from-trace API to post-execution trace inputs: `TraceBuildInputs` no longer carries full execution output, `prove_from_trace_sync()` takes `TraceProvingInputs`, and `ProvingOptions` no longer include `ExecutionOptions` ([#2948](https://github.com/0xMiden/miden-vm/pull/2948)). -- Redesigned the hasher chiplet to use a controller/permutation split architecture with permutation calls deduplication ([#2927](https://github.com/0xMiden/miden-vm/pull/2927)). -- Documented that enum variants are module-level constants and must be unique within a module ([#2932]((https://github.com/0xMiden/miden-vm/pull/2932)). -- Refactor trace generation to row-major format ([#2937](https://github.com/0xMiden/miden-vm/pull/2937)). -- Documented non-overlap requirement for `memcopy_words`, `memcopy_elements`, and AEAD encrypt/decrypt procedures ([#2941](https://github.com/0xMiden/miden-vm/pull/2941)). -- Added chainable `Test` builders for common test setup in `miden-utils-testing` ([#2957](https://github.com/0xMiden/miden-vm/pull/2957)). -- Speed-up AUX range check trace generation by changing divisors to a flat Vec layout ([#2966](https://github.com/0xMiden/miden-vm/pull/2966)). -## 0.22.1 - -#### Enhancements - Implemented project assembly ([#2877](https://github.com/0xMiden/miden-vm/pull/2877)). - Added `FastProcessor::into_parts()` to extract advice provider, memory, and precompile transcript after step-based execution ([#2901](https://github.com/0xMiden/miden-vm/pull/2901)).