[uninit] Use type information from locals if argument has type void* in function signature

Summary: In an intra-procedural analysis we assume that parameters passed by reference to a function will be initialized inside that function. We use the type information of an actual parameter to initialize the fields of the struct. This does not work if a function has a parameter of type void* as the actual parameters also has type void*. To solve this issue, we use type information from local variables.

Reviewed By: jvillard

Differential Revision: D20670253

fbshipit-source-id: dc9f051ef

infer/src/checkers/uninit.ml

@@ -40,6 +40,37 @@ let should_report_on_type t =
 
 type extras = {formals: FormalMap.t; summary: Summary.t}
 
+module Initial = struct
+  let get_locals tenv pdesc =
+    List.fold (Procdesc.get_locals pdesc) ~init:[]
+      ~f:(fun acc (var_data : ProcAttributes.var_data) ->
+        let pvar = Pvar.mk var_data.name (Procdesc.get_proc_name pdesc) in
+        let base_access_expr = HilExp.AccessExpression.base (Var.of_pvar pvar, var_data.typ) in
+        match var_data.typ.Typ.desc with
+        | Typ.Tstruct qual_name
+        (* T30105165 remove filtering after we improve union translation *)
+          when not (Typ.Name.is_union qual_name) -> (
+          match Tenv.lookup tenv qual_name with
+          | Some {fields} ->
+              let flist =
+                List.fold
+                  ~f:(fun acc' (fn, _, _) ->
+                    HilExp.AccessExpression.field_offset base_access_expr fn :: acc' )
+                  ~init:acc fields
+              in
+              base_access_expr :: flist
+              (* for struct we take the struct address, and the access_path
+                 to the fields one level down *)
+          | _ ->
+              acc )
+        | Typ.Tarray {elt} ->
+            HilExp.AccessExpression.array_offset base_access_expr elt None :: acc
+        | Typ.Tptr _ ->
+            base_access_expr :: HilExp.AccessExpression.dereference base_access_expr :: acc
+        | _ ->
+            base_access_expr :: acc )
+end
+
 module TransferFunctions (CFG : ProcCfg.S) = struct
   module CFG = CFG
   module Domain = RecordDomain
@@ -240,8 +271,6 @@ module TransferFunctions (CFG : ProcCfg.S) = struct
           {astate with maybe_uninit_vars}
         else astate
     | Call (_, call, actuals, _, loc) ->
-        (* in case of intraprocedural only analysis we assume that parameters passed by reference
-           to a function will be initialized inside that function *)
         let pname_opt = match call with Direct pname -> Some pname | Indirect _ -> None in
         let callee_formals_opt = Option.bind pname_opt ~f:get_formals in
         let is_initializing_all_args =
@@ -274,11 +303,15 @@ module TransferFunctions (CFG : ProcCfg.S) = struct
                       MaybeUninitVars.remove_all_fields tenv base
                         (MaybeUninitVars.remove access_expr_to_remove acc)
                   | _, {Typ.desc= Tptr _} -> (
+                    (* in case of intraprocedural only analysis we assume that parameters passed by reference
+                       to a function will be initialized inside that function *)
                     match pname_opt with
                     | Some pname when Config.uninit_interproc ->
                         remove_initialized_params summary pname acc idx access_expr_to_remove true
                     | _ ->
-                        MaybeUninitVars.remove_everything_under tenv access_expr_to_remove acc )
+                        let locals = MaybeUninitVars.of_list (Initial.get_locals tenv pdesc) in
+                        MaybeUninitVars.remove_everything_under tenv locals access_expr_to_remove
+                          acc )
                   | _ ->
                       acc )
               | HilExp.Closure (_, apl) ->
@@ -306,37 +339,6 @@ end
 module CFG = ProcCfg.Normal
 module Analyzer = LowerHil.MakeAbstractInterpreter (TransferFunctions (CFG))
 
