Merge remote-tracking branch 'origin/main' into HEAD

Author: tstellar

bolt/include/bolt/Core/BinaryFunction.h

@@ -2356,10 +2356,9 @@ class BinaryFunction {
   bool postProcessIndirectBranches(MCPlusBuilder::AllocatorIdTy AllocId);
 
   /// Validate that all data references to function offsets are claimed by
-  /// recognized jump tables. Register externally referenced blocks as entry
-  /// points. Returns true if there are no unclaimed externally referenced
-  /// offsets.
-  bool validateExternallyReferencedOffsets();
+  /// recognized jump tables. Returns true if there are no unclaimed externally
+  /// referenced offsets.
+  bool validateInternalRefDataRelocations();
 
   /// Return all call site profile info for this function.
   IndirectCallSiteProfile &getAllCallSites() { return AllCallSites; }

bolt/lib/Core/BinaryFunction.cpp

@@ -2041,41 +2041,47 @@ void BinaryFunction::postProcessJumpTables() {
   }
 }
 
-bool BinaryFunction::validateExternallyReferencedOffsets() {
-  SmallPtrSet<MCSymbol *, 4> JTTargets;
-  for (const JumpTable *JT : llvm::make_second_range(JumpTables))
-    JTTargets.insert_range(JT->Entries);
-
-  bool HasUnclaimedReference = false;
-  for (uint64_t Destination : ExternallyReferencedOffsets) {
-    // Ignore __builtin_unreachable().
-    if (Destination == getSize())
-      continue;
-    // Ignore constant islands
-    if (isInConstantIsland(Destination + getAddress()))
-      continue;
+bool BinaryFunction::validateInternalRefDataRelocations() {
+  if (InternalRefDataRelocations.empty())
+    return true;
 
-    if (BinaryBasicBlock *BB = getBasicBlockAtOffset(Destination)) {
-      // Check if the externally referenced offset is a recognized jump table
-      // target.
-      if (JTTargets.contains(BB->getLabel()))
-        continue;
+  // Rely on the user hint that all data refs are valid and only used as
+  // destinations by indirect branch in the same function.
+  if (opts::StrictMode)
+    return true;
 
-      if (opts::Verbosity >= 1) {
-        BC.errs() << "BOLT-WARNING: unclaimed data to code reference (possibly "
-                  << "an unrecognized jump table entry) to " << BB->getName()
-                  << " in " << *this << "\n";
-      }
-      auto L = BC.scopeLock();
-      addEntryPoint(*BB);
-    } else {
-      BC.errs() << "BOLT-WARNING: unknown data to code reference to offset "
-                << Twine::utohexstr(Destination) << " in " << *this << "\n";
-      setIgnored();
+  DenseSet<uint64_t> UnclaimedRelocations(InternalRefDataRelocations);
+  for (const JumpTable *JT : llvm::make_second_range(JumpTables)) {
+    uint64_t EntryAddress = JT->getAddress();
+    while (EntryAddress < JT->getAddress() + JT->getSize()) {
+      UnclaimedRelocations.erase(EntryAddress);
+      EntryAddress += JT->EntrySize;
     }
-    HasUnclaimedReference = true;
   }
-  return !HasUnclaimedReference;
+
+  if (UnclaimedRelocations.empty())
+    return true;
+
+  BC.errs() << "BOLT-WARNING: " << UnclaimedRelocations.size()
+            << " unclaimed data relocation"
+            << (UnclaimedRelocations.size() > 1 ? "s" : "")
+            << " remain against function " << *this;
+  if (opts::Verbosity) {
+    BC.errs() << ":\n";
+    for (uint64_t RelocationAddress : UnclaimedRelocations) {
+      const Relocation *Relocation = BC.getRelocationAt(RelocationAddress);
+      BC.errs() << "  ";
+      if (Relocation)
+        BC.errs() << *Relocation;
+      else
+        BC.errs() << "<missing relocation>";
+      BC.errs() << '\n';
+    }
+  } else {
+    BC.errs() << ". Re-run with -v=1 to see the list\n";
+  }
+
+  return false;
 }
 
 bool BinaryFunction::postProcessIndirectBranches(
@@ -2200,14 +2206,6 @@ bool BinaryFunction::postProcessIndirectBranches(
     LastIndirectJumpBB->updateJumpTableSuccessors();
   }
 
-  // Validate that all data references to function offsets are claimed by
-  // recognized jump tables. Register externally referenced blocks as entry
-  // points.
-  if (!opts::StrictMode && hasInternalReference()) {
-    if (!validateExternallyReferencedOffsets())
-      return false;
-  }
-
   if (HasUnknownControlFlow && !BC.HasRelocations)
     return false;
 
@@ -2496,12 +2494,18 @@ Error BinaryFunction::buildCFG(MCPlusBuilder::AllocatorIdTy AllocatorId) {
   CurrentState = State::CFG;
 
   // Make any necessary adjustments for indirect branches.
-  if (!postProcessIndirectBranches(AllocatorId)) {
-    if (opts::Verbosity) {
-      BC.errs() << "BOLT-WARNING: failed to post-process indirect branches for "
-                << *this << '\n';
-    }
+  bool ValidCFG = postProcessIndirectBranches(AllocatorId);
+  if (!ValidCFG && opts::Verbosity) {
+    BC.errs() << "BOLT-WARNING: failed to post-process indirect branches for "
+              << *this << '\n';
+  }
+
+  // Validate that all data references to function offsets are claimed by
+  // recognized jump tables.
+  if (ValidCFG)
+    ValidCFG = validateInternalRefDataRelocations();
 
+  if (!ValidCFG) {
     if (BC.isAArch64())
       PreserveNops = BC.HasRelocations;
 

bolt/test/X86/unclaimed-jt-entries.s

@@ -32,12 +32,7 @@
 
 # RUN: llvm-bolt %t.exe -v=1 -o %t.out 2>&1 | FileCheck %s
 
-# CHECK: BOLT-WARNING: unclaimed data to code reference (possibly an unrecognized jump table entry) to .Ltmp[[#]] in main
-# CHECK: BOLT-WARNING: unclaimed data to code reference (possibly an unrecognized jump table entry) to .Ltmp[[#]] in main
-# CHECK: BOLT-WARNING: unclaimed data to code reference (possibly an unrecognized jump table entry) to .Ltmp[[#]] in main
-# CHECK: BOLT-WARNING: unclaimed data to code reference (possibly an unrecognized jump table entry) to .Ltmp[[#]] in main
-# CHECK: BOLT-WARNING: unclaimed data to code reference (possibly an unrecognized jump table entry) to .Ltmp[[#]] in main
-# CHECK: BOLT-WARNING: failed to post-process indirect branches for main
+# CHECK: BOLT-WARNING: 11 unclaimed data relocations remain against function main
 
   .text
   .globl main

bolt/test/runtime/X86/unclaimed-jt-entries.s

@@ -18,14 +18,9 @@
 
 # RUN: llvm-mc -filetype=obj -triple x86_64-unknown-unknown %s -o %t.o
 # RUN: %clang %cflags %S/Inputs/unclaimed-jt-entries.c -no-pie %t.o -o %t.exe -Wl,-q
-# RUN: llvm-bolt %t.exe -v=1 -o %t.out --sequential-disassembly 2>&1 | FileCheck %s
+# RUN: llvm-bolt %t.exe -o %t.out 2>&1 | FileCheck %s
 
-# CHECK: BOLT-WARNING: unclaimed data to code reference (possibly an unrecognized jump table entry) to .Ltmp[[#]] in func
-# CHECK: BOLT-WARNING: unclaimed data to code reference (possibly an unrecognized jump table entry) to .Ltmp[[#]] in func
-# CHECK: BOLT-WARNING: unclaimed data to code reference (possibly an unrecognized jump table entry) to .Ltmp[[#]] in func
-# CHECK: BOLT-WARNING: unclaimed data to code reference (possibly an unrecognized jump table entry) to .Ltmp[[#]] in func
-# CHECK: BOLT-WARNING: unclaimed data to code reference (possibly an unrecognized jump table entry) to .Ltmp[[#]] in func
-# CHECK: BOLT-WARNING: failed to post-process indirect branches for func
+# CHECK: BOLT-WARNING: 11 unclaimed data relocations remain against function func
 
 # Run the optimized binary
 # RUN: %t.out 3 | FileCheck %s --check-prefix=CHECK3

clang-tools-extra/clang-tidy/.clang-tidy

@@ -9,6 +9,11 @@ Checks: >
   -bugprone-narrowing-conversions,
   -bugprone-unchecked-optional-access,
   -bugprone-unused-return-value,
+  cppcoreguidelines-init-variables,
+  cppcoreguidelines-missing-std-forward,
+  cppcoreguidelines-rvalue-reference-param-not-moved,
+  cppcoreguidelines-virtual-class-destructor,
+  google-readability-casting,
   misc-const-correctness,
   modernize-*,
   -modernize-avoid-c-arrays,

clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp

@@ -526,7 +526,8 @@ void ClangTidyDiagnosticConsumer::forwardDiagnostic(const Diagnostic &Info) {
       Builder << Qualifiers::fromOpaqueValue(Info.getRawArg(Index));
       break;
     case clang::DiagnosticsEngine::ak_qualtype:
-      Builder << QualType::getFromOpaquePtr((void *)Info.getRawArg(Index));
+      Builder << QualType::getFromOpaquePtr(
+          reinterpret_cast<void *>(Info.getRawArg(Index)));
       break;
     case clang::DiagnosticsEngine::ak_declarationname:
       Builder << DeclarationName::getFromOpaqueInteger(Info.getRawArg(Index));

clang-tools-extra/clang-tidy/NoLintDirectiveHandler.cpp

@@ -19,7 +19,6 @@
 #include "clang/Basic/SourceManager.h"
 #include "clang/Tooling/Core/Diagnostic.h"
 #include "llvm/ADT/STLExtras.h"
-#include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringMap.h"
 #include "llvm/ADT/StringSwitch.h"

clang-tools-extra/clang-tidy/altera/StructPackAlignCheck.cpp

@@ -63,9 +63,8 @@ void StructPackAlignCheck::check(const MatchFinder::MatchResult &Result) {
     const QualType StructFieldTy = StructField->getType();
     if (StructFieldTy->isIncompleteType())
       return;
-    const unsigned int StructFieldWidth =
-        (unsigned int)Result.Context->getTypeInfo(StructFieldTy.getTypePtr())
-            .Width;
+    const unsigned int StructFieldWidth = static_cast<unsigned int>(
+        Result.Context->getTypeInfo(StructFieldTy.getTypePtr()).Width);
     FieldSizes.emplace_back(StructFieldWidth, StructField->getFieldIndex());
     // FIXME: Recommend a reorganization of the struct (sort by StructField
     // size, largest to smallest).
@@ -79,7 +78,7 @@ void StructPackAlignCheck::check(const MatchFinder::MatchResult &Result) {
       CharUnits::fromQuantity(std::max<clang::CharUnits::QuantityType>(
           std::ceil(static_cast<float>(TotalBitSize) / CharSize), 1));
   const CharUnits MaxAlign = CharUnits::fromQuantity(
-      std::ceil((float)Struct->getMaxAlignment() / CharSize));
+      std::ceil(static_cast<float>(Struct->getMaxAlignment()) / CharSize));
   const CharUnits CurrAlign =
       Result.Context->getASTRecordLayout(Struct).getAlignment();
   const CharUnits NewAlign = computeRecommendedAlignment(MinByteSize);
@@ -99,8 +98,7 @@ void StructPackAlignCheck::check(const MatchFinder::MatchResult &Result) {
     diag(Struct->getLocation(),
          "accessing fields in struct %0 is inefficient due to padding; only "
          "needs %1 bytes but is using %2 bytes")
-        << Struct << (int)MinByteSize.getQuantity()
-        << (int)CurrSize.getQuantity()
+        << Struct << MinByteSize.getQuantity() << CurrSize.getQuantity()
         << FixItHint::CreateInsertion(Struct->getEndLoc().getLocWithOffset(1),
                                       " __attribute__((packed))");
     diag(Struct->getLocation(),
@@ -112,8 +110,7 @@ void StructPackAlignCheck::check(const MatchFinder::MatchResult &Result) {
 
   FixItHint FixIt;
   auto *Attribute = Struct->getAttr<AlignedAttr>();
-  const std::string NewAlignQuantity =
-      std::to_string((int)NewAlign.getQuantity());
+  const std::string NewAlignQuantity = std::to_string(NewAlign.getQuantity());
   if (Attribute) {
     FixIt = FixItHint::CreateReplacement(
         Attribute->getRange(),
@@ -130,7 +127,7 @@ void StructPackAlignCheck::check(const MatchFinder::MatchResult &Result) {
     diag(Struct->getLocation(),
          "accessing fields in struct %0 is inefficient due to poor alignment; "
          "currently aligned to %1 bytes, but recommended alignment is %2 bytes")
-        << Struct << (int)CurrAlign.getQuantity() << NewAlignQuantity << FixIt;
+        << Struct << CurrAlign.getQuantity() << NewAlignQuantity << FixIt;
 
     diag(Struct->getLocation(),
          "use \"__attribute__((aligned(%0)))\" to align struct %1 to %0 bytes",

clang-tools-extra/clang-tidy/altera/UnrollLoopsCheck.cpp

@@ -208,20 +208,22 @@ bool UnrollLoopsCheck::hasLargeNumIterations(const Stmt *Statement,
       return true;
     switch (Op->getOpcode()) {
     case (BO_AddAssign):
-      Iterations = std::ceil(float(EndValue - InitValue) / ConstantValue);
+      Iterations =
+          std::ceil(static_cast<float>(EndValue - InitValue) / ConstantValue);
       break;
     case (BO_SubAssign):
-      Iterations = std::ceil(float(InitValue - EndValue) / ConstantValue);
+      Iterations =
+          std::ceil(static_cast<float>(InitValue - EndValue) / ConstantValue);
       break;
     case (BO_MulAssign):
-      Iterations =
-          1 + ((std::log((double)EndValue) - std::log((double)InitValue)) /
-               std::log((double)ConstantValue));
+      Iterations = 1 + ((std::log(static_cast<double>(EndValue)) -
+                         std::log(static_cast<double>(InitValue))) /
+                        std::log(static_cast<double>(ConstantValue)));
       break;
     case (BO_DivAssign):
-      Iterations =
-          1 + ((std::log((double)InitValue) - std::log((double)EndValue)) /
-               std::log((double)ConstantValue));
+      Iterations = 1 + ((std::log(static_cast<double>(InitValue)) -
+                         std::log(static_cast<double>(EndValue))) /
+                        std::log(static_cast<double>(ConstantValue)));
       break;
     default:
       // All other operators are not handled; assume large bounds.

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

@@ -11,7 +11,6 @@
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/SmallVector.h"
-#include <algorithm>
 
 using namespace clang::ast_matchers;