[racerd] behave more angelically when dealing with certain methods without summary

Summary:
RacerD is angelic in the sense that when a method has no summary, no accesses are added to the symbolic state when we call that method.  However, when the method returns an object we then proceed to access, this leads to non-angelic behaviour: if the object is assumed to be un-owned, then the accesses may lead to a race.

This manifests itself on Litho components, which are generated code without sources and thus RacerD has no CFG to analyse, and therefore produces no summary.  The `Builder` patterns used in these classes are ubiquitous, and full of spurious races due to the fact that the returned objects, even though freshly allocated, are un-owned as far as RacerD knows.

Here, instead of going full-angelic and assuming that every method without a summary returns an owned object (which is a bit extreme), we try to model the `Builder` pattern wrt ownership.  That is, static `create` methods returning `Builder` types are assumed to return full ownership; and, non-static methods of a `Builder` class which return the same type as their receiver are assumed to return conditional ownership.

Reviewed By: ezgicicek

Differential Revision: D18748423

fbshipit-source-id: bd53d4b67

infer/src/concurrency/RacerD.ml

@@ -183,8 +183,17 @@ module TransferFunctions (CFG : ProcCfg.S) = struct
   let call_without_summary callee_pname ret_base call_flags actuals astate =
     let open RacerDModels in
     let open RacerDDomain in
-    let should_assume_returns_ownership (call_flags : CallFlags.t) actuals =
-      (not call_flags.cf_interface) && List.is_empty actuals
+    let should_assume_returns_ownership callee_pname (call_flags : CallFlags.t) actuals =
+      (* non-interface methods with no summary and no parameters *)
+      ((not call_flags.cf_interface) && List.is_empty actuals)
+      || (* static [$Builder] creation methods *)
+      creates_builder callee_pname
+    in
+    let should_assume_returns_conditional_ownership callee_pname =
+      (* non-interface methods with  no parameters  *)
+      is_abstract_getthis_like callee_pname
+      || (* non-static [$Builder] methods with same return type as receiver type *)
+      is_builder_passthrough callee_pname
     in
     if is_box callee_pname then
       match actuals with
@@ -200,14 +209,13 @@ module TransferFunctions (CFG : ProcCfg.S) = struct
           else astate
       | _ ->
           astate
-    else if should_assume_returns_ownership call_flags actuals then
-      (* assume non-interface methods with no summary and no parameters return ownership *)
+    else if should_assume_returns_ownership callee_pname call_flags actuals then
       let ownership =
         OwnershipDomain.add (AccessExpression.base ret_base) OwnershipAbstractValue.owned
           astate.ownership
       in
       {astate with ownership}
-    else if is_abstract_getthis_like callee_pname then
+    else if should_assume_returns_conditional_ownership callee_pname then
       (* assume abstract, single-parameter methods whose return type is equal to that of the first
          formal return conditional ownership -- an example is getThis in Litho *)
       let ownership =

infer/src/concurrency/RacerDModels.ml

@@ -369,6 +369,12 @@ let is_safe_access (access : 'a HilExp.Access.t) prefix_exp tenv =
       false
 
 
+let is_builder_class tname = String.is_suffix ~suffix:"$Builder" (Typ.Name.to_string tname)
+
+let is_builder_method java_pname =
+  is_builder_class (Typ.Procname.Java.get_class_type_name java_pname)
+
+
 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
@@ -381,9 +387,6 @@ let should_flag_interface_call tenv exps call_flags pname =
            || 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)
@@ -403,7 +406,7 @@ let should_flag_interface_call tenv exps call_flags pname =
   | Typ.Procname.Java java_pname ->
       call_flags.CallFlags.cf_interface
       && (not (is_java_library java_pname))
-      && (not (is_builder_function java_pname))
+      && (not (is_builder_method java_pname))
       (* can't ask anyone to annotate interfaces in library code, and Builders should always be
          thread-safe (would be unreasonable to ask everyone to annotate them) *)
       && ConcurrencyModels.find_override_or_superclass_annotated ~attrs_of_pname
@@ -476,3 +479,31 @@ let is_abstract_getthis_like callee =
              true
          | _ ->
              false )
+
+
+let creates_builder callee =
+  (match callee with Typ.Procname.Java jpname -> Typ.Procname.Java.is_static jpname | _ -> false)
+  && String.equal "create" (Typ.Procname.get_method callee)
+  && attrs_of_pname callee
+     |> Option.exists ~f:(fun (attrs : ProcAttributes.t) ->
+            match attrs.ret_type with
+            | Typ.{desc= Tptr ({desc= Tstruct ret_class}, _)} ->
+                is_builder_class ret_class
+            | _ ->
+                false )
+
+
+let is_builder_passthrough callee =
+  match callee with
+  | Typ.Procname.Java java_pname ->
+      (not (Typ.Procname.Java.is_static java_pname))
+      && is_builder_method java_pname
+      && attrs_of_pname callee
+         |> Option.exists ~f:(fun (attrs : ProcAttributes.t) ->
+                match attrs.formals with
+                | (_, typ) :: _ when Typ.equal typ attrs.ret_type ->
+                    true
+                | _ ->
+                    false )
+  | _ ->
+      false

infer/src/concurrency/RacerDModels.mli

@@ -55,3 +55,9 @@ val is_synchronized_container : Typ.Procname.t -> HilExp.AccessExpression.t -> T
 
 val is_abstract_getthis_like : Typ.Procname.t -> bool
 (** is the callee an abstract method with one argument where argument type is equal to return type *)
+
+val creates_builder : Typ.Procname.t -> bool
+(** is the callee a static java method returning a [Builder] object? *)
+
+val is_builder_passthrough : Typ.Procname.t -> bool
+(** is the callee a non-static [Builder] method returning the same type as its receiver *)