diff --git a/infer/src/concurrency/RacerDModels.ml b/infer/src/concurrency/RacerDModels.ml index 5f620ac15..433076222 100644 --- a/infer/src/concurrency/RacerDModels.ml +++ b/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 diff --git a/infer/tests/codetoanalyze/java/racerd/AndroidModels.java b/infer/tests/codetoanalyze/java/racerd/AndroidModels.java index 6f4f6b603..aa53ed60c 100644 --- a/infer/tests/codetoanalyze/java/racerd/AndroidModels.java +++ b/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(); + } } diff --git a/infer/tests/codetoanalyze/java/racerd/issues.exp b/infer/tests/codetoanalyze/java/racerd/issues.exp index 0a906b784..2ca1b9dd4 100644 --- a/infer/tests/codetoanalyze/java/racerd/issues.exp +++ b/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`]