[clang] create C++ temporaries on CXXBindTemporaryExpr when appropriate

Summary:
Like `MaterializeTemporaryExpr`, `CXXBindTemporaryExpr` is used by clang
to signal that a temporary variable is needed to hold the result of a
(sub-)expression. But, unlike the former, the latter only requires a
C++ temporary to be created in *some* of the cases, namely when the
sub-expression is not performing initialisation for a known variable
already (in particular one already created by
`MaterializeTemporaryExpr`! Or an actual program variable being
assigned).

Previously, we were missing creating (and thus destroying) these
C++ temporaries when needed. This diff fixes that by registering
`CXXBindTemporaryExpr` as a source of C++ temporaries in CScope.ml, and
adding logic to destroy only the C++ temporaries that ended up being
actually used.

Reviewed By: skcho

Differential Revision: D30455859

fbshipit-source-id: baf7029eb

infer/src/IR/Procdesc.ml

@@ -535,6 +535,11 @@ let get_loc pdesc = pdesc.attributes.loc
 (** Return name and type of local variables *)
 let get_locals pdesc = pdesc.attributes.locals
 
+let is_local pdesc pvar =
+  List.exists (get_locals pdesc) ~f:(fun {ProcAttributes.name} ->
+      Mangled.equal name (Pvar.get_name pvar) )
+
+
 let has_added_return_param pdesc = pdesc.attributes.has_added_return_param
 
 let is_ret_type_pod pdesc = pdesc.attributes.is_ret_type_pod

infer/src/IR/Procdesc.mli

@@ -259,6 +259,10 @@ val get_loc : t -> Location.t
 val get_locals : t -> ProcAttributes.var_data list
 (** Return name and type and attributes of local variables *)
 
+val is_local : t -> Pvar.t -> bool
+(** [is_local pdesc pvar] is [true] iff [pvar] is a local variable of [pdesc]. This function
+    performs a linear search on the local variables of [pdesc]. *)
+
 val get_nodes : t -> Node.t list
 (** Return the nodes, excluding the start node and the exit node. *)
 

infer/src/clang/cScope.ml

@@ -218,22 +218,24 @@ module Variables = struct
 end
 
 module CXXTemporaries = struct
-  let rec visit_stmt_aux context stmt ~needs_marker temporaries =
-    match (stmt : Clang_ast_t.stmt) with
+  let rec visit_stmt_aux (context : CContext.t) (stmt : Clang_ast_t.stmt) ~needs_marker temporaries
+      =
+    match stmt with
     | MaterializeTemporaryExpr
         ( stmt_info
         , stmt_list
         , expr_info
         , { mtei_decl_ref=
               (* C++ temporaries bound to a const reference see their lifetimes extended to that of
                  the reference *)
-              None } ) ->
+              None } )
+    | CXXBindTemporaryExpr (stmt_info, stmt_list, expr_info, _) ->
         let pvar, typ = CVar_decl.materialize_cpp_temporary context stmt_info expr_info in
         L.debug Capture Verbose "+%a:%a@," (Pvar.pp Pp.text) pvar (Typ.pp Pp.text) typ ;
         let marker =
           if needs_marker then (
             let marker_pvar =
-              Pvar.mk_tmp "_temp_marker_" (Procdesc.get_proc_name context.CContext.procdesc)
+              Pvar.mk_tmp "_temp_marker_" (Procdesc.get_proc_name context.procdesc)
             in
             L.debug Capture Verbose "Attaching marker %a to %a@," (Pvar.pp Pp.text) marker_pvar
               (Pvar.pp Pp.text) pvar ;

infer/src/clang/cTrans.ml

@@ -4089,6 +4089,28 @@ module CTrans_funct (F : CModule_type.CFrontend) : CModule_type.CTranslation = s
     res_trans
 
 
+  and cxxBindTemporaryExpr_trans trans_state stmt_info stmt_list expr_info =
+    (* Only create a temporary if there *isn't* already a variable to store the result into, eg [X x
+       = X();] will generate a [CXXBindTemporaryExpr] to hold [X()] but clang doesn't expect a
+       temporary to be actually created here as we can store the result in [x] directly (and have to
+       since C++17 –in contexts where clang does expect the temporary to be created we'll get an
+       intermediate copy or move constructor call, which in particular has the effect of setting
+       [trans_state.var_exp_typ] to [None]).
+
+       Since the presence of [trans_state.var_exp_typ] is highly dependent on the current context,
+       [CScope.CXXTemporaries.get_destroyable_temporaries] will compute an over-approximation of the
+       C++ temporaries needed by a given expression. We can tell which ones are actually used as
+       they are recorded as local variables by [materializeTemporaryExpr_trans]. This trick is used
+       by [exprWithCleanups_trans] to compute the accurate set of C++ temporaries to destroy. *)
+    if Option.is_none trans_state.var_exp_typ then
+      (* actually create a C++ temporary to hold the result of the sub-expression *)
+      materializeTemporaryExpr_trans trans_state stmt_info stmt_list expr_info
+    else
+      (* do nothing and translate the sub-expression directly if we already have a variable to
+         initialize *)
+      parenExpr_trans trans_state stmt_info.Clang_ast_t.si_source_range stmt_list
+
+
   and compoundLiteralExpr_trans trans_state stmt_list stmt_info expr_info =
     let stmt = match stmt_list with [stmt] -> stmt | _ -> assert false in
     let var_exp_typ =
