Skip to content

Commit

Permalink
Potential with_mut fix.
Browse files Browse the repository at this point in the history
  • Loading branch information
someguynamedjosh committed Sep 1, 2023
1 parent b19afb0 commit 8ba9309
Show file tree
Hide file tree
Showing 11 changed files with 212 additions and 78 deletions.
30 changes: 30 additions & 0 deletions examples/src/fail_tests/swap_refs_for_use_after_free.rs
@@ -0,0 +1,30 @@
use ouroboros::self_referencing;

pub struct PrintStrRef<'a>(&'a str);

impl Drop for PrintStrRef<'_> {
fn drop(&mut self) {
println!("Dropping {}", self.0);
}
}

#[self_referencing]
pub struct Tricky {
data1: String,
#[borrows(data1)]
#[covariant]
ref1: PrintStrRef<'this>,
data2: String,
}

fn main() {
let mut t = Tricky::new(
"A".to_owned(),
|a| PrintStrRef(a),
"B".to_owned(),
);
t.with_mut(|fields| {
*fields.ref1 = PrintStrRef(fields.data2);
});
drop(t);
}
26 changes: 26 additions & 0 deletions examples/src/fail_tests/swap_refs_for_use_after_free.stderr
@@ -0,0 +1,26 @@
error[E0597]: `t` does not live long enough
--> src/fail_tests/swap_refs_for_use_after_free.rs:26:5
|
26 | / t.with_mut(|fields| {
27 | | *fields.ref1 = PrintStrRef(fields.data2);
28 | | });
| | ^
| | |
| |______borrowed value does not live long enough
| argument requires that `t` is borrowed for `'static`
29 | drop(t);
30 | }
| - `t` dropped here while still borrowed

error[E0505]: cannot move out of `t` because it is borrowed
--> src/fail_tests/swap_refs_for_use_after_free.rs:29:10
|
26 | / t.with_mut(|fields| {
27 | | *fields.ref1 = PrintStrRef(fields.data2);
28 | | });
| | -
| | |
| |______borrow of `t` occurs here
| argument requires that `t` is borrowed for `'static`
29 | drop(t);
| ^ move out of `t` occurs here
1 change: 1 addition & 0 deletions ouroboros_macro/Cargo.toml
Expand Up @@ -13,6 +13,7 @@ proc-macro = true

