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

C++, no exceptions, and OOM in operator new #406

Open
alexcrichton opened this issue Apr 5, 2023 · 3 comments
Open

C++, no exceptions, and OOM in operator new #406

alexcrichton opened this issue Apr 5, 2023 · 3 comments

Comments

@alexcrichton
Copy link
Collaborator

This example program is intended to allocate 4 wasm pages of memory:

#include <vector>
#include <stdio.h>

int main() {
  std::vector<int> v;
  for (int i = 0; i < 65536; i++) {
    v.push_back(4);
  }
}

This can be compiled to have a max wasm memory size of 2 pages, though, which will cause the program to OOM at runtime:

$ clang++ foo.cpp -o foo.wasm -O2 -Wl,--stack-first,--max-memory=131072 -fno-exceptions

which produces foo.wasm.gz.

When run:

$ wasmtime run ./foo.wasm

this program will (currently for me) infinite loop.

The issue seems to be that when compiled with -fno-exceptions, which I believe is currently required of C++ code compiled with wasi-libc to wasm since __cxa_throw isn't otherwise defined, then the operator new operator is defined to return NULL for failed allocations. For a native platform this is fine because it probably faults quickly when accessing this address. For Wasm, however, this is a valid address so the program keeps going.

This was, for me, an unfortunately slow way for me to discover that the original program I was working with was OOM-ing and needed a --max-memory argument to be passed that was larger than the base. I'm not sure how best to improve this, though, since I suspect aborting on failure would not be spec-compliant with C++.

@alexcrichton
Copy link
Collaborator Author

Also cc @abrown on this since, by default, compiling with threads sets --max-memory to the base size of memory, so any C++ application compiling with threads will run into this until they specify a --max-memory as well. (mostly something for documentation for now)

@kripken
Copy link
Member

kripken commented Apr 5, 2023

For comparison, here is what emscripten does:

https://github.com/emscripten-core/emscripten/blob/b2f3b50b0291ed6b0e3058194b1b6db927ab4c33/system/lib/libcxx/src/new.cpp#L78-L85

That code will abort rather than return 0 from new. In practice that avoids a lot of difficult-to-debug situations like what you hit @alexcrichton , and I'm not a spec expert but I think it's acceptable since per the spec you can always add a system set_new_handler that aborts anyhow. We've had this behavior for many years and have not had complaints from users that I can recall.

Slightly complicating this is that emscripten also does the same for malloc and not just new:

https://github.com/emscripten-core/emscripten/blob/b2f3b50b0291ed6b0e3058194b1b6db927ab4c33/src/settings.js#L123-L143

That is definitely nonstandard (but very useful for the above reason) and so we have an option to turn that off (to return 0 from malloc), which some users require as they want to recover from OOMs.

@alexcrichton
Copy link
Collaborator Author

Ah makes sense, and thanks for the links! My sense of spec compliance originated from libcxx's current source where the intention seems to be to return NULL, but if that's the route to go to handle this then this is moreso an issue for wasi-sdk rather than wasi-libc since that's where the libcxx library is built. (and perhaps I should have opened this issue over there instead of here...)

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

No branches or pull requests

2 participants