merge main into amd-staging

Change-Id: Ie53f04f08c32d71d5c65d46a9edc0538027f8277

bolt/include/bolt/Core/BinaryFunction.h

@@ -2407,6 +2407,19 @@ inline raw_ostream &operator<<(raw_ostream &OS,
   return OS;
 }
 
+/// Compare function by index if it is valid, fall back to the original address
+/// otherwise.
+inline bool compareBinaryFunctionByIndex(const BinaryFunction *A,
+                                         const BinaryFunction *B) {
+  if (A->hasValidIndex() && B->hasValidIndex())
+    return A->getIndex() < B->getIndex();
+  if (A->hasValidIndex() && !B->hasValidIndex())
+    return true;
+  if (!A->hasValidIndex() && B->hasValidIndex())
+    return false;
+  return A->getAddress() < B->getAddress();
+}
+
 } // namespace bolt
 
 // GraphTraits specializations for function basic block graphs (CFGs)

bolt/lib/Core/BinaryContext.cpp

@@ -1607,13 +1607,7 @@ std::vector<BinaryFunction *> BinaryContext::getSortedFunctions() {
                   SortedFunctions.begin(),
                   [](BinaryFunction &BF) { return &BF; });
 
-  llvm::stable_sort(SortedFunctions,
-                    [](const BinaryFunction *A, const BinaryFunction *B) {
-                      if (A->hasValidIndex() && B->hasValidIndex()) {
-                        return A->getIndex() < B->getIndex();
-                      }
-                      return A->hasValidIndex();
-                    });
+  llvm::stable_sort(SortedFunctions, compareBinaryFunctionByIndex);
   return SortedFunctions;
 }
 

bolt/lib/Core/BinaryFunction.cpp

