[𝘀𝗽𝗿] changes introduced through rebase

Created using spr 1.3.4

[skip ci]

Author: agozillon

bolt/lib/Rewrite/RewriteInstance.cpp

@@ -5432,6 +5432,17 @@ uint64_t RewriteInstance::getNewFunctionOrDataAddress(uint64_t OldAddress) {
   if (BD && BD->isMoved())
     return BD->getOutputAddress();
 
+  if (const BinaryFunction *BF =
+          BC->getBinaryFunctionContainingAddress(OldAddress)) {
+    if (BF->isEmitted()) {
+      BC->errs() << "BOLT-ERROR: unable to get new address corresponding to "
+                    "input address 0x"
+                 << Twine::utohexstr(OldAddress) << " in function " << *BF
+                 << ". Consider adding this function to --skip-funcs=...\n";
+      exit(1);
+    }
+  }
+
   return 0;
 }
 

bolt/test/RISCV/unnamed-sym-no-entry.c

@@ -8,7 +8,7 @@
 /// Verify that the binary indeed contains an unnamed symbol at _start
 // RUN: llvm-readelf -s %t | FileCheck %s --check-prefix=CHECK-ELF
 // CHECK-ELF-DAG: [[#%x,START:]] {{.*}} FUNC GLOBAL DEFAULT [[#%d,SECTION:]] _start{{$}}
-// CHECK-ELF-DAG: [[#%x,START]] {{.*}} NOTYPE LOCAL DEFAULT [[#SECTION]] {{$}}
+// CHECK-ELF-DAG: [[#%x,START]] {{.*}} NOTYPE LOCAL DEFAULT [[#SECTION]] .L0 {{$}}
 
 /// Verify that BOLT did not create an extra entry point for the unnamed symbol
 // RUN: llvm-bolt -o %t.bolt %t --print-cfg | FileCheck %s

bolt/test/X86/indirect-goto-pie.test

@@ -0,0 +1,16 @@
+# Check that llvm-bolt fails to process PIC binaries with computed goto, as the
+# support is not there yet for correctly updating dynamic relocations
+# referencing code inside functions.
+
+REQUIRES: x86_64-linux
+
+RUN: %clang %S/Inputs/indirect_goto.c -o %t -fpic -pie -Wl,-q
+RUN: not llvm-bolt %t -o %t.bolt --relocs=1 --print-cfg --print-only=main \
+RUN:   |& FileCheck %s
+
+# Check that processing works if main() is skipped.
+RUN: llvm-bolt %t -o %t.bolt --relocs=1 --skip-funcs=main
+
+CHECK:  jmpq    *%rax # UNKNOWN CONTROL FLOW
+
+CHECK: BOLT-ERROR: unable to get new address

bolt/test/X86/shrinkwrapping-do-not-pessimize.s

@@ -53,6 +53,6 @@ end_if_1:
   .size _start, .-_start
 
   .data
-rel:  .quad end_if_1
+rel:  .quad _start
 
 # CHECK:   BOLT-INFO: Shrink wrapping moved 0 spills inserting load/stores and 0 spills inserting push/pops

bolt/test/runtime/X86/Inputs/indirect_goto.c

@@ -54,6 +54,7 @@
 #include "PosixReturnCheck.h"
 #include "RedundantBranchConditionCheck.h"
 #include "ReservedIdentifierCheck.h"
+#include "ReturnConstRefFromParameterCheck.h"
 #include "SharedPtrArrayMismatchCheck.h"
 #include "SignalHandlerCheck.h"
 #include "SignedCharMisuseCheck.h"
@@ -137,6 +138,8 @@ class BugproneModule : public ClangTidyModule {
         "bugprone-inaccurate-erase");
     CheckFactories.registerCheck<IncorrectEnableIfCheck>(
         "bugprone-incorrect-enable-if");
