-
Notifications
You must be signed in to change notification settings - Fork 320
jit: Enable JIT without "std" feature, but with global alloc #127
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
base: main
Are you sure you want to change the base?
Conversation
|
Hi Nate, nice to hear from you 🙂 and thanks a lot for this work! I haven't looked at the details yet, but have you looked at the work from #125? The JIT should be available in |
|
Yes I saw #125, but passing the library a pointer of bytes creates problems for my use case. Now I have to track those bytes when I don't want to. I would much rather implement GlobalAlloc. Core library crates that have strong use cases for running without "std" use the "alloc" feature coupled with a use of the |
|
In this case, the rbpf library cannot ensure that the memory has executable permissions. Developers need to ensure that the memory allocated by GlobalAlloc has executable permissions, which seems unreasonable and dangerous. |
Add a new "alloc" feature that indicates that the crate importing rbpf has an implementation of the GlobalAlloc trait. Memory protection is ignored in the "no_std" environment. Signed-off-by: Nate Sweet <[email protected]>
6508eb8 to
4b1ff20
Compare
|
I cleaned up the commit which had some whitespace issues.
Only if they turn on the alloc feature, which is almost always in a kernel or/and embedded environment (which can't offer executable protection). This commit is specifically for this scenario. |
qmonnet
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for the delay - took me some time to look into this. Thank you also Linfeng for the feedback.
I agree with Nate, it sounds acceptable to leave the user responsible for checking memory permissions, given that the alloc feature is opt-in.
Maybe we could add a comment about this in the feature description in the README, though?
Code looks good otherwise, with just some typos in the README update.
Could you please add a full test or example to show how to use the feature? (It doesn't have to be a complex one, but something providing a basic implementation for GlobalAlloc and running a JITed program on that would help users (and maintainers 😇) understand how to use it.
| The `rbpf` crate has a Cargoe featured named "alloc" that is not enabled by | ||
| default. `std` must be turned off and is mutually exclusive with alloc. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| The `rbpf` crate has a Cargoe featured named "alloc" that is not enabled by | |
| default. `std` must be turned off and is mutually exclusive with alloc. | |
| The `rbpf` crate has a Cargo feature named "alloc" that is not enabled by | |
| default. `std` must be turned off and is mutually exclusive with `alloc`. |
|
We also need to adjust the CI runs, where we build with |
Add a new "alloc" feature that indicates that the crate importing rbpf has an implementation of the GlobalAlloc trait. Memory protection is ignored in the "no_std" environment.