Do not angelically remove the file attribute on the reciever for virtual calls

Reviewed By: peterogithub

Differential Revision: D3377851

fbshipit-source-id: 22c2ea1

infer/src/backend/symExec.ml

@@ -1384,7 +1384,7 @@ and check_untainted exp taint_kind caller_pname callee_pname prop =
 (** execute a call for an unknown or scan function *)
 and unknown_or_scan_call ~is_scan ret_type_option ret_annots
     { Builtin.tenv; pdesc; prop_= pre; path; ret_ids;
-      args= actual_pars; proc_name= callee_pname; loc; } =
+      args; proc_name= callee_pname; loc; instr; } =
   let remove_file_attribute prop =
     let do_exp p (e, _) =
       let do_attribute q = function
@@ -1393,7 +1393,13 @@ and unknown_or_scan_call ~is_scan ret_type_option ret_annots
             Prop.remove_attribute res q
         | _ -> q in
       IList.fold_left do_attribute p (Prop.get_exp_attributes p e) in
-    IList.fold_left do_exp prop actual_pars in
+    let filtered_args =
+      match args, instr with
+      | _:: other_args, Sil.Call (_, _, _, _, { Sil.cf_virtual }) when cf_virtual ->
+          (* Do not remove the file attribute on the reciver for virtual calls *)
+          other_args
+      | _ -> args in
+    IList.fold_left do_exp prop filtered_args in
   let add_tainted_pre prop actuals caller_pname callee_pname =
     if Config.taint_analysis then
       match Taint.accepts_sensitive_params callee_pname None with
@@ -1417,11 +1423,14 @@ and unknown_or_scan_call ~is_scan ret_type_option ret_annots
       (function
         | Sil.Lvar _, Sil.Tptr _ -> true
         | _ -> false)
-      actual_pars in
+      args in
   let has_nullable_annot = Annotations.ia_is_nullable ret_annots in
   let pre_final =
     (* in Java, assume that skip functions close resources passed as params *)
-    let pre_1 = if !Config.curr_language = Config.Java then remove_file_attribute pre else pre in
+    let pre_1 =
+      if Procname.is_java callee_pname
+      then remove_file_attribute pre
+      else pre in
     let pre_2 = match ret_ids, ret_type_option with
       | [ret_id], Some ret_typ ->
           add_constraints_on_retval
@@ -1430,7 +1439,7 @@ and unknown_or_scan_call ~is_scan ret_type_option ret_annots
           pre_1 in
     let pre_3 = add_constraints_on_actuals_by_ref tenv pre_2 actuals_by_ref callee_pname loc in
     let caller_pname = Cfg.Procdesc.get_proc_name pdesc in
-    add_tainted_pre pre_3 actual_pars caller_pname callee_pname in
+    add_tainted_pre pre_3 args caller_pname callee_pname in
   if is_scan (* if scan function, don't mark anything with undef attributes *)
   then [(Tabulation.remove_constant_string_class pre_final, path)]
   else

infer/tests/build_systems/expected_outputs/ant_report.json

@@ -759,6 +759,11 @@
     "file": "codetoanalyze/java/infer/AnalysisStops.java",
     "procedure": "void AnalysisStops.skipFunctionInLoopMayCauseFalseNegative()"
   },
+  {
+    "bug_type": "RESOURCE_LEAK",
+    "file": "codetoanalyze/java/infer/CloseableAsResourceExample.java",
+    "procedure": "void CloseableAsResourceExample.skippedVritualCallDoesNotCloseResourceOnReceiver()"
+  },
   {
     "bug_type": "RESOURCE_LEAK",
     "file": "codetoanalyze/java/infer/ResourceLeaks.java",

infer/tests/build_systems/expected_outputs/buck_report.json

@@ -759,6 +759,11 @@
     "file": "infer/tests/codetoanalyze/java/infer/AnalysisStops.java",
     "procedure": "void AnalysisStops.skipFunctionInLoopMayCauseFalseNegative()"
   },
+  {
+    "bug_type": "RESOURCE_LEAK",
+    "file": "infer/tests/codetoanalyze/java/infer/CloseableAsResourceExample.java",
+    "procedure": "void CloseableAsResourceExample.skippedVritualCallDoesNotCloseResourceOnReceiver()"
+  },
   {
     "bug_type": "RESOURCE_LEAK",
     "file": "infer/tests/codetoanalyze/java/infer/ResourceLeaks.java",

infer/tests/codetoanalyze/java/infer/CloseableAsResourceExample.java

@@ -28,6 +28,10 @@ class SomeResource implements Closeable {
   }
 
   public void close() {}
+
+  native void foo(int i);
+  native static void bar(SomeResource r);
+
 }
 
 class Resource implements Closeable {
@@ -159,4 +163,14 @@ public class CloseableAsResourceExample {
     MyResource res = new MyResource();
   }
 
+  void skippedCallClosesResourceOnArgs() {
+    SomeResource res = new SomeResource();
+    SomeResource.bar(res);
+  }
+
+  void skippedVritualCallDoesNotCloseResourceOnReceiver() {
+    SomeResource res = new SomeResource();
+    res.foo(42);
+  }
+
 }

