[C++] Fix model of std::vector

Summary:
Infer used to report null dereference when field was accessed later:
```
vector<int> v;
int& a = v[0]; // should be EMPTY_VECTOR_ACCESS here, but it wasn't reported
int b = a; // was NULL_DEREFERENCE here
```
To avoid this problem, model all accesses to vector as dereference of its internal `beginPtr` field.

Reviewed By: jberdine

Differential Revision: D4481942

fbshipit-source-id: 2142894

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

@@ -19,6 +19,7 @@
 #include <cassert>
 
 #include <infer_model/common.h>
+#include <infer_model/infer_traits.h>
 
 #ifdef INFER_USE_LIBCPP
 // libc++ vector header includes it, but it breaks
@@ -53,6 +54,9 @@ struct vector_ref<bool> {
 template <class T>
 T* __infer_skip__get_nondet_val() {}
 
+template <class T>
+void __infer_deref_first_arg(T* ptr) INFER_MODEL_AS_DEREF_FIRST_ARG;
+
 // WARNING: do not add any new fields to std::vector model. sizeof(std::vector)
 // = 24 !!
 template <class _Tp, class _Allocator = allocator<_Tp>>
@@ -82,7 +86,14 @@ class vector {
   value_type* endPtr = nullptr;
   value_type* __ignore;
 
-  value_type* get() const { return beginPtr; }
+  value_type* get() const {
+    // we have to resort to that hack so that infer can truly dereference
+    // beginPtr.
+    // Another way to model that would be 'auto tmp = *beginPtr' but that will
+    // trigger copy constructor that may not exist for value_type
+    __infer_deref_first_arg(beginPtr);
+    return beginPtr;
+  }
 
   void allocate(size_type size) {
     // assume that allocation will produce non-empty vector regardless of the
@@ -98,11 +109,16 @@ class vector {
 
   template <class Iter>
   void allocate_iter(Iter begin, Iter end) {
-    if (begin != end) {
+    // very simplified implementation to avoid false positives
+    allocate(1);
+    // infer doesn't understand iterators well which leads to skip functions
+    // they in effect lead to false positives in situations when empty vector
+    // is impossible. Implemenatation should look like this:
+    /*if (begin != end) {
       allocate(1);
     } else {
       deallocate();
-    }
+    }*/
   }
 
   /* std::vector implementation */

infer/src/backend/InferPrint.re

@@ -344,7 +344,8 @@ let should_report (issue_kind: Exceptions.err_kind) issue_type error_desc eclass
               field_not_null_checked,
               null_dereference,
               parameter_not_null_checked,
-              premature_nil_termination
+              premature_nil_termination,
+              empty_vector_access
             ];
           List.mem equal::Localise.equal null_deref_issue_types issue_type
         };

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

@@ -118,6 +118,16 @@ codetoanalyze/cpp/errors/subtyping/dynamic_cast.cpp, dynamic__cast::wrongPointer
 codetoanalyze/cpp/errors/subtyping/dynamic_cast.cpp, dynamic__cast::wrongReferenceCast, 3, CLASS_CAST_EXCEPTION, [start of procedure dynamic__cast::wrongReferenceCast(),start of procedure Base,return from a call to dynamic__cast::Base_Base]
 codetoanalyze/cpp/errors/subtyping/dynamic_cast.cpp, dynamic__cast::wrongReferenceCastNotAssigned, 3, CLASS_CAST_EXCEPTION, [start of procedure dynamic__cast::wrongReferenceCastNotAssigned(),start of procedure Base,return from a call to dynamic__cast::Base_Base]
 codetoanalyze/cpp/errors/subtyping/subtyping_check.cpp, B_setFG, 4, DIVIDE_BY_ZERO, [start of procedure setFG,start of procedure setF,return from a call to A_setF,Condition is true]
+codetoanalyze/cpp/errors/vector/access_field_later.cpp, getIntPtr, 1, RETURN_VALUE_IGNORED, [start of procedure getIntPtr()]
+codetoanalyze/cpp/errors/vector/access_field_later.cpp, getIntPtr, 2, EMPTY_VECTOR_ACCESS, [start of procedure getIntPtr()]
+codetoanalyze/cpp/errors/vector/access_field_later.cpp, getWithCopy, 1, RETURN_VALUE_IGNORED, [start of procedure getWithCopy()]
+codetoanalyze/cpp/errors/vector/access_field_later.cpp, getWithCopy, 2, EMPTY_VECTOR_ACCESS, [start of procedure getWithCopy()]
+codetoanalyze/cpp/errors/vector/access_field_later.cpp, getWithCopyPtr, 1, RETURN_VALUE_IGNORED, [start of procedure getWithCopyPtr()]
+codetoanalyze/cpp/errors/vector/access_field_later.cpp, getWithCopyPtr, 2, EMPTY_VECTOR_ACCESS, [start of procedure getWithCopyPtr()]
+codetoanalyze/cpp/errors/vector/access_field_later.cpp, getWithoutCopy, 1, RETURN_VALUE_IGNORED, [start of procedure getWithoutCopy()]
+codetoanalyze/cpp/errors/vector/access_field_later.cpp, getWithoutCopy, 2, EMPTY_VECTOR_ACCESS, [start of procedure getWithoutCopy()]
+codetoanalyze/cpp/errors/vector/access_field_later.cpp, getWithoutCopyPtr, 1, RETURN_VALUE_IGNORED, [start of procedure getWithoutCopyPtr()]
+codetoanalyze/cpp/errors/vector/access_field_later.cpp, getWithoutCopyPtr, 2, EMPTY_VECTOR_ACCESS, [start of procedure getWithoutCopyPtr()]
 codetoanalyze/cpp/errors/vector/empty_access.cpp, access_empty, 2, EMPTY_VECTOR_ACCESS, [start of procedure access_empty()]
 codetoanalyze/cpp/errors/vector/empty_access.cpp, assign_empty, 4, EMPTY_VECTOR_ACCESS, [start of procedure assign_empty()]
 codetoanalyze/cpp/errors/vector/empty_access.cpp, clear_empty, 3, EMPTY_VECTOR_ACCESS, [start of procedure clear_empty()]
@@ -125,9 +135,9 @@ codetoanalyze/cpp/errors/vector/empty_access.cpp, copy_empty, 3, EMPTY_VECTOR_AC
 codetoanalyze/cpp/errors/vector/empty_access.cpp, empty_check_access_empty, 2, EMPTY_VECTOR_ACCESS, [start of procedure empty_check_access_empty(),Condition is true]
 codetoanalyze/cpp/errors/vector/empty_access.cpp, getter_empty, 0, EMPTY_VECTOR_ACCESS, [start of procedure getter_empty(),start of procedure get_vector(),return from a call to get_vector]
 codetoanalyze/cpp/errors/vector/empty_access.cpp, size_check0_empty, 2, EMPTY_VECTOR_ACCESS, [start of procedure size_check0_empty(),Condition is true]
-codetoanalyze/cpp/errors/vector/empty_access.cpp, vector_as_param_by_value_empty, 2, EMPTY_VECTOR_ACCESS, [start of procedure vector_as_param_by_value_empty(),start of procedure vector_param_by_value_access()]
+codetoanalyze/cpp/errors/vector/empty_access.cpp, vector_as_param_by_value_empty, 2, EMPTY_VECTOR_ACCESS, [start of procedure vector_as_param_by_value_empty(),start of procedure vector_param_by_value_access(),return from a call to vector_param_by_value_access]
 codetoanalyze/cpp/errors/vector/empty_access.cpp, vector_as_param_clear, 3, EMPTY_VECTOR_ACCESS, [start of procedure vector_as_param_clear(),start of procedure vector_param_clear(),return from a call to vector_param_clear]
-codetoanalyze/cpp/errors/vector/empty_access.cpp, vector_as_param_empty, 2, EMPTY_VECTOR_ACCESS, [start of procedure vector_as_param_empty(),start of procedure vector_param_access()]
+codetoanalyze/cpp/errors/vector/empty_access.cpp, vector_as_param_empty, 2, EMPTY_VECTOR_ACCESS, [start of procedure vector_as_param_empty(),start of procedure vector_param_access(),return from a call to vector_param_access]
 codetoanalyze/cpp/errors/vector/iterator_access.cpp, iterator_access::possible_npe, 4, NULL_DEREFERENCE, [start of procedure iterator_access::possible_npe(),Condition is true,Condition is true,Condition is true]
 codetoanalyze/cpp/shared/attributes/deprecated_hack.cpp, derefFirstArg2_null_deref, 2, NULL_DEREFERENCE, [start of procedure derefFirstArg2_null_deref()]
 codetoanalyze/cpp/shared/attributes/deprecated_hack.cpp, derefFirstArg3_null_deref, 2, NULL_DEREFERENCE, [start of procedure derefFirstArg3_null_deref(),start of procedure derefFirstArg3()]

infer/tests/codetoanalyze/cpp/errors/vector/access_field_later.cpp

@@ -0,0 +1,56 @@
+/*
+ * Copyright (c) 2017 - 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.
+ */
+#include <vector>
+
+// has copy constructor
+struct WithCopy {
+  int f1;
+  int f2;
+};
+
+// no copy constructor - important to catch wrong implementations of std::vector
+// model
+struct WithoutCopy {
+  WithoutCopy() = default;
+  WithoutCopy(WithoutCopy&& x) = default;
+  WithoutCopy(const WithoutCopy& x) = delete;
+  WithoutCopy& operator=(const WithoutCopy&) = delete;
+  int f1;
+  int f2;
+};
+
+int getIntPtr(int id, std::vector<int*>& v) {
+  int x = v.size();
+  int** res = &v[id];
+  return **res;
+}
+
+int getWithCopy(int id, std::vector<WithCopy>& v) {
+  int x = v.size();
+  WithCopy* res = &v[id];
+  return res->f1;
+}
+
+int getWithCopyPtr(int id, std::vector<WithCopy*>& v) {
+  int x = v.size();
+  WithCopy** res = &v[id];
+  return (*res)->f1;
+}
+
+int getWithoutCopy(int id, std::vector<WithoutCopy>& v) {
+  int x = v.size();
+  WithoutCopy* res = &v[id];
+  return res->f1;
+}
+
+int getWithoutCopyPtr(int id, std::vector<WithoutCopy*>& v) {
+  int x = v.size();
+  WithoutCopy** res = &v[id];
+  return (*res)->f1;
+}