Fixing Resource leak false positives assuming developers use raii.

Reviewed By: jvillard

Differential Revision: D3579581

fbshipit-source-id: 9c74d92

infer/src/IR/Sil.rei

@@ -397,6 +397,10 @@ let binop_invert: Binop.t => exp => exp => exp;
 
 let mem_kind_compare: mem_kind => mem_kind => int;
 
+let res_act_kind_compare: res_act_kind => res_act_kind => int;
+
+let resource_compare: resource => resource => int;
+
 let attribute_compare: attribute => attribute => int;
 
 let attribute_equal: attribute => attribute => bool;

infer/src/backend/interproc.ml

@@ -767,6 +767,15 @@ let extract_specs tenv pdesc pathset : Prop.normal Specs.spec list =
 let collect_postconditions wl tenv pdesc : Paths.PathSet.t * Specs.Visitedset.t =
   let pname = Cfg.Procdesc.get_proc_name pdesc in
   let pathset = collect_analysis_result wl pdesc in
+
+  (* Assuming C++ developers use RAII, remove resources from the constructor posts *)
+  let pathset = match pname with
+    | Procname.ObjC_Cpp _ ->
+        if (Procname.is_constructor pname) then
+          Paths.PathSet.map (Prop.remove_resource_attribute Sil.Racquire Sil.Rfile) pathset
+        else pathset
+    | _ -> pathset in
+
   L.d_strln
     ("#### [FUNCTION " ^ Procname.to_string pname ^ "] Analysis result ####");
   Propset.d Prop.prop_emp (Paths.PathSet.to_propset pathset);

infer/src/backend/prop.ml

@@ -1879,29 +1879,34 @@ let mark_vars_as_undefined prop vars_to_mark callee_pname ret_annots loc path_po
     | _ -> prop in
   IList.fold_left (fun prop id -> mark_var_as_undefined id prop) prop vars_to_mark
 
-(** Remove an attribute from all the atoms in the heap *)
-let remove_attribute att prop =
+let remove_attribute_by_filter ~f prop =
   let atom_remove atom pi = match atom with
-    | Sil.Aneq (_, Sil.Attribute att_old)
-    | Sil.Aneq (Sil.Attribute att_old, _) ->
-        if Sil.attribute_equal att_old att then
+    | Sil.Aneq (e, Sil.Attribute att_old)
+    | Sil.Aneq (Sil.Attribute att_old, e) ->
+        if f att_old e then
           pi
         else atom:: pi
     | _ -> atom:: pi in
   let pi' = IList.fold_right atom_remove (get_pi prop) [] in
   replace_pi pi' prop
 
+(** Remove an attribute from all the atoms in the heap *)
+let remove_attribute att =
+  let f att_old _ = Sil.attribute_equal att_old att in
+  remove_attribute_by_filter ~f
+
+let remove_resource_attribute ra_kind ra_res =
+  let f att_old _ = match att_old with
+    | Sil.Aresource res_action ->
+        Sil.res_act_kind_compare res_action.Sil.ra_kind ra_kind == 0
+        && Sil.resource_compare res_action.Sil.ra_res ra_res == 0
+    | _ -> false in
+  remove_attribute_by_filter ~f
+
 let remove_attribute_from_exp att prop exp =
   let nexp = exp_normalize_prop prop exp in
-  let atom_remove atom pi = match atom with
-    | Sil.Aneq (e, Sil.Attribute att_old)
-    | Sil.Aneq (Sil.Attribute att_old, e) ->
-        if Sil.attribute_equal att_old att && Sil.exp_equal nexp e then
-          pi
-        else atom:: pi
-    | _ -> atom:: pi in
-  let pi' = IList.fold_right atom_remove (get_pi prop) [] in
-  replace_pi pi' prop
+  let f att_old e = Sil.attribute_equal att_old att && Sil.exp_equal nexp e in
+  remove_attribute_by_filter ~f prop
 
 (* Replace an attribute OBJC_NULL($n1) with OBJC_NULL(var) when var = $n1, and also sets $n1 = 0 *)
 let replace_objc_null prop lhs_exp rhs_exp =

infer/src/backend/prop.mli

@@ -326,6 +326,8 @@ val mark_vars_as_undefined : normal t -> Sil.exp list -> Procname.t -> Typ.item_
 (** Remove an attribute from all the atoms in the heap *)
 val remove_attribute : Sil.attribute -> 'a t -> normal t
 
+val remove_resource_attribute : Sil.res_act_kind -> Sil.resource -> 'a t -> normal t
+
 (** [replace_objc_null lhs rhs].
     If rhs has the objc_null attribute, replace the attribute and set the lhs = 0 *)
 val replace_objc_null : normal t -> exp -> exp -> normal t

infer/tests/codetoanalyze/cpp/errors/resource_leaks/raii.cpp

@@ -0,0 +1,39 @@
+/*
+ * Copyright (c) 2016 - present Facebook, Inc.
+ * All rights reserved.
+ *
+ * This source code is licensed under the BSD style license found in the
+ * 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 <stdio.h>
+
+class OpenFile {
+ public:
+  OpenFile(const char* filename) { _filename = fopen(filename, "r"); }
+
+  ~OpenFile() { fclose(_filename); }
+
+ private:
+  FILE* _filename;
+};
+
+void resource_leak(const char* filename) {
+  char buffer[100];
+  FILE* file = fopen(filename, "r");
+
+  if (file == NULL)
+    return;
+  else
+    fgets(buffer, 100, file);
+
+  return;
+}
+
+void no_resource_leak(const char* filename) {
+  OpenFile f(filename);
+  return;
+}
+
+int main() {}

infer/tests/endtoend/cpp/infer/ResourceLeakRaiiTest.java

@@ -0,0 +1,62 @@
+/*
+ * Copyright (c) 2015 - present Facebook, Inc.
+ * All rights reserved.
+ *
+ * This source code is licensed under the BSD style license found in the
+ * 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.
+ */
+
+package endtoend.cpp.infer;
+
+import static org.hamcrest.MatcherAssert.assertThat;
+import static utils.matchers.ResultContainsExactly.containsExactly;
+
+import com.google.common.collect.ImmutableList;
+
+import org.junit.BeforeClass;
+import org.junit.ClassRule;
+import org.junit.Test;
+
+import java.io.IOException;
+
+import utils.DebuggableTemporaryFolder;
+import utils.InferException;
+import utils.InferResults;
+import utils.InferRunner;
+
+public class ResourceLeakRaiiTest {
+
+  public static final String FILE =
+      "infer/tests/codetoanalyze/cpp/errors/resource_leaks/raii.cpp";
+
+  private static ImmutableList<String> inferCmd;
+
+  public static final String RESOURCE_LEAK = "RESOURCE_LEAK";
+
+  @ClassRule
+  public static DebuggableTemporaryFolder folder =
+      new DebuggableTemporaryFolder();
+
+  @BeforeClass
+  public static void runInfer() throws InterruptedException, IOException {
+    inferCmd = InferRunner.createCPPInferCommandWithMLBuckets(folder, FILE, "cpp");
+  }
+
+  @Test
+  public void whenInferRunsOnObject_leakThenMLIsFound()
+      throws InterruptedException, IOException, InferException {
+    InferResults inferResults = InferRunner.runInferCPP(inferCmd);
+    String[] methods = { "resource_leak" };
+    assertThat(
+        "Results should contain " + RESOURCE_LEAK,
+        inferResults,
+        containsExactly(
+            RESOURCE_LEAK,
+            FILE,
+            methods
+        )
+    );
+  }
+
+}