Allow use of pinned heap registers

[shravanrn]

@pchickey @awortman-fastly @iximeow Slightly cleaned up and rebased version of #273 with some comment fixes

I've left the code mostly as is although some of the changes here seem orthogonal. Wanted to ask about this first before removing the code

• Context save for sandbox is also saving the value for register r8
• In stack.rs, there is a stack overflow test where the size of the stack seems to be bumped.

Let me know if I should remove either of these, and I can update the PR

[iximeow]

🎉 thank you for fixing up conflicts here!

I do have two requests:

• it looks like the r8 preservation is no longer necessary, remove that please!
• can you make this a bit in ModuleFeatures, set by lucetc? right now there's a fairly tight coordination between "pinning the heap is enabled" and "which register is the one for heap pinning", but I could imagine a runtime wanting to reject non-pinning modules, or needing to declare which register is used for heap pinning at compile time.

Also, I don't think I ever investigated what made the magic value for stack.rs change. Can you see if 481 is still not enough to cause the expected overflow? If it's not, I'd like to ask you to figure out what the right updated value is - I think I just picked 591 as a big number that would make it overflow, with intent to minimize it when it came time to try landing the change.

[shravanrn]

Sorry, just wrapping some other work. Will update the pr shortly

[shravanrn]

Made the changes. Hopefully the r15 bit is not too hacky --- i was trying to avoid pulling in new dependencies.
Also, restored the stack size in the test. Haven't tested this though. Waiting for CI to complete, and will fix things

[shravanrn]

@iximeow looks like the tests pass with the old stack size. I think this is ready for review?

[shravanrn]

ping 🙂

[iximeow]

Thought I'd taken a look but neglected to click "Submit review", sorry - one additional change I think would be good before landing this.

Glad to see that the stack.rs change didn't need to stick though, thank you!

[iximeow]

How's it sound to make this an Option<HeapPinStyle>? Then None means "no pinning", and HeapPinStyle can be written like

#[repr(u8)]
enum HeapPinStyle {
    PinR15 = 1
}

that we can extend if need be in the future?

I mostly want to avoid a cranelift RegUnit, even in u16 form in lucet-module - the stability of those numbers isn't guaranteed, so we could misinterpret an old module, or an old runtime could misinterpret a new module.

[pchickey]

This PR was closed as a byproduct of deleting the branch named master. If this is still an active PR, re-open as a new PR against main.

[iximeow]

Just a heads up, I pushed a small commit here that specifically lists what heap pinning approach lucetc builds a module expecting. With that (or a change like it) I'd be happy to ✔️ but I can't push to this branch so either you can cherry pick it or I can rebase these two and put up a new PR. Either works!

We've enabled CircleCI for some of our CI workflow since this branch got put up, so either way it'll need to pick up that configuration to get a pass on CI now.