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

Cons element_iter! macro #8

Open
CeleritasCelery opened this issue Apr 11, 2022 · 0 comments
Open

Cons element_iter! macro #8

CeleritasCelery opened this issue Apr 11, 2022 · 0 comments
Labels
unsound? Unsure if this unsafe code is sound

Comments

@CeleritasCelery
Copy link
Owner

This one is a beast.

rune/src/cons/iter.rs

Lines 166 to 189 in 7136b74

macro_rules! element_iter {
($ident:ident, $obj:expr, $gc:ident) => {
let mut root_elem = None;
let mut root_cons = None;
$crate::make_root_owner!(owner);
let mut gc_root_elem = unsafe { $crate::arena::RootStruct::new($gc.get_root_set()) };
let mut gc_root_cons = unsafe { $crate::arena::RootStruct::new($gc.get_root_set()) };
#[allow(unused_qualifications)]
let list: $crate::object::List = $obj.try_into()?;
if let $crate::object::List::Cons(x) = list {
root_elem =
unsafe { Some($crate::arena::Root::new($crate::arena::RootObj::default())) };
root_cons = unsafe { Some($crate::arena::Root::new($crate::arena::RootCons::new(!x))) };
gc_root_elem.set(root_elem.as_mut().unwrap());
gc_root_cons.set(root_cons.as_mut().unwrap());
} else {
std::mem::forget(gc_root_elem);
std::mem::forget(gc_root_cons);
}
#[allow(unused_mut)]
let mut $ident = $crate::cons::ElemStreamIter::new(&root_elem, &root_cons, owner);
};
}

Essentially what we need to do here is create a iterator where the returned value is rooted on each iteration and can only live for that iteration. This is because Cons cells are globally mutable and elements of the list could become unreachable at any point. So if the objects outlive their iteration cycle they could potentially get collected and lead to use-after-free. The only way to do this without GAT's is to use the streaming-iterator crate. This redefines the Iterator next method to bind the lifetime to the borrow of the iterator like this.

    fn next(&mut self) -> Option<&Self::Item> {
        self.advance();
        (*self).get()
    }

So this means we need a way to root the Item on every cycle. So we declare two new roots (one for the item, one for the cons we are using for iteration).

let mut gc_root_elem = unsafe { $crate::arena::RootStruct::new($gc.get_root_set()) };
let mut gc_root_cons = unsafe { $crate::arena::RootStruct::new($gc.get_root_set()) };

However we have to handle the case where the iterator is empty. In that case we can't set the root because we don't have anything to set it with (see #3 for the invariant of rooting). So we declare some Option types that will hold our pinned location on the stack.

let mut root_elem = None;
let mut root_cons = None;

Then we can conditionally set the value of those Option types of our cons is not nil. If it is nil then we simply call forget on the roots so that the drop glue is not called. If the drop glue was called it would pop items off the root stack that we never pushed on.

if let $crate::object::List::Cons(x) = list {
    root_elem =
        unsafe { Some($crate::arena::Root::new($crate::arena::RootObj::default())) };
    root_cons = unsafe { Some($crate::arena::Root::new($crate::arena::RootCons::new(!x))) };
    gc_root_elem.set(root_elem.as_mut().unwrap());
    gc_root_cons.set(root_cons.as_mut().unwrap());
} else {
    std::mem::forget(gc_root_elem);
    std::mem::forget(gc_root_cons);
}

Why this is sound

We are using streaming-iterator to make sure objects can't escape an iteration cycle. During that iteration cycle, we ensure that both item and cons are rooted. The root locations cannot move and store an Option, which allows us to conditionally set them. If the cons we are going to iterator over is nil then we make sure to forget the root so drop glue is not called.

@CeleritasCelery CeleritasCelery added the unsound? Unsure if this unsafe code is sound label Apr 11, 2022
@CeleritasCelery CeleritasCelery changed the title Cons element iterator Cons element_iter! macro Apr 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
unsound? Unsure if this unsafe code is sound
Projects
None yet
Development

No branches or pull requests

1 participant