Allow initializer expressions to reference defined immutable globals - #2806
Open
ANAMASGARD wants to merge 3 commits into
Open
Allow initializer expressions to reference defined immutable globals#2806ANAMASGARD wants to merge 3 commits into
ANAMASGARD wants to merge 3 commits into
Conversation
The validator rejected global.get of any non-imported global inside initializer expressions (elem/data offsets and global initializers), matching an old spec restriction. The spec now allows referencing defined immutable globals; only mutable globals remain forbidden. Remove the imported-only check in SharedValidator::OnGlobalGet and publish each defined global to the validator only after its initializer has been validated, so a global is visible to later globals but not to itself (self- and forward references stay rejected). Apply the same ordering fix in both validator.cc and BinaryReaderInterp. Update spec goldens via vendored copies of elem/data/global under test/old-spec (the pinned testsuite predates the spec change) and add test/regress/issue-2526.txt. Signed-off-by: Gaurav Chaudhary <chaudharygaurav2004@gmail.com>
Fixes WebAssembly#2526 by removing the obsolete imported-global restriction while preserving mutable-global rejection and publish-after-init ordering in both Validator and BinaryReaderInterp. Split OnGlobalType from OnGlobal so declaration validation runs before init-expr validation without delaying CheckReferenceType. Gate publication on a successful type check. Add runtime-observable regressions (call_indirect, memory load, global chain), minimal old-spec patches for core/proposal suites, and teach update-spec-tests.py about old-spec overrides.
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.
Fixes #2526
Problem
WABT currently rejects
global.getof any non-imported global in initializer expressions. This prevents element/data segment offsets and global initializers from referencing immutable globals defined earlier in the same module.The current WebAssembly validation rules allow
global.getto reference a previously defined immutable global. The referenced global must still be immutable and previously defined; self-references, forward references, invalid indexes, and wrong types remain invalid.Fix
Remove the imported-global restriction from
SharedValidator::OnGlobalGetwhile preserving the mutable-global check.Split global handling into
OnGlobalType(declaration validation viaCheckReferenceType) andOnGlobal(publication intoglobals_). Validate each initializer before publishing the global so it is visible to later globals but not to itself. Failed type checks do not publish the global.Apply the same ordering semantics to both
Validator::CheckModuleandBinaryReaderInterp.Remove the now-unused
num_imported_globals_bookkeeping.Tests
Add regression coverage in
test/regress/issue-2526.txtfor:call_indirect→ 42)i32.load8_u→ 97)$a→$b→$c)Update affected spec-test expectations using minimal patched snapshots under
test/old-spec/(coreglobal/elem/dataplus proposal suites) and extendtest/update-spec-tests.pyto preserve those overrides during regeneration.Test plan
./test/run-tests.py issue-2526./test/run-tests.py spec/global spec/elem spec/data./test/run-tests.py wasm2c/spec/global wasm2c/spec/elem wasm2c/spec/datacmake --build out --target run-unittestscmake --build out --target run-c-api-testscmake --build out --target run-tests--enable-all,--enable-extended-conston valid reprowasm-validate/wasm-interppaths