Skip to content

Commit

Permalink
[hack] Modify function reference arg only when needed
Browse files Browse the repository at this point in the history
Summary:
When `$f(...)` is invoked, we modify the receiver as,

```
lateBinding($f, ...)
===>
lateBinding($f.this, ...)
```

to pass a proper function reference to `lateBinding`.  However, this
modification was applied unconditionally, e.g.

```
hhbc_new_vec($f, ...)
===>
hhbc_new_vec($f.this, ...)
```

which introduced a wrong abstract semantics.

This diff changed it to modify the receiver only when the method name is `lateBinding`.

Reviewed By: geralt-encore

Differential Revision:
D66235651

Privacy Context Container: L1208441

fbshipit-source-id: 90ccfd77be393af2f1226e1da76246ab22541fc5
  • Loading branch information
skcho authored and facebook-github-bot committed Nov 20, 2024
1 parent f8c763e commit 92b31a1
Show file tree
Hide file tree
Showing 4 changed files with 32 additions and 16 deletions.
8 changes: 8 additions & 0 deletions infer/src/IR/Procname.ml
Original file line number Diff line number Diff line change
Expand Up @@ -587,6 +587,10 @@ module Hack = struct
{class_name= Some static_class_name; function_name= "_86constinit"; arity= Some arity}


let is_invoke {function_name= name} = String.equal name "__invoke"

let is_late_binding {function_name= name} = String.equal name "lateBinding"

let is_xinit {function_name= name} =
String.equal name "_86pinit" || String.equal name "_86cinit" || String.equal name "_86constinit"

Expand Down Expand Up @@ -997,6 +1001,10 @@ let is_hack_construct = function
false


let is_hack_invoke = function Hack classname -> Hack.is_invoke classname | _ -> false

let is_hack_late_binding = function Hack classname -> Hack.is_late_binding classname | _ -> false

let is_hack_xinit = function Hack classname -> Hack.is_xinit classname | _ -> false

let is_hack_internal procname =
Expand Down
16 changes: 10 additions & 6 deletions infer/src/IR/Procname.mli
Original file line number Diff line number Diff line change
Expand Up @@ -487,18 +487,22 @@ val is_erlang_call_unqualified : t -> bool

val is_erlang_call_qualified : t -> bool

val is_hack_builtins : t -> bool

val is_hack_constinit : t -> bool

val has_hack_classname : t -> bool

val is_hack_async_name : t -> bool
(* Checks if the function name starts with "gen", which is a (lint-checked) convention for it being async at Meta *)

val is_hack_construct : t -> bool
val is_hack_builtins : t -> bool

val is_hack_xinit : t -> bool
val is_hack_constinit : t -> bool

val is_hack_construct : t -> bool

val is_hack_internal : t -> bool
(* Check if the function is not a model nor one of the internal functions necessary for implementation *)

val is_hack_invoke : t -> bool

val is_hack_late_binding : t -> bool

val is_hack_xinit : t -> bool
19 changes: 9 additions & 10 deletions infer/src/pulse/Pulse.ml
Original file line number Diff line number Diff line change
Expand Up @@ -563,10 +563,11 @@ module PulseTransferFunctions = struct
(astate, func_args)


let modify_receiver_if_hack_function_reference path location astate func_args =
let modify_receiver_if_hack_function_reference path location astate callee_pname func_args =
let open IOption.Let_syntax in
( match func_args with
| ({ProcnameDispatcher.Call.FuncArg.arg_payload= value} as arg) :: args ->
| ({ProcnameDispatcher.Call.FuncArg.arg_payload= value} as arg) :: args
when Option.exists callee_pname ~f:Procname.is_hack_late_binding ->
let function_addr_hist = ValueOrigin.addr_hist value in
let* dynamic_type_name, _ = function_addr_hist |> fst |> get_dynamic_type_name astate in
if Typ.Name.Hack.is_generated_curry dynamic_type_name then
Expand All @@ -578,7 +579,7 @@ module PulseTransferFunctions = struct
in
(astate, {arg with arg_payload= ValueOrigin.unknown class_object} :: args)
else None
| [] ->
| _ ->
None )
|> Option.value ~default:(astate, func_args)

Expand All @@ -601,16 +602,14 @@ module PulseTransferFunctions = struct
else None


let is_closure_call opt_procname =
Option.exists opt_procname ~f:(fun procname ->
Language.curr_language_is Hack && String.equal (Procname.get_method procname) "__invoke" )


let lookup_virtual_method_info {InterproceduralAnalysis.tenv; proc_desc; exe_env} path func_args
call_loc astate callee_pname default_info =
let caller = Procdesc.get_proc_name proc_desc in
let record_call_resolution_if_closure resolution astate =
if Config.pulse_monitor_transitive_callees && is_closure_call callee_pname then
if
Config.pulse_monitor_transitive_callees
&& Option.exists callee_pname ~f:Procname.is_hack_invoke
then
AbductiveDomain.record_call_resolution ~caller:proc_desc call_loc Closure resolution astate
else astate
in
Expand Down Expand Up @@ -767,7 +766,7 @@ module PulseTransferFunctions = struct
else (astate, callee_pname, func_args)
in
let astate, func_args =
modify_receiver_if_hack_function_reference path call_loc astate func_args
modify_receiver_if_hack_function_reference path call_loc astate callee_pname func_args
in
let astate =
match (callee_pname, func_args) with
Expand Down
5 changes: 5 additions & 0 deletions infer/tests/codetoanalyze/hack/pulse/uninit_method.hack
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,11 @@ class MyClass implements MyInterface {
}

final class MyFinalClass extends MyClass {
public function closure_in_vector_ok(): void {
foreach (vec[self::foo<>] as $f) {
$_ = $f();
}
}
}

final class MyFinalUnknownClass extends UnknownClass {
Expand Down

0 comments on commit 92b31a1

Please sign in to comment.