do not report on captured vars in objc blocks

Reviewed By: jeremydubreil

Differential Revision: D6173238

fbshipit-source-id: dc93a54

Makefile

@@ -67,7 +67,7 @@ BUILD_SYSTEMS_TESTS += xcodebuild_no_xcpretty objc_getters_setters
 DIRECT_TESTS += \
   objc_frontend objc_errors objc_linters objc_ioslints \
 	objcpp_frontend objcpp_linters objc_linters-for-test-only objcpp_linters-for-test-only \
-	objc_linters-def-folder objc_checkers objc_liveness
+	objc_linters-def-folder objc_checkers objc_liveness objc_uninit
 ifneq ($(XCPRETTY),no)
 BUILD_SYSTEMS_TESTS += xcodebuild
 endif

infer/src/IR/Procdesc.ml

@@ -544,3 +544,22 @@ let is_specialized pdesc =
   let attributes = get_attributes pdesc in
   attributes.ProcAttributes.is_specialized
 
+
+(* true if pvar is a captred variable of a cpp lambda or obcj block *)
+let is_captured_var procdesc pvar =
+  let procname = get_proc_name procdesc in
+  let pvar_name = Pvar.get_name pvar in
+  let pvar_matches (name, _) = Mangled.equal name pvar_name in
+  let is_captured_var_cpp_lambda =
+    (* var is captured if the procedure is a lambda and the var is not in the locals or formals *)
+    Typ.Procname.is_cpp_lambda procname
+    && not
+         ( List.exists ~f:pvar_matches (get_locals procdesc)
+         || List.exists ~f:pvar_matches (get_formals procdesc) )
+  in
+  let is_captured_var_objc_block =
+    (* var is captured if the procedure is a objc block and the var is in the captured *)
+    Typ.Procname.is_objc_block procname && List.exists ~f:pvar_matches (get_captured procdesc)
+  in
+  is_captured_var_cpp_lambda || is_captured_var_objc_block
+

infer/src/IR/Procdesc.mli

@@ -249,3 +249,7 @@ val is_loop_head : t -> Node.t -> bool
 val pp_signature : Format.formatter -> t -> unit
 
 val is_specialized : t -> bool
+
+(* true if pvar is a captred variable of a cpp lambda or obcj block *)
+
+val is_captured_var : t -> Pvar.t -> bool

infer/src/checkers/liveness.ml

@@ -61,25 +61,6 @@ end
 module CFG = ProcCfg.OneInstrPerNode (ProcCfg.Backward (ProcCfg.Exceptional))
 module Analyzer = AbstractInterpreter.Make (CFG) (TransferFunctions)
 
-let is_captured_var procdesc pvar =
-  let procname = Procdesc.get_proc_name procdesc in
-  let pvar_name = Pvar.get_name pvar in
-  let pvar_matches (name, _) = Mangled.equal name pvar_name in
-  let is_captured_var_cpp_lambda =
-    (* var is captured if the procedure is a lambda and the var is not in the locals or formals *)
-    Typ.Procname.is_cpp_lambda procname
-    && not
-         ( List.exists ~f:pvar_matches (Procdesc.get_locals procdesc)
-         || List.exists ~f:pvar_matches (Procdesc.get_formals procdesc) )
-  in
-  let is_captured_var_objc_block =
-    (* var is captured if the procedure is a objc block and the var is in the captured *)
-    Typ.Procname.is_objc_block procname
-    && List.exists ~f:pvar_matches (Procdesc.get_captured procdesc)
-  in
-  is_captured_var_cpp_lambda || is_captured_var_objc_block
-
-
 let is_whitelisted_var var =
   let whitelisted_vars = ["__assert_file__"; "__assert_fn__"] in
   List.exists
@@ -96,7 +77,7 @@ let checker {Callbacks.tenv; summary; proc_desc} : Specs.summary =
     | Sil.Store (Lvar pvar, _, _, loc)
       when not
              ( Pvar.is_frontend_tmp pvar || Pvar.is_return pvar || Pvar.is_global pvar
-             || Domain.mem (Var.of_pvar pvar) live_vars || is_captured_var proc_desc pvar
+             || Domain.mem (Var.of_pvar pvar) live_vars || Procdesc.is_captured_var proc_desc pvar
              || is_whitelisted_var pvar ) ->
         let issue_id = IssueType.dead_store.unique_id in
         let message = F.asprintf "The value written to %a is never used" (Pvar.pp Pp.text) pvar in

infer/src/checkers/uninit.ml

@@ -31,20 +31,32 @@ end)
 
 let intraprocedural_only = true
 
-let is_address_pvar e = match e with Exp.Lvar _ -> true | _ -> false
+let is_type_pointer t = match t.Typ.desc with Typ.Tptr _ -> true | _ -> false
+
+let exp_in_set exp vset =
+  let _, pvars = Exp.get_vars exp in
+  List.exists ~f:(fun pv -> D.mem (Var.of_pvar pv) vset) pvars
+
 
 module TransferFunctions (CFG : ProcCfg.S) = struct
   module CFG = CFG
   module Domain = RecordDomain
 
