Adding implementation for LifetimeSafetyAnalysis::PrintStats

.ci/generate_test_report_lib_test.py

@@ -407,7 +407,6 @@ def test_no_failures_multiple_build_failed_ninja_log(self):
                 ]
             ],
         )
-        print(test)
         self.assertEqual(
             generate_test_report_lib.generate_report(
                 "Foo",

.ci/premerge_advisor_explain.py

@@ -40,7 +40,9 @@ def main(commit_sha: str, build_log_files: list[str]):
             explanation_request["failures"].append(
                 {"name": name, "message": failure_message}
             )
-    advisor_response = requests.get(PREMERGE_ADVISOR_URL, json=explanation_request)
+    advisor_response = requests.get(
+        PREMERGE_ADVISOR_URL, json=explanation_request, timeout=5
+    )
     if advisor_response.status_code == 200:
         print(advisor_response.json())
     else:

.ci/premerge_advisor_upload.py

@@ -45,7 +45,7 @@ def main(commit_sha, workflow_run_number, build_log_files):
         for name, failure_message in ninja_failures:
             failure_info["failures"].append({"name": name, "message": failure_message})
     for premerge_advisor_url in PREMERGE_ADVISOR_URLS:
-        requests.post(premerge_advisor_url, json=failure_info)
+        requests.post(premerge_advisor_url, json=failure_info, timeout=5)
 
 
 if __name__ == "__main__":

.github/instructions/lldb.instructions.md

@@ -0,0 +1,79 @@
+---
+applyTo: lldb/**/*
+---
+
+When reviewing code, focus on:
+
+## Language, Libraries & Standards
+
+- Target C++17 and avoid vendor-specific extensions.
+- For Python scripts, follow PEP 8.
+- Prefer standard library or LLVM support libraries instead of reinventing data structures.
+
+## Comments & Documentation
+
+- Each source file should include the standard LLVM file header.
+- Header files must have proper header guards.
+- Non-trivial classes and public methods should have Doxygen documentation.
+- Use `//` or `///` comments normally; avoid block comments unless necessary.
+- Non-trivial code should have comments explaining what it does and why. Avoid comments that explain how it does it at a micro level.
+
+## Language & Compiler Issues
+
+- Write portable code; wrap non-portable code in interfaces.
+- Do not use RTTI or exceptions.
+- Prefer C++-style casts over C-style casts.
+- Do not use static constructors.
+- Use `class` or `struct` consistently; `struct` only for all-public data.
+- When then same class is declared or defined multiple times, make sure it's consistently done using either `class` or `struct`.
+
+## Headers & Library Layering
+
+- Include order: module header → local/private headers → project headers → system headers.
+- Headers must compile standalone (include all dependencies).
+- Maintain proper library layering; avoid circular dependencies.
+- Include minimally; use forward declarations where possible.
+- Keep internal headers private to modules.
+- Use full namespace qualifiers for out-of-line definitions.
+
+## Control Flow & Structure
+
+- Prefer early exits over deep nesting.
+- Do not use `else` after `return`, `continue`, `break`, or `goto`.
+- Encapsulate loops that compute predicates into helper functions.
+
+## Naming
+
+- LLDB's code style differs from LLVM's coding style.
+- Variables are `snake_case`.
+- Functions and methods are `UpperCamelCase`.
+- Static, global and member variables have `s_`, `g_` and `m_` prefixes respectively.
+
+## General Guidelines
+
+- Use `assert` liberally; prefer `llvm_unreachable` for unreachable states.
+- Do not use `using namespace std;` in headers.
+- Provide a virtual method anchor for classes defined in headers.
+- Do not use default labels in fully covered switches over enumerations.
+- Use range-based for loops wherever possible.
+- Capture `end()` outside loops if not using range-based iteration.
+- Including `<iostream>` is forbidded. Use LLVM’s `raw_ostream` instead.
+- Don’t use `inline` when defining a function in a class definition.
+
+## Microscopic Details
+
+- Preserve existing style in modified code.
+- Prefer pre-increment (`++i`) when value is unused.
+- Use `private`, `protected`, or `public` keyword as appropriate to restrict class member visibility.
+- Omit braces for single-statement `if`, `else`, `while`, `for` unless needed.
+
+## Review Style
+
+- Be specific and actionable in feedback.
+- Explain the "why" behind recommendations.
+- Link back to the LLVM Coding Standards: https://llvm.org/docs/CodingStandards.html.
+- Ask clarifying questions when code intent is unclear.
+
+Ignore formatting and assume that's handled by external tools like `clang-format` and `black`.
+Remember that these standards are **guidelines**.
+Always prioritize consistency with the style that is already being used by the surrounding code.

.github/copilot-instructions.md → .github/instructions/llvm.instructions.md

@@ -1,3 +1,7 @@
+---
+applyTo: llvm/**/*
+---
+
 When performing a code review, pay close attention to code modifying a function's
 control flow. Could the change result in the corruption of performance profile
 data? Could the change result in invalid debug information, in particular for

.github/workflows/release-binaries-all.yml

@@ -90,7 +90,6 @@ jobs:
         runs-on:
           - ubuntu-22.04
           - ubuntu-22.04-arm
-          - macos-13
           - macos-14
 
     uses: ./.github/workflows/release-binaries.yml

.github/workflows/release-binaries.yml

@@ -21,7 +21,6 @@ on:
         options:
           - ubuntu-22.04
           - ubuntu-22.04-arm
-          - macos-13
           - macos-14
 
   workflow_call:
@@ -130,8 +129,6 @@ jobs:
           target_cmake_flags="$target_cmake_flags -DBOOTSTRAP_BOOTSTRAP_COMPILER_RT_ENABLE_IOS=OFF"
           if [ "$RUNNER_ARCH" = "ARM64" ]; then
             arches=arm64
-          else
-            arches=x86_64
           fi
           target_cmake_flags="$target_cmake_flags -DBOOTSTRAP_BOOTSTRAP_DARWIN_osx_ARCHS=$arches -DBOOTSTRAP_BOOTSTRAP_DARWIN_osx_BUILTIN_ARCHS=$arches"
         fi
@@ -147,14 +144,6 @@ jobs:
             build_runs_on="depot-${{ inputs.runs-on }}-16"
             test_runs_on=$build_runs_on
             ;;
-          macos-13)
-            if [ "$GITHUB_EVENT_NAME" = "pull_request" ]; then
-              build_runs_on="${{ inputs.runs-on }}"
-            else
-              build_runs_on="macos-13-large"
-            fi
-            test_runs_on="${{ inputs.runs-on }}"
-            ;;
           macos-14)
             if [ "$GITHUB_EVENT_NAME" = "pull_request" ]; then
               build_runs_on="${{ inputs.runs-on }}"