@@ -4472,6 +4494,12 @@ module CTrans_funct (F : CModule_type.CFrontend) : CModule_type.CTranslation = s
       CLocation.location_of_stmt_info trans_state.context.translation_unit_context.source_file
         stmt_info
     in
+    (* We need to pass a set of temporaries to destroy to the translation of the sub-expression but
+       we cannot know exactly what they will be until *after* the translation (see also
+       [CXXBindTemporaryExpr])... Fortunately the translation of the sub-expression only needs to
+       know a *superset* of the temporaries to destroy, which is easier to compute. We'll get the
+       real set of temporaries to destroy after when filtering to keep only those that are local
+       variables. *)
     let temporaries_to_destroy =
       CScope.CXXTemporaries.get_destroyable_temporaries trans_state.context stmt_list
     in
@@ -4493,6 +4521,12 @@ module CTrans_funct (F : CModule_type.CFrontend) : CModule_type.CTranslation = s
         stmt_info
     in
     let sub_expr_result = instruction trans_state stmt in
+    (* HACK: we can know which C++ temporaries were actually used by the translation of the
+       sub-expression by inspecting the local variables *)
+    let temporaries_to_destroy =
+      List.filter temporaries_to_destroy ~f:(fun {CScope.pvar} ->
+          Procdesc.is_local trans_state.context.procdesc pvar )
+    in
     L.debug Capture Verbose "sub_expr_result.control=%a@\n" pp_control sub_expr_result.control ;
     let init_markers_to_zero_instrs =
       List.fold temporaries_to_destroy ~init:[] ~f:(fun instrs {CScope.marker} ->
@@ -4657,7 +4691,11 @@ module CTrans_funct (F : CModule_type.CFrontend) : CModule_type.CTranslation = s
 
   (** Instrument program to know when C++ temporaries created conditionally have indeed been created
       and need to be destroyed. This is a common compilation scheme for temporary variables created
-      conditionally in expressions, due to either [a?b:c] or short-circuiting of [&&] or [||]. *)
+      conditionally in expressions, due to either [a?b:c] or short-circuiting of [&&] or [||].
+
+      Similarly to [exprWithCleanups_trans], only act on the C++ temporaries that we ended up using
+      in the translation of the sub-expression. Here the fact we only take variables initialized by
+      the sub-expression ensures that this is always the case. *)
   and instruction_insert_cxx_temporary_markers trans_state stmt =
     let stmt_result = instruction_translate trans_state stmt in
     let stmt_info, _ = Clang_ast_proj.get_stmt_tuple stmt in
@@ -4894,13 +4932,12 @@ module CTrans_funct (F : CModule_type.CFrontend) : CModule_type.CTranslation = s
         cxxDeleteExpr_trans trans_state stmt_info stmt_list delete_expr_info
     | MaterializeTemporaryExpr (stmt_info, stmt_list, expr_info, _) ->
         materializeTemporaryExpr_trans trans_state stmt_info stmt_list expr_info
+    | CXXBindTemporaryExpr (stmt_info, stmt_list, expr_info, _) ->
+        cxxBindTemporaryExpr_trans trans_state stmt_info stmt_list expr_info
     | CompoundLiteralExpr (stmt_info, stmt_list, expr_info) ->
         compoundLiteralExpr_trans trans_state stmt_list stmt_info expr_info
     | InitListExpr (stmt_info, stmts, expr_info) ->
         initListExpr_trans trans_state stmt_info expr_info stmts