-  type extras = FormalMap.t
+  let report message loc summary =
+    let issue_id = IssueType.uninitialized_value.unique_id in
+    let ltr = [Errlog.make_trace_element 0 loc "" []] in
+    let exn = Exceptions.Checkers (issue_id, Localise.verbatim_desc message) in
+    Reporting.log_error summary ~loc ~ltr exn
+
+
+  type extras = FormalMap.t * Specs.summary
 
-  let exec_instr (astate: Domain.astate) {ProcData.extras} _ (instr: HilInstr.t) =
+  let exec_instr (astate: Domain.astate) {ProcData.pdesc; ProcData.extras} _ (instr: HilInstr.t) =
     match instr with
-    | Assign (((lhs_var, _), _), HilExp.AccessPath (((rhs_var, rhs_typ) as rhs_base), _), _) ->
+    | Assign (((lhs_var, lhs_typ), _), HilExp.AccessPath (((rhs_var, rhs_typ) as rhs_base), _), loc) ->
         let uninit_vars = D.remove lhs_var astate.uninit_vars in
         let prepost =
-          if FormalMap.is_formal rhs_base extras
+          if FormalMap.is_formal rhs_base (fst extras)
              && match rhs_typ.desc with Typ.Tptr _ -> true | _ -> false
           then
             let pre' = D.add rhs_var (fst astate.prepost) in
@@ -52,6 +64,16 @@ module TransferFunctions (CFG : ProcCfg.S) = struct
             (pre', post)
           else astate.prepost
         in
+        ( match rhs_var with
+        | Var.ProgramVar pv
+          when not (Pvar.is_frontend_tmp pv) && not (Procdesc.is_captured_var pdesc pv)
+               && exp_in_set (Exp.Lvar pv) uninit_vars && not (is_type_pointer lhs_typ) ->
+            let message =
+              F.asprintf "The value read from %a was never initialized" Exp.pp (Exp.Lvar pv)
+            in
+            report message loc (snd extras)
+        | _ ->
+            () ) ;
         {astate with uninit_vars; prepost}
     | Assign (((lhs_var, _), _), _, _) ->
         let uninit_vars = D.remove lhs_var astate.uninit_vars in
@@ -65,6 +87,9 @@ module TransferFunctions (CFG : ProcCfg.S) = struct
               match actual_exp with
               | HilExp.AccessPath ((actual_var, {Typ.desc= Tptr _}), _) ->
                   D.remove actual_var acc
+              | HilExp.Closure (_, apl) ->
+                  (* remove the captured variables of a block *)
+                  List.fold ~f:(fun acc' ((av, _), _) -> D.remove av acc') ~init:acc apl
               | _ ->
                   acc)
             ~init:astate.uninit_vars actuals
@@ -97,92 +122,10 @@ let checker {Callbacks.tenv; summary; proc_desc} : Specs.summary =
     , IdAccessPathMapDomain.empty )
   in
   let invariant_map =
