Group arguments of apply_call that are only related with class inte…

…rvals

Summary:
So that the readability is improved. Currently `apply_call` takes 13 arguments,
which is difficult to understand. This diff reduces to 10 arguments.

Maybe we can further group `is_class_method` and `is_static_method` as well,
since they are also used only when applying the class intervals. I decided
against it for now, because:
- the other four arguments can be conveniently grouped / expressed as
  `CallInfoIntervals.t`, and
- when calling `apply_call`, argument `call_info_intervals` is almost always
  stored "directly" (i.e., without any modifications) into the taint, except
  that sometimes field `caller_interval` need to be updated based on the
  results of applying class intervals -- this is also made explicit in this
  diff with the OCaml record update.

Reviewed By: alexkassil

Differential Revision: D51910033

fbshipit-source-id: 1a47c5fa85a04baedf99aa67b1ea1283a56913e7

source/interprocedural_analyses/taint/backwardAnalysis.ml

@@ -511,12 +511,16 @@ module State (FunctionContext : FUNCTION_CONTEXT) = struct
       |> BackwardState.Tree.add_local_breadcrumb (Features.tito ())
       |> BackwardState.Tree.join taint_tree
     in
-    let is_self_call = Ast.Expression.is_self_call ~callee in
-    let is_cls_call = Ast.Expression.is_cls_call ~callee in
-    let receiver_class_interval =
-      receiver_class
-      >>| Interprocedural.ClassIntervalSetGraph.SharedMemory.of_class class_interval_graph
-      |> Option.value ~default:Interprocedural.ClassIntervalSet.top
+    let call_info_intervals =
+      {
+        Domains.CallInfoIntervals.is_self_call = Ast.Expression.is_self_call ~callee;
+        is_cls_call = Ast.Expression.is_cls_call ~callee;
+        caller_interval = FunctionContext.caller_class_interval;
+        receiver_interval =
+          receiver_class
+          >>| Interprocedural.ClassIntervalSetGraph.SharedMemory.of_class class_interval_graph
+          |> Option.value ~default:Interprocedural.ClassIntervalSet.top;
+      }
     in
     let analyze_argument
         (arguments_taint, state)
@@ -534,12 +538,9 @@ module State (FunctionContext : FUNCTION_CONTEXT) = struct
           ~call_target
           ~arguments
           ~sink_matches
-          ~is_self_call
-          ~is_cls_call
           ~is_class_method
           ~is_static_method
-          ~caller_class_interval:FunctionContext.caller_class_interval
-          ~receiver_class_interval
+          ~call_info_intervals
       in
       let taint_in_taint_out =
         if apply_tito then

source/interprocedural_analyses/taint/callModel.ml

@@ -290,12 +290,9 @@ let sink_trees_of_argument
     ~call_target:({ CallGraph.CallTarget.target; _ } as call_target)
     ~arguments
     ~sink_matches
-    ~is_self_call
-    ~is_cls_call
     ~is_class_method
     ~is_static_method
-    ~caller_class_interval
-    ~receiver_class_interval
+    ~call_info_intervals
   =
   let to_sink_tree_with_identifier { AccessPath.root; actual_path; formal_path } =
     let sink_tree =
@@ -306,12 +303,9 @@ let sink_trees_of_argument
            ~callee:(Some target)
            ~arguments
            ~port:root
-           ~is_self_call
-           ~is_cls_call
            ~is_class_method
            ~is_static_method
-           ~caller_class_interval
-           ~receiver_class_interval
+           ~call_info_intervals
       |> BackwardState.Tree.read ~transform_non_leaves formal_path
       |> BackwardState.Tree.prepend actual_path
     in
@@ -447,12 +441,9 @@ let return_sink ~resolution ~location ~callee ~sink_model =
               non-empty, to provide more information about the flow. *)
          ~arguments:[]
          ~port:AccessPath.Root.LocalResult
-         ~is_self_call:false
-         ~is_cls_call:false
          ~is_class_method:false
          ~is_static_method:false
