Skip to content

Commit

Permalink
Introduce the module AccessPath.Path
Browse files Browse the repository at this point in the history
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
  • Loading branch information
arthaud authored and facebook-github-bot committed Jun 28, 2023
1 parent daa866b commit 53627dd
Show file tree
Hide file tree
Showing 12 changed files with 60 additions and 66 deletions.
26 changes: 19 additions & 7 deletions source/interprocedural_analyses/taint/accessPath.ml
Original file line number Diff line number Diff line change
Expand Up @@ -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 }]

Expand Down Expand Up @@ -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

Expand All @@ -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
Expand Down
16 changes: 11 additions & 5 deletions source/interprocedural_analyses/taint/accessPath.mli
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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]

Expand Down
8 changes: 4 additions & 4 deletions source/interprocedural_analyses/taint/callModel.ml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
4 changes: 2 additions & 2 deletions source/interprocedural_analyses/taint/callModel.mli
Original file line number Diff line number Diff line change
Expand Up @@ -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 ->
Expand All @@ -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 ->
Expand Down
34 changes: 6 additions & 28 deletions source/interprocedural_analyses/taint/domains.ml
Original file line number Diff line number Diff line change
Expand Up @@ -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[
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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 } ->
Expand Down Expand Up @@ -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 ->
Expand All @@ -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 ->
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions source/interprocedural_analyses/taint/features.ml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 1 addition & 2 deletions source/interprocedural_analyses/taint/issue.mli
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 1 addition & 2 deletions source/interprocedural_analyses/taint/model.mli
Original file line number Diff line number Diff line change
Expand Up @@ -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 ->
Expand Down
10 changes: 5 additions & 5 deletions source/interprocedural_analyses/taint/modelParseResult.ml
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down
14 changes: 7 additions & 7 deletions source/interprocedural_analyses/taint/modelParseResult.mli
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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

Expand Down
2 changes: 1 addition & 1 deletion source/interprocedural_analyses/taint/modelParser.mli
Original file line number Diff line number Diff line change
Expand Up @@ -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 ->
Expand Down
2 changes: 1 addition & 1 deletion source/interprocedural_analyses/taint/test/modelTest.ml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 53627dd

Please sign in to comment.