infer/tests/endtoend/java/infer/CloseableAsResourceTest.java

@@ -44,9 +44,10 @@ public class CloseableAsResourceTest {
         "failToCloseWithCloseQuietly",
         "sourceOfNullWithResourceLeak",
         "leakFoundWhenIndirectlyImplementingCloseable",
+        "skippedVritualCallDoesNotCloseResourceOnReceiver",
     };
     assertThat(
-        "Results should not contain resource leak errors",
+        "Results should contain resource leak errors",
         inferResults,
         containsExactly(
             RESOURCE_LEAK,

infer/tests/utils/InferResults.java

@@ -45,47 +45,47 @@ public class InferResults {
     this.inferCmd = inferCmd;
   }
 
-  public void parseInferResultsFromString(
+  public void parseInferResultsFromString(Pattern pattern, String errorString)
-      Pattern pattern,
+    throws IOException, InferException {
-      String errorString) throws IOException, InferException {
+
-
+    try (CSVReader reader = new CSVReader(new StringReader(errorString))) {
-    CSVReader reader = new CSVReader(new StringReader(errorString));
+      List<String[]> lines = reader.readAll();
-    List<String[]> lines = reader.readAll();
+      Path root = Paths.get(System.getProperty("user.dir"));
-    Path root = Paths.get(System.getProperty("user.dir"));
+
-
+      for (String[] items : lines) {
-    for (String[] items : lines) {
+        String errorKind = items[1].trim();
-      String errorKind = items[1].trim();
+        String errorType = items[2].trim();
-      String errorType = items[2].trim();
+        if (errorKind.equals("ERROR") ||
-      if (errorKind.equals("ERROR") ||
+            errorType.equals("RETURN_VALUE_IGNORED") ||
-          errorType.equals("RETURN_VALUE_IGNORED") ||
+            errorType.equals("ASSIGN_POINTER_WARNING") ||
-          errorType.equals("ASSIGN_POINTER_WARNING") ||
+            errorType.equals("STRONG_DELEGATE_WARNING") ||
-          errorType.equals("STRONG_DELEGATE_WARNING") ||
+            errorType.equals("DIRECT_ATOMIC_PROPERTY_ACCESS") ||
-          errorType.equals("DIRECT_ATOMIC_PROPERTY_ACCESS") ||
+            errorType.equals("CXX_REFERENCE_CAPTURED_IN_OBJC_BLOCK") ||
-          errorType.equals("CXX_REFERENCE_CAPTURED_IN_OBJC_BLOCK") ||
+            errorType.equals("REGISTERED_OBSERVER_BEING_DEALLOCATED") ||
-          errorType.equals("REGISTERED_OBSERVER_BEING_DEALLOCATED") ||
+            errorType.equals("GLOBAL_VARIABLE_INITIALIZED_WITH_FUNCTION_OR_METHOD_CALL") ||
-          errorType.equals("GLOBAL_VARIABLE_INITIALIZED_WITH_FUNCTION_OR_METHOD_CALL") ||
+            errorType.equals("IMMUTABLE_CAST") ||
-          errorType.equals("IMMUTABLE_CAST") ||
+            errorType.equals("PARAMETER_NOT_NULL_CHECKED") ||
-          errorType.equals("PARAMETER_NOT_NULL_CHECKED") ||
+            errorType.equals("DANGLING_POINTER_DEREFERENCE") ||
-          errorType.equals("DANGLING_POINTER_DEREFERENCE") ||
+            errorType.equals("IVAR_NOT_NULL_CHECKED") ||
-          errorType.equals("IVAR_NOT_NULL_CHECKED") ||
+            errorType.startsWith("ERADICATE")) {
-          errorType.startsWith("ERADICATE")) {
+          Integer errorLine = Integer.parseInt(items[5].trim());
-        Integer errorLine = Integer.parseInt(items[5].trim());
+          String procedure = items[6];
-        String procedure = items[6];
+          Path path = Paths.get(items[8]);
-        Path path = Paths.get(items[8]);
+          if (path.isAbsolute()) {
-        if (path.isAbsolute()) {
+            path = root.relativize(Paths.get(items[8]));
-          path = root.relativize(Paths.get(items[8]));
+          }
-        }
+          Matcher methodMatcher = pattern.matcher(procedure);
-        Matcher methodMatcher = pattern.matcher(procedure);
+          boolean matching = methodMatcher.find();
-        boolean matching = methodMatcher.find();
+          if (matching) {
-        if (matching) {
+            procedure = methodMatcher.group(2);
-          procedure = methodMatcher.group(2);
+            if (procedure == null) {
-          if (procedure == null) {
+              throw new InferException("Unexpected method name structure.");
-            throw new InferException("Unexpected method name structure.");
+            }
           }
+          procedure = procedure.trim();
+          InferError error = new InferError(errorType, path, procedure, errorLine);
+          errors.add(error);
         }
-        procedure = procedure.trim();
-        InferError error = new InferError(errorType, path, procedure, errorLine);
-        errors.add(error);
       }
     }
   }