-         ~caller_class_interval:Interprocedural.ClassIntervalSet.top
-         ~receiver_class_interval:Interprocedural.ClassIntervalSet.top
+         ~call_info_intervals:Domains.CallInfoIntervals.top
   in
   let breadcrumbs_to_attach, via_features_to_attach =
     BackwardState.extract_features_to_attach

source/interprocedural_analyses/taint/callModel.mli

@@ -79,12 +79,9 @@ val sink_trees_of_argument
   call_target:CallGraph.CallTarget.t ->
   arguments:Expression.Call.Argument.t list ->
   sink_matches:AccessPath.argument_match list ->
-  is_self_call:bool ->
-  is_cls_call:bool ->
   is_class_method:bool ->
   is_static_method:bool ->
-  caller_class_interval:ClassIntervalSet.t ->
-  receiver_class_interval:ClassIntervalSet.t ->
+  call_info_intervals:Domains.CallInfoIntervals.t ->
   Domains.SinkTreeWithHandle.t list
 
 val type_breadcrumbs_of_calls : CallGraph.CallTarget.t list -> Features.BreadcrumbSet.t

source/interprocedural_analyses/taint/domains.ml

@@ -571,12 +571,9 @@ module type TAINT_DOMAIN = sig
     port:AccessPath.Root.t ->
     path:AccessPath.Path.t ->
     element:t ->
-    is_self_call:bool ->
-    is_cls_call:bool ->
     is_class_method:bool ->
     is_static_method:bool ->
-    caller_class_interval:ClassIntervalSet.t ->
-    receiver_class_interval:ClassIntervalSet.t ->
+    call_info_intervals:CallInfoIntervals.t ->
     t
 
   (* Return the taint with only essential elements. *)
@@ -1132,10 +1129,9 @@ end = struct
   let apply_class_interval
       ~is_static_method
       ~is_class_method
-      ~caller_class_interval
-      ~receiver_class_interval
-      ~is_self_call
-      ~is_cls_call
+      ~call_info_intervals:
+        ({ CallInfoIntervals.is_self_call; is_cls_call; caller_interval; receiver_interval } as
+        call_info_intervals)
       local_taint
     =
     let { CallInfoIntervals.caller_interval = callee_class_interval; _ } =
@@ -1158,17 +1154,12 @@ end = struct
          is the same treatment as a function call. *)
       LocalTaintDomain.update
         LocalTaintDomain.Slots.CallInfoIntervals
-        {
-          CallInfoIntervals.caller_interval = caller_class_interval;
-          receiver_interval = receiver_class_interval;
-          is_self_call;
-          is_cls_call;
-        }
+        call_info_intervals
         local_taint
     else (* Case B: Call non-static methods. *)
       let new_interval, should_propagate =
         (* Decide if the taint can be propagated from the call. *)
-        intersect callee_class_interval receiver_class_interval
+        intersect callee_class_interval receiver_interval
       in
       if not should_propagate then
         LocalTaintDomain.bottom
@@ -1177,30 +1168,20 @@ end = struct
         let new_interval, should_propagate =
           (* Then impose the caller's interval, because the call chain so far is still on the same
              object (i.e., `self` or `cls`). *)
-          intersect new_interval caller_class_interval
+          intersect new_interval caller_interval
         in
         if not should_propagate then
           LocalTaintDomain.bottom
         else
           LocalTaintDomain.update
             LocalTaintDomain.Slots.CallInfoIntervals
-            {
-              CallInfoIntervals.caller_interval = new_interval;
-              receiver_interval = receiver_class_interval;
-              is_self_call;
-              is_cls_call;
-            }
+            { call_info_intervals with caller_interval = new_interval }
             local_taint
       else (* Case B.2: Call instance methods on any other objects. *)
         LocalTaintDomain.update
           LocalTaintDomain.Slots.CallInfoIntervals
