Merge branch 'main' into users/boomanaiden154/build-metrics-container

Author: boomanaiden154

.ci/generate_test_report.py

@@ -18,7 +18,7 @@ def junit_from_xml(xml):
 
 class TestReports(unittest.TestCase):
     def test_title_only(self):
-        self.assertEqual(_generate_report("Foo", []), ("", None))
+        self.assertEqual(_generate_report("Foo", []), ("", "success"))
 
     def test_no_tests_in_testsuite(self):
         self.assertEqual(
@@ -336,7 +336,7 @@ def _generate_report(title, junit_objects, size_limit=1024 * 1024, list_failures
                     )
 
     if not tests_run:
-        return ("", style)
+        return ("", None)
 
     style = "error" if tests_failed else "success"
     report = [f"# {title}", ""]

.ci/metrics/Dockerfile

@@ -1,4 +1,8 @@
+<<<<<<< HEAD
 FROM python:3.12
+=======
+FROM docker.io/python:3.12
+>>>>>>> main
 
 COPY requirements.lock.txt ./
 RUN pip3 install --no-cache-dir -r requirements.lock.txt

.ci/metrics/metrics.py

@@ -2,7 +2,9 @@
 import time
 import os
 from dataclasses import dataclass
+import sys
 
+import github
 from github import Github
 from github import Auth
 
@@ -11,6 +13,7 @@
 )
 GITHUB_PROJECT = "llvm/llvm-project"
 WORKFLOWS_TO_TRACK = ["Check code formatting"]
+SCRAPE_INTERVAL_SECONDS = 5 * 60
 
 
 @dataclass
@@ -23,7 +26,7 @@ class JobMetrics:
     workflow_id: int
 
 
