Merge remote-tracking branch 'origin/main' into vplan-runtime-checks

Author: fhahn

.github/workflows/libcxx-build-and-test.yaml

@@ -49,7 +49,8 @@ env:
 jobs:
   stage1:
     if: github.repository_owner == 'llvm'
-    runs-on: libcxx-runners-8-set
+    runs-on: libcxx-runners-set
+    container: ghcr.io/libcxx/actions-builder:testing-2024-09-21
     continue-on-error: false
     strategy:
       fail-fast: false
@@ -85,7 +86,8 @@ jobs:
             **/crash_diagnostics/*
   stage2:
     if: github.repository_owner == 'llvm'
-    runs-on: libcxx-runners-8-set
+    runs-on: libcxx-runners-set
+    container: ghcr.io/libcxx/actions-builder:testing-2024-09-21
     needs: [ stage1 ]
     continue-on-error: false
     strategy:
@@ -162,20 +164,21 @@ jobs:
           'benchmarks',
           'bootstrapping-build'
         ]
-        machine: [ 'libcxx-runners-8-set' ]
+        machine: [ 'libcxx-runners-set' ]
         include:
         - config: 'generic-cxx26'
-          machine: libcxx-runners-8-set
+          machine: libcxx-runners-set
         - config: 'generic-asan'
-          machine: libcxx-runners-8-set
+          machine: libcxx-runners-set
         - config: 'generic-tsan'
-          machine: libcxx-runners-8-set
+          machine: libcxx-runners-set
         - config: 'generic-ubsan'
-          machine: libcxx-runners-8-set
+          machine: libcxx-runners-set
         # Use a larger machine for MSAN to avoid timeout and memory allocation issues.
         - config: 'generic-msan'
-          machine: libcxx-runners-8-set
+          machine: libcxx-runners-set
     runs-on: ${{ matrix.machine }}
+    container: ghcr.io/libcxx/actions-builder:testing-2024-09-21
     steps:
       - uses: actions/checkout@v4
       - name: ${{ matrix.config }}

bolt/lib/Rewrite/RewriteInstance.cpp

@@ -789,9 +789,44 @@ void RewriteInstance::discoverFileObjects() {
     BinarySection Section(*BC, *cantFail(Sym.getSection()));
     return Section.isAllocatable();
   };
+  auto checkSymbolInSection = [this](const SymbolInfo &S) {
+    // Sometimes, we encounter symbols with addresses outside their section. If
+    // such symbols happen to fall into another section, they can interfere with
+    // disassembly. Notably, this occurs with AArch64 marker symbols ($d and $t)
+    // that belong to .eh_frame, but end up pointing into .text.
+    // As a workaround, we ignore all symbols that lie outside their sections.
+    auto Section = cantFail(S.Symbol.getSection());
+
+    // Accept all absolute symbols.
+    if (Section == InputFile->section_end())
+      return true;
+
+    uint64_t SecStart = Section->getAddress();
+    uint64_t SecEnd = SecStart + Section->getSize();
+    uint64_t SymEnd = S.Address + ELFSymbolRef(S.Symbol).getSize();
+    if (S.Address >= SecStart && SymEnd <= SecEnd)
+      return true;
+
+    auto SymType = cantFail(S.Symbol.getType());
+    // Skip warnings for common benign cases.
+    if (opts::Verbosity < 1 && SymType == SymbolRef::ST_Other)
+      return false; // E.g. ELF::STT_TLS.
+
+    auto SymName = S.Symbol.getName();
+    auto SecName = cantFail(S.Symbol.getSection())->getName();
+    BC->errs() << "BOLT-WARNING: ignoring symbol "
+               << (SymName ? *SymName : "[unnamed]") << " at 0x"
+               << Twine::utohexstr(S.Address) << ", which lies outside "
+               << (SecName ? *SecName : "[unnamed]") << "\n";
+
+    return false;
+  };
   for (const SymbolRef &Symbol : InputFile->symbols())
-    if (isSymbolInMemory(Symbol))
-      SortedSymbols.push_back({cantFail(Symbol.getAddress()), Symbol});
+    if (isSymbolInMemory(Symbol)) {
+      SymbolInfo SymInfo{cantFail(Symbol.getAddress()), Symbol};
+      if (checkSymbolInSection(SymInfo))
+        SortedSymbols.push_back(SymInfo);
+    }
 
   auto CompareSymbols = [this](const SymbolInfo &A, const SymbolInfo &B) {
     if (A.Address != B.Address)

bolt/test/AArch64/Inputs/spurious-marker-symbol.yaml

@@ -0,0 +1,56 @@
+--- !ELF
+FileHeader:
+  Class:            ELFCLASS64
+  Data:             ELFDATA2LSB
+  Type:             ET_EXEC
+  Machine:          EM_AARCH64
+  Entry:            0x2a0000
+ProgramHeaders:
+  - Type:           PT_PHDR
+    Flags:          [ PF_R ]
+    VAddr:          0x40
+    Align:          0x8
+    FileSize:       0xa8
+    MemSize:        0xa8
+    Offset:         0x40
+  - Type:           PT_LOAD
+    Flags:          [ PF_R ]
+    VAddr:          0x0
+    Align:          0x10000
+    FileSize:       0xf8
+    MemSize:        0xf8
+    Offset:         0x0
+  - Type:           PT_LOAD
+    Flags:          [ PF_X, PF_R ]
+    VAddr:          0x2a0000
+    Align:          0x10000
+    FirstSec:       .text
+    LastSec:        .ignored
+Sections:
+  - Name:           .text
+    Type:           SHT_PROGBITS
+    Flags:          [ SHF_ALLOC, SHF_EXECINSTR ]
+    Address:        0x2a0000
+    AddressAlign:   0x4
+    Content:        400580d2c0035fd6
+  - Name:           .ignored
+    Type:           SHT_PROGBITS
+    Flags:          [ SHF_ALLOC ]
+    Address:        0x2a0008
+    AddressAlign:   0x8
+    Size:           0x8
+  - Name:           .eh_frame
+    Type:           SHT_PROGBITS
+    Flags:          [ SHF_ALLOC ]
+    Address:        0x2a0010
+    AddressAlign:   0x8
+    Content:        1000000000000000017a520004781e010b0c1f00140000001800000000002a0008000000000e01410e010000
+Symbols:
+  - Name:           func
+    Section:        .text
+    Value:          0x2a0000
+    Size:           0x8
+  - Name:           '$d.42'
+    Section:        .ignored
+    Value:          0x2a0004
+...

bolt/test/AArch64/spurious-marker-symbol.test

@@ -0,0 +1,61 @@
+// Check that marker symbols ($d, $x) denoting data embedded in code are ignored
+// if they fall outside their respective sections.
+
+// RUN: yaml2obj %S/Inputs/spurious-marker-symbol.yaml -o %t.exe
+// RUN: llvm-bolt %t.exe -o %t.bolt 2>&1 | FileCheck %s
+// CHECK: 1 out of 1 functions were overwritten
+// RUN: llvm-objdump -j .text -d %t.bolt | FileCheck %s -check-prefix=CHECK-DISASM
+// CHECK-DISASM: func
+// CHECK-DISASM: 2a0000: d2800540   mov
+// CHECK-DISASM: 2a0004: d65f03c0   ret
+
+// The YAML encodes the following assembly and debug information:
+
+  .text
+  .globl func
+  .type func, %function
+func:
+  mov    x0, #42
+// $d.42:    (symbol in .ignored, with an address in .text)
+  ret
+
+// .eh_frame contains minimal DWARF with a CFA operation on the `ret`. BOLT
+// should ignore the spurious `$d.42`. If it doesn't, then it will stop
+// disassembling after the `mov` and will fail to process the second
+// DW_CFA_def_cfa_offset.
+//
+// CIE
+//    length:                       00000010
+//    CIE_id:                       00000000
+//    version:                            01
+//    augmentation:
+//      "zR"                        7a 52 00
+//      - read augmentation data
+//      - read FDE pointer encoding
+//    code_alignment_factor:              04
+//    data_alignment_factor:              78  (-8)
+//    return_address_register:            1e  (r30 / lr)
+//
+//    augmentation data:
+//      length:                           01
+//      FDE pointers are absptr+sdata4    0b
+//
+//    initial_instructions:
+//      DW_CFA_def_cfa (31, 0):     0c 1f 00
+//
+// Encoding: 10000000'00000000'01'7a5200'04'78'1e'10'0b'0c1f00
+//
+// FDE
+//    length:                       00000014
+//    CIE_pointer:                  00000018  (backwards offset from here to CIE)
+//    initial_location:             002a0000  (`func` as absptr+sdata4)
+//    address_range:                00000008
+//    augmentation data:
+//      length:                           00
+//    instructions:
+//      DW_CFA_def_cfa_offset (1)      0e 01
+//      DW_CFA_advance_loc (1)            41  (`ret` at 0x2a0004)
+//      DW_CFA_def_cfa_offset (1)      0e 01  Fails unless $d.42 is ignored.
+//      DW_CFA_nop                     00 00
+//
+// Encoding: 14000000'18000000'00002a00'08000000'000e0141'0e010000

clang-tools-extra/clang-include-fixer/InMemorySymbolIndex.cpp

@@ -21,7 +21,7 @@ InMemorySymbolIndex::InMemorySymbolIndex(
 
 std::vector<SymbolAndSignals>
 InMemorySymbolIndex::search(llvm::StringRef Identifier) {
-  auto I = LookupTable.find(std::string(Identifier));
+  auto I = LookupTable.find(Identifier);
   if (I != LookupTable.end())
     return I->second;
   return {};

clang-tools-extra/clang-include-fixer/InMemorySymbolIndex.h

@@ -27,7 +27,8 @@ class InMemorySymbolIndex : public SymbolIndex {
   search(llvm::StringRef Identifier) override;
 
 private:
-  std::map<std::string, std::vector<find_all_symbols::SymbolAndSignals>>
+  std::map<std::string, std::vector<find_all_symbols::SymbolAndSignals>,
+           std::less<>>
       LookupTable;
 };
 

clang-tools-extra/clang-query/Query.cpp

@@ -44,7 +44,9 @@ bool HelpQuery::run(llvm::raw_ostream &OS, QuerySession &QS) const {
         "  set bind-root (true|false)        "
         "Set whether to bind the root matcher to \"root\".\n"
         "  set print-matcher (true|false)    "
-        "Set whether to print the current matcher,\n"
+        "Set whether to print the current matcher.\n"
+        "  set enable-profile (true|false)   "
+        "Set whether to enable matcher profiling.\n"
         "  set traversal <kind>              "
         "Set traversal kind of clang-query session. Available kinds are:\n"
         "    AsIs                            "
@@ -82,27 +84,53 @@ namespace {
 
 struct CollectBoundNodes : MatchFinder::MatchCallback {
   std::vector<BoundNodes> &Bindings;
-  CollectBoundNodes(std::vector<BoundNodes> &Bindings) : Bindings(Bindings) {}
+  StringRef Unit;
+  CollectBoundNodes(std::vector<BoundNodes> &Bindings, StringRef Unit)
+      : Bindings(Bindings), Unit(Unit) {}
   void run(const MatchFinder::MatchResult &Result) override {
     Bindings.push_back(Result.Nodes);
   }
+  StringRef getID() const override { return Unit; }
+};
+
+struct QueryProfiler {
+  llvm::StringMap<llvm::TimeRecord> Records;
+
+  ~QueryProfiler() {
+    llvm::TimerGroup TG("clang-query", "clang-query matcher profiling",
+                        Records);
+    TG.print(llvm::errs());
+    llvm::errs().flush();
+  }
 };
 
 } // namespace
 
 bool MatchQuery::run(llvm::raw_ostream &OS, QuerySession &QS) const {
   unsigned MatchCount = 0;
 
+  std::optional<QueryProfiler> Profiler;
+  if (QS.EnableProfile)
+    Profiler.emplace();
+
   for (auto &AST : QS.ASTs) {
-    MatchFinder Finder;
+    ast_matchers::MatchFinder::MatchFinderOptions FinderOptions;
+    std::optional<llvm::StringMap<llvm::TimeRecord>> Records;
+    if (QS.EnableProfile) {
+      Records.emplace();
+      FinderOptions.CheckProfiling.emplace(*Records);
+    }
+
+    MatchFinder Finder(FinderOptions);
     std::vector<BoundNodes> Matches;
     DynTypedMatcher MaybeBoundMatcher = Matcher;
     if (QS.BindRoot) {
       std::optional<DynTypedMatcher> M = Matcher.tryBind("root");
       if (M)
         MaybeBoundMatcher = *M;
     }
-    CollectBoundNodes Collect(Matches);
+    StringRef OrigSrcName = AST->getOriginalSourceFileName();
+    CollectBoundNodes Collect(Matches, OrigSrcName);
     if (!Finder.addDynamicMatcher(MaybeBoundMatcher, &Collect)) {
       OS << "Not a valid top-level matcher.\n";
       return false;
@@ -111,6 +139,8 @@ bool MatchQuery::run(llvm::raw_ostream &OS, QuerySession &QS) const {
     ASTContext &Ctx = AST->getASTContext();
     Ctx.getParentMapContext().setTraversalKind(QS.TK);
     Finder.matchAST(Ctx);
+    if (QS.EnableProfile)
+      Profiler->Records[OrigSrcName] += (*Records)[OrigSrcName];
 
     if (QS.PrintMatcher) {
       SmallVector<StringRef, 4> Lines;

clang-tools-extra/clang-query/QueryParser.cpp

@@ -182,6 +182,7 @@ enum ParsedQueryVariable {
   PQV_Output,
   PQV_BindRoot,
   PQV_PrintMatcher,
+  PQV_EnableProfile,
   PQV_Traversal
 };
 
@@ -285,6 +286,7 @@ QueryRef QueryParser::doParse() {
             .Case("output", PQV_Output)
             .Case("bind-root", PQV_BindRoot)
             .Case("print-matcher", PQV_PrintMatcher)
+            .Case("enable-profile", PQV_EnableProfile)
             .Case("traversal", PQV_Traversal)
             .Default(PQV_Invalid);
     if (VarStr.empty())
@@ -303,6 +305,9 @@ QueryRef QueryParser::doParse() {
     case PQV_PrintMatcher:
       Q = parseSetBool(&QuerySession::PrintMatcher);
       break;
+    case PQV_EnableProfile:
+      Q = parseSetBool(&QuerySession::EnableProfile);
+      break;
     case PQV_Traversal:
       Q = parseSetTraversalKind(&QuerySession::TK);
       break;

clang-tools-extra/clang-query/QuerySession.h

@@ -26,7 +26,7 @@ class QuerySession {
   QuerySession(llvm::ArrayRef<std::unique_ptr<ASTUnit>> ASTs)
       : ASTs(ASTs), PrintOutput(false), DiagOutput(true),
         DetailedASTOutput(false), BindRoot(true), PrintMatcher(false),
-        Terminate(false), TK(TK_AsIs) {}
+        EnableProfile(false), Terminate(false), TK(TK_AsIs) {}
 
   llvm::ArrayRef<std::unique_ptr<ASTUnit>> ASTs;
 
@@ -36,6 +36,7 @@ class QuerySession {
 
   bool BindRoot;
   bool PrintMatcher;
+  bool EnableProfile;
   bool Terminate;
 
   TraversalKind TK;

clang-tools-extra/clang-tidy/abseil/DurationRewriter.cpp

@@ -244,7 +244,7 @@ std::optional<DurationScale> getScaleForDurationInverse(llvm::StringRef Name) {
        {"ToDoubleNanoseconds", DurationScale::Nanoseconds},
        {"ToInt64Nanoseconds", DurationScale::Nanoseconds}});
 
-  auto ScaleIter = ScaleMap.find(std::string(Name));
+  auto ScaleIter = ScaleMap.find(Name);
   if (ScaleIter == ScaleMap.end())
     return std::nullopt;
 
@@ -260,7 +260,7 @@ std::optional<DurationScale> getScaleForTimeInverse(llvm::StringRef Name) {
        {"ToUnixMicros", DurationScale::Microseconds},
        {"ToUnixNanos", DurationScale::Nanoseconds}});
 
-  auto ScaleIter = ScaleMap.find(std::string(Name));
+  auto ScaleIter = ScaleMap.find(Name);
   if (ScaleIter == ScaleMap.end())
     return std::nullopt;