-          {
-            (* Reset the interval to be the caller's interval. *)
-            CallInfoIntervals.caller_interval = caller_class_interval;
-            receiver_interval = receiver_class_interval;
-            is_self_call;
-            is_cls_call;
-          }
+          (* Reset the interval to be the caller's interval. *)
+          call_info_intervals
           local_taint
 
 
@@ -1212,12 +1193,9 @@ end = struct
       ~port
       ~path
       ~element:taint
-      ~is_self_call
-      ~is_cls_call
       ~is_class_method
       ~is_static_method
-      ~caller_class_interval
-      ~receiver_class_interval
+      ~call_info_intervals
     =
     let callees =
       match callee with
@@ -1290,23 +1268,11 @@ end = struct
         | Some (Target.Function _) ->
             LocalTaintDomain.update
               LocalTaintDomain.Slots.CallInfoIntervals
-              {
-                CallInfoIntervals.caller_interval = caller_class_interval;
-                receiver_interval = receiver_class_interval;
-                is_self_call;
-                is_cls_call;
-              }
+              call_info_intervals
               local_taint
         | Some (Target.Method _)
         | Some (Target.Override _) ->
-            apply_class_interval
-              ~is_static_method
-              ~is_class_method
-              ~caller_class_interval
-              ~receiver_class_interval
-              ~is_self_call
-              ~is_cls_call
-              local_taint
+            apply_class_interval ~is_static_method ~is_class_method ~call_info_intervals local_taint
       in
       match call_info with
       | CallInfo.Origin _
@@ -1454,12 +1420,9 @@ module MakeTaintTree (Taint : TAINT_DOMAIN) () = struct
       ~callee
       ~arguments
       ~port
-      ~is_self_call
-      ~is_cls_call
       ~is_class_method
       ~is_static_method
-      ~caller_class_interval
-      ~receiver_class_interval
+      ~call_info_intervals
       taint_tree
     =
     let transform_path (path, tip) =
@@ -1472,12 +1435,9 @@ module MakeTaintTree (Taint : TAINT_DOMAIN) () = struct
           ~port
           ~path
           ~element:tip
-          ~is_self_call
-          ~is_cls_call
           ~is_class_method
           ~is_static_method
-          ~caller_class_interval
-          ~receiver_class_interval )
+          ~call_info_intervals )
     in
     transform Path Map ~f:transform_path taint_tree
 

source/interprocedural_analyses/taint/forwardAnalysis.ml

@@ -507,12 +507,16 @@ module State (FunctionContext : FUNCTION_CONTEXT) = struct
       arguments
       Model.pp
       taint_model;
-    let is_self_call = Ast.Expression.is_self_call ~callee in
-    let is_cls_call = Ast.Expression.is_cls_call ~callee in
-    let receiver_class_interval =
-      receiver_class
-      >>| Interprocedural.ClassIntervalSetGraph.SharedMemory.of_class class_interval_graph
-      |> Option.value ~default:Interprocedural.ClassIntervalSet.top
+    let call_info_intervals =
+      {
+        Domains.CallInfoIntervals.is_self_call = Ast.Expression.is_self_call ~callee;
+        is_cls_call = Ast.Expression.is_cls_call ~callee;
+        caller_interval = FunctionContext.caller_class_interval;
+        receiver_interval =
+          receiver_class
+          >>| Interprocedural.ClassIntervalSetGraph.SharedMemory.of_class class_interval_graph
+          |> Option.value ~default:Interprocedural.ClassIntervalSet.top;
+      }
     in
     let convert_tito_path_to_taint
         ~argument
@@ -614,12 +618,9 @@ module State (FunctionContext : FUNCTION_CONTEXT) = struct
           ~call_target
           ~arguments
           ~sink_matches
-          ~is_self_call
-          ~is_cls_call
           ~is_class_method
           ~is_static_method
-          ~caller_class_interval:FunctionContext.caller_class_interval
-          ~receiver_class_interval
+          ~call_info_intervals
       in
       let tito_effects =
         if apply_tito then
@@ -700,12 +701,9 @@ module State (FunctionContext : FUNCTION_CONTEXT) = struct
              ~callee:(Some target)
              ~arguments
              ~port:AccessPath.Root.LocalResult
-             ~is_self_call
-             ~is_cls_call
              ~is_class_method
              ~is_static_method
-             ~caller_class_interval:FunctionContext.caller_class_interval
-             ~receiver_class_interval
+             ~call_info_intervals
       else
         ForwardState.Tree.empty
     in