+    CheckFactories.registerCheck<ReturnConstRefFromParameterCheck>(
+        "bugprone-return-const-ref-from-parameter");
     CheckFactories.registerCheck<SwitchMissingDefaultCaseCheck>(
         "bugprone-switch-missing-default-case");
     CheckFactories.registerCheck<IncDecInConditionsCheck>(

bolt/test/runtime/X86/indirect-goto-pie.test

@@ -26,6 +26,7 @@ add_clang_library(clangTidyBugproneModule
   ImplicitWideningOfMultiplicationResultCheck.cpp
   InaccurateEraseCheck.cpp
   IncorrectEnableIfCheck.cpp
+  ReturnConstRefFromParameterCheck.cpp
   SuspiciousStringviewDataUsageCheck.cpp
   SwitchMissingDefaultCaseCheck.cpp
   IncDecInConditionsCheck.cpp

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

@@ -0,0 +1,34 @@
+//===--- ReturnConstRefFromParameterCheck.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 "ReturnConstRefFromParameterCheck.h"
+#include "../utils/Matchers.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::bugprone {
+
+void ReturnConstRefFromParameterCheck::registerMatchers(MatchFinder *Finder) {
+  Finder->addMatcher(
+      returnStmt(hasReturnValue(declRefExpr(to(parmVarDecl(hasType(
+                     hasCanonicalType(matchers::isReferenceToConst())))))))
+          .bind("ret"),
+      this);
+}
+
+void ReturnConstRefFromParameterCheck::check(
+    const MatchFinder::MatchResult &Result) {
+  const auto *R = Result.Nodes.getNodeAs<ReturnStmt>("ret");
+  diag(R->getRetValue()->getBeginLoc(),
+       "returning a constant reference parameter may cause a use-after-free "
+       "when the parameter is constructed from a temporary");
+}
+
+} // namespace clang::tidy::bugprone

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

@@ -0,0 +1,40 @@
+//===--- ReturnConstRefFromParameterCheck.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_RETURNCONSTREFFROMPARAMETERCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_RETURNCONSTREFFROMPARAMETERCHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang::tidy::bugprone {
+
+/// Detects return statements that return a constant reference parameter as
+/// constant reference. This may cause use-after-free errors if the caller uses
+/// xvalues as arguments.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/bugprone/return-const-ref-from-parameter.html
+class ReturnConstRefFromParameterCheck : public ClangTidyCheck {
+public:
+  ReturnConstRefFromParameterCheck(StringRef Name, ClangTidyContext *Context)
+      : ClangTidyCheck(Name, Context) {}
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+  std::optional<TraversalKind> getCheckTraversalKind() const override {
+    // Use 'AsIs' to make sure the return type is exactly the same as the
+    // parameter type.
+    return TK_AsIs;
+  }
+  bool isLanguageVersionSupported(const LangOptions &LangOpts) const override {
+    return LangOpts.CPlusPlus;
+  }
+};
+
+} // namespace clang::tidy::bugprone
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_RETURNCONSTREFFROMPARAMETERCHECK_H

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

@@ -43,7 +43,9 @@ void UseStartsEndsWithCheck::registerMatchers(MatchFinder *Finder) {
       callee(cxxMethodDecl(hasName("find")).bind("find_fun")),
       // ... on a class with a starts_with function.
       on(hasType(
-          hasCanonicalType(hasDeclaration(ClassWithStartsWithFunction)))));
+          hasCanonicalType(hasDeclaration(ClassWithStartsWithFunction)))),
+      // Bind search expression.
+      hasArgument(0, expr().bind("search_expr")));
 
   const auto RFindExpr = cxxMemberCallExpr(
       // A method call with a second argument of zero...
@@ -52,15 +54,68 @@ void UseStartsEndsWithCheck::registerMatchers(MatchFinder *Finder) {
       callee(cxxMethodDecl(hasName("rfind")).bind("find_fun")),
       // ... on a class with a starts_with function.
       on(hasType(
-          hasCanonicalType(hasDeclaration(ClassWithStartsWithFunction)))));
+          hasCanonicalType(hasDeclaration(ClassWithStartsWithFunction)))),
+      // Bind search expression.
+      hasArgument(0, expr().bind("search_expr")));
+
+  // Match a string literal and an integer or strlen() call matching the length.
+  const auto HasStringLiteralAndLengthArgs = [](const auto StringArgIndex,
+                                                const auto LengthArgIndex) {
+    return allOf(
+        hasArgument(StringArgIndex, stringLiteral().bind("string_literal_arg")),
+        hasArgument(LengthArgIndex,
+                    anyOf(integerLiteral().bind("integer_literal_size_arg"),
+                          callExpr(callee(functionDecl(parameterCountIs(1),
+                                                       hasName("strlen"))),
+                                   hasArgument(0, stringLiteral().bind(
+                                                      "strlen_arg"))))));
+  };
+
+  // Match a string variable and a call to length() or size().
+  const auto HasStringVariableAndSizeCallArgs = [](const auto StringArgIndex,
+                                                   const auto LengthArgIndex) {
+    return allOf(
+        hasArgument(StringArgIndex, declRefExpr(hasDeclaration(
+                                        decl().bind("string_var_decl")))),
+        hasArgument(LengthArgIndex,
+                    cxxMemberCallExpr(
+                        callee(cxxMethodDecl(isConst(), parameterCountIs(0),
+                                             hasAnyName("size", "length"))),
+                        on(declRefExpr(
+                            to(decl(equalsBoundNode("string_var_decl"))))))));
+  };
 
-  const auto FindOrRFindExpr =
-      cxxMemberCallExpr(anyOf(FindExpr, RFindExpr)).bind("find_expr");
+  // Match either one of the two cases above.
+  const auto HasStringAndLengthArgs =
+      [HasStringLiteralAndLengthArgs, HasStringVariableAndSizeCallArgs](
+          const auto StringArgIndex, const auto LengthArgIndex) {
+        return anyOf(
+            HasStringLiteralAndLengthArgs(StringArgIndex, LengthArgIndex),
+            HasStringVariableAndSizeCallArgs(StringArgIndex, LengthArgIndex));
+      };
+
+  const auto CompareExpr = cxxMemberCallExpr(
+      // A method call with three arguments...
+      argumentCountIs(3),
+      // ... where the first argument is zero...
+      hasArgument(0, ZeroLiteral),
+      // ... named compare...
+      callee(cxxMethodDecl(hasName("compare")).bind("find_fun")),
+      // ... on a class with a starts_with function...
+      on(hasType(
+          hasCanonicalType(hasDeclaration(ClassWithStartsWithFunction)))),
+      // ... where the third argument is some string and the second a length.
+      HasStringAndLengthArgs(2, 1),
+      // Bind search expression.
+      hasArgument(2, expr().bind("search_expr")));
 
   Finder->addMatcher(
-      // Match [=!]= with a zero on one side and a string.(r?)find on the other.
-      binaryOperator(hasAnyOperatorName("==", "!="),
-                     hasOperands(FindOrRFindExpr, ZeroLiteral))
+      // Match [=!]= with a zero on one side and (r?)find|compare on the other.
+      binaryOperator(
+          hasAnyOperatorName("==", "!="),
+          hasOperands(cxxMemberCallExpr(anyOf(FindExpr, RFindExpr, CompareExpr))
+                          .bind("find_expr"),
+                      ZeroLiteral))
           .bind("expr"),
       this);
 }
