-
Notifications
You must be signed in to change notification settings - Fork 182
Treat pointer and Box locals of function items as ordinary variables
#4892
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?
Changes from all commits
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 |
|---|---|---|
|
|
@@ -365,31 +365,15 @@ impl GotocCtx<'_, '_> { | |
| /// If a local is a function definition, ignore the local variable name and | ||
| /// generate a function call based on the def id. | ||
| /// | ||
| /// Note that this is finicky. A local might be a function definition, a | ||
| /// pointer to one, or a boxed pointer to one. For example, the | ||
| /// auto-generated code for Fn::call_once uses a local FnDef to call the | ||
| /// wrapped function, while the auto-generated code for Fn::call and | ||
| /// Fn::call_mut both use pointers to a FnDef. In these cases, we need to | ||
| /// generate an expression that references the existing FnDef rather than | ||
| /// a named variable. | ||
| /// For example, the auto-generated code for Fn::call_once uses a local FnDef to call the | ||
| /// wrapped function. A function item is zero-sized, so every value of it is the same and we | ||
| /// can use its singleton instead of a named variable. | ||
| /// | ||
| /// Recursively finds the actual FnDef from a pointer or box. | ||
| /// A pointer to a function item, or a `Box` of one, is not zero-sized: it is an ordinary | ||
| /// variable that holds whatever address was assigned to it. | ||
| fn codegen_local_fndef(&mut self, ty: Ty, loc: Location) -> Option<Expr> { | ||
| match ty.kind() { | ||
| // A local that is itself a FnDef, like Fn::call_once | ||
| TyKind::RigidTy(RigidTy::FnDef(def, args)) => Some(self.codegen_fndef(def, &args, loc)), | ||
|
Member
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. Praise: Deleting these arms is the right fix rather than patching around them. I checked the claim in the old comment that |
||
| // A local can be pointer to a FnDef, like Fn::call and Fn::call_mut | ||
| TyKind::RigidTy(RigidTy::RawPtr(inner, _)) => self | ||
| .codegen_local_fndef(inner, loc) | ||
| .map(|f| if f.can_take_address_of() { f.address_of() } else { f }), | ||
| // A local can be a boxed function pointer | ||
| TyKind::RigidTy(RigidTy::Adt(def, args)) if def.is_box() => { | ||
| let boxed_ty = self.codegen_ty_stable(ty); | ||
| // The type of `T` for `Box<T>` can be derived from the first definition args. | ||
| let inner_ty = args.0[0].ty().unwrap(); | ||
| self.codegen_local_fndef(*inner_ty, loc) | ||
| .map(|f| self.box_value(f.address_of(), boxed_ty)) | ||
| } | ||
| _ => None, | ||
| } | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,35 @@ | ||
| // Copyright Kani Contributors | ||
| // SPDX-License-Identifier: Apache-2.0 OR MIT | ||
|
|
||
| //! Checks pointers to a function item. A function item is zero-sized, but a pointer to one is an | ||
| //! ordinary value that holds the address it was given. | ||
| //! See <https://github.com/model-checking/kani/issues/2255>. | ||
|
|
||
| fn foo() -> u32 { | ||
| 42 | ||
| } | ||
|
|
||
| fn null_of<T>(_: T) -> *const T { | ||
| core::ptr::null() | ||
| } | ||
|
|
||
| /// The program from the issue: `{:p}` formats the address of the function item. | ||
| #[kani::proof] | ||
| #[allow(function_item_references)] | ||
| fn check_print_address() { | ||
| println!("{:p}", &foo); | ||
| } | ||
|
|
||
| #[kani::proof] | ||
| fn check_pointer_value() { | ||
| let p: *const _ = &foo; | ||
| assert!(!p.is_null()); | ||
| assert!(null_of(foo).is_null()); | ||
| } | ||
|
|
||
| /// Fails if a pointer to a function item is read as some other address than the one assigned. | ||
| #[kani::proof] | ||
| #[kani::should_panic] | ||
| fn check_null_stays_null() { | ||
|
Member
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. Suggestion: This harness catches the soundness regression, but only indirectly. Nothing calls |
||
| assert!(!null_of(foo).is_null()); | ||
| } | ||
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.
Nit: "generate a function call based on the def id" predates this PR and isn't quite right: it returns the function item's singleton, not a call. Since this comment is being rewritten anyway, it could say "use the function item's singleton instead of the named variable".