[clang] 9e0fc67 - [clang][dataflow] Model the behavior of various optional members

Stanislav Gatev via cfe-commits cfe-commits at lists.llvm.org
Sun Mar 13 23:51:23 PDT 2022


Author: Stanislav Gatev
Date: 2022-03-14T06:50:14Z
New Revision: 9e0fc67683781fe4bd9bc4085084faec1126b41b

URL: https://github.com/llvm/llvm-project/commit/9e0fc67683781fe4bd9bc4085084faec1126b41b
DIFF: https://github.com/llvm/llvm-project/commit/9e0fc67683781fe4bd9bc4085084faec1126b41b.diff

LOG: [clang][dataflow] Model the behavior of various optional members

Model `make_optional`, optional's default constructor, `emplace`,
`reset`, and `operator bool` members.

Reviewed-by: xazax.hun

Differential Revision: https://reviews.llvm.org/D121378

Added: 
    

Modified: 
    clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
    clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp

Removed: 
    


################################################################################
diff  --git a/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp b/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
index bb6aa5ff74e2c..0963e8e452448 100644
--- a/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
+++ b/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
@@ -11,6 +11,8 @@
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Casting.h"
 #include <cassert>
+#include <memory>
+#include <utility>
 
 namespace clang {
 namespace dataflow {
@@ -41,6 +43,21 @@ static auto isOptionalOperatorCallWithName(llvm::StringRef OperatorName) {
                              callee(cxxMethodDecl(ofClass(optionalClass()))));
 }
 
+static auto isMakeOptionalCall() {
+  return callExpr(
+      callee(functionDecl(hasAnyName(
+          "std::make_optional", "base::make_optional", "absl::make_optional"))),
+      hasOptionalType());
+}
+
+/// Creates a symbolic value for an `optional` value using `HasValueVal` as the
+/// symbolic value of its "has_value" property.
+StructValue &createOptionalValue(Environment &Env, BoolValue &HasValueVal) {
+  auto OptionalVal = std::make_unique<StructValue>();
+  OptionalVal->setProperty("has_value", HasValueVal);
+  return Env.takeOwnership(std::move(OptionalVal));
+}
+
 /// Returns the symbolic value that represents the "has_value" property of the
 /// optional value `Val`. Returns null if `Val` is null.
 static BoolValue *getHasValue(Value *Val) {
@@ -75,6 +92,13 @@ static void transferUnwrapCall(const Expr *UnwrapExpr, const Expr *ObjectExpr,
   State.Lattice.getSourceLocations().insert(ObjectExpr->getBeginLoc());
 }
 
+void transferMakeOptionalCall(const CallExpr *E, LatticeTransferState &State) {
+  auto &Loc = State.Env.createStorageLocation(*E);
+  State.Env.setStorageLocation(*E, Loc);
+  State.Env.setValue(
+      Loc, createOptionalValue(State.Env, State.Env.getBoolLiteralValue(true)));
+}
+
 static void transferOptionalHasValueCall(const CXXMemberCallExpr *CallExpr,
                                          LatticeTransferState &State) {
   if (auto *OptionalVal = cast_or_null<StructValue>(
@@ -89,6 +113,26 @@ static void transferOptionalHasValueCall(const CXXMemberCallExpr *CallExpr,
   }
 }
 
+void transferEmplaceCall(const CXXMemberCallExpr *E,
+                         LatticeTransferState &State) {
+  if (auto *OptionalLoc = State.Env.getStorageLocation(
+          *E->getImplicitObjectArgument(), SkipPast::ReferenceThenPointer)) {
+    State.Env.setValue(
+        *OptionalLoc,
+        createOptionalValue(State.Env, State.Env.getBoolLiteralValue(true)));
+  }
+}
+
+void transferResetCall(const CXXMemberCallExpr *E,
+                       LatticeTransferState &State) {
+  if (auto *OptionalLoc = State.Env.getStorageLocation(
+          *E->getImplicitObjectArgument(), SkipPast::ReferenceThenPointer)) {
+    State.Env.setValue(
+        *OptionalLoc,
+        createOptionalValue(State.Env, State.Env.getBoolLiteralValue(false)));
+  }
+}
+
 static auto buildTransferMatchSwitch() {
   return MatchSwitchBuilder<LatticeTransferState>()
       // Attach a symbolic "has_value" state to optional values that we see for
@@ -96,6 +140,9 @@ static auto buildTransferMatchSwitch() {
       .CaseOf(expr(anyOf(declRefExpr(), memberExpr()), hasOptionalType()),
               initializeOptionalReference)
 
+      // make_optional
+      .CaseOf(isMakeOptionalCall(), transferMakeOptionalCall)
+
       // optional::value
       .CaseOf(
           isOptionalMemberCallWithName("value"),
@@ -115,6 +162,16 @@ static auto buildTransferMatchSwitch() {
       .CaseOf(isOptionalMemberCallWithName("has_value"),
               transferOptionalHasValueCall)
 
+      // optional::operator bool
+      .CaseOf(isOptionalMemberCallWithName("operator bool"),
+              transferOptionalHasValueCall)
+
+      // optional::emplace
+      .CaseOf(isOptionalMemberCallWithName("emplace"), transferEmplaceCall)
+
+      // optional::reset
+      .CaseOf(isOptionalMemberCallWithName("reset"), transferResetCall)
+
       .Build();
 }
 

diff  --git a/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp b/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
index e3840e1171f39..3ea39c6be9993 100644
--- a/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
+++ b/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
@@ -29,9 +29,23 @@ using namespace test;
 using ::testing::Pair;
 using ::testing::UnorderedElementsAre;
 
+// FIXME: Move header definitions in separate file(s).
 static constexpr char StdTypeTraitsHeader[] = R"(
+#ifndef TYPE_TRAITS_H
+#define TYPE_TRAITS_H
+
 namespace std {
 
+typedef decltype(sizeof(char)) size_t;
+
+template <typename T, T V>
+struct integral_constant {
+  static constexpr T value = V;
+};
+
+using true_type = integral_constant<bool, true>;
+using false_type = integral_constant<bool, false>;
+
 template< class T > struct remove_reference      {typedef T type;};
 template< class T > struct remove_reference<T&>  {typedef T type;};
 template< class T > struct remove_reference<T&&> {typedef T type;};
@@ -39,21 +53,135 @@ template< class T > struct remove_reference<T&&> {typedef T type;};
 template <class T>
   using remove_reference_t = typename remove_reference<T>::type;
 
+template <class T>
+struct remove_extent {
+  typedef T type;
+};
+
+template <class T>
+struct remove_extent<T[]> {
+  typedef T type;
+};
+
+template <class T, size_t N>
+struct remove_extent<T[N]> {
+  typedef T type;
+};
+
+template <class T>
+struct is_array : false_type {};
+
+template <class T>
+struct is_array<T[]> : true_type {};
+
+template <class T, size_t N>
+struct is_array<T[N]> : true_type {};
+
+template <class>
+struct is_function : false_type {};
+
+template <class Ret, class... Args>
+struct is_function<Ret(Args...)> : true_type {};
+
+namespace detail {
+
+template <class T>
+struct type_identity {
+  using type = T;
+};  // or use type_identity (since C++20)
+
+template <class T>
+auto try_add_pointer(int) -> type_identity<typename remove_reference<T>::type*>;
+template <class T>
+auto try_add_pointer(...) -> type_identity<T>;
+
+}  // namespace detail
+
+template <class T>
+struct add_pointer : decltype(detail::try_add_pointer<T>(0)) {};
+
+template <bool B, class T, class F>
+struct conditional {
+  typedef T type;
+};
+
+template <class T, class F>
+struct conditional<false, T, F> {
+  typedef F type;
+};
+
+template <class T>
+struct remove_cv {
+  typedef T type;
+};
+template <class T>
+struct remove_cv<const T> {
+  typedef T type;
+};
+template <class T>
+struct remove_cv<volatile T> {
+  typedef T type;
+};
+template <class T>
+struct remove_cv<const volatile T> {
+  typedef T type;
+};
+
+template <class T>
+struct decay {
+ private:
+  typedef typename remove_reference<T>::type U;
+
+ public:
+  typedef typename conditional<
+      is_array<U>::value, typename remove_extent<U>::type*,
+      typename conditional<is_function<U>::value, typename add_pointer<U>::type,
+                           typename remove_cv<U>::type>::type>::type type;
+};
+
 } // namespace std
+
+#endif // TYPE_TRAITS_H
 )";
 
 static constexpr char StdUtilityHeader[] = R"(
+#ifndef UTILITY_H
+#define UTILITY_H
+
 #include "std_type_traits.h"
 
 namespace std {
 
 template <typename T>
-constexpr std::remove_reference_t<T>&& move(T&& x);
+constexpr remove_reference_t<T>&& move(T&& x);
+
+} // namespace std
+
+#endif // UTILITY_H
+)";
+
+static constexpr char StdInitializerListHeader[] = R"(
+#ifndef INITIALIZER_LIST_H
+#define INITIALIZER_LIST_H
+
+namespace std {
+
+template <typename T>
+class initializer_list {
+ public:
+  initializer_list() noexcept;
+};
 
 } // namespace std
+
+#endif // INITIALIZER_LIST_H
 )";
 
 static constexpr char StdOptionalHeader[] = R"(
+#include "std_initializer_list.h"
+#include "std_type_traits.h"
+#include "std_utility.h"
+
 namespace std {
 
 template <typename T>
@@ -74,13 +202,41 @@ class optional {
   const T&& value() const&&;
   T&& value() &&;
 
+  template <typename U>
+  constexpr T value_or(U&& v) const&;
+  template <typename U>
+  T value_or(U&& v) &&;
+
+  template <typename... Args>
+  T& emplace(Args&&... args);
+
+  template <typename U, typename... Args>
+  T& emplace(std::initializer_list<U> ilist, Args&&... args);
+
+  void reset() noexcept;
+
+  constexpr explicit operator bool() const noexcept;
   constexpr bool has_value() const noexcept;
 };
 
+template <typename T>
+constexpr optional<typename std::decay<T>::type> make_optional(T&& v);
+
+template <typename T, typename... Args>
+constexpr optional<T> make_optional(Args&&... args);
+
+template <typename T, typename U, typename... Args>
+constexpr optional<T> make_optional(std::initializer_list<U> il,
+                                    Args&&... args);
+
 } // namespace std
 )";
 
 static constexpr char AbslOptionalHeader[] = R"(
+#include "std_initializer_list.h"
+#include "std_type_traits.h"
+#include "std_utility.h"
+
 namespace absl {
 
 template <typename T>
@@ -101,13 +257,41 @@ class optional {
   const T&& value() const&&;
   T&& value() &&;
 
+  template <typename U>
+  constexpr T value_or(U&& v) const&;
+  template <typename U>
+  T value_or(U&& v) &&;
+
+  template <typename... Args>
+  T& emplace(Args&&... args);
+
+  template <typename U, typename... Args>
+  T& emplace(std::initializer_list<U> ilist, Args&&... args);
+
+  void reset() noexcept;
+
+  constexpr explicit operator bool() const noexcept;
   constexpr bool has_value() const noexcept;
 };
 
+template <typename T>
+constexpr optional<typename std::decay<T>::type> make_optional(T&& v);
+
+template <typename T, typename... Args>
+constexpr optional<T> make_optional(Args&&... args);
+
+template <typename T, typename U, typename... Args>
+constexpr optional<T> make_optional(std::initializer_list<U> il,
+                                    Args&&... args);
+
 } // namespace absl
 )";
 
 static constexpr char BaseOptionalHeader[] = R"(
+#include "std_initializer_list.h"
+#include "std_type_traits.h"
+#include "std_utility.h"
+
 namespace base {
 
 template <typename T>
@@ -128,9 +312,33 @@ class Optional {
   const T&& value() const&&;
   T&& value() &&;
 
+  template <typename U>
+  constexpr T value_or(U&& v) const&;
+  template <typename U>
+  T value_or(U&& v) &&;
+
+  template <typename... Args>
+  T& emplace(Args&&... args);
+
+  template <typename U, typename... Args>
+  T& emplace(std::initializer_list<U> ilist, Args&&... args);
+
+  void reset() noexcept;
+
+  constexpr explicit operator bool() const noexcept;
   constexpr bool has_value() const noexcept;
 };
 
+template <typename T>
+constexpr Optional<typename std::decay<T>::type> make_optional(T&& v);
+
+template <typename T, typename... Args>
+constexpr Optional<T> make_optional(Args&&... args);
+
+template <typename T, typename U, typename... Args>
+constexpr Optional<T> make_optional(std::initializer_list<U> il,
+                                    Args&&... args);
+
 } // namespace base
 )";
 
@@ -177,6 +385,7 @@ class UncheckedOptionalAccessTest
     ReplaceAllOccurrences(SourceCode, "$optional", GetParam().TypeName);
 
     std::vector<std::pair<std::string, std::string>> Headers;
+    Headers.emplace_back("std_initializer_list.h", StdInitializerListHeader);
     Headers.emplace_back("std_type_traits.h", StdTypeTraitsHeader);
     Headers.emplace_back("std_utility.h", StdUtilityHeader);
     Headers.emplace_back("std_optional.h", StdOptionalHeader);
@@ -185,8 +394,12 @@ class UncheckedOptionalAccessTest
     Headers.emplace_back("unchecked_optional_access_test.h", R"(
       #include "absl_optional.h"
       #include "base_optional.h"
+      #include "std_initializer_list.h"
       #include "std_optional.h"
       #include "std_utility.h"
+
+      template <typename T>
+      T Make();
     )");
     const tooling::FileContentMappings FileContents(Headers.begin(),
                                                     Headers.end());
@@ -315,7 +528,7 @@ TEST_P(UncheckedOptionalAccessTest, UnwrapUsingOperatorArrowNoCheck) {
       UnorderedElementsAre(Pair("check", "unsafe: input.cc:9:7")));
 }
 