[dependencies]
heck = "0.4.1"
itertools = "0.11.0"
proc-macro2 = "1.0"
proc-macro-error = "1.0.4"
quote = "1.0"
Expand Down
1 change: 0 additions & 1 deletion ouroboros_macro/src/generate/constructor.rs
Expand Up @@ -13,7 +13,6 @@ pub fn create_builder_and_constructor(
) -> Result<(Ident, TokenStream, TokenStream), Error> {
let struct_name = info.ident.clone();
let generic_args = info.generic_arguments();
let generic_args_with_static_lifetimes = info.generic_arguments_with_static_lifetimes();

let vis = if options.do_pub_extras {
info.vis.clone()
Expand Down
3 changes: 2 additions & 1 deletion ouroboros_macro/src/generate/mod.rs
Expand Up @@ -6,5 +6,6 @@ pub mod struc;
pub mod summon_checker;
pub mod try_constructor;
pub mod type_asserts;
pub mod with_all;
pub mod with;
pub mod with_each;
pub mod with_mut;
Expand Up @@ -14,17 +14,13 @@ pub fn make_with_all_function(
};
let mut fields = Vec::new();
let mut field_assignments = Vec::new();
let mut mut_fields = Vec::new();
let mut mut_field_assignments = Vec::new();
// I don't think the reverse is necessary but it does make the expanded code more uniform.
for field in info.fields.iter().rev() {
let field_name = &field.name;
let field_type = &field.typ;
if field.field_type == FieldType::Tail {
fields.push(quote! { #visibility #field_name: &'outer_borrow #field_type });
field_assignments.push(quote! { #field_name: &this.#field_name });
mut_fields.push(quote! { #visibility #field_name: &'outer_borrow mut #field_type });
mut_field_assignments.push(quote! { #field_name: &mut this.#field_name });
} else if field.field_type == FieldType::Borrowed {
let ass = quote! { #field_name: unsafe {
::ouroboros::macro_help::change_lifetime(
Expand All @@ -33,8 +29,6 @@ pub fn make_with_all_function(
} };
fields.push(quote! { #visibility #field_name: &'this #field_type });
field_assignments.push(ass.clone());
mut_fields.push(quote! { #visibility #field_name: &'this #field_type });
mut_field_assignments.push(ass);
} else if field.field_type == FieldType::BorrowedMut {
// Add nothing because we cannot borrow something that has already been mutably
// borrowed.
Expand All @@ -43,9 +37,7 @@ pub fn make_with_all_function(

for (ty, ident) in info.generic_consumers() {
fields.push(quote! { #ident: ::core::marker::PhantomData<#ty> });
mut_fields.push(quote! { #ident: ::core::marker::PhantomData<#ty> });
field_assignments.push(quote! { #ident: ::core::marker::PhantomData });
mut_field_assignments.push(quote! { #ident: ::core::marker::PhantomData });
}
let new_generic_params = info.borrowed_generic_params();
let new_generic_args = info.borrowed_generic_arguments();
Expand All @@ -58,14 +50,6 @@ pub fn make_with_all_function(
),
info.ident.to_string()
);
let mut_struct_documentation = format!(
concat!(
"A struct for holding mutable references to all ",
"[tail fields](https://docs.rs/ouroboros/latest/ouroboros/attr.self_referencing.html#definitions) in an instance of ",
"[`{0}`]({0})."
),
info.ident.to_string()
);
let ltname = format!("'{}", info.fake_lifetime());
let lifetime = Lifetime::new(&ltname, Span::call_site());
let generic_where = if let Some(clause) = &info.generics.where_clause {
Expand All @@ -85,33 +69,19 @@ pub fn make_with_all_function(
let struct_defs = quote! {
#[doc=#struct_documentation]
#visibility struct BorrowedFields #new_generic_params #generic_where { #(#fields),* }
#[doc=#mut_struct_documentation]
#visibility struct BorrowedMutFields #new_generic_params #generic_where { #(#mut_fields),* }
};
let borrowed_fields_type = quote! { BorrowedFields<#(#new_generic_args),*> };
let borrowed_mut_fields_type = quote! { BorrowedMutFields<#(#new_generic_args),*> };
let documentation = concat!(
"This method provides immutable references to all ",
"[tail and immutably borrowed fields](https://docs.rs/ouroboros/latest/ouroboros/attr.self_referencing.html#definitions).",
);
let mut_documentation = concat!(
"This method provides mutable references to all ",
"[tail fields](https://docs.rs/ouroboros/latest/ouroboros/attr.self_referencing.html#definitions).",
);
let documentation = if !options.do_no_doc {
quote! {
#[doc=#documentation]
}
} else {
quote! { #[doc(hidden)] }
};
let mut_documentation = if !options.do_no_doc {
quote! {
#[doc=#mut_documentation]
}
} else {
quote! { #[doc(hidden)] }
};
let fn_defs = quote! {
#documentation
#[inline(always)]
Expand All @@ -124,17 +94,6 @@ pub fn make_with_all_function(
#(#field_assignments),*
})
}
#mut_documentation
#[inline(always)]
#visibility fn with_mut <'outer_borrow, ReturnType>(
&'outer_borrow mut self,
user: impl for<'this> ::core::ops::FnOnce(#borrowed_mut_fields_type) -> ReturnType
) -> ReturnType {
let this = unsafe { self.actual_data.assume_init_mut() };
user(BorrowedMutFields {
#(#mut_field_assignments),*
})
}
};
Ok((struct_defs, fn_defs))
}
2 changes: 0 additions & 2 deletions ouroboros_macro/src/generate/with_each.rs
Expand Up @@ -5,8 +5,6 @@ use syn::Error;

pub fn make_with_functions(info: &StructInfo, options: Options) -> Result<Vec<TokenStream>, Error> {
let mut users = Vec::new();
let internal_struct = &info.internal_ident;
let generic_args = info.generic_arguments();
for field in &info.fields {
let visibility = &field.vis;
let field_name = &field.name;
Expand Down
128 changes: 128 additions & 0 deletions ouroboros_macro/src/generate/with_mut.rs
@@ -0,0 +1,128 @@
use crate::{
info_structures::{FieldType, Options, StructInfo},
utils::{replace_this_with_lifetime, uses_this_lifetime},
};
use itertools::Itertools;
use proc_macro2::{Span, TokenStream};
use quote::{format_ident, quote};
use syn::{Error, Lifetime, WhereClause};

pub fn make_with_all_mut_function(
info: &StructInfo,
options: Options,
) -> Result<(TokenStream, TokenStream), Error> {
let visibility = if options.do_pub_extras {
info.vis.clone()
} else {
syn::parse_quote! { pub(super) }
};
let mut mut_fields = Vec::new();
let mut mut_field_assignments = Vec::new();
let mut lifetime_idents = Vec::new();
// I don't think the reverse is necessary but it does make the expanded code more uniform.
for field in info.fields.iter().rev() {
let field_name = &field.name;
let field_type = &field.typ;
let lifetime = format_ident!("this{}", lifetime_idents.len());
if uses_this_lifetime(quote! { #field_type }) || field.field_type == FieldType::Borrowed {
lifetime_idents.push(lifetime.clone());
}
let field_type = replace_this_with_lifetime(quote! { #field_type }, lifetime.clone());
if field.field_type == FieldType::Tail {
mut_fields.push(quote! { #visibility #field_name: &'outer_borrow mut #field_type });
mut_field_assignments.push(quote! { #field_name: &mut this.#field_name });
} else if field.field_type == FieldType::Borrowed {
let ass = quote! { #field_name: unsafe {
::ouroboros::macro_help::change_lifetime(
&*this.#field_name
)
} };
let lt = Lifetime::new(&format!("'{}", lifetime.to_string()), Span::call_site());
mut_fields.push(quote! { #visibility #field_name: &#lt #field_type });
mut_field_assignments.push(ass);
} else if field.field_type == FieldType::BorrowedMut {
// Add nothing because we cannot borrow something that has already been mutably
// borrowed.
}
}

for (ty, ident) in info.generic_consumers() {
mut_fields.push(quote! { #ident: ::core::marker::PhantomData<#ty> });
mut_field_assignments.push(quote! { #ident: ::core::marker::PhantomData });
}

let mut new_generic_params = info.generic_params().clone();
for lt in &lifetime_idents {
let lt = Lifetime::new(&format!("'{}", lt.to_string()), Span::call_site());
new_generic_params.insert(0, syn::parse_quote! { #lt });
}
new_generic_params.insert(0, syn::parse_quote! { 'outer_borrow });
let mut new_generic_args = info.generic_arguments();
let mut lifetimes = Vec::new();
for lt in &lifetime_idents {
let lt = Lifetime::new(&format!("'{}", lt.to_string()), Span::call_site());
lifetimes.push(lt.clone());
new_generic_args.insert(0, quote! { #lt });
}
new_generic_args.insert(0, quote! { 'outer_borrow });

let mut_struct_documentation = format!(
concat!(
"A struct for holding mutable references to all ",
"[tail fields](https://docs.rs/ouroboros/latest/ouroboros/attr.self_referencing.html#definitions) in an instance of ",
"[`{0}`]({0})."
),
info.ident.to_string()
);
let fake_lifetime = Lifetime::new(&format!("'{}", info.fake_lifetime()), Span::call_site());
let mut generic_where = if let Some(clause) = &info.generics.where_clause {
clause.clone()
} else {
syn::parse_quote! { where }
};
for lt in &lifetime_idents {
let lt = Lifetime::new(&format!("'{}", lt.to_string()), Span::call_site());
let extra: WhereClause = syn::parse_quote! { where #fake_lifetime: #lt };
generic_where
.predicates
.extend(extra.predicates.into_iter());
}
for (outlives, lt) in lifetime_idents.iter().tuple_windows() {
let lt = Lifetime::new(&format!("'{}", lt.to_string()), Span::call_site());
let outlives = Lifetime::new(&format!("'{}", outlives.to_string()), Span::call_site());
let extra: WhereClause = syn::parse_quote! { where #lt: #outlives };
generic_where
.predicates
.extend(extra.predicates.into_iter());
}
let struct_defs = quote! {
#[doc=#mut_struct_documentation]
#visibility struct BorrowedMutFields <#new_generic_params> #generic_where { #(#mut_fields),* }
};
let borrowed_mut_fields_type = quote! { BorrowedMutFields<#(#new_generic_args),*> };
let mut_documentation = concat!(
"This method provides mutable references to all ",
"[tail fields](https://docs.rs/ouroboros/latest/ouroboros/attr.self_referencing.html#definitions).",
);
let mut_documentation = if !options.do_no_doc {
quote! {
#[doc=#mut_documentation]
}
} else {
quote! { #[doc(hidden)] }
};
let fn_defs = quote! {
#mut_documentation
#[inline(always)]
#visibility fn with_mut <'outer_borrow, ReturnType>(
&'outer_borrow mut self,
user: impl for<#(#lifetimes),*> ::core::ops::FnOnce(#borrowed_mut_fields_type) -> ReturnType
) -> ReturnType {
let this = unsafe { self.actual_data.assume_init_mut() };
user(BorrowedMutFields {
#(#mut_field_assignments),*
})
}
};
Ok((struct_defs, fn_defs))
}
24 changes: 1 addition & 23 deletions ouroboros_macro/src/info_structures.rs
Expand Up @@ -3,7 +3,7 @@ use proc_macro2::{Ident, TokenStream};
use quote::{format_ident, quote, ToTokens};
use syn::{
punctuated::Punctuated, token::Comma, Attribute, ConstParam, Error, GenericParam, Generics,
LifetimeParam, Type, TypeParam, Visibility, WhereClause,
LifetimeParam, Type, TypeParam, Visibility,
};

#[derive(Clone, Copy)]
Expand Down Expand Up @@ -90,20 +90,6 @@ impl StructInfo {
&self.generics.params
}

pub fn generic_params_without_lifetimes(&self) -> Vec<&GenericParam> {
self.generics
.params
.iter()
.filter(|p| {
if let GenericParam::Lifetime(..) = p {
false
} else {
true
}
})
.collect()
}

/// Same as generic_params but with 'this and 'outer_borrow prepended.
pub fn borrowed_generic_params(&self) -> TokenStream {
if self.generic_params().is_empty() {
Expand Down Expand Up @@ -132,14 +118,6 @@ impl StructInfo {
make_generic_arguments(self.generics.params.iter().collect())
}

pub fn generic_arguments_with_static_lifetimes(&self) -> Vec<TokenStream> {
let mut result = make_generic_arguments(self.generic_params_without_lifetimes());
for _ in 0..self.generics.lifetimes().count() {
result.insert(0, quote! { 'static });
}
result
}

/// Same as generic_arguments but with 'outer_borrow and 'this prepended.
pub fn borrowed_generic_arguments(&self) -> Vec<TokenStream> {
let mut args = self.generic_arguments();
Expand Down

0 comments on commit 8ba9309

Please sign in to comment.