Fix boxed-object crash/garbage in isDynamicImport's shared-lib load - #262
Merged
Merged
Conversation
The isDynamicImport branch (the code path every ordinary `import '...'` actually takes, per mlirGenImportSharedLib's dynamic-by-default '.' hack) resolved a cross-module symbol's address and read sizeof(reconstructed type) bytes directly there. For an untyped/inferred object-literal export, the exporter's global is a single boxed pointer (ObjectType lowers to a bare `ptr`) to a separately-allocated tuple, not the tuple inline - so the load read the pointer's own 8 bytes as the first field and unrelated adjacent memory as the rest, producing garbage values or a JIT-lookup crash on `0x100000000`-shaped wild addresses (reported with a live repro and full stack trace). Re-adds a `@boxed` decorator (DeclarationPrinter.cpp emits it, matching the mechanism removed in 5b33995) but fixes where it's actually consumed this time: isDynamicImport's load, not the dead declarationMode-only branch from that earlier attempt. When set, loads the boxed pointer first, then loads through it - two dereferences instead of one. Re-enables both `object-literal-untyped` tests (disabled since they previously hung/crashed) - they now pass, including direct field read/mutate and the interface-cast path. 732/732 suite, zero regressions, zero disabled.
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
isDynamicImport(the code path every ordinaryimport '...'actually takes) read a cross-module symbol's data inline, but an untyped/inferred object-literal export's symbol is actually a single boxed pointer to a separately-allocated tuple, not the tuple inline. This caused garbage field values or a JIT-lookup crash on wild addresses.@boxeddeclaration decorator (same mechanism removed in 5b33995), but consumed correctly this time: insideisDynamicImport's load, which loads the pointer then dereferences through it, instead of the deaddeclarationMode-only branch used previously.object-literal-untypedtests (disabled after PR Round-trip boxed-ness across @dllimport boundary via a sibling @boxed decorator #261 since they hung/crashed at the time) — they now pass end-to-end.Reported with a live repro (
I:\Playground\1.ts) and full crash stack trace showing the JIT-lookup failure.Test plan