From ce711d7e8a73796ada69be33565d283ca533d0e3 Mon Sep 17 00:00:00 2001 From: Dino Distefano Date: Fri, 3 Nov 2017 05:38:27 -0700 Subject: [PATCH] [uninit] Reporting parameters calls which are not passed by reference Reviewed By: sblackshear Differential Revision: D6209749 fbshipit-source-id: 2c59a1a --- infer/src/checkers/uninit.ml | 42 +++++--- .../tests/codetoanalyze/cpp/uninit/issues.exp | 2 + .../tests/codetoanalyze/cpp/uninit/uninit.cpp | 100 ++++++++++++++++-- 3 files changed, 120 insertions(+), 24 deletions(-) diff --git a/infer/src/checkers/uninit.ml b/infer/src/checkers/uninit.ml index d166774b9..8c3703b00 100644 --- a/infer/src/checkers/uninit.ml +++ b/infer/src/checkers/uninit.ml @@ -42,7 +42,8 @@ module TransferFunctions (CFG : ProcCfg.S) = struct module CFG = CFG module Domain = RecordDomain - let report message loc summary = + let report var loc summary = + let message = F.asprintf "The value read from %a was never initialized" Var.pp var 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 @@ -51,6 +52,27 @@ module TransferFunctions (CFG : ProcCfg.S) = struct type extras = FormalMap.t * Specs.summary + let should_report_var pdesc uninit_vars var = + match var with + | Var.ProgramVar pv -> + not (Pvar.is_frontend_tmp pv) && not (Procdesc.is_captured_var pdesc pv) + && exp_in_set (Exp.Lvar pv) uninit_vars + | _ -> + false + + + let report_on_function_params pdesc uninit_vars actuals loc extras = + List.iter + ~f:(fun e -> + match e with + | HilExp.AccessPath ((var, t), _) + when should_report_var pdesc uninit_vars var && not (is_type_pointer t) -> + report var loc (snd extras) + | _ -> + ()) + actuals + + let exec_instr (astate: Domain.astate) {ProcData.pdesc; ProcData.extras} _ (instr: HilInstr.t) = match instr with | Assign (((lhs_var, lhs_typ), _), HilExp.AccessPath (((rhs_var, rhs_typ) as rhs_base), _), loc) -> @@ -64,21 +86,14 @@ 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) - | _ -> - () ) ; + (* check on lhs_typ to avoid false positive when assigning a pointer to another *) + if should_report_var pdesc uninit_vars rhs_var && not (is_type_pointer lhs_typ) then + report rhs_var loc (snd extras) ; {astate with uninit_vars; prepost} | Assign (((lhs_var, _), _), _, _) -> let uninit_vars = D.remove lhs_var astate.uninit_vars in {astate with uninit_vars} - | Call (_, _, actuals, _, _) -> + | 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 = @@ -88,12 +103,13 @@ module TransferFunctions (CFG : ProcCfg.S) = struct | HilExp.AccessPath ((actual_var, {Typ.desc= Tptr _}), _) -> D.remove actual_var acc | HilExp.Closure (_, apl) -> - (* remove the captured variables of a block *) + (* remove the captured variables of a block/lambda *) List.fold ~f:(fun acc' ((av, _), _) -> D.remove av acc') ~init:acc apl | _ -> acc) ~init:astate.uninit_vars actuals in + report_on_function_params pdesc uninit_vars actuals loc extras ; {astate with uninit_vars} | Assume _ -> astate diff --git a/infer/tests/codetoanalyze/cpp/uninit/issues.exp b/infer/tests/codetoanalyze/cpp/uninit/issues.exp index 8d4e9b33a..c443da408 100644 --- a/infer/tests/codetoanalyze/cpp/uninit/issues.exp +++ b/infer/tests/codetoanalyze/cpp/uninit/issues.exp @@ -1,4 +1,6 @@ codetoanalyze/cpp/uninit/uninit.cpp, bad1, 2, UNINITIALIZED_VALUE, [] +codetoanalyze/cpp/uninit/uninit.cpp, bad2, 2, UNINITIALIZED_VALUE, [] codetoanalyze/cpp/uninit/uninit.cpp, branch1_FP, 11, UNINITIALIZED_VALUE, [] codetoanalyze/cpp/uninit/uninit.cpp, loop1_FP, 10, UNINITIALIZED_VALUE, [] codetoanalyze/cpp/uninit/uninit.cpp, no_init_return_bad, 2, UNINITIALIZED_VALUE, [] +codetoanalyze/cpp/uninit/uninit.cpp, ret_undef, 2, UNINITIALIZED_VALUE, [] diff --git a/infer/tests/codetoanalyze/cpp/uninit/uninit.cpp b/infer/tests/codetoanalyze/cpp/uninit/uninit.cpp index 924aa1342..88feca526 100644 --- a/infer/tests/codetoanalyze/cpp/uninit/uninit.cpp +++ b/infer/tests/codetoanalyze/cpp/uninit/uninit.cpp @@ -36,7 +36,9 @@ int ok1() { int b; no_init(&a); - b = a; // OK (only intraprocedural) + b = a; // OK only for intraprocedural case (we assume that something passed by + // reference is initialized). When analysis extended to + // interprocedural, it should report a warning. return b; } @@ -45,7 +47,9 @@ int ok2() { int c; no_init(&a); - c = inc(a); // OK (only intraprocedural) + c = inc(a); // OK only for intraprocedural case (we assume that something + // passed by reference is initialized). When analysis extended to + // interprocedural, it should report a warning. return c; } @@ -54,7 +58,8 @@ int ok3() { int c; init(&a); - c = a; // OK + c = a; // no report since the variable could be initialized when passed by + // reference in previous call return c; } @@ -65,7 +70,8 @@ int ok4() { init(&a); - c = inc(a); // ok + c = inc(a); // no report since the variable could be initialized when passed + // by reference in previous call return c; } @@ -76,9 +82,11 @@ int ok5() { no_init(&a); - b = a; // ok (only intraprocedural) + b = a; // OK only for intraprocedural case (we assume that something passed by + // reference is initialized). When analysis extended to + // interprocedural, it should report a warning. - c = inc(b); // do not report as it depends from line 31 + c = inc(b); // do not report as it depends from line above return c; } @@ -90,13 +98,15 @@ int square_no_init(int x, int& res) { return res * res; } void use_square_ok1() { int i; - square_init(2, i); + square_init(2, i); // OK since i is initialized when passed by reference } int use_square_ok2() { int i; - i = square_no_init(2, i); // OK only intraprocedural + i = square_no_init( + 2, i); // OK only for intraprocedural case. When analysis extended + // to interprocedural, it should report. return i; } @@ -113,7 +123,8 @@ int branch1_FP() { } if (ok) { - return size; + return size; // report here because size initialized only on the then-branch + // above } return 0; @@ -129,7 +140,7 @@ int loop1_FP() { break; } - return size; + return size; // report here because size initialized only inside loop } int ok6() { @@ -142,7 +153,74 @@ int ok6() { void deref_magic_addr_ok() { *(int*)0xdeadbeef = 0; } char ok7() { - char buf[1024], *res = buf; + char buf[1024], *res = buf; // OK, because we copy an address res[1] = 'a'; return res[1]; } + +void use_an_int(int); + +void bad2() { + int a; + use_an_int(a); // Report as we pass an unitialized value +} + +void ok8() { + int a; + init(&a); // no report since the variable could be initialized when passed by + // reference. +} + +int ret_undef() { + int* p; + return *p; // report as p was not initialized +} + +int ret_undef_FP() { + int* p; + int* q; + p = q; // no report as we copy an address + return *p; // NO report as we don't keep track of aliasing (for now) +} + +void use_an_int2(int*); + +int ok9() { + int buf[1024]; + use_an_int2(buf); // no report as we pass the pointer to buf + return 1; +} + +void FN_capture_read_bad() { + int x; + [x]() { + int y = x; + return; + }(); // We should report that captured x is not init +} + +void init_capture_read_ok() { + int x; + [x = 0]() { + int y = x; + return; + } + (); +} + +void init_capture_ok() { + [i = 0]() { return i; }; +} + +void FN_capture_by_ref_reuseBad() { + int x; + [&x]() { + int y = x; + }(); // We don't report here as we only do intraprocedural analysis for now +} + +int capture_by_ref_init_ok() { + int x; + [&x]() { x = 1; }(); + return x; +}