Merge remote-tracking branch 'origin/main' into libcxx-overriden-function-detection

Author: petrhosek

.github/workflows/build-ci-container.yml

@@ -60,7 +60,7 @@ jobs:
       - name: Test Container
         run: |
           for image in ${{ steps.vars.outputs.container-name-tag }} ${{  steps.vars.outputs.container-name }}; do
-            podman run --rm -it $image /usr/bin/bash -x -c 'printf '\''#include <iostream>\nint main(int argc, char **argv) { std::cout << "Hello\\n"; }'\'' | clang++ -x c++ - && ./a.out | grep Hello'
+            podman run --rm -it $image /usr/bin/bash -x -c 'cd $HOME && printf '\''#include <iostream>\nint main(int argc, char **argv) { std::cout << "Hello\\n"; }'\'' | clang++ -x c++ - && ./a.out | grep Hello'
           done
 
   push-ci-container:

.github/workflows/containers/github-action-ci/Dockerfile

@@ -41,13 +41,13 @@ RUN ninja -C ./build stage2-clang-bolt stage2-install-distribution && ninja -C .
 FROM base
 
 COPY --from=stage1-toolchain $LLVM_SYSROOT $LLVM_SYSROOT
-    
+
 # Need to install curl for hendrikmuhs/ccache-action
 # Need nodejs for some of the GitHub actions.
 # Need perl-modules for clang analyzer tests.
 # Need git for SPIRV-Tools tests.
 RUN apt-get update && \
-    apt-get install -y \
+    DEBIAN_FRONTEND=noninteractive apt-get install -y \
     binutils \ 
     cmake \
     curl \
@@ -56,7 +56,22 @@ RUN apt-get update && \
     ninja-build \
     nodejs \
     perl-modules \
-    python3-psutil
+    python3-psutil \
+
+    # These are needed by the premerge pipeline. Pip is used to install
+    # dependent python packages and ccache is used for build caching. File and
+    # tzdata are used for tests.
+    python3-pip \
+    ccache \
+    file \
+    tzdata
 
 ENV LLVM_SYSROOT=$LLVM_SYSROOT
 ENV PATH=${LLVM_SYSROOT}/bin:${PATH}
+
+# Create a new user to avoid test failures related to a lack of expected
+# permissions issues in some tests. Set the user id to 1001 as that is the
+# user id that Github Actions uses to perform the checkout action.
+RUN useradd gha -u 1001 -m -s /bin/bash
+USER gha
+

.github/workflows/premerge.yaml

@@ -0,0 +1,69 @@
+name: LLVM Premerge Checks
+
+permissions:
+  contents: read
+
+on:
+  pull_request:
+    paths:
+      - .github/workflows/premerge.yaml
+  push:
+    branches:
+      - 'main'
+
+jobs:
+  premerge-checks-linux:
+    if: github.repository_owner == 'llvm'
+    runs-on: llvm-premerge-linux-runners
+    concurrency:
+      group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.sha }}
+      cancel-in-progress: true
+    container:
+      image: ghcr.io/llvm/ci-ubuntu-22.04:latest
+    defaults:
+      run:
+        shell: bash
+    steps:
+      - name: Checkout LLVM
+        uses: actions/checkout@v4
+        with:
+          fetch-depth: 2
+      - name: Setup ccache
+        uses: hendrikmuhs/[email protected]
+      - name: Build and Test
+        run: |
+          git config --global --add safe.directory '*'
+
+          modified_files=$(git diff --name-only HEAD~1...HEAD)
+          modified_dirs=$(echo "$modified_files" | cut -d'/' -f1 | sort -u)
+
+          echo $modified_files
+          echo $modified_dirs
+
+          . ./.ci/compute-projects.sh
+
+          all_projects="bolt clang clang-tools-extra compiler-rt cross-project-tests flang libc libclc lld lldb llvm mlir openmp polly pstl"
+          modified_projects="$(keep-modified-projects ${all_projects})"
+
+          linux_projects_to_test=$(exclude-linux $(compute-projects-to-test 0 ${modified_projects}))
+          linux_check_targets=$(check-targets ${linux_projects_to_test} | sort | uniq)
+          linux_projects=$(add-dependencies ${linux_projects_to_test} | sort | uniq)
+
+          linux_runtimes_to_test=$(compute-runtimes-to-test ${linux_projects_to_test})
+          linux_runtime_check_targets=$(check-targets ${linux_runtimes_to_test} | sort | uniq)
+          linux_runtimes=$(echo ${linux_runtimes_to_test} | sort | uniq)
+
+          if [[ "${linux_projects}" == "" ]]; then
+            echo "No projects to build"
+            exit 0
+          fi
+
+          echo "Building projects: ${linux_projects}"
+          echo "Running project checks targets: ${linux_check_targets}"
+          echo "Building runtimes: ${linux_runtimes}"
+          echo "Running runtimes checks targets: ${linux_runtime_check_targets}"
+
+          export CC=/opt/llvm/bin/clang
+          export CXX=/opt/llvm/bin/clang++
+
+          ./.ci/monolithic-linux.sh "$(echo ${linux_projects} | tr ' ' ';')" "$(echo ${linux_check_targets})" "$(echo ${linux_runtimes} | tr ' ' ';')" "$(echo ${linux_runtime_check_targets})"

