[clang-tidy] [analyzer] Move nondeterministic pointer usage check to tidy

This change moves the alpha.nondeterministic.PointerSorting and
alpha.nondeterministic.PointerIteration static analyzer checkers to
a single clang-tidy check. Those checkers were implemented as clang-tidy
checks wrapped in the static analyzer framework. The documentation was
updated to describe what the checks can and cannot do, and testing
was completed on a broad set of open source projects.

Author: einvbri

clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp

@@ -49,6 +49,7 @@
 #include "MultipleStatementMacroCheck.h"
 #include "NoEscapeCheck.h"
 #include "NonZeroEnumToBoolConversionCheck.h"
+#include "NondeterministicPointerUsageCheck.h"
 #include "NotNullTerminatedResultCheck.h"
 #include "OptionalValueConversionCheck.h"
 #include "ParentVirtualCallCheck.h"
@@ -174,6 +175,8 @@ class BugproneModule : public ClangTidyModule {
         "bugprone-multiple-new-in-one-expression");
     CheckFactories.registerCheck<MultipleStatementMacroCheck>(
         "bugprone-multiple-statement-macro");
+    CheckFactories.registerCheck<NondeterministicPointerUsageCheck>(
+        "bugprone-nondeterministic-pointer-iteration-order");
     CheckFactories.registerCheck<OptionalValueConversionCheck>(
         "bugprone-optional-value-conversion");
     CheckFactories.registerCheck<PointerArithmeticOnPolymorphicObjectCheck>(

clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt

@@ -45,6 +45,7 @@ add_clang_library(clangTidyBugproneModule
   MultipleNewInOneExpressionCheck.cpp
   MultipleStatementMacroCheck.cpp
   NoEscapeCheck.cpp
+  NondeterministicPointerUsageCheck.cpp
   NonZeroEnumToBoolConversionCheck.cpp
   NotNullTerminatedResultCheck.cpp
   OptionalValueConversionCheck.cpp

clang-tools-extra/clang-tidy/bugprone/NondeterministicPointerUsageCheck.cpp

@@ -0,0 +1,67 @@
+//===------- NondetermnisticPointerUsageCheck.cpp - clang-tidy ------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "NondeterministicPointerUsageCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/Lex/Lexer.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::bugprone {
+
+void NondeterministicPointerUsageCheck::registerMatchers(MatchFinder *Finder) {
+
+  auto LoopVariable = varDecl(hasType(hasCanonicalType(pointerType())));
+
+  auto RangeInit = declRefExpr(to(varDecl(hasType(recordDecl(
+      anyOf(hasName("std::unordered_set"), hasName("std::unordered_map"),
+            hasName("std::unordered_multiset"),
+            hasName("std::unordered_multimap")))))));
+
+  Finder->addMatcher(
+      stmt(cxxForRangeStmt(hasRangeInit(RangeInit.bind("rangeinit")),
+                           hasLoopVariable(LoopVariable.bind("loopVar"))))
+          .bind("cxxForRangeStmt"),
+      this);
+
+  auto SortFuncM = anyOf(callee(functionDecl(hasName("std::is_sorted"))),
+                         callee(functionDecl(hasName("std::nth_element"))),
+                         callee(functionDecl(hasName("std::sort"))),
+                         callee(functionDecl(hasName("std::partial_sort"))),
+                         callee(functionDecl(hasName("std::partition"))),
+                         callee(functionDecl(hasName("std::stable_partition"))),
+                         callee(functionDecl(hasName("std::stable_sort"))));
+
+  auto IteratesPointerEltsM = hasArgument(
+      0,
+      cxxMemberCallExpr(on(hasType(cxxRecordDecl(has(fieldDecl(hasType(
+          hasCanonicalType(pointsTo(hasCanonicalType(pointerType())))))))))));
+
+  Finder->addMatcher(stmt(callExpr(allOf(SortFuncM, IteratesPointerEltsM)))
+                         .bind("sortsemantic"),
+                     this);
+}
+
+void NondeterministicPointerUsageCheck::check(
+    const MatchFinder::MatchResult &Result) {
+  const auto *ForRangePointers =
+      Result.Nodes.getNodeAs<Stmt>("cxxForRangeStmt");
+  const auto *SortPointers = Result.Nodes.getNodeAs<Stmt>("sortsemantic");
+
+  if ((ForRangePointers) && !(ForRangePointers->getBeginLoc().isMacroID())) {
+    const auto *Node = dyn_cast<CXXForRangeStmt>(ForRangePointers);
+    diag(Node->getRParenLoc(), "Iteration of pointers is nondeterministic");
+  }
+
+  if ((SortPointers) && !(SortPointers->getBeginLoc().isMacroID())) {
+    const auto *Node = dyn_cast<Stmt>(SortPointers);
+    diag(Node->getBeginLoc(), "Sorting pointers is nondeterministic");
+  }
+}
+
+} // namespace clang::tidy::bugprone

clang-tools-extra/clang-tidy/bugprone/NondeterministicPointerUsageCheck.h

@@ -0,0 +1,34 @@
+//===------- NondeterministicPointerUsageCheck.h - clang-tidy ---*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_NONDETERMINISTICPOINTERUSAGECHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_NONDETERMINISTICPOINTERUSAGECHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang::tidy::bugprone {
+
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/bugprone/nondeterministic-pointer-usage.html
+class NondeterministicPointerUsageCheck : public ClangTidyCheck {
+public:
+  NondeterministicPointerUsageCheck(StringRef Name, ClangTidyContext *Context)
+      : ClangTidyCheck(Name, Context) {}
+  bool isLanguageVersionSupported(const LangOptions &LangOpts) const override {
+    return LangOpts.CPlusPlus;
+  }
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+  std::optional<TraversalKind> getCheckTraversalKind() const override {
+    return TK_IgnoreUnlessSpelledInSource;
+  }
+};
+
+} // namespace clang::tidy::bugprone
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_NONDETERMINISTICPOINTERUSAGECHECK_H

