Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Generate combinational tests for linear memory functions #1233

Merged
merged 2 commits into from
Nov 21, 2023

Conversation

jayz22
Copy link
Contributor

@jayz22 jayz22 commented Nov 20, 2023

Generate linear memory tests

What

Resolves #1144

This PR introduces new macro code synth_linear_memory_tests.rs that generates tests (tests/linear_memory.rs) for linear memory host functions.
There are three different types of test, depending on the form of input argument combinations (a host function may have multiple):
a). "lm_pos + len (bytes)" where either the initial lm_pos or the range can be out of bound. Involved host functions:

  • "bytes_new_from_linear_memory"
  • "string_new_from_linear_memory"
  • "symbol_new_from_linear_memory"
  • "bytes_copy_from_linear_memory"
  • "bytes_copy_to_linear_memory"
  • "string_copy_to_linear_memory"
  • "symbol_copy_to_linear_memory"

b). "val_pos + len (number of Vals)", where either the initial val_pos or the range can go out of bound. Here Val must also be 8 bytes forming a valid env::Val. Involved host functions:

  • "map_new_from_linear_memory"
  • "vec_new_from_linear_memory"
  • "map_unpack_to_linear_memory"
  • "vec_unpack_to_linear_memory"

c) An array of keys where each key is a 8-byte slice consist of 4-byte ptr + 4-byte len, where either the ptr or len can go out of range. Involved host functions:

  • "map_new_from_linear_memory"
  • "symbol_index_in_linear_memory"
  • "map_unpack_to_linear_memory"

It also involves generating a wasm module for each host function, setting up sections of linear memory, preload it with some values. Please refer to the big comment in the file for details.

Why

[TODO: Why this change is being made. Include any context required to understand the why.]

Known limitations

[TODO or N/A]

@jayz22 jayz22 requested review from graydon, sisuresh, dmkozh and a team as code owners November 20, 2023 03:43
Copy link
Contributor

@graydon graydon left a comment

Choose a reason for hiding this comment

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

In general I'm only so-so comfortable with this code as-written:

  • The code imperatively fills linear memory rather than using a static data section in each wasm, which makes the tests run slower than they need to and avoids exercising the data-section paths (it's also confusing that the code talks about sections while not actually using sections; it seems to just be using the term "section" as a synonym for "region" or "extent" or something, but "section" has a specific meaning in wasm modules)
  • The code probably doesn't have to be macro-generated; I can see starting there from the perspective of giving yourself maximum freedom to factor the work / maximum generated-tests-per-unit-of-effort but the effect is to make the tests harder to understand and maintain long-term (this is always the risk with macros or any kind of metaprograms)

That said: we're in a rush and these tests do seem to exercise the surface area we want, so I'm going to merge now. I'll file a followup bug to maybe revisit these tests in the future and simplify them.

@graydon graydon added this pull request to the merge queue Nov 21, 2023
Merged via the queue into stellar:main with commit e2e6ff6 Nov 21, 2023
10 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.

[test] linear memory misbehaviour
2 participants