[thread-safety] fix bad read/write race traces

Summary:
Read/write race errors should always show one trace for a read and one trace for a write.
We forget to pass the conflicting writes to the reporting function in one case, which prevented us from showing a well-formed trace.

Fixed it by making the `conflicts` parameter non-optional

Reviewed By: jberdine

Differential Revision: D5209332

fbshipit-source-id: 05da01a

infer/src/checkers/ThreadSafety.ml

@@ -1027,7 +1027,7 @@ let make_trace_with_conflicts conflicts original_path pdesc =
   | [] ->
       original_trace
 
-let report_thread_safety_violation tenv pdesc ~make_description ?(conflicts=[]) access =
+let report_thread_safety_violation tenv pdesc ~make_description ~conflicts access =
   let open ThreadSafetyDomain in
   let pname = Procdesc.get_proc_name pdesc in
   let report_one_path ((_, sinks) as path) =
@@ -1157,6 +1157,7 @@ let report_unsafe_accesses ~is_file_threadsafe aggregated_access_map =
                       tenv
                       pdesc
                       ~make_description:make_unprotected_write_description
+                      ~conflicts:[]
                       access;
                     update_reported access pname reported_acc
                   end
@@ -1220,6 +1221,7 @@ let report_unsafe_accesses ~is_file_threadsafe aggregated_access_map =
                 tenv
                 pdesc
                 ~make_description:(make_read_write_race_description conflicting_writes)
+                ~conflicts:(List.map ~f:(fun (access, _, _, _, _) -> access) conflicting_writes)
                 access;
               update_reported access pname reported_acc
             end in

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

@@ -1,4 +1,4 @@
-codetoanalyze/cpp/threadsafety/basics.cpp, basics::Basic_get2, 3, THREAD_SAFETY_VIOLATION, [access to `suspiciously_written`]
+codetoanalyze/cpp/threadsafety/basics.cpp, basics::Basic_get2, 3, THREAD_SAFETY_VIOLATION, [<Beginning of read trace>,access to `suspiciously_written`,<Beginning of write trace>,access to `suspiciously_written`]
 codetoanalyze/cpp/threadsafety/basics.cpp, basics::Basic_get3, 0, THREAD_SAFETY_VIOLATION, [<Beginning of read trace>,access to `not_guarded`,<Beginning of write trace>,access to `not_guarded`]
 codetoanalyze/cpp/threadsafety/basics.cpp, basics::Basic_get4, 0, THREAD_SAFETY_VIOLATION, [<Beginning of read trace>,access to `suspiciously_read`,<Beginning of write trace>,access to `suspiciously_read`]
 codetoanalyze/cpp/threadsafety/basics_with_mutex.cpp, basics::BasicsWithHeader_get1, 0, THREAD_SAFETY_VIOLATION, [<Beginning of read trace>,access to `field_1`,<Beginning of write trace>,access to `field_1`]

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

@@ -72,7 +72,7 @@ codetoanalyze/java/threadsafety/RaceWithMainThread.java, void RaceWithMainThread
 codetoanalyze/java/threadsafety/RaceWithMainThread.java, void RaceWithMainThread.conditional_isMainThread_ElseBranch_Bad(), 7, THREAD_SAFETY_VIOLATION, [access to `codetoanalyze.java.checkers.RaceWithMainThread.ff`]
 codetoanalyze/java/threadsafety/RaceWithMainThread.java, void RaceWithMainThread.conditional_isMainThread_Negation_Bad(), 3, THREAD_SAFETY_VIOLATION, [access to `codetoanalyze.java.checkers.RaceWithMainThread.ff`]
 codetoanalyze/java/threadsafety/RaceWithMainThread.java, void RaceWithMainThread.conditional_isUiThread_ElseBranch_Bad(), 7, THREAD_SAFETY_VIOLATION, [access to `codetoanalyze.java.checkers.RaceWithMainThread.ff`]
-codetoanalyze/java/threadsafety/RaceWithMainThread.java, void RaceWithMainThread.readProtectedUnthreadedBad(), 3, THREAD_SAFETY_VIOLATION, [access to `codetoanalyze.java.checkers.RaceWithMainThread.f`]
+codetoanalyze/java/threadsafety/RaceWithMainThread.java, void RaceWithMainThread.readProtectedUnthreadedBad(), 3, THREAD_SAFETY_VIOLATION, [<Beginning of read trace>,access to `codetoanalyze.java.checkers.RaceWithMainThread.f`,<Beginning of write trace>,access to `codetoanalyze.java.checkers.RaceWithMainThread.f`]
 codetoanalyze/java/threadsafety/RaceWithMainThread.java, void RaceWithMainThread.read_unprotected_unthreaded1_Bad(), 2, THREAD_SAFETY_VIOLATION, [<Beginning of read trace>,access to `codetoanalyze.java.checkers.RaceWithMainThread.f1`,<Beginning of write trace>,access to `codetoanalyze.java.checkers.RaceWithMainThread.f1`]
 codetoanalyze/java/threadsafety/RaceWithMainThread.java, void RaceWithMainThread.read_unprotected_unthreaded_Bad(), 2, THREAD_SAFETY_VIOLATION, [<Beginning of read trace>,access to `codetoanalyze.java.checkers.RaceWithMainThread.f`,<Beginning of write trace>,access to `codetoanalyze.java.checkers.RaceWithMainThread.f`]
 codetoanalyze/java/threadsafety/ReadWriteRaces.java, Object ReadWriteRaces.callUnprotecteReadInCallee(), 1, THREAD_SAFETY_VIOLATION, [<Beginning of read trace>,call to Object ReadWriteRaces.unprotectedReadInCallee(),access to `codetoanalyze.java.checkers.ReadWriteRaces.field1`,<Beginning of write trace>,access to `codetoanalyze.java.checkers.ReadWriteRaces.field1`]
@@ -97,8 +97,8 @@ codetoanalyze/java/threadsafety/ThreadSafeExample.java, void YesThreadSafeExtend
 codetoanalyze/java/threadsafety/ThreadSafeMethods.java, Object ThreadSafeMethods.readSameFieldAsThreadSafeMethod1Bad(), 1, THREAD_SAFETY_VIOLATION, [<Beginning of read trace>,access to `codetoanalyze.java.checkers.ThreadSafeMethods.field1`,<Beginning of write trace>,access to `codetoanalyze.java.checkers.ThreadSafeMethods.field1`]
 codetoanalyze/java/threadsafety/ThreadSafeMethods.java, Object ThreadSafeMethods.readSameFieldAsThreadSafeMethod2Bad(), 1, THREAD_SAFETY_VIOLATION, [<Beginning of read trace>,access to `codetoanalyze.java.checkers.ThreadSafeMethods.field4`,<Beginning of write trace>,access to `codetoanalyze.java.checkers.ThreadSafeMethods.field4`]
 codetoanalyze/java/threadsafety/ThreadSafeMethods.java, Object ThreadSafeMethods.readSameFieldAsThreadSafeMethod3Bad(), 1, THREAD_SAFETY_VIOLATION, [<Beginning of read trace>,access to `codetoanalyze.java.checkers.ThreadSafeMethods.field5`,<Beginning of write trace>,access to `codetoanalyze.java.checkers.ThreadSafeMethods.field5`]
-codetoanalyze/java/threadsafety/ThreadSafeMethods.java, Object ThreadSafeMethods.readSameFieldAsThreadSafeMethodWhileSynchronized1Bad(), 1, THREAD_SAFETY_VIOLATION, [access to `codetoanalyze.java.checkers.ThreadSafeMethods.field1`]
-codetoanalyze/java/threadsafety/ThreadSafeMethods.java, Object ThreadSafeMethods.synchronizedReadBad(), 1, THREAD_SAFETY_VIOLATION, [access to `codetoanalyze.java.checkers.ThreadSafeMethods.field5`]
+codetoanalyze/java/threadsafety/ThreadSafeMethods.java, Object ThreadSafeMethods.readSameFieldAsThreadSafeMethodWhileSynchronized1Bad(), 1, THREAD_SAFETY_VIOLATION, [<Beginning of read trace>,access to `codetoanalyze.java.checkers.ThreadSafeMethods.field1`,<Beginning of write trace>,access to `codetoanalyze.java.checkers.ThreadSafeMethods.field1`]
+codetoanalyze/java/threadsafety/ThreadSafeMethods.java, Object ThreadSafeMethods.synchronizedReadBad(), 1, THREAD_SAFETY_VIOLATION, [<Beginning of read trace>,access to `codetoanalyze.java.checkers.ThreadSafeMethods.field5`,<Beginning of write trace>,access to `codetoanalyze.java.checkers.ThreadSafeMethods.field5`]
 codetoanalyze/java/threadsafety/ThreadSafeMethods.java, Object ThreadSafeMethods.threadSafeMethodReadBad(), 1, THREAD_SAFETY_VIOLATION, [<Beginning of read trace>,access to `codetoanalyze.java.checkers.ThreadSafeMethods.field2`,<Beginning of write trace>,access to `codetoanalyze.java.checkers.ThreadSafeMethods.field2`]
 codetoanalyze/java/threadsafety/ThreadSafeMethods.java, Object ThreadSafeMethodsSubclass.readThreadSafeFieldOfOverrideBad(), 1, THREAD_SAFETY_VIOLATION, [<Beginning of read trace>,access to `codetoanalyze.java.checkers.ThreadSafeMethodsSubclass.subclassField`,<Beginning of write trace>,access to `codetoanalyze.java.checkers.ThreadSafeMethodsSubclass.subclassField`]
 codetoanalyze/java/threadsafety/ThreadSafeMethods.java, void ThreadSafeMethods.threadSafeMethodWriteBad(), 1, THREAD_SAFETY_VIOLATION, [access to `codetoanalyze.java.checkers.ThreadSafeMethods.field1`]