-module Initial = struct
-  let get_locals tenv pdesc =
-    List.fold (Procdesc.get_locals pdesc) ~init:[]
-      ~f:(fun acc (var_data : ProcAttributes.var_data) ->
-        let pvar = Pvar.mk var_data.name (Procdesc.get_proc_name pdesc) in
-        let base_access_expr = HilExp.AccessExpression.base (Var.of_pvar pvar, var_data.typ) in
-        match var_data.typ.Typ.desc with
-        | Typ.Tstruct qual_name
-        (* T30105165 remove filtering after we improve union translation *)
-          when not (Typ.Name.is_union qual_name) -> (
-          match Tenv.lookup tenv qual_name with
-          | Some {fields} ->
-              let flist =
-                List.fold
-                  ~f:(fun acc' (fn, _, _) ->
-                    HilExp.AccessExpression.field_offset base_access_expr fn :: acc' )
-                  ~init:acc fields
-              in
-              base_access_expr :: flist
-              (* for struct we take the struct address, and the access_path
-                 to the fields one level down *)
-          | _ ->
-              acc )
-        | Typ.Tarray {elt} ->
-            HilExp.AccessExpression.array_offset base_access_expr elt None :: acc
-        | Typ.Tptr _ ->
-            base_access_expr :: HilExp.AccessExpression.dereference base_access_expr :: acc
-        | _ ->
-            base_access_expr :: acc )
-end
-
 let checker {Callbacks.exe_env; summary} : Summary.t =
   let proc_desc = Summary.get_proc_desc summary in
   let proc_name = Summary.get_proc_name summary in

infer/src/checkers/uninitDomain.ml

@@ -43,20 +43,31 @@ module MaybeUninitVars = struct
         maybe_uninit_vars
 
 
-  let remove_all_fields tenv base maybe_uninit_vars =
+  let find_access_expr_typ tenv access_expr vars =
-    match base with
+    try HilExp.AccessExpression.get_typ (find access_expr vars) tenv with Caml.Not_found -> None
-    | _, {Typ.desc= Tptr ({Typ.desc= Tstruct name_struct}, _)} | _, {Typ.desc= Tstruct name_struct}
+
-      -> (
+
-      match Tenv.lookup tenv name_struct with
+  let remove_all_fields tenv ?(locals = empty) base maybe_uninit_vars =
-      | Some {fields} ->
+    let remove_all_fields_inner base_type =
-          List.fold fields ~init:maybe_uninit_vars ~f:(fun acc (fn, _, _) ->
+      match base_type.Typ.desc with
-              remove
+      | Typ.Tptr ({Typ.desc= Tstruct name_struct}, _) | Typ.Tstruct name_struct -> (
-                (HilExp.AccessExpression.field_offset (HilExp.AccessExpression.base base) fn)
+        match Tenv.lookup tenv name_struct with
-                acc )
+        | Some {fields} ->
+            List.fold fields ~init:maybe_uninit_vars ~f:(fun acc (fn, _, _) ->
+                remove
+                  (HilExp.AccessExpression.field_offset (HilExp.AccessExpression.base base) fn)
+                  acc )
+        | _ ->
+            maybe_uninit_vars )
       | _ ->
-          maybe_uninit_vars )
+          maybe_uninit_vars
-    | _ ->
+    in
-        maybe_uninit_vars
+    match base with
+    | _, {Typ.desc= Tptr ({Typ.desc= Tvoid}, _)} ->
+        Option.value_map ~default:maybe_uninit_vars ~f:remove_all_fields_inner
+          (find_access_expr_typ tenv (HilExp.AccessExpression.base base) locals)
+    | _, typ ->
+        remove_all_fields_inner typ
 
 
   let remove_dereference_access base maybe_uninit_vars =
@@ -79,9 +90,10 @@ module MaybeUninitVars = struct
         maybe_uninit_vars
 
 
-  let remove_everything_under tenv access_expr maybe_uninit_vars =
+  let remove_everything_under tenv locals access_expr maybe_uninit_vars =
     let base = HilExp.AccessExpression.get_base access_expr in
-    maybe_uninit_vars |> remove access_expr |> remove_all_fields tenv base
+    maybe_uninit_vars |> remove access_expr
+    |> remove_all_fields tenv ~locals base
     |> remove_all_array_elements base |> remove_dereference_access base
 end
 

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

@@ -10,7 +10,6 @@ codetoanalyze/cpp/uninit/members.cpp, access_pointer_members_bad, 5, UNINITIALIZ
 codetoanalyze/cpp/uninit/struct.cpp, pass_basic_type_field_bad, 3, UNINITIALIZED_VALUE, no_bucket, ERROR, []
 codetoanalyze/cpp/uninit/struct.cpp, struct_partial_init_bad, 6, UNINITIALIZED_VALUE, no_bucket, ERROR, []
 codetoanalyze/cpp/uninit/struct.cpp, struct_uninit_bad, 3, UNINITIALIZED_VALUE, no_bucket, ERROR, []
-codetoanalyze/cpp/uninit/uninit.cpp, FP_pointer_param_void_star_ok, 4, UNINITIALIZED_VALUE, no_bucket, ERROR, []
 codetoanalyze/cpp/uninit/uninit.cpp, bad1, 2, UNINITIALIZED_VALUE, no_bucket, ERROR, []
 codetoanalyze/cpp/uninit/uninit.cpp, bad2, 2, UNINITIALIZED_VALUE, no_bucket, ERROR, []
 codetoanalyze/cpp/uninit/uninit.cpp, branch1_FP, 11, UNINITIALIZED_VALUE, no_bucket, ERROR, []

infer/tests/codetoanalyze/cpp/uninit/uninit.cpp

@@ -269,11 +269,11 @@ int no_warning_noreturn_callee_ok(bool t) {
 
 void some_f(void* p);
 
-int* FP_pointer_param_void_star_ok() {
+int* pointer_param_void_star_ok() {
   A a;
   int* res;
-  some_f(&a); // the type of a here is void*, hence no fields are found
+  some_f(&a);
-  return a.ptr; // false positive
+  return a.ptr;
 }
 
 short union_ok() {