[starvation][whole-program] static initializer attributes

Summary: Similar to constructor established attributes, we do the same here for static initializers.  That is, attributes of static properties are injected into the initial state of every method.

Reviewed By: ezgicicek

Differential Revision: D18763192

fbshipit-source-id: 3879a27c5

infer/src/concurrency/starvation.ml

@@ -275,36 +275,37 @@ end
 
 module Analyzer = LowerHil.MakeAbstractInterpreter (TransferFunctions (ProcCfg.Normal))
 
-(** Before analyzing Java instance methods (non-constructor, non-static), compute the attributes of
+(** Compute the attributes (of static variables) set up by the class initializer. *)
-    instance variables that all constructors agree on after termination. *)
+let set_class_init_attributes procname (astate : Domain.t) =
-let add_constructor_attributes tenv procname (astate : Domain.t) =
   let open Domain in
-  let skip_constructor_analysis =
+  let attributes =
-    match procname with
+    Typ.Procname.get_class_type_name procname
-    | Typ.Procname.Java jname ->
+    |> Option.map ~f:(fun tname -> Typ.Procname.(Java (Java.get_class_initializer tname)))
-        (* constructor attributes affect instance fields, which static methods cannot access;
+    |> Option.bind ~f:Payload.read_toplevel_procedure
-           ditto for the class initializer; finally, don't recurse if already in constructor *)
+    |> Option.value_map ~default:AttributeDomain.top ~f:(fun summary -> summary.attributes)
-        Typ.Procname.Java.(is_static jname || is_constructor jname || is_class_initializer jname)
-    | _ ->
-        (* don't do anything for non-Java code *)
-        true
   in
-  if skip_constructor_analysis then astate
+  ({astate with attributes} : t)
-  else
+
+
+(** Compute the attributes of instance variables that all constructors agree on. *)
+let set_constructor_attributes tenv procname (astate : Domain.t) =
+  let open Domain in
   (* make a local [this] variable, for replacing all constructor attribute map keys' roots *)
   let local_this = Pvar.mk Mangled.this procname |> Var.of_pvar in
   let make_local exp =
+    (* contract here matches that of [StarvationDomain.summary_of_astate] *)
     let var, typ = HilExp.AccessExpression.get_base exp in
-      let newvar =
+    if Var.is_global var then
+      (* let expressions rooted at globals unchanged, these are probably from class initialiser *)
+      exp
+    else (
       assert (Var.is_this var) ;
-        local_this
+      HilExp.AccessExpression.replace_base ~remove_deref_after_base:false (local_this, typ) exp )
-      in
-      HilExp.AccessExpression.replace_base ~remove_deref_after_base:false (newvar, typ) exp
   in
   let localize_attrs attributes =
     AttributeDomain.(fold (fun exp attr acc -> add (make_local exp) attr acc) attributes empty)
   in
-    (* get class of current procedure *)
+  let attributes =
     Typ.Procname.get_class_type_name procname
     (* retrieve its definition *)
     |> Option.bind ~f:(Tenv.lookup tenv)
@@ -319,7 +320,26 @@ let add_constructor_attributes tenv procname (astate : Domain.t) =
     (* join all the attribute maps together *)
     |> List.reduce ~f:AttributeDomain.join
     |> Option.value ~default:AttributeDomain.top
-    |> fun attributes -> {astate with attributes}
+  in
+  {astate with attributes}
+
+
+let set_initial_attributes tenv procname astate =
+  match procname with
+  | Typ.Procname.Java java_pname when Typ.Procname.Java.is_class_initializer java_pname ->
+      (* we are analyzing the class initializer, don't go through on-demand again *)
+      astate
+  | Typ.Procname.Java java_pname
+    when Typ.Procname.Java.(is_constructor java_pname || is_static java_pname) ->
+      (* analyzing a constructor or static method, so we need the attributes established by the
+         class initializer *)
+      set_class_init_attributes procname astate
+  | Typ.Procname.Java _ ->
+      (* we are analyzing an instance method, so we need constructor-established attributes
+         which will include those by the class initializer *)
+      set_constructor_attributes tenv procname astate
+  | _ ->
+      astate
 
 
 let analyze_procedure {Callbacks.exe_env; summary} =
@@ -362,8 +382,8 @@ let analyze_procedure {Callbacks.exe_env; summary} =
     in
     let initial =
       Domain.bottom
-      (* set the attributes of instance variables to those achieved by all constructors *)
+      (* set the attributes of instance variables set up by all constructors or the class initializer *)
-      |> add_constructor_attributes tenv procname
+      |> set_initial_attributes tenv procname
       |> set_lock_state_for_synchronized_proc |> set_thread_status_by_annotation
     in
     Analyzer.compute_post proc_data ~initial

infer/src/concurrency/starvationDomain.ml

@@ -690,19 +690,22 @@ let integrate_summary ?tenv ?lhs callsite (astate : t) (summary : summary) =
 let summary_of_astate : Procdesc.t -> t -> summary =
  fun proc_desc astate ->
   let proc_name = Procdesc.get_proc_name proc_desc in
-  let is_java_constr =
+  let attributes =
-    match (proc_name : Typ.Procname.t) with
+    let var_predicate =
-    | Java jname ->
+      match proc_name with
-        Typ.Procname.Java.is_constructor jname
+      | Typ.Procname.Java jname when Typ.Procname.Java.is_class_initializer jname ->
+          (* only keep static attributes for the class initializer *)
+          fun v -> Var.is_global v
+      | Typ.Procname.Java jname when Typ.Procname.Java.is_constructor jname ->
+          (* only keep static attributes or ones that have [this] as their root *)
+          fun v -> Var.is_this v || Var.is_global v
       | _ ->
-        false
+          (* non-constructor/class initializer or non-java, don't keep any attributes *)
+          Fn.const false
     in
-  let attributes =
+    AttributeDomain.filter
-    if is_java_constr then
+      (fun exp _ -> HilExp.AccessExpression.get_base exp |> fst |> var_predicate)
-      (* only keep attributes that have [this] as their root *)
+      astate.attributes
-      let filter_this exp _ = HilExp.AccessExpression.get_base exp |> fst |> Var.is_this in
-      AttributeDomain.filter filter_this astate.attributes
-    else AttributeDomain.empty
   in
   let return_attribute =
     let return_var_exp =

infer/tests/codetoanalyze/java/starvation-whole-program/StaticInitAttributes.java

@@ -0,0 +1,66 @@
+/*
+ * Copyright (c) Facebook, Inc. and its affiliates.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ */
+
+import android.os.Binder;
+import android.os.Handler;
+import android.os.Looper;
+import android.os.RemoteException;
+import java.util.concurrent.Executor;
+
+class StaticInitAttributes {
+  static Binder binder;
+
+  private static void doTransact() {
+    try {
+      binder.transact(0, null, null, 0);
+    } catch (RemoteException e) {
+    }
+  }
+
+  @ForUiThread static Executor mUiThreadExecutor = null;
+
+  static Executor mUiExecutor;
+  static Executor mNonUiExecutor;
+  static Handler mUiHandler;
+  static Runnable mBadRunnable;
+  static Runnable mOkRunnable;
+
+  static {
+    mUiExecutor = mUiThreadExecutor;
+    mNonUiExecutor = Executors.getBackgroundExecutor();
+    mUiHandler = new Handler(Looper.getMainLooper());
+    mBadRunnable =
+        new Runnable() {
+          @Override
+          public void run() {
+            doTransact();
+          }
+        };
+
+    mOkRunnable =
+        new Runnable() {
+          @Override
+          public void run() {}
+        };
+  }
+
+  public void postBlockingCallToUIExecutorBad() {
+    mUiExecutor.execute(mBadRunnable);
+  }
+
+  public void postNoopCallToUIExecutorOk() {
+    mUiExecutor.execute(mOkRunnable);
+  }
+
+  public void postBlockingCallToNonUIExecutorOk() {
+    mNonUiExecutor.execute(mBadRunnable);
+  }
+
+  public void postBlockingCallToUIHandlerBad() {
+    mUiHandler.post(mBadRunnable);
+  }
+}

infer/tests/codetoanalyze/java/starvation-whole-program/issues.exp

@@ -32,6 +32,8 @@ codetoanalyze/java/starvation-whole-program/MyServiceConnection.java, MyServiceC
 codetoanalyze/java/starvation-whole-program/MyServiceConnection.java, MyServiceConnection.onNullBinding(android.content.ComponentName):void, 31, STARVATION, no_bucket, ERROR, [`void MyServiceConnection.onNullBinding(ComponentName)`,Method call: `void MyServiceConnection.bad()`,calls `boolean Binder.transact(int,Parcel,Parcel,int)`]
 codetoanalyze/java/starvation-whole-program/MyServiceConnection.java, MyServiceConnection.onServiceConnected(android.content.ComponentName,android.os.IBinder):void, 36, STARVATION, no_bucket, ERROR, [`void MyServiceConnection.onServiceConnected(ComponentName,IBinder)`,Method call: `void MyServiceConnection.bad()`,calls `boolean Binder.transact(int,Parcel,Parcel,int)`]
 codetoanalyze/java/starvation-whole-program/MyServiceConnection.java, MyServiceConnection.onServiceDisconnected(android.content.ComponentName):void, 41, STARVATION, no_bucket, ERROR, [`void MyServiceConnection.onServiceDisconnected(ComponentName)`,Method call: `void MyServiceConnection.bad()`,calls `boolean Binder.transact(int,Parcel,Parcel,int)`]
+codetoanalyze/java/starvation-whole-program/StaticInitAttributes.java, StaticInitAttributes.postBlockingCallToUIExecutorBad():void, 52, STARVATION, no_bucket, ERROR, [`void StaticInitAttributes.postBlockingCallToUIExecutorBad()`,Method call: `void StaticInitAttributes$1.run()`,Method call: `void StaticInitAttributes.doTransact()`,calls `boolean Binder.transact(int,Parcel,Parcel,int)`]
+codetoanalyze/java/starvation-whole-program/StaticInitAttributes.java, StaticInitAttributes.postBlockingCallToUIHandlerBad():void, 64, STARVATION, no_bucket, ERROR, [`void StaticInitAttributes.postBlockingCallToUIHandlerBad()`,Method call: `void StaticInitAttributes$1.run()`,Method call: `void StaticInitAttributes.doTransact()`,calls `boolean Binder.transact(int,Parcel,Parcel,int)`]
 codetoanalyze/java/starvation-whole-program/ThreadScheduling.java, ThreadScheduling.scheduleBlockingCallOnContendedLockBad():void, 36, STARVATION, no_bucket, ERROR, [[Trace 1] `void ThreadScheduling.scheduleBlockingCallOnContendedLockBad()`,Method call: `void ThreadScheduling$2.run()`, locks `this.monitorA` in `class ThreadScheduling`,[Trace 2] `void ThreadScheduling.scheduleBlockingCallOnContendedLockBad()`,Method call: `void ThreadScheduling$1.run()`, locks `this.monitorA` in `class ThreadScheduling`,Method call: `void ThreadScheduling.doTransact()`,calls `boolean Binder.transact(int,Parcel,Parcel,int)`]
 codetoanalyze/java/starvation-whole-program/ThreadScheduling.java, ThreadScheduling.scheduleBlockingCallOnContendedLockViaInheritanceBad():void, 89, STARVATION, no_bucket, ERROR, [[Trace 1] `void ThreadScheduling.scheduleBlockingCallOnContendedLockViaInheritanceBad()`,Method call: `void ThreadScheduling$5.run()`, locks `this.monitorD` in `class ThreadScheduling`,[Trace 2] `void ThreadScheduling.scheduleBlockingCallOnContendedLockViaInheritanceBad()`,Method call: `void ThreadScheduling$BadThread.run()`, locks `this.monitorD` in `class ThreadScheduling`,Method call: `void ThreadScheduling.doTransact()`,calls `boolean Binder.transact(int,Parcel,Parcel,int)`]
 codetoanalyze/java/starvation-whole-program/ThreadScheduling.java, ThreadScheduling.scheduleDeadlockBad():void, 60, DEADLOCK, no_bucket, ERROR, [[Trace 1] `void ThreadScheduling.scheduleDeadlockBad()`,Method call: `void ThreadScheduling$3.run()`, locks `this.monitorB` in `class ThreadScheduling`, locks `this.monitorC` in `class ThreadScheduling`,[Trace 2] `void ThreadScheduling.scheduleDeadlockBad()`,Method call: `void ThreadScheduling$4.run()`, locks `this.monitorC` in `class ThreadScheduling`, locks `this.monitorB` in `class ThreadScheduling`]