clang-tools-extra/docs/ReleaseNotes.rst

@@ -121,6 +121,12 @@ New checks
   Gives warnings for tagged unions, where the number of tags is
   different from the number of data members inside the union.
 
+- New :doc:`bugprone-nondeterministic-pointer-iteration-order
+  <clang-tidy/checks/bugprone/nondeterministic-pointer-iteration-order>`
+  check.
+
+  Finds nondeterministic usages of pointers in unordered containers.
+
 New check aliases
 ^^^^^^^^^^^^^^^^^
 

clang-tools-extra/docs/clang-tidy/checks/bugprone/nondeterministic-pointer-iteration-order.rst

@@ -0,0 +1,35 @@
+.. title:: clang-tidy - bugprone-nondeterministic-pointer-iteration-order
+
+bugprone-nondeterministic-pointer-iteration-order
+=================================================
+
+Finds nondeterministic usages of pointers in unordered containers.
+
+One canonical example is iteration across a container of pointers.
+
+.. code-block:: c++
+
+  {
+    int a = 1, b = 2;
+    std::unordered_set<int *> UnorderedPtrSet = {&a, &b};
+    for (auto i : UnorderedPtrSet)
+      f(i);
+  }
+
+Another such example is sorting a container of pointers.
+
+.. code-block:: c++
+
+  {
+    int a = 1, b = 2;
+    std::vector<int *> VectorOfPtr = {&a, &b};
+    std::sort(VectorOfPtr.begin(), VectorOfPtr.end());
+  }
+
+Iteration of a containers of pointers may present the order of different
+pointers differently across different runs of a program. In some cases this
+may be acceptable behavior, in others this may be unexpected behavior. This
+check is advisory for this reason.
+
+This check only detects range-based for loops over unordered sets. Other
+similar usages will not be found and are false negatives.

clang-tools-extra/test/clang-tidy/checkers/bugprone/Inputs/system-header-simulator/sim_algorithm

@@ -0,0 +1,31 @@
+#ifndef _SIM_ALGORITHM
+#define _SIM_ALGORITHM
+
+#pragma clang system_header
+
+namespace std {
+
+template<class ForwardIt>
+bool is_sorted(ForwardIt first, ForwardIt last);
+
+template <class RandomIt>
+void nth_element(RandomIt first, RandomIt nth, RandomIt last);
+
+template<class RandomIt>
+void partial_sort(RandomIt first, RandomIt middle, RandomIt last);
+
+template<class RandomIt>
+void sort (RandomIt first, RandomIt last);
+
+template<class RandomIt>
+void stable_sort(RandomIt first, RandomIt last);
+
+template<class BidirIt, class UnaryPredicate>
+BidirIt partition(BidirIt first, BidirIt last, UnaryPredicate p);
+
+template<class BidirIt, class UnaryPredicate>
+BidirIt stable_partition(BidirIt first, BidirIt last, UnaryPredicate p);
+
+} // namespace std
+
+#endif // _SIM_ALGORITHM

clang-tools-extra/test/clang-tidy/checkers/bugprone/Inputs/system-header-simulator/sim_c++config.h

@@ -0,0 +1,11 @@
+#ifndef _SIM_CPP_CONFIG_H
+#define _SIM_CPP_CONFIG_H
+
+#pragma clang system_header
+
+typedef unsigned char uint8_t;
+
+typedef __typeof__(sizeof(int)) size_t;
+typedef __typeof__((char*)0-(char*)0) ptrdiff_t;
+
+#endif // _SIM_CPP_CONFIG_H

clang-tools-extra/test/clang-tidy/checkers/bugprone/Inputs/system-header-simulator/sim_initializer_list