bolt/include/bolt/Core/BinaryContext.h

@@ -781,11 +781,6 @@ class BinaryContext {
     uint64_t PseudoProbeLooseMatchedSampleCount{0};
     ///   the count of call matched samples
     uint64_t CallMatchedSampleCount{0};
-    ///   the number of stale functions that have matching number of blocks in
-    ///   the profile
-    uint64_t NumStaleFuncsWithEqualBlockCount{0};
-    ///   the number of blocks that have matching size but a differing hash
-    uint64_t NumStaleBlocksWithEqualIcount{0};
   } Stats;
 
   // Original binary execution count stats.
@@ -937,6 +932,16 @@ class BinaryContext {
   std::pair<const MCSymbol *, uint64_t>
   handleAddressRef(uint64_t Address, BinaryFunction &BF, bool IsPCRel);
 
+  /// When \p Address inside function \p BF is a target of a control transfer
+  /// instruction (branch) from another function, return a corresponding symbol
+  /// that should be used by the branch. For example, main or secondary entry
+  /// point.
+  ///
+  /// If \p Address is an invalid destination, such as a constant island, return
+  /// nullptr and mark \p BF as ignored, since we cannot properly handle a
+  /// branch to a constant island.
+  MCSymbol *handleExternalBranchTarget(uint64_t Address, BinaryFunction &BF);
+
   /// Analyze memory contents at the given \p Address and return the type of
   /// memory contents (such as a possible jump table).
   MemoryContentsType analyzeMemoryAt(uint64_t Address, BinaryFunction &BF);

bolt/include/bolt/Passes/MarkRAStates.h

@@ -13,11 +13,16 @@
 #define BOLT_PASSES_MARK_RA_STATES
 
 #include "bolt/Passes/BinaryPasses.h"
+#include <mutex>
 
 namespace llvm {
 namespace bolt {
 
 class MarkRAStates : public BinaryFunctionPass {
+  // setIgnored() is not thread-safe, but the pass is running on functions in
+  // parallel.
+  std::mutex IgnoreMutex;
+
 public:
   explicit MarkRAStates() : BinaryFunctionPass(false) {}
 

bolt/lib/Core/BinaryContext.cpp

@@ -518,6 +518,23 @@ BinaryContext::handleAddressRef(uint64_t Address, BinaryFunction &BF,
   return std::make_pair(TargetSymbol, 0);
 }
 
+MCSymbol *BinaryContext::handleExternalBranchTarget(uint64_t Address,
+                                                    BinaryFunction &BF) {
+  if (BF.isInConstantIsland(Address)) {
+    BF.setIgnored();
+    this->outs() << "BOLT-WARNING: ignoring entry point at address 0x"
+                 << Twine::utohexstr(Address)
+                 << " in constant island of function " << BF << '\n';
+    return nullptr;
+  }
+
+  const uint64_t Offset = Address - BF.getAddress();
+  assert(Offset < BF.getSize() &&
+         "Address should be inside the referenced function");
+
+  return Offset ? BF.addEntryPointAtOffset(Offset) : BF.getSymbol();
+}
+
 MemoryContentsType BinaryContext::analyzeMemoryAt(uint64_t Address,
                                                   BinaryFunction &BF) {
   if (!isX86())
@@ -1399,17 +1416,10 @@ void BinaryContext::processInterproceduralReferences() {
             << Function.getPrintName() << " and "
             << TargetFunction->getPrintName() << '\n';
       }
-      if (uint64_t Offset = Address - TargetFunction->getAddress()) {
-        if (!TargetFunction->isInConstantIsland(Address)) {
-          TargetFunction->addEntryPointAtOffset(Offset);
-        } else {
-          TargetFunction->setIgnored();
-          this->outs() << "BOLT-WARNING: Ignoring entry point at address 0x"
-                       << Twine::utohexstr(Address)
-                       << " in constant island of function " << *TargetFunction
-                       << '\n';
-        }
-      }
+
+      // Create an extra entry point if needed. Can also render the target
+      // function ignored if the reference is invalid.
+      handleExternalBranchTarget(Address, *TargetFunction);
 
       continue;
     }

bolt/lib/Core/BinaryFunction.cpp

@@ -1697,11 +1697,12 @@ bool BinaryFunction::scanExternalRefs() {
       if (!TargetFunction || ignoreFunctionRef(*TargetFunction))
         continue;
 
-      const uint64_t FunctionOffset =
-          TargetAddress - TargetFunction->getAddress();
+      // Get a reference symbol for the function when address is a valid code
+      // reference.
       BranchTargetSymbol =
-          FunctionOffset ? TargetFunction->addEntryPointAtOffset(FunctionOffset)
-                         : TargetFunction->getSymbol();
+          BC.handleExternalBranchTarget(TargetAddress, *TargetFunction);
+      if (!BranchTargetSymbol)
+        continue;
     }
 
     // Can't find more references. Not creating relocations since we are not

bolt/lib/Passes/BinaryPasses.cpp

@@ -1508,12 +1508,6 @@ Error PrintProgramStats::runOnFunctions(BinaryContext &BC) {
   if (NumAllStaleFunctions) {
     const float PctStale =
         NumAllStaleFunctions / (float)NumAllProfiledFunctions * 100.0f;
-    const float PctStaleFuncsWithEqualBlockCount =
-        (float)BC.Stats.NumStaleFuncsWithEqualBlockCount /
-        NumAllStaleFunctions * 100.0f;
-    const float PctStaleBlocksWithEqualIcount =
-        (float)BC.Stats.NumStaleBlocksWithEqualIcount /
-        BC.Stats.NumStaleBlocks * 100.0f;
     auto printErrorOrWarning = [&]() {
       if (PctStale > opts::StaleThreshold)
         BC.errs() << "BOLT-ERROR: ";
@@ -1536,17 +1530,6 @@ Error PrintProgramStats::runOnFunctions(BinaryContext &BC) {
                 << "%) belong to functions with invalid"
                    " (possibly stale) profile.\n";
     }
-    BC.outs() << "BOLT-INFO: " << BC.Stats.NumStaleFuncsWithEqualBlockCount
-              << " stale function"
-              << (BC.Stats.NumStaleFuncsWithEqualBlockCount == 1 ? "" : "s")
-              << format(" (%.1f%% of all stale)",
-                        PctStaleFuncsWithEqualBlockCount)
-              << " have matching block count.\n";
-    BC.outs() << "BOLT-INFO: " << BC.Stats.NumStaleBlocksWithEqualIcount
-              << " stale block"
-              << (BC.Stats.NumStaleBlocksWithEqualIcount == 1 ? "" : "s")
-              << format(" (%.1f%% of all stale)", PctStaleBlocksWithEqualIcount)
-              << " have matching icount.\n";
     if (PctStale > opts::StaleThreshold) {
       return createFatalBOLTError(
           Twine("BOLT-ERROR: stale functions exceed specified threshold of ") +

bolt/lib/Passes/MarkRAStates.cpp

@@ -43,10 +43,11 @@ bool MarkRAStates::runOnFunction(BinaryFunction &BF) {
         // Not all functions have .cfi_negate_ra_state in them. But if one does,
         // we expect psign/pauth instructions to have the hasNegateRAState
         // annotation.
-        BF.setIgnored();
         BC.outs() << "BOLT-INFO: inconsistent RAStates in function "
                   << BF.getPrintName()
                   << ": ptr sign/auth inst without .cfi_negate_ra_state\n";
+        std::lock_guard<std::mutex> Lock(IgnoreMutex);
+        BF.setIgnored();
         return false;
       }
     }
@@ -67,6 +68,7 @@ bool MarkRAStates::runOnFunction(BinaryFunction &BF) {
           BC.outs() << "BOLT-INFO: inconsistent RAStates in function "
                     << BF.getPrintName()
                     << ": ptr signing inst encountered in Signed RA state\n";
+          std::lock_guard<std::mutex> Lock(IgnoreMutex);
           BF.setIgnored();
           return false;
         }
@@ -80,6 +82,7 @@ bool MarkRAStates::runOnFunction(BinaryFunction &BF) {
                     << BF.getPrintName()
                     << ": ptr authenticating inst encountered in Unsigned RA "
                        "state\n";
+          std::lock_guard<std::mutex> Lock(IgnoreMutex);
           BF.setIgnored();
           return false;
         }

bolt/lib/Profile/DataAggregator.cpp

@@ -1321,7 +1321,8 @@ std::error_code DataAggregator::parseAggregatedLBREntry() {
     }
 
     using SSI = StringSwitch<int>;
-    AddrNum = SSI(Str).Cases("T", "R", 3).Case("S", 1).Case("E", 0).Default(2);
+    AddrNum =
+        SSI(Str).Cases({"T", "R"}, 3).Case("S", 1).Case("E", 0).Default(2);
     CounterNum = SSI(Str).Case("B", 2).Case("E", 0).Default(1);
   }
 
@@ -2215,7 +2216,7 @@ DataAggregator::writeAggregatedFile(StringRef OutputFilename) const {
     OutFile << "boltedcollection\n";
   if (opts::BasicAggregation) {
     OutFile << "no_lbr";
-    for (const StringMapEntry<std::nullopt_t> &Entry : EventNames)
+    for (const StringMapEntry<EmptyStringSetTag> &Entry : EventNames)
       OutFile << " " << Entry.getKey();
     OutFile << "\n";
 
@@ -2291,7 +2292,7 @@ std::error_code DataAggregator::writeBATYAML(BinaryContext &BC,
 
   ListSeparator LS(",");
   raw_string_ostream EventNamesOS(BP.Header.EventNames);
-  for (const StringMapEntry<std::nullopt_t> &EventEntry : EventNames)
+  for (const StringMapEntry<EmptyStringSetTag> &EventEntry : EventNames)
     EventNamesOS << LS << EventEntry.first().str();
 
   BP.Header.Flags = opts::BasicAggregation ? BinaryFunction::PF_BASIC

bolt/lib/Profile/YAMLProfileReader.cpp

@@ -350,9 +350,6 @@ bool YAMLProfileReader::parseFunctionProfile(
              << MismatchedCalls << " calls, and " << MismatchedEdges
              << " edges in profile did not match function " << BF << '\n';
 
-    if (YamlBF.NumBasicBlocks != BF.size())
-      ++BC.Stats.NumStaleFuncsWithEqualBlockCount;
-
     if (!opts::InferStaleProfile)
       return false;
     ArrayRef<ProbeMatchSpec> ProbeMatchSpecs;

bolt/lib/Profile/YAMLProfileWriter.cpp

@@ -382,7 +382,7 @@ std::error_code YAMLProfileWriter::writeProfile(const RewriteInstance &RI) {
   StringSet<> EventNames = RI.getProfileReader()->getEventNames();
   if (!EventNames.empty()) {
     std::string Sep;
-    for (const StringMapEntry<std::nullopt_t> &EventEntry : EventNames) {
+    for (const StringMapEntry<EmptyStringSetTag> &EventEntry : EventNames) {
       BP.Header.EventNames += Sep + EventEntry.first().str();
       Sep = ",";
     }

bolt/test/AArch64/constant-island-entry.s

@@ -1,11 +1,16 @@
-// This test checks that we ignore functions which add an entry point that
-// is in a constant island.
+## This test checks that we ignore functions which add an entry point that
+## is in a constant island.
 
 # RUN: llvm-mc -filetype=obj -triple aarch64-unknown-unknown %s -o %t.o
 # RUN: %clang %cflags %t.o -pie -Wl,-q -o %t.exe
+
+## Check when the caller is successfully disassembled.
 # RUN: llvm-bolt %t.exe -o %t.bolt 2>&1 | FileCheck %s
 
-# CHECK: BOLT-WARNING: Ignoring entry point at address 0x{{[0-9a-f]+}} in constant island of function func
+## Skip caller to check the identical warning is triggered from ScanExternalRefs().
+# RUN: llvm-bolt %t.exe -o %t.bolt -skip-funcs=caller 2>&1 | FileCheck %s
+
+# CHECK: BOLT-WARNING: ignoring entry point at address 0x{{[0-9a-f]+}} in constant island of function func
 
 .globl func
 .type func, %function