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

Push WASM memory management to WasmModule #656

Merged
merged 4 commits into from
Jun 21, 2022
Merged

Push WASM memory management to WasmModule #656

merged 4 commits into from
Jun 21, 2022

Conversation

csegarragonz
Copy link
Collaborator

@csegarragonz csegarragonz commented Jun 20, 2022

In this PR I introduce support for mmap/munmap in WAMR. In the process, I push most of the memory management implementation from WAVMWasmModule to WasmModule to avoid duplication between WAMR and WAVM.

The implementation is still quite generic, and we can get away with few calls to the actual WASM runtime.

@csegarragonz csegarragonz self-assigned this Jun 20, 2022
@@ -674,17 +674,125 @@ void WasmModule::writeWasmEnvToMemory(uint32_t envPointers, uint32_t envBuffer)

uint32_t WasmModule::growMemory(size_t nBytes)
{
throw std::runtime_error("growMemory not implemented");
if (nBytes == 0) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

All the implementation in this file is copied from WAVMWasmModule with the necessary changes to make it runtime-agnostic.

@@ -1008,198 +1008,56 @@ U32 WAVMWasmModule::mmapFile(U32 fd, size_t length)
return wasmPtr;
}

U32 WAVMWasmModule::growMemory(size_t nBytes)
bool WAVMWasmModule::doGrowMemory(uint32_t pageChange)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This diff is very hard to read, but I have left the sense-checks that we can only do in WAVM. You can see the new code here.

} else {
return module->growMemory(increment);
}
SPDLOG_WARN("SGX-WAMR sbrk does not allocate more memory");
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We don't really want to be growing memory outside the enclave. Memory management will have to be implemented inside the enclave.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Should this be an error then?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We still call this method from SGX code, so even if we do nothing we must not error out in order for functions to finish.

@csegarragonz csegarragonz added the wasm WebAssembly label Jun 20, 2022
Copy link
Collaborator

@Shillaker Shillaker left a comment

Choose a reason for hiding this comment

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

Nice, LGTM!

} else {
return module->growMemory(increment);
}
SPDLOG_WARN("SGX-WAMR sbrk does not allocate more memory");
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should this be an error then?

@csegarragonz csegarragonz merged commit b76190e into main Jun 21, 2022
@csegarragonz csegarragonz deleted the wasm-memory branch June 21, 2022 17:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
wasm WebAssembly
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants