Merge branch 'upstream' into x86-match-hidden-broadcast

Author: RKSimon

.github/CODEOWNERS

@@ -131,7 +131,7 @@
 /bolt/ @aaupov @maksfb @rafaelauler @ayermolo @dcci @yota9
 
 # Bazel build system.
-/utils/bazel/ @rupprecht @keith
+/utils/bazel/ @rupprecht @keith @aaronmondal
 
 # InstallAPI and TextAPI
 /llvm/**/TextAPI/ @cyndyishida

.github/workflows/libcxx-build-containers.yml

@@ -9,7 +9,6 @@ name: Build Docker images for libc++ CI
 
 permissions:
   contents: read
-  packages: write
 
 on:
   push:

bolt/tools/driver/llvm-bolt.cpp

@@ -173,24 +173,14 @@ void boltMode(int argc, char **argv) {
   }
 }
 
-static std::string GetExecutablePath(const char *Argv0) {
-  SmallString<256> ExecutablePath(Argv0);
-  // Do a PATH lookup if Argv0 isn't a valid path.
-  if (!llvm::sys::fs::exists(ExecutablePath))
-    if (llvm::ErrorOr<std::string> P =
-            llvm::sys::findProgramByName(ExecutablePath))
-      ExecutablePath = *P;
-  return std::string(ExecutablePath);
-}
-
 int main(int argc, char **argv) {
   // Print a stack trace if we signal out.
   sys::PrintStackTraceOnErrorSignal(argv[0]);
   PrettyStackTraceProgram X(argc, argv);
 
   llvm_shutdown_obj Y; // Call llvm_shutdown() on exit.
 
-  std::string ToolPath = GetExecutablePath(argv[0]);
+  std::string ToolPath = llvm::sys::fs::getMainExecutable(argv[0], nullptr);
 
   // Initialize targets and assembly printers/parsers.
   llvm::InitializeAllTargetInfos();

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

@@ -82,7 +82,7 @@ void StringConstructorCheck::registerMatchers(MatchFinder *Finder) {
   Finder->addMatcher(
       cxxConstructExpr(
           hasDeclaration(cxxMethodDecl(hasName("basic_string"))),
-          hasArgument(0, hasType(qualType(isInteger()))),
+          argumentCountIs(2), hasArgument(0, hasType(qualType(isInteger()))),
           hasArgument(1, hasType(qualType(isInteger()))),
           anyOf(
               // Detect the expression: string('x', 40);
@@ -102,7 +102,7 @@ void StringConstructorCheck::registerMatchers(MatchFinder *Finder) {
       cxxConstructExpr(
           hasDeclaration(cxxConstructorDecl(ofClass(
               cxxRecordDecl(hasAnyName(removeNamespaces(StringNames)))))),
-          hasArgument(0, hasType(CharPtrType)),
+          argumentCountIs(2), hasArgument(0, hasType(CharPtrType)),
           hasArgument(1, hasType(isInteger())),
           anyOf(
               // Detect the expression: string("...", 0);
@@ -114,7 +114,34 @@ void StringConstructorCheck::registerMatchers(MatchFinder *Finder) {
               // Detect the expression: string("lit", 5)
               allOf(hasArgument(0, ConstStrLiteral.bind("literal-with-length")),
                     hasArgument(1, ignoringParenImpCasts(
-                                       integerLiteral().bind("int"))))))
+                                       integerLiteral().bind("length"))))))
+          .bind("constructor"),
+      this);
+
+  // Check the literal string constructor with char pointer, start position and
+  // length parameters. [i.e. string (const char* s, size_t pos, size_t count);]
+  Finder->addMatcher(
+      cxxConstructExpr(
+          hasDeclaration(cxxConstructorDecl(ofClass(
+              cxxRecordDecl(hasAnyName(removeNamespaces(StringNames)))))),
+          argumentCountIs(3), hasArgument(0, hasType(CharPtrType)),
+          hasArgument(1, hasType(qualType(isInteger()))),
+          hasArgument(2, hasType(qualType(isInteger()))),
+          anyOf(
+              // Detect the expression: string("...", 1, 0);
+              hasArgument(2, ZeroExpr.bind("empty-string")),
+              // Detect the expression: string("...", -4, 1);
+              hasArgument(1, NegativeExpr.bind("negative-pos")),
+              // Detect the expression: string("...", 0, -4);
+              hasArgument(2, NegativeExpr.bind("negative-length")),
+              // Detect the expression: string("lit", 0, 0x1234567);
+              hasArgument(2, LargeLengthExpr.bind("large-length")),
+              // Detect the expression: string("lit", 1, 5)
+              allOf(hasArgument(0, ConstStrLiteral.bind("literal-with-length")),
+                    hasArgument(
+                        1, ignoringParenImpCasts(integerLiteral().bind("pos"))),
+                    hasArgument(2, ignoringParenImpCasts(
+                                       integerLiteral().bind("length"))))))
           .bind("constructor"),
       this);
 
@@ -155,14 +182,27 @@ void StringConstructorCheck::check(const MatchFinder::MatchResult &Result) {
     diag(Loc, "constructor creating an empty string");
   } else if (Result.Nodes.getNodeAs<Expr>("negative-length")) {
     diag(Loc, "negative value used as length parameter");
+  } else if (Result.Nodes.getNodeAs<Expr>("negative-pos")) {
+    diag(Loc, "negative value used as position of the "
+              "first character parameter");
   } else if (Result.Nodes.getNodeAs<Expr>("large-length")) {
     if (WarnOnLargeLength)
       diag(Loc, "suspicious large length parameter");
   } else if (Result.Nodes.getNodeAs<Expr>("literal-with-length")) {
     const auto *Str = Result.Nodes.getNodeAs<StringLiteral>("str");
-    const auto *Lit = Result.Nodes.getNodeAs<IntegerLiteral>("int");
-    if (Lit->getValue().ugt(Str->getLength())) {
+    const auto *Length = Result.Nodes.getNodeAs<IntegerLiteral>("length");
+    if (Length->getValue().ugt(Str->getLength())) {
       diag(Loc, "length is bigger than string literal size");
+      return;
+    }
+    if (const auto *Pos = Result.Nodes.getNodeAs<IntegerLiteral>("pos")) {
+      if (Pos->getValue().uge(Str->getLength())) {
+        diag(Loc, "position of the first character parameter is bigger than "
+                  "string literal character range");
+      } else if (Length->getValue().ugt(
+                     (Str->getLength() - Pos->getValue()).getZExtValue())) {
+        diag(Loc, "length is bigger than remaining string literal size");
+      }
     }
   } else if (const auto *Ptr = Result.Nodes.getNodeAs<Expr>("from-ptr")) {
     Expr::EvalResult ConstPtr;

clang-tools-extra/clang-tidy/misc/RedundantExpressionCheck.cpp

@@ -139,10 +139,8 @@ static bool areEquivalentExpr(const Expr *Left, const Expr *Right) {
     return cast<BinaryOperator>(Left)->getOpcode() ==
            cast<BinaryOperator>(Right)->getOpcode();
   case Stmt::UnaryExprOrTypeTraitExprClass:
-    const auto *LeftUnaryExpr =
-        cast<UnaryExprOrTypeTraitExpr>(Left);
-    const auto *RightUnaryExpr =
-        cast<UnaryExprOrTypeTraitExpr>(Right);
+    const auto *LeftUnaryExpr = cast<UnaryExprOrTypeTraitExpr>(Left);
+    const auto *RightUnaryExpr = cast<UnaryExprOrTypeTraitExpr>(Right);
     if (LeftUnaryExpr->isArgumentType() && RightUnaryExpr->isArgumentType())
       return LeftUnaryExpr->getKind() == RightUnaryExpr->getKind() &&
              LeftUnaryExpr->getArgumentType() ==
@@ -699,7 +697,8 @@ static bool retrieveRelationalIntegerConstantExpr(
 
     Symbol = OverloadedOperatorExpr->getArg(IntegerConstantIsFirstArg ? 1 : 0);
     OperandExpr = OverloadedOperatorExpr;
-    Opcode = BinaryOperator::getOverloadedOpcode(OverloadedOperatorExpr->getOperator());
+    Opcode = BinaryOperator::getOverloadedOpcode(
+        OverloadedOperatorExpr->getOperator());
 
     if (!retrieveIntegerConstantExpr(Result, Id, Value, ConstExpr))
       return false;
@@ -728,7 +727,8 @@ static bool retrieveRelationalIntegerConstantExpr(
 }
 
 // Checks for expressions like (X == 4) && (Y != 9)
-static bool areSidesBinaryConstExpressions(const BinaryOperator *&BinOp, const ASTContext *AstCtx) {
+static bool areSidesBinaryConstExpressions(const BinaryOperator *&BinOp,
+                                           const ASTContext *AstCtx) {
   const auto *LhsBinOp = dyn_cast<BinaryOperator>(BinOp->getLHS());
   const auto *RhsBinOp = dyn_cast<BinaryOperator>(BinOp->getRHS());
 
@@ -747,6 +747,28 @@ static bool areSidesBinaryConstExpressions(const BinaryOperator *&BinOp, const A
   return false;
 }
 
+static bool areSidesBinaryConstExpressionsOrDefinesOrIntegerConstant(
+    const BinaryOperator *&BinOp, const ASTContext *AstCtx) {
+  if (areSidesBinaryConstExpressions(BinOp, AstCtx))
+    return true;
+
+  const Expr *Lhs = BinOp->getLHS();
+  const Expr *Rhs = BinOp->getRHS();
+
+  if (!Lhs || !Rhs)
+    return false;
+
+  auto IsDefineExpr = [AstCtx](const Expr *E) {
+    const SourceRange Lsr = E->getSourceRange();
+    if (!Lsr.getBegin().isMacroID() || E->isValueDependent() ||
+        !E->isIntegerConstantExpr(*AstCtx))
+      return false;
+    return true;
+  };
+
+  return IsDefineExpr(Lhs) || IsDefineExpr(Rhs);
+}
+
 // Retrieves integer constant subexpressions from binary operator expressions
 // that have two equivalent sides.
 // E.g.: from (X == 5) && (X == 5) retrieves 5 and 5.
@@ -785,7 +807,7 @@ static bool retrieveConstExprFromBothSides(const BinaryOperator *&BinOp,
 }
 
 static bool isSameRawIdentifierToken(const Token &T1, const Token &T2,
-                        const SourceManager &SM) {
+                                     const SourceManager &SM) {
   if (T1.getKind() != T2.getKind())
     return false;
   if (T1.isNot(tok::raw_identifier))
@@ -808,8 +830,8 @@ static bool areExprsFromDifferentMacros(const Expr *LhsExpr,
                                         const ASTContext *AstCtx) {
   if (!LhsExpr || !RhsExpr)
     return false;
-  SourceRange Lsr = LhsExpr->getSourceRange();
-  SourceRange Rsr = RhsExpr->getSourceRange();
+  const SourceRange Lsr = LhsExpr->getSourceRange();
+  const SourceRange Rsr = RhsExpr->getSourceRange();
   if (!Lsr.getBegin().isMacroID() || !Rsr.getBegin().isMacroID())
     return false;
 
@@ -847,11 +869,83 @@ static bool areExprsMacroAndNonMacro(const Expr *&LhsExpr,
   if (!LhsExpr || !RhsExpr)
     return false;
 
-  SourceLocation LhsLoc = LhsExpr->getExprLoc();
-  SourceLocation RhsLoc = RhsExpr->getExprLoc();
+  const SourceLocation LhsLoc = LhsExpr->getExprLoc();
+  const SourceLocation RhsLoc = RhsExpr->getExprLoc();
 
   return LhsLoc.isMacroID() != RhsLoc.isMacroID();
 }
+
+static bool areStringsSameIgnoreSpaces(const llvm::StringRef Left,
+                                       const llvm::StringRef Right) {
+  if (Left == Right)
+    return true;
+
+  // Do running comparison ignoring spaces
+  llvm::StringRef L = Left.trim();
+  llvm::StringRef R = Right.trim();
+  while (!L.empty() && !R.empty()) {
+    L = L.ltrim();
+    R = R.ltrim();
+    if (L.empty() && R.empty())
+      return true;
+    // If symbol compared are different ==> strings are not the same
+    if (L.front() != R.front())
+      return false;
+    L = L.drop_front();
+    R = R.drop_front();
+  }
+  return L.empty() && R.empty();
+}
+
+static bool areExprsSameMacroOrLiteral(const BinaryOperator *BinOp,
+                                       const ASTContext *Context) {
+
+  if (!BinOp)
+    return false;
+
+  const Expr *Lhs = BinOp->getLHS();
+  const Expr *Rhs = BinOp->getRHS();
+  const SourceManager &SM = Context->getSourceManager();
+
+  const SourceRange Lsr = Lhs->getSourceRange();
+  const SourceRange Rsr = Rhs->getSourceRange();
+  if (Lsr.getBegin().isMacroID()) {
+    // Left is macro so right macro too
+    if (Rsr.getBegin().isMacroID()) {
+      // Both sides are macros so they are same macro or literal
+      const llvm::StringRef L = Lexer::getSourceText(
+          CharSourceRange::getTokenRange(Lsr), SM, Context->getLangOpts(), 0);
+      const llvm::StringRef R = Lexer::getSourceText(
+          CharSourceRange::getTokenRange(Rsr), SM, Context->getLangOpts(), 0);
+      return areStringsSameIgnoreSpaces(L, R);
+    }
+    // Left is macro but right is not so they are not same macro or literal
+    return false;
+  }
+  const auto *Lil = dyn_cast<IntegerLiteral>(Lhs);
+  const auto *Ril = dyn_cast<IntegerLiteral>(Rhs);
+  if (Lil && Ril)
+    return Lil->getValue() == Ril->getValue();
+
+  const auto *LStrl = dyn_cast<StringLiteral>(Lhs);
+  const auto *RStrl = dyn_cast<StringLiteral>(Rhs);
+  if (Lil && Ril) {
+    const llvm::StringRef L = Lexer::getSourceText(
+        CharSourceRange::getTokenRange(LStrl->getBeginLoc()), SM,
+        Context->getLangOpts(), 0);
+    const llvm::StringRef R = Lexer::getSourceText(
+        CharSourceRange::getTokenRange(RStrl->getBeginLoc()), SM,
+        Context->getLangOpts(), 0);
+    return L.compare(R) == 0;
+  }
+
+  const auto *Lbl = dyn_cast<CXXBoolLiteralExpr>(Lhs);
+  const auto *Rbl = dyn_cast<CXXBoolLiteralExpr>(Rhs);
+  if (Lbl && Rbl)
+    return Lbl->getValue() == Rbl->getValue();
+
+  return false;
+}
 } // namespace
 
 void RedundantExpressionCheck::registerMatchers(MatchFinder *Finder) {
@@ -1089,7 +1183,6 @@ static bool exprEvaluatesToSymbolic(BinaryOperatorKind Opcode, APSInt Value) {
          ((Opcode == BO_And || Opcode == BO_AndAssign) && ~Value == 0);
 }
 
-
 void RedundantExpressionCheck::checkBitwiseExpr(
     const MatchFinder::MatchResult &Result) {
   if (const auto *ComparisonOperator = Result.Nodes.getNodeAs<BinaryOperator>(
@@ -1134,8 +1227,8 @@ void RedundantExpressionCheck::checkBitwiseExpr(
                                      ConstExpr))
       return;
 
-    if((Value != 0 && ~Value != 0) || Sym->getExprLoc().isMacroID())
-        return;
+    if ((Value != 0 && ~Value != 0) || Sym->getExprLoc().isMacroID())
+      return;
 
     SourceLocation Loc = IneffectiveOperator->getOperatorLoc();
 
@@ -1276,19 +1369,23 @@ void RedundantExpressionCheck::check(const MatchFinder::MatchResult &Result) {
       return;
     }
 
-    if (areSidesBinaryConstExpressions(BinOp, Result.Context)) {
+    if (areSidesBinaryConstExpressionsOrDefinesOrIntegerConstant(
+            BinOp, Result.Context)) {
       const Expr *LhsConst = nullptr, *RhsConst = nullptr;
       BinaryOperatorKind MainOpcode{}, SideOpcode{};
-
-      if (!retrieveConstExprFromBothSides(BinOp, MainOpcode, SideOpcode,
-                                          LhsConst, RhsConst, Result.Context))
-        return;
-
-      if (areExprsFromDifferentMacros(LhsConst, RhsConst, Result.Context) ||
-          areExprsMacroAndNonMacro(LhsConst, RhsConst))
-        return;
+      if (areSidesBinaryConstExpressions(BinOp, Result.Context)) {
+        if (!retrieveConstExprFromBothSides(BinOp, MainOpcode, SideOpcode,
+                                            LhsConst, RhsConst, Result.Context))
+          return;
+
+        if (areExprsFromDifferentMacros(LhsConst, RhsConst, Result.Context) ||
+            areExprsMacroAndNonMacro(LhsConst, RhsConst))
+          return;
+      } else {
+        if (!areExprsSameMacroOrLiteral(BinOp, Result.Context))
+          return;
+      }
     }
-
     diag(BinOp->getOperatorLoc(), "both sides of operator are equivalent");
   }
 

clang-tools-extra/docs/ReleaseNotes.rst

@@ -97,10 +97,19 @@ New check aliases
 Changes in existing checks
 ^^^^^^^^^^^^^^^^^^^^^^^^^^
 
+- Improved :doc:`bugprone-string-constructor
+  <clang-tidy/checks/bugprone/string-constructor>` check to find suspicious
+  calls of ``std::string`` constructor with char pointer, start position and
+  length parameters.
+
 - Improved :doc:`bugprone-unsafe-functions
   <clang-tidy/checks/bugprone/unsafe-functions>` check to allow specifying
   additional C++ member functions to match.
 
+- Improved :doc:`misc-redundant-expression
+  <clang-tidy/checks/misc/redundant-expression>` check by providing additional
+  examples and fixing some macro related false positives.
+
 Removed checks
 ^^^^^^^^^^^^^^
 

clang-tools-extra/docs/clang-tidy/checks/bugprone/string-constructor.rst

@@ -21,6 +21,7 @@ Examples:
 .. code-block:: c++
 
   std::string("test", 200);   // Will include random characters after "test".
+  std::string("test", 2, 5);  // Will include random characters after "st".
   std::string_view("test", 200);
 
 Creating an empty string from constructors with parameters is considered
@@ -31,8 +32,19 @@ Examples:
 .. code-block:: c++
 
   std::string("test", 0);   // Creation of an empty string.
+  std::string("test", 1, 0);
   std::string_view("test", 0);
 
+Passing an invalid first character position parameter to constructor will
+cause ``std::out_of_range`` exception at runtime. 
+
+Examples:
+
+.. code-block:: c++
+
+  std::string("test", -1, 10); // Negative first character position.
+  std::string("test", 10, 10); // First character position is bigger than string literal character range".
+
 Options
 -------