From 74670cb0ba220dd72c518f4cde35ebfb1b3bb780 Mon Sep 17 00:00:00 2001 From: Sam Blackshear Date: Thu, 30 Nov 2017 08:21:34 -0800 Subject: [PATCH] [dead stores] don't warn on dead stores of ScopeGuard's Reviewed By: da319 Differential Revision: D6139339 fbshipit-source-id: 8dee51f --- infer/src/checkers/liveness.ml | 35 +++++++++++++++---- .../cpp/liveness/dead_stores.cpp | 18 +++++++++- .../codetoanalyze/cpp/liveness/issues.exp | 2 +- 3 files changed, 46 insertions(+), 9 deletions(-) diff --git a/infer/src/checkers/liveness.ml b/infer/src/checkers/liveness.ml index a5f1afef9..3aa20a520 100644 --- a/infer/src/checkers/liveness.ml +++ b/infer/src/checkers/liveness.ml @@ -101,10 +101,32 @@ let checker {Callbacks.tenv; summary; proc_desc} : Specs.summary = | _ -> false in - let should_report pvar live_vars = + let matcher_scope_guard = + QualifiedCppName.Match.of_fuzzy_qual_names + [ "folly::RWSpinLock::ReadHolder" + ; "folly::RWSpinLock::WriteHolder" + ; "folly::SpinLockGuard" + ; "folly::ScopeGuard" + ; "std::lock_guard" + ; "std::scoped_lock" + ; "std::unique_lock" ] + in + (* It's fine to have a dead store on a type that uses the "scope guard" pattern. These types + are only read in their destructors, and this is expected/ok. + (e.g., https://github.com/facebook/folly/blob/master/folly/ScopeGuard.h). *) + let rec is_scope_guard = function + | {Typ.desc= Tstruct name} -> + QualifiedCppName.Match.match_qualifiers matcher_scope_guard (Typ.Name.qual_name name) + | {Typ.desc= Tptr (typ, _)} -> + is_scope_guard typ + | _ -> + false + in + let should_report pvar typ 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 ) + || Domain.mem (Var.of_pvar pvar) live_vars || Procdesc.is_captured_var proc_desc pvar + || is_scope_guard typ ) in let log_report pvar loc = let issue_id = IssueType.dead_store.unique_id in @@ -114,12 +136,12 @@ let checker {Callbacks.tenv; summary; proc_desc} : Specs.summary = Reporting.log_error summary ~loc ~ltr exn in let report_dead_store live_vars = function - | Sil.Store (Lvar pvar, _, rhs_exp, loc) - when should_report pvar live_vars && not (is_sentinel_exp rhs_exp) -> + | Sil.Store (Lvar pvar, typ, rhs_exp, loc) + when should_report pvar typ 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 -> + (None, Exp.Const Cfun (Typ.Procname.ObjC_Cpp _ as pname), (Exp.Lvar pvar, typ) :: _, loc, _) + when Typ.Procname.is_constructor pname && should_report pvar typ live_vars -> log_report pvar loc | _ -> () @@ -138,4 +160,3 @@ let checker {Callbacks.tenv; summary; proc_desc} : Specs.summary = in List.iter (CFG.nodes cfg) ~f:report_on_node ; summary - diff --git a/infer/tests/codetoanalyze/cpp/liveness/dead_stores.cpp b/infer/tests/codetoanalyze/cpp/liveness/dead_stores.cpp index 19f6eaefd..483fc8c01 100644 --- a/infer/tests/codetoanalyze/cpp/liveness/dead_stores.cpp +++ b/infer/tests/codetoanalyze/cpp/liveness/dead_stores.cpp @@ -7,7 +7,13 @@ * of patent rights can be found in the PATENTS file in the same directory. */ +#include #include +#include + +namespace folly { +class ScopeGuard {}; +} // namespace folly namespace dead_stores { @@ -243,7 +249,6 @@ void placement_new_ok(int len, int* ptr) { } // we don't report on dead stores where the RHS is 0, 0.0, false, nullptr, etc. - bool sentinel_bool_ok() { bool b = false; b = true; @@ -280,6 +285,11 @@ int* sentinel_ptr_ok(int* j) { return i; } +void scope_guard_ok() { + // realistically, it would be something like guard = folly::makeGuard(); + folly::ScopeGuard* guard = nullptr; +} + struct S { ~S() {} }; @@ -312,4 +322,10 @@ B& struct_rvalue_ref_used_ok() { return b; } +std::mutex my_mutex; + +void dead_lock_guard_ok() { std::lock_guard lock(my_mutex); } + +void dead_unique_lock_ok() { std::unique_lock lock(my_mutex); } + } diff --git a/infer/tests/codetoanalyze/cpp/liveness/issues.exp b/infer/tests/codetoanalyze/cpp/liveness/issues.exp index 3859fd733..3bf2ae202 100644 --- a/infer/tests/codetoanalyze/cpp/liveness/issues.exp +++ b/infer/tests/codetoanalyze/cpp/liveness/issues.exp @@ -6,7 +6,7 @@ codetoanalyze/cpp/liveness/dead_stores.cpp, dead_stores::dead_then_live_bad, 1, 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] codetoanalyze/cpp/liveness/dead_stores.cpp, dead_stores::init_capture_reassign_bad, 1, DEAD_STORE, [Write of unused value] -codetoanalyze/cpp/liveness/dead_stores.cpp, dead_stores::lambda_bad::lambda_dead_stores.cpp:147:11_operator(), 1, DEAD_STORE, [Write of unused value] +codetoanalyze/cpp/liveness/dead_stores.cpp, dead_stores::lambda_bad::lambda_dead_stores.cpp:153:11_operator(), 1, DEAD_STORE, [Write of unused value] codetoanalyze/cpp/liveness/dead_stores.cpp, dead_stores::plus_plus1_bad, 2, DEAD_STORE, [Write of unused value] codetoanalyze/cpp/liveness/dead_stores.cpp, dead_stores::plus_plus2_bad, 2, DEAD_STORE, [Write of unused value] codetoanalyze/cpp/liveness/dead_stores.cpp, dead_stores::plus_plus3_bad, 2, DEAD_STORE, [Write of unused value]