From 4a75df2a83e031fd16d01a3fd7612c42988d5163 Mon Sep 17 00:00:00 2001 From: Nikos Gorogiannis Date: Mon, 25 Mar 2019 04:14:52 -0700 Subject: [PATCH] [racerd] report only writes for GuardedBy Reviewed By: mbouaziz Differential Revision: D14594263 fbshipit-source-id: a8cab1765 --- infer/src/concurrency/RacerD.ml | 1 + infer/src/concurrency/RacerDDomain.mli | 1 + infer/tests/build_systems/ant/issues.exp | 23 +++---- .../java/racerd/GuardedByTests.java | 61 ++++++++++++------- .../codetoanalyze/java/racerd/issues.exp | 12 +--- 5 files changed, 53 insertions(+), 45 deletions(-) diff --git a/infer/src/concurrency/RacerD.ml b/infer/src/concurrency/RacerD.ml index b2e6e671f..dff19c491 100644 --- a/infer/src/concurrency/RacerD.ml +++ b/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 *) diff --git a/infer/src/concurrency/RacerDDomain.mli b/infer/src/concurrency/RacerDDomain.mli index 50b33c91d..489c72136 100644 --- a/infer/src/concurrency/RacerDDomain.mli +++ b/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 diff --git a/infer/tests/build_systems/ant/issues.exp b/infer/tests/build_systems/ant/issues.exp index 869b2581e..39d584d37 100644 --- a/infer/tests/build_systems/ant/issues.exp +++ b/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.guardedBySelfReferenceOK():void, 409, GUARDEDBY_VIOLATION, no_bucket, ERROR, [access to `this.self_reference`] +codetoanalyze/java/infer/GuardedByExample.java, codetoanalyze.java.infer.GuardedByExample.byRefTrickyBad():java.lang.Object, 281, THREAD_SAFETY_VIOLATION, no_bucket, ERROR, [,access to `this.g`,,access to `this.g`] 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, [,access to `this.f`,,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.readFBadButSuppressed():void, 71, GUARDEDBY_VIOLATION, no_bucket, ERROR, [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.readFBad():void, 66, THREAD_SAFETY_VIOLATION, no_bucket, ERROR, [,access to `this.f`,,access to `this.f`] +codetoanalyze/java/infer/GuardedByExample.java, codetoanalyze.java.infer.GuardedByExample.readFBadButSuppressed():void, 71, THREAD_SAFETY_VIOLATION, no_bucket, ERROR, [,access to `this.f`,,access to `this.f`] +codetoanalyze/java/infer/GuardedByExample.java, codetoanalyze.java.infer.GuardedByExample.readFBadButSuppressedOther():void, 76, THREAD_SAFETY_VIOLATION, no_bucket, ERROR, [,access to `this.f`,,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, [,access to `this.f`,,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, [,access to `this.f`,,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, [,access to `this.f`,,access to `this.f`] codetoanalyze/java/infer/GuardedByExample.java, codetoanalyze.java.infer.GuardedByExample.readFOkSynchronized():void, 127, THREAD_SAFETY_VIOLATION, no_bucket, ERROR, [,access to `this.f`,,access to `this.f`] codetoanalyze/java/infer/GuardedByExample.java, codetoanalyze.java.infer.GuardedByExample.readGFromCopyOk():void, 267, THREAD_SAFETY_VIOLATION, no_bucket, ERROR, [,access to `this.mCopyOfG`,,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, [,access to `this.f`,,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, [,access to `infer.GuardedByExample.sGuardedByClass`,,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()] diff --git a/infer/tests/codetoanalyze/java/racerd/GuardedByTests.java b/infer/tests/codetoanalyze/java/racerd/GuardedByTests.java index f95db9e43..1891cbd34 100644 --- a/infer/tests/codetoanalyze/java/racerd/GuardedByTests.java +++ b/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; - } - - public void unlockedWriteBad() { - f = 0; + a = 0; } public void lockedWriteOk() { synchronized (mlock) { - f = 0; + a = 0; } } - public int unlockedReadBad() { - return f; + @GuardedBy("mLock") + 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() { - return f; + void suppressedWrite() { + 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; diff --git a/infer/tests/codetoanalyze/java/racerd/issues.exp b/infer/tests/codetoanalyze/java/racerd/issues.exp index cef8324c8..bd40ada35 100644 --- a/infer/tests/codetoanalyze/java/racerd/issues.exp +++ b/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.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.interprocUnlockedReadBad():int, 58, THREAD_SAFETY_VIOLATION, no_bucket, ERROR, [,call to int GuardedByTests.privateUnlockedReadOk(),access to `this.f`,,access to `this.f`] -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, [,call to void GuardedByTests.privateUnlockedWriteOk(),access to `this.f`,,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, [,access to `this.f`,,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, [,access to `this.f`,,access to `this.f`] +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.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.unlockedWriteBad():void, 35, GUARDEDBY_VIOLATION, no_bucket, ERROR, [access to `this.b`] codetoanalyze/java/racerd/Inference.java, codetoanalyze.java.checkers.Inference.read4OutsideSyncBad():int, 64, THREAD_SAFETY_VIOLATION, no_bucket, ERROR, [,access to `this.mField4`,,access to `this.mField4`] codetoanalyze/java/racerd/Inference.java, codetoanalyze.java.checkers.Inference.unprotectedRead1Bad():int, 21, THREAD_SAFETY_VIOLATION, no_bucket, ERROR, [,access to `this.mField1`,,access to `this.mField1`] codetoanalyze/java/racerd/Inference.java, codetoanalyze.java.checkers.Inference.unprotectedRead2Bad():int, 34, THREAD_SAFETY_VIOLATION, no_bucket, ERROR, [,access to `this.mField2`,,access to `this.mField2`]