fix: disambiguate import resolution by module+name (closes #97)#100
Merged
fix: disambiguate import resolution by module+name (closes #97)#100
Conversation
When a core module imports two functions with the same field name from different host modules — e.g., (import "wasi:clocks/wall-clock@0.2.0" "now" (func ...)) (import "wasi:clocks/monotonic-clock@0.2.0" "now" (func ...)) — the merger's intra-component resolution lookup matched on `import.name` only and would route both `now` callers to whichever ModuleResolution happened to be first in the list. The losing caller ended up pointing at a void-returning shim where its callers expected an i64 return, producing fused bytecode that fails wasmparser validation with "type mismatch: expected i32 but nothing on stack". Found via `meld fuse --validate` on TinyGo's `file_ops_component.wasm` (reported as issue #97). The TinyGo runtime calls both clock `now`s, and the wall-clock-now site was rewritten as `call <monotonic-now-shim>` which is `(param i32) -> ()` instead of `(result i64)`, so the call expected an i32 on the stack and the subsequent `local.set` had nothing to consume. Fix: * resolver.rs adds `from_import_module` to ModuleResolution and populates it at the three call sites that push resolutions. * merger.rs gates the four lookup sites on both module and name (empty `from_import_module` is treated as a wildcard for backward compat with synthesized resolutions). Adds tests/file_ops_validate.rs as a regression test that fuses the upstream fixture and asserts wasmparser::Validator passes. The fixture is currently gitignored (*.wasm); the test no-ops if missing so this commit is CI-safe but the test only catches regressions locally. Follow-up: replace with a synthetic minimal fixture. Verified: * cargo test --package meld-core --lib: 175/175 pass * cargo test --package meld-core: ~287 tests across 14 suites pass * `meld fuse --validate /tmp/file_ops.wasm`: now passes (was failing with "type mismatch" at offset 0x1e449 in func 131) Closes #97. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The first cut depended on a 844K file_ops_component.wasm under tests/issue_97/, which is gitignored by *.wasm so the test silently skipped in CI. Replace with a synthetic minimal P2 component built inline via wasm-encoder: - Provider module 0 exports `now: () -> (i64, i32)` (wall-clock-style) - Provider module 1 exports `now: () -> i64` (monotonic-clock-style) - Consumer module 2 imports both `now`s under different host module names (wasi:clocks/wall-clock and wasi:clocks/monotonic-clock) Wired through an InstanceSection so both imports resolve to distinct providers inside the same component. Verified the test catches the regression: temporarily reverting the four from_import_module filter clauses in merger.rs makes the test fail with `type mismatch: values remaining on stack at end of block`. Drops the FIXTURE constant, fixture_exists() helper, and silent skip path. The test now runs in CI without external assets. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #97. The intra-component import resolution lookup matched on import field name only, conflating two
nowimports fromwasi:clocks/wall-clockandwasi:clocks/monotonic-clock. The losing caller's call site was rewritten to point at a void-returning shim where its callers expected an i64 return — producing fused bytecode that failedwasm-tools validatewithtype mismatch: expected i32 but nothing on stack.Changes
meld-core/src/resolver.rsaddsfrom_import_module: StringtoModuleResolutionand populates it at the three call sites that push resolutions.meld-core/src/merger.rsrequiresres.from_import_module == imp.module(in addition to the existing name match) at the four lookup sites. Emptyfrom_import_moduleis treated as a wildcard for backward compat with synthesized resolutions.meld-core/tests/file_ops_validate.rsregression test that fuses the upstream fixture and assertswasmparser::Validatorpasses.Test plan
cargo test --package meld-core --lib— 175/175 passcargo test --package meld-core— ~287 tests across 14 suites passcargo test --test file_ops_validate— passes locally with fixture presentmeld fuse --validate /tmp/file_ops.wasm) now reportsValidation passedKnown follow-up
The fixture is gitignored (
*.wasm); the test no-ops if missing, so this PR is CI-safe but the regression is only caught locally. A follow-up will replace the fixture with a synthetic minimal component built viawasm-encoder.🤖 Generated with Claude Code