-TEST_P(UncheckedOptionalAccessTest, UnwrapWithCheck) {
+TEST_P(UncheckedOptionalAccessTest, HasValueCheck) {
   ExpectLatticeChecksFor(R"(
     #include "unchecked_optional_access_test.h"
 
@@ -329,13 +542,179 @@ TEST_P(UncheckedOptionalAccessTest, UnwrapWithCheck) {
                          UnorderedElementsAre(Pair("check", "safe")));
 }
 
+TEST_P(UncheckedOptionalAccessTest, OperatorBoolCheck) {
+  ExpectLatticeChecksFor(R"(
+    #include "unchecked_optional_access_test.h"
+
+    void target($ns::$optional<int> opt) {
+      if (opt) {
+        opt.value();
+        /*[[check]]*/
+      }
+    }
+  )",
+                         UnorderedElementsAre(Pair("check", "safe")));
+}
+
+TEST_P(UncheckedOptionalAccessTest, UnwrapFunctionCallResultNoCheck) {
+  ExpectLatticeChecksFor(
+      R"(
+    #include "unchecked_optional_access_test.h"
+
+    void target() {
+      Make<$ns::$optional<int>>().value();
+      (void)0;
+      /*[[check]]*/
+    }
+  )",
+      UnorderedElementsAre(Pair("check", "unsafe: input.cc:5:7")));
+
+  ExpectLatticeChecksFor(
+      R"(
+    #include "unchecked_optional_access_test.h"
+
+    void target($ns::$optional<int> opt) {
+      std::move(opt).value();
+      /*[[check]]*/
+    }
+  )",
+      UnorderedElementsAre(Pair("check", "unsafe: input.cc:5:7")));
+}
+
+TEST_P(UncheckedOptionalAccessTest, DefaultConstructor) {
+  ExpectLatticeChecksFor(
+      R"(
+    #include "unchecked_optional_access_test.h"
+
+    void target() {
+      $ns::$optional<int> opt;
+      opt.value();
+      /*[[check]]*/
+    }
+  )",
+      UnorderedElementsAre(Pair("check", "unsafe: input.cc:6:7")));
+}
+
+TEST_P(UncheckedOptionalAccessTest, MakeOptional) {
+  ExpectLatticeChecksFor(R"(
+    #include "unchecked_optional_access_test.h"
+
+    void target() {
+      $ns::$optional<int> opt = $ns::make_optional(0);
+      opt.value();
+      /*[[check]]*/
+    }
+  )",
+                         UnorderedElementsAre(Pair("check", "safe")));
+
+  ExpectLatticeChecksFor(R"(
+    #include "unchecked_optional_access_test.h"
+
+    struct Foo {
+      Foo(int, int);
+    };
+
+    void target() {
+      $ns::$optional<Foo> opt = $ns::make_optional<Foo>(21, 22);
+      opt.value();
+      /*[[check]]*/
+    }
+  )",
+                         UnorderedElementsAre(Pair("check", "safe")));
+
+  ExpectLatticeChecksFor(R"(
+    #include "unchecked_optional_access_test.h"
+
+    struct Foo {
+      constexpr Foo(std::initializer_list<char>);
+    };
+
+    void target() {
+      char a = 'a';
+      $ns::$optional<Foo> opt = $ns::make_optional<Foo>({a});
+      opt.value();
+      /*[[check]]*/
+    }
+  )",
+                         UnorderedElementsAre(Pair("check", "safe")));
+}
+
+TEST_P(UncheckedOptionalAccessTest, ValueOr) {
+  ExpectLatticeChecksFor(R"(
+    #include "unchecked_optional_access_test.h"
+
+    void target() {
+      $ns::$optional<int> opt;
+      opt.value_or(0);
+      (void)0;
+      /*[[check]]*/
+    }
+  )",
+                         UnorderedElementsAre(Pair("check", "safe")));
+}
+
+TEST_P(UncheckedOptionalAccessTest, Emplace) {
+  ExpectLatticeChecksFor(R"(
+    #include "unchecked_optional_access_test.h"
+
+    void target() {
+      $ns::$optional<int> opt;
+      opt.emplace(0);
+      opt.value();
+      /*[[check]]*/
+    }
+  )",
+                         UnorderedElementsAre(Pair("check", "safe")));
+
+  ExpectLatticeChecksFor(R"(
+    #include "unchecked_optional_access_test.h"
+
+    void target($ns::$optional<int> *opt) {
+      opt->emplace(0);
+      opt->value();
+      /*[[check]]*/
+    }
+  )",
+                         UnorderedElementsAre(Pair("check", "safe")));
+
+  // FIXME: Add tests that call `emplace` in conditional branches.
+}
+
+TEST_P(UncheckedOptionalAccessTest, Reset) {
+  ExpectLatticeChecksFor(
+      R"(
+    #include "unchecked_optional_access_test.h"
+
+    void target() {
+      $ns::$optional<int> opt = $ns::make_optional(0);
+      opt.reset();
+      opt.value();
+      /*[[check]]*/
+    }
+  )",
+      UnorderedElementsAre(Pair("check", "unsafe: input.cc:7:7")));
+
+  ExpectLatticeChecksFor(
+      R"(
+    #include "unchecked_optional_access_test.h"
+
+    void target() {
+      $ns::$optional<int> *opt;
+      *opt = $ns::make_optional(0);
+      opt->reset();
+      opt->value();
+      /*[[check]]*/
+    }
+  )",
+      UnorderedElementsAre(Pair("check", "unsafe: input.cc:8:7")));
+
+  // FIXME: Add tests that call `reset` in conditional branches.
+}
+
 // FIXME: Add support for:
-// - constructors (default, copy, move, non-standard)
+// - constructors (copy, move, non-standard)
 // - assignment operators (default, copy, move, non-standard)
-// - operator bool
-// - emplace
-// - reset
-// - value_or
 // - swap
-// - make_optional
 // - invalidation (passing optional by non-const reference/pointer)
+// - `value_or(nullptr) != nullptr`, `value_or(0) != 0`, `value_or("").empty()`
+// - nested `optional` values


        


More information about the cfe-commits mailing list