-def get_metrics(github_repo, workflows_to_track):
+def get_metrics(github_repo: github.Repository, workflows_to_track: dict[str, int]):
     """Gets the metrics for specified Github workflows.
 
     This function takes in a list of workflows to track, and optionally the
@@ -44,25 +47,24 @@ def get_metrics(github_repo, workflows_to_track):
 
     workflow_metrics = []
 
-    workflows_to_include = {}
-    for workflow_to_track in workflows_to_track:
-        workflows_to_include[workflow_to_track] = True
-    workflows_left_to_include = len(workflows_to_track)
+    workflows_to_include = set(workflows_to_track.keys())
 
-    while True:
+    while len(workflows_to_include) > 0:
         workflow_run = next(workflow_runs)
         if workflow_run.status != "completed":
             continue
 
-        interesting_workflow = False
-        for workflow_name in workflows_to_track:
-            if workflow_run.name == workflow_name:
-                interesting_workflow = True
-                break
-        if not interesting_workflow:
+        # This workflow was already sampled for this run, or is not tracked at
+        # all. Ignoring.
+        if workflow_run.name not in workflows_to_include:
             continue
 
-        if not workflows_to_include[workflow_run.name]:
+        # There were no new workflow invocations since the previous scrape.
+        # The API returns a sorted list with the most recent invocations first,
+        # so we can stop looking for this particular workflow. Continue to grab
+        # information on the other workflows of interest, if present.
+        if workflows_to_track[workflow_run.name] == workflow_run.id:
+            workflows_to_include.remove(workflow_run.name)
             continue
 
         workflow_jobs = workflow_run.jobs()
@@ -89,14 +91,15 @@ def get_metrics(github_repo, workflows_to_track):
             workflows_to_track[workflow_run.name] is None
             or workflows_to_track[workflow_run.name] == workflow_run.id
         ):
-            workflows_left_to_include -= 1
-            workflows_to_include[workflow_run.name] = False
+            workflows_to_include.remove(workflow_run.name)
         if (
             workflows_to_track[workflow_run.name] is not None
-            and workflows_left_to_include == 0
+            and len(workflows_to_include) == 0
         ):
             break
 
+        # The timestamp associated with the event is expected by Grafana to be
+        # in nanoseconds.
         created_at_ns = int(created_at.timestamp()) * 10**9
 
         workflow_metrics.append(
@@ -110,9 +113,6 @@ def get_metrics(github_repo, workflows_to_track):
             )
         )
 
-        if workflows_left_to_include == 0:
-            break
-
     return workflow_metrics
 
 
@@ -143,7 +143,9 @@ def upload_metrics(workflow_metrics, metrics_userid, api_key):
     )
 
     if response.status_code < 200 or response.status_code >= 300:
-        print(f"Failed to submit data to Grafana: {response.status_code}")
+        print(
+            f"Failed to submit data to Grafana: {response.status_code}", file=sys.stderr
+        )
 
 
 def main():
@@ -164,16 +166,16 @@ def main():
     while True:
         current_metrics = get_metrics(github_repo, workflows_to_track)
         if len(current_metrics) == 0:
-            print("No metrics found to upload.")
+            print("No metrics found to upload.", file=sys.stderr)
             continue
 
         upload_metrics(current_metrics, grafana_metrics_userid, grafana_api_key)
-        print(f"Uploaded {len(current_metrics)} metrics")
+        print(f"Uploaded {len(current_metrics)} metrics", file=sys.stderr)
 
         for workflow_metric in reversed(current_metrics):
             workflows_to_track[workflow_metric.job_name] = workflow_metric.workflow_id
 
-        time.sleep(5 * 60)
+        time.sleep(SCRAPE_INTERVAL_SECONDS)
 
 
 if __name__ == "__main__":

.github/new-issues-labeler.yml

@@ -30,3 +30,9 @@
 
 'infra:commit-access-request':
   - '/Request Commit Access/'
+
+'false-positive':
+  - '\bfalse[- ]positive\b'
+
+'false-negative':
+  - '\bfalse[- ]negative\b'

bolt/include/bolt/Core/BinaryFunction.h

@@ -2407,6 +2407,19 @@ inline raw_ostream &operator<<(raw_ostream &OS,
   return OS;
 }
 
+/// Compare function by index if it is valid, fall back to the original address
+/// otherwise.
+inline bool compareBinaryFunctionByIndex(const BinaryFunction *A,
+                                         const BinaryFunction *B) {
+  if (A->hasValidIndex() && B->hasValidIndex())
+    return A->getIndex() < B->getIndex();
+  if (A->hasValidIndex() && !B->hasValidIndex())
+    return true;
+  if (!A->hasValidIndex() && B->hasValidIndex())
+    return false;
+  return A->getAddress() < B->getAddress();
+}
+
 } // namespace bolt
 
 // GraphTraits specializations for function basic block graphs (CFGs)

bolt/lib/Core/BinaryContext.cpp

@@ -1607,13 +1607,7 @@ std::vector<BinaryFunction *> BinaryContext::getSortedFunctions() {
                   SortedFunctions.begin(),
                   [](BinaryFunction &BF) { return &BF; });
 
-  llvm::stable_sort(SortedFunctions,
-                    [](const BinaryFunction *A, const BinaryFunction *B) {
-                      if (A->hasValidIndex() && B->hasValidIndex()) {
-                        return A->getIndex() < B->getIndex();
-                      }
-                      return A->hasValidIndex();
-                    });
+  llvm::stable_sort(SortedFunctions, compareBinaryFunctionByIndex);
   return SortedFunctions;
 }
 

bolt/lib/Core/BinaryFunction.cpp

@@ -385,16 +385,7 @@ bool BinaryFunction::isForwardCall(const MCSymbol *CalleeSymbol) const {
   if (CalleeBF) {
     if (CalleeBF->isInjected())
       return true;
-
-    if (hasValidIndex() && CalleeBF->hasValidIndex()) {
-      return getIndex() < CalleeBF->getIndex();
-    } else if (hasValidIndex() && !CalleeBF->hasValidIndex()) {
-      return true;
-    } else if (!hasValidIndex() && CalleeBF->hasValidIndex()) {
-      return false;
-    } else {
-      return getAddress() < CalleeBF->getAddress();
-    }
+    return compareBinaryFunctionByIndex(this, CalleeBF);
   } else {
     // Absolute symbol.
     ErrorOr<uint64_t> CalleeAddressOrError = BC.getSymbolValue(*CalleeSymbol);

bolt/lib/Passes/ReorderFunctions.cpp

@@ -473,16 +473,7 @@ Error ReorderFunctions::runOnFunctions(BinaryContext &BC) {
                     [](BinaryFunction &BF) { return &BF; });
 
     // Sort functions by index.
-    llvm::stable_sort(SortedFunctions,
-                      [](const BinaryFunction *A, const BinaryFunction *B) {
-                        if (A->hasValidIndex() && B->hasValidIndex())
-                          return A->getIndex() < B->getIndex();
-                        if (A->hasValidIndex() && !B->hasValidIndex())
-                          return true;
-                        if (!A->hasValidIndex() && B->hasValidIndex())
-                          return false;
-                        return A->getAddress() < B->getAddress();
-                      });
+    llvm::stable_sort(SortedFunctions, compareBinaryFunctionByIndex);
 
     for (const BinaryFunction *Func : SortedFunctions) {
       if (!Func->hasValidIndex())

bolt/lib/Rewrite/RewriteInstance.cpp

@@ -2927,6 +2927,23 @@ void RewriteInstance::handleRelocation(const SectionRef &RelocatedSection,
     LLVM_DEBUG(dbgs() << "BOLT-DEBUG: ignoring relocation from data to data\n");
 }
 
+static BinaryFunction *getInitFunctionIfStaticBinary(BinaryContext &BC) {
+  // Workaround for https://github.com/llvm/llvm-project/issues/100096
+  // ("[BOLT] GOT array pointer incorrectly rewritten"). In aarch64
+  // static glibc binaries, the .init section's _init function pointer can
+  // alias with a data pointer for the end of an array. GOT rewriting
+  // currently can't detect this and updates the data pointer to the
+  // moved _init, causing a runtime crash. Skipping _init on the other
+  // hand should be harmless.
+  if (!BC.IsStaticExecutable)
+    return nullptr;
+  const BinaryData *BD = BC.getBinaryDataByName("_init");
+  if (!BD || BD->getSectionName() != ".init")
+    return nullptr;
+  LLVM_DEBUG(dbgs() << "BOLT-DEBUG: skip _init in for GOT workaround.\n");
+  return BC.getBinaryFunctionAtAddress(BD->getAddress());
+}
+
 void RewriteInstance::selectFunctionsToProcess() {
   // Extend the list of functions to process or skip from a file.
   auto populateFunctionNames = [](cl::opt<std::string> &FunctionNamesFile,
@@ -3047,6 +3064,9 @@ void RewriteInstance::selectFunctionsToProcess() {
     return true;
   };
 
+  if (BinaryFunction *Init = getInitFunctionIfStaticBinary(*BC))
+    Init->setIgnored();
+
   for (auto &BFI : BC->getBinaryFunctions()) {
     BinaryFunction &Function = BFI.second;
 

bolt/test/AArch64/check-init-not-moved.s

@@ -0,0 +1,43 @@
+# Regression test for https://github.com/llvm/llvm-project/issues/100096
+# static glibc binaries crash on startup because _init is moved and
+# shares its address with an array end pointer. The GOT rewriting can't
+# tell the two pointers apart and incorrectly updates the _array_end
+# address. Test checks that _init is not moved.
+
+# RUN: llvm-mc -filetype=obj -triple aarch64-unknown-unknown %s -o %t.o
+# RUN: %clang %cflags %t.o -o %t.exe -Wl,-q -static -Wl,--section-start=.data=0x1000 -Wl,--section-start=.init=0x1004
+# RUN: llvm-bolt %t.exe -o %t.bolt
+# RUN: llvm-nm %t.exe | FileCheck --check-prefix=CHECK-ORIGINAL %s
+# RUN: llvm-nm %t.bolt | FileCheck --check-prefix=CHECK-BOLTED %s
+
+.section .data
+.globl _array_end
+_array_start:
+    .word 0x0
+
+_array_end:
+.section .init,"ax",@progbits
+.globl _init
+
+# Check that bolt doesn't move _init.
+#
+# CHECK-ORIGINAL: 0000000000001004 T _init
+# CHECK-BOLTED:   0000000000001004 T _init
+_init:
+    ret
+
+.section .text,"ax",@progbits
+.globl _start
+
+# Check that bolt is moving some other functions.
+#
+# CHECK-ORIGINAL:   0000000000001008 T _start
+# CHECK-BOLTED-NOT: 0000000000001008 T _start
+_start:
+    bl _init
+    adrp x0, #:got:_array_end
+    ldr x0, [x0, #:gotpage_lo15:_array_end]
+    adrp x0, #:got:_init
+    ldr x0, [x0, #:gotpage_lo15:_init]
+    ret
+