diff --git a/infer/src/checkers/liveness.ml b/infer/src/checkers/liveness.ml index 5e3797dca..a5f1afef9 100644 --- a/infer/src/checkers/liveness.ml +++ b/infer/src/checkers/liveness.ml @@ -34,7 +34,32 @@ module TransferFunctions (CFG : ProcCfg.S) = struct ~init:astate' pvars + let add_live_actuals actuals call_exp live_acc = + let add_live_actuals_ exps acc = + List.fold exps ~f:(fun acc_ (exp, _) -> exp_add_live exp acc_) ~init:acc + in + match call_exp with + | Exp.Const Cfun (Typ.Procname.ObjC_Cpp _ as pname) when Typ.Procname.is_constructor pname -> ( + match + (* first actual passed to a C++ constructor is actually written, not read *) + actuals + with + | (Exp.Lvar pvar, _) :: exps -> + Domain.remove (Var.of_pvar pvar) live_acc |> add_live_actuals_ exps + | exps -> + add_live_actuals_ exps live_acc ) + | _ -> + add_live_actuals_ actuals live_acc + + let exec_instr astate _ _ = function + | Sil.Load (lhs_id, _, _, _) when Ident.is_none lhs_id -> + (* dummy deref inserted by frontend--don't count as a read *) + astate + | Sil.Call (_, Exp.Const Cfun (Typ.Procname.ObjC_Cpp _ as pname), _, _, _) + when Typ.Procname.is_destructor pname -> + (* don't count destructor calls as a read *) + astate | Sil.Load (lhs_id, rhs_exp, _, _) -> Domain.remove (Var.of_id lhs_id) astate |> exp_add_live rhs_exp | Sil.Store (Lvar lhs_pvar, _, rhs_exp, _) -> @@ -47,12 +72,11 @@ module TransferFunctions (CFG : ProcCfg.S) = struct exp_add_live lhs_exp astate |> exp_add_live rhs_exp | Sil.Prune (exp, _, _, _) -> exp_add_live exp astate - | Sil.Call (ret_id, call_exp, params, _, _) -> + | Sil.Call (ret_id, call_exp, actuals, _, _) -> Option.value_map ~f:(fun (ret_id, _) -> Domain.remove (Var.of_id ret_id) astate) ~default:astate ret_id - |> exp_add_live call_exp - |> fun x -> List.fold_right ~f:exp_add_live (List.map ~f:fst params) ~init:x + |> exp_add_live call_exp |> add_live_actuals actuals call_exp | Sil.Declare_locals _ | Remove_temps _ | Abstract _ | Nullify _ -> astate @@ -77,17 +101,26 @@ let checker {Callbacks.tenv; summary; proc_desc} : Specs.summary = | _ -> false in + let should_report pvar live_vars = + not + ( Pvar.is_frontend_tmp pvar || Pvar.is_return pvar || Pvar.is_global pvar + || Domain.mem (Var.of_pvar pvar) live_vars || Procdesc.is_captured_var proc_desc pvar ) + in + let log_report pvar loc = + 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 + let ltr = [Errlog.make_trace_element 0 loc "Write of unused value" []] in + let exn = Exceptions.Checkers (issue_id, Localise.verbatim_desc message) in + Reporting.log_error summary ~loc ~ltr exn + in let report_dead_store live_vars = function | Sil.Store (Lvar pvar, _, rhs_exp, loc) - when not - ( Pvar.is_frontend_tmp pvar || Pvar.is_return pvar || Pvar.is_global pvar - || Domain.mem (Var.of_pvar pvar) live_vars || Procdesc.is_captured_var proc_desc pvar - || is_sentinel_exp rhs_exp ) -> - 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 - let ltr = [Errlog.make_trace_element 0 loc "Write of unused value" []] in - let exn = Exceptions.Checkers (issue_id, Localise.verbatim_desc message) in - Reporting.log_error summary ~loc ~ltr exn + when should_report pvar live_vars && not (is_sentinel_exp rhs_exp) -> + log_report pvar loc + | Sil.Call + (None, Exp.Const Cfun (Typ.Procname.ObjC_Cpp _ as pname), (Exp.Lvar pvar, _) :: _, loc, _) + when Typ.Procname.is_constructor pname && should_report pvar live_vars -> + log_report pvar loc | _ -> () in diff --git a/infer/tests/codetoanalyze/cpp/liveness/dead_stores.cpp b/infer/tests/codetoanalyze/cpp/liveness/dead_stores.cpp index 273b1b067..19f6eaefd 100644 --- a/infer/tests/codetoanalyze/cpp/liveness/dead_stores.cpp +++ b/infer/tests/codetoanalyze/cpp/liveness/dead_stores.cpp @@ -280,4 +280,36 @@ int* sentinel_ptr_ok(int* j) { return i; } +struct S { + ~S() {} +}; + +typedef S&& B; + +S mk_s() { + S s; + return s; +}; + +// s gets read by the destructor for S +void dead_struct_value1_bad() { S s = mk_s(); } + +// need to handle operator= in order to detect this case +void FN_dead_struct_value2_bad() { + S s = mk_s(); + s = mk_s(); +} + +void dead_struct_rvalue_ref_bad() { B b = mk_s(); } + +S struct_value_used_ok() { + S s = mk_s(); + return s; +} + +B& struct_rvalue_ref_used_ok() { + B b = mk_s(); + return b; +} + } diff --git a/infer/tests/codetoanalyze/cpp/liveness/issues.exp b/infer/tests/codetoanalyze/cpp/liveness/issues.exp index 70468c247..3859fd733 100644 --- a/infer/tests/codetoanalyze/cpp/liveness/issues.exp +++ b/infer/tests/codetoanalyze/cpp/liveness/issues.exp @@ -1,5 +1,7 @@ codetoanalyze/cpp/liveness/dead_stores.cpp, dead_stores::FP_assign_array_tricky_ok, 3, DEAD_STORE, [Write of unused value] codetoanalyze/cpp/liveness/dead_stores.cpp, dead_stores::dead_pointer_bad, 2, DEAD_STORE, [Write of unused value] +codetoanalyze/cpp/liveness/dead_stores.cpp, dead_stores::dead_struct_rvalue_ref_bad, 0, DEAD_STORE, [Write of unused value] +codetoanalyze/cpp/liveness/dead_stores.cpp, dead_stores::dead_struct_value1_bad, 0, DEAD_STORE, [Write of unused value] codetoanalyze/cpp/liveness/dead_stores.cpp, dead_stores::dead_then_live_bad, 1, DEAD_STORE, [Write of unused value] codetoanalyze/cpp/liveness/dead_stores.cpp, dead_stores::easy_bad, 0, DEAD_STORE, [Write of unused value] codetoanalyze/cpp/liveness/dead_stores.cpp, dead_stores::init_capture_no_call_bad, 1, DEAD_STORE, [Write of unused value]