From 9868f7f76304dcf7a600fda899cc9f92951983fe Mon Sep 17 00:00:00 2001 From: Jules Villard Date: Tue, 18 Dec 2018 03:25:46 -0800 Subject: [PATCH] [pulse] warn on returning address of C++ temporary Summary: When a C++ temporary goes out of scope, tag its address in the heap with a new attribute `AddressOfCppTemporary` so that we can later check that we don't return it. Reviewed By: da319 Differential Revision: D13466898 fbshipit-source-id: 8808338b4 --- infer/src/checkers/PulseDomain.ml | 35 +++++++++++++++++-- .../tests/codetoanalyze/cpp/pulse/issues.exp | 2 ++ .../tests/codetoanalyze/cpp/pulse/returns.cpp | 4 +-- 3 files changed, 36 insertions(+), 5 deletions(-) diff --git a/infer/src/checkers/PulseDomain.ml b/infer/src/checkers/PulseDomain.ml index fa02e98b2..b8616e4c6 100644 --- a/infer/src/checkers/PulseDomain.ml +++ b/infer/src/checkers/PulseDomain.ml @@ -48,6 +48,7 @@ module Attribute = struct type t = (* DO NOT MOVE, see toplevel comment *) | Invalid of Invalidation.t + | AddressOfCppTemporary of Var.t | Closure of Typ.Procname.t * (AccessPath.base * HilExp.AccessExpression.t * AbstractAddress.t) list @@ -58,6 +59,8 @@ module Attribute = struct let pp f = function | Invalid invalidation -> Invalidation.pp f invalidation + | AddressOfCppTemporary var -> + F.fprintf f "&%a" Var.pp var | Closure (pname, captured, location) -> F.fprintf f "%a[%a] (%a)" Typ.Procname.pp pname (Pp.seq ~sep:"," @@ -763,6 +766,18 @@ module Operations = struct let check_address_of_local_variable proc_desc address astate = let proc_location = Procdesc.get_loc proc_desc in let proc_name = Procdesc.get_proc_name proc_desc in + let check_address_of_cpp_temporary () = + Memory.find_opt address astate.heap + |> Option.fold_result ~init:() ~f:(fun () (_, attrs) -> + Container.fold_result ~fold:(IContainer.fold_of_pervasives_fold ~fold:Attributes.fold) + attrs ~init:() ~f:(fun () attr -> + match attr with + | Attribute.AddressOfCppTemporary variable -> + Error + (Diagnostic.StackVariableAddressEscape {variable; location= proc_location}) + | _ -> + Ok () ) ) + in let check_address_of_stack_variable () = Container.fold_result ~fold:(IContainer.fold_of_pervasives_map_fold ~fold:Stack.fold) astate.stack ~init:() ~f:(fun () (variable, var_address) -> @@ -774,12 +789,26 @@ module Operations = struct then Error (Diagnostic.StackVariableAddressEscape {variable; location= proc_location}) else Ok () ) in - check_address_of_stack_variable () >>| fun () -> astate + check_address_of_cpp_temporary () >>= check_address_of_stack_variable >>| fun () -> astate + + + let mark_address_of_cpp_temporary variable address heap = + Memory.add_attributes address (Attributes.singleton (AddressOfCppTemporary variable)) heap let remove_vars vars astate = - let stack = List.fold ~f:(fun var stack -> Stack.remove stack var) ~init:astate.stack vars in - if phys_equal stack astate.stack then astate else {astate with stack} + let heap = + List.fold vars ~init:astate.heap ~f:(fun heap var -> + match Stack.find_opt var astate.stack with + | Some address when Var.is_cpp_temporary var -> + (* TODO: it would be good to record the location of the temporary creation in the + stack and save it here in the attribute for reporting *) + mark_address_of_cpp_temporary var address heap + | _ -> + heap ) + in + let stack = List.fold ~f:(fun stack var -> Stack.remove var stack) ~init:astate.stack vars in + if phys_equal stack astate.stack && phys_equal heap astate.heap then astate else {stack; heap} end module Closures = struct diff --git a/infer/tests/codetoanalyze/cpp/pulse/issues.exp b/infer/tests/codetoanalyze/cpp/pulse/issues.exp index e92d93130..2a5298205 100644 --- a/infer/tests/codetoanalyze/cpp/pulse/issues.exp +++ b/infer/tests/codetoanalyze/cpp/pulse/issues.exp @@ -6,6 +6,8 @@ codetoanalyze/cpp/pulse/join.cpp, invalidate_node_alias_bad, 12, USE_AFTER_DELET codetoanalyze/cpp/pulse/returns.cpp, returns::return_deleted_bad, 4, USE_AFTER_DELETE, no_bucket, ERROR, [invalidated by call to `delete x` at line 112, column 3 here,accessed `x` 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, 0, STACK_VARIABLE_ADDRESS_ESCAPE, no_bucket, ERROR, [] +codetoanalyze/cpp/pulse/returns.cpp, returns::return_variable_stack_reference1_bad, 0, STACK_VARIABLE_ADDRESS_ESCAPE, no_bucket, ERROR, [] +codetoanalyze/cpp/pulse/returns.cpp, returns::return_variable_stack_reference2_bad, 0, STACK_VARIABLE_ADDRESS_ESCAPE, no_bucket, ERROR, [] codetoanalyze/cpp/pulse/use_after_delete.cpp, delete_in_branch_bad, 5, USE_AFTER_DELETE, no_bucket, ERROR, [invalidated by call to `delete s` at line 57, column 5 here,accessed `s` here] codetoanalyze/cpp/pulse/use_after_delete.cpp, delete_in_loop_bad, 3, USE_AFTER_DELETE, no_bucket, ERROR, [invalidated by call to `delete s` at line 82, column 5 here,accessed `s` here] codetoanalyze/cpp/pulse/use_after_delete.cpp, deref_deleted_bad, 3, USE_AFTER_DELETE, no_bucket, ERROR, [invalidated by call to `delete s` at line 18, column 3 here,accessed `s` here] diff --git a/infer/tests/codetoanalyze/cpp/pulse/returns.cpp b/infer/tests/codetoanalyze/cpp/pulse/returns.cpp index ff763b6d7..16eb24053 100644 --- a/infer/tests/codetoanalyze/cpp/pulse/returns.cpp +++ b/infer/tests/codetoanalyze/cpp/pulse/returns.cpp @@ -19,12 +19,12 @@ struct S { const int& return_literal_stack_reference_bad() { return 1; } -const int& FN_return_variable_stack_reference1_bad() { +const int& return_variable_stack_reference1_bad() { const int& x = 2; return x; } -const int& FN_return_variable_stack_reference2_bad() { +const int& return_variable_stack_reference2_bad() { const int& x = 2; const int& y = x; return y;