[-Wunsafe-buffer-usage][BoundsSafety] Extend safe patterns of arguments to counted_by parameters (#9971)

* [-Wunsafe-buffer-usage][BoundsSafety] Extend safe patterns of arguments to counted_by parameters

Add support of
- overloaded subscript operator of hardened view/containers, and
- calls to const member functions of 0 arguments

Now it can match patterns like:

```
std::span<char> D, S;
std::array<std::span<char>, 10> A;

memcpy(D.first(S.size()).data(), S.data(), S.size());
memcpy(D.first(A[x].size()).data(), A[x].data(), A[x].size());
```

(rdar://143986251)

Author: ziqingluo-90

clang/lib/Analysis/UnsafeBufferUsage.cpp

@@ -408,48 +408,23 @@ getDependentValuesFromCall(const CountAttributedType *CAT,
   return {std::move(Values)};
 }
 
-// Checks if Self and Other are the same member bases. This supports only very
-// simple forms of member bases.
-bool isSameMemberBase(const Expr *Self, const Expr *Other) {
-  for (;;) {
-    if (Self == Other)
-      return true;
-
-    const auto *SelfICE = dyn_cast<ImplicitCastExpr>(Self);
-    const auto *OtherICE = dyn_cast<ImplicitCastExpr>(Other);
-    if (SelfICE && OtherICE &&
-        SelfICE->getCastKind() == OtherICE->getCastKind() &&
-        (SelfICE->getCastKind() == CK_LValueToRValue ||
-         SelfICE->getCastKind() == CK_UncheckedDerivedToBase)) {
-      Self = SelfICE->getSubExpr();
-      Other = OtherICE->getSubExpr();
-    }
-
-    const auto *SelfDRE = dyn_cast<DeclRefExpr>(Self);
-    const auto *OtherDRE = dyn_cast<DeclRefExpr>(Other);
-    if (SelfDRE && OtherDRE)
-      return SelfDRE->getDecl() == OtherDRE->getDecl();
-
-    if (isa<CXXThisExpr>(Self) && isa<CXXThisExpr>(Other)) {
-      // `Self` and `Other` should be evaluated at the same state so `this` must
-      // mean the same thing for both:
-      return true;
-    }
-
-    const auto *SelfME = dyn_cast<MemberExpr>(Self);
-    const auto *OtherME = dyn_cast<MemberExpr>(Other);
-    if (!SelfME || !OtherME ||
-        SelfME->getMemberDecl() != OtherME->getMemberDecl()) {
-      return false;
-    }
-
-    Self = SelfME->getBase();
-    Other = OtherME->getBase();
-  }
-}
-
 // Impl of `isCompatibleWithCountExpr`.  See `isCompatibleWithCountExpr` for
-// document.
+// high-level document.
+//
+// This visitor compares two expressions in a tiny subset of the language,
+// including DRE of VarDecls, constants, binary operators, subscripts,
+// dereferences, member accesses, and member function calls.
+//
+// - For constants, they are literal constants and expressions have
+//   compile-time constant values.
+// - For a supported dereference expression, it can be either of the forms '*e'
+//   or '*&e', where 'e' is a supported expression.
+// - For a subscript expression, it can be either an array subscript or
+//   overloaded subscript operator.
+// - For a member function call, it must be a call to a 'const' (non-static)
+//   member function with zero argument.  This is to ensure side-effect free.
+//   Other kinds of function calls are not supported, so an expression of the
+//   form `f(...)` is not supported.
 struct CompatibleCountExprVisitor
     : public ConstStmtVisitor<CompatibleCountExprVisitor, bool, const Expr *> {
   using BaseVisitor =
@@ -523,6 +498,10 @@ struct CompatibleCountExprVisitor
     return false;
   }
 
+  bool VisitCXXThisExpr(const CXXThisExpr *SelfThis, const Expr *Other) {
+    return isa<CXXThisExpr>(Other->IgnoreParenImpCasts());
+  }
+
   bool VisitDeclRefExpr(const DeclRefExpr *SelfDRE, const Expr *Other) {
     const ValueDecl *SelfVD = SelfDRE->getDecl();
 
@@ -544,7 +523,7 @@ struct CompatibleCountExprVisitor
     const auto *OtherME = dyn_cast<MemberExpr>(O);
     if (MemberBase && OtherME) {
       return OtherME->getMemberDecl() == SelfVD &&
-             isSameMemberBase(OtherME->getBase(), MemberBase);
+             Visit(OtherME->getBase(), MemberBase);
     }
 
     return false;
@@ -557,7 +536,12 @@ struct CompatibleCountExprVisitor
       return true;
     if (const auto *DRE = dyn_cast<DeclRefExpr>(Other->IgnoreParenImpCasts()))
       return MemberBase && Self->getMemberDecl() == DRE->getDecl() &&
-             isSameMemberBase(Self->getBase(), MemberBase);
+             Visit(Self->getBase(), MemberBase);
+    if (const auto *OtherME =
+            dyn_cast<MemberExpr>(Other->IgnoreParenImpCasts())) {
+      return Self->getMemberDecl() == OtherME->getMemberDecl() &&
+             Visit(Self->getBase(), OtherME->getBase());
+    }
     return false;
   }
 
@@ -587,6 +571,54 @@ struct CompatibleCountExprVisitor
 
     return false;
   }
+
+  // Support any overloaded operator[] so long as it is a const method.
+  bool VisitCXXOperatorCallExpr(const CXXOperatorCallExpr *SelfOpCall,
+                                const Expr *Other) {
+    if (SelfOpCall->getOperator() != OverloadedOperatorKind::OO_Subscript)
+      return false;
+
+    const auto *MD = dyn_cast<CXXMethodDecl>(SelfOpCall->getCalleeDecl());
+
+    if (!MD || !MD->isConst())
+      return false;
+    if (const auto *OtherOpCall =
+            dyn_cast<CXXOperatorCallExpr>(Other->IgnoreParenImpCasts()))
+      if (SelfOpCall->getOperator() == OtherOpCall->getOperator()) {
+        return Visit(SelfOpCall->getArg(0), OtherOpCall->getArg(0)) &&
+               Visit(SelfOpCall->getArg(1), OtherOpCall->getArg(1));
+      }
+    return false;
+  }
+
+  // Support array/pointer subscript. Even though these operators are generally
+  // considered unsafe, they can be safely used on constant arrays with
+  // known-safe literal indexes.
+  bool VisitArraySubscriptExpr(const ArraySubscriptExpr *SelfAS,
+                               const Expr *Other) {
+    if (const auto *OtherAS =
+            dyn_cast<ArraySubscriptExpr>(Other->IgnoreParenImpCasts()))
+      return Visit(SelfAS->getLHS(), OtherAS->getLHS()) &&
+             Visit(SelfAS->getRHS(), OtherAS->getRHS());
+    return false;
+  }
+
+  // Support non-static member call:
+  bool VisitCXXMemberCallExpr(const CXXMemberCallExpr *SelfCall,
+                              const Expr *Other) {
+    const CXXMethodDecl *MD = SelfCall->getMethodDecl();
+
+    // The callee member function must be a const function with no parameter:
+    if (MD->isConst() && MD->param_empty()) {
+      if (auto *OtherCall =
+              dyn_cast<CXXMemberCallExpr>(Other->IgnoreParenImpCasts())) {
+        return OtherCall->getMethodDecl() == MD &&
+               Visit(SelfCall->getImplicitObjectArgument(),
+                     OtherCall->getImplicitObjectArgument());
+      }
+    }
+    return false;
+  }
 };
 
 // TL'DR:

