[starvation] treat locks in (and accessed from) inner classes properly

Reviewed By: sblackshear

Differential Revision: D7427659

fbshipit-source-id: 9abf1ad

infer/src/IR/AccessPath.ml

@@ -261,3 +261,49 @@ module BaseMap = PrettyPrintable.MakePPMap (struct
 
   let pp = pp_base
 end)
+
+(* transform an access path that starts on "this" of an inner class but which breaks out to
+   access outer class fields to the outermost one *)
+let inner_class_normalize p =
+  let open Typ in
+  let is_synthetic_this pvar = Pvar.get_simplified_name pvar |> String.is_prefix ~prefix:"this$" in
+  let mk_pvar_as name pvar = Pvar.get_declaring_function pvar |> Option.map ~f:(Pvar.mk name) in
+  let aux = function
+    (* (this:InnerClass* ).(this$n:OuterClassAccessor).f. ... -> (this:OuterClass* ).f . ... *)
+    | Some
+        ( ( (Var.ProgramVar pvar as root)
+          , ({desc= Tptr (({desc= Tstruct name} as cls), pkind)} as ptr) )
+        , (FieldAccess first) :: accesses )
+      when Pvar.is_this pvar && Fieldname.Java.is_outer_instance first ->
+        Name.Java.get_outer_class name
+        |> Option.map ~f:(fun outer_name ->
+               let outer_class = mk ~default:cls (Tstruct outer_name) in
+               let outer_ptr = mk ~default:ptr (Tptr (outer_class, pkind)) in
+               ((root, outer_ptr), accesses) )
+    (* this$n.(this$m:OuterClassAccessor).f ... -> (this$m:OuterClass* ).f . ... *)
+    (* happens in ctrs only *)
+    | Some
+        ( (Var.ProgramVar pvar, ({desc= Tptr (({desc= Tstruct name} as cls), pkind)} as ptr))
+        , (FieldAccess first) :: accesses )
+      when is_synthetic_this pvar && Fieldname.Java.is_outer_instance first ->
+        Name.Java.get_outer_class name
+        |> Option.bind ~f:(fun outer_name ->
+               let outer_class = mk ~default:cls (Tstruct outer_name) in
+               let outer_ptr = mk ~default:ptr (Tptr (outer_class, pkind)) in
+               let varname = Fieldname.to_flat_string first |> Mangled.from_string in
+               mk_pvar_as varname pvar
+               |> Option.map ~f:(fun new_pvar ->
+                      let base = base_of_pvar new_pvar outer_ptr in
+                      (base, accesses) ) )
+    (* this$n.f ... -> this.f . ... *)
+    (* happens in ctrs only *)
+    | Some ((Var.ProgramVar pvar, typ), all_accesses)
+      when is_synthetic_this pvar ->
+        let varname = Mangled.from_string "this" in
+        mk_pvar_as varname pvar
+        |> Option.map ~f:(fun new_pvar -> (base_of_pvar new_pvar typ, all_accesses))
+    | _ ->
+        None
+  in
+  let rec loop path_opt = match aux path_opt with None -> path_opt | res -> loop res in
+  loop (Some p) |> Option.value ~default:p

infer/src/IR/AccessPath.mli

@@ -61,6 +61,17 @@ val append : t -> access list -> t
 val is_prefix : t -> t -> bool
 (** return true if [ap1] is a prefix of [ap2]. returns true for equal access paths *)
 
+val inner_class_normalize : t -> t
+(** transform an access path that starts on "this" of an inner class but which breaks out to
+   access outer class fields to the outermost one.
+   Cases handled (recursively):
+- (this:InnerClass* ).(this$n:OuterClassAccessor).f. ... -> (this:OuterClass* ).f . ...
+- this$n.(this$m:OuterClassAccessor).f ... -> (this$m:OuterClass* ).f . ...
+  (happens in ctrs only)
+- this$n.f ... -> this.f . ...
+  (happens in ctrs only)
+*)
+
 val equal : t -> t -> bool
 
 val equal_base : base -> base -> bool

infer/src/IR/Pvar.ml

@@ -55,6 +55,14 @@ let compare_modulo_this x y =
 
 let equal = [%compare.equal : t]
 
+let get_declaring_function pv =
+  match pv.pv_kind with
+  | Local_var n | Callee_var n | Abduced_retvar (n, _) | Abduced_ref_param (n, _, _) ->
+      Some n
+  | Global_var _ | Seed_var ->
+      None
+
+
 let pp_translation_unit fmt = function None -> () | Some fname -> SourceFile.pp fmt fname
 
 let pp_ f pv =

infer/src/IR/Pvar.mli

