Skip to content

Fix Memory64Lowering table.grow return value for -1 failure#8318

Merged
kripken merged 3 commits intoWebAssembly:mainfrom
sumleo:fix-memory64-table-grow
Feb 13, 2026
Merged

Fix Memory64Lowering table.grow return value for -1 failure#8318
kripken merged 3 commits intoWebAssembly:mainfrom
sumleo:fix-memory64-table-grow

Conversation

@sumleo
Copy link
Contributor

@sumleo sumleo commented Feb 12, 2026

Summary

visitTableGrow used a plain i64.extend_i32_u on the table.grow return value, but table.grow returns -1 (i32) on failure. i64.extend_i32_u(0xFFFFFFFF) produces 0x00000000FFFFFFFF (4294967295), not i64 -1 (0xFFFFFFFFFFFFFFFF). Code checking result == -1 would fail to detect the failure.

The analogous visitMemoryGrow already has the correct handling: it uses a local.tee + if/else to check for -1 and return i64.const -1 explicitly.

Fix: Apply the same if/else -1 check pattern from visitMemoryGrow to visitTableGrow.

Split from #8311 per review feedback.

Test plan

  • Updated existing memory64-lowering.wast test expectations
  • New lit test memory64-lowering-table-grow.wast verifying the -1 handling
  • All 309 unit tests pass

visitTableGrow used a plain i64.extend_i32_u on the table.grow
return value, but table.grow returns -1 on failure.
i64.extend_i32_u(0xFFFFFFFF) produces 0x00000000FFFFFFFF
(4294967295), not i64 -1 (0xFFFFFFFFFFFFFFFF). Code checking
result == -1 would fail to detect the failure.

Apply the same if/else -1 check pattern already used by
visitMemoryGrow.
(func $table-grow-minus-one (result i64)
;; table.grow returns -1 on failure. The lowering must check for -1 and
;; return i64(-1) instead of i64.extend_i32_u(i32(-1)) which would be
;; 4294967295.
Copy link
Member

Choose a reason for hiding this comment

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

This looks like it duplicates the existing test in the file below? Let's just use that one, but please move the comment from here to there.

- Add comment explaining why curr->type is set to i32 (the table.grow
  node is reused as a child of local.tee storing to an i32 local)
- Remove duplicate test file memory64-lowering-table-grow.wast
- Move the -1 failure explanation comment to the existing test_table_grow
  function in memory64-lowering.wast
@AI-Reviewer-QS
Copy link

Addressed both comments:

  1. curr->type = Type::i32: The table.grow node is reused as a child of local.tee which stores to an i32 local, so it needs to be i32 to match the lowered table. This is the same pattern as visitMemoryGrow at line 137. Added a comment to clarify.

  2. Test consolidation: Removed the duplicate test file memory64-lowering-table-grow.wast and moved the -1 failure explanation comment to the existing $test_table_grow function in memory64-lowering.wast.

Copy link
Member

@kripken kripken left a comment

Choose a reason for hiding this comment

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

Looks good, thanks. Let's just remove the comment - good point that it is identical to the code above, and it isn't commented there either, this is just a general pattern in this file. So let's not make this place inconsistent.

@kripken kripken enabled auto-merge (squash) February 13, 2026 17:41
@kripken kripken merged commit da18c0d into WebAssembly:main Feb 13, 2026
17 checks passed
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.

3 participants