@@ -385,16 +385,7 @@ bool BinaryFunction::isForwardCall(const MCSymbol *CalleeSymbol) const {
   if (CalleeBF) {
     if (CalleeBF->isInjected())
       return true;
-
-    if (hasValidIndex() && CalleeBF->hasValidIndex()) {
-      return getIndex() < CalleeBF->getIndex();
-    } else if (hasValidIndex() && !CalleeBF->hasValidIndex()) {
-      return true;
-    } else if (!hasValidIndex() && CalleeBF->hasValidIndex()) {
-      return false;
-    } else {
-      return getAddress() < CalleeBF->getAddress();
-    }
+    return compareBinaryFunctionByIndex(this, CalleeBF);
   } else {
     // Absolute symbol.
     ErrorOr<uint64_t> CalleeAddressOrError = BC.getSymbolValue(*CalleeSymbol);

bolt/lib/Passes/ReorderFunctions.cpp

@@ -473,16 +473,7 @@ Error ReorderFunctions::runOnFunctions(BinaryContext &BC) {
                     [](BinaryFunction &BF) { return &BF; });
 
     // Sort functions by index.
-    llvm::stable_sort(SortedFunctions,
-                      [](const BinaryFunction *A, const BinaryFunction *B) {
-                        if (A->hasValidIndex() && B->hasValidIndex())
-                          return A->getIndex() < B->getIndex();
-                        if (A->hasValidIndex() && !B->hasValidIndex())
-                          return true;
-                        if (!A->hasValidIndex() && B->hasValidIndex())
-                          return false;
-                        return A->getAddress() < B->getAddress();
-                      });
+    llvm::stable_sort(SortedFunctions, compareBinaryFunctionByIndex);
 
     for (const BinaryFunction *Func : SortedFunctions) {
       if (!Func->hasValidIndex())

bolt/test/AArch64/data-at-0-offset.c

@@ -1,4 +1,4 @@
-// RUN: %clang %cflags -O2 -fPIE -Wl,-q -pie  %s -o %t.exe
+// RUN: %clang %cflags -O2 -fPIE -std=gnu99 -Wl,-q -pie  %s -o %t.exe
 // RUN: llvm-bolt %t.exe -o %t.bolt 2>&1 | FileCheck %s
 // CHECK-NOT: BOLT-WARNING: unable to disassemble instruction at offset
 

bolt/test/AArch64/double_jump.cpp

@@ -1,6 +1,6 @@
 // A contrived example to test the double jump removal peephole.
 
-// RUN: %clang %cflags -O0 %s -o %t.exe
+// RUN: %clangxx %cxxflags -O0 %s -o %t.exe
 // RUN: llvm-bolt %t.exe -o %t.bolt --peepholes=double-jumps | \
 // RUN:   FileCheck %s -check-prefix=CHECKBOLT
 // RUN: llvm-objdump --no-print-imm-hex -d %t.bolt | FileCheck %s

bolt/test/R_ABS.pic.lld.cpp

@@ -6,7 +6,7 @@
  * with libc available.
  * REQUIRES: system-linux
  *
- * RUN: %clang %cflags -fPIC -shared %s -o %t.so -Wl,-q -fuse-ld=lld
+ * RUN: %clangxx %cxxflags -fPIC -shared %s -o %t.so -Wl,-q -fuse-ld=lld
  * RUN: llvm-bolt %t.so -o %t.so.bolt --relocs
  */
 

bolt/test/X86/double-jump.test

@@ -4,7 +4,7 @@
 ## correctly on Windows e.g. subshell execution
 REQUIRES: shell
 
-RUN: %clang %cflags %p/Inputs/double_jump.cpp -o %t.exe
+RUN: %clangxx %cxxflags %p/Inputs/double_jump.cpp -o %t.exe
 RUN: (llvm-bolt %t.exe --peepholes=double-jumps \
 RUN:   --eliminate-unreachable -o %t 2>&1 \
 RUN:   && llvm-objdump -d %t --print-imm-hex --no-show-raw-insn) | FileCheck %s

bolt/test/X86/dwarf5-df-inlined-subroutine-gc-sections-range.test

@@ -6,7 +6,7 @@
 ; RUN: -split-dwarf-file=main.dwo -o main.o
 ; RUN: llvm-mc -dwarf-version=5 -filetype=obj -triple x86_64-unknown-linux %p/Inputs/dwarf5-df-inlined-subroutine-gc-sections-range-helper.s \
 ; RUN: -split-dwarf-file=helper.dwo -o helper.o
-; RUN: %clang --target=x86_64-pc-linux -fuse-ld=lld -Wl,-gc-sections -Wl,-q -gdwarf-5 -gsplit-dwarf=split main.o helper.o -o main.exe
+; RUN: %clang -fuse-ld=lld -Wl,-gc-sections -Wl,-q -gdwarf-5 -gsplit-dwarf=split main.o helper.o -o main.exe
 ; RUN: llvm-bolt main.exe -o main.exe.bolt --update-debug-sections
 ; RUN: llvm-dwarfdump --debug-addr main.exe > log.txt
 ; RUN: llvm-dwarfdump --debug-rnglists --verbose --show-form main.dwo >> log.txt

bolt/test/X86/jmp-optimization.test

@@ -4,7 +4,7 @@
 ## correctly on Windows e.g. unsupported parameter expansion
 REQUIRES: shell
 
-RUN: %clang %cflags -O2 %S/Inputs/jmp_opt{,2,3}.cpp -o %t
+RUN: %clangxx %cxxflags -O2 %S/Inputs/jmp_opt{,2,3}.cpp -o %t
 RUN: llvm-bolt -inline-small-functions %t -o %t.bolt
 RUN: llvm-objdump -d %t.bolt --print-imm-hex | FileCheck %s
 

bolt/test/X86/match-functions-with-call-graph.test

@@ -2,7 +2,7 @@
 
 # REQUIRES: system-linux
 # RUN: split-file %s %t
-# RUN: %clang %cflags %t/main.cpp -o %t.exe -Wl,-q -nostdlib
+# RUN: %clangxx %cxxflags %t/main.cpp -o %t.exe -Wl,-q -nostdlib
 # RUN: llvm-bolt %t.exe -o %t.out --data %t/yaml --profile-ignore-hash -v=1 \
 # RUN:   --dyno-stats --print-cfg --infer-stale-profile=1 --match-with-call-graph 2>&1 | FileCheck %s
 

bolt/test/pie.test

@@ -4,7 +4,7 @@
 ## on Linux systems where the host triple matches the target.
 REQUIRES: system-linux
 
-RUN: %clang %cflags -fPIC -pie %p/Inputs/jump_table_icp.cpp -o %t
+RUN: %clangxx %cxxflags -fPIC -pie %p/Inputs/jump_table_icp.cpp -o %t
 RUN: llvm-bolt %t -o %t.null 2>&1 | FileCheck %s
 
 CHECK: BOLT-INFO: shared object or position-independent executable detected

bolt/test/runtime/X86/instrumentation-indirect.c

@@ -38,7 +38,7 @@ int main(int argc, char **argv) {
 /*
 REQUIRES: system-linux,bolt-runtime,lit-max-individual-test-time
 
-RUN: %clang %cflags %s -o %t.exe -Wl,-q -pie -fpie
+RUN: %clang %cflags -D_GNU_SOURCE %s -o %t.exe -Wl,-q -pie -fpie
 
 RUN: llvm-bolt %t.exe --instrument --instrumentation-file=%t.fdata \
 RUN:   --instrumentation-wait-forks=1 --conservative-instrumentation \

bolt/test/runtime/bolt-reserved.cpp

@@ -5,7 +5,7 @@
  * new sections.
  */
 
-// RUN: %clang %s -o %t.exe -Wl,-q
+// RUN: %clangxx %s -o %t.exe -Wl,-q
 // RUN: llvm-bolt %t.exe -o %t.bolt.exe 2>&1 | FileCheck %s
 // RUN: %t.bolt.exe
 
@@ -16,7 +16,7 @@
  * not enough for allocating new sections.
  */
 
-// RUN: %clang %s -o %t.tiny.exe -Wl,--no-eh-frame-hdr -Wl,-q -DTINY
+// RUN: %clangxx %s -o %t.tiny.exe -Wl,--no-eh-frame-hdr -Wl,-q -DTINY
 // RUN: not llvm-bolt %t.tiny.exe -o %t.tiny.bolt.exe 2>&1 | \
 // RUN:   FileCheck %s --check-prefix=CHECK-TINY
 

clang-tools-extra/clang-tidy/modernize/UseStartsEndsWithCheck.cpp

@@ -18,6 +18,17 @@
 using namespace clang::ast_matchers;
 
 namespace clang::tidy::modernize {
+
+static bool isNegativeComparison(const Expr *ComparisonExpr) {
+  if (const auto *BO = llvm::dyn_cast<BinaryOperator>(ComparisonExpr))
+    return BO->getOpcode() == BO_NE;
+
+  if (const auto *Op = llvm::dyn_cast<CXXOperatorCallExpr>(ComparisonExpr))
+    return Op->getOperator() == OO_ExclaimEqual;
+
+  return false;
+}
+
 struct NotLengthExprForStringNode {
   NotLengthExprForStringNode(std::string ID, DynTypedNode Node,
                              ASTContext *Context)
@@ -171,10 +182,26 @@ void UseStartsEndsWithCheck::registerMatchers(MatchFinder *Finder) {
                              hasRHS(lengthExprForStringNode("needle")))))
           .bind("expr"),
       this);
+
+  // Case 6: X.substr(0, LEN(Y)) [!=]= Y -> starts_with.
+  Finder->addMatcher(
+      cxxOperatorCallExpr(
+          hasAnyOperatorName("==", "!="),
+          hasOperands(
+              expr().bind("needle"),
+              cxxMemberCallExpr(
+                  argumentCountIs(2), hasArgument(0, ZeroLiteral),
+                  hasArgument(1, lengthExprForStringNode("needle")),
+                  callee(cxxMethodDecl(hasName("substr"),
+                                       ofClass(OnClassWithStartsWithFunction))
+                             .bind("find_fun")))
+                  .bind("find_expr")))
+          .bind("expr"),
+      this);
 }
 
 void UseStartsEndsWithCheck::check(const MatchFinder::MatchResult &Result) {
-  const auto *ComparisonExpr = Result.Nodes.getNodeAs<BinaryOperator>("expr");
+  const auto *ComparisonExpr = Result.Nodes.getNodeAs<Expr>("expr");
   const auto *FindExpr = Result.Nodes.getNodeAs<CXXMemberCallExpr>("find_expr");
   const auto *FindFun = Result.Nodes.getNodeAs<CXXMethodDecl>("find_fun");
   const auto *SearchExpr = Result.Nodes.getNodeAs<Expr>("needle");
@@ -183,39 +210,42 @@ void UseStartsEndsWithCheck::check(const MatchFinder::MatchResult &Result) {
   const auto *EndsWithFunction =
       Result.Nodes.getNodeAs<CXXMethodDecl>("ends_with_fun");
   assert(bool(StartsWithFunction) != bool(EndsWithFunction));
+
   const CXXMethodDecl *ReplacementFunction =
       StartsWithFunction ? StartsWithFunction : EndsWithFunction;
 
-  if (ComparisonExpr->getBeginLoc().isMacroID())
+  if (ComparisonExpr->getBeginLoc().isMacroID() ||
+      FindExpr->getBeginLoc().isMacroID())
     return;
 
-  const bool Neg = ComparisonExpr->getOpcode() == BO_NE;
+  // Make sure FindExpr->getArg(0) can be used to make a range in the FitItHint.
+  if (FindExpr->getNumArgs() == 0)
+    return;
 
-  auto Diagnostic =
-      diag(FindExpr->getExprLoc(), "use %0 instead of %1() %select{==|!=}2 0")
-      << ReplacementFunction->getName() << FindFun->getName() << Neg;
+  // Retrieve the source text of the search expression.
+  const auto SearchExprText = Lexer::getSourceText(
+      CharSourceRange::getTokenRange(SearchExpr->getSourceRange()),
+      *Result.SourceManager, Result.Context->getLangOpts());
 
-  // Remove possible arguments after search expression and ' [!=]= .+' suffix.
-  Diagnostic << FixItHint::CreateReplacement(
-      CharSourceRange::getTokenRange(
-          Lexer::getLocForEndOfToken(SearchExpr->getEndLoc(), 0,
-                                     *Result.SourceManager, getLangOpts()),
-          ComparisonExpr->getEndLoc()),
-      ")");
+  auto Diagnostic = diag(FindExpr->getExprLoc(), "use %0 instead of %1")
+                    << ReplacementFunction->getName() << FindFun->getName();
 
-  // Remove possible '.+ [!=]= ' prefix.
+  // Remove everything before the function call.
   Diagnostic << FixItHint::CreateRemoval(CharSourceRange::getCharRange(
       ComparisonExpr->getBeginLoc(), FindExpr->getBeginLoc()));
 
-  // Replace method name by '(starts|ends)_with'.
-  // Remove possible arguments before search expression.
+  // Rename the function to `starts_with` or `ends_with`.
+  Diagnostic << FixItHint::CreateReplacement(FindExpr->getExprLoc(),
+                                             ReplacementFunction->getName());
+
+  // Replace arguments and everything after the function call.
   Diagnostic << FixItHint::CreateReplacement(
-      CharSourceRange::getCharRange(FindExpr->getExprLoc(),
-                                    SearchExpr->getBeginLoc()),
-      (ReplacementFunction->getName() + "(").str());
+      CharSourceRange::getTokenRange(FindExpr->getArg(0)->getBeginLoc(),
+                                     ComparisonExpr->getEndLoc()),
+      (SearchExprText + ")").str());
 
-  // Add possible negation '!'.
-  if (Neg)
+  // Add negation if necessary.
+  if (isNegativeComparison(ComparisonExpr))
     Diagnostic << FixItHint::CreateInsertion(FindExpr->getBeginLoc(), "!");
 }
 

clang-tools-extra/docs/ReleaseNotes.rst

@@ -262,8 +262,9 @@ Changes in existing checks
   ``NULL``/``__null`` (but not ``0``) when used with a templated type.
 
 - Improved :doc:`modernize-use-starts-ends-with
-  <clang-tidy/checks/modernize/use-starts-ends-with>` check to handle two cases
-  that can be replaced with ``ends_with``
+  <clang-tidy/checks/modernize/use-starts-ends-with>` check to handle two new
+  cases from ``rfind`` and ``compare`` to ``ends_with``, and one new case from
+  ``substr`` to ``starts_with``, and a small adjustment to the  diagnostic message.
 
 - Improved :doc:`modernize-use-std-format
   <clang-tidy/checks/modernize/use-std-format>` check to support replacing

clang-tools-extra/docs/clang-tidy/checks/modernize/use-starts-ends-with.rst

@@ -7,26 +7,16 @@ Checks for common roundabout ways to express ``starts_with`` and ``ends_with``
 and suggests replacing with the simpler method when it is available. Notably, 
 this will work with ``std::string`` and ``std::string_view``.
 
-.. code-block:: c++
+Covered scenarios:
 
-  std::string s = "...";
-  if (s.find("prefix") == 0) { /* do something */ }
-  if (s.rfind("prefix", 0) == 0) { /* do something */ }
-  if (s.compare(0, strlen("prefix"), "prefix") == 0) { /* do something */ }
-  if (s.compare(s.size() - strlen("suffix"), strlen("suffix"), "suffix") == 0) {
-    /* do something */
-  }
-  if (s.rfind("suffix") == (s.length() - 6)) {
-    /* do something */
-  }
-
-becomes
-
-.. code-block:: c++
-
-  std::string s = "...";
-  if (s.starts_with("prefix")) { /* do something */ }
-  if (s.starts_with("prefix")) { /* do something */ }
-  if (s.starts_with("prefix")) { /* do something */ }
-  if (s.ends_with("suffix")) { /* do something */ }
-  if (s.ends_with("suffix")) { /* do something */ }
+==================================================== =====================
+Expression                                           Replacement
+---------------------------------------------------- ---------------------
+``u.find(v) == 0``                                   ``u.starts_with(v)``
+``u.rfind(v, 0) != 0``                               ``!u.starts_with(v)``
+``u.compare(0, v.size(), v) == 0``                   ``u.starts_with(v)``
+``u.substr(0, v.size()) == v``                       ``u.starts_with(v)``
+``v != u.substr(0, v.size())``                       ``!u.starts_with(v)``
+``u.compare(u.size() - v.size(), v.size(), v) == 0`` ``u.ends_with(v)``
+``u.rfind(v) == u.size() - v.size()``                ``u.ends_with(v)``
+==================================================== =====================

clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/string

@@ -60,6 +60,8 @@ struct basic_string {
   _Type& insert(size_type pos, const C* s);
   _Type& insert(size_type pos, const C* s, size_type n);
 
+  _Type substr(size_type pos = 0, size_type count = npos) const;
+
   constexpr bool starts_with(std::basic_string_view<C, T> sv) const noexcept;
   constexpr bool starts_with(C ch) const noexcept;
   constexpr bool starts_with(const C* s) const;