[infer][threadsafety] Dedup, chapter 2.

Summary:
Stop multiple reports per line happening. These come about
because of interprocedural access to multiple fields. Present one trace,
and summary information about other accesses.

Reviewed By: sblackshear

Differential Revision: D4636232

fbshipit-source-id: 9039fea

infer/src/checkers/ThreadSafety.ml

@@ -810,6 +810,11 @@ let combine_conditional_unconditional_writes conditional_writes unconditional_wr
     conditional_writes
     unconditional_writes
 
+let equal_locs (sink1 : ThreadSafetyDomain.TraceElem.t)
+    (sink2 : ThreadSafetyDomain.TraceElem.t) =
+  Location.equal
+    (CallSite.loc (ThreadSafetyDomain.TraceElem.call_site sink1))
+    (CallSite.loc (ThreadSafetyDomain.TraceElem.call_site sink2))
 
 let equal_accesses (sink1 : ThreadSafetyDomain.TraceElem.t)
     (sink2 : ThreadSafetyDomain.TraceElem.t) =
@@ -855,26 +860,35 @@ let collect_conflicting_writes sink tab =
       )
     procs_and_writes
 
-(* keep only the first copy of an access per procedure  *)
+(* keep only the first copy of an access per procedure,
+   and keep at most one warning per line (they are usually interprocedural accesses
+   to different fields generated by the same call)  *)
 let de_dup trace =
-  let original_sinks = ThreadSafetyDomain.PathDomain.sinks trace in
-  let list_of_original_sinks = ThreadSafetyDomain.PathDomain.Sinks.elements original_sinks in
-  let de_duped_sinks =
+  let select_representatives original_sinks predicate =
+    let list_of_original_sinks = ThreadSafetyDomain.PathDomain.Sinks.elements original_sinks in
     ThreadSafetyDomain.PathDomain.Sinks.filter
       (fun sink ->
          (* for each sink we will keep one in the equivalence class of those
-            with same access path. We select that by using find_exn to get
+            satisfying predicate. We select that by using find_exn to get
             the first element equivalent ot sink in a list of sinks. This
             first element is the dedup representative, and it happens to
             typically be the first such access in a method.  *)
          let first_sink =
            List.find_exn
-             ~f:(fun sink2 -> equal_accesses sink sink2)
+             ~f:(fun sink2 -> predicate sink sink2)
              list_of_original_sinks in
          Int.equal (ThreadSafetyDomain.TraceElem.compare sink first_sink) 0
       )
       original_sinks in
-  ThreadSafetyDomain.PathDomain.update_sinks trace de_duped_sinks
+  let de_duped_sinks_by_accesses = select_representatives
+      (ThreadSafetyDomain.PathDomain.sinks trace)
+      equal_accesses in
+  let de_duped_sinks_by_locs_and_accesses = select_representatives
+      de_duped_sinks_by_accesses
+      equal_locs in
+  ThreadSafetyDomain.PathDomain.update_sinks trace de_duped_sinks_by_locs_and_accesses
+
+
 
 (*A helper function used in the error reporting*)
 let pp_accesses_sink fmt ~is_write_access sink =

infer/tests/codetoanalyze/java/threadsafety/DeDup.java

@@ -15,6 +15,18 @@ import javax.annotation.concurrent.ThreadSafe;
 class DeDup{
 
 Integer field;
+Integer fielda, fieldb;
+
+  /* Only want one rather than two reports */
+  void two_fields(){
+    foo();
+  }
+
+  private void foo() {
+    fielda = 88;
+    fieldb = 99;
+   }
+
 
   /*Only the first write should be reported*/
   void two_writes(){

infer/tests/codetoanalyze/java/threadsafety/issues.exp

@@ -27,6 +27,7 @@ codetoanalyze/java/threadsafety/Containers.java, void Containers.mapPutBad(Strin
 codetoanalyze/java/threadsafety/Containers.java, void Containers.mapRemoveBad(String), 1, THREAD_SAFETY_VIOLATION, [access to container codetoanalyze.java.checkers.Containers.mMap]
 codetoanalyze/java/threadsafety/Containers.java, void Containers.mapSubclassWriteBad(HashMap,String), 1, THREAD_SAFETY_VIOLATION, [access to container ]
 codetoanalyze/java/threadsafety/DeDup.java, void DeDup.twoWritesOneInCaller(), 1, THREAD_SAFETY_VIOLATION, [call to void DeDup.writeToField(),access to codetoanalyze.java.checkers.DeDup.field]
+codetoanalyze/java/threadsafety/DeDup.java, void DeDup.two_fields(), 1, THREAD_SAFETY_VIOLATION, [call to void DeDup.foo(),access to codetoanalyze.java.checkers.DeDup.fielda]
 codetoanalyze/java/threadsafety/DeDup.java, void DeDup.two_reads(), 2, THREAD_SAFETY_VIOLATION, [access to codetoanalyze.java.checkers.DeDup.field]
 codetoanalyze/java/threadsafety/DeDup.java, void DeDup.two_writes(), 1, THREAD_SAFETY_VIOLATION, [access to codetoanalyze.java.checkers.DeDup.field]
 codetoanalyze/java/threadsafety/DeDup.java, void DeDup.write_read(), 2, THREAD_SAFETY_VIOLATION, [access to codetoanalyze.java.checkers.DeDup.field]