From 2baf3f84561cb306a431198781211f12e117081f Mon Sep 17 00:00:00 2001 From: Andrzej Kotulski Date: Fri, 26 Aug 2016 03:32:03 -0700 Subject: [PATCH] Replace shared_ptr structs with T* pointers Summary: Make std::shared_ptr translated as T* inside infer. This will make reporting better since smart pointers are really pointers not structs - this form is much easier for the analyzer to understand. This requires changes to the model of shared_ptr as well. Reviewed By: jvillard Differential Revision: D3587255 fbshipit-source-id: b86fb36 --- facebook-clang-plugins | 2 +- .../cpp/include/infer_model/infer_traits.h | 15 +++ .../cpp/include/infer_model/shared_ptr.h | 114 +++++++++++++----- infer/src/clang/cFrontend_decl.ml | 11 +- infer/src/clang/cTrans.ml | 22 ++-- infer/src/clang/cTypes_decl.ml | 36 +++++- .../cpp/frontend/lambda/lambda1.cpp | 2 +- .../tests/endtoend/cpp/infer/LambdaTest.java | 1 + 8 files changed, 156 insertions(+), 47 deletions(-) create mode 100644 infer/models/cpp/include/infer_model/infer_traits.h diff --git a/facebook-clang-plugins b/facebook-clang-plugins index 9ce2266db..0a3172fa8 160000 --- a/facebook-clang-plugins +++ b/facebook-clang-plugins @@ -1 +1 @@ -Subproject commit 9ce2266db9bdf23d70ddaed093decdeeb6bf8db3 +Subproject commit 0a3172fa8d00279b6bb98bedb12bd6cade4c4424 diff --git a/infer/models/cpp/include/infer_model/infer_traits.h b/infer/models/cpp/include/infer_model/infer_traits.h new file mode 100644 index 000000000..eb6762ba1 --- /dev/null +++ b/infer/models/cpp/include/infer_model/infer_traits.h @@ -0,0 +1,15 @@ +/* + * Copyright (c) 2016 - present Facebook, Inc. + * All rights reserved. + * + * This source code is licensed under the BSD style license found in the + * LICENSE file in the root directory of this source tree. An additional grant + * of patent rights can be found in the PATENTS file in the same directory. + */ + +namespace infer_traits { +// all friends of this class and will be translated as type T by infer +// frontend instead of their own type +template +class TranslateAsType {}; +} // namespace infer_traits diff --git a/infer/models/cpp/include/infer_model/shared_ptr.h b/infer/models/cpp/include/infer_model/shared_ptr.h index b26f0bbe2..e3339783c 100644 --- a/infer/models/cpp/include/infer_model/shared_ptr.h +++ b/infer/models/cpp/include/infer_model/shared_ptr.h @@ -11,24 +11,66 @@ // ASSERT that __cplusplus >= 201103L #include +#include INFER_NAMESPACE_STD_BEGIN // use inheritance to avoid compilation errors when using // methods / non-member functions that are not modeled -// WARNING: sizeof(shared_ptr) = 24, not 16 - this may +// WARNING: if sizeof(shared_ptr) becomes different than 16, it may // lead to compilation errors template class shared_ptr : public std__shared_ptr { + // translate shared_ptr as type T* + friend class infer_traits::TranslateAsType; + + // shared_ptr in infer is translated as T*. + // Some facts: + // 1. shared_ptr* translated as T** + // 2. typeof(this) translated as T** + // 3. typeof(this) in clang's AST is shared_ptr* + // When writing models for shared_ptr, we need to use infer's representation + // In order to achieve that and not break compilation, there is some ugly + // casting going around. We are using void* and void** to make compilation + // happy - infer doesn't care about those types that much since they are + // pointers anyway. + // Example of model_X function declaration: + // static void model_X(infer_shared_ptr_t self, ... params) + // model_X are really C functions, but to simplify linking process, they are + // defined inside shared_ptr class as static methods. + // When using model_X functions, call them like so: + // model_X(__cast_to_infer_ptr(this), args) + + /// type of 'this' in shared_ptr as seen by infer + typedef const void** infer_shared_ptr_t; +// use it to avoid compilation errors and make infer analyzer happy +#define __cast_to_infer_ptr(self) ((infer_shared_ptr_t)self) + + static void model_set(infer_shared_ptr_t self, const void* value) { + *self = value; + } + + static void model_copy(infer_shared_ptr_t self, infer_shared_ptr_t other) { + /* TODO - increase refcount*/ + *self = *other; + } + + static void model_move(infer_shared_ptr_t self, infer_shared_ptr_t other) { + model_copy(self, other); + model_set(other, nullptr); + } + + static T* model_get(infer_shared_ptr_t self) { return (T*)(*self); } + + static void model_swap(infer_shared_ptr_t infer_self, + infer_shared_ptr_t infer_other) { + const void* t = *infer_self; + *infer_self = *infer_other; + *infer_other = t; + } + public: -#if INFER_USE_LIBCPP - using std__shared_ptr::__ptr_; -#define __data __ptr_ -#else - using __shared_ptr::_M_ptr; -#define __data _M_ptr -#endif // Conversion constructors to allow implicit conversions. // it's here purely to avoid compilation errors template { shared_ptr(const std__shared_ptr& r, T* p) noexcept {} // constructors: - constexpr shared_ptr() noexcept { __data = nullptr; } + constexpr shared_ptr() noexcept { + model_set(__cast_to_infer_ptr(this), nullptr); + } shared_ptr(nullptr_t) : shared_ptr() {} @@ -52,7 +96,7 @@ class shared_ptr : public std__shared_ptr { template ::value>::type> explicit shared_ptr(Y* p) { - __data = p; + model_set(__cast_to_infer_ptr(this), p); } template { template shared_ptr(const shared_ptr& r, T* p) noexcept { - __data = nullptr; /* TODO */ + model_set(__cast_to_infer_ptr(this), nullptr); /* TODO */ } - shared_ptr(const shared_ptr& r) noexcept - : shared_ptr(r.__data) { /* TODO - increase refcount*/ + shared_ptr(const shared_ptr& r) noexcept { + model_copy(__cast_to_infer_ptr(this), __cast_to_infer_ptr(&r)); } template ::value>::type> - shared_ptr(const shared_ptr& r) noexcept - : shared_ptr(r.__data) { /* TODO - increase refcount*/ + shared_ptr(const shared_ptr& r) noexcept { + model_copy(__cast_to_infer_ptr(this), __cast_to_infer_ptr(&r)); } - shared_ptr(shared_ptr&& r) noexcept : shared_ptr(r.__data) { - r.__data = nullptr; + shared_ptr(shared_ptr&& r) noexcept { + model_move(__cast_to_infer_ptr(this), __cast_to_infer_ptr(&r)); } template ::value>::type> - shared_ptr(shared_ptr&& r) noexcept : shared_ptr(r.__data) { - r.__data = nullptr; + shared_ptr(shared_ptr&& r) noexcept { + model_move(__cast_to_infer_ptr(this), __cast_to_infer_ptr(&r)); } template { // assignment: shared_ptr& operator=(const shared_ptr& r) noexcept { // shared_ptr(r).swap(*this); - __data = r.__data; + model_copy(__cast_to_infer_ptr(this), __cast_to_infer_ptr(&r)); return *this; } @@ -130,13 +175,13 @@ class shared_ptr : public std__shared_ptr { typename = typename enable_if::value>::type> shared_ptr& operator=(const shared_ptr& r) noexcept { // shared_ptr(r).swap(*this); - __data = r.__data; + model_copy(__cast_to_infer_ptr(this), __cast_to_infer_ptr(&r)); return *this; } shared_ptr& operator=(shared_ptr&& r) noexcept { // shared_ptr(std::move(r)).swap(*this); - __data = r.__data; + model_move(__cast_to_infer_ptr(this), __cast_to_infer_ptr(&r)); return *this; } @@ -144,7 +189,7 @@ class shared_ptr : public std__shared_ptr { typename = typename enable_if::value>::type> shared_ptr& operator=(shared_ptr&& r) { // shared_ptr(std::move(r)).swap(*this); - __data = r.__data; + model_move(__cast_to_infer_ptr(this), __cast_to_infer_ptr(&r)); return *this; } @@ -162,9 +207,7 @@ class shared_ptr : public std__shared_ptr { // modifiers: void swap(shared_ptr& r) noexcept { - T* tmp = r.__data; - r.__data = __data; - __data = tmp; + model_swap(__cast_to_infer_ptr(this), __cast_to_infer_ptr(&r)); } void reset() noexcept { reset((T*)nullptr); } @@ -177,7 +220,7 @@ class shared_ptr : public std__shared_ptr { delete __data; } */ - __data = p; + model_set(__cast_to_infer_ptr(this), p); // TODO adjust refcounts } @@ -197,14 +240,21 @@ class shared_ptr : public std__shared_ptr { } // observers: - T* get() const noexcept { return __data; } + + T* get() const noexcept { return model_get(__cast_to_infer_ptr(this)); } typename add_lvalue_reference::type operator*() const noexcept { - return *__data; + return *model_get(__cast_to_infer_ptr(this)); + } + T* operator->() const noexcept { + return model_get(__cast_to_infer_ptr(this)); } - T* operator->() const noexcept { return __data; } long use_count() const noexcept { return 2; /* FIXME */ } bool unique() const noexcept { return use_count() == 1; /* FIXME */ } - explicit operator bool() const noexcept { return (bool)__data; } + explicit operator bool() const noexcept { + // for some reason analyzer can't cast to bool correctly, trick with two + // negations creates right specs for this function + return !!(bool)(model_get(__cast_to_infer_ptr(this))); + } template bool owner_before(shared_ptr const& b) const { return true; /* FIXME - use non-det*/ @@ -334,5 +384,5 @@ shared_ptr make_shared(Args&&... args) { return shared_ptr(new T(std::forward(args)...)); } -#undef __data +#undef __cast_to_infer_ptr INFER_NAMESPACE_STD_END diff --git a/infer/src/clang/cFrontend_decl.ml b/infer/src/clang/cFrontend_decl.ml index e9bc775b5..e34dba9d0 100644 --- a/infer/src/clang/cFrontend_decl.ml +++ b/infer/src/clang/cFrontend_decl.ml @@ -225,10 +225,13 @@ struct | None -> ()) | _ -> ()); match dec with - (* Currently C/C++ record decl treated in the same way *) - | ClassTemplateSpecializationDecl (_, _, _, _, decl_list, _, _, _, _) - | CXXRecordDecl (_, _, _, _, decl_list, _, _, _) - | RecordDecl (_, _, _, _, decl_list, _, _) -> + (* Note that C and C++ records are treated the same way + Skip translating implicit struct declarations, unless they have + full definition (which happens with C++ lambdas) *) + | ClassTemplateSpecializationDecl (di, _, _, _, decl_list, _, rdi, _, _) + | CXXRecordDecl (di, _, _, _, decl_list, _, rdi, _) + | RecordDecl (di, _, _, _, decl_list, _, rdi) + when (not di.di_is_implicit) || rdi.rdi_is_complete_definition -> let is_method_decl decl = match decl with | CXXMethodDecl _ | CXXConstructorDecl _ | CXXConversionDecl _ | CXXDestructorDecl _ | FunctionTemplateDecl _ -> diff --git a/infer/src/clang/cTrans.ml b/infer/src/clang/cTrans.ml index 255921399..f028f0fa3 100644 --- a/infer/src/clang/cTrans.ml +++ b/infer/src/clang/cTrans.ml @@ -902,7 +902,7 @@ struct { res_trans_to_parent with exps = res_trans_call.exps } and cxx_method_construct_call_trans trans_state_pri result_trans_callee params_stmt - si function_type is_cpp_call_virtual = + si function_type is_cpp_call_virtual extra_res_trans = let open CContext in let context = trans_state_pri.context in let procname = Cfg.Procdesc.get_proc_name context.procdesc in @@ -935,13 +935,12 @@ struct let res_trans_call = create_call_instr trans_state_pri function_type sil_method actual_params sil_loc call_flags ~is_objc_method:false in let nname = "Call " ^ (Sil.exp_to_string sil_method) in - let all_res_trans = result_trans_subexprs @ [res_trans_call] in + let all_res_trans = result_trans_subexprs @ [res_trans_call; extra_res_trans] in let result_trans_to_parent = PriorityNode.compute_results_to_parent trans_state_pri sil_loc nname si all_res_trans in Cg.add_edge context.CContext.cg procname callee_pname; { result_trans_to_parent with exps = res_trans_call.exps } - and cxxMemberCallExpr_trans trans_state si stmt_list expr_info = let context = trans_state.context in (* Structure is the following: *) @@ -959,11 +958,12 @@ struct let fn_type_no_ref = CTypes_decl.get_type_from_expr_info expr_info context.CContext.tenv in let function_type = add_reference_if_glvalue fn_type_no_ref expr_info in cxx_method_construct_call_trans trans_state_pri result_trans_callee params_stmt - si function_type is_cpp_call_virtual + si function_type is_cpp_call_virtual empty_res_trans and cxxConstructExpr_trans trans_state si params_stmt ei cxx_constr_info = let context = trans_state.context in let trans_state_pri = PriorityNode.try_claim_priority_node trans_state si in + let sil_loc = CLocation.get_sil_location si context in let decl_ref = cxx_constr_info.Clang_ast_t.xcei_decl_ref in let var_exp, class_type = match trans_state.var_exp_typ with | Some exp_typ -> exp_typ @@ -978,11 +978,19 @@ struct exps = [(var_exp, this_type)]; initd_exps = [var_exp]; } in + let tmp_res_trans = { empty_res_trans with exps = [(var_exp, class_type)] } in + (* When class type is translated as pointer (std::shared_ptr for example), there needs + to be extra Load instruction before returning the trans_result of constructorExpr. + There is no LValueToRvalue cast in the AST afterwards since clang doesn't know + that class type is translated as pointer type. It gets added here instead. *) + let extra_res_trans = match class_type with + | Typ.Tptr _ -> dereference_value_from_result sil_loc tmp_res_trans ~strip_pointer:false + | _ -> tmp_res_trans in let res_trans_callee = decl_ref_trans trans_state this_res_trans si decl_ref ~is_constructor_init:false in let res_trans = cxx_method_construct_call_trans trans_state_pri res_trans_callee - params_stmt si Typ.Tvoid false in - { res_trans with exps = [(var_exp, class_type)] } + params_stmt si Typ.Tvoid false extra_res_trans in + { res_trans with exps=extra_res_trans.exps } and cxx_destructor_call_trans trans_state si this_res_trans class_type_ptr = let trans_state_pri = PriorityNode.try_claim_priority_node trans_state si in @@ -990,7 +998,7 @@ struct let is_cpp_call_virtual = res_trans_callee.is_cpp_call_virtual in if res_trans_callee.exps <> [] then cxx_method_construct_call_trans trans_state_pri res_trans_callee [] si Typ.Tvoid - is_cpp_call_virtual + is_cpp_call_virtual empty_res_trans else empty_res_trans and objCMessageExpr_trans_special_cases trans_state si obj_c_message_expr_info diff --git a/infer/src/clang/cTypes_decl.ml b/infer/src/clang/cTypes_decl.ml index 3bb7fd356..b8536d179 100644 --- a/infer/src/clang/cTypes_decl.ml +++ b/infer/src/clang/cTypes_decl.ml @@ -147,6 +147,25 @@ let add_struct_to_tenv tenv typ = let typename = Typename.TN_csu(csu, mangled) in Tenv.add tenv typename struct_typ +let get_translate_as_friend_decl decl_list = + let is_translate_as_friend_name (_, name_info) = + let translate_as_str = "infer_traits::TranslateAsType" in + string_contains translate_as_str (Ast_utils.get_qualified_name name_info) in + let get_friend_decl_opt (decl : Clang_ast_t.decl) = match decl with + | FriendDecl (_, `Type type_ptr) -> Ast_utils.get_decl_from_typ_ptr type_ptr + | _ -> None in + let is_translate_as_friend_decl decl = + match get_friend_decl_opt decl with + | Some decl -> + let named_decl_tuple_opt = Clang_ast_proj.get_named_decl_tuple decl in + Option.map_default is_translate_as_friend_name false named_decl_tuple_opt + | None -> false in + match get_friend_decl_opt (IList.find is_translate_as_friend_decl decl_list) with + | Some (Clang_ast_t.ClassTemplateSpecializationDecl (_, _, _, _, _, _, _, _, [`Type t_ptr])) -> + Some t_ptr + | _ -> None + | exception Not_found -> None + let rec get_struct_fields tenv decl = let open Clang_ast_t in let decl_list = match decl with @@ -166,7 +185,20 @@ let rec get_struct_fields tenv decl = IList.flatten (base_class_fields @ (IList.map do_one_decl decl_list)) (* For a record declaration it returns/constructs the type *) -and get_struct_cpp_class_declaration_type tenv decl = +and get_record_declaration_type tenv decl = + match get_record_custom_type tenv decl with + | Some t -> t + | None -> get_record_declaration_struct_type tenv decl + +and get_record_custom_type tenv decl = + let open Clang_ast_t in + match decl with + | ClassTemplateSpecializationDecl (_, _, _, _, decl_list, _, _, _, _) + | CXXRecordDecl (_, _, _, _, decl_list, _, _, _) -> + Option.map (type_ptr_to_sil_type tenv) (get_translate_as_friend_decl decl_list) + | _ -> None + +and get_record_declaration_struct_type tenv decl = let open Clang_ast_t in match decl with | ClassTemplateSpecializationDecl (_, _, _, type_ptr, decl_list, _, record_decl_info, _, _) @@ -231,7 +263,7 @@ and add_types_from_decl_to_tenv tenv decl = let open Clang_ast_t in match decl with | ClassTemplateSpecializationDecl _ | CXXRecordDecl _ | RecordDecl _ -> - get_struct_cpp_class_declaration_type tenv decl + get_record_declaration_type tenv decl | ObjCInterfaceDecl _ -> ObjcInterface_decl.interface_declaration type_ptr_to_sil_type tenv decl | ObjCImplementationDecl _ -> ObjcInterface_decl.interface_impl_declaration type_ptr_to_sil_type tenv decl diff --git a/infer/tests/codetoanalyze/cpp/frontend/lambda/lambda1.cpp b/infer/tests/codetoanalyze/cpp/frontend/lambda/lambda1.cpp index ec40890b4..c0aa964fd 100644 --- a/infer/tests/codetoanalyze/cpp/frontend/lambda/lambda1.cpp +++ b/infer/tests/codetoanalyze/cpp/frontend/lambda/lambda1.cpp @@ -16,7 +16,7 @@ int bar() { } int foo() { - + auto unused = []() { return 1 / 0; }; auto y = [](int i) { return ++i; }; return 5 / (4 - y(3)); } diff --git a/infer/tests/endtoend/cpp/infer/LambdaTest.java b/infer/tests/endtoend/cpp/infer/LambdaTest.java index 061962710..10fe53742 100644 --- a/infer/tests/endtoend/cpp/infer/LambdaTest.java +++ b/infer/tests/endtoend/cpp/infer/LambdaTest.java @@ -50,6 +50,7 @@ public class LambdaTest { String[] procedures = { "bar", "foo", + "foo::lambda_infer_tests_codetoanalyze_cpp_frontend_lambda_lambda1.cpp:19:17_operator()", }; InferResults inferResults = InferRunner.runInferCPP(inferCmd); assertThat(