Skip to content

[10.0.0]: x64: Remove recursion in to_amode helper #6997

Merged
alexcrichton merged 2 commits into
bytecodealliance:release-10.0.0from
alexcrichton:amode-10
Sep 11, 2023
Merged

[10.0.0]: x64: Remove recursion in to_amode helper #6997
alexcrichton merged 2 commits into
bytecodealliance:release-10.0.0from
alexcrichton:amode-10

Conversation

@alexcrichton
Copy link
Copy Markdown
Member

This is a backport of #6968 to the 10.0.0 release branch of Wasmtime. Note that AArch64 is not backported as its recursion is not present on this branch.

This commit removes the recursion present in the x64 backend's
`to_amode` and `to_amode_add` helpers. The recursion is currently
unbounded and controlled by user input meaning it's not too hard to
craft user input which triggers stack overflow in the host. By removing
recursion there's no need to worry about this since the stack depth will
never be hit.

The main concern with removing recursion is that code quality may not be
quite as good any more. The purpose of the recursion here is to "hunt
for constants" and update the immediate `Offset32`, and now without
recursion only at most one constant is found and folded instead of an
arbitrary number of constants as before. This should continue to produce
the same code as before so long as optimizations are enabled, but
without optimizations this will produce worse code than before.

Note with a hypothetical mid-end optimization that does this constant
folding for us the rules here could be further simplified to purely
consider the shape of the input `Value` to amode computation without
considering constants at all.
@alexcrichton alexcrichton requested review from a team as code owners September 11, 2023 16:19
@alexcrichton alexcrichton requested review from elliottt and pchickey and removed request for a team and elliottt September 11, 2023 16:19
@alexcrichton alexcrichton merged commit 5a239f6 into bytecodealliance:release-10.0.0 Sep 11, 2023
@alexcrichton alexcrichton deleted the amode-10 branch September 11, 2023 17:02
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