-    Analyzer.exec_cfg cfg (ProcData.make proc_desc tenv formal_map) ~initial:init ~debug:false
-  in
-  (* Check if an expression is in set of variables *)
-  let exp_in_set exp vset =
-    let _, pvars = Exp.get_vars exp in
-    List.exists ~f:(fun pv -> D.mem (Var.of_pvar pv) vset) pvars
-  in
-  let zip_actual_formal_params callee_pname actual_params =
-    match Ondemand.get_proc_desc callee_pname with
-    | Some pdesc ->
-        let formals, _ = List.unzip (Procdesc.get_formals pdesc) in
-        let actual, _ = List.unzip actual_params in
-        (List.zip actual formals, actual)
-    | _ ->
-        (None, [])
-  in
-  let deref_actual_params callee_pname actual_params deref_formal_params =
-    match zip_actual_formal_params callee_pname actual_params with
-    | None, _ ->
-        []
-    | Some assoc_actual_formal, _ ->
-        List.fold
-          ~f:(fun acc (a, f) ->
-            let fe = Exp.Lvar (Pvar.mk f callee_pname) in
-            if exp_in_set fe deref_formal_params then a :: acc else acc)
-          ~init:[] assoc_actual_formal
-  in
-  let report_uninit_value uninit_vars instr =
-    let report message loc =
-      let issue_id = IssueType.uninitialized_value.unique_id in
-      let ltr = [Errlog.make_trace_element 0 loc "" []] in
-      let exn = Exceptions.Checkers (issue_id, Localise.verbatim_desc message) in
-      Reporting.log_error summary ~loc ~ltr exn
-    in
-    match instr with
-    | Sil.Load (_, Exp.Lvar pv, _, loc)
-      when not (Pvar.is_frontend_tmp pv) && exp_in_set (Exp.Lvar pv) uninit_vars ->
-        let message =
-          F.asprintf "The value read from %a was never initialized" Exp.pp (Exp.Lvar pv)
-        in
-        report message loc
-    | Sil.Store (lhs, _, Exp.Lvar pv, loc)
-      when not (Pvar.is_frontend_tmp pv) && exp_in_set (Exp.Lvar pv) uninit_vars
-           && not (is_address_pvar lhs) ->
-        let message =
-          F.asprintf "The value read from %a was never initialized" Exp.pp (Exp.Lvar pv)
-        in
-        report message loc
-    | Sil.Call (_, Exp.Const Const.Cfun callee_pname, actual_params, loc, _)
-      when not intraprocedural_only -> (
-      match Summary.read_summary proc_desc callee_pname with
-      | Some {pre= deref_formal_params; post= _} ->
-          let deref_actual_params =
-            deref_actual_params callee_pname actual_params deref_formal_params
-          in
-          List.iter
-            ~f:(fun (e, _) ->
-              if exp_in_set e uninit_vars && List.mem ~equal:Exp.equal deref_actual_params e then
-                let message =
-                  F.asprintf "The value of %a is read in %a but was never initialized" Exp.pp e
-                    Typ.Procname.pp callee_pname
-                in
-                report message loc)
-            actual_params
-      | _ ->
-          () )
-    | _ ->
-        ()
-  in
-  let report_on_node node =
-    List.iter (CFG.instr_ids node) ~f:(fun (instr, node_id_opt) ->
-        match node_id_opt with
-        | Some node_id -> (
-          match Analyzer.extract_pre node_id invariant_map with
-          | Some
-              ( { RecordDomain.uninit_vars= uninitialized_vars
-                ; RecordDomain.aliased_vars= _
-                ; RecordDomain.prepost= _ }
-              , _ ) ->
-              report_uninit_value uninitialized_vars instr
-          | None ->
-              () )
-        | None ->
-            () )
+    Analyzer.exec_cfg cfg
+      (ProcData.make proc_desc tenv (formal_map, summary))
+      ~initial:init ~debug:false
   in
-  List.iter (CFG.nodes cfg) ~f:report_on_node ;
   match Analyzer.extract_post (CFG.id (CFG.exit_node cfg)) invariant_map with
   | Some
       ( {RecordDomain.uninit_vars= _; RecordDomain.aliased_vars= _; RecordDomain.prepost= pre, post}

infer/src/clang/cAst_utils.ml

@@ -568,4 +568,3 @@ let has_block_attribute decl =
 let is_implicit_decl decl =
   let decl_info = Clang_ast_proj.get_decl_tuple decl in
   decl_info.Clang_ast_t.di_is_implicit
-

infer/tests/codetoanalyze/objc/uninit/Makefile

@@ -0,0 +1,20 @@
+# Copyright (c) 2017 - present Facebook, Inc.
+# All rights reserved.
+#
+# This source code is licensed under the BSD style license found in the
+# LICENSE file in the root directory of this source tree. An additional grant
+# of patent rights can be found in the PATENTS file in the same directory.
+
+TESTS_DIR = ../../..
+
+ANALYZER = checkers
+# see explanations in cpp/errors/Makefile for the custom isystem
+CLANG_OPTIONS = -x objective-c -c
+INFER_OPTIONS = --uninit-only --debug-exceptions --project-root $(TESTS_DIR)
+INFERPRINT_OPTIONS = --issues-tests
+
+SOURCES = uninit_blocks.m
+
+include $(TESTS_DIR)/clang.make
+
+infer-out/report.json: $(MAKEFILE_LIST)

infer/tests/codetoanalyze/objc/uninit/issues.exp

@@ -0,0 +1 @@
+codetoanalyze/objc/uninit/uninit_blocks.m, A_bad1, 5, UNINITIALIZED_VALUE, []

infer/tests/codetoanalyze/objc/uninit/uninit_blocks.m

@@ -0,0 +1,49 @@
+/*
+ * Copyright (c) 2017 - present Facebook, Inc.
+ * All rights reserved.
+ *
+ * This source code is licensed under the BSD style license found in the
+ * LICENSE file in the root directory of this source tree. An additional grant
+ * of patent rights can be found in the PATENTS file in the same directory.
+ */
+#import <Foundation/NSObject.h>
+
+@interface A : NSObject
+@end
+
+typedef void (^MyBlock)();
+
+@interface C : NSObject
+
++ (void)use_block:(MyBlock)block;
+
+@end
+
+@implementation A
+
++ (int)ok1 {
+  __block int a;
+  [C use_block:^() {
+    a = 10;
+  }];
+  return a;
+};
+
++ (int)bad1 {
+  int a;
+  [C use_block:^() {
+    int x = 0;
+  }];
+  return a;
+};
+
++ (instancetype)ok2 {
+  static id sharedInstance;
+  ^{
+    sharedInstance = [[self alloc] init];
+  }();
+
+  return sharedInstance;
+}
+
+@end