[racerd] report only writes for GuardedBy

Reviewed By: mbouaziz

Differential Revision: D14594263

fbshipit-source-id: a8cab1765

infer/src/concurrency/RacerD.ml

@@ -939,6 +939,7 @@ let should_report_on_proc tenv procdesc =
 let should_report_guardedby_violation classname_str ({snapshot; tenv; procdesc} : reported_access)
     =
   (not snapshot.lock)
+  && RacerDDomain.TraceElem.is_write snapshot.access
   && Procdesc.get_proc_name procdesc |> Typ.Procname.is_java
   &&
   (* restrict check to access paths of length one *)

infer/src/concurrency/RacerDDomain.mli

@@ -31,6 +31,7 @@ module TraceElem : sig
   include ExplicitTrace.TraceElem with type elem_t = Access.t
 
   val is_write : t -> bool
+  (** is it a write OR a container write *)
 
   val is_container_write : t -> bool
 

infer/tests/build_systems/ant/issues.exp

@@ -55,47 +55,40 @@ codetoanalyze/java/infer/GuardedByExample.java, codetoanalyze.java.infer.Guarded
 codetoanalyze/java/infer/GuardedByExample.java, codetoanalyze.java.infer.GuardedByExample$Sub.badSub():void, 1, UNSAFE_GUARDED_BY_ACCESS, no_bucket, ERROR, [start of procedure badSub()]
 codetoanalyze/java/infer/GuardedByExample.java, codetoanalyze.java.infer.GuardedByExample$Sub.badSub():void, 491, GUARDEDBY_VIOLATION, no_bucket, ERROR, [access to `this.xForSub`]
 codetoanalyze/java/infer/GuardedByExample.java, codetoanalyze.java.infer.GuardedByExample.accessUnrecognizedGuardedByFieldsOk():void, 307, GUARDEDBY_VIOLATION, no_bucket, ERROR, [access to `this.uiThread1`]
-codetoanalyze/java/infer/GuardedByExample.java, codetoanalyze.java.infer.GuardedByExample.accessUnrecognizedGuardedByFieldsOk():void, 308, GUARDEDBY_VIOLATION, no_bucket, ERROR, [access to `this.uiThread1`]
 codetoanalyze/java/infer/GuardedByExample.java, codetoanalyze.java.infer.GuardedByExample.accessUnrecognizedGuardedByFieldsOk():void, 309, GUARDEDBY_VIOLATION, no_bucket, ERROR, [access to `this.uiThread2`]
-codetoanalyze/java/infer/GuardedByExample.java, codetoanalyze.java.infer.GuardedByExample.accessUnrecognizedGuardedByFieldsOk():void, 310, GUARDEDBY_VIOLATION, no_bucket, ERROR, [access to `this.uiThread2`]
 codetoanalyze/java/infer/GuardedByExample.java, codetoanalyze.java.infer.GuardedByExample.accessUnrecognizedGuardedByFieldsOk():void, 311, GUARDEDBY_VIOLATION, no_bucket, ERROR, [access to `this.uiThread3`]
-codetoanalyze/java/infer/GuardedByExample.java, codetoanalyze.java.infer.GuardedByExample.accessUnrecognizedGuardedByFieldsOk():void, 312, GUARDEDBY_VIOLATION, no_bucket, ERROR, [access to `this.uiThread3`]
 codetoanalyze/java/infer/GuardedByExample.java, codetoanalyze.java.infer.GuardedByExample.accessUnrecognizedGuardedByFieldsOk():void, 313, GUARDEDBY_VIOLATION, no_bucket, ERROR, [access to `this.nonExpression`]
