Correct function type for vtable shim (fix some boxed closures) (rust…

…-lang#335)

Correct vtable shim self type (add fixme for other cases)

Co-authored-by: Daniel Schwartz-Narbonne <danielsn@users.noreply.github.com>

compiler/rustc_codegen_llvm/src/gotoc/place.rs

@@ -202,32 +202,35 @@ impl<'tcx> GotocCtx<'tcx> {
     /// 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 or a
-    /// 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.
-    pub fn codegen_local_fndef(&mut self, l: Local) -> Option<Expr> {
-        let t = self.local_ty(l);
-        match t.kind() {
+    /// 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.
+    ///
+    /// Recursively finds the actual FnDef from a pointer or box.
+    pub fn codegen_local_fndef(&mut self, ty: &'tcx ty::TyS<'tcx>) -> Option<Expr> {
+        match ty.kind() {
             // A local that is itself a FnDef, like Fn::call_once
             ty::FnDef(defid, substs) => Some(self.codegen_fndef(*defid, substs, None)),
-            // A local that is a pointer to a FnDef, like Fn::call and Fn::call_mut
-            ty::RawPtr(inner) => match inner.ty.kind() {
-                ty::FnDef(defid, substs) => {
-                    Some(self.codegen_fndef(*defid, substs, None).address_of())
-                }
-                _ => None,
-            },
+            // A local can be pointer to a FnDef, like Fn::call and Fn::call_mut
+            ty::RawPtr(inner) => self.codegen_local_fndef(inner.ty).map(|f| f.address_of()),
+            // A local can be a boxed function pointer
+            ty::Adt(def, _) if def.is_box() => {
+                let boxed_ty = self.codegen_ty(ty);
+                self.codegen_local_fndef(ty.boxed_ty())
+                    .map(|f| self.box_value(f.address_of(), boxed_ty))
+            }
             _ => None,
         }
     }
 
     /// Codegen for a local
     pub fn codegen_local(&mut self, l: Local) -> Expr {
         // Check if the local is a function definition (see comment above)
-        if let Some(fn_def) = self.codegen_local_fndef(l) {
+        if let Some(fn_def) = self.codegen_local_fndef(self.local_ty(l)) {
             return fn_def;
         }
 

compiler/rustc_codegen_llvm/src/gotoc/typ.rs

@@ -1,6 +1,6 @@
 // Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
 // SPDX-License-Identifier: Apache-2.0 OR MIT
-use super::cbmc::goto_program::{DatatypeComponent, Expr, Symbol, SymbolTable, Type};
+use super::cbmc::goto_program::{DatatypeComponent, Expr, Parameter, Symbol, SymbolTable, Type};
 use super::cbmc::utils::aggr_name;
 use super::metadata::GotocCtx;
 use crate::btree_map;
@@ -186,6 +186,7 @@ impl<'tcx> GotocCtx<'tcx> {
             // The virtual methods on the trait ref. Some auto traits have no methods.
             if let Some(principal) = binder.principal() {
                 let poly = principal.with_self_ty(self.tcx, t);
+                let poly = self.tcx.erase_regions(poly);
                 let mut flds = self
                     .tcx
                     .vtable_entries(poly)
@@ -954,7 +955,7 @@ impl<'tcx> GotocCtx<'tcx> {
         let sig = self.current_fn().sig();
         let sig = self.tcx.normalize_erasing_late_bound_regions(ty::ParamEnv::reveal_all(), sig);
         // we don't call [codegen_function_sig] because we want to get a bit more metainformation.
-        let params = sig
+        let mut params: Vec<Parameter> = sig
             .inputs()
             .iter()
             .enumerate()
@@ -970,6 +971,18 @@ impl<'tcx> GotocCtx<'tcx> {
                 }
             })
             .collect();
+
+        // For vtable shims, we need to modify fn(self, ...) to fn(self: *mut Self, ...),
+        // since the vtable functions expect a pointer as the first argument. See the comment
+        // and similar code in compiler/rustc_mir/src/shim.rs.
+        if let ty::InstanceDef::VtableShim(..) = self.current_fn().instance().def {
+            if let Some(self_param) = params.first() {
+                let ident = self_param.identifier().map(|i| i.to_string());
+                let ty = self_param.typ().clone();
+                params[0] = Type::parameter(ident.clone(), ident, ty.to_pointer());
+            }
+        }
+
         if sig.c_variadic {
             Type::variadic_code(params, self.codegen_ty(sig.output()))
         } else {

src/test/cbmc/DynTrait/boxed_closure.rs

@@ -1,13 +1,6 @@
 // Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
 // SPDX-License-Identifier: Apache-2.0 OR MIT
 
-//rmc-flags: --no-memory-safety-checks
-
-// We use `--no-memory-safety-checks` in this test to avoid getting
-// a verification failure:
-// [pointer_dereference.7] invalid function pointer: FAILURE
-// Tracking issue: https://github.com/model-checking/rmc/issues/307
-
 // Check that we can codegen a boxed dyn closure
 
 fn main() {

src/test/cbmc/DynTrait/boxed_closure_fail_fixme.rs → src/test/cbmc/DynTrait/boxed_closure_fail.rs

@@ -3,11 +3,6 @@
 // rmc-verify-fail
 
 // Check that we can codegen a boxed dyn closure and fail an inner assertion
-
-// This current verifies "successfully" because the closure is not actually
-// called in the resulting GotoC code.
-// https://github.com/model-checking/rmc/issues/240
-
 include!("../../rmc-prelude.rs");
 
 fn main() {

src/test/cbmc/DynTrait/nested_closures_fixme.rs

@@ -0,0 +1,35 @@
+// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
+// SPDX-License-Identifier: Apache-2.0 OR MIT
+
+// Check that we can codegen various nesting structures of boxes and
+// pointer to closures.
+
+// FIXME: several cases fail because we need to "retuple" closures,
+// see: https://rust-lang.zulipchat.com/#narrow/stream/182449-t-compiler.2Fhelp/topic/Determine.20untupled.20closure.20args.20from.20Instance.3F
+
+fn main() {
+    // Create a nested boxed once-callable closure
+    let f: Box<Box<dyn FnOnce(i32)>> = Box::new(Box::new(|x| assert!(x == 1)));
+    f(1);
+
+    // Create a pointer to a closure
+    let g = |y| assert!(y == 2);
+    let p: &dyn Fn(i32) = &g;
+    p(2);
+
+    // Additional level of pointer nesting
+    let q: &dyn Fn(i32) = &p;
+    q(2);
+
+    // Create a boxed pointer to a closure
+    let r: Box<&dyn Fn(i32)> = Box::new(&g);
+    r(2);
+
+    // Another boxed box
+    let s: Box<Box<dyn Fn(i32)>> = Box::new(Box::new(|x| assert!(x == 3)));
+    s(3);
+
+    // A pointer to the boxed box
+    let t: &Box<Box<dyn Fn(i32)>> = &s;
+    t(3);
+}