Replace shared_ptr<T> structs with T* pointers

Summary:
Make std::shared_ptr<T> 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

@@ -1 +1 @@
-Subproject commit 9ce2266db9bdf23d70ddaed093decdeeb6bf8db3
+Subproject commit 0a3172fa8d00279b6bb98bedb12bd6cade4c4424

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 T>
+class TranslateAsType {};
+} // namespace infer_traits

infer/models/cpp/include/infer_model/shared_ptr.h

@@ -11,24 +11,66 @@
 // ASSERT that __cplusplus >= 201103L
 
 #include <infer_model/common.h>
+#include <infer_model/infer_traits.h>
 
 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 T>
 class shared_ptr : public std__shared_ptr<T> {
 
+  // translate shared_ptr as type T*
+  friend class infer_traits::TranslateAsType<T*>;
+
+  // shared_ptr<T> in infer is translated as T*.
+  // Some facts:
+  // 1. shared_ptr<T>* translated as T**
+  // 2. typeof(this) translated as T**
+  // 3. typeof(this) in clang's AST is shared_ptr<T>*
+  // 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<T> 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<T>::__ptr_;
-#define __data __ptr_
-#else
-  using __shared_ptr<T>::_M_ptr;
-#define __data _M_ptr
-#endif
   // Conversion constructors to allow implicit conversions.
   // it's here purely to avoid compilation errors
   template <class Y,
@@ -39,7 +81,9 @@ class shared_ptr : public std__shared_ptr<T> {
   shared_ptr(const std__shared_ptr<Y>& 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<T> {
   template <class Y,
             typename = typename enable_if<is_convertible<Y*, T*>::value>::type>
   explicit shared_ptr(Y* p) {
-    __data = p;
+    model_set(__cast_to_infer_ptr(this), p);
   }
 
   template <class Y,
@@ -74,27 +118,28 @@ class shared_ptr : public std__shared_ptr<T> {
 
   template <class Y>
   shared_ptr(const shared_ptr<Y>& 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<T>(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 <class Y,
+
             typename = typename enable_if<is_convertible<Y*, T*>::value>::type>
-  shared_ptr(const shared_ptr<Y>& r) noexcept
-      : shared_ptr<T>(r.__data) { /* TODO - increase refcount*/
+  shared_ptr(const shared_ptr<Y>& r) noexcept {
+    model_copy(__cast_to_infer_ptr(this), __cast_to_infer_ptr(&r));
   }
 
-  shared_ptr(shared_ptr&& r) noexcept : shared_ptr<T>(r.__data) {
-    r.__data = nullptr;
+  shared_ptr(shared_ptr&& r) noexcept {
+    model_move(__cast_to_infer_ptr(this), __cast_to_infer_ptr(&r));
   }
 
   template <class Y,
             typename = typename enable_if<is_convertible<Y*, T*>::value>::type>
-  shared_ptr(shared_ptr<Y>&& r) noexcept : shared_ptr<T>(r.__data) {
-    r.__data = nullptr;
+  shared_ptr(shared_ptr<Y>&& r) noexcept {
+    model_move(__cast_to_infer_ptr(this), __cast_to_infer_ptr(&r));
   }
 
   template <class Y,
@@ -122,7 +167,7 @@ class shared_ptr : public std__shared_ptr<T> {
   // assignment:
   shared_ptr& operator=(const shared_ptr& r) noexcept {
     // shared_ptr<T>(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<T> {
             typename = typename enable_if<is_convertible<Y*, T*>::value>::type>
   shared_ptr& operator=(const shared_ptr<Y>& r) noexcept {
     // shared_ptr<T>(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<T>(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<T> {
             typename = typename enable_if<is_convertible<Y*, T*>::value>::type>
   shared_ptr& operator=(shared_ptr<Y>&& r) {
     // shared_ptr<T>(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<T> {
 
   // 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<T> {
       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<T> {
   }
 
   // observers:
-  T* get() const noexcept { return __data; }
+
+  T* get() const noexcept { return model_get(__cast_to_infer_ptr(this)); }
   typename add_lvalue_reference<T>::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 <class U>
   bool owner_before(shared_ptr<U> const& b) const {
     return true; /* FIXME - use non-det*/
@@ -334,5 +384,5 @@ shared_ptr<T> make_shared(Args&&... args) {
   return shared_ptr<T>(new T(std::forward<Args>(args)...));
 }
 
-#undef __data
+#undef __cast_to_infer_ptr
 INFER_NAMESPACE_STD_END

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 _ ->

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

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

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));
 }

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(