-    | CXXBindTemporaryExpr ({Clang_ast_t.si_source_range}, stmt_list, _, _) ->
-        (* right now we ignore this expression and try to translate the child node *)
-        parenExpr_trans trans_state si_source_range stmt_list
     | CXXDynamicCastExpr (stmt_info, stmts, _, _, qual_type, _) ->
         cxxDynamicCastExpr_trans trans_state stmt_info stmts qual_type
     | CXXDefaultArgExpr (_, _, _, default_expr_info)

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

@@ -104,6 +104,7 @@ codetoanalyze/cpp/biabduction/smart_ptr/unique_ptr_deref.cpp, unique_ptr::FP_res
 codetoanalyze/cpp/biabduction/smart_ptr/unique_ptr_deref.cpp, unique_ptr::FP_reset_ptr_deref_ok, 2, BIABDUCTION_MEMORY_LEAK, CPP, ERROR, [start of procedure unique_ptr::FP_reset_ptr_deref_ok(),Skipping unique_ptr<true,_void>: method has no implementation,Skipping reset: method has no implementation]
 codetoanalyze/cpp/biabduction/smart_ptr/unique_ptr_deref.cpp, unique_ptr::FP_unique_ptr_move_deref_ok, 1, BIABDUCTION_MEMORY_LEAK, CPP, ERROR, [start of procedure unique_ptr::FP_unique_ptr_move_deref_ok(),Skipping unique_ptr<true,_void>: method has no implementation]
 codetoanalyze/cpp/biabduction/smart_ptr/unique_ptr_deref.cpp, unique_ptr::unique_ptr_assign_deref_ok, 1, BIABDUCTION_MEMORY_LEAK, CPP, ERROR, [start of procedure unique_ptr::unique_ptr_assign_deref_ok(),Skipping unique_ptr<true,_void>: method has no implementation]
+codetoanalyze/cpp/biabduction/smart_ptr/weak_ptr_compil.cpp, weak_ptr_lock_repro_large::RDC::create::lambda_smart_ptr_weak_ptr_compil.cpp:60:13::operator()::lambda_smart_ptr_weak_ptr_compil.cpp:64:29::operator(), 2, Cannot_star, no_bucket, ERROR, [start of procedure operator(),Skipping operator(): method has no implementation,start of procedure DCC,return from a call to weak_ptr_lock_repro_large::DCC::DCC,Skipping ~DCC: method has no implementation,Skipping joinT<weak_ptr_lock_repro_large::RDC>: method has no implementation]
 codetoanalyze/cpp/biabduction/smart_ptr/weak_ptr_compil.cpp, weak_ptr_lock_repro_small::ERROR_foo, 3, Cannot_star, no_bucket, ERROR, [start of procedure weak_ptr_lock_repro_small::ERROR_foo(),Skipping lock: method has no implementation,start of procedure weak_ptr_lock_repro_small::joinT<int>(),return from a call to weak_ptr_lock_repro_small::joinT<int>]
 codetoanalyze/cpp/biabduction/static_local/nonstatic_local_bad.cpp, nonstatic_local_caller, 2, DANGLING_POINTER_DEREFERENCE, no_bucket, ERROR, [start of procedure nonstatic_local_caller(),start of procedure nonstatic_local_bad(),return from a call to nonstatic_local_bad]
 codetoanalyze/cpp/biabduction/subtyping/cast_with_enforce.cpp, cast_with_enforce::cast_with_npe, 3, NULL_DEREFERENCE, B1, ERROR, [start of procedure cast_with_enforce::cast_with_npe(),start of procedure Base,return from a call to cast_with_enforce::Base::Base]

infer/tests/codetoanalyze/cpp/frontend/destructors/break_scope.cpp.dot

@@ -226,11 +226,11 @@ digraph cfg {
 
 
 	 "test_for_range#break_scope#2115859683356214080.ad34c277f8d086eb0a22c75fc80fb235_20" -> "test_for_range#break_scope#2115859683356214080.ad34c277f8d086eb0a22c75fc80fb235_19" ;
-"test_for_range#break_scope#2115859683356214080.ad34c277f8d086eb0a22c75fc80fb235_21" [label="21:  DeclStmt \n   VARIABLE_DECLARED(0$?%__sil_tmpSIL_materialize_temp__n$41:break_scope::X const ); [line 47, column 12]\n  n$44=_fun_break_scope::iterator::operator*(&__begin1:break_scope::iterator&,&0$?%__sil_tmpSIL_materialize_temp__n$41:break_scope::X*) assign_last [line 47, column 12]\n " shape="box"]
+"test_for_range#break_scope#2115859683356214080.ad34c277f8d086eb0a22c75fc80fb235_21" [label="21:  DeclStmt \n   VARIABLE_DECLARED(0$?%__sil_tmpSIL_materialize_temp__n$41:break_scope::X const ); [line 47, column 12]\n  n$45=_fun_break_scope::iterator::operator*(&__begin1:break_scope::iterator&,&0$?%__sil_tmpSIL_materialize_temp__n$41:break_scope::X*) assign_last [line 47, column 12]\n " shape="box"]
 
 
 	 "test_for_range#break_scope#2115859683356214080.ad34c277f8d086eb0a22c75fc80fb235_21" -> "test_for_range#break_scope#2115859683356214080.ad34c277f8d086eb0a22c75fc80fb235_22" ;
-"test_for_range#break_scope#2115859683356214080.ad34c277f8d086eb0a22c75fc80fb235_22" [label="22:  Destruction(temporaries cleanup) \n   n$45=_fun_break_scope::X::X(&x:break_scope::X*,&0$?%__sil_tmpSIL_materialize_temp__n$41:break_scope::X const &) [line 47, column 12]\n  _=*&0$?%__sil_tmpSIL_materialize_temp__n$41:break_scope::X const  [line 47, column 12]\n  n$47=_fun_break_scope::X::~X(&0$?%__sil_tmpSIL_materialize_temp__n$41:break_scope::X const *) injected [line 47, column 12]\n " shape="box"]
+"test_for_range#break_scope#2115859683356214080.ad34c277f8d086eb0a22c75fc80fb235_22" [label="22:  Destruction(temporaries cleanup) \n   n$46=_fun_break_scope::X::X(&x:break_scope::X*,&0$?%__sil_tmpSIL_materialize_temp__n$41:break_scope::X const &) [line 47, column 12]\n  _=*&0$?%__sil_tmpSIL_materialize_temp__n$41:break_scope::X const  [line 47, column 12]\n  n$48=_fun_break_scope::X::~X(&0$?%__sil_tmpSIL_materialize_temp__n$41:break_scope::X const *) injected [line 47, column 12]\n " shape="box"]
 
 
 	 "test_for_range#break_scope#2115859683356214080.ad34c277f8d086eb0a22c75fc80fb235_22" -> "test_for_range#break_scope#2115859683356214080.ad34c277f8d086eb0a22c75fc80fb235_17" ;
@@ -243,11 +243,11 @@ digraph cfg {
 
 
 	 "test_for_range#break_scope#2115859683356214080.ad34c277f8d086eb0a22c75fc80fb235_24" -> "test_for_range#break_scope#2115859683356214080.ad34c277f8d086eb0a22c75fc80fb235_11" ;
-"test_for_range#break_scope#2115859683356214080.ad34c277f8d086eb0a22c75fc80fb235_25" [label="25:  DeclStmt \n   VARIABLE_DECLARED(x1:break_scope::X); [line 46, column 3]\n  n$49=_fun_break_scope::X::X(&x1:break_scope::X*) [line 46, column 5]\n " shape="box"]
+"test_for_range#break_scope#2115859683356214080.ad34c277f8d086eb0a22c75fc80fb235_25" [label="25:  DeclStmt \n   VARIABLE_DECLARED(x1:break_scope::X); [line 46, column 3]\n  n$50=_fun_break_scope::X::X(&x1:break_scope::X*) [line 46, column 5]\n " shape="box"]
 
 
 	 "test_for_range#break_scope#2115859683356214080.ad34c277f8d086eb0a22c75fc80fb235_25" -> "test_for_range#break_scope#2115859683356214080.ad34c277f8d086eb0a22c75fc80fb235_24" ;
-"test_for_range#break_scope#2115859683356214080.ad34c277f8d086eb0a22c75fc80fb235_26" [label="26:  DeclStmt \n   VARIABLE_DECLARED(vector:break_scope::vec); [line 45, column 3]\n  n$50=_fun_break_scope::vec::vec(&vector:break_scope::vec*) [line 45, column 7]\n " shape="box"]
+"test_for_range#break_scope#2115859683356214080.ad34c277f8d086eb0a22c75fc80fb235_26" [label="26:  DeclStmt \n   VARIABLE_DECLARED(vector:break_scope::vec); [line 45, column 3]\n  n$51=_fun_break_scope::vec::vec(&vector:break_scope::vec*) [line 45, column 7]\n " shape="box"]
 
 
 	 "test_for_range#break_scope#2115859683356214080.ad34c277f8d086eb0a22c75fc80fb235_26" -> "test_for_range#break_scope#2115859683356214080.ad34c277f8d086eb0a22c75fc80fb235_25" ;
@@ -590,11 +590,11 @@ digraph cfg {
 "operator*#iterator#break_scope(class break_scope::X)#(4328339407583570703).89adb890a0c29514eda31053987e2050_2" [label="2: Exit break_scope::iterator::operator* \n  " color=yellow style=filled]
 
 
-"operator*#iterator#break_scope(class break_scope::X)#(4328339407583570703).89adb890a0c29514eda31053987e2050_3" [label="3:  DeclStmt \n   VARIABLE_DECLARED(0$?%__sil_tmpSIL_materialize_temp__n$1:break_scope::X const ); [line 42, column 40]\n  n$2=*&this:break_scope::iterator const * [line 42, column 40]\n  n$3=*n$2.vector:break_scope::vec const * [line 42, column 40]\n  _=*n$3:break_scope::vec const  [line 42, column 40]\n  n$5=*&this:break_scope::iterator const * [line 42, column 52]\n  n$6=*n$5.position:int [line 42, column 52]\n  n$8=_fun_break_scope::vec::get(n$3:break_scope::vec const *,n$6:int,&0$?%__sil_tmpSIL_materialize_temp__n$1:break_scope::X*) assign_last [line 42, column 40]\n " shape="box"]
+"operator*#iterator#break_scope(class break_scope::X)#(4328339407583570703).89adb890a0c29514eda31053987e2050_3" [label="3:  DeclStmt \n   VARIABLE_DECLARED(0$?%__sil_tmpSIL_materialize_temp__n$1:break_scope::X const ); [line 42, column 40]\n  n$3=*&this:break_scope::iterator const * [line 42, column 40]\n  n$4=*n$3.vector:break_scope::vec const * [line 42, column 40]\n  _=*n$4:break_scope::vec const  [line 42, column 40]\n  n$6=*&this:break_scope::iterator const * [line 42, column 52]\n  n$7=*n$6.position:int [line 42, column 52]\n  n$9=_fun_break_scope::vec::get(n$4:break_scope::vec const *,n$7:int,&0$?%__sil_tmpSIL_materialize_temp__n$1:break_scope::X*) assign_last [line 42, column 40]\n " shape="box"]
 
 
 	 "operator*#iterator#break_scope(class break_scope::X)#(4328339407583570703).89adb890a0c29514eda31053987e2050_3" -> "operator*#iterator#break_scope(class break_scope::X)#(4328339407583570703).89adb890a0c29514eda31053987e2050_4" ;
-"operator*#iterator#break_scope(class break_scope::X)#(4328339407583570703).89adb890a0c29514eda31053987e2050_4" [label="4:  Destruction(temporaries cleanup) \n   n$9=_fun_break_scope::X::X(n$0:break_scope::X*,&0$?%__sil_tmpSIL_materialize_temp__n$1:break_scope::X const &) [line 42, column 40]\n  _=*&0$?%__sil_tmpSIL_materialize_temp__n$1:break_scope::X const  [line 42, column 60]\n  n$11=_fun_break_scope::X::~X(&0$?%__sil_tmpSIL_materialize_temp__n$1:break_scope::X const *) injected [line 42, column 60]\n " shape="box"]
+"operator*#iterator#break_scope(class break_scope::X)#(4328339407583570703).89adb890a0c29514eda31053987e2050_4" [label="4:  Destruction(temporaries cleanup) \n   n$10=_fun_break_scope::X::X(n$0:break_scope::X*,&0$?%__sil_tmpSIL_materialize_temp__n$1:break_scope::X const &) [line 42, column 40]\n  _=*&0$?%__sil_tmpSIL_materialize_temp__n$1:break_scope::X const  [line 42, column 60]\n  n$12=_fun_break_scope::X::~X(&0$?%__sil_tmpSIL_materialize_temp__n$1:break_scope::X const *) injected [line 42, column 60]\n " shape="box"]
 
 
 	 "operator*#iterator#break_scope(class break_scope::X)#(4328339407583570703).89adb890a0c29514eda31053987e2050_4" -> "operator*#iterator#break_scope(class break_scope::X)#(4328339407583570703).89adb890a0c29514eda31053987e2050_6" ;