[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

@@ -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
-

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 <mutex>
 #include <new>
+#include <thread>
+
+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<std::mutex> lock(my_mutex); }
+
+void dead_unique_lock_ok() { std::unique_lock<std::mutex> lock(my_mutex); }
+
 }

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]