clang/test/SemaCXX/warn-unsafe-buffer-usage-count-attributed-pointer-argument.cpp

@@ -4,12 +4,12 @@
 #include <stddef.h>
 
 namespace std {
-
 template <typename T, size_t N>
 struct array {
   const T *data() const noexcept;
   T *data() noexcept;
   size_t size() const noexcept;
+  const T &operator[](const size_t idx) const;
 };
 
 template <typename CharT>
@@ -40,6 +40,7 @@ struct span {
   span<T> first(size_t count) const noexcept;
   span<T> last(size_t count) const noexcept;
   span<T> subspan(size_t offset, size_t count) const noexcept;
+  const T &operator[](const size_t idx) const;
 };
 
 template <typename T>
@@ -71,7 +72,7 @@ void cb_cchar_42(const char *__counted_by(42) s);
 // expected-note@+1 19{{consider using a safe container and passing '.data()' to the parameter 'p' and '.size()' to its dependent parameter 'count' or 'std::span' and passing '.first(...).data()' to the parameter 'p'}}
 void cb_int(int *__counted_by(count) p, size_t count);
 
-// expected-note@+1 11{{consider using a safe container and passing '.data()' to the parameter 'p' and '.size()' to its dependent parameter 'count' or 'std::span' and passing '.first(...).data()' to the parameter 'p'}}
+// expected-note@+1 33{{consider using a safe container and passing '.data()' to the parameter 'p' and '.size()' to its dependent parameter 'count' or 'std::span' and passing '.first(...).data()' to the parameter 'p'}}
 void cb_cint(const int *__counted_by(count) p, size_t count);
 
 // expected-note@+1 10{{consider using 'std::span' and passing '.first(...).data()' to the parameter 'p'}}
@@ -367,6 +368,129 @@ void single_variable() {
   sb_cvoid(&Arr, sizeof(decltype(Var))); // expected-warning{{unsafe assignment to function parameter of count-attributed type}}
 }
 
+namespace test_subscript {
+  void foo(std::span<int> S) {
+    std::array<int, 10> A;
+
+    cb_cint(S.first(S[5]).data(), S[5]);
+    cb_cint(S.first(A[5]).data(), A[5]);
+    cb_cint(S.subspan(0, S[5]).data(), S[5]);
+    cb_cint(S.subspan(0, A[5]).data(), A[5]);
+    cb_cint(S.subspan(0, S[5]).data(), A[5]); // expected-warning{{unsafe assignment to function parameter of count-attributed type}}
+    cb_cint(S.subspan(0, S[5]).data(), S[6]); // expected-warning{{unsafe assignment to function parameter of count-attributed type}}
+
+    unsigned x;
+
+    cb_cint(S.first(S[x]).data(), S[x]);
+    cb_cint(S.first(A[x]).data(), A[x]);
+    cb_cint(S.subspan(0, S[x]).data(), S[x]);
+    cb_cint(S.subspan(0, A[x]).data(), A[x]);
+    cb_cint(S.first(S[x + 1]).data(), S[x + 1]);
+    cb_cint(S.subspan(0, A[x + 1]).data(), A[x + 1]);
+    cb_cint(S.first(A[A[x]]).data(), A[A[x]]);
+    cb_cint(S.first(A[A[x] + 1]).data(), A[A[x] + 1]);
+    cb_cint(S.first(S[x + 1]).data(), S[x - 1]);   // expected-warning{{unsafe assignment to function parameter of count-attributed type}}
+    cb_cint(S.first(A[A[x]]).data(), A[A[x] + 1]); // expected-warning{{unsafe assignment to function parameter of count-attributed type}}
+
+    std::array<std::array<int, 10>, 10> A2d;
+
+    cb_cint(S.first(A2d[x][5]).data(), A2d[x][5]);
+    cb_cint(S.first(A2d[x][5]).data(), A2d[5][x]); // expected-warning{{unsafe assignment to function parameter of count-attributed type}}
+  }
+
+  struct T {
+    std::array<int, 10> a;
+  } t, *tp;
+
+  struct TT {
+    struct T t;
+    struct T * tp;
+  } tt, *ttp;
+
+  void bar(std::span<int> S) {
+    unsigned x;
+
+    cb_cint(S.first(t.a[x]).data(), t.a[x]);
+    cb_cint(S.first(tp->a[x]).data(), tp->a[x]);
+    cb_cint(S.first(tt.t.a[x]).data(), tt.t.a[x]);
+    cb_cint(S.first(ttp->t.a[x]).data(), ttp->t.a[x]);
+    cb_cint(S.first(tt.tp->a[x]).data(), tt.tp->a[x]);
+    cb_cint(S.first(ttp->tp->a[x]).data(), ttp->tp->a[x]);
+    cb_cint(S.first(t.a[x]).data(), tp->a[x]);     // expected-warning{{unsafe assignment to function parameter of count-attributed type}}
+    cb_cint(S.first(tt.t.a[x]).data(), tt.tp->a[x]); // expected-warning{{unsafe assignment to function parameter of count-attributed type}}
+    cb_cint(S.first(ttp->t.a[x]).data(), ttp->tp->a[x]); // expected-warning{{unsafe assignment to function parameter of count-attributed type}}
+
+    int cArr[10];
+
+    cb_cint(S.first(cArr[5]).data(), cArr[5]);
+    cb_cint(S.first(cArr[1]).data(), cArr[sizeof(char)]);
+    cb_cint(S.first(cArr[5]).data(), cArr[6]); // expected-warning{{unsafe assignment to function parameter of count-attributed type}}
+    cb_cint(S.first(cArr[5]).data(), cArr[sizeof(char)]);// expected-warning{{unsafe assignment to function parameter of count-attributed type}}
+
+    std::array<struct TT, 10> ttA;
+
+    cb_cint(S.first(ttA[4].t.a[x]).data(), ttA[4].t.a[x]);
+    cb_cint(S.first(ttA[4].tp->a[x]).data(), ttA[4].tp->a[x]);
+    cb_cint(S.first(ttA[3].t.a[x]).data(), ttA[4].t.a[x]);// expected-warning{{unsafe assignment to function parameter of count-attributed type}}
+    cb_cint(S.first(ttA[3].tp->a[x]).data(), ttA[4].tp->a[x]);// expected-warning{{unsafe assignment to function parameter of count-attributed type}}
+  }
+} //namespace test_subscript
+
+namespace test_member_fun_call {
+  struct T {
+    unsigned size() {return 0;}                   // missing 'const'
+    unsigned size(int x) const {return 0;}        // has arguments
+    unsigned supported_size() const {return 0;}
+  } t, *tp;
+
+  void foo(std::span<int> S1, std::span<int> S2) {
+    cb_cint(S1.first(S2.size()).data(), S2.size());
+    cb_cint(S1.first(S2.size()).data(), S1.size()); // expected-warning{{unsafe assignment to function parameter of count-attributed type}}
+
+
+    cb_cint(S1.first(t.supported_size()).data(), t.supported_size());
+    cb_cint(S1.first(tp->supported_size()).data(), tp->supported_size());
+    cb_cint(S1.first(t.size()).data(), t.size());   // expected-warning{{unsafe assignment to function parameter of count-attributed type}}
+    cb_cint(S1.first(t.size(0)).data(), t.size(0)); // expected-warning{{unsafe assignment to function parameter of count-attributed type}}
+    cb_cint(S1.first(t.supported_size()).data(), tp->supported_size()); // expected-warning{{unsafe assignment to function parameter of count-attributed}}
+  }
+  void bar(std::span<int> S) {
+    std::array<struct T, 10> Ts;
+    std::array<struct T*, 10> Tps;
+    unsigned x;
+
+    cb_cint(S.first(Ts[5].supported_size()).data(), Ts[5].supported_size());
+    cb_cint(S.first(Ts[x].supported_size()).data(), Ts[x].supported_size());
+    cb_cint(S.first(Tps[5]->supported_size()).data(), Tps[5]->supported_size());
+    cb_cint(S.first(Ts[x].supported_size()).data(), Ts[5].supported_size());  // expected-warning{{unsafe assignment to function parameter of count-attributed}}
+    cb_cint(S.first(Ts[x].supported_size()).data(), Tps[x]->supported_size());// expected-warning{{unsafe assignment to function parameter of count-attributed}}
+  }
+
+  template<typename Ty, size_t N>
+  struct GenT {
+    unsigned size() {return 0;}                   // missing 'const'
+    unsigned size(int x) const {return 0;}        // has arguments
+    static unsigned static_size() {return 0;}     // static function is not supported
+    unsigned supported_size() const {return 0;}
+
+    void test(std::span<int> S) {
+      cb_cint(S.first(supported_size()).data(), supported_size());
+      cb_cint(S.first(supported_size()).data(), size()); // expected-warning{{unsafe assignment to function parameter of count-attributed}}
+    }
+  };
+
+  void baz(std::span<int> S) {
+    GenT<int, 5> T5;
+    GenT<int, 10> T10;
+
+    cb_cint(S.first(T5.supported_size()).data(), T5.supported_size());
+    cb_cint(S.first(T5.static_size()).data(), T5.static_size()); // expected-warning{{unsafe assignment to function parameter of count-attributed}}
+    cb_cint(S.first(GenT<int, 5>::static_size()).data(), GenT<int, 5>::static_size()); // expected-warning{{unsafe assignment to function parameter of count-attributed}}
+    cb_cint(S.first(T5.supported_size()).data(), T10.supported_size()); // expected-warning{{unsafe assignment to function parameter of count-attributed}}
+    T5.test(S);
+  }
+} // namespace test_member_fun_call
+
 namespace test_member_access {
   class Base {
   public: