From 859b816e953f4d97b53d8316b5cace8572647260 Mon Sep 17 00:00:00 2001 From: jrm Date: Mon, 2 Nov 2015 12:35:57 -0800 Subject: [PATCH] No longer report context leak on private methods Summary: public The context leaks were reported multiple times. If a leaks was found on method `f()` and `g()` calls `f()`, then the same leak was report both in `f()` and in `g()`. Reviewed By: sblackshear Differential Revision: D2598110 fb-gh-sync-id: ca90b57 --- infer/src/backend/cfg.ml | 5 + infer/src/backend/cfg.mli | 3 + infer/src/backend/interproc.ml | 2 +- infer/tests/ant_report.json | 2 +- infer/tests/buck_report.json | 2 +- .../java/infer/ContextLeaks.java | 161 +++++++++--------- .../endtoend/java/infer/ContextLeaksTest.java | 2 +- 7 files changed, 95 insertions(+), 82 deletions(-) diff --git a/infer/src/backend/cfg.ml b/infer/src/backend/cfg.ml index 9ac48a43c..b4278e8c9 100644 --- a/infer/src/backend/cfg.ml +++ b/infer/src/backend/cfg.ml @@ -460,6 +460,10 @@ module Node = struct let proc_desc_get_captured proc_desc = proc_desc.pd_attributes.ProcAttributes.captured + (** Return the visibility attribute *) + let proc_desc_get_access proc_desc = + proc_desc.pd_attributes.ProcAttributes.access + let proc_desc_get_nodes proc_desc = proc_desc.pd_nodes @@ -645,6 +649,7 @@ module Procdesc = struct let get_loc = Node.proc_desc_get_loc let get_locals = Node.proc_desc_get_locals let get_captured = Node.proc_desc_get_captured + let get_access = Node.proc_desc_get_access let get_nodes = Node.proc_desc_get_nodes let get_slope = Node.proc_desc_get_slope let get_sliced_slope = Node.proc_desc_get_sliced_slope diff --git a/infer/src/backend/cfg.mli b/infer/src/backend/cfg.mli index cd5ef02a8..c7cb05621 100644 --- a/infer/src/backend/cfg.mli +++ b/infer/src/backend/cfg.mli @@ -71,6 +71,9 @@ module Procdesc : sig (** Return name and type of block's captured variables *) val get_captured : t -> (Mangled.t * Sil.typ) list + (** Return the visibility attribute *) + val get_access : t -> Sil.access + val get_nodes : t -> node list (** Get the procedure's nodes up until the first branching *) diff --git a/infer/src/backend/interproc.ml b/infer/src/backend/interproc.ml index 5a9c0bc55..c2779df7c 100644 --- a/infer/src/backend/interproc.ml +++ b/infer/src/backend/interproc.ml @@ -702,7 +702,7 @@ let extract_specs tenv pdesc pathset : Prop.normal Specs.spec list = (* let () = L.out "@.AFTER abs:@.$%a@." (Prop.pp_prop Utils.pe_text) prop'' in *) let pre, post = Prop.extract_spec prop'' in let pre' = Prop.normalize (Prop.prop_sub sub pre) in - if !Config.curr_language = Config.Java then + if !Config.curr_language = Config.Java && Cfg.Procdesc.get_access pdesc <> Sil.Private then report_context_leaks pname (Prop.get_sigma post) tenv; let post' = if Prover.check_inconsistency_base prop then None diff --git a/infer/tests/ant_report.json b/infer/tests/ant_report.json index ea74a5d09..1cd292aae 100644 --- a/infer/tests/ant_report.json +++ b/infer/tests/ant_report.json @@ -28,7 +28,7 @@ }, { "type": "CONTEXT_LEAK", - "procedure": "void ContextLeaks.handlerLeak()", + "procedure": "void ContextLeaks.indirectHandlerLeak()", "file": "codetoanalyze/java/infer/ContextLeaks.java" }, { diff --git a/infer/tests/buck_report.json b/infer/tests/buck_report.json index 6e2b61632..9cfaafd22 100644 --- a/infer/tests/buck_report.json +++ b/infer/tests/buck_report.json @@ -26,7 +26,7 @@ }, { "type": "CONTEXT_LEAK", - "procedure": "void ContextLeaks.handlerLeak()", + "procedure": "void ContextLeaks.indirectHandlerLeak()", "file": "infer/tests/codetoanalyze/java/infer/ContextLeaks.java" }, { diff --git a/infer/tests/codetoanalyze/java/infer/ContextLeaks.java b/infer/tests/codetoanalyze/java/infer/ContextLeaks.java index 521254461..a5dc86b17 100644 --- a/infer/tests/codetoanalyze/java/infer/ContextLeaks.java +++ b/infer/tests/codetoanalyze/java/infer/ContextLeaks.java @@ -16,100 +16,105 @@ import android.os.Handler; public class ContextLeaks extends Activity { - static Object sFld; + static Object sFld; - public void directLeak() { - sFld = this; - } + void directLeak() { + sFld = this; + } - public void leakThenFix() { - sFld = this; - sFld = null; - } + public void leakThenFix() { + sFld = this; + sFld = null; + } - public void nonActivityNoLeak() { - sFld = new Object(); - } + public void nonActivityNoLeak() { + sFld = new Object(); + } - static class Obj { - public Object f; - } + static class Obj { + public Object f; + } - public void indirectLeak() { - Obj o = new Obj(); - o.f = this; - sFld = o; - } + public void indirectLeak() { + Obj o = new Obj(); + o.f = this; + sFld = o; + } - public void indirectLeakThenFix() { - Obj o = new Obj(); - o.f = this; - sFld = o; - o.f = null; - } + public void indirectLeakThenFix() { + Obj o = new Obj(); + o.f = this; + sFld = o; + o.f = null; + } - class NonStaticInner {} + class NonStaticInner { + } - public void nonStaticInnerClassLeak() { - sFld = new NonStaticInner(); - } + public void nonStaticInnerClassLeak() { + sFld = new NonStaticInner(); + } - public void nonStaticInnerClassLeakThenFix() { - sFld = new NonStaticInner(); - sFld = null; - } + public void nonStaticInnerClassLeakThenFix() { + sFld = new NonStaticInner(); + sFld = null; + } - private Object o; + private Object o; - public void leakAfterInstanceFieldWrite() { - this.o = new Object(); - sFld = this; - } + public void leakAfterInstanceFieldWrite() { + this.o = new Object(); + sFld = this; + } - public static class Singleton { - - private static Singleton instance; - private Context context; - - private Singleton(Context context) { - this.context = context; - } - - public static Singleton getInstance(Context context) { - if(instance == null) { - instance = new Singleton(context); - } - return instance; - } - } - - Singleton singletonLeak() { - return Singleton.getInstance(this); - } - - Singleton singletonNoLeak() { - return Singleton.getInstance(this.getApplicationContext()); - } + public static class Singleton { - private Handler handler = new Handler(); + private static Singleton instance; + private Context context; - public void handlerLeak() { - Runnable r = - new Runnable() { - public void run() { - } - }; - handler.postDelayed(r, 10000); + private Singleton(Context context) { + this.context = context; } - public void handlerNoLeak() { - Runnable r = - new Runnable() { - public void run() { - } - }; - handler.postDelayed(r, 10000); - handler.removeCallbacks(r); + public static Singleton getInstance(Context context) { + if (instance == null) { + instance = new Singleton(context); + } + return instance; } + } + + public Singleton singletonLeak() { + return Singleton.getInstance(this); + } + + public Singleton singletonNoLeak() { + return Singleton.getInstance(this.getApplicationContext()); + } + + private Handler handler = new Handler(); + + public void indirectHandlerLeak() { + handlerLeak(); + } + + private void handlerLeak() { + Runnable r = + new Runnable() { + public void run() { + } + }; + handler.postDelayed(r, 10000); + } + + public void handlerNoLeak() { + Runnable r = + new Runnable() { + public void run() { + } + }; + handler.postDelayed(r, 10000); + handler.removeCallbacks(r); + } } diff --git a/infer/tests/endtoend/java/infer/ContextLeaksTest.java b/infer/tests/endtoend/java/infer/ContextLeaksTest.java index c11be06fe..03ffdea81 100644 --- a/infer/tests/endtoend/java/infer/ContextLeaksTest.java +++ b/infer/tests/endtoend/java/infer/ContextLeaksTest.java @@ -45,7 +45,7 @@ public class ContextLeaksTest { "nonStaticInnerClassLeak", "leakAfterInstanceFieldWrite", "singletonLeak", - "handlerLeak" + "indirectHandlerLeak" }; assertThat( "Results should contain " + CONTEXT_LEAK,