@@ -2389,12 +2387,9 @@ module State (FunctionContext : FUNCTION_CONTEXT) = struct
         ~callee
         ~arguments:[]
         ~port:AccessPath.Root.LocalResult
-        ~is_self_call:false
-        ~is_cls_call:false
         ~is_class_method:false
         ~is_static_method:false
-        ~caller_class_interval:Interprocedural.ClassIntervalSet.top
-        ~receiver_class_interval:Interprocedural.ClassIntervalSet.top
+        ~call_info_intervals:Domains.CallInfoIntervals.top
         FunctionContext.string_combine_partial_sink_tree
     in
     let string_literal_taint = StringFormatCall.implicit_string_literal_sources string_literal in
@@ -2973,12 +2968,13 @@ module State (FunctionContext : FUNCTION_CONTEXT) = struct
                (* Provide leaf callable names when sources originate from parameters. *)
              ~arguments:[]
              ~port:parameter_root
-             ~is_self_call:false
-             ~is_cls_call:false
              ~is_class_method:false
              ~is_static_method:false
-             ~caller_class_interval:FunctionContext.caller_class_interval
-             ~receiver_class_interval:Interprocedural.ClassIntervalSet.top
+             ~call_info_intervals:
+               {
+                 Domains.CallInfoIntervals.top with
+                 caller_interval = FunctionContext.caller_class_interval;
+               }
       in
       let default_value_taint, state =
         match value with

source/interprocedural_analyses/taint/globalModel.ml

@@ -76,12 +76,9 @@ let get_source { models; resolution; location; interval } =
          ~callee:(Some target)
          ~arguments:[]
          ~port:AccessPath.Root.LocalResult
-         ~is_self_call:false
-         ~is_cls_call:false
          ~is_class_method:false
          ~is_static_method:false
-         ~caller_class_interval:interval
-         ~receiver_class_interval:Interprocedural.ClassIntervalSet.top
+         ~call_info_intervals:{ Domains.CallInfoIntervals.top with caller_interval = interval }
     |> ForwardState.Tree.join existing
   in
   List.fold ~init:ForwardState.Tree.bottom ~f:to_source models
@@ -103,12 +100,9 @@ let get_sinks { models; resolution; location; interval } =
            ~callee:(Some target)
            ~arguments:[]
            ~port:AccessPath.Root.LocalResult
-           ~is_self_call:false
-           ~is_cls_call:false
            ~is_class_method:false
            ~is_static_method:false
-           ~caller_class_interval:interval
-           ~receiver_class_interval:Interprocedural.ClassIntervalSet.top
+           ~call_info_intervals:{ Domains.CallInfoIntervals.top with caller_interval = interval }
     in
     { Domains.SinkTreeWithHandle.sink_tree; handle = IssueHandle.Sink.make_global ~call_target }
   in

source/interprocedural_analyses/taint/test/domainTest.ml

@@ -39,12 +39,9 @@ let test_partition_call_map context =
       ~port:AccessPath.Root.LocalResult
       ~path:[Abstract.TreeDomain.Label.create_name_index "a"]
       ~element:taint
-      ~is_self_call:false
-      ~is_cls_call:false
       ~is_class_method:false
       ~is_static_method:false
-      ~caller_class_interval:Interprocedural.ClassIntervalSet.top
-      ~receiver_class_interval:Interprocedural.ClassIntervalSet.top
+      ~call_info_intervals:Domains.CallInfoIntervals.top
   in
   let call_taint2 =
     ForwardTaint.apply_call
@@ -55,12 +52,9 @@ let test_partition_call_map context =
       ~port:AccessPath.Root.LocalResult
       ~path:[Abstract.TreeDomain.Label.create_name_index "a"]
       ~element:taint
-      ~is_self_call:false
-      ~is_cls_call:false
       ~is_class_method:false
       ~is_static_method:false
-      ~caller_class_interval:Interprocedural.ClassIntervalSet.top
-      ~receiver_class_interval:Interprocedural.ClassIntervalSet.top
+      ~call_info_intervals:Domains.CallInfoIntervals.top
   in
   let joined = ForwardTaint.join call_taint1 call_taint2 in
   assert_equal