Skip to content

Fix array.init_elem to use source offset when reading segment data#8349

Open
sumleo wants to merge 1 commit intoWebAssembly:mainfrom
sumleo:fix-array-init-elem-offset
Open

Fix array.init_elem to use source offset when reading segment data#8349
sumleo wants to merge 1 commit intoWebAssembly:mainfrom
sumleo:fix-array-init-elem-offset

Conversation

@sumleo
Copy link
Contributor

@sumleo sumleo commented Feb 20, 2026

Summary

  • Fixed a bug in the interpreter's visitArrayInitElem where the source offset parameter was ignored when reading from the element segment
  • The loop used seg->data[i] instead of seg->data[offsetVal + i], always reading from the start of the segment regardless of the specified offset
  • The offset was already correctly computed and validated in the bounds check but not used in the actual data access, now consistent with visitArrayInitData

Test plan

  • Builds successfully
  • All 309 unit tests pass (binaryen-unittests)

The interpreter's visitArrayInitElem was ignoring the source offset
parameter when reading from the element segment. It used seg->data[i]
instead of seg->data[offsetVal + i], meaning it always read from the
beginning of the segment regardless of the specified offset.

The offset was already correctly computed and used in the bounds check,
but not in the actual data access loop. This is now consistent with how
visitArrayInitData handles its offset parameter.
@sumleo sumleo force-pushed the fix-array-init-elem-offset branch from e1542a7 to 33b75d2 Compare February 21, 2026 02:42
// in the interpreter.
data->values[indexVal + i] = self()->visit(seg->data[i]).getSingleValue();
data->values[indexVal + i] =
self()->visit(seg->data[offsetVal + i]).getSingleValue();
Copy link
Member

@kripken kripken Feb 23, 2026

Choose a reason for hiding this comment

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

Hmm, this looks right, but the spec test array_init_elem.wast does not appear to be disabled, and it appears to have tests with a nonzero offset... how is that possible? @tlively @stevenfontanella any ideas?

Copy link
Member

Choose a reason for hiding this comment

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

It looks like the segment used in that test only contains the same reference over and over, so there doesn't end up being a difference when we just read the first element repeatedly.

It's also troubling that the TODO a few lines up doesn't cause us to fail any tests!

Copy link
Member

Choose a reason for hiding this comment

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

@tlively should we file a bug against the spec or spec test suite or something like that?

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