From 53627dd105d9debee7feb34858079a569e7f11b2 Mon Sep 17 00:00:00 2001 From: Maxime Arthaud Date: Wed, 28 Jun 2023 12:23:20 -0700 Subject: [PATCH] Introduce the module AccessPath.Path Summary: Let's introduce a module `AccessPath.Path`, which is just an alias for `Abstract.TreeDomain.Label.path`. This simplifies a bit our codebase, since we don't have to use `pp_path`/`equal_path`/`compare_path`/etc anymore. `AccessPath.Path` feels also more idiomatic. Reviewed By: tianhan0 Differential Revision: D47095719 fbshipit-source-id: aa856fba96fd9e1eaeb8b8c7405236f5c9681c25 --- .../taint/accessPath.ml | 26 ++++++++++---- .../taint/accessPath.mli | 16 ++++++--- .../taint/callModel.ml | 8 ++--- .../taint/callModel.mli | 4 +-- .../interprocedural_analyses/taint/domains.ml | 34 ++++--------------- .../taint/features.ml | 4 +-- .../interprocedural_analyses/taint/issue.mli | 3 +- .../interprocedural_analyses/taint/model.mli | 3 +- .../taint/modelParseResult.ml | 10 +++--- .../taint/modelParseResult.mli | 14 ++++---- .../taint/modelParser.mli | 2 +- .../taint/test/modelTest.ml | 2 +- 12 files changed, 60 insertions(+), 66 deletions(-) diff --git a/source/interprocedural_analyses/taint/accessPath.ml b/source/interprocedural_analyses/taint/accessPath.ml index 4e0cda22812..cd7e6e24230 100644 --- a/source/interprocedural_analyses/taint/accessPath.ml +++ b/source/interprocedural_analyses/taint/accessPath.ml @@ -117,10 +117,24 @@ module Root = struct module Set = Caml.Set.Make (T) end +module Path = struct + type t = Abstract.TreeDomain.Label.t list + + let pp = Abstract.TreeDomain.Label.pp_path + + let show = Abstract.TreeDomain.Label.show_path + + let compare = Abstract.TreeDomain.Label.compare_path ?cmp:None + + let equal = Abstract.TreeDomain.Label.equal_path + + let is_prefix = Abstract.TreeDomain.Label.is_prefix +end + type argument_match = { root: Root.t; - actual_path: Abstract.TreeDomain.Label.path; - formal_path: Abstract.TreeDomain.Label.path; + actual_path: Path.t; + formal_path: Path.t; } [@@deriving compare, show { with_path = false }] @@ -259,13 +273,11 @@ let match_actuals_to_formals arguments roots = type t = { root: Root.t; - path: Abstract.TreeDomain.Label.path; + path: Path.t; } [@@deriving compare] -let pp formatter { root; path } = - Format.fprintf formatter "%a%a" Root.pp root Abstract.TreeDomain.Label.pp_path path - +let pp formatter { root; path } = Format.fprintf formatter "%a%a" Root.pp root Path.pp path let show access_path = Format.asprintf "%a" pp access_path @@ -281,7 +293,7 @@ let get_index expression = | None -> Abstract.TreeDomain.Label.AnyIndex -let to_json { root; path } = `String (Root.to_string root ^ Abstract.TreeDomain.Label.show_path path) +let to_json { root; path } = `String (Root.to_string root ^ Path.show path) let of_expression expression = let rec of_expression path = function diff --git a/source/interprocedural_analyses/taint/accessPath.mli b/source/interprocedural_analyses/taint/accessPath.mli index e94d8dd7f77..388c958aeb8 100644 --- a/source/interprocedural_analyses/taint/accessPath.mli +++ b/source/interprocedural_analyses/taint/accessPath.mli @@ -33,15 +33,21 @@ module Root : sig module Set : Caml.Set.S with type elt = t end +module Path : sig + type t = Abstract.TreeDomain.Label.t list [@@deriving compare, eq, show] + + val is_prefix : prefix:t -> t -> bool +end + type t = { root: Root.t; - path: Abstract.TreeDomain.Label.path; + path: Path.t; } [@@deriving show, compare] -val create : Root.t -> Abstract.TreeDomain.Label.path -> t +val create : Root.t -> Path.t -> t -val extend : t -> path:Abstract.TreeDomain.Label.path -> t +val extend : t -> path:Path.t -> t val of_expression : Expression.t -> t option @@ -51,8 +57,8 @@ val to_json : t -> Yojson.Safe.t type argument_match = { root: Root.t; - actual_path: Abstract.TreeDomain.Label.path; - formal_path: Abstract.TreeDomain.Label.path; + actual_path: Path.t; + formal_path: Path.t; } [@@deriving compare, show] diff --git a/source/interprocedural_analyses/taint/callModel.ml b/source/interprocedural_analyses/taint/callModel.ml index b22432011ef..7c65797648b 100644 --- a/source/interprocedural_analyses/taint/callModel.ml +++ b/source/interprocedural_analyses/taint/callModel.ml @@ -340,8 +340,8 @@ module ExtraTraceForTransforms = struct let from_sink_trees ~argument_access_path ~named_transforms ~tito_roots ~sink_trees = let accumulate_extra_traces_from_sink_path (path, tip) so_far = let is_prefix = - Abstract.TreeDomain.Label.is_prefix ~prefix:path argument_access_path - || Abstract.TreeDomain.Label.is_prefix ~prefix:argument_access_path path + AccessPath.Path.is_prefix ~prefix:path argument_access_path + || AccessPath.Path.is_prefix ~prefix:argument_access_path path in if not is_prefix then so_far @@ -367,8 +367,8 @@ module ExtraTraceForTransforms = struct let remove_sink (sink_path, sink_tip) = let find_tito (tito_path, _) so_far = so_far - || Abstract.TreeDomain.Label.is_prefix ~prefix:tito_path sink_path - || Abstract.TreeDomain.Label.is_prefix ~prefix:sink_path tito_path + || AccessPath.Path.is_prefix ~prefix:tito_path sink_path + || AccessPath.Path.is_prefix ~prefix:sink_path tito_path in let exist_tito = BackwardState.Tree.fold BackwardState.Tree.Path ~f:find_tito ~init:false tito_tree diff --git a/source/interprocedural_analyses/taint/callModel.mli b/source/interprocedural_analyses/taint/callModel.mli index 7b71084d284..87bd83e8d14 100644 --- a/source/interprocedural_analyses/taint/callModel.mli +++ b/source/interprocedural_analyses/taint/callModel.mli @@ -67,7 +67,7 @@ val taint_in_taint_out_mapping val return_paths_and_collapse_depths : kind:Sinks.t -> tito_taint:BackwardTaint.t -> - (Abstract.TreeDomain.Label.path * Features.CollapseDepth.t) list + (AccessPath.Path.t * Features.CollapseDepth.t) list val sink_trees_of_argument : resolution:Resolution.t -> @@ -88,7 +88,7 @@ module ExtraTraceForTransforms : sig (* Collect sink taints that will be used as first hops of extra traces, i.e., whose call info matches the given callee roots and whose taint match the given named transforms *) val from_sink_trees - : argument_access_path:Abstract.TreeDomain.Label.path -> + : argument_access_path:AccessPath.Path.t -> named_transforms:TaintTransform.t list -> tito_roots:AccessPath.Root.Set.t -> sink_trees:Domains.SinkTreeWithHandle.t list -> diff --git a/source/interprocedural_analyses/taint/domains.ml b/source/interprocedural_analyses/taint/domains.ml index da3e07a85df..c51d60d555e 100644 --- a/source/interprocedural_analyses/taint/domains.ml +++ b/source/interprocedural_analyses/taint/domains.ml @@ -16,7 +16,7 @@ * ForwardState: Map[ * key: AccessPath.Root = Variable|NamedParameter|..., * value: Tree[ - * edges: Abstract.TreeDomain.Label.path = Index of string|Field of string|..., + * edges: Abstract.TreeDomain.Label.t = Index of string|Field of string|..., * nodes: ForwardTaint: Map[ * key: CallInfo = Declaration|Origin of location|CallSite of {port; path; callees}, * value: LocalTaint: Tuple[ @@ -87,7 +87,7 @@ module CallInfo = struct (* Taint propagated from a call. *) | CallSite of { port: AccessPath.Root.t; - path: Abstract.TreeDomain.Label.path; + path: AccessPath.Path.t; location: Location.WithModule.t; callees: Target.t list; } @@ -154,26 +154,6 @@ module CallInfo = struct "call", call_json - let less_or_equal ~left ~right = - match left, right with - | ( CallSite - { path = path_left; location = location_left; port = port_left; callees = callees_left }, - CallSite - { - path = path_right; - location = location_right; - port = port_right; - callees = callees_right; - } ) -> - [%compare.equal: AccessPath.Root.t] port_left port_right - && Location.WithModule.compare location_left location_right = 0 - && [%compare.equal: Target.t list] callees_left callees_right - && Abstract.TreeDomain.Label.compare_path path_right path_left = 0 - | _ -> [%compare.equal: t] left right - - - let widen set = set - let strip_for_callsite = function | Origin _ -> Origin Location.WithModule.any | CallSite { port; path; location = _; callees } -> @@ -566,7 +546,7 @@ module type TAINT_DOMAIN = sig callee:Target.t option -> arguments:Ast.Expression.Call.Argument.t list -> port:AccessPath.Root.t -> - path:Abstract.TreeDomain.Label.path -> + path:AccessPath.Path.t -> element:t -> is_self_call:bool -> caller_class_interval:ClassIntervalSet.t -> @@ -578,8 +558,7 @@ module type TAINT_DOMAIN = sig val to_json : expand_overrides:OverrideGraph.SharedMemory.t option -> - is_valid_callee: - (port:AccessPath.Root.t -> path:Abstract.TreeDomain.Label.path -> callee:Target.t -> bool) -> + is_valid_callee:(port:AccessPath.Root.t -> path:AccessPath.Path.t -> callee:Target.t -> bool) -> filename_lookup:(Reference.t -> string option) option -> export_leaf_names:ExportLeafNames.t -> t -> @@ -1282,7 +1261,7 @@ end = struct in root_name |> Option.map ~f:(fun root -> - Format.asprintf "leaf:%s%a" root Abstract.TreeDomain.Label.pp_path path) + Format.asprintf "leaf:%s%a" root AccessPath.Path.pp path) in LeafName.{ leaf = Target.external_name callee; port } |> LeafNameInterned.intern in @@ -1631,8 +1610,7 @@ module MakeTaintEnvironment (Taint : TAINT_DOMAIN) () = struct in let ports = Tree.fold Tree.Path ~f:path_to_json tree ~init:[] - |> List.dedup_and_sort ~compare:(fun (p1, _) (p2, _) -> - Abstract.TreeDomain.Label.compare_path p1 p2) + |> List.dedup_and_sort ~compare:(fun (p1, _) (p2, _) -> AccessPath.Path.compare p1 p2) |> List.rev_map ~f:(fun (_, fields) -> `Assoc fields) in List.rev_append ports json_list diff --git a/source/interprocedural_analyses/taint/features.ml b/source/interprocedural_analyses/taint/features.ml index f1602af33d1..06e2c714aaf 100644 --- a/source/interprocedural_analyses/taint/features.ml +++ b/source/interprocedural_analyses/taint/features.ml @@ -453,9 +453,9 @@ module CollapseDepth = struct end module ReturnAccessPath = struct - type t = Abstract.TreeDomain.Label.t list + type t = AccessPath.Path.t - let show = Abstract.TreeDomain.Label.show_path + let show = AccessPath.Path.show end module ReturnAccessPathTree = struct diff --git a/source/interprocedural_analyses/taint/issue.mli b/source/interprocedural_analyses/taint/issue.mli index a54c2bb9472..f02ec140d7a 100644 --- a/source/interprocedural_analyses/taint/issue.mli +++ b/source/interprocedural_analyses/taint/issue.mli @@ -39,8 +39,7 @@ val canonical_location : t -> Location.WithModule.t val to_json : taint_configuration:TaintConfiguration.Heap.t -> expand_overrides:OverrideGraph.SharedMemory.t option -> - is_valid_callee: - (port:AccessPath.Root.t -> path:Abstract.TreeDomain.Label.path -> callee:Target.t -> bool) -> + is_valid_callee:(port:AccessPath.Root.t -> path:AccessPath.Path.t -> callee:Target.t -> bool) -> filename_lookup:(Reference.t -> string option) -> t -> Yojson.Safe.t diff --git a/source/interprocedural_analyses/taint/model.mli b/source/interprocedural_analyses/taint/model.mli index be5e397b455..d1f8c9d7931 100644 --- a/source/interprocedural_analyses/taint/model.mli +++ b/source/interprocedural_analyses/taint/model.mli @@ -110,8 +110,7 @@ val should_externalize : t -> bool val to_json : expand_overrides:OverrideGraph.SharedMemory.t option -> - is_valid_callee: - (port:AccessPath.Root.t -> path:Abstract.TreeDomain.Label.path -> callee:Target.t -> bool) -> + is_valid_callee:(port:AccessPath.Root.t -> path:AccessPath.Path.t -> callee:Target.t -> bool) -> filename_lookup:(Ast.Reference.t -> string option) option -> export_leaf_names:ExportLeafNames.t -> Target.t -> diff --git a/source/interprocedural_analyses/taint/modelParseResult.ml b/source/interprocedural_analyses/taint/modelParseResult.ml index 31f62e25391..4738215f670 100644 --- a/source/interprocedural_analyses/taint/modelParseResult.ml +++ b/source/interprocedural_analyses/taint/modelParseResult.ml @@ -43,10 +43,10 @@ module TaintFeatures = struct type t = { breadcrumbs: Features.Breadcrumb.t list; via_features: Features.ViaFeature.t list; - applies_to: Abstract.TreeDomain.Label.path option; - parameter_path: Abstract.TreeDomain.Label.path option; - return_path: Abstract.TreeDomain.Label.path option; - update_path: Abstract.TreeDomain.Label.path option; + applies_to: AccessPath.Path.t option; + parameter_path: AccessPath.Path.t option; + return_path: AccessPath.Path.t option; + update_path: AccessPath.Path.t option; leaf_names: Features.LeafName.t list; leaf_name_provided: bool; trace_length: int option; @@ -141,7 +141,7 @@ module TaintFeatures = struct | None -> features in let add_path_option ~name path features = - add_option ~name ~pp:Abstract.TreeDomain.Label.pp_path path features + add_option ~name ~pp:AccessPath.Path.pp path features in let add_collapse_depth features = match collapse_depth with diff --git a/source/interprocedural_analyses/taint/modelParseResult.mli b/source/interprocedural_analyses/taint/modelParseResult.mli index b6ed3cb12a0..1313ecdfe51 100644 --- a/source/interprocedural_analyses/taint/modelParseResult.mli +++ b/source/interprocedural_analyses/taint/modelParseResult.mli @@ -30,10 +30,10 @@ module TaintFeatures : sig type t = { breadcrumbs: Features.Breadcrumb.t list; via_features: Features.ViaFeature.t list; - applies_to: Abstract.TreeDomain.Label.path option; - parameter_path: Abstract.TreeDomain.Label.path option; - return_path: Abstract.TreeDomain.Label.path option; - update_path: Abstract.TreeDomain.Label.path option; + applies_to: AccessPath.Path.t option; + parameter_path: AccessPath.Path.t option; + return_path: AccessPath.Path.t option; + update_path: AccessPath.Path.t option; leaf_names: Features.LeafName.t list; leaf_name_provided: bool; trace_length: int option; @@ -68,11 +68,11 @@ module TaintKindsWithFeatures : sig val from_via_feature : Features.ViaFeature.t -> t - val from_parameter_path : Abstract.TreeDomain.Label.path -> t + val from_parameter_path : AccessPath.Path.t -> t - val from_return_path : Abstract.TreeDomain.Label.path -> t + val from_return_path : AccessPath.Path.t -> t - val from_update_path : Abstract.TreeDomain.Label.path -> t + val from_update_path : AccessPath.Path.t -> t val from_collapse_depth : CollapseDepth.t -> t diff --git a/source/interprocedural_analyses/taint/modelParser.mli b/source/interprocedural_analyses/taint/modelParser.mli index 2b78423b45b..60154fbbaa0 100644 --- a/source/interprocedural_analyses/taint/modelParser.mli +++ b/source/interprocedural_analyses/taint/modelParser.mli @@ -39,7 +39,7 @@ val parse_access_path : path:PyrePath.t option -> location:Ast.Location.t -> Ast.Expression.t -> - (Abstract.TreeDomain.Label.path, ModelVerificationError.t) result + (AccessPath.Path.t, ModelVerificationError.t) result val parse_decorator_modes : path:PyrePath.t -> diff --git a/source/interprocedural_analyses/taint/test/modelTest.ml b/source/interprocedural_analyses/taint/test/modelTest.ml index 283d0ec246c..ae30123d241 100644 --- a/source/interprocedural_analyses/taint/test/modelTest.ml +++ b/source/interprocedural_analyses/taint/test/modelTest.ml @@ -7206,7 +7206,7 @@ let test_access_path _ = assert_bool (Format.asprintf "Unexpected access path error: %a" ModelVerificationError.pp error) false - | Ok path -> assert_equal ~printer:Label.show_path ~cmp:Label.equal_path expected path + | Ok path -> assert_equal ~printer:AccessPath.Path.show ~cmp:AccessPath.Path.equal expected path in let assert_invalid_path ~source ~expected = match parse_access_path source with