2023-03-08 (#1199)

checker/Cargo.toml

@@ -1,7 +1,7 @@
 [package]
 
 name = "mirai"
-version = "1.1.4"
+version = "1.1.5"
 authors = ["Herman Venter <herman_venter@msn.com>"]
 description = "A static analysis tool for Rust, based on Abstract Interpretation of MIR"
 repository = "https://github.com/facebookexperimental/MIRAI"

checker/src/abstract_value.rs

@@ -103,6 +103,9 @@ pub const TOP: AbstractValue = make_value(Expression::Top);
 /// An abstract domain element that all represent the single concrete value, true.
 pub const TRUE: AbstractValue = make_value(Expression::CompileTimeConstant(ConstantDomain::True));
 
+/// An abstract domain element that all represent an empty tuple or empty struct.
+pub const UNIT: AbstractValue = make_value(Expression::CompileTimeConstant(ConstantDomain::Unit));
+
 /// An abstract domain element that represents a dummy untagged value.
 /// It is only used as the default value for the tag field of non-scalar values.
 pub const DUMMY_UNTAGGED_VALUE: AbstractValue =

checker/src/body_visitor.rs

@@ -20,7 +20,7 @@ use rustc_middle::mir;
 use rustc_middle::ty::subst::SubstsRef;
 use rustc_middle::ty::{AdtDef, Const, Ty, TyCtxt, TyKind, TypeAndMut, UintTy};
 
-use crate::abstract_value::{self, AbstractValue, AbstractValueTrait};
+use crate::abstract_value::{self, AbstractValue, AbstractValueTrait, BOTTOM};
 use crate::block_visitor::BlockVisitor;
 use crate::call_visitor::CallVisitor;
 use crate::constant_domain::ConstantDomain;
@@ -190,6 +190,7 @@ impl<'analysis, 'compilation, 'tcx> BodyVisitor<'analysis, 'compilation, 'tcx> {
         if cfg!(DEBUG) {
             utils::pretty_print_mir(self.tcx, self.def_id);
         }
+        debug!("entered body of {:?}", self.def_id);
         *self.active_calls_map.entry(self.def_id).or_insert(0) += 1;
         let saved_heap_counter = self.cv.constant_value_cache.swap_heap_counter(0);
 
@@ -637,7 +638,7 @@ impl<'analysis, 'compilation, 'tcx> BodyVisitor<'analysis, 'compilation, 'tcx> {
             } else {
                 None
             };
-            let ty = self.tcx.type_of(def_id);
+            let ty = self.tcx.type_of(def_id).skip_binder();
             let func_const = self
                 .cv
                 .constant_value_cache
@@ -2419,7 +2420,7 @@ impl<'analysis, 'compilation, 'tcx> BodyVisitor<'analysis, 'compilation, 'tcx> {
     pub fn get_array_length(&self, length: &'tcx Const<'tcx>) -> usize {
         let param_env = self.type_visitor().get_param_env();
         length
-            .try_eval_usize(self.tcx, param_env)
+            .try_eval_target_usize(self.tcx, param_env)
             .expect("Array length constant to have a known value") as usize
     }
 
@@ -2477,6 +2478,10 @@ impl<'analysis, 'compilation, 'tcx> BodyVisitor<'analysis, 'compilation, 'tcx> {
             }
             update(self, target_path.clone(), value.clone());
         }
+        if let Expression::CompileTimeConstant(ConstantDomain::Function(..)) = &value.expression {
+            let target_path = Path::new_function(target_path.clone());
+            update(self, target_path, value.clone());
+        }
         let target_type = ExpressionType::from(root_rustc_type.kind());
         let mut no_children = true;
         // If a non primitive parameter is just returned from a function, for example,
@@ -2518,6 +2523,14 @@ impl<'analysis, 'compilation, 'tcx> BodyVisitor<'analysis, 'compilation, 'tcx> {
                 if path.is_rooted_by_parameter() {
                     update(self, target_path.clone(), value.clone());
                     no_children = false;
+                } else if no_children && self.type_visitor.is_empty_struct(root_rustc_type.kind()) {
+                    // Add a dummy field so that we don't end up with an unknown local var
+                    // as the value of any target that target_path may get copied or moved to.
+                    // That is problematic because an empty struct is a compile time constant
+                    // and not an unknown value.
+                    let field_path = Path::new_field(target_path, 0);
+                    update(self, field_path, Rc::new(BOTTOM));
+                    return;
                 }
             }
         }

checker/src/call_visitor.rs

@@ -108,7 +108,12 @@ impl<'call, 'block, 'analysis, 'compilation, 'tcx>
         func_args: &Option<Rc<Vec<Rc<FunctionReference>>>>,
         initial_type_cache: &Option<Rc<HashMap<Rc<Path>, Ty<'tcx>>>>,
     ) -> Summary {
-        let func_type = self.block_visitor.bv.tcx.type_of(self.callee_def_id);
+        let func_type = self
+            .block_visitor
+            .bv
+            .tcx
+            .type_of(self.callee_def_id)
+            .skip_binder();
         trace!("summarizing {:?}: {:?}", self.callee_def_id, func_type);
         let tcx = self.block_visitor.bv.tcx;
         if tcx.is_mir_available(self.callee_def_id) {
@@ -201,7 +206,7 @@ impl<'call, 'block, 'analysis, 'compilation, 'tcx>
             trace!(
                 "devirtualize resolving def_id {:?}: {:?}",
                 self.callee_def_id,
-                tcx.type_of(self.callee_def_id)
+                tcx.type_of(self.callee_def_id).skip_binder()
             );
             trace!("devirtualize resolving func_ref {:?}", self.callee_func_ref,);
             trace!("gen_args {:?}", gen_args);
@@ -213,7 +218,11 @@ impl<'call, 'block, 'analysis, 'compilation, 'tcx>
                     return;
                 }
             }