@@ -29,6 +29,9 @@ val compare_modulo_this : t -> t -> int
 val equal : t -> t -> bool
 (** Equality for pvar's *)
 
+val get_declaring_function : t -> Typ.Procname.t option
+(** if not a global, return function declaring var *)
+
 val d : t -> unit
 (** Dump a program variable. *)
 

infer/src/concurrency/starvation.ml

@@ -55,7 +55,7 @@ module TransferFunctions (CFG : ProcCfg.S) = struct
         match AccessExpression.to_access_path access_exp with
         | (((Var.ProgramVar pvar, _) as base), _) as path
           when is_formal base || Pvar.is_global pvar ->
-            Some path
+            Some (AccessPath.inner_class_normalize path)
         | _ ->
             (* ignore paths on local or logical variables *)
             None )

infer/tests/codetoanalyze/java/starvation/InnerClass.java

@@ -0,0 +1,55 @@
+/*
+ * Copyright (c) 2018 - present Facebook, Inc.
+ * All rights reserved.
+ *
+ * This source code is licensed under the BSD style license found in the
+ * LICENSE file in the root directory of this source tree. An additional grant
+ * of patent rights can be found in the PATENTS file in the same directory.
+ */
+
+import java.lang.Object;
+
+class InnerClass {
+  synchronized void outerInnerOk(InnerClassA a) {
+    a.foo();
+  }
+
+  synchronized void bar() {}
+
+  synchronized void outerInnerBad(InnerClassA a) {
+    a.baz();
+  }
+
+  class InnerClassA {
+    void foo() {
+      synchronized(InnerClass.this) {}
+    }
+
+    void outerInnerOk() {
+      synchronized(InnerClass.this) {
+        InnerClass.this.bar();
+      }
+    }
+
+    synchronized void baz() {}
+
+    synchronized void innerOuterBad() {
+      InnerClass.this.bar();
+    }
+
+    // ctrs generate different access paths so test these too
+    // following should not be flagged
+    InnerClassA() {
+      synchronized(InnerClass.this) {
+        InnerClass.this.bar();
+      }
+    }
+
+    // following should be flagged with outer_inner_bad()
+    InnerClassA(Object o) {
+      synchronized(this) {
+        InnerClass.this.bar();
+      }
+    }
+  }
+}

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

@@ -1,3 +1,5 @@
+codetoanalyze/java/starvation/InnerClass.java, void InnerClass$InnerClassA.innerOuterBad(), 0, STARVATION, ERROR, [[Trace 1] Lock acquisition: locks this in class InnerClass$InnerClassA*,Method call: void InnerClass.bar(),Lock acquisition: locks this in class InnerClass*,[Trace 2] Lock acquisition: locks this in class InnerClass*,Method call: void InnerClass$InnerClassA.baz(),Lock acquisition: locks this in class InnerClass$InnerClassA*]
+codetoanalyze/java/starvation/InnerClass.java, void InnerClass.outerInnerBad(InnerClass$InnerClassA), 0, STARVATION, ERROR, [[Trace 1] Lock acquisition: locks this in class InnerClass*,Method call: void InnerClass$InnerClassA.baz(),Lock acquisition: locks this in class InnerClass$InnerClassA*,[Trace 2] Method start: InnerClass$InnerClassA.<init>(InnerClass,Object),Lock acquisition: locks this in class InnerClass$InnerClassA*,Method call: void InnerClass.bar(),Lock acquisition: locks this in class InnerClass*]
 codetoanalyze/java/starvation/Interclass.java, void Interclass.interclass1_bad(InterclassA), 0, STARVATION, ERROR, [[Trace 1] Lock acquisition: locks this in class Interclass*,Method call: void InterclassA.interclass1_bad(),Lock acquisition: locks this in class InterclassA*,[Trace 2] Lock acquisition: locks this in class InterclassA*,Method call: void Interclass.interclass2_bad(),Lock acquisition: locks this in class Interclass*]
 codetoanalyze/java/starvation/Interproc.java, void Interproc.interproc1_bad(InterprocA), 0, STARVATION, ERROR, [[Trace 1] Lock acquisition: locks this in class Interproc*,Method call: void Interproc.interproc2_bad(InterprocA),Lock acquisition: locks b in class InterprocA*,[Trace 2] Lock acquisition: locks this in class InterprocA*,Method call: void InterprocA.interproc2_bad(Interproc),Lock acquisition: locks d in class Interproc*]
 codetoanalyze/java/starvation/Intraproc.java, void IntraprocA.intra_bad(Intraproc), 0, STARVATION, ERROR, [[Trace 1] Method start: void IntraprocA.intra_bad(Intraproc),Lock acquisition: locks this in class IntraprocA*,Lock acquisition: locks o in class Intraproc*,[Trace 2] Method start: void Intraproc.intra_bad(IntraprocA),Lock acquisition: locks this in class Intraproc*,Lock acquisition: locks o in class IntraprocA*]