[traces] don't hang when unrolling a mutually recursive trace

Reviewed By: jberdine Differential Revision: D4272213 fbshipit-source-id: b8a0939
ryzn0518 · Dec 5, 2016 · 4b98543 · 4b98543
1 parent 731ffa0
commit 4b98543
Show file tree

Hide file tree

Showing 3 changed files with 25 additions and 8 deletions.
diff --git a/infer/src/checkers/Trace.ml b/infer/src/checkers/Trace.ml
@@ -91,28 +91,28 @@ end
 module Expander (TraceElem : TraceElem.S) = struct
 
   let expand elem0 ~elems_passthroughs_of_pname =
-    let rec expand_ elem elems_passthroughs_acc =
+    let rec expand_ elem (elems_passthroughs_acc, seen_acc) =
       let elem_site = TraceElem.call_site elem in
       let elem_kind = TraceElem.kind elem in
+      let seen_acc' = CallSite.Set.add elem_site seen_acc in
       let elems, passthroughs = elems_passthroughs_of_pname (CallSite.pname elem_site) in
-      let is_recursive elem_site callee_elem =
-        Procname.equal
-          (CallSite.pname elem_site) (CallSite.pname (TraceElem.call_site callee_elem)) in
+      let is_recursive callee_elem seen =
+        CallSite.Set.mem (TraceElem.call_site callee_elem) seen in
       (* find sinks that are the same kind as the caller, but have a different procname *)
       let matching_elems =
         IList.filter
           (fun callee_elem ->
              TraceElem.Kind.compare (TraceElem.kind callee_elem) elem_kind = 0 &&
-             not (is_recursive elem_site callee_elem))
+             not (is_recursive callee_elem seen_acc'))
           elems in
       match matching_elems with
       | callee_elem :: _ ->
           (* TODO: pick the shortest path to a sink here instead (t14242809) *)
           (* arbitrarily pick one elem and explore it further *)
-          expand_ callee_elem ((elem, passthroughs) :: elems_passthroughs_acc)
+          expand_ callee_elem ((elem, passthroughs) :: elems_passthroughs_acc, seen_acc')
       | _ ->
-          (elem, Passthrough.Set.empty) :: elems_passthroughs_acc in
-    expand_ elem0 []
+          (elem, Passthrough.Set.empty) :: elems_passthroughs_acc, seen_acc' in
+    fst (expand_ elem0 ([], CallSite.Set.empty))
 end
 
 module Make (Spec : Spec) = struct

diff --git a/infer/tests/codetoanalyze/java/threadsafety/ThreadSafeExample.java b/infer/tests/codetoanalyze/java/threadsafety/ThreadSafeExample.java
@@ -31,6 +31,20 @@ public void tsBad() {
     f = 24;
   }
 
+  public void recursiveBad() {
+    f = 44;
+    recursiveBad();
+  }
+
+  private void evenOk() {
+    f = 44;
+    oddBad();
+  }
+
+  public void oddBad() {
+    evenOk(); // should report here
+  }
+
   // shouldn't report here because it's a private method
   private void assignInPrivateMethodOk() {
     f = 24;

diff --git a/infer/tests/codetoanalyze/java/threadsafety/issues.exp b/infer/tests/codetoanalyze/java/threadsafety/issues.exp
@@ -7,4 +7,7 @@ codetoanalyze/java/threadsafety/ThreadSafeExample.java, void ExtendsThreadSafeEx
 codetoanalyze/java/threadsafety/ThreadSafeExample.java, void ExtendsThreadSafeExample.tsOK(), 1, THREAD_SAFETY_ERROR, [access to codetoanalyze.java.checkers.ExtendsThreadSafeExample.field]
 codetoanalyze/java/threadsafety/ThreadSafeExample.java, void ThreadSafeExample.callPublicMethodBad(), 1, THREAD_SAFETY_ERROR, [call to void ThreadSafeExample.assignInPrivateMethodOk(),access to codetoanalyze.java.checkers.ThreadSafeExample.f]
 codetoanalyze/java/threadsafety/ThreadSafeExample.java, void ThreadSafeExample.deeperTraceBad(), 1, THREAD_SAFETY_ERROR, [call to void ThreadSafeExample.callAssignInPrivateMethod(),call to void ThreadSafeExample.assignInPrivateMethodOk(),access to codetoanalyze.java.checkers.ThreadSafeExample.f]
+codetoanalyze/java/threadsafety/ThreadSafeExample.java, void ThreadSafeExample.oddBad(), 1, THREAD_SAFETY_ERROR, [call to void ThreadSafeExample.evenOk(),access to codetoanalyze.java.checkers.ThreadSafeExample.f]
+codetoanalyze/java/threadsafety/ThreadSafeExample.java, void ThreadSafeExample.recursiveBad(), 1, THREAD_SAFETY_ERROR, [access to codetoanalyze.java.checkers.ThreadSafeExample.f]
+codetoanalyze/java/threadsafety/ThreadSafeExample.java, void ThreadSafeExample.recursiveBad(), 2, THREAD_SAFETY_ERROR, [call to void ThreadSafeExample.recursiveBad(),access to codetoanalyze.java.checkers.ThreadSafeExample.f]
 codetoanalyze/java/threadsafety/ThreadSafeExample.java, void ThreadSafeExample.tsBad(), 1, THREAD_SAFETY_ERROR, [access to codetoanalyze.java.checkers.ThreadSafeExample.f]