-
Notifications
You must be signed in to change notification settings - Fork 142
Add option to restrict vtable function pointers #536
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
Changes from all commits
19bb530
d55a2be
995322a
ed5a30c
91ca00d
0536e87
30e1e90
fc7ea4d
3c9376a
93337d4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -98,6 +98,32 @@ impl Serialize for InternedString { | |
| } | ||
| } | ||
|
|
||
| struct InternedStringVisitor; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should really go with InternedString itself. |
||
|
|
||
| impl<'de> serde::Deserialize<'de> for InternedString { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you please add a unit test for this? |
||
| fn deserialize<D>(deserializer: D) -> Result<Self, D::Error> | ||
| where | ||
| D: serde::Deserializer<'de>, | ||
| { | ||
| deserializer.deserialize_str(InternedStringVisitor) | ||
| } | ||
| } | ||
|
|
||
| impl<'de> serde::de::Visitor<'de> for InternedStringVisitor { | ||
| type Value = InternedString; | ||
|
|
||
| fn expecting(&self, formatter: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { | ||
| formatter.write_str("a String like thing") | ||
| } | ||
|
|
||
| fn visit_str<E>(self, v: &str) -> Result<Self::Value, E> | ||
| where | ||
| E: serde::de::Error, | ||
| { | ||
| Ok(v.into()) | ||
| } | ||
| } | ||
|
|
||
| impl Serialize for Symbol { | ||
| fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error> | ||
| where | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -178,7 +178,7 @@ impl<'tcx> GotocCtx<'tcx> { | |
| self.find_function(fname) | ||
| } | ||
| } | ||
| ty::Dynamic(_, _) => unimplemented!(), | ||
| ty::Dynamic(_, _) => None, | ||
|
||
| ty::Projection(_) | ty::Opaque(_, _) => { | ||
| let normalized = self.tcx.normalize_erasing_regions(ty::ParamEnv::reveal_all(), t); | ||
| self.codegen_assumption_ref_ptr(fname, t, normalized, is_ref) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,7 +2,7 @@ | |
| // SPDX-License-Identifier: Apache-2.0 OR MIT | ||
| use super::typ::{is_pointer, pointee_type, TypeExt}; | ||
| use crate::utils::{dynamic_fat_ptr, slice_fat_ptr}; | ||
| use crate::GotocCtx; | ||
| use crate::{GotocCtx, VtableCtx}; | ||
| use cbmc::goto_program::{BuiltinFn, Expr, Location, Stmt, Symbol, Type}; | ||
| use cbmc::utils::{aggr_tag, BUG_REPORT_URL}; | ||
| use cbmc::MachineModel; | ||
|
|
@@ -722,7 +722,17 @@ impl<'tcx> GotocCtx<'tcx> { | |
|
|
||
| // Lookup in the symbol table using the full symbol table name/key | ||
| let fn_name = self.symbol_name(instance); | ||
|
|
||
| if let Some(fn_symbol) = self.symbol_table.lookup(&fn_name) { | ||
| if self.vtable_ctx.emit_vtable_restrictions { | ||
| // Add to the possible method names for this trait type | ||
| self.vtable_ctx.add_possible_method( | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would move the |
||
| self.normalized_trait_name(t).into(), | ||
| idx, | ||
| fn_name.into(), | ||
| ); | ||
| } | ||
|
|
||
| // Create a pointer to the method | ||
| // Note that the method takes a self* as the first argument, but the vtable field type has a void* as the first arg. | ||
| // So we need to cast it at the end. | ||
|
|
@@ -746,13 +756,22 @@ impl<'tcx> GotocCtx<'tcx> { | |
| trait_ty: &'tcx ty::TyS<'tcx>, | ||
| ) -> Expr { | ||
| let drop_instance = Instance::resolve_drop_in_place(self.tcx, ty).polymorphize(self.tcx); | ||
| let drop_sym_name = self.symbol_name(drop_instance); | ||
| let drop_sym_name: InternedString = self.symbol_name(drop_instance).into(); | ||
|
|
||
| // The drop instance has the concrete object type, for consistency with | ||
| // type codegen we need the trait type for the function parameter. | ||
| let trait_fn_ty = self.trait_vtable_drop_type(trait_ty); | ||
|
|
||
| if let Some(drop_sym) = self.symbol_table.lookup(&drop_sym_name) { | ||
| if let Some(drop_sym) = self.symbol_table.lookup(drop_sym_name) { | ||
| if self.vtable_ctx.emit_vtable_restrictions { | ||
| // Add to the possible method names for this trait type | ||
| self.vtable_ctx.add_possible_method( | ||
| self.normalized_trait_name(trait_ty).into(), | ||
| VtableCtx::drop_index(), | ||
| drop_sym_name, | ||
| ); | ||
| } | ||
|
|
||
| Expr::symbol_expression(drop_sym_name, drop_sym.clone().typ) | ||
| .address_of() | ||
| .cast_to(trait_fn_ty) | ||
|
|
@@ -761,37 +780,37 @@ impl<'tcx> GotocCtx<'tcx> { | |
| // for it. Build and insert a function that just calls an unimplemented block | ||
| // to maintain soundness. | ||
| let drop_sym_name = format!("{}_unimplemented", self.symbol_name(drop_instance)); | ||
|
|
||
| // Function body | ||
| let unimplemented = self | ||
| .codegen_unimplemented( | ||
| format!("drop_in_place for {}", drop_sym_name).as_str(), | ||
| Type::empty(), | ||
| let drop_sym = self.ensure(&drop_sym_name, |ctx, name| { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For the drop case. Would it make more sense for us to replace the function call with the unimplemented assertion instead of generating the function body? |
||
| // Function body | ||
| let unimplemented = ctx | ||
| .codegen_unimplemented( | ||
| format!("drop_in_place for {}", drop_sym_name).as_str(), | ||
| Type::empty(), | ||
| Location::none(), | ||
| "https://github.com/model-checking/rmc/issues/281", | ||
| ) | ||
| .as_stmt(Location::none()); | ||
|
|
||
| // Declare symbol for the single, self parameter | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not for this PR, but this feels like something that would happen often enough to be worth a helper function |
||
| let param_name = format!("{}::1::var{:?}", drop_sym_name, 0); | ||
| let param_sym = Symbol::variable( | ||
| param_name.clone(), | ||
| param_name, | ||
| ctx.codegen_ty(trait_ty).to_pointer(), | ||
| Location::none(), | ||
| ); | ||
| ctx.symbol_table.insert(param_sym.clone()); | ||
|
|
||
| // Build and insert the function itself | ||
| Symbol::function( | ||
| name, | ||
| Type::code(vec![param_sym.to_function_parameter()], Type::empty()), | ||
| Some(Stmt::block(vec![unimplemented], Location::none())), | ||
| NO_PRETTY_NAME, | ||
| Location::none(), | ||
| "https://github.com/model-checking/rmc/issues/281", | ||
| ) | ||
| .as_stmt(Location::none()); | ||
|
|
||
| // Declare symbol for the single, self parameter | ||
| let param_name = format!("{}::1::var{:?}", drop_sym_name, 0); | ||
| let param_sym = Symbol::variable( | ||
| param_name.clone(), | ||
| param_name, | ||
| self.codegen_ty(trait_ty).to_pointer(), | ||
| Location::none(), | ||
| ); | ||
| self.symbol_table.insert(param_sym.clone()); | ||
|
|
||
| // Build and insert the function itself | ||
| let sym = Symbol::function( | ||
| &drop_sym_name, | ||
| Type::code(vec![param_sym.to_function_parameter()], Type::empty()), | ||
| Some(Stmt::block(vec![unimplemented], Location::none())), | ||
| NO_PRETTY_NAME, | ||
| Location::none(), | ||
| ); | ||
| self.symbol_table.insert(sym.clone()); | ||
| Expr::symbol_expression(drop_sym_name, sym.typ).address_of().cast_to(trait_fn_ty) | ||
| }); | ||
| drop_sym.to_expr().address_of().cast_to(trait_fn_ty) | ||
| } | ||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -8,5 +8,7 @@ | |
|
|
||
| mod current_fn; | ||
| mod goto_ctx; | ||
| mod vtable_ctx; | ||
|
|
||
| pub use goto_ctx::GotocCtx; | ||
| pub use vtable_ctx::VtableCtx; | ||
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.
What are we using from here?
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 compiler's
FxHashMap, not strictly necessary/could be replaced with a vanillaHashMapThere 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.
If easy, could do that here; if not, lets just do that as a cleanup PR