Skip to content

Commit

Permalink
[infer][hack][textual] fixing the bad optionnal-args wrappers
Browse files Browse the repository at this point in the history
Summary:
for each Hack virtual function with optionnal arguments, hackc generates
wrappers of the form
```
define .wrapper enclosing.name($this, $arg0, ..., $arg_k) : ... {
#b0:
  // computes default values and store them in $arg_{k+1}, ..., $arg_{k+q}
...
#b_last:
  n0 = load &$arg0
  ...
  n_{k+q} = load &$arg_q
  n_{k+q+1} = load &$this
  n_{k+q+2} = n_{k+q+1+1}.?.name(n0, ..., n_{k+q+1})
  ret n_{k+q+2}
}
```
This forward call should be static, not virtual. We implement a small Textual->Textual transformation that
replaces it with a static call using the enclosing name found in the signature of the wrapper.

Reviewed By: jvillard

Differential Revision:
D66965276

Privacy Context Container: L1208441

fbshipit-source-id: 46dcf86a543890562032542fa31ebd4e49f7ee91
  • Loading branch information
davidpichardie authored and facebook-github-bot committed Dec 11, 2024
1 parent 0fb7520 commit 5d753cb
Show file tree
Hide file tree
Showing 2 changed files with 46 additions and 5 deletions.
47 changes: 46 additions & 1 deletion infer/src/textual/TextualTransform.ml
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,51 @@ end

let fix_closure_app = FixClosureAppExpr.transform

module FixHackWrapper = struct
let is_forward_to_static_method_wrapper (pdesc : ProcDesc.t) =
(* hackc generates two kinds of wrapper but we are not interested in the
forward-to-static-method case *)
Int.equal (List.length pdesc.nodes) 1


let replace_forward_call ({procdecl; nodes} as pdesc : ProcDesc.t) =
(* we expect the last node to be of the form:
n0: *HackMixed = load &$param1
n1: *HackMixed = load &$param2
...
n_{k+1} = n_k.?.name(n0, n1, ...)
ret n_{k+1}
*)
let nodes =
match List.rev nodes with
| [] ->
[]
| last_node :: others ->
let instrs =
List.map last_node.Node.instrs ~f:(function
| Instr.Let {id; exp= Exp.Call {proc; args; kind= Virtual}; loc} ->
let enclosing_class = procdecl.qualified_name.enclosing_class in
let proc : QualifiedProcName.t = {proc with enclosing_class} in
let instr = Instr.Let {id; exp= Exp.Call {proc; args; kind= NonVirtual}; loc} in
instr
| instr ->
instr )
in
{last_node with Node.instrs} :: others |> List.rev
in
{pdesc with nodes}


let transform module_ =
module_map_procs module_ ~f:(fun pdesc : ProcDesc.t ->
let has_wrapper_attr =
List.exists pdesc.procdecl.attributes ~f:Textual.Attr.is_hack_wrapper
in
if has_wrapper_attr && not (is_forward_to_static_method_wrapper pdesc) then
replace_forward_call pdesc
else pdesc )
end

module Subst = struct
let rec of_exp_one exp ~id ~by =
let open Exp in
Expand Down Expand Up @@ -900,5 +945,5 @@ let run lang module_ =
let decls_env =
if new_decls_were_added then TextualDecls.make_decls module_ |> snd else decls_env
in
let module_ = module_ |> let_propagation |> out_of_ssa in
let module_ = module_ |> let_propagation |> FixHackWrapper.transform |> out_of_ssa in
(module_, decls_env)
4 changes: 0 additions & 4 deletions infer/tests/codetoanalyze/hack/pulse/issues.exp
Original file line number Diff line number Diff line change
Expand Up @@ -217,10 +217,6 @@ propagators.hack, Propagators::Flows$static.propWithSinkToSinkBad, 2, TAINT_ERRO
propagators.hack, Propagators::Flows$static.propWithSinkToSinkBad, 3, TAINT_ERROR, no_bucket, ERROR, [source of the taint here: value returned from `Propagators::Source$static.getTainted` with kind `Propagators`,flows to this sink: value passed as argument `#0` to `Propagators::Sink$static.process` with kind `Propagators`], source: Propagators::Source$static.getTainted, sink: Propagators::Sink$static.process, tainted expression: $t1
recursion.hack, Recursion::Node.infinite_bad, 1, MUTUAL_RECURSION_CYCLE, no_bucket, WARNING, [`Recursion::Node.infinite_bad` makes a recursive call to `Recursion::Node.infinite_bad`]
recursion.hack, Recursion::Node.infinite_same_args_bad, 1, INFINITE_RECURSION, no_bucket, WARNING, [`Recursion::Node.infinite_same_args_bad` makes a recursive call to `Recursion::Node.infinite_same_args_bad` with the same argument values]
recursion.hack, Recursion::AbstractA.__construct, 0, MUTUAL_RECURSION_CYCLE, no_bucket, WARNING, [`Recursion::AbstractA.__construct` calls `Recursion::B.__construct`,`Recursion::B.__construct` makes a recursive call to `Recursion::AbstractA.__construct`]
recursion.hack, Recursion::AbstractA.FP_optional_arg_in_hierarchy_ok, 0, MUTUAL_RECURSION_CYCLE, no_bucket, WARNING, [`Recursion::AbstractA.FP_optional_arg_in_hierarchy_ok` calls `Recursion::B.FP_optional_arg_in_hierarchy_ok`,`Recursion::B.FP_optional_arg_in_hierarchy_ok` makes a recursive call to `Recursion::AbstractA.FP_optional_arg_in_hierarchy_ok`]
recursion.hack, Recursion::B.__construct, 1, MUTUAL_RECURSION_CYCLE, no_bucket, WARNING, [`Recursion::B.__construct` calls `Recursion::AbstractA.__construct`,`Recursion::AbstractA.__construct` makes a recursive call to `Recursion::B.__construct`]
recursion.hack, Recursion::B.FP_optional_arg_in_hierarchy_ok, 1, MUTUAL_RECURSION_CYCLE, no_bucket, WARNING, [`Recursion::B.FP_optional_arg_in_hierarchy_ok` calls `Recursion::AbstractA.FP_optional_arg_in_hierarchy_ok`,`Recursion::AbstractA.FP_optional_arg_in_hierarchy_ok` makes a recursive call to `Recursion::B.FP_optional_arg_in_hierarchy_ok`]
recursion.hack, Recursion::AA$static.something, 1, INFINITE_RECURSION, no_bucket, WARNING, [`Recursion::AA$static.something` calls `Recursion::AA$static.something_else`,`Recursion::AA$static.something_else` calls `Recursion::BB$static.something`,`Recursion::BB$static.something` calls `Recursion::BB$static.something_else`,`Recursion::BB$static.something_else` makes a recursive call to `Recursion::AA$static.something` with the same argument values]
recursion.hack, Recursion::AA$static.something_else, 1, INFINITE_RECURSION, no_bucket, WARNING, [`Recursion::AA$static.something_else` calls `Recursion::BB$static.something`,`Recursion::BB$static.something` calls `Recursion::BB$static.something_else`,`Recursion::BB$static.something_else` calls `Recursion::AA$static.something`,`Recursion::AA$static.something` makes a recursive call to `Recursion::AA$static.something_else` with the same argument values]
recursion.hack, Recursion::BB$static.something, 1, INFINITE_RECURSION, no_bucket, WARNING, [`Recursion::BB$static.something` calls `Recursion::BB$static.something_else`,`Recursion::BB$static.something_else` calls `Recursion::AA$static.something`,`Recursion::AA$static.something` calls `Recursion::AA$static.something_else`,`Recursion::AA$static.something_else` makes a recursive call to `Recursion::BB$static.something` with the same argument values]
Expand Down

0 comments on commit 5d753cb

Please sign in to comment.