Skip to content

Commit

Permalink
[pulse] Avoid destructing artificial variables related to coroutine
Browse files Browse the repository at this point in the history
Summary:
This diff avoids destructing the artificial variables that are related to coroutine, `__promise` and
`__coro_frame`.

https://github.com/llvm/llvm-project/blob/fc6faa1113e9069f41b5500db051210af0eea843/lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.cpp#L38-L39

Before this diff, they were addressed as if local variables, so that they were nullified/destructed when out of
scope, which introduced false positives of use-after-lifetime.

Reviewed By: ngorogiannis

Differential Revision: D52700500

fbshipit-source-id: 1a941acbae735eaa56c635ccedea6daa56978628
  • Loading branch information
skcho authored and facebook-github-bot committed Jan 12, 2024
1 parent 0dc82b0 commit 6e5d879
Show file tree
Hide file tree
Showing 9 changed files with 22 additions and 4 deletions.
2 changes: 2 additions & 0 deletions infer/src/IR/Mangled.ml
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,8 @@ let self = from_string "self"

let is_self = function {plain= "self"} -> true | _ -> false

let is_artificial = function {plain= "__promise" | "__coro_frame"} -> true | _ -> false

let return_param = from_string "__return_param"

let is_return_param = function {plain= "__return_param"} -> true | _ -> false
Expand Down
2 changes: 2 additions & 0 deletions infer/src/IR/Mangled.mli
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@ val self : t [@@warning "-unused-value-declaration"]

val is_self : t -> bool

val is_artificial : t -> bool

val return_param : t

val is_return_param : t -> bool
Expand Down
2 changes: 2 additions & 0 deletions infer/src/IR/Pvar.ml
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,8 @@ let is_this pvar = Mangled.is_this (get_name pvar)
(** Check if a pvar is the special "self" var *)
let is_self pvar = Mangled.is_self (get_name pvar)

let is_artificial pvar = Mangled.is_artificial (get_name pvar)

(** Check if the pvar is a return var *)
let is_return pv = Mangled.equal (get_name pv) Ident.name_return

Expand Down
3 changes: 3 additions & 0 deletions infer/src/IR/Pvar.mli
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,9 @@ val is_this : t -> bool
val is_self : t -> bool
(** Check if a pvar is the special "self" var *)

val is_artificial : t -> bool
(** Check if a pvar is an artificial variable related coroutine, "__promise" or "__coro_frame" *)

val is_frontend_tmp : t -> bool
(** return true if [pvar] is a temporary variable generated by the frontend *)

Expand Down
2 changes: 2 additions & 0 deletions infer/src/IR/Var.ml
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,8 @@ let is_none = function LogicalVar id -> Ident.is_none id | _ -> false

let is_this = function ProgramVar pv -> Pvar.is_this pv | LogicalVar _ -> false

let is_artificial = function ProgramVar pv -> Pvar.is_artificial pv | LogicalVar _ -> false

let get_all_vars_in_exp e =
let acc = Exp.free_vars e |> Sequence.map ~f:of_id in
Exp.program_vars e |> Sequence.map ~f:of_pvar |> Sequence.append acc
Expand Down
2 changes: 2 additions & 0 deletions infer/src/IR/Var.mli
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,8 @@ val is_none : t -> bool

val is_this : t -> bool

val is_artificial : t -> bool

val appears_in_source_code : t -> bool
(** return true if this variable appears in source code (i.e., is not a LogicalVar or a
frontend-generated ProgramVar) *)
Expand Down
7 changes: 5 additions & 2 deletions infer/src/backend/preanal.ml
Original file line number Diff line number Diff line change
Expand Up @@ -381,7 +381,9 @@ module Liveness = struct
let is_local pvar = not (Liveness.is_always_in_scope proc_desc pvar) in
let prepend_node_nullify_instructions loc pvars instrs =
List.fold pvars ~init:instrs ~f:(fun instrs pvar ->
if is_local pvar then Sil.Metadata (Nullify (pvar, loc)) :: instrs else instrs )
if is_local pvar && not (Pvar.is_artificial pvar) then
Sil.Metadata (Nullify (pvar, loc)) :: instrs
else instrs )
in
let node_deadvars_instruction loc vars =
let local_vars =
Expand All @@ -399,6 +401,7 @@ module Liveness = struct
let dead_vars, pvars_to_nullify =
VarDomain.fold
(fun var (dead_vars, pvars_to_nullify) ->
let dead_vars = if Var.is_artificial var then dead_vars else var :: dead_vars in
let pvars_to_nullify =
match Var.get_pvar var with
| Some pvar when not (AddressTaken.Domain.mem pvar address_taken_vars) ->
Expand All @@ -408,7 +411,7 @@ module Liveness = struct
| _ ->
pvars_to_nullify
in
(var :: dead_vars, pvars_to_nullify) )
(dead_vars, pvars_to_nullify) )
to_nullify ([], [])
in
let loc = Procdesc.Node.get_last_loc node in
Expand Down
4 changes: 3 additions & 1 deletion infer/src/pulse/Pulse.ml
Original file line number Diff line number Diff line change
Expand Up @@ -310,7 +310,9 @@ module PulseTransferFunctions = struct
if flags.cf_injected_destructor then
match (callee_pname, actuals) with
| Some (Procname.ObjC_Cpp pname), [(Exp.Lvar pvar, typ)]
when Pvar.is_local pvar && not (Procname.ObjC_Cpp.is_inner_destructor pname) ->
when Pvar.is_local pvar
&& (not (Procname.ObjC_Cpp.is_inner_destructor pname))
&& not (Pvar.is_artificial pvar) ->
(* ignore inner destructors, only trigger out of scope on the final destructor call *)
Some (pvar, typ)
| _ ->
Expand Down
2 changes: 1 addition & 1 deletion infer/src/pulse/PulseAbductiveDomain.ml
Original file line number Diff line number Diff line change
Expand Up @@ -972,7 +972,7 @@ module Internal = struct
else None )
in
match var with
| Var.ProgramVar pvar ->
| Var.ProgramVar pvar when not (Pvar.is_artificial pvar) ->
get_local_typ_opt pvar
|> Option.value_map ~default:acc
~f:(add_out_of_scope_attribute addr pvar location history acc)
Expand Down

0 comments on commit 6e5d879

Please sign in to comment.