bolt/docs/CommandLineArgumentReference.md

@@ -498,9 +498,12 @@
   Automatically put hot code on 2MB page(s) (hugify) at runtime. No manual call
   to hugify is needed in the binary (which is what --hot-text relies on).
 
-- `--icf`
+- `--icf=<value>`
 
   Fold functions with identical code
+  - `all`:  Enable identical code folding
+  - `none`: Disable identical code folding (default)
+  - `safe`: Enable safe identical code folding
 
 - `--icp`
 

bolt/include/bolt/Core/BinaryFunction.h

@@ -428,6 +428,9 @@ class BinaryFunction {
   /// Function order for streaming into the destination binary.
   uint32_t Index{-1U};
 
+  /// Function is referenced by a non-control flow instruction.
+  bool HasAddressTaken{false};
+
   /// Get basic block index assuming it belongs to this function.
   unsigned getIndex(const BinaryBasicBlock *BB) const {
     assert(BB->getIndex() < BasicBlocks.size());
@@ -822,6 +825,14 @@ class BinaryFunction {
     return nullptr;
   }
 
+  /// Return true if function is referenced in a non-control flow instruction.
+  /// This flag is set when the code and relocation analyses are being
+  /// performed, which occurs when safe ICF (Identical Code Folding) is enabled.
+  bool hasAddressTaken() const { return HasAddressTaken; }
+
+  /// Set whether function is referenced in a non-control flow instruction.
+  void setHasAddressTaken(bool AddressTaken) { HasAddressTaken = AddressTaken; }
+
   /// Returns the raw binary encoding of this function.
   ErrorOr<ArrayRef<uint8_t>> getData() const;
 
@@ -2135,6 +2146,9 @@ class BinaryFunction {
   // adjustments.
   void handleAArch64IndirectCall(MCInst &Instruction, const uint64_t Offset);
 
+  /// Analyze instruction to identify a function reference.
+  void analyzeInstructionForFuncReference(const MCInst &Inst);
+
   /// Scan function for references to other functions. In relocation mode,
   /// add relocations for external references. In non-relocation mode, detect
   /// and mark new entry points.

bolt/include/bolt/Passes/IdenticalCodeFolding.h

@@ -11,6 +11,7 @@
 
 #include "bolt/Core/BinaryFunction.h"
 #include "bolt/Passes/BinaryPasses.h"
+#include "llvm/ADT/SparseBitVector.h"
 
 namespace llvm {
 namespace bolt {
@@ -20,22 +21,72 @@ namespace bolt {
 ///
 class IdenticalCodeFolding : public BinaryFunctionPass {
 protected:
-  bool shouldOptimize(const BinaryFunction &BF) const override {
-    if (BF.hasUnknownControlFlow())
-      return false;
-    if (BF.isFolded())
-      return false;
-    if (BF.hasSDTMarker())
-      return false;
-    return BinaryFunctionPass::shouldOptimize(BF);
-  }
+  /// Return true if the function is safe to fold.
+  bool shouldOptimize(const BinaryFunction &BF) const override;
 
 public:
+  enum class ICFLevel {
+    None, /// No ICF. (Default)
+    Safe, /// Safe ICF.
+    All,  /// Aggressive ICF.
+  };
   explicit IdenticalCodeFolding(const cl::opt<bool> &PrintPass)
       : BinaryFunctionPass(PrintPass) {}
 
   const char *getName() const override { return "identical-code-folding"; }
   Error runOnFunctions(BinaryContext &BC) override;
+
+private:
+  /// Bit vector of memory addresses of vtables.
+  llvm::SparseBitVector<> VTableBitVector;
+
+  /// Return true if the memory address is in a vtable.
+  bool isAddressInVTable(uint64_t Address) const {
+    return VTableBitVector.test(Address / 8);
+  }
+
+  /// Mark memory address of a vtable as used.
+  void setAddressUsedInVTable(uint64_t Address) {
+    VTableBitVector.set(Address / 8);
+  }
+
+  /// Scan symbol table and mark memory addresses of
+  /// vtables.
+  void initVTableReferences(const BinaryContext &BC);
+
+  /// Analyze code section and relocations and mark functions that are not
+  /// safe to fold.
+  void markFunctionsUnsafeToFold(BinaryContext &BC);
+
+  /// Process static and dynamic relocations in the data sections to identify
+  /// function references, and mark them as unsafe to fold. It filters out
+  /// symbol references that are in vtables.
+  void analyzeDataRelocations(BinaryContext &BC);
+
+  /// Process functions that have been disassembled and mark functions that are
+  /// used in non-control flow instructions as unsafe to fold.
+  void analyzeFunctions(BinaryContext &BC);
+};
+
+class DeprecatedICFNumericOptionParser
+    : public cl::parser<IdenticalCodeFolding::ICFLevel> {
+public:
+  explicit DeprecatedICFNumericOptionParser(cl::Option &O)
+      : cl::parser<IdenticalCodeFolding::ICFLevel>(O) {}
+
+  bool parse(cl::Option &O, StringRef ArgName, StringRef Arg,
+             IdenticalCodeFolding::ICFLevel &Value) {
+    if (Arg == "0" || Arg == "1") {
+      Value = (Arg == "0") ? IdenticalCodeFolding::ICFLevel::None
+                           : IdenticalCodeFolding::ICFLevel::All;
+      errs() << formatv("BOLT-WARNING: specifying numeric value \"{0}\" "
+                        "for option -{1} is deprecated\n",
+                        Arg, ArgName);
+      return false;
+    }
+    return cl::parser<IdenticalCodeFolding::ICFLevel>::parse(O, ArgName, Arg,
+                                                             Value);
+  }
 };
 
 } // namespace bolt

bolt/lib/Core/BinaryFunction.cpp

@@ -1504,6 +1504,20 @@ MCSymbol *BinaryFunction::registerBranch(uint64_t Src, uint64_t Dst) {
   return Target;
 }
 
+void BinaryFunction::analyzeInstructionForFuncReference(const MCInst &Inst) {
+  for (const MCOperand &Op : MCPlus::primeOperands(Inst)) {
+    if (!Op.isExpr())
+      continue;
+    const MCExpr &Expr = *Op.getExpr();
+    if (Expr.getKind() != MCExpr::SymbolRef)
+      continue;
+    const MCSymbol &Symbol = cast<MCSymbolRefExpr>(Expr).getSymbol();
+    // Set HasAddressTaken for a function regardless of the ICF level.
+    if (BinaryFunction *BF = BC.getFunctionForSymbol(&Symbol))
+      BF->setHasAddressTaken(true);
+  }
+}
+
 bool BinaryFunction::scanExternalRefs() {
   bool Success = true;
   bool DisassemblyFailed = false;
@@ -1624,6 +1638,8 @@ bool BinaryFunction::scanExternalRefs() {
                              [](const MCOperand &Op) { return Op.isExpr(); })) {
       // Skip assembly if the instruction may not have any symbolic operands.
       continue;
+    } else {
+      analyzeInstructionForFuncReference(Instruction);
     }
 
     // Emit the instruction using temp emitter and generate relocations.