Skip to content
This repository has been archived by the owner on Aug 14, 2023. It is now read-only.

Runtime creates wasm memory #138

Merged
merged 38 commits into from
May 26, 2020
Merged

Conversation

YaronWittenstein
Copy link
Contributor

@YaronWittenstein YaronWittenstein commented May 24, 2020

Motivation

This PR removes the Buffer primitive from SVM and uses plain Wasm Memory instead.

Future PRs:

@YaronWittenstein YaronWittenstein self-assigned this May 24, 2020
@YaronWittenstein YaronWittenstein added the medium Medium-sized task label May 25, 2020
@YaronWittenstein YaronWittenstein marked this pull request as ready for review May 25, 2020 20:14

helpers::buffer_create(ctx.data, ARGS_BUF_ID, func_buf.len() as u32);
let func_size = func_buf.len();
let view = &memory.view::<u8>()[0..func_size];
Copy link
Contributor

Choose a reason for hiding this comment

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

What about bound checking for the memory size?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this should not happen here.
when we allocate memory we know the size of the func_buf

Copy link
Contributor

Choose a reason for hiding this comment

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

But you're currently don't check it there - do you mean that you will add that check later, or that you'll verify that func_buf size is less than 1 page size?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

  • The validate_template will check that func_buf isn't big enough.
  • The alloc_wasmer_memory will have to allocate enough memory for the program to run.

I've added an issue: #140

@@ -458,7 +453,7 @@ where
func_index
}

fn prepare_args_and_memory(&self, tx: &AppTransaction) -> Vec<wasmer_runtime::Value> {
fn prepare_func_args(&self, tx: &AppTransaction) -> Vec<wasmer_runtime::Value> {
debug!("runtime `prepare_args_and_memory`");
Copy link
Contributor

Choose a reason for hiding this comment

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

prepare_args_and_memory -> prepare_func_args ?

// 3) executing an app-transaction
let func_idx = 1;
let func_buf = vec![0x10, 0x20, 0x30, 0x40, 0x50, 0x60, 0x70, 0x80];
let func_buf_ptr = WasmValue::I32(0);
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you give more explanations how this works (in conjunction with the runtime_func_buf.wast code)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have an idea of how to make it easier to understand.

I prefer to open a new PR that will add two new vmcalls (that we'll need)
and change the test to be easier

// is larger than the `func_buf`.
//
// Each wasm instance memory contains at least one `WASM Page`. (A `Page` size is 64KB)
// The `len(func_buf)`
Copy link
Contributor

Choose a reason for hiding this comment

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

This line is not clear.

@YaronWittenstein YaronWittenstein merged commit 9329da1 into master May 26, 2020
@YaronWittenstein YaronWittenstein deleted the runtime-creates-wasm-memory branch May 26, 2020 14:16
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
medium Medium-sized task
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants