[pulse] better treatment of variables going out of scope

Summary:
Detect when a variable goes out of scope. When that's the case, mark its
address *and* its contents as invalid.

Give subsequent uses a USE_AFTER_LIFETIME error type instead of
USE_AFTER_DESTRUCTOR.

Reviewed By: jberdine

Differential Revision: D14387147

fbshipit-source-id: a2c530fda

infer/src/base/IssueType.ml

@@ -417,8 +417,6 @@ let unsafe_guarded_by_access = from_string "UNSAFE_GUARDED_BY_ACCESS"
 
 let use_after_delete = from_string "USE_AFTER_DELETE"
 
-let use_after_destructor = from_string "USE_AFTER_DESTRUCTOR"
-
 let use_after_free = from_string "USE_AFTER_FREE"
 
 let use_after_lifetime = from_string "USE_AFTER_LIFETIME"

infer/src/base/IssueType.mli

@@ -311,8 +311,6 @@ val unsafe_guarded_by_access : t
 
 val use_after_delete : t
 
-val use_after_destructor : t
-
 val use_after_free : t
 
 val use_after_lifetime : t

infer/src/pulse/Pulse.ml

@@ -51,17 +51,6 @@ module PulseTransferFunctions = struct
 
   type extras = Summary.t
 
-  let is_destructor = function
-    | Typ.Procname.ObjC_Cpp clang_pname ->
-        Typ.Procname.ObjC_Cpp.is_destructor clang_pname
-        && not
-             (* Our frontend generates synthetic inner destructors to model invocation of base class
-           destructors correctly; see D5834555/D7189239 *)
-             (Typ.Procname.ObjC_Cpp.is_inner_destructor clang_pname)
-    | _ ->
-        false
-
-
   let rec exec_assign summary (lhs_access : HilExp.AccessExpression.t) (rhs_exp : HilExp.t) loc
       astate =
     (* try to evaluate [rhs_exp] down to an abstract address or generate a fresh one *)
@@ -150,27 +139,31 @@ module PulseTransferFunctions = struct
 
 
   (** has an object just gone out of scope? *)
-  let get_destructed_object (call : HilInstr.call) (actuals : HilExp.t list) (flags : CallFlags.t)
+  let get_out_of_scope_object (call : HilInstr.call) (actuals : HilExp.t list)
-      =
+      (flags : CallFlags.t) =
     (* injected destructors are precisely inserted where an object goes out of scope *)
     if flags.cf_injected_destructor then
       match (call, actuals) with
-      | Direct callee_pname, [AccessExpression destroyed_access] when is_destructor callee_pname ->
+      | ( Direct (Typ.Procname.ObjC_Cpp pname)
-          Some (callee_pname, destroyed_access)
+        , [AccessExpression (*AddressOf (Base _) as *) destroyed_access] )
+        when not (Typ.Procname.ObjC_Cpp.is_inner_destructor pname) ->
+          (* ignore inner destructors, only trigger out of scope on the final destructor call *)
+          Some destroyed_access
       | _ ->
           None
     else None
 
 
-  (** [destroyed_access_expr] has just gone out of scope and in now invalid *)
+  (** [out_of_scope_access_expr] has just gone out of scope and in now invalid *)
-  let destruct_object callee_pname call_loc destroyed_access_expr astate =
+  let exec_object_out_of_scope call_loc out_of_scope_access_expr astate =
-    let destroyed_object = HilExp.AccessExpression.dereference destroyed_access_expr in
+    (* invalidate both [&x] and [x]: reading either is now forbidden *)
-    (* TODO: need an [OutOfScope] invalidation reason instead of [CppDestructor]? *)
+    let invalidate access_expr =
-    PulseOperations.invalidate
+      PulseOperations.invalidate
-      (PulseTrace.Immediate
+        (PulseTrace.Immediate {imm= GoneOutOfScope (access_expr, call_loc); location= call_loc})
-         { imm= PulseInvalidation.CppDestructor (callee_pname, destroyed_object, call_loc)
+        call_loc access_expr
-         ; location= call_loc })
+    in
-      call_loc destroyed_object astate
+    invalidate (HilExp.AccessExpression.dereference out_of_scope_access_expr) astate
+    >>= invalidate out_of_scope_access_expr
 
 
   let dispatch_call summary ret (call : HilInstr.call) (actuals : HilExp.t list) flags call_loc
@@ -191,13 +184,12 @@ module PulseTransferFunctions = struct
         PerfEvent.(log (fun logger -> log_begin_event logger ~name:"pulse interproc call" ())) ;
         let posts = interprocedural_call summary ret call actuals flags call_loc astate in
         PerfEvent.(log (fun logger -> log_end_event logger ())) ;
-        match get_destructed_object call actuals flags with
+        match get_out_of_scope_object call actuals flags with
-        | Some (destructor_pname, access_expr) ->
+        | Some access_expr ->
             L.d_printfln "%a is going out of scope" HilExp.AccessExpression.pp access_expr ;
             posts
             >>= fun posts ->
-            List.map posts ~f:(fun astate ->
+            List.map posts ~f:(fun astate -> exec_object_out_of_scope call_loc access_expr astate)
-                destruct_object destructor_pname call_loc access_expr astate )
             |> Result.all
         | None ->
             posts )

infer/src/pulse/PulseInvalidation.ml

@@ -40,7 +40,7 @@ let pp_std_vector_function f = function
 type t =
   | CFree of HilExp.AccessExpression.t * Location.t
   | CppDelete of HilExp.AccessExpression.t * Location.t
-  | CppDestructor of Typ.Procname.t * HilExp.AccessExpression.t * Location.t
+  | GoneOutOfScope of HilExp.AccessExpression.t * Location.t
   | Nullptr
   | StdVector of std_vector_function * HilExp.AccessExpression.t * Location.t
 [@@deriving compare]
@@ -50,8 +50,8 @@ let issue_type_of_cause = function
       IssueType.use_after_free
   | CppDelete _ ->
       IssueType.use_after_delete
-  | CppDestructor _ ->
+  | GoneOutOfScope _ ->
-      IssueType.use_after_destructor
+      IssueType.use_after_lifetime
   | Nullptr ->
       IssueType.null_dereference
   | StdVector _ ->
@@ -63,9 +63,8 @@ let pp f = function
       F.fprintf f "by call to `free()` on `%a`" HilExp.AccessExpression.pp access_expr
   | CppDelete (access_expr, _) ->
       F.fprintf f "by `delete` on `%a`" HilExp.AccessExpression.pp access_expr
-  | CppDestructor (proc_name, access_expr, _) ->
+  | GoneOutOfScope (access_expr, _) ->
-      F.fprintf f "by destructor call `%a()` on `%a`" Typ.Procname.pp proc_name
+      F.fprintf f "`%a` gone out of scope" HilExp.AccessExpression.pp access_expr
-        HilExp.AccessExpression.pp access_expr
   | Nullptr ->
       F.fprintf f "null pointer"
   | StdVector (std_vector_f, access_expr, _) ->

infer/src/pulse/PulseInvalidation.mli

@@ -22,7 +22,7 @@ val pp_std_vector_function : Format.formatter -> std_vector_function -> unit
 type t =
   | CFree of HilExp.AccessExpression.t * Location.t
   | CppDelete of HilExp.AccessExpression.t * Location.t
-  | CppDestructor of Typ.Procname.t * HilExp.AccessExpression.t * Location.t
+  | GoneOutOfScope of HilExp.AccessExpression.t * Location.t
   | Nullptr
   | StdVector of std_vector_function * HilExp.AccessExpression.t * Location.t
 [@@deriving compare]

infer/tests/codetoanalyze/cpp/pulse/closures.cpp

@@ -128,7 +128,7 @@ int ref_capture_return_local_lambda_ok() {
   return f().f;
 }
 
-S& ref_capture_return_local_lambda_bad() {
+S& FN_ref_capture_return_local_lambda_bad() {
   S x;
   auto f = [&x](void) -> S& {
     // no way to know if ok here

infer/tests/codetoanalyze/cpp/pulse/issues.exp

@@ -1,12 +1,16 @@
+codetoanalyze/cpp/pulse/basics.cpp, aggregate_reassign2_ok, 5, USE_AFTER_LIFETIME, no_bucket, ERROR, [variable declared,invalidated `&(a)` gone out of scope here,accessed `a.str` here]
+codetoanalyze/cpp/pulse/basics.cpp, aggregate_reassign3_ok, 5, USE_AFTER_LIFETIME, no_bucket, ERROR, [variable declared,invalidated `&(a)` gone out of scope here,accessed `a.a.str` here]
+codetoanalyze/cpp/pulse/basics.cpp, aggregate_reassign_ok, 4, USE_AFTER_LIFETIME, no_bucket, ERROR, [variable declared,invalidated `&(s)` gone out of scope here,accessed `s.i` here]
 codetoanalyze/cpp/pulse/basics.cpp, multiple_invalidations_branch_bad, 6, USE_AFTER_DELETE, no_bucket, ERROR, [invalidated by `delete` on `ptr` here,accessed `*(ptr)` here]
 codetoanalyze/cpp/pulse/basics.cpp, multiple_invalidations_loop_bad, 3, USE_AFTER_DELETE, no_bucket, ERROR, [invalidated by `delete` on `ptr` here,accessed `ptr` here]
-codetoanalyze/cpp/pulse/closures.cpp, implicit_ref_capture_destroy_invoke_bad, 6, USE_AFTER_DESTRUCTOR, no_bucket, ERROR, [variable declared,`&(s)` captured as `s`,invalidated by destructor call `S::~S()` on `s` here,accessed `&(f)` here]
+codetoanalyze/cpp/pulse/closures.cpp, implicit_ref_capture_destroy_invoke_bad, 6, USE_AFTER_LIFETIME, no_bucket, ERROR, [variable declared,`&(s)` captured as `s`,invalidated `s` gone out of scope here,accessed `&(f)` here]
-codetoanalyze/cpp/pulse/closures.cpp, ref_capture_destroy_invoke_bad, 6, USE_AFTER_DESTRUCTOR, no_bucket, ERROR, [variable declared,`&(s)` captured as `s`,invalidated by destructor call `S::~S()` on `s` here,accessed `&(f)` here]
+codetoanalyze/cpp/pulse/closures.cpp, ref_capture_destroy_invoke_bad, 6, USE_AFTER_LIFETIME, no_bucket, ERROR, [variable declared,`&(s)` captured as `s`,invalidated `s` gone out of scope here,accessed `&(f)` here]
 codetoanalyze/cpp/pulse/interprocedural.cpp, delete_inner_then_write_bad, 2, USE_AFTER_DELETE, no_bucket, ERROR, [invalidated during call to `wraps_delete_inner` here,invalidated by `delete` on `x` here,accessed during call to `wraps_read` here,accessed during call to `wraps_read_inner` here,accessed `x->f` here]
 codetoanalyze/cpp/pulse/interprocedural.cpp, delete_then_read_bad, 2, USE_AFTER_DELETE, no_bucket, ERROR, [invalidated by `delete` on `x` here,accessed during call to `wraps_read` here,accessed during call to `wraps_read_inner` here,accessed `x->f` here]
 codetoanalyze/cpp/pulse/interprocedural.cpp, delete_then_write_bad, 2, USE_AFTER_DELETE, no_bucket, ERROR, [invalidated during call to `wraps_delete` here,invalidated during call to `wraps_delete_inner` here,invalidated by `delete` on `x` here,accessed during call to `wraps_read` here,accessed during call to `wraps_read_inner` here,accessed `x->f` here]
 codetoanalyze/cpp/pulse/interprocedural.cpp, feed_invalid_into_access_bad, 2, USE_AFTER_DELETE, no_bucket, ERROR, [assigned to `y`,assigned to `return`,returned from call to `may_return_invalid_ptr_ok()`,assigned to `y`,invalidated during call to `may_return_invalid_ptr_ok` here,invalidated by `delete` on `y` here,accessed during call to `call_store` here,accessed during call to `store` here,accessed `y->p` here]
 codetoanalyze/cpp/pulse/join.cpp, invalidate_node_alias_bad, 12, USE_AFTER_DELETE, no_bucket, ERROR, [assigned to `result`,invalidated by `delete` on `result` here,accessed `*(result)` here]
+codetoanalyze/cpp/pulse/reference_wrapper.cpp, reference_wrapper_stack_bad, 2, USE_AFTER_LIFETIME, no_bucket, ERROR, [assigned to `this->b`,returned from call to `ReferenceWrapperStack::ReferenceWrapperStack()`,invalidated during call to `getwrapperStack` here,invalidated `&(b)` gone out of scope here,accessed `rw.b->f` here]
 codetoanalyze/cpp/pulse/returns.cpp, returns::return_literal_stack_reference_bad, 0, STACK_VARIABLE_ADDRESS_ESCAPE, no_bucket, ERROR, []
 codetoanalyze/cpp/pulse/returns.cpp, returns::return_stack_pointer_bad, 1, STACK_VARIABLE_ADDRESS_ESCAPE, no_bucket, ERROR, []
 codetoanalyze/cpp/pulse/returns.cpp, returns::return_variable_stack_reference1_bad, 1, STACK_VARIABLE_ADDRESS_ESCAPE, no_bucket, ERROR, []
@@ -26,7 +30,7 @@ codetoanalyze/cpp/pulse/use_after_destructor.cpp, use_after_destructor::placemen
 codetoanalyze/cpp/pulse/use_after_destructor.cpp, use_after_destructor::reinit_after_explicit_destructor2_bad, 5, USE_AFTER_DELETE, no_bucket, ERROR, [variable declared,invalidated during call to `use_after_destructor::S::~S` here,invalidated during call to `use_after_destructor::S::__infer_inner_destructor_~S` here,invalidated by `delete` on `this->f` here,accessed during call to `use_after_destructor::S::~S` here,accessed during call to `use_after_destructor::S::__infer_inner_destructor_~S` here,accessed `this->f` here]
 codetoanalyze/cpp/pulse/use_after_destructor.cpp, use_after_destructor::use_after_destructor_bad, 3, USE_AFTER_DELETE, no_bucket, ERROR, [assigned to `this->f`,returned from call to `use_after_destructor::S::S()`,invalidated during call to `use_after_destructor::S::~S` here,invalidated during call to `use_after_destructor::S::__infer_inner_destructor_~S` here,invalidated by `delete` on `this->f` here,accessed `*(s.f)` here]
 codetoanalyze/cpp/pulse/use_after_destructor.cpp, use_after_destructor::use_after_scope1_bad, 7, USE_AFTER_DELETE, no_bucket, ERROR, [variable declared,invalidated during call to `use_after_destructor::S::~S` here,invalidated during call to `use_after_destructor::S::__infer_inner_destructor_~S` here,invalidated by `delete` on `this->f` here,accessed during call to `use_after_destructor::S::~S` here,accessed during call to `use_after_destructor::S::__infer_inner_destructor_~S` here,accessed `this->f` here]
-codetoanalyze/cpp/pulse/use_after_destructor.cpp, use_after_destructor::use_after_scope4_bad, 6, USE_AFTER_DESTRUCTOR, no_bucket, ERROR, [variable declared,invalidated by destructor call `use_after_destructor::C::~C()` on `c` here,accessed `pc->f` here]
+codetoanalyze/cpp/pulse/use_after_destructor.cpp, use_after_destructor::use_after_scope4_bad, 6, USE_AFTER_LIFETIME, no_bucket, ERROR, [variable declared,assigned to `pc`,invalidated `&(c)` gone out of scope here,accessed `pc->f` here]
 codetoanalyze/cpp/pulse/use_after_free.cpp, double_free_simple_bad, 2, USE_AFTER_FREE, no_bucket, ERROR, [invalidated by call to `free()` on `x` here,accessed `x` here]
 codetoanalyze/cpp/pulse/use_after_free.cpp, use_after_free_simple_bad, 2, USE_AFTER_FREE, no_bucket, ERROR, [invalidated by call to `free()` on `x` here,accessed `*(x)` here]
 codetoanalyze/cpp/pulse/vector.cpp, FP_init_fill_then_push_back_loop_ok, 6, VECTOR_INVALIDATION, no_bucket, ERROR, [returned from call to `std::vector::at(&(vec),(unsigned long) 1)`,assigned to `elt`,invalidated potentially invalidated by call to `std::vector::push_back()` on `&(vec)` here,accessed `*(elt)` here]

infer/tests/codetoanalyze/cpp/pulse/reference_wrapper.cpp

@@ -42,7 +42,7 @@ ReferenceWrapperStack getwrapperStack() {
   return b;
 }
 
-int FN_reference_wrapper_stack_bad() {
+int reference_wrapper_stack_bad() {
   ReferenceWrapperStack rw = getwrapperStack();
   return rw.b->f; // we want to report use after lifetime bug here
 }