Merge branch 'users/boomanaiden154/refactor-buildkite-shell-scripts' into users/boomanaiden154/github-actions-linux-pipeline

Author: boomanaiden154

.ci/generate_test_report.py

@@ -5,6 +5,7 @@
 # python3 -m unittest discover -p generate_test_report.py
 
 import argparse
+import os
 import subprocess
 import unittest
 from io import StringIO
@@ -267,6 +268,46 @@ def test_report_dont_list_failures(self):
             ),
         )
 
+    def test_report_dont_list_failures_link_to_log(self):
+        self.assertEqual(
+            _generate_report(
+                "Foo",
+                [
+                    junit_from_xml(
+                        dedent(
+                            """\
+          <?xml version="1.0" encoding="UTF-8"?>
+          <testsuites time="0.02">
+          <testsuite name="Bar" tests="1" failures="1" skipped="0" time="0.02">
+          <testcase classname="Bar/test_1" name="test_1" time="0.02">
+            <failure><![CDATA[Output goes here]]></failure>
+          </testcase>
+          </testsuite>
+          </testsuites>"""
+                        )
+                    )
+                ],
+                list_failures=False,
+                buildkite_info={
+                    "BUILDKITE_ORGANIZATION_SLUG": "organization_slug",
+                    "BUILDKITE_PIPELINE_SLUG": "pipeline_slug",
+                    "BUILDKITE_BUILD_NUMBER": "build_number",
+                    "BUILDKITE_JOB_ID": "job_id",
+                },
+            ),
+            (
+                dedent(
+                    """\
+          # Foo
+
+          * 1 test failed
+
+          Failed tests and their output was too large to report. [Download](https://buildkite.com/organizations/organization_slug/pipelines/pipeline_slug/builds/build_number/jobs/job_id/download.txt) the build's log file to see the details."""
+                ),
+                "error",
+            ),
+        )
+
     def test_report_size_limit(self):
         self.assertEqual(
             _generate_report(
@@ -308,7 +349,13 @@ def test_report_size_limit(self):
 # listed. This minimal report will always fit into an annotation.
 # If include failures is False, total number of test will be reported but their names
 # and output will not be.
-def _generate_report(title, junit_objects, size_limit=1024 * 1024, list_failures=True):
+def _generate_report(
+    title,
+    junit_objects,
+    size_limit=1024 * 1024,
+    list_failures=True,
+    buildkite_info=None,
+):
     if not junit_objects:
         return ("", "success")
 
@@ -354,11 +401,21 @@ def plural(num_tests):
         report.append(f"* {tests_failed} {plural(tests_failed)} failed")
 
     if not list_failures:
+        if buildkite_info is not None:
+            log_url = (
+                "https://buildkite.com/organizations/{BUILDKITE_ORGANIZATION_SLUG}/"
+                "pipelines/{BUILDKITE_PIPELINE_SLUG}/builds/{BUILDKITE_BUILD_NUMBER}/"
+                "jobs/{BUILDKITE_JOB_ID}/download.txt".format(**buildkite_info)
+            )
+            download_text = f"[Download]({log_url})"
+        else:
+            download_text = "Download"
+
         report.extend(
             [
                 "",
                 "Failed tests and their output was too large to report. "
-                "Download the build's log file to see the details.",
+                f"{download_text} the build's log file to see the details.",
             ]
         )
     elif failures:
@@ -381,13 +438,23 @@ def plural(num_tests):
 
     report = "\n".join(report)
     if len(report.encode("utf-8")) > size_limit:
-        return _generate_report(title, junit_objects, size_limit, list_failures=False)
+        return _generate_report(
+            title,
+            junit_objects,
+            size_limit,
+            list_failures=False,
+            buildkite_info=buildkite_info,
+        )
 
     return report, style
 
 
-def generate_report(title, junit_files):
-    return _generate_report(title, [JUnitXml.fromfile(p) for p in junit_files])
+def generate_report(title, junit_files, buildkite_info):
+    return _generate_report(
+        title,
+        [JUnitXml.fromfile(p) for p in junit_files],
+        buildkite_info=buildkite_info,
+    )
 
 
 if __name__ == "__main__":
@@ -399,7 +466,18 @@ def generate_report(title, junit_files):
     parser.add_argument("junit_files", help="Paths to JUnit report files.", nargs="*")
     args = parser.parse_args()
 
-    report, style = generate_report(args.title, args.junit_files)
+    # All of these are required to build a link to download the log file.
+    env_var_names = [
+        "BUILDKITE_ORGANIZATION_SLUG",
+        "BUILDKITE_PIPELINE_SLUG",
+        "BUILDKITE_BUILD_NUMBER",
+        "BUILDKITE_JOB_ID",
+    ]
+    buildkite_info = {k: v for k, v in os.environ.items() if k in env_var_names}
+    if len(buildkite_info) != len(env_var_names):
+        buildkite_info = None
+
+    report, style = generate_report(args.title, args.junit_files, buildkite_info)
 
     if report:
         p = subprocess.Popen(

bolt/docs/BinaryAnalysis.md

@@ -0,0 +1,20 @@
+# BOLT-based binary analysis
+
+As part of post-link-time optimizing, BOLT needs to perform a range of analyses
+on binaries such as recontructing control flow graphs, and more.
+
+The `llvm-bolt-binary-analysis` tool enables running requested binary analyses
+on binaries, and generating reports. It does this by building on top of the
+analyses implemented in the BOLT libraries.
+
+## Which binary analyses are implemented?
+
+At the moment, no binary analyses are implemented.
+
+The goal is to make it easy using a plug-in framework to add your own analyses.
+
+## How to add your own binary analysis
+
+_TODO: this section needs to be written. Ideally, we should have a simple
+"example" or "template" analysis that can be the starting point for implementing
+custom analyses_

bolt/include/bolt/Core/DIEBuilder.h

@@ -162,7 +162,7 @@ class DIEBuilder {
 
   /// Clone an attribute in reference format.
   void cloneDieOffsetReferenceAttribute(
-      DIE &Die, const DWARFUnit &U, const DWARFDie &InputDIE,
+      DIE &Die, DWARFUnit &U, const DWARFDie &InputDIE,
       const DWARFAbbreviationDeclaration::AttributeSpec AttrSpec, uint64_t Ref);
 
   /// Clone an attribute in block format.

bolt/include/bolt/Core/DebugNames.h

@@ -72,8 +72,8 @@ class DWARF5AcceleratorTable {
     return std::move(FullTableBuffer);
   }
   /// Adds a DIE that is referenced across CUs.
-  void addCrossCUDie(const DIE *Die) {
-    CrossCUDies.insert({Die->getOffset(), Die});
+  void addCrossCUDie(DWARFUnit *Unit, const DIE *Die) {
+    CrossCUDies.insert({Die->getOffset(), {Unit, Die}});
   }
   /// Returns true if the DIE can generate an entry for a cross cu reference.
   /// This only checks TAGs of a DIE because when this is invoked DIE might not
@@ -145,7 +145,7 @@ class DWARF5AcceleratorTable {
   llvm::DenseMap<uint64_t, uint32_t> CUOffsetsToPatch;
   // Contains a map of Entry ID to Entry relative offset.
   llvm::DenseMap<uint64_t, uint32_t> EntryRelativeOffsets;
-  llvm::DenseMap<uint64_t, const DIE *> CrossCUDies;
+  llvm::DenseMap<uint64_t, std::pair<DWARFUnit *, const DIE *>> CrossCUDies;
   /// Adds Unit to either CUList, LocalTUList or ForeignTUList.
   /// Input Unit being processed, and DWO ID if Unit is being processed comes
   /// from a DWO section.
@@ -191,6 +191,29 @@ class DWARF5AcceleratorTable {
   void emitData();
   /// Emit augmentation string.
   void emitAugmentationString() const;
+  /// Creates a new entry for a given DIE.
+  std::optional<BOLTDWARF5AccelTableData *>
+  addEntry(DWARFUnit &DU, const DIE &CurrDie,
+           const std::optional<uint64_t> &DWOID,
+           const std::optional<BOLTDWARF5AccelTableData *> &Parent,
+           const std::optional<std::string> &Name,
+           const uint32_t NumberParentsInChain);
+  /// Returns UnitID for a given DWARFUnit.
+  uint32_t getUnitID(const DWARFUnit &Unit,
+                     const std::optional<uint64_t> &DWOID, bool &IsTU);
+  std::optional<std::string> getName(DWARFUnit &DU,
+                                     const std::optional<uint64_t> &DWOID,
+                                     const std::string &NameToUse,
+                                     DIEValue ValName);
+  /// Processes a DIE with references to other DIEs for DW_AT_name and
+  /// DW_AT_linkage_name resolution.
+  /// If DW_AT_name exists method creates a new entry for this DIE and returns
+  /// it.
+  std::optional<BOLTDWARF5AccelTableData *> processReferencedDie(
+      DWARFUnit &Unit, const DIE &Die, const std::optional<uint64_t> &DWOID,
+      const std::optional<BOLTDWARF5AccelTableData *> &Parent,
+      const std::string &NameToUse, const uint32_t NumberParentsInChain,
+      const dwarf::Attribute &Attr);
 };
 } // namespace bolt
 } // namespace llvm

bolt/include/bolt/Rewrite/RewriteInstance.h

@@ -164,6 +164,9 @@ class RewriteInstance {
 
   void preregisterSections();
 
+  /// run analyses requested in binary analysis mode.
+  void runBinaryAnalyses();
+
   /// Run optimizations that operate at the binary, or post-linker, level.
   void runOptimizationPasses();
 

bolt/include/bolt/Utils/CommandLineOpts.h

@@ -18,6 +18,7 @@
 namespace opts {
 
 extern bool HeatmapMode;
+extern bool BinaryAnalysisMode;
 
 extern llvm::cl::OptionCategory BoltCategory;
 extern llvm::cl::OptionCategory BoltDiffCategory;
@@ -27,6 +28,7 @@ extern llvm::cl::OptionCategory BoltOutputCategory;
 extern llvm::cl::OptionCategory AggregatorCategory;
 extern llvm::cl::OptionCategory BoltInstrCategory;
 extern llvm::cl::OptionCategory HeatmapCategory;
+extern llvm::cl::OptionCategory BinaryAnalysisCategory;
 
 extern llvm::cl::opt<unsigned> AlignText;
 extern llvm::cl::opt<unsigned> AlignFunctions;

bolt/lib/Core/BinaryEmitter.cpp

@@ -46,13 +46,17 @@ BreakFunctionNames("break-funcs",
   cl::Hidden,
   cl::cat(BoltCategory));
 
-static cl::list<std::string>
-FunctionPadSpec("pad-funcs",
-  cl::CommaSeparated,
-  cl::desc("list of functions to pad with amount of bytes"),
-  cl::value_desc("func1:pad1,func2:pad2,func3:pad3,..."),
-  cl::Hidden,
-  cl::cat(BoltCategory));
+cl::list<std::string>
+    FunctionPadSpec("pad-funcs", cl::CommaSeparated,
+                    cl::desc("list of functions to pad with amount of bytes"),
+                    cl::value_desc("func1:pad1,func2:pad2,func3:pad3,..."),
+                    cl::Hidden, cl::cat(BoltCategory));
+
+cl::list<std::string> FunctionPadBeforeSpec(
+    "pad-funcs-before", cl::CommaSeparated,
+    cl::desc("list of functions to pad with amount of bytes"),
+    cl::value_desc("func1:pad1,func2:pad2,func3:pad3,..."), cl::Hidden,
+    cl::cat(BoltCategory));
 
 static cl::opt<bool> MarkFuncs(
     "mark-funcs",
@@ -70,11 +74,12 @@ X86AlignBranchBoundaryHotOnly("x86-align-branch-boundary-hot-only",
   cl::init(true),
   cl::cat(BoltOptCategory));
 
-size_t padFunction(const BinaryFunction &Function) {
+size_t padFunction(const cl::list<std::string> &Spec,
+                   const BinaryFunction &Function) {
   static std::map<std::string, size_t> FunctionPadding;
 
-  if (FunctionPadding.empty() && !FunctionPadSpec.empty()) {
-    for (std::string &Spec : FunctionPadSpec) {
+  if (FunctionPadding.empty() && !Spec.empty()) {
+    for (const std::string &Spec : Spec) {
       size_t N = Spec.find(':');
       if (N == std::string::npos)
         continue;
@@ -319,6 +324,32 @@ bool BinaryEmitter::emitFunction(BinaryFunction &Function,
     Streamer.emitCodeAlignment(Function.getAlign(), &*BC.STI);
   }
 
+  if (size_t Padding =
+          opts::padFunction(opts::FunctionPadBeforeSpec, Function)) {
+    // Handle padFuncsBefore after the above alignment logic but before
+    // symbol addresses are decided.
+    if (!BC.HasRelocations) {
+      BC.errs() << "BOLT-ERROR: -pad-before-funcs is not supported in "
+                << "non-relocation mode\n";
+      exit(1);
+    }
+
+    // Preserve Function.getMinAlign().
+    if (!isAligned(Function.getMinAlign(), Padding)) {
+      BC.errs() << "BOLT-ERROR: user-requested " << Padding
+                << " padding bytes before function " << Function
+                << " is not a multiple of the minimum function alignment ("
+                << Function.getMinAlign().value() << ").\n";
+      exit(1);
+    }
+
+    LLVM_DEBUG(dbgs() << "BOLT-DEBUG: padding before function " << Function
+                      << " with " << Padding << " bytes\n");
+
+    // Since the padding is not executed, it can be null bytes.
+    Streamer.emitFill(Padding, 0);
+  }
+
   MCContext &Context = Streamer.getContext();
   const MCAsmInfo *MAI = Context.getAsmInfo();
 
@@ -373,7 +404,7 @@ bool BinaryEmitter::emitFunction(BinaryFunction &Function,
   emitFunctionBody(Function, FF, /*EmitCodeOnly=*/false);
 
   // Emit padding if requested.
-  if (size_t Padding = opts::padFunction(Function)) {
+  if (size_t Padding = opts::padFunction(opts::FunctionPadSpec, Function)) {
     LLVM_DEBUG(dbgs() << "BOLT-DEBUG: padding function " << Function << " with "
                       << Padding << " bytes\n");
     Streamer.emitFill(Padding, MAI->getTextAlignFillValue());

bolt/lib/Core/DIEBuilder.cpp

@@ -622,7 +622,7 @@ DWARFDie DIEBuilder::resolveDIEReference(
 }
 
 void DIEBuilder::cloneDieOffsetReferenceAttribute(
-    DIE &Die, const DWARFUnit &U, const DWARFDie &InputDIE,
+    DIE &Die, DWARFUnit &U, const DWARFDie &InputDIE,
     const DWARFAbbreviationDeclaration::AttributeSpec AttrSpec, uint64_t Ref) {
   DIE *NewRefDie = nullptr;
   DWARFUnit *RefUnit = nullptr;
@@ -654,7 +654,7 @@ void DIEBuilder::cloneDieOffsetReferenceAttribute(
     // Adding referenced DIE to DebugNames to be used when entries are created
     // that contain cross cu references.
     if (DebugNamesTable.canGenerateEntryWithCrossCUReference(U, Die, AttrSpec))
-      DebugNamesTable.addCrossCUDie(DieInfo.Die);
+      DebugNamesTable.addCrossCUDie(&U, DieInfo.Die);
     // no matter forward reference or backward reference, we are supposed
     // to calculate them in `finish` due to the possible modification of
     // the DIE.