From dfb8b421ac79e909d44fe3477b0b7db7c743a922 Mon Sep 17 00:00:00 2001 From: Dino Distefano Date: Tue, 28 Nov 2017 04:59:15 -0800 Subject: [PATCH] Extending uninit to struct Reviewed By: sblackshear Differential Revision: D6348941 fbshipit-source-id: 2d49118 --- infer/src/checkers/uninit.ml | 46 ++++++++++++++++--- .../tests/codetoanalyze/cpp/uninit/issues.exp | 2 + .../tests/codetoanalyze/cpp/uninit/struct.cpp | 19 +++++++- 3 files changed, 59 insertions(+), 8 deletions(-) diff --git a/infer/src/checkers/uninit.ml b/infer/src/checkers/uninit.ml index f85dad73a..78740f598 100644 --- a/infer/src/checkers/uninit.ml +++ b/infer/src/checkers/uninit.ml @@ -53,8 +53,8 @@ module TransferFunctions (CFG : ProcCfg.S) = struct module CFG = CFG module Domain = RecordDomain - let report var loc summary = - let message = F.asprintf "The value read from %a was never initialized" Var.pp var in + let report ap loc summary = + let message = F.asprintf "The value read from %a was never initialized" AccessPath.pp ap in 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 @@ -78,7 +78,7 @@ module TransferFunctions (CFG : ProcCfg.S) = struct match e with | HilExp.AccessPath ((var, t), al) when should_report_var pdesc tenv uninit_vars ((var, t), al) && not (is_type_pointer t) -> - report var loc (snd extras) + report ((var, t), al) loc (snd extras) | _ -> ()) actuals @@ -103,12 +103,38 @@ module TransferFunctions (CFG : ProcCfg.S) = struct D.remove (base, [AccessPath.ArrayAccess (snd base, [])]) uninit_vars + let get_formals call = + match Ondemand.get_proc_desc call with + | Some proc_desc -> + Procdesc.get_formals proc_desc + | _ -> + [] + + + let is_struct t = match Typ.name t with Some _ -> true | _ -> false + + let function_expect_a_pointer call idx = + let formals = get_formals call in + match List.nth formals idx with Some (_, typ) -> is_type_pointer typ | _ -> false + + + let is_dummy_constructor_of_a_struct call = + let is_dummy_constructor_of_struct = + match get_formals call with + | [(_, {Typ.desc= Typ.Tptr ({Typ.desc= Tstruct _}, _)})] -> + true + | _ -> + false + in + Typ.Procname.is_constructor call && is_dummy_constructor_of_struct + + let exec_instr (astate: Domain.astate) {ProcData.pdesc; ProcData.extras; ProcData.tenv} _ (instr: HilInstr.t) = match instr with | Assign ( (((lhs_var, lhs_typ), apl) as lhs_ap) - , HilExp.AccessPath (((rhs_var, rhs_typ) as rhs_base), al) + , HilExp.AccessPath (((_, rhs_typ) as rhs_base), al) , loc ) -> let uninit_vars' = D.remove lhs_ap astate.uninit_vars in let uninit_vars = @@ -128,7 +154,7 @@ module TransferFunctions (CFG : ProcCfg.S) = struct in (* check on lhs_typ to avoid false positive when assigning a pointer to another *) if should_report_var pdesc tenv uninit_vars (rhs_base, al) && not (is_type_pointer lhs_typ) - then report rhs_var loc (snd extras) ; + then report (rhs_base, al) loc (snd extras) ; {astate with uninit_vars; prepost} | Assign (lhs, _, _) -> let uninit_vars = D.remove lhs astate.uninit_vars in @@ -136,12 +162,14 @@ module TransferFunctions (CFG : ProcCfg.S) = struct | Call (_, Direct callee_pname, _, _, _) when Typ.Procname.equal callee_pname BuiltinDecl.objc_cpp_throw -> {astate with uninit_vars= D.empty} + | Call (_, HilInstr.Direct call, _, _, _) when is_dummy_constructor_of_a_struct call -> + astate | Call (_, HilInstr.Direct 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 uninit_vars = - List.fold - ~f:(fun acc actual_exp -> + List.foldi + ~f:(fun idx acc actual_exp -> match actual_exp with | HilExp.AccessPath (((_, {Typ.desc= Tarray _}) as base), al) when is_blacklisted_function call -> @@ -155,6 +183,10 @@ module TransferFunctions (CFG : ProcCfg.S) = struct | HilExp.AccessPath (((_, {Typ.desc= Tptr (t', _)}) as base), al) -> let acc' = D.remove (base, al) acc in remove_array_element (fst base, t') acc' + | HilExp.AccessPath (((_, t) as base), al) + when is_struct t && List.length al > 0 && function_expect_a_pointer call idx -> + (* Access to a field of a struct by reference *) + D.remove (base, al) acc | HilExp.Closure (_, apl) -> (* remove the captured variables of a block/lambda *) List.fold ~f:(fun acc' (base, _) -> D.remove (base, []) acc') ~init:acc apl diff --git a/infer/tests/codetoanalyze/cpp/uninit/issues.exp b/infer/tests/codetoanalyze/cpp/uninit/issues.exp index c82ac57b8..d3bb6abbf 100644 --- a/infer/tests/codetoanalyze/cpp/uninit/issues.exp +++ b/infer/tests/codetoanalyze/cpp/uninit/issues.exp @@ -1,3 +1,5 @@ +codetoanalyze/cpp/uninit/struct.cpp, pass_basic_type_field_bad, 3, UNINITIALIZED_VALUE, [] +codetoanalyze/cpp/uninit/struct.cpp, struct_uninit_bad, 3, UNINITIALIZED_VALUE, [] codetoanalyze/cpp/uninit/uninit.cpp, FP_no_warning_noreturn_callee_ok, 7, UNINITIALIZED_VALUE, [] codetoanalyze/cpp/uninit/uninit.cpp, bad1, 2, UNINITIALIZED_VALUE, [] codetoanalyze/cpp/uninit/uninit.cpp, bad2, 2, UNINITIALIZED_VALUE, [] diff --git a/infer/tests/codetoanalyze/cpp/uninit/struct.cpp b/infer/tests/codetoanalyze/cpp/uninit/struct.cpp index d4ff39600..62d546973 100644 --- a/infer/tests/codetoanalyze/cpp/uninit/struct.cpp +++ b/infer/tests/codetoanalyze/cpp/uninit/struct.cpp @@ -6,6 +6,7 @@ * 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. */ + #include struct SimpleRuleKey { @@ -38,7 +39,7 @@ std::string struct_uninit_ok() { return s1; } -int FN_struct_uninit() { +int struct_uninit_bad() { int k; SimpleRuleKey srk; k = srk.i; // Should reports here: srk was not initialized and i has type int @@ -91,6 +92,22 @@ int init_field_via_function_ptr_ok() { return t->n; } +int get_a_int_pointer(int* x) { return *x; }; + +void pass_pointer_of_field_OK() { + struct s my_t; + + get_a_int_pointer(&my_t.n); +} + +int get_an_int(int x){}; + +void pass_basic_type_field_bad() { + struct s my_t; + + get_an_int(my_t.n); // pass an uninit int +} + enum class FieldType : uint8_t { Stop = 0x0, True = 0x1,