[racerd] consider interfaces extending android.os.IInterface as automatically threadsafe

Reviewed By: jvillard

Differential Revision: D14772236

fbshipit-source-id: fab604f66

infer/src/concurrency/RacerDModels.ml

@@ -367,45 +367,48 @@ let is_safe_access access prefix_path tenv =
       false
 
 
-let should_flag_interface_call tenv exp call_flags pname =
-  (* return true if this function is library code from the JDK core libraries or Android *)
-  let is_java_library = function
-    | Typ.Procname.Java java_pname -> (
-      match Typ.Procname.Java.get_package java_pname with
-      | Some package_name ->
-          String.is_prefix ~prefix:"java." package_name
-          || String.is_prefix ~prefix:"android." package_name
-          || String.is_prefix ~prefix:"com.google." package_name
-      | None ->
-          false )
-    | _ ->
-        false
-  in
-  let is_builder_function = function
-    | Typ.Procname.Java java_pname ->
-        String.is_suffix ~suffix:"$Builder" (Typ.Procname.Java.get_class_name java_pname)
-    | _ ->
-        false
-  in
+let should_flag_interface_call tenv exps call_flags pname =
   let thread_safe_or_thread_confined annot =
     Annotations.ia_is_thread_safe annot || Annotations.ia_is_thread_confined annot
   in
-  call_flags.CallFlags.cf_interface && Typ.Procname.is_java pname
-  && (not (is_java_library pname || is_builder_function pname))
-  (* can't ask anyone to annotate interfaces in library code, and Builder's should always be
-     thread-safe (would be unreasonable to ask everyone to annotate them) *)
-  && (not (PatternMatch.check_class_attributes thread_safe_or_thread_confined tenv pname))
-  && (not (has_return_annot thread_safe_or_thread_confined pname))
-  &&
-  match exp with
-  | HilExp.AccessExpression receiver_access_exp :: _ -> (
-      HilExp.AccessExpression.to_access_path receiver_access_exp
-      |> AccessPath.truncate
-      |> function
-      | receiver_prefix, Some receiver_field ->
-          is_safe_access receiver_field receiver_prefix tenv |> not
-      | _ ->
-          true )
+  (* is this function in library code from the JDK core libraries or Android? *)
+  let is_java_library java_pname =
+    Typ.Procname.Java.get_package java_pname
+    |> Option.exists ~f:(fun package_name ->
+           String.is_prefix ~prefix:"java." package_name
+           || String.is_prefix ~prefix:"android." package_name
+           || String.is_prefix ~prefix:"com.google." package_name )
+  in
+  let is_builder_function java_pname =
+    String.is_suffix ~suffix:"$Builder" (Typ.Procname.Java.get_class_name java_pname)
+  in
+  let receiver_is_not_safe exps tenv =
+    List.hd exps
+    |> Option.bind ~f:(fun exp -> HilExp.get_access_exprs exp |> List.hd)
+    |> Option.map ~f:HilExp.AccessExpression.to_access_path
+    |> Option.map ~f:AccessPath.truncate
+    |> Option.exists ~f:(function
+         | receiver_prefix, Some receiver_field ->
+             not (is_safe_access receiver_field receiver_prefix tenv)
+         | _ ->
+             true )
+  in
+  let implements_threadsafe_interface java_pname tenv =
+    (* generated classes implementing this interface are always threadsafe *)
+    Typ.Procname.Java.get_class_type_name java_pname
+    |> fun tname -> PatternMatch.is_subtype_of_str tenv tname "android.os.IInterface"
+  in
+  match pname with
+  | Typ.Procname.Java java_pname ->
+      call_flags.CallFlags.cf_interface
+      && (not (is_java_library java_pname))
+      && (not (is_builder_function java_pname))
+      (* can't ask anyone to annotate interfaces in library code, and Builder's should always be
+      thread-safe (would be unreasonable to ask everyone to annotate them) *)
+      && (not (PatternMatch.check_class_attributes thread_safe_or_thread_confined tenv pname))
+      && (not (has_return_annot thread_safe_or_thread_confined pname))
+      && receiver_is_not_safe exps tenv
+      && not (implements_threadsafe_interface java_pname tenv)
   | _ ->
       false
 

infer/tests/codetoanalyze/java/racerd/AndroidModels.java

@@ -12,10 +12,15 @@ import android.content.Context;
 import android.content.res.AssetManager;
 import android.content.res.Configuration;
 import android.content.res.Resources;
+import android.os.IBinder;
+import android.os.IInterface;
 import android.util.DisplayMetrics;
 import android.view.View;
 import javax.annotation.concurrent.ThreadSafe;
 
+// aidl generated classes implementing this interface are automatically threadsafe
+interface AidlInterface extends IInterface {}
+
 class MyActivity extends Activity {}
 
 class MyResources extends Resources {
@@ -67,4 +72,8 @@ public class AndroidModels {
     MyView view = (MyView) activity.findViewById(-1);
     view.mField = true; // ok;
   }
+
+  public IBinder safeByDefaultInterfaceCallOk(AidlInterface i) {
+    return i.asBinder();
+  }
 }

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

@@ -1,4 +1,4 @@
-codetoanalyze/java/racerd/AndroidModels.java, codetoanalyze.java.checkers.AndroidModels.someResourceMethodsNotFunctionalBad():void, 58, THREAD_SAFETY_VIOLATION, no_bucket, ERROR, [access to `this.mField`]
+codetoanalyze/java/racerd/AndroidModels.java, codetoanalyze.java.checkers.AndroidModels.someResourceMethodsNotFunctionalBad():void, 63, THREAD_SAFETY_VIOLATION, no_bucket, ERROR, [access to `this.mField`]
 codetoanalyze/java/racerd/Annotations.java, codetoanalyze.java.checkers.Annotations.FP_functionalAcrossBoxingLongOk():int, 342, THREAD_SAFETY_VIOLATION, no_bucket, ERROR, [access to `this.mBoxedLong2`]
 codetoanalyze/java/racerd/Annotations.java, codetoanalyze.java.checkers.Annotations.FP_functionalAcrossUnboxingOk():boolean, 312, THREAD_SAFETY_VIOLATION, no_bucket, ERROR, [access to `this.mBool2`]
 codetoanalyze/java/racerd/Annotations.java, codetoanalyze.java.checkers.Annotations.conditional2_bad(boolean):void, 180, THREAD_SAFETY_VIOLATION, no_bucket, ERROR, [access to `this.ii`]