-            let abi = tcx.type_of(self.callee_def_id).fn_sig(tcx).abi();
+            let abi = tcx
+                .type_of(self.callee_def_id)
+                .skip_binder()
+                .fn_sig(tcx)
+                .abi();
             let resolved_instance = if abi == rustc_target::spec::abi::Abi::Rust {
                 Some(rustc_middle::ty::Instance::resolve(
                     tcx,
@@ -232,7 +241,7 @@ impl<'call, 'block, 'analysis, 'compilation, 'tcx>
                     return;
                 }
                 self.callee_def_id = resolved_def_id;
-                let resolved_ty = tcx.type_of(resolved_def_id);
+                let resolved_ty = tcx.type_of(resolved_def_id).skip_binder();
                 let resolved_map = self.type_visitor().get_generic_arguments_map(
                     resolved_def_id,
                     instance.substs,

checker/src/callbacks.rs

@@ -113,7 +113,6 @@ impl rustc_driver::Callbacks for MiraiCallbacks {
         queries
             .global_ctxt()
             .unwrap()
-            .peek_mut()
             .enter(|tcx| self.analyze_with_mirai(compiler, tcx));
         if self.test_run {
             // We avoid code gen for test cases because LLVM is not used in a thread safe manner.

checker/src/fixed_point_visitor.rs

@@ -205,9 +205,7 @@ impl<'fixed, 'analysis, 'compilation, 'tcx>
         let old_state = self.out_state.clone();
         for bb in blocks {
             check_for_early_break!(self.bv);
-            if !self.already_visited.contains(&bb)
-                && self.dominators.is_dominated_by(bb, loop_anchor)
-            {
+            if !self.already_visited.contains(&bb) && self.dominators.dominates(loop_anchor, bb) {
                 last_block = bb;
                 // Visit the next block, or the entire nested loop anchored by it.
                 if bb == loop_anchor {
@@ -250,7 +248,7 @@ impl<'fixed, 'analysis, 'compilation, 'tcx>
                 .filter_map(|pred_bb| {
                     // If the predecessor can only be reached via bb then bb and pred_bb are
                     // part of the loop body.
-                    let is_loop_back = self.dominators.is_dominated_by(*pred_bb, bb);
+                    let is_loop_back = self.dominators.dominates(bb, *pred_bb);
                     if iteration_count == 1 && is_loop_back {
                         // For the first iteration of the loop body we only want state that
                         // precedes the body. Normally, the state of a block that is part of the
@@ -353,7 +351,7 @@ fn add_predecessors_then_root_block<'tcx>(
         if already_added.contains(pred_bb) {
             continue;
         };
-        if dominators.is_dominated_by(*pred_bb, root_block) {
+        if dominators.dominates(root_block, *pred_bb) {
             // pred_bb has still to be added, so it has a greater index than root_block, making root_block the loop anchor.
             //todo: checked_assume!(root_block.index() < pred_bb.index());
             // Root block has a smaller index than pred_bb because it has not already been added.

checker/src/options.rs

@@ -84,10 +84,11 @@ pub struct Options {
 }
 
 /// Represents diag level.
-#[derive(Clone, Copy, Debug, Eq, PartialEq, PartialOrd)]
+#[derive(Clone, Copy, Debug, Default, Eq, PartialEq, PartialOrd)]
 pub enum DiagLevel {
     /// When a function calls a function without a body and with no foreign function summary, the call assumed to be
     /// correct and any diagnostics that depend on the result of the call in some way are suppressed.
+    #[default]
     Default,
     /// Like Default, but emit a diagnostic if there is a call to a function without a body and with no foreign function summary.
     Verify,
@@ -100,12 +101,6 @@ pub enum DiagLevel {
     Paranoid,
 }
 
-impl Default for DiagLevel {
-    fn default() -> Self {
-        DiagLevel::Default
-    }
-}
-
 impl Options {
     /// Parse options from an argument string. The argument string will be split using unix
     /// shell escaping rules. Any content beyond the leftmost `--` token will be returned

checker/src/path.rs

@@ -596,7 +596,7 @@ impl Path {
 
     /// Creates a path to the static defined by def_id.
     pub fn new_static(tcx: TyCtxt<'_>, def_id: DefId) -> Rc<Path> {
-        let ty = tcx.type_of(def_id);
+        let ty = tcx.type_of(def_id).skip_binder();
         let name = utils::summary_key_str(tcx, def_id);
         Rc::new(
             PathEnum::StaticVariable {
@@ -979,16 +979,12 @@ impl PathRefinement for Rc<Path> {
                 }
             }
             // An impossible downcast is equivalent to BOTTOM
-            if let PathSelector::Downcast(_, variant) = selector.as_ref() {
+            if let PathSelector::Downcast(_, _, discr_val) = selector.as_ref() {
                 let discriminator = Path::new_discriminant(canonical_qualifier.clone());
                 if let Some(val) = environment.value_at(&discriminator) {
-                    if let Expression::CompileTimeConstant(ConstantDomain::U128(ordinal)) =
-                        &val.expression
-                    {
-                        if (*variant as u128) != *ordinal {
-                            // The downcast is impossible in this calling context
-                            return Path::new_computed(Rc::new(abstract_value::BOTTOM));
-                        }
+                    if val.is_compile_time_constant() && val.ne(discr_val) {
+                        // The downcast is impossible in this calling context
+                        return Path::new_computed(Rc::new(abstract_value::BOTTOM));
                     }
                 }
             }
@@ -1189,8 +1185,9 @@ pub enum PathSelector {
     ConstantSlice { from: u64, to: u64, from_end: bool },
 
     /// "Downcast" to a variant of an ADT. Currently, MIR only introduces
-    /// this for ADTs with more than one variant. The value is the ordinal of the variant.
-    Downcast(Rc<str>, usize),
+    /// this for ADTs with more than one variant. The usize value is the ordinal of the variant.
+    /// The constant value is the tag value that corresponds to the variant.
+    Downcast(Rc<str>, usize, Rc<AbstractValue>),
 
     /// Select the struct model field with the given name.
     /// A model field is a specification construct used during MIRAI verification
@@ -1233,7 +1230,9 @@ impl Debug for PathSelector {
             PathSelector::ConstantSlice { from, to, from_end } => {
                 f.write_fmt(format_args!("[{from} : {to}, from_end: {from_end}]"))
             }
-            PathSelector::Downcast(name, index) => f.write_fmt(format_args!("as {name}({index})")),
+            PathSelector::Downcast(name, index, val) => {
+                f.write_fmt(format_args!("as {name}({index}, {val:?})"))
+            }
             PathSelector::ModelField(name) => name.fmt(f),
             PathSelector::TagField => f.write_str("$tag"),
         }