[C++ Safe Buffers][BoundsSafety] Cherrypicks for 143986251 & 145705060 (#10156)

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

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)

* [Safe Buffers][BoundsSafety] Fix a bug in the interop analysis that can cause infinite loops

The interop analysis substitutes formal parameters to arguments in AST
visiting.  Conceptually, the substitution should be done exactly once.
But the previous implementation never checked if the substitution has
happened.  In cases where the argument contains DREs referring to the
Decl of the corresponding formal parameter, the analysis enters an
infinite loop.  A typical example is the following.

```
void f(int * __counted_by(n) p, size_t n) {
  f(p, n);
}
```

This commit fixes the issue.

(rdar://145705060)

(cherry picked from commit 31f72f7)

Author: ziqingluo-90

clang/lib/Analysis/UnsafeBufferUsage.cpp

@@ -412,66 +412,55 @@ 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 *> {
+    : public ConstStmtVisitor<CompatibleCountExprVisitor, bool, const Expr *,
+                              bool> {
+  // The third 'bool' type parameter for each visit method indicates whether a
+  // parameter has been substituted with its argument expression in the
+  // expression AST branch being visited.  Since the argument expression may
+  // contain DREs referencing to the same parameter Decl, the analysis may hit
+  // an infinite loop of not knowing whether the substitution has happened.  A
+  // typical example that could introduce infinite loop without this knowledge
+  // is shown below.
+  // ```
+  //  void f(int * __counted_by(n) p, size_t n) {
+  //    f(p, n);
+  //  }
+  // ```
   using BaseVisitor =
-      ConstStmtVisitor<CompatibleCountExprVisitor, bool, const Expr *>;
+      ConstStmtVisitor<CompatibleCountExprVisitor, bool, const Expr *, bool>;
 
   const Expr *MemberBase;
   const DependentValuesTy *DependentValues;
   ASTContext &Ctx;
 
   // If `Deref` has the form `*&e`, return `e`; otherwise return nullptr.
-  const Expr *trySimplifyDerefAddressof(const UnaryOperator *Deref) {
+  const Expr *trySimplifyDerefAddressof(const UnaryOperator *Deref,
+                                        bool hasBeenSubstituted) {
     const Expr *DerefOperand = Deref->getSubExpr()->IgnoreParenImpCasts();
 
     if (const auto *UO = dyn_cast<UnaryOperator>(DerefOperand))
       if (UO->getOpcode() == UO_AddrOf)
         return UO->getSubExpr();
     if (const auto *DRE = dyn_cast<DeclRefExpr>(DerefOperand)) {
-      if (!DependentValues)
+      if (!DependentValues || hasBeenSubstituted)
         return nullptr;
 
       auto I = DependentValues->find(DRE->getDecl());
@@ -489,18 +478,22 @@ struct CompatibleCountExprVisitor
                                       ASTContext &Ctx)
       : MemberBase(MemberBase), DependentValues(DependentValues), Ctx(Ctx) {}
 
-  bool VisitStmt(const Stmt *S, const Expr *E) { return false; }
+  bool VisitStmt(const Stmt *S, const Expr *E, bool hasBeenSubstituted) {
+    return false;
+  }
 
-  bool VisitImplicitCastExpr(const ImplicitCastExpr *SelfICE,
-                             const Expr *Other) {
-    return Visit(SelfICE->getSubExpr(), Other);
+  bool VisitImplicitCastExpr(const ImplicitCastExpr *SelfICE, const Expr *Other,
+                             bool hasBeenSubstituted) {
+    return Visit(SelfICE->getSubExpr(), Other, hasBeenSubstituted);
   }
 
-  bool VisitParenExpr(const ParenExpr *SelfPE, const Expr *Other) {
-    return Visit(SelfPE->getSubExpr(), Other);
+  bool VisitParenExpr(const ParenExpr *SelfPE, const Expr *Other,
+                      bool hasBeenSubstituted) {
+    return Visit(SelfPE->getSubExpr(), Other, hasBeenSubstituted);
   }
 
-  bool VisitIntegerLiteral(const IntegerLiteral *SelfIL, const Expr *Other) {
+  bool VisitIntegerLiteral(const IntegerLiteral *SelfIL, const Expr *Other,
+                           bool hasBeenSubstituted) {
     if (const auto *IntLit =
             dyn_cast<IntegerLiteral>(Other->IgnoreParenImpCasts())) {
       return SelfIL == IntLit ||
@@ -510,7 +503,8 @@ struct CompatibleCountExprVisitor
   }
 
   bool VisitUnaryExprOrTypeTraitExpr(const UnaryExprOrTypeTraitExpr *Self,
-                                     const Expr *Other) {
+                                     const Expr *Other,
+                                     bool hasBeenSubstituted) {
     // If `Self` is a `sizeof` expression, try to evaluate and compare the two
     // expressions as constants:
     if (Self->getKind() == UnaryExprOrTypeTrait::UETT_SizeOf) {
@@ -527,13 +521,19 @@ struct CompatibleCountExprVisitor
     return false;
   }
 
-  bool VisitDeclRefExpr(const DeclRefExpr *SelfDRE, const Expr *Other) {
+  bool VisitCXXThisExpr(const CXXThisExpr *SelfThis, const Expr *Other,
+                        bool hasBeenSubstituted) {
+    return isa<CXXThisExpr>(Other->IgnoreParenImpCasts());
+  }
+
+  bool VisitDeclRefExpr(const DeclRefExpr *SelfDRE, const Expr *Other,
+                        bool hasBeenSubstituted) {
     const ValueDecl *SelfVD = SelfDRE->getDecl();
 
-    if (DependentValues) {
+    if (DependentValues && !hasBeenSubstituted) {
       const auto It = DependentValues->find(SelfVD);
       if (It != DependentValues->end())
-        return Visit(It->second, Other);
+        return Visit(It->second, Other, true);
     }
 
     const auto *O = Other->IgnoreParenImpCasts();
@@ -548,49 +548,110 @@ struct CompatibleCountExprVisitor
     const auto *OtherME = dyn_cast<MemberExpr>(O);
     if (MemberBase && OtherME) {
       return OtherME->getMemberDecl() == SelfVD &&
-             isSameMemberBase(OtherME->getBase(), MemberBase);
+             Visit(OtherME->getBase(), MemberBase, hasBeenSubstituted);
     }
 
     return false;
   }
 
-  bool VisitMemberExpr(const MemberExpr *Self, const Expr *Other) {
+  bool VisitMemberExpr(const MemberExpr *Self, const Expr *Other,
+                       bool hasBeenSubstituted) {
     // Even though we don't support member expression in counted-by, actual
     // arguments can be member expressions.
     if (Self == Other)
       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, hasBeenSubstituted);
+    if (const auto *OtherME =
+            dyn_cast<MemberExpr>(Other->IgnoreParenImpCasts())) {
+      return Self->getMemberDecl() == OtherME->getMemberDecl() &&
+             Visit(Self->getBase(), OtherME->getBase(), hasBeenSubstituted);
+    }
     return false;
   }
 
-  bool VisitUnaryOperator(const UnaryOperator *SelfUO, const Expr *Other) {
+  bool VisitUnaryOperator(const UnaryOperator *SelfUO, const Expr *Other,
+                          bool hasBeenSubstituted) {
     if (SelfUO->getOpcode() != UO_Deref)
       return false; // We don't support any other unary operator
 
     if (const auto *OtherUO =
             dyn_cast<UnaryOperator>(Other->IgnoreParenImpCasts())) {
       if (SelfUO->getOpcode() == OtherUO->getOpcode())
-        return Visit(SelfUO->getSubExpr(), OtherUO->getSubExpr());
+        return Visit(SelfUO->getSubExpr(), OtherUO->getSubExpr(),
+                     hasBeenSubstituted);
     }
     // If `Other` is not a dereference expression, try to simplify `SelfUO`:
-    if (const auto *SimplifiedSelf = trySimplifyDerefAddressof(SelfUO)) {
-      return Visit(SimplifiedSelf, Other);
+    if (const auto *SimplifiedSelf =
+            trySimplifyDerefAddressof(SelfUO, hasBeenSubstituted)) {
+      return Visit(SimplifiedSelf, Other, hasBeenSubstituted);
     }
     return false;
   }
 
-  bool VisitBinaryOperator(const BinaryOperator *SelfBO, const Expr *Other) {
+  bool VisitBinaryOperator(const BinaryOperator *SelfBO, const Expr *Other,
+                           bool hasBeenSubstituted) {
     const auto *OtherBO =
         dyn_cast<BinaryOperator>(Other->IgnoreParenImpCasts());
     if (OtherBO && OtherBO->getOpcode() == SelfBO->getOpcode()) {
-      return Visit(SelfBO->getLHS(), OtherBO->getLHS()) &&
-             Visit(SelfBO->getRHS(), OtherBO->getRHS());
+      return Visit(SelfBO->getLHS(), OtherBO->getLHS(), hasBeenSubstituted) &&
+             Visit(SelfBO->getRHS(), OtherBO->getRHS(), hasBeenSubstituted);
     }
 
     return false;
   }
+
+  // Support any overloaded operator[] so long as it is a const method.
+  bool VisitCXXOperatorCallExpr(const CXXOperatorCallExpr *SelfOpCall,
+                                const Expr *Other, bool hasBeenSubstituted) {
+    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),
+                     hasBeenSubstituted) &&
+               Visit(SelfOpCall->getArg(1), OtherOpCall->getArg(1),
+                     hasBeenSubstituted);
+      }
+    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, bool hasBeenSubstituted) {
+    if (const auto *OtherAS =
+            dyn_cast<ArraySubscriptExpr>(Other->IgnoreParenImpCasts()))
+      return Visit(SelfAS->getLHS(), OtherAS->getLHS(), hasBeenSubstituted) &&
+             Visit(SelfAS->getRHS(), OtherAS->getRHS(), hasBeenSubstituted);
+    return false;
+  }
+
+  // Support non-static member call:
+  bool VisitCXXMemberCallExpr(const CXXMemberCallExpr *SelfCall,
+                              const Expr *Other, bool hasBeenSubstituted) {
+    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(),
+                     hasBeenSubstituted);
+      }
+    }
+    return false;
+  }
 };
 
 // TL'DR:
@@ -632,7 +693,7 @@ bool isCompatibleWithCountExpr(const Expr *E, const Expr *ExpectedCountExpr,
                                const DependentValuesTy *DependentValues,
                                ASTContext &Ctx) {
   CompatibleCountExprVisitor Visitor(MemberBase, DependentValues, Ctx);
-  return Visitor.Visit(ExpectedCountExpr, E);
+  return Visitor.Visit(ExpectedCountExpr, E, /* hasBeenSubstituted*/ false);
 }
 
 // Returns if a pair of expressions contain method calls to .data()/.c_str() and