diff --git a/CHANGELOG.md b/CHANGELOG.md index ca81458e01..a0e94e45dc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ - 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)). +- Fixed assembler panics caused by mixing modules parsed with one source manager into an assembler created with another source manager; these cases now return a structured linker error instead ([#2987](https://github.com/0xMiden/miden-vm/pull/2987)). ## 0.22.0 (2026-03-18) diff --git a/Cargo.lock b/Cargo.lock index 73ca6d82b8..dbdbacd7a7 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1350,7 +1350,7 @@ dependencies = [ "num", "num-bigint", "pretty_assertions", - "rand 0.9.2", + "rand 0.9.3", "rand_chacha", "rstest", "serde_json", @@ -1387,7 +1387,7 @@ dependencies = [ "p3-miden-lifted-stark", "p3-symmetric", "p3-util", - "rand 0.9.2", + "rand 0.9.3", "rand_chacha", "rand_core 0.9.5", "rand_hc", @@ -2396,7 +2396,7 @@ checksum = "4b45fcc2344c680f5025fe57779faef368840d0bd1f42f216291f0dc4ace4744" dependencies = [ "bitflags", "num-traits", - "rand 0.9.2", + "rand 0.9.3", "rand_chacha", "rand_xorshift", "regex-syntax", @@ -2451,9 +2451,9 @@ checksum = "f8dcc9c7d52a811697d2151c701e0d08956f92b0e24136cf4cf27b57a6a0d9bf" [[package]] name = "rand" -version = "0.9.2" +version = "0.9.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "6db2770f06117d490610c7488547d543617b21bfa07796d7a12f6f1bd53850d1" +checksum = "7ec095654a25171c2124e9e3393a930bddbffdc939556c914957a4c3e0a87166" dependencies = [ "rand_chacha", "rand_core 0.9.5", diff --git a/crates/assembly-syntax/src/ast/module.rs b/crates/assembly-syntax/src/ast/module.rs index a7fd99004b..7b7e24eb46 100644 --- a/crates/assembly-syntax/src/ast/module.rs +++ b/crates/assembly-syntax/src/ast/module.rs @@ -126,6 +126,10 @@ 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, if any. + /// + /// Programmatically-constructed modules do not have a backing source file. + source_file: Option>, } /// Constants @@ -153,6 +157,7 @@ impl Module { kind, items: Default::default(), advice_map: Default::default(), + source_file: None, } } @@ -173,6 +178,12 @@ impl Module { self } + /// Associates the source file from which this module was parsed. + pub(crate) fn with_source_file(mut self, source_file: Arc) -> Self { + self.source_file = Some(source_file); + self + } + /// Sets the [Path] for this module pub fn set_path(&mut self, path: impl AsRef) { self.path = path.as_ref().to_path_buf(); @@ -440,6 +451,11 @@ impl Module { &self.advice_map } + /// Returns the source file from which this module was parsed, if any. + pub fn source_file(&self) -> Option<&Arc> { + self.source_file.as_ref() + } + /// 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..302d05d566 100644 --- a/crates/assembly-syntax/src/sema/mod.rs +++ b/crates/assembly-syntax/src/sema/mod.rs @@ -44,7 +44,11 @@ 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) + .with_source_file(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 8962ebe95c..7415a92221 100644 --- a/crates/assembly/src/linker/mod.rs +++ b/crates/assembly/src/linker/mod.rs @@ -55,7 +55,7 @@ use miden_assembly_syntax::{ self, Alias, AttributeSet, GlobalItemIndex, InvocationTarget, InvokeKind, ItemIndex, Module, ModuleIndex, Path, SymbolResolution, Visibility, types, }, - debuginfo::{SourceManager, SourceSpan, Span, Spanned}, + debuginfo::{SourceFile, SourceManager, SourceSpan, Span, Spanned}, library::{ItemInfo, ModuleInfo}, }; use miden_core::{Word, advice::AdviceMap, program::Kernel}; @@ -74,6 +74,13 @@ use self::{ resolver::*, }; +fn source_manager_owns_file(source_manager: &dyn SourceManager, file: &SourceFile) -> bool { + match source_manager.get(file.id()) { + Ok(found) => core::ptr::addr_eq(Arc::as_ptr(&found), file), + Err(_) => false, + } +} + /// Represents the current status of a symbol in the state of the [Linker] #[derive(Debug, Default, Copy, Clone, PartialEq, Eq)] pub enum LinkStatus { @@ -277,6 +284,12 @@ impl Linker { pub fn link_module(&mut self, module: &mut Module) -> Result { log::debug!(target: "linker", "adding unprocessed module {}", module.path()); + if let Some(source_file) = module.source_file() + && !source_manager_owns_file(self.source_manager.as_ref(), 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() }); diff --git a/crates/assembly/src/tests.rs b/crates/assembly/src/tests.rs index 2c443bf035..b763ab55b3 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::{Block, Instruction, Op, Path, Procedure, Visibility}, + debuginfo::{DefaultSourceManager, SourceManager, Span}, + diagnostics::WrapErr, + library::LibraryExport, }; use miden_core::{ Felt, Word, assert_matches, @@ -4115,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 = { @@ -4124,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() }; @@ -4140,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 @@ -5097,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(); @@ -5789,3 +5793,80 @@ fn test_linking_recursive_expansion_via_renamed_aliases() -> TestResult { Ok(()) } + +#[test] +fn mismatched_source_manager_caught_before_lowering() { + 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); + + let sm_b: Arc = Arc::new(DefaultSourceManager::default()); + let module = Module::parser(ModuleKind::Library) + .parse_str("lib::external", "@locals(4) pub proc bar\n loc_loadw_be.2\nend", 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}"); +} + +#[test] +fn issue_2778_parser_api_source_manager_mismatch_is_reported_without_panic() { + use std::panic::{AssertUnwindSafe, catch_unwind}; + + let assembler_source_manager: Arc = + Arc::new(DefaultSourceManager::default()); + let parser_source_manager: Arc = Arc::new(DefaultSourceManager::default()); + + let source = r#" +use miden::protocol::output_note + +@locals(1) +pub proc create_withdraw_return_note(tag: felt, note_type: felt, recipient: word, amount_0_out: felt, amount_1_out: felt) + # loc.0: note_id + # => [tag, aux, note_type, execution_hint, PAYBACK_NOTE_RECIPIENT] + exec.output_note::create_note + # => [note_id] + loc_store.0 + # => [] +end +"#; + let module = Module::parser(ModuleKind::Library) + .parse_str("test::lib", source, parser_source_manager) + .unwrap(); + + let assembled = catch_unwind(AssertUnwindSafe(|| { + Assembler::new(assembler_source_manager).assemble_library([module]) + })); + + assert!( + assembled.is_ok(), + "assembler panicked while reporting a source manager mismatch" + ); + let err = assembled.unwrap().expect_err("should fail with source manager mismatch"); + let rendered = err.to_string(); + assert!(rendered.contains("source manager mismatch"), "unexpected error: {rendered}"); +} + +#[test] +fn programmatic_module_without_source_file_assembles_ok() -> TestResult { + let context = TestContext::default(); + + let mut module = Module::new(ModuleKind::Library, "lib::programmatic"); + 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(()) +} diff --git a/crates/debug-types/src/source_file.rs b/crates/debug-types/src/source_file.rs index c8b36a1261..4df8436039 100644 --- a/crates/debug-types/src/source_file.rs +++ b/crates/debug-types/src/source_file.rs @@ -191,12 +191,20 @@ impl SourceFile { Some(SourceSpan::at(self.id, offset.0)) } + /// Get a [FileLineCol] equivalent to the start of the given [SourceSpan] + pub(crate) fn try_location(&self, span: SourceSpan) -> Option { + if span.source_id() != self.id { + return None; + } + + self.content.location(ByteIndex(span.into_range().start)) + } + /// 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"); - self.content - .location(ByteIndex(span.into_range().start)) + self.try_location(span) .expect("invalid source span: starting byte is out of bounds") } } diff --git a/crates/debug-types/src/source_manager.rs b/crates/debug-types/src/source_manager.rs index a798e52ac6..f46eebd2e1 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.try_location(span).ok_or(SourceManagerError::InvalidBounds)) } fn location_to_span(&self, loc: Location) -> Option {