Skip to content
This repository has been archived by the owner on Mar 7, 2021. It is now read-only.

Must use fallible allocation and not panic #148

Open
bill-myers opened this issue Aug 30, 2019 · 2 comments
Open

Must use fallible allocation and not panic #148

bill-myers opened this issue Aug 30, 2019 · 2 comments
Labels
enhancement New feature or request upstream-rust Work that needs to happen in Rust itself

Comments

@bill-myers
Copy link

The code seems to use Box::new at times, which panics on allocation failure.

This is obviously not acceptable, since the kernel must not panic on allocation failure, so this crate must NOT use Box, Vec, etc. and instead use fallible alternatives.

@geofft
Copy link
Collaborator

geofft commented Aug 30, 2019

Yup. I think rust-lang/rust#48043 is the relevant tracking issue? We should switch over as soon as there's a fallible version of libcollections. (I don't think we want to maintain our own version of libcollections in the interim, but if someone already has a crate that gets us fallible vectors etc. that would likely be worth switching to.)

That said, a Rust panic isn't the worst thing - we turn it into a BUG(), not a kernel panic, so only the current thread dies, not the entire kernel. Still not fun, especially since we disable unwinding and so destructors don't run, it's not catchable, etc., but for code that runs on typical desktop/server machines where kmalloc failing is a sign that something is going horribly wrong anyway, it's not terribly far from what you'd actually want to do.

(If we can turn on unwinding, one intermediate step might be a helper function that catches allocation failure at the FFI boundary and returns -ENOMEM, or something.)

@geofft
Copy link
Collaborator

geofft commented Aug 30, 2019

(Re turning on unwinding, see #26. I also need to chase the claim that it's UB to unwind past a Rust frame into a C frame... the Itanium ABI has a whole section about making sure that it's well-defined to unwind a stack with frames from different languages, that's the whole point of eh_personality I think!)

@alex alex added the enhancement New feature or request label Sep 1, 2019
@alex alex added the upstream-rust Work that needs to happen in Rust itself label Aug 9, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request upstream-rust Work that needs to happen in Rust itself
Projects
None yet
Development

No branches or pull requests

3 participants