@@ -0,0 +1,39 @@
+#ifndef _INITIALIZER_LIST
+#define _INITIALIZER_LIST
+
+#pragma clang system_header
+#
+#include "sim_c++config.h" // size_t
+
+namespace std {
+
+template <class _E>
+class initializer_list  {
+  const _E* __begin_;
+  size_t    __size_;
+
+  initializer_list(const _E* __b, size_t __s)
+    : __begin_(__b),
+      __size_(__s)
+  {}
+
+public:
+  typedef _E        value_type;
+  typedef const _E& reference;
+  typedef const _E& const_reference;
+  typedef size_t    size_type;
+
+  typedef const _E* iterator;
+  typedef const _E* const_iterator;
+
+  initializer_list() : __begin_(0), __size_(0) {}
+
+  size_t    size()  const {return __size_;}
+  const _E* begin() const {return __begin_;}
+  const _E* end()   const {return __begin_ + __size_;}
+
+}; // class initializer_list
+
+} // namespace std
+
+#endif // _INITIALIZER_LIST

clang-tools-extra/test/clang-tidy/checkers/bugprone/Inputs/system-header-simulator/sim_iterator_base

@@ -0,0 +1,22 @@
+#ifndef _SIM_ITERATOR_BASE
+#define _SIM_ITERATOR_BASE
+
+namespace std {
+
+struct input_iterator_tag { };
+struct output_iterator_tag { };
+struct forward_iterator_tag : public input_iterator_tag { };
+struct bidirectional_iterator_tag : public forward_iterator_tag { };
+struct random_access_iterator_tag : public bidirectional_iterator_tag { };
+
+template <typename Iterator> struct iterator_traits {
+  typedef typename Iterator::difference_type difference_type;
+  typedef typename Iterator::value_type value_type;
+  typedef typename Iterator::pointer pointer;
+  typedef typename Iterator::reference reference;
+  typedef typename Iterator::iterator_category iterator_category;
+};
+
+} // namespace std
+
+#endif // _SIM_ITERATOR_BASE

clang-tools-extra/test/clang-tidy/checkers/bugprone/Inputs/system-header-simulator/sim_map

@@ -0,0 +1,35 @@
+
+#ifndef _SIM_MAP
+#define _SIM_MAP
+
+#pragma clang system_header
+#include "sim_stl_pair"
+
+namespace std {
+
+template <typename Key, typename Value>
+class map {
+ public:
+  using value_type = pair<Key, Value>;
+  map();
+  map(initializer_list<pair<Key, Value>> initList);
+  value_type& operator[](const Key& key);
+  value_type& operator[](Key&& key);
+    class iterator {
+    public:
+      iterator(Key *key): ptr(key) {}
+      iterator& operator++() { ++ptr; return *this; }
+      bool operator!=(const iterator &other) const { return ptr != other.ptr; }
+      const Key &operator*() const { return *ptr; }
+    private:
+      Key *ptr;
+    };
+  public:
+    Key *val;
+    iterator begin() const { return iterator(val); }
+    iterator end() const { return iterator(val + 1); }
+};
+
+} // namespace std
+
+#endif // _SIM_MAP

clang-tools-extra/test/clang-tidy/checkers/bugprone/Inputs/system-header-simulator/sim_set

@@ -0,0 +1,45 @@
+
+#ifndef _SIM_SET
+#define _SIM_SET
+
+#pragma clang system_header
+#include "sim_initializer_list"
+
+namespace std {
+
+template< class T = void >
+struct less;
+
+template< class T >
+struct allocator;
+
+template< class Key >
+struct hash;
+
+template<
+  class Key,
+  class Compare = std::less<Key>,
+  class Alloc = std::allocator<Key>
+> class set {
+  public:
+    set(initializer_list<Key> __list) {}
+
+    class iterator {
+    public:
+      iterator(Key *key): ptr(key) {}
+      iterator& operator++() { ++ptr; return *this; }
+      bool operator!=(const iterator &other) const { return ptr != other.ptr; }
+      const Key &operator*() const { return *ptr; }
+    private:
+      Key *ptr;
+    };
+
+  public:
+    Key *val;
+    iterator begin() const { return iterator(val); }
+    iterator end() const { return iterator(val + 1); }
+};
+
+} // namespace std
+
+#endif // _SIM_SET

clang-tools-extra/test/clang-tidy/checkers/bugprone/Inputs/system-header-simulator/sim_stl_pair

@@ -0,0 +1,33 @@
+#ifndef _SIM_STL_PAIR
+#define _SIM_STL_PAIR
+
+#pragma clang system_header
+
+#include "sim_type_traits"
+
+namespace std {
+
+
+template <class T1, class T2>
+struct pair {
+  T1 first;
+  T2 second;
+
+  pair() : first(), second() {}
+  pair(const T1 &a, const T2 &b) : first(a), second(b) {}
+
+  template<class U1, class U2>
+  pair(const pair<U1, U2> &other) : first(other.first),
+                                      second(other.second) {}
+};
+
+template <typename T1, typename T2>
+pair<typename remove_reference<T1>::type, typename remove_reference<T2>::type>
+make_pair(T1 &&, T2 &&) {
+  return {};
+};
+
+} // namespace std
+
+#endif // _SIM_STL_PAIR
+