Skip to content

Fix array.new_default validation for non-defaultable element types (#4853)#4924

Open
orange-dot wants to merge 1 commit intobytecodealliance:mainfrom
orange-dot:fix/4853-array-new-default-validation
Open

Fix array.new_default validation for non-defaultable element types (#4853)#4924
orange-dot wants to merge 1 commit intobytecodealliance:mainfrom
orange-dot:fix/4853-array-new-default-validation

Conversation

@orange-dot
Copy link
Copy Markdown

Summary

Fix loader validation so array.new_default rejects array types whose element type is non-defaultable.

Root Cause

The wasm/interpreter loader accepted array.new_default for array element reference types that cannot have a default value, which allowed an invalid module to get past validation.

Changes

  • add a narrow defaultability check for array.new_default in the wasm/interpreter loader path
  • keep array.new, array.new_fixed, and runtime/AOT paths unchanged
  • add a focused negative GC regression fixture and test for raw .wasm load

Impact

Invalid .wasm modules using array.new_default with non-defaultable element types now fail in the wasm/interpreter load/validation path instead of being accepted.

This PR does not extend the same validation to external or pre-existing .aot artifacts.

Validation

  • configured and built the focused tests/unit/gc lane in build/unit-gc-4853
  • ran ctest --test-dir build/unit-gc-4853 -R WasmGCTest --output-on-failure
  • verified all focused GC tests passed, including the new negative regression
  • ran a broader local tests/unit X86_64 FULL_TEST=ON validation pass and got 1763/1768 passing
  • the targeted regression WasmGCTest.Test_array_new_default_non_defaultable_elem also passed in that broader run
  • the remaining 5 failures appeared to be pre-existing and outside this change's scope: SSP null-parameter handling, thread-manager termination tests, two llm-enhanced-test AOT/toolchain-dependent cases, and one host-resource-heavy memory64 8GB case

Notes

  • This PR is intentionally narrow and does not claim that the full pre-existing suite is green.
  • This PR intentionally does not claim AOT coverage.
  • A local AOT follow-up was explored separately but is not part of this PR.
  • The focused GC signal for this change is green.
  • The residual failures do not appear to be in the loader or GC paths touched by this change.

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.

1 participant