From 17da853fa81c82e604f53a53a71aee548df29200 Mon Sep 17 00:00:00 2001 From: Jeremy Dubreil Date: Thu, 2 Jun 2016 14:37:34 -0700 Subject: [PATCH] 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 | 19 +++-- .../expected_outputs/ant_report.json | 5 ++ .../expected_outputs/buck_report.json | 5 ++ .../infer/CloseableAsResourceExample.java | 14 ++++ .../java/infer/CloseableAsResourceTest.java | 3 +- infer/tests/utils/InferResults.java | 78 +++++++++---------- 6 files changed, 79 insertions(+), 45 deletions(-) diff --git a/infer/src/backend/symExec.ml b/infer/src/backend/symExec.ml index 526dfcd3d..cdd24e782 100644 --- a/infer/src/backend/symExec.ml +++ b/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 diff --git a/infer/tests/build_systems/expected_outputs/ant_report.json b/infer/tests/build_systems/expected_outputs/ant_report.json index 30b53189a..e8860417f 100644 --- a/infer/tests/build_systems/expected_outputs/ant_report.json +++ b/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", diff --git a/infer/tests/build_systems/expected_outputs/buck_report.json b/infer/tests/build_systems/expected_outputs/buck_report.json index e78ddf12d..4ff82a3c2 100644 --- a/infer/tests/build_systems/expected_outputs/buck_report.json +++ b/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", diff --git a/infer/tests/codetoanalyze/java/infer/CloseableAsResourceExample.java b/infer/tests/codetoanalyze/java/infer/CloseableAsResourceExample.java index 5db3b779a..402981be8 100644 --- a/infer/tests/codetoanalyze/java/infer/CloseableAsResourceExample.java +++ b/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); + } + } diff --git a/infer/tests/endtoend/java/infer/CloseableAsResourceTest.java b/infer/tests/endtoend/java/infer/CloseableAsResourceTest.java index 013f2a8dc..70efeb79d 100644 --- a/infer/tests/endtoend/java/infer/CloseableAsResourceTest.java +++ b/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, diff --git a/infer/tests/utils/InferResults.java b/infer/tests/utils/InferResults.java index d011ee5c4..9cc07be2f 100644 --- a/infer/tests/utils/InferResults.java +++ b/infer/tests/utils/InferResults.java @@ -45,47 +45,47 @@ public class InferResults { this.inferCmd = inferCmd; } - public void parseInferResultsFromString( - Pattern pattern, - String errorString) throws IOException, InferException { - - CSVReader reader = new CSVReader(new StringReader(errorString)); - List lines = reader.readAll(); - Path root = Paths.get(System.getProperty("user.dir")); - - for (String[] items : lines) { - String errorKind = items[1].trim(); - String errorType = items[2].trim(); - if (errorKind.equals("ERROR") || - errorType.equals("RETURN_VALUE_IGNORED") || - errorType.equals("ASSIGN_POINTER_WARNING") || - errorType.equals("STRONG_DELEGATE_WARNING") || - errorType.equals("DIRECT_ATOMIC_PROPERTY_ACCESS") || - errorType.equals("CXX_REFERENCE_CAPTURED_IN_OBJC_BLOCK") || - errorType.equals("REGISTERED_OBSERVER_BEING_DEALLOCATED") || - errorType.equals("GLOBAL_VARIABLE_INITIALIZED_WITH_FUNCTION_OR_METHOD_CALL") || - errorType.equals("IMMUTABLE_CAST") || - errorType.equals("PARAMETER_NOT_NULL_CHECKED") || - errorType.equals("DANGLING_POINTER_DEREFERENCE") || - errorType.equals("IVAR_NOT_NULL_CHECKED") || - errorType.startsWith("ERADICATE")) { - Integer errorLine = Integer.parseInt(items[5].trim()); - String procedure = items[6]; - Path path = Paths.get(items[8]); - if (path.isAbsolute()) { - path = root.relativize(Paths.get(items[8])); - } - Matcher methodMatcher = pattern.matcher(procedure); - boolean matching = methodMatcher.find(); - if (matching) { - procedure = methodMatcher.group(2); - if (procedure == null) { - throw new InferException("Unexpected method name structure."); + public void parseInferResultsFromString(Pattern pattern, String errorString) + throws IOException, InferException { + + try (CSVReader reader = new CSVReader(new StringReader(errorString))) { + List lines = reader.readAll(); + Path root = Paths.get(System.getProperty("user.dir")); + + for (String[] items : lines) { + String errorKind = items[1].trim(); + String errorType = items[2].trim(); + if (errorKind.equals("ERROR") || + errorType.equals("RETURN_VALUE_IGNORED") || + errorType.equals("ASSIGN_POINTER_WARNING") || + errorType.equals("STRONG_DELEGATE_WARNING") || + errorType.equals("DIRECT_ATOMIC_PROPERTY_ACCESS") || + errorType.equals("CXX_REFERENCE_CAPTURED_IN_OBJC_BLOCK") || + errorType.equals("REGISTERED_OBSERVER_BEING_DEALLOCATED") || + errorType.equals("GLOBAL_VARIABLE_INITIALIZED_WITH_FUNCTION_OR_METHOD_CALL") || + errorType.equals("IMMUTABLE_CAST") || + errorType.equals("PARAMETER_NOT_NULL_CHECKED") || + errorType.equals("DANGLING_POINTER_DEREFERENCE") || + errorType.equals("IVAR_NOT_NULL_CHECKED") || + errorType.startsWith("ERADICATE")) { + Integer errorLine = Integer.parseInt(items[5].trim()); + String procedure = items[6]; + Path path = Paths.get(items[8]); + if (path.isAbsolute()) { + path = root.relativize(Paths.get(items[8])); + } + Matcher methodMatcher = pattern.matcher(procedure); + boolean matching = methodMatcher.find(); + if (matching) { + procedure = methodMatcher.group(2); + if (procedure == null) { + 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); } } }