-
Notifications
You must be signed in to change notification settings - Fork 59
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
Stack size as a property of an instance #194
Comments
I haven't added a macro to set it, but it should be relatively simple to add one in the
Hm? I don't understand. How is making it a property of the instance non-deterministic? (: If anything I imagine that making it a property of the module is the way to make it deterministic, because the module itself should know how much stack it needs, and not the execution environment (so the execution environment never needs to know how much stack space the module needs). |
Well, when you put it like that, I'm starting to argue my own point 🤔 I'm thinking about the PVF use case, of course, and we always wanted the stack size to be deterministically controllable by the environment. First, because we wanted to have a hard limit for that, and second, because we wanted to be able to bump that limit for the whole environment if needed. But if we make it module-defined... Well, okay, we can rule out modules going over the hard limit on pre-checking. We also would need a parachain to perform a runtime upgrade if it finds it needs more stack -- but that, indeed, increases granularity. Do we really need it to be environment-controlled? Let me think a bit more about that. In the meantime, I'll try to add a macro to define the stack size. One page is obviously way too small even for a simplest PVFs (one more little bug report is that the stack overflow looks like a nameless "Trap" without debug enabled, and even with full tracing you need quite some time to figure out that the out-of-bound memory region write is indeed a stack overflow). |
Yes, because you do get a trap on any out of bounds memory access (and unlike WASM the stack here is just normal memory), and the VM currently doesn't print out which exact memory address was accessed. So this is not a bug, just a missing feature. It wouldn't be that hard for the VM to also grab the address which the guest tried to access, check whether it's near the stack, and print out that it's most likely a stack overflow. |
Currently, stack size is defined as a property of program blob/module. There are two problems with that:
.polkavm_min_stack_size
ELF section, but IIUC no means are provided for end developers to actually set this value);I'd propose to replace (or supplement) the
Module::instantiate()
with something likeModule::instance_builder().with_stack_size(...).build()
.If we agree on the necessity of the implementation described and on the interface, I'm ready to work on it.
The text was updated successfully, but these errors were encountered: