From 2cf3bfeea145d3761b8de4aff8a9973150a8a201 Mon Sep 17 00:00:00 2001 From: Peter O'Hearn Date: Thu, 9 Mar 2017 16:11:21 -0800 Subject: [PATCH] [infer][threadsafety] Stop Unprotected Write and Read/Write races from being reported on the same line Summary: When both an unprotected write and a read/write race emanate from the same line, undoubtedly because of interprocedurality, strip the read/write report (for now). Perhaps report the info in more succinct form later, but keep to one report/line. Reviewed By: sblackshear Differential Revision: D4685102 fbshipit-source-id: 291cf20 --- infer/src/checkers/ThreadSafety.ml | 23 +++++++++++++++---- .../java/threadsafety/DeDup.java | 18 +++++++++++++++ .../java/threadsafety/issues.exp | 2 ++ 3 files changed, 39 insertions(+), 4 deletions(-) diff --git a/infer/src/checkers/ThreadSafety.ml b/infer/src/checkers/ThreadSafety.ml index 36442eca5..8f660a33b 100644 --- a/infer/src/checkers/ThreadSafety.ml +++ b/infer/src/checkers/ThreadSafety.ml @@ -949,7 +949,18 @@ let de_dup trace = equal_locs in ThreadSafetyDomain.PathDomain.update_sinks trace de_duped_sinks_by_locs_and_accesses - +let strip_reads_that_have_co_located_write reads writes = + let set_of_read_sinks = ThreadSafetyDomain.PathDomain.sinks reads in + let set_of_write_sinks = ThreadSafetyDomain.PathDomain.sinks writes in + let stripped_read_sinks = + ThreadSafetyDomain.PathDomain.Sinks.filter + (fun sink -> not (ThreadSafetyDomain.PathDomain.Sinks.exists + (fun sink2 -> equal_locs sink sink2) + set_of_write_sinks + ) + ) + set_of_read_sinks in + ThreadSafetyDomain.PathDomain.update_sinks reads stripped_read_sinks (*A helper function used in the error reporting*) let pp_accesses_sink fmt ~is_write_access sink = @@ -1096,10 +1107,14 @@ let process_results_table file_env tab = accesses (AccessDomain.empty, AccessDomain.empty) in begin - report_thread_safety_violations - proc_env make_unprotected_write_description (get_possibly_unsafe_writes writes) tab; + let unsafe_writes = get_possibly_unsafe_writes writes in let unsafe_reads = get_possibly_unsafe_reads reads in - report_reads proc_env unsafe_reads tab + let stripped_unsafe_reads = strip_reads_that_have_co_located_write + unsafe_reads + unsafe_writes in + report_thread_safety_violations + proc_env make_unprotected_write_description unsafe_writes tab; + report_reads proc_env stripped_unsafe_reads tab end ) tab diff --git a/infer/tests/codetoanalyze/java/threadsafety/DeDup.java b/infer/tests/codetoanalyze/java/threadsafety/DeDup.java index a4abdad15..b72635e29 100644 --- a/infer/tests/codetoanalyze/java/threadsafety/DeDup.java +++ b/infer/tests/codetoanalyze/java/threadsafety/DeDup.java @@ -59,4 +59,22 @@ Integer fielda, fieldb; field = 33; } + Integer colocated_read, colocated_write; + + /*Should only report colocated write, not read, from readandwrite()*/ + void colocated_read_write(){ + read_and_write(); + } + + /*Should report*/ + void separate_write_to_colocated_read(){ + colocated_read= 88; + } + + private void read_and_write() { + Integer x = colocated_read; + colocated_write= 99; + } + + } diff --git a/infer/tests/codetoanalyze/java/threadsafety/issues.exp b/infer/tests/codetoanalyze/java/threadsafety/issues.exp index 70d1dc3c8..e5ce4d1f2 100644 --- a/infer/tests/codetoanalyze/java/threadsafety/issues.exp +++ b/infer/tests/codetoanalyze/java/threadsafety/issues.exp @@ -28,6 +28,8 @@ codetoanalyze/java/threadsafety/Containers.java, void Containers.mapPutAllBad(Ma codetoanalyze/java/threadsafety/Containers.java, void Containers.mapPutBad(String,String), 1, THREAD_SAFETY_VIOLATION, [access to container codetoanalyze.java.checkers.Containers.mMap] 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.colocated_read_write(), 1, THREAD_SAFETY_VIOLATION, [call to void DeDup.read_and_write(),access to codetoanalyze.java.checkers.DeDup.colocated_write] +codetoanalyze/java/threadsafety/DeDup.java, void DeDup.separate_write_to_colocated_read(), 1, THREAD_SAFETY_VIOLATION, [access to codetoanalyze.java.checkers.DeDup.colocated_read] 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]