@@ -69,23 +124,42 @@ void UseStartsEndsWithCheck::check(const MatchFinder::MatchResult &Result) {
   const auto *ComparisonExpr = Result.Nodes.getNodeAs<BinaryOperator>("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>("search_expr");
   const auto *StartsWithFunction =
       Result.Nodes.getNodeAs<CXXMethodDecl>("starts_with_fun");
 
+  const auto *StringLiteralArg =
+      Result.Nodes.getNodeAs<StringLiteral>("string_literal_arg");
+  const auto *IntegerLiteralSizeArg =
+      Result.Nodes.getNodeAs<IntegerLiteral>("integer_literal_size_arg");
+  const auto *StrlenArg = Result.Nodes.getNodeAs<StringLiteral>("strlen_arg");
+
+  // Filter out compare cases where the length does not match string literal.
+  if (StringLiteralArg && IntegerLiteralSizeArg &&
+      StringLiteralArg->getLength() !=
+          IntegerLiteralSizeArg->getValue().getZExtValue()) {
+    return;
+  }
+
+  if (StringLiteralArg && StrlenArg &&
+      StringLiteralArg->getLength() != StrlenArg->getLength()) {
+    return;
+  }
+
   if (ComparisonExpr->getBeginLoc().isMacroID()) {
     return;
   }
 
   const bool Neg = ComparisonExpr->getOpcode() == BO_NE;
 
   auto Diagnostic =
-      diag(FindExpr->getBeginLoc(), "use %0 instead of %1() %select{==|!=}2 0")
+      diag(FindExpr->getExprLoc(), "use %0 instead of %1() %select{==|!=}2 0")
       << StartsWithFunction->getName() << FindFun->getName() << Neg;
 
-  // Remove possible zero second argument and ' [!=]= 0' suffix.
+  // Remove possible arguments after search expression and ' [!=]= 0' suffix.
   Diagnostic << FixItHint::CreateReplacement(
       CharSourceRange::getTokenRange(
-          Lexer::getLocForEndOfToken(FindExpr->getArg(0)->getEndLoc(), 0,
+          Lexer::getLocForEndOfToken(SearchExpr->getEndLoc(), 0,
                                      *Result.SourceManager, getLangOpts()),
           ComparisonExpr->getEndLoc()),
       ")");
@@ -94,11 +168,12 @@ void UseStartsEndsWithCheck::check(const MatchFinder::MatchResult &Result) {
   Diagnostic << FixItHint::CreateRemoval(CharSourceRange::getCharRange(
       ComparisonExpr->getBeginLoc(), FindExpr->getBeginLoc()));
 
-  // Replace '(r?)find' with 'starts_with'.
+  // Replace method name by 'starts_with'.
+  // Remove possible arguments before search expression.
   Diagnostic << FixItHint::CreateReplacement(
-      CharSourceRange::getTokenRange(FindExpr->getExprLoc(),
-                                     FindExpr->getExprLoc()),
-      StartsWithFunction->getName());
+      CharSourceRange::getCharRange(FindExpr->getExprLoc(),
+                                    SearchExpr->getBeginLoc()),
+      (StartsWithFunction->getName() + "(").str());
 
   // Add possible negation '!'.
   if (Neg) {

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

@@ -13,9 +13,10 @@
 
 namespace clang::tidy::modernize {
 
-/// Checks whether a ``find`` or ``rfind`` result is compared with 0 and
-/// suggests replacing with ``starts_with`` when the method exists in the class.
-/// Notably, this will work with ``std::string`` and ``std::string_view``.
+/// Checks for common roundabout ways to express ``starts_with`` and
+/// ``ends_with`` and suggests replacing with ``starts_with`` when the method is
+/// available. Notably, this will work with ``std::string`` and
+/// ``std::string_view``.
 ///
 /// For the user-facing documentation see:
 /// http://clang.llvm.org/extra/clang-tidy/checks/modernize/use-starts-ends-with.html

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

@@ -117,6 +117,13 @@ New checks
   Detects error-prone Curiously Recurring Template Pattern usage, when the CRTP
   can be constructed outside itself and the derived class.
 
+- New :doc:`bugprone-return-const-ref-from-parameter
+  <clang-tidy/checks/bugprone/return-const-ref-from-parameter>` check.
+
+  Detects return statements that return a constant reference parameter as constant
+  reference. This may cause use-after-free errors if the caller uses xvalues as
+  arguments.
+
 - New :doc:`bugprone-suspicious-stringview-data-usage
   <clang-tidy/checks/bugprone/suspicious-stringview-data-usage>` check.
 
@@ -266,6 +273,10 @@ Changes in existing checks
   <clang-tidy/checks/modernize/use-override>` check to also remove any trailing
   whitespace when deleting the ``virtual`` keyword.
 
+- Improved :doc:`modernize-use-starts-ends-with
+  <clang-tidy/checks/modernize/use-starts-ends-with>` check to also handle
+  calls to ``compare`` method.
+
 - Improved :doc:`modernize-use-using <clang-tidy/checks/modernize/use-using>`
   check by adding support for detection of typedefs declared on function level.