-codetoanalyze/java/infer/GuardedByExample.java, codetoanalyze.java.infer.GuardedByExample.accessUnrecognizedGuardedByFieldsOk():void, 314, GUARDEDBY_VIOLATION, no_bucket, ERROR, [access to `this.nonExpression`]
 codetoanalyze/java/infer/GuardedByExample.java, codetoanalyze.java.infer.GuardedByExample.badGuardedByNormalLock():void, 1, UNSAFE_GUARDED_BY_ACCESS, no_bucket, ERROR, [start of procedure badGuardedByNormalLock()]
 codetoanalyze/java/infer/GuardedByExample.java, codetoanalyze.java.infer.GuardedByExample.badGuardedByNormalLock():void, 526, GUARDEDBY_VIOLATION, no_bucket, ERROR, [access to `this.guardedbynl`]
 codetoanalyze/java/infer/GuardedByExample.java, codetoanalyze.java.infer.GuardedByExample.badGuardedByReentrantLock():void, 1, UNSAFE_GUARDED_BY_ACCESS, no_bucket, ERROR, [start of procedure badGuardedByReentrantLock()]
 codetoanalyze/java/infer/GuardedByExample.java, codetoanalyze.java.infer.GuardedByExample.badGuardedByReentrantLock():void, 530, GUARDEDBY_VIOLATION, no_bucket, ERROR, [access to `this.guardedbyrel`]
 codetoanalyze/java/infer/GuardedByExample.java, codetoanalyze.java.infer.GuardedByExample.byRefTrickyBad():java.lang.Object, 5, UNSAFE_GUARDED_BY_ACCESS, no_bucket, ERROR, [start of procedure byRefTrickyBad()]
-codetoanalyze/java/infer/GuardedByExample.java, codetoanalyze.java.infer.GuardedByExample.byRefTrickyBad():java.lang.Object, 281, GUARDEDBY_VIOLATION, no_bucket, ERROR, [access to `this.g`]
+codetoanalyze/java/infer/GuardedByExample.java, codetoanalyze.java.infer.GuardedByExample.byRefTrickyBad():java.lang.Object, 281, THREAD_SAFETY_VIOLATION, no_bucket, ERROR, [<Read trace>,access to `this.g`,<Write trace>,access to `this.g`]
-codetoanalyze/java/infer/GuardedByExample.java, codetoanalyze.java.infer.GuardedByExample.guardedBySelfReferenceOK():void, 409, GUARDEDBY_VIOLATION, no_bucket, ERROR, [access to `this.self_reference`]
 codetoanalyze/java/infer/GuardedByExample.java, codetoanalyze.java.infer.GuardedByExample.guardedByTypeSyntaxBad():void, 1, UNSAFE_GUARDED_BY_ACCESS, no_bucket, ERROR, [start of procedure guardedByTypeSyntaxBad()]
 codetoanalyze/java/infer/GuardedByExample.java, codetoanalyze.java.infer.GuardedByExample.guardedByTypeSyntaxBad():void, 2, UNSAFE_GUARDED_BY_ACCESS, no_bucket, ERROR, [start of procedure guardedByTypeSyntaxBad()]
 codetoanalyze/java/infer/GuardedByExample.java, codetoanalyze.java.infer.GuardedByExample.guardedByTypeSyntaxBad():void, 573, GUARDEDBY_VIOLATION, no_bucket, ERROR, [access to `this.guardedByLock1`]
 codetoanalyze/java/infer/GuardedByExample.java, codetoanalyze.java.infer.GuardedByExample.guardedByTypeSyntaxBad():void, 574, GUARDEDBY_VIOLATION, no_bucket, ERROR, [access to `this.guardedByLock2`]
-codetoanalyze/java/infer/GuardedByExample.java, codetoanalyze.java.infer.GuardedByExample.itselfOK():void, 425, GUARDEDBY_VIOLATION, no_bucket, ERROR, [access to `this.itself_fld`]
 codetoanalyze/java/infer/GuardedByExample.java, codetoanalyze.java.infer.GuardedByExample.readFAfterBlockBad():void, 3, UNSAFE_GUARDED_BY_ACCESS, no_bucket, ERROR, [start of procedure readFAfterBlockBad()]
-codetoanalyze/java/infer/GuardedByExample.java, codetoanalyze.java.infer.GuardedByExample.readFAfterBlockBad():void, 98, GUARDEDBY_VIOLATION, no_bucket, ERROR, [access to `this.f`]
+codetoanalyze/java/infer/GuardedByExample.java, codetoanalyze.java.infer.GuardedByExample.readFAfterBlockBad():void, 98, THREAD_SAFETY_VIOLATION, no_bucket, ERROR, [<Read trace>,access to `this.f`,<Write trace>,access to `this.f`]
 codetoanalyze/java/infer/GuardedByExample.java, codetoanalyze.java.infer.GuardedByExample.readFBad():void, 1, UNSAFE_GUARDED_BY_ACCESS, no_bucket, ERROR, [start of procedure readFBad()]
-codetoanalyze/java/infer/GuardedByExample.java, codetoanalyze.java.infer.GuardedByExample.readFBad():void, 66, GUARDEDBY_VIOLATION, no_bucket, ERROR, [access to `this.f`]
+codetoanalyze/java/infer/GuardedByExample.java, codetoanalyze.java.infer.GuardedByExample.readFBad():void, 66, THREAD_SAFETY_VIOLATION, no_bucket, ERROR, [<Read trace>,access to `this.f`,<Write trace>,access to `this.f`]
-codetoanalyze/java/infer/GuardedByExample.java, codetoanalyze.java.infer.GuardedByExample.readFBadButSuppressed():void, 71, GUARDEDBY_VIOLATION, no_bucket, ERROR, [access to `this.f`]
+codetoanalyze/java/infer/GuardedByExample.java, codetoanalyze.java.infer.GuardedByExample.readFBadButSuppressed():void, 71, THREAD_SAFETY_VIOLATION, no_bucket, ERROR, [<Read trace>,access to `this.f`,<Write trace>,access to `this.f`]
-codetoanalyze/java/infer/GuardedByExample.java, codetoanalyze.java.infer.GuardedByExample.readFBadButSuppressedOther():void, 76, GUARDEDBY_VIOLATION, no_bucket, ERROR, [access to `this.f`]
+codetoanalyze/java/infer/GuardedByExample.java, codetoanalyze.java.infer.GuardedByExample.readFBadButSuppressedOther():void, 76, THREAD_SAFETY_VIOLATION, no_bucket, ERROR, [<Read trace>,access to `this.f`,<Write trace>,access to `this.f`]
 codetoanalyze/java/infer/GuardedByExample.java, codetoanalyze.java.infer.GuardedByExample.readFBadWrongAnnotation():void, 1, UNSAFE_GUARDED_BY_ACCESS, no_bucket, ERROR, [start of procedure readFBadWrongAnnotation()]
-codetoanalyze/java/infer/GuardedByExample.java, codetoanalyze.java.infer.GuardedByExample.readFBadWrongAnnotation():void, 109, GUARDEDBY_VIOLATION, no_bucket, ERROR, [access to `this.f`]
+codetoanalyze/java/infer/GuardedByExample.java, codetoanalyze.java.infer.GuardedByExample.readFBadWrongAnnotation():void, 109, THREAD_SAFETY_VIOLATION, no_bucket, ERROR, [<Read trace>,access to `this.f`,<Write trace>,access to `this.f`]
 codetoanalyze/java/infer/GuardedByExample.java, codetoanalyze.java.infer.GuardedByExample.readFBadWrongLock():void, 2, UNSAFE_GUARDED_BY_ACCESS, no_bucket, ERROR, [start of procedure readFBadWrongLock()]
 codetoanalyze/java/infer/GuardedByExample.java, codetoanalyze.java.infer.GuardedByExample.readFBadWrongLock():void, 85, THREAD_SAFETY_VIOLATION, no_bucket, ERROR, [<Read trace>,access to `this.f`,<Write trace>,access to `this.f`]
-codetoanalyze/java/infer/GuardedByExample.java, codetoanalyze.java.infer.GuardedByExample.readFOkMethodAnnotated():void, 114, GUARDEDBY_VIOLATION, no_bucket, ERROR, [access to `this.f`]
+codetoanalyze/java/infer/GuardedByExample.java, codetoanalyze.java.infer.GuardedByExample.readFOkMethodAnnotated():void, 114, THREAD_SAFETY_VIOLATION, no_bucket, ERROR, [<Read trace>,access to `this.f`,<Write trace>,access to `this.f`]
 codetoanalyze/java/infer/GuardedByExample.java, codetoanalyze.java.infer.GuardedByExample.readFOkSynchronized():void, 127, THREAD_SAFETY_VIOLATION, no_bucket, ERROR, [<Read trace>,access to `this.f`,<Write trace>,access to `this.f`]
 codetoanalyze/java/infer/GuardedByExample.java, codetoanalyze.java.infer.GuardedByExample.readGFromCopyOk():void, 267, THREAD_SAFETY_VIOLATION, no_bucket, ERROR, [<Read trace>,access to `this.mCopyOfG`,<Write trace>,access to `this.mCopyOfG`]
 codetoanalyze/java/infer/GuardedByExample.java, codetoanalyze.java.infer.GuardedByExample.readHBad():void, 2, UNSAFE_GUARDED_BY_ACCESS, no_bucket, ERROR, [start of procedure readHBad()]
 codetoanalyze/java/infer/GuardedByExample.java, codetoanalyze.java.infer.GuardedByExample.readHBadSynchronizedMethodShouldntHelp():void, 1, UNSAFE_GUARDED_BY_ACCESS, no_bucket, ERROR, [start of procedure readHBadSynchronizedMethodShouldntHelp()]
-codetoanalyze/java/infer/GuardedByExample.java, codetoanalyze.java.infer.GuardedByExample.readWriteLockOk():void, 189, GUARDEDBY_VIOLATION, no_bucket, ERROR, [access to `this.i`]
 codetoanalyze/java/infer/GuardedByExample.java, codetoanalyze.java.infer.GuardedByExample.reassignCopyOk():void, 149, THREAD_SAFETY_VIOLATION, no_bucket, ERROR, [access to `this.mCopyOfG`]
 codetoanalyze/java/infer/GuardedByExample.java, codetoanalyze.java.infer.GuardedByExample.synchronizedMethodReadBad():void, 1, UNSAFE_GUARDED_BY_ACCESS, no_bucket, ERROR, [start of procedure synchronizedMethodReadBad()]
 codetoanalyze/java/infer/GuardedByExample.java, codetoanalyze.java.infer.GuardedByExample.synchronizedMethodReadBad():void, 138, THREAD_SAFETY_VIOLATION, no_bucket, ERROR, [<Read trace>,access to `this.f`,<Write trace>,access to `this.f`]
 codetoanalyze/java/infer/GuardedByExample.java, codetoanalyze.java.infer.GuardedByExample.synchronizedMethodWriteBad():void, 1, UNSAFE_GUARDED_BY_ACCESS, no_bucket, ERROR, [start of procedure synchronizedMethodWriteBad()]
 codetoanalyze/java/infer/GuardedByExample.java, codetoanalyze.java.infer.GuardedByExample.synchronizedOnThisBad():void, 1, UNSAFE_GUARDED_BY_ACCESS, no_bucket, ERROR, [start of procedure synchronizedOnThisBad()]
-codetoanalyze/java/infer/GuardedByExample.java, codetoanalyze.java.infer.GuardedByExample.synchronizedOnThisBad():void, 205, GUARDEDBY_VIOLATION, no_bucket, ERROR, [access to `infer.GuardedByExample.sGuardedByClass`]
+codetoanalyze/java/infer/GuardedByExample.java, codetoanalyze.java.infer.GuardedByExample.synchronizedOnThisBad():void, 205, THREAD_SAFETY_VIOLATION, no_bucket, ERROR, [<Read trace>,access to `infer.GuardedByExample.sGuardedByClass`,<Write trace>,access to `infer.GuardedByExample.sGuardedByClass`]
 codetoanalyze/java/infer/GuardedByExample.java, codetoanalyze.java.infer.GuardedByExample.writeFAfterBlockBad():void, 3, UNSAFE_GUARDED_BY_ACCESS, no_bucket, ERROR, [start of procedure writeFAfterBlockBad()]
 codetoanalyze/java/infer/GuardedByExample.java, codetoanalyze.java.infer.GuardedByExample.writeFAfterBlockBad():void, 104, GUARDEDBY_VIOLATION, no_bucket, ERROR, [access to `this.f`]
 codetoanalyze/java/infer/GuardedByExample.java, codetoanalyze.java.infer.GuardedByExample.writeFBad():void, 1, UNSAFE_GUARDED_BY_ACCESS, no_bucket, ERROR, [start of procedure writeFBad()]

infer/tests/codetoanalyze/java/racerd/GuardedByTests.java

@@ -15,58 +15,77 @@ public class GuardedByTests {
   private Object mlock = new Object();
 
   @GuardedBy("mLock")
-  private int f;
+  private int a;
 
   public GuardedByTests() {
     // don't warn on reads or writes of Guarded fields in constructor
-    f = 0;
+    a = 0;
-  }
-
-  public void unlockedWriteBad() {
-    f = 0;
   }
 
   public void lockedWriteOk() {
     synchronized (mlock) {
-      f = 0;
+      a = 0;
     }
   }
 
-  public int unlockedReadBad() {
+  @GuardedBy("mLock")
-    return f;
+  private int b;
+
+  public void unlockedWriteBad() {
+    b = 0;
+  }
+
+  @GuardedBy("mLock")
+  private int c;
+
+  public int unlockedReadOk() {
+    return c;
   }
 
   public int lockedReadOk() {
     synchronized (mlock) {
-      return f;
+      return c;
     }
   }
 
+  @GuardedBy("mLock")
+  private int d;
+
   private void privateUnlockedWriteOk() {
-    f = 0;
+    d = 0;
   }
 
-  private int privateUnlockedReadOk() {
-    return f;
-  }
 
   public void interprocUnlockedWriteBad() {
     privateUnlockedWriteOk();
   }
 
-  public int interprocUnlockedReadBad() {
+  @GuardedBy("mLock")
+  private int e;
+
+  private int privateUnlockedReadOk() {
+    return e;
+  }
+
+  public int interprocUnlockedReadOk() {
     return privateUnlockedReadOk();
   }
 
+  @GuardedBy("mLock")
+  private int f;
+
   // NB ThreadSafe annotation disables GuardedBy check too
   @ThreadSafe(enableChecks = false)
-  int suppressedRead() {
+  void suppressedWrite() {
-    return f;
+    f = 0;
   }
 
+  @GuardedBy("mLock")
+  private int h;
+
   @VisibleForTesting
   public void visibleForTestingOk() {
-    f = 0;
+    h = 0;
   }
 
   static Object slock = new Object();
@@ -80,14 +99,14 @@ public class GuardedByTests {
   }
 
   @GuardedBy("this")
-  int g;
+  int i;
 
   synchronized void syncWriteOk() {
-    g = 5;
+    i = 5;
   }
 
   synchronized int syncReadOk() {
-    return g;
+    return i;
   }
 
   GuardedByOther o;

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

@@ -51,15 +51,9 @@ codetoanalyze/java/racerd/DeepOwnership.java, DeepOwnership.reassignBaseToGlobal
 codetoanalyze/java/racerd/Dispatch.java, codetoanalyze.java.checkers.Dispatch.callUnannotatedInterfaceBad(codetoanalyze.java.checkers.UnannotatedInterface):void, 49, INTERFACE_NOT_THREAD_SAFE, no_bucket, ERROR, [Call to un-annotated interface method void UnannotatedInterface.foo()]
 codetoanalyze/java/racerd/Dispatch.java, codetoanalyze.java.checkers.Dispatch.callUnannotatedInterfaceIndirectBad(codetoanalyze.java.checkers.NotThreadSafe,codetoanalyze.java.checkers.UnannotatedInterface):void, 53, INTERFACE_NOT_THREAD_SAFE, no_bucket, ERROR, [call to void NotThreadSafe.notThreadSafeOk(UnannotatedInterface),Call to un-annotated interface method void UnannotatedInterface.foo()]
 codetoanalyze/java/racerd/Dispatch.java, codetoanalyze.java.checkers.ThreadConfinedField.interfaceCallOnNormalFieldBad():void, 102, INTERFACE_NOT_THREAD_SAFE, no_bucket, ERROR, [Call to un-annotated interface method void UnannotatedInterface.foo()]
-codetoanalyze/java/racerd/GuardedByTests.java, codetoanalyze.java.infer.GuardedByOther.accessBad():void, 109, GUARDEDBY_VIOLATION, no_bucket, ERROR, [access to `this.x`]
+codetoanalyze/java/racerd/GuardedByTests.java, codetoanalyze.java.infer.GuardedByOther.accessBad():void, 128, GUARDEDBY_VIOLATION, no_bucket, ERROR, [access to `this.x`]
-codetoanalyze/java/racerd/GuardedByTests.java, codetoanalyze.java.infer.GuardedByTests.interprocUnlockedReadBad():int, 58, GUARDEDBY_VIOLATION, no_bucket, ERROR, [call to int GuardedByTests.privateUnlockedReadOk(),access to `this.f`]
+codetoanalyze/java/racerd/GuardedByTests.java, codetoanalyze.java.infer.GuardedByTests.interprocUnlockedWriteBad():void, 60, GUARDEDBY_VIOLATION, no_bucket, ERROR, [call to void GuardedByTests.privateUnlockedWriteOk(),access to `this.d`]
-codetoanalyze/java/racerd/GuardedByTests.java, codetoanalyze.java.infer.GuardedByTests.interprocUnlockedReadBad():int, 58, THREAD_SAFETY_VIOLATION, no_bucket, ERROR, [<Read trace>,call to int GuardedByTests.privateUnlockedReadOk(),access to `this.f`,<Write trace>,access to `this.f`]
+codetoanalyze/java/racerd/GuardedByTests.java, codetoanalyze.java.infer.GuardedByTests.unlockedWriteBad():void, 35, GUARDEDBY_VIOLATION, no_bucket, ERROR, [access to `this.b`]
-codetoanalyze/java/racerd/GuardedByTests.java, codetoanalyze.java.infer.GuardedByTests.interprocUnlockedWriteBad():void, 54, GUARDEDBY_VIOLATION, no_bucket, ERROR, [call to void GuardedByTests.privateUnlockedWriteOk(),access to `this.f`]
-codetoanalyze/java/racerd/GuardedByTests.java, codetoanalyze.java.infer.GuardedByTests.interprocUnlockedWriteBad():void, 54, THREAD_SAFETY_VIOLATION, no_bucket, ERROR, [<Write on unknown thread>,call to void GuardedByTests.privateUnlockedWriteOk(),access to `this.f`,<Write on background thread>,access to `this.f`]
-codetoanalyze/java/racerd/GuardedByTests.java, codetoanalyze.java.infer.GuardedByTests.unlockedReadBad():int, 36, GUARDEDBY_VIOLATION, no_bucket, ERROR, [access to `this.f`]
-codetoanalyze/java/racerd/GuardedByTests.java, codetoanalyze.java.infer.GuardedByTests.unlockedReadBad():int, 36, THREAD_SAFETY_VIOLATION, no_bucket, ERROR, [<Read trace>,access to `this.f`,<Write trace>,access to `this.f`]
-codetoanalyze/java/racerd/GuardedByTests.java, codetoanalyze.java.infer.GuardedByTests.unlockedWriteBad():void, 26, GUARDEDBY_VIOLATION, no_bucket, ERROR, [access to `this.f`]
-codetoanalyze/java/racerd/GuardedByTests.java, codetoanalyze.java.infer.GuardedByTests.unlockedWriteBad():void, 26, THREAD_SAFETY_VIOLATION, no_bucket, ERROR, [<Write on unknown thread>,access to `this.f`,<Write on background thread>,access to `this.f`]
 codetoanalyze/java/racerd/Inference.java, codetoanalyze.java.checkers.Inference.read4OutsideSyncBad():int, 64, THREAD_SAFETY_VIOLATION, no_bucket, ERROR, [<Read trace>,access to `this.mField4`,<Write trace>,access to `this.mField4`]
 codetoanalyze/java/racerd/Inference.java, codetoanalyze.java.checkers.Inference.unprotectedRead1Bad():int, 21, THREAD_SAFETY_VIOLATION, no_bucket, ERROR, [<Read trace>,access to `this.mField1`,<Write trace>,access to `this.mField1`]
 codetoanalyze/java/racerd/Inference.java, codetoanalyze.java.checkers.Inference.unprotectedRead2Bad():int, 34, THREAD_SAFETY_VIOLATION, no_bucket, ERROR, [<Read trace>,access to `this.mField2`,<Write trace>,access to `this.mField2`]