From 41729410ac95ee47c438e464d922fcce59a5cf55 Mon Sep 17 00:00:00 2001 From: Nikos Gorogiannis Date: Thu, 3 Oct 2019 05:08:23 -0700 Subject: [PATCH] [starvation] remove rarely-used logic for identifying locks Summary: Ideally the analyser should equate locks `this.x.f` and `a.x.f` in different methods if they can alias. The heuristic removed here was rarely used and is in the way of a re-write of the analysis. It was also badly implemented, as this should ideally be the comparison relation of `Lock`. Reviewed By: mityal Differential Revision: D17602827 fbshipit-source-id: 4f4576c1a --- infer/src/concurrency/starvationDomain.ml | 15 +-------------- .../codetoanalyze/java/starvation/Interproc.java | 2 +- .../codetoanalyze/java/starvation/Intraproc.java | 2 +- .../codetoanalyze/java/starvation/issues.exp | 2 -- 4 files changed, 3 insertions(+), 18 deletions(-) diff --git a/infer/src/concurrency/starvationDomain.ml b/infer/src/concurrency/starvationDomain.ml index ae3a83949..965a9fa8a 100644 --- a/infer/src/concurrency/starvationDomain.ml +++ b/infer/src/concurrency/starvationDomain.ml @@ -27,19 +27,6 @@ module Lock = struct let equal = [%compare.equal: t] - let equal_modulo_base (((root, typ), aclist) as l) (((root', typ'), aclist') as l') = - phys_equal l l' - || - match (root, root') with - | Var.LogicalVar _, Var.LogicalVar _ -> - (* only class objects are supposed to appear as idents *) - equal l l' - | Var.ProgramVar _, Var.ProgramVar _ -> - [%compare.equal: Typ.t * AccessPath.access list] (typ, aclist) (typ', aclist') - | _, _ -> - false - - let pp = AccessPath.pp let owner_class ((_, {Typ.desc}), _) = @@ -144,7 +131,7 @@ module Order = struct let may_deadlock {elem= {first; eventually}} {elem= {first= first'; eventually= eventually'}} = match (eventually.elem, eventually'.elem) with | LockAcquire e, LockAcquire e' -> - Lock.equal_modulo_base first e' && Lock.equal_modulo_base first' e + Lock.equal first e' && Lock.equal first' e | _, _ -> false diff --git a/infer/tests/codetoanalyze/java/starvation/Interproc.java b/infer/tests/codetoanalyze/java/starvation/Interproc.java index 7aa2ba0a7..fbb81fc8e 100644 --- a/infer/tests/codetoanalyze/java/starvation/Interproc.java +++ b/infer/tests/codetoanalyze/java/starvation/Interproc.java @@ -36,7 +36,7 @@ class Interproc { } class InterprocA { - synchronized void interproc1Bad(Interproc c) { + synchronized void FN_interproc1Bad(Interproc c) { interproc2Bad(c); } diff --git a/infer/tests/codetoanalyze/java/starvation/Intraproc.java b/infer/tests/codetoanalyze/java/starvation/Intraproc.java index fb7a5e9c7..b09d8959a 100644 --- a/infer/tests/codetoanalyze/java/starvation/Intraproc.java +++ b/infer/tests/codetoanalyze/java/starvation/Intraproc.java @@ -31,7 +31,7 @@ class Intraproc { } class IntraprocA { - void intraBad(Intraproc o) { + void FN_intraBad(Intraproc o) { synchronized (this) { synchronized (o) { } diff --git a/infer/tests/codetoanalyze/java/starvation/issues.exp b/infer/tests/codetoanalyze/java/starvation/issues.exp index 065133d31..60b3dddaa 100644 --- a/infer/tests/codetoanalyze/java/starvation/issues.exp +++ b/infer/tests/codetoanalyze/java/starvation/issues.exp @@ -21,8 +21,6 @@ codetoanalyze/java/starvation/IndirectBlock.java, IndirectBlock.takeRemoteExpens codetoanalyze/java/starvation/InnerClass.java, InnerClass$InnerClassA.(InnerClass,java.lang.Object), 47, DEADLOCK, no_bucket, ERROR, [[Trace 1] `InnerClass$InnerClassA.(InnerClass,Object)`, locks `this` in `class InnerClass$InnerClassA`,Method call: `void InnerClass.bar()`, locks `this` in `class InnerClass`,[Trace 2] `void InnerClass.outerInnerBad(InnerClass$InnerClassA)`, locks `this` in `class InnerClass`,Method call: `void InnerClass$InnerClassA.baz()`, locks `this` in `class InnerClass$InnerClassA`] codetoanalyze/java/starvation/InnerClass.java, InnerClass$InnerClassA.innerOuterBad():void, 33, DEADLOCK, no_bucket, ERROR, [[Trace 1] `void InnerClass$InnerClassA.innerOuterBad()`, locks `this` in `class InnerClass$InnerClassA`,Method call: `void InnerClass.bar()`, locks `this` in `class InnerClass`,[Trace 2] `void InnerClass.outerInnerBad(InnerClass$InnerClassA)`, locks `this` in `class InnerClass`,Method call: `void InnerClass$InnerClassA.baz()`, locks `this` in `class InnerClass$InnerClassA`] codetoanalyze/java/starvation/Interclass.java, Interclass.interclass1Bad(InterclassA):void, 10, DEADLOCK, no_bucket, ERROR, [[Trace 1] `void Interclass.interclass1Bad(InterclassA)`, locks `this` in `class Interclass`,Method call: `void InterclassA.interclass1Bad()`, locks `this` in `class InterclassA`,[Trace 2] `void InterclassA.interclass2Bad(Interclass)`, locks `this` in `class InterclassA`,Method call: `void Interclass.interclass2Bad()`, locks `this` in `class Interclass`] -codetoanalyze/java/starvation/Interproc.java, Interproc.interproc1Bad(InterprocA):void, 9, DEADLOCK, no_bucket, ERROR, [[Trace 1] `void Interproc.interproc1Bad(InterprocA)`, locks `this` in `class Interproc`,Method call: `void Interproc.interproc2Bad(InterprocA)`, locks `b` in `class InterprocA`,[Trace 2] `void InterprocA.interproc1Bad(Interproc)`, locks `this` in `class InterprocA`,Method call: `void InterprocA.interproc2Bad(Interproc)`, locks `d` in `class Interproc`] -codetoanalyze/java/starvation/Intraproc.java, Intraproc.intraBad(IntraprocA):void, 10, DEADLOCK, no_bucket, ERROR, [[Trace 1] `void Intraproc.intraBad(IntraprocA)`, locks `this` in `class Intraproc`, locks `o` in `class IntraprocA`,[Trace 2] `void IntraprocA.intraBad(Intraproc)`, locks `this` in `class IntraprocA`, locks `o` in `class Intraproc`] codetoanalyze/java/starvation/LegacySync.java, LegacySync.onUiThreadOpBad():java.lang.Object, 25, STARVATION, no_bucket, ERROR, [[Trace 1] `Object LegacySync.onUiThreadOpBad()`, locks `this.table` in `class LegacySync`,[Trace 2] `void LegacySync.notOnUiThreadSyncedBad()`, locks `this.table` in `class LegacySync`,calls `Object Future.get()`,[Trace 1 on UI thread] `Object LegacySync.onUiThreadOpBad()`,`Object LegacySync.onUiThreadOpBad()` is annotated `UiThread`] codetoanalyze/java/starvation/LocklessTests.java, LocklessTestsA.locklessMethod():void, 23, LOCKLESS_VIOLATION, no_bucket, ERROR, [`void LocklessTestsA.locklessMethod()`, locks `this` in `class LocklessTestsA`] codetoanalyze/java/starvation/LocklessTests.java, LocklessTestsB.locklessMethod():void, 39, LOCKLESS_VIOLATION, no_bucket, ERROR, [`void LocklessTestsB.locklessMethod()`, locks `this` in `class LocklessTestsB`]