Skip to content

Validate global imports#8487

Open
stevenfontanella wants to merge 1 commit intomainfrom
global-validation
Open

Validate global imports#8487
stevenfontanella wants to merge 1 commit intomainfrom
global-validation

Conversation

@stevenfontanella
Copy link
Member

@stevenfontanella stevenfontanella commented Mar 18, 2026

  • Add a RuntimeGlobal class to wrap the global's runtime value as well as its definition so we can check its type + mutability for import validation
    • Global imports are invariant for mutable globals and covariant for immutable globals:
      image. Mutability must match as well.
  • Fix old_import.wast to account for instantiations which should correctly fail. This test is now mostly the same as test/spec/imports.wast, but there are small differences so I plan to keep it until the upstream testsuite imports.wast passes.

Example error message:

[trap Imported global Mref_ex.g-var-func with definition (global $global$6 (mut (ref func)) (ref.func $f))
 isn't compatible with import declaration: (import "Mref_ex" "g-var-func" (global $gimport$0 (mut funcref)))
]

Part of #8261.

'imports.wast', # Requires fixing handling of mutation to imported globals
'proposals/threads/imports.wast', # Missing memory type validation on instantiation
'linking.wast', # Missing global type validation on instantiation
'linking.wast', # Incorrectly allows subtyping for table imports
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this mean the test is wrong or that our implementation is wrong?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Our implementation is wrong, table types should be invariant for imports but I implemented them as covariant (same mistake as Java): link. I'll address this in the next PR.

Image

bool isSubType(const Global& global) const;

private:
const Global definition;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we have this be a pointer to the underlying definition owned by some defining module? It's a little weird to have a Global that is just hanging out and not owned by some module.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that would make the lifetimes/pointers more complicated. Now we have

ModuleRunner -> RuntimeGlobal

If we made the global a pointer, it would be

ModuleRunner -> RuntimeGlobal -> ModuleRunner -> Module

Basically the RuntimeGlobal would have to point back at the Module owned by ModuleRunner. In practice everything will live as long as the ModuleRunner, but it would be hard to tell from reading the code and if more complicated cases some up.

I also think that it's ok for a Global to stand on its own. It might be instantiated in different modules, so the Global itself isn't tied to a particular instance, only the RuntimeGlobal is. Does that seem reasonable?

@stevenfontanella stevenfontanella force-pushed the global-validation branch 2 times, most recently from 2591c89 to ba2c167 Compare March 18, 2026 19:43
@stevenfontanella stevenfontanella changed the title Validation for imported globals Validate global imports Mar 18, 2026
@stevenfontanella stevenfontanella marked this pull request as ready for review March 18, 2026 20:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants