From 615489a6198ae5c0aec6aef9ec2f4b7394570544 Mon Sep 17 00:00:00 2001 From: Sam Blackshear Date: Fri, 31 Mar 2017 11:31:02 -0700 Subject: [PATCH] [thread-safety] don't report on protected reads Summary: *Unless* the unprotected write runs on the main thread and the read doesn't. Otherwise, we'll already report on the unprotected write, and we don't want to duplicate. Reviewed By: peterogithub Differential Revision: D4798357 fbshipit-source-id: 5de06a0 --- infer/src/checkers/ThreadSafety.ml | 10 ++++++---- .../java/threadsafety/ReadWriteRaces.java | 13 ------------- .../codetoanalyze/java/threadsafety/issues.exp | 2 -- 3 files changed, 6 insertions(+), 19 deletions(-) diff --git a/infer/src/checkers/ThreadSafety.ml b/infer/src/checkers/ThreadSafety.ml index 46a314b18..4701d16c1 100644 --- a/infer/src/checkers/ThreadSafety.ml +++ b/infer/src/checkers/ThreadSafety.ml @@ -1100,13 +1100,15 @@ let report_unsafe_accesses ~should_report aggregated_access_map = update_reported access pname reported_acc end | Access.Read, AccessPrecondition.Protected -> - (* protected read. report unprotected writes as conflicts *) + (* protected read. report unprotected, threaded writes as conflicts *) let unprotected_writes = List.filter - ~f:(fun (access, pre, _, _, _) -> + ~f:(fun (access, pre, other_threaded, _, _) -> match pre with - | AccessPrecondition.Unprotected _ -> TraceElem.is_write access - | _ -> false) + | AccessPrecondition.Unprotected _ -> + TraceElem.is_write access && (other_threaded && not threaded) + | _ -> + false) accesses in if List.is_empty unprotected_writes then diff --git a/infer/tests/codetoanalyze/java/threadsafety/ReadWriteRaces.java b/infer/tests/codetoanalyze/java/threadsafety/ReadWriteRaces.java index 36ded5208..41c6099b6 100644 --- a/infer/tests/codetoanalyze/java/threadsafety/ReadWriteRaces.java +++ b/infer/tests/codetoanalyze/java/threadsafety/ReadWriteRaces.java @@ -109,17 +109,4 @@ class ReadWriteRaces { } } - // write outside sync, read inside of sync races - - Object field4; - - public void unprotectedWrite4Bad() { - this.field4 = new Object(); - } - - public synchronized Object protectedRead4Bad() { - return this.field4; - } - - } diff --git a/infer/tests/codetoanalyze/java/threadsafety/issues.exp b/infer/tests/codetoanalyze/java/threadsafety/issues.exp index 4ded6abbd..7017e13d8 100644 --- a/infer/tests/codetoanalyze/java/threadsafety/issues.exp +++ b/infer/tests/codetoanalyze/java/threadsafety/issues.exp @@ -63,7 +63,6 @@ codetoanalyze/java/threadsafety/Ownership.java, void Ownership.writeToOwnedInCal 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.read_unprotected_unthreaded_Bad(), 2, THREAD_SAFETY_VIOLATION, [access to codetoanalyze.java.checkers.RaceWithMainThread.f] codetoanalyze/java/threadsafety/ReadWriteRaces.java, Object ReadWriteRaces.callUnprotecteReadInCallee(), 1, THREAD_SAFETY_VIOLATION, [call to Object ReadWriteRaces.unprotectedReadInCallee(),access to codetoanalyze.java.checkers.ReadWriteRaces.field1] -codetoanalyze/java/threadsafety/ReadWriteRaces.java, Object ReadWriteRaces.protectedRead4Bad(), 1, THREAD_SAFETY_VIOLATION, [access to codetoanalyze.java.checkers.ReadWriteRaces.field4] codetoanalyze/java/threadsafety/ReadWriteRaces.java, Object ReadWriteRaces.unprotectedRead1(), 1, THREAD_SAFETY_VIOLATION, [access to codetoanalyze.java.checkers.ReadWriteRaces.field1] codetoanalyze/java/threadsafety/ReadWriteRaces.java, Object ReadWriteRaces.unprotectedRead2(), 1, THREAD_SAFETY_VIOLATION, [access to codetoanalyze.java.checkers.ReadWriteRaces.field2] codetoanalyze/java/threadsafety/ReadWriteRaces.java, Object ReadWriteRaces.unprotectedRead3(), 1, THREAD_SAFETY_VIOLATION, [access to codetoanalyze.java.checkers.ReadWriteRaces.field3] @@ -71,7 +70,6 @@ codetoanalyze/java/threadsafety/ReadWriteRaces.java, void ReadWriteRaces.m1(), 2 codetoanalyze/java/threadsafety/ReadWriteRaces.java, void ReadWriteRaces.m2(), 1, THREAD_SAFETY_VIOLATION, [access to codetoanalyze.java.checkers.ReadWriteRaces.racy] codetoanalyze/java/threadsafety/ReadWriteRaces.java, void ReadWriteRaces.m3(), 1, THREAD_SAFETY_VIOLATION, [access to codetoanalyze.java.checkers.ReadWriteRaces.racy] codetoanalyze/java/threadsafety/ReadWriteRaces.java, void ReadWriteRaces.readInCalleeOutsideSyncBad(int), 1, THREAD_SAFETY_VIOLATION, [call to int C.get(),access to codetoanalyze.java.checkers.C.x] -codetoanalyze/java/threadsafety/ReadWriteRaces.java, void ReadWriteRaces.unprotectedWrite4Bad(), 1, THREAD_SAFETY_VIOLATION, [access to codetoanalyze.java.checkers.ReadWriteRaces.field4] codetoanalyze/java/threadsafety/ThreadSafeExample.java, void ExtendsThreadSafeExample.newmethodBad(), 1, THREAD_SAFETY_VIOLATION, [access to codetoanalyze.java.checkers.ExtendsThreadSafeExample.field] codetoanalyze/java/threadsafety/ThreadSafeExample.java, void ExtendsThreadSafeExample.tsOK(), 1, THREAD_SAFETY_VIOLATION, [access to codetoanalyze.java.checkers.ExtendsThreadSafeExample.field] codetoanalyze/java/threadsafety/ThreadSafeExample.java, void ThreadSafeExample.callPublicMethodBad(), 1, THREAD_SAFETY_VIOLATION, [call to void ThreadSafeExample.assignInPrivateMethodOk(),access to codetoanalyze.java.checkers.ThreadSafeExample.f]