[Remarks] Add support for linking remarks

Remarks are usually emitted per-TU, and for generating a standalone
remark file that can be shipped with the linked binary we need some kind
of tool to merge everything together.

The remarks::RemarkLinker class takes care of this and:

* Deduplicates remarks
* Filters remarks with no debug location
* Merges string tables from all the entries

As an output, it provides an iterator range that can be used to
serialize the remarks to a file.

Differential Revision: https://reviews.llvm.org/D69141

include/llvm/Remarks/Remark.h

@@ -110,6 +110,21 @@ private:
 DEFINE_SIMPLE_CONVERSION_FUNCTIONS(Remark, LLVMRemarkEntryRef)
 
 /// Comparison operators for Remark objects and dependent objects.
+
+template <typename T>
+bool operator<(const Optional<T> &LHS, const Optional<T> &RHS) {
+  // Sorting based on optionals should result in all `None` entries to appear
+  // before the valid entries. For example, remarks with no debug location will
+  // appear first.
+  if (!LHS && !RHS)
+    return false;
+  if (!LHS && RHS)
+    return true;
+  if (LHS && !RHS)
+    return false;
+  return *LHS < *RHS;
+}
+
 inline bool operator==(const RemarkLocation &LHS, const RemarkLocation &RHS) {
   return LHS.SourceFilePath == RHS.SourceFilePath &&
          LHS.SourceLine == RHS.SourceLine &&
@@ -120,6 +135,11 @@ inline bool operator!=(const RemarkLocation &LHS, const RemarkLocation &RHS) {
   return !(LHS == RHS);
 }
 
+inline bool operator<(const RemarkLocation &LHS, const RemarkLocation &RHS) {
+  return std::make_tuple(LHS.SourceFilePath, LHS.SourceLine, LHS.SourceColumn) <
+         std::make_tuple(RHS.SourceFilePath, RHS.SourceLine, RHS.SourceColumn);
+}
+
 inline bool operator==(const Argument &LHS, const Argument &RHS) {
   return LHS.Key == RHS.Key && LHS.Val == RHS.Val && LHS.Loc == RHS.Loc;
 }
@@ -128,6 +148,11 @@ inline bool operator!=(const Argument &LHS, const Argument &RHS) {
   return !(LHS == RHS);
 }
 
+inline bool operator<(const Argument &LHS, const Argument &RHS) {
+  return std::make_tuple(LHS.Key, LHS.Val, LHS.Loc) <
+         std::make_tuple(RHS.Key, RHS.Val, RHS.Loc);
+}
+
 inline bool operator==(const Remark &LHS, const Remark &RHS) {
   return LHS.RemarkType == RHS.RemarkType && LHS.PassName == RHS.PassName &&
          LHS.RemarkName == RHS.RemarkName &&
@@ -139,6 +164,13 @@ inline bool operator!=(const Remark &LHS, const Remark &RHS) {
   return !(LHS == RHS);
 }
 
+inline bool operator<(const Remark &LHS, const Remark &RHS) {
+  return std::make_tuple(LHS.RemarkType, LHS.PassName, LHS.RemarkName,
+                         LHS.FunctionName, LHS.Loc, LHS.Hotness, LHS.Args) <
+         std::make_tuple(RHS.RemarkType, RHS.PassName, RHS.RemarkName,
+                         RHS.FunctionName, RHS.Loc, RHS.Hotness, RHS.Args);
+}
+
 } // end namespace remarks
 } // end namespace llvm
 

include/llvm/Remarks/RemarkFormat.h

@@ -27,6 +27,9 @@ enum class Format { Unknown, YAML, YAMLStrTab, Bitstream };
 /// Parse and validate a string for the remark format.
 Expected<Format> parseFormat(StringRef FormatStr);
 
+/// Parse and validate a magic number to a remark format.
+Expected<Format> magicToFormat(StringRef Magic);
+
 } // end namespace remarks
 } // end namespace llvm
 

include/llvm/Remarks/RemarkLinker.h

@@ -0,0 +1,100 @@
+//===-- llvm/Remarks/RemarkLinker.h -----------------------------*- C++/-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+// This file provides an interface to link together multiple remark files.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_REMARKS_REMARK_LINKER_H
+#define LLVM_REMARKS_REMARK_LINKER_H
+
+#include "llvm/Object/ObjectFile.h"
+#include "llvm/Remarks/Remark.h"
+#include "llvm/Remarks/RemarkFormat.h"
+#include "llvm/Remarks/RemarkStringTable.h"
+#include "llvm/Support/Error.h"
+#include <memory>
+#include <set>
+
+namespace llvm {
+namespace remarks {
+
+struct RemarkLinker {
+private:
+  /// Compare through the pointers.
+  struct RemarkPtrCompare {
+    bool operator()(const std::unique_ptr<Remark> &LHS,
+                    const std::unique_ptr<Remark> &RHS) const {
+      assert(LHS && RHS && "Invalid pointers to compare.");
+      return *LHS < *RHS;
+    };
+  };
+
+  /// The main string table for the remarks.
+  /// Note: all remarks should use the strings from this string table to avoid
+  /// dangling references.
+  StringTable StrTab;
+
+  /// A set holding unique remarks.
+  /// FIXME: std::set is probably not the most appropriate data structure here.
+  /// Due to the limitation of having a move-only key, there isn't another
+  /// obvious choice for now.
+  std::set<std::unique_ptr<Remark>, RemarkPtrCompare> Remarks;
+
+  /// A path to append before the external file path found in remark metadata.
+  Optional<std::string> PrependPath;
+
+  /// Keep this remark. If it's already in the set, discard it.
+  Remark &keep(std::unique_ptr<Remark> Remark);
+
+public:
+  /// Set a path to prepend to the external file path.
+  void setExternalFilePrependPath(StringRef PrependPath);
+
+  /// Link the remarks found in \p Buffer.
+  /// If \p RemarkFormat is not provided, try to deduce it from the metadata in
+  /// \p Buffer.
+  /// \p Buffer can be either a standalone remark container or just
+  /// metadata. This takes care of uniquing and merging the remarks.
+  Error link(StringRef Buffer, Optional<Format> RemarkFormat = None);
+
+  /// Link the remarks found in \p Obj by looking for the right section and
+  /// calling the method above.
+  Error link(const object::ObjectFile &Obj,
+             Optional<Format> RemarkFormat = None);
+
+  /// Serialize the linked remarks to the stream \p OS, using the format \p
+  /// RemarkFormat.
+  /// This clears internal state such as the string table.
+  /// Note: this implies that the serialization mode is standalone.
+  Error serialize(raw_ostream &OS, Format RemarksFormat) const;
+
+  /// Check whether there are any remarks linked.
+  bool empty() const { return Remarks.empty(); }
+
+  /// Return a collection of the linked unique remarks to iterate on.
+  /// Ex:
+  /// for (const Remark &R : RL.remarks() { [...] }
+  using iterator =
+      pointee_iterator<std::set<std::unique_ptr<Remark>>::iterator>;
+
+  iterator_range<iterator> remarks() const {
+    return {Remarks.begin(), Remarks.end()};
+  }
+};
+
+/// Returns a buffer with the contents of the remarks section depending on the
+/// format of the file. If the section doesn't exist, this returns an empty
+/// optional.
+Expected<Optional<StringRef>>
+getRemarksSectionContents(const object::ObjectFile &Obj);
+
+} // end namespace remarks
+} // end namespace llvm
+
+#endif /* LLVM_REMARKS_REMARK_LINKER_H */

lib/Remarks/CMakeLists.txt

@@ -3,6 +3,7 @@ add_llvm_library(LLVMRemarks
   BitstreamRemarkSerializer.cpp
   Remark.cpp
   RemarkFormat.cpp
+  RemarkLinker.cpp
   RemarkParser.cpp
   RemarkSerializer.cpp
   RemarkStringTable.cpp

lib/Remarks/RemarkFormat.cpp

@@ -12,6 +12,7 @@
 
 #include "llvm/Remarks/RemarkFormat.h"
 #include "llvm/ADT/StringSwitch.h"
+#include "llvm/Remarks/BitstreamRemarkContainer.h"
 
 using namespace llvm;
 using namespace llvm::remarks;
@@ -30,3 +31,17 @@ Expected<Format> llvm::remarks::parseFormat(StringRef FormatStr) {
 
   return Result;
 }
+
+Expected<Format> llvm::remarks::magicToFormat(StringRef Magic) {
+  auto Result =
+      StringSwitch<Format>(Magic)
+          .StartsWith("--- ", Format::YAML) // This is only an assumption.
+          .StartsWith(remarks::Magic, Format::YAMLStrTab)
+          .StartsWith(remarks::ContainerMagic, Format::Bitstream)
+          .Default(Format::Unknown);
+
+  if (Result == Format::Unknown)
+    return createStringError(std::make_error_code(std::errc::invalid_argument),
+                             "Unknown remark magic: '%s'", Magic.data());
+  return Result;
+}

lib/Remarks/RemarkLinker.cpp

@@ -0,0 +1,126 @@
+//===- RemarkLinker.cpp ---------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+// This file provides an implementation of the remark linker.
+//
+//===----------------------------------------------------------------------===//
+
+#include "llvm/Remarks/RemarkLinker.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Remarks/BitstreamRemarkContainer.h"
+#include "llvm/Remarks/RemarkParser.h"
+#include "llvm/Remarks/RemarkSerializer.h"
+#include "llvm/Support/Error.h"
+
+using namespace llvm;
+using namespace llvm::remarks;
+
+static Expected<StringRef>
+getRemarksSectionName(const object::ObjectFile &Obj) {
+  if (Obj.isMachO())
+    return StringRef("__remarks");
+  // ELF -> .remarks, but there is no ELF support at this point.
+  return createStringError(std::errc::illegal_byte_sequence,
+                           "Unsupported file format.");
+}
+
+Expected<Optional<StringRef>>
+llvm::remarks::getRemarksSectionContents(const object::ObjectFile &Obj) {
+  Expected<StringRef> SectionName = getRemarksSectionName(Obj);
+  if (!SectionName)
+    return SectionName.takeError();
+
+  for (const object::SectionRef &Section : Obj.sections()) {
+    Expected<StringRef> MaybeName = Section.getName();
+    if (!MaybeName)
+      return MaybeName.takeError();
+    if (*MaybeName != *SectionName)
+      continue;
+
+    if (Expected<StringRef> Contents = Section.getContents())
+      return *Contents;
+    else
+      return Contents.takeError();
+  }
+  return Optional<StringRef>{};
+}
+
+Remark &RemarkLinker::keep(std::unique_ptr<Remark> Remark) {
+  StrTab.internalize(*Remark);
+  auto Inserted = Remarks.insert(std::move(Remark));
+  return **Inserted.first;
+}
+
+void RemarkLinker::setExternalFilePrependPath(StringRef PrependPathIn) {
+  PrependPath = PrependPathIn;
+}
+
+// Discard remarks with no source location.
+static bool shouldKeepRemark(const Remark &R) { return R.Loc.hasValue(); }
+
+Error RemarkLinker::link(StringRef Buffer, Optional<Format> RemarkFormat) {
+  if (!RemarkFormat) {
+    Expected<Format> ParserFormat = magicToFormat(Buffer);
+    if (!ParserFormat)
+      return ParserFormat.takeError();
+    RemarkFormat = *ParserFormat;
+  }
+
+  Expected<std::unique_ptr<RemarkParser>> MaybeParser =
+      createRemarkParserFromMeta(
+          *RemarkFormat, Buffer, /*StrTab=*/None,
+          PrependPath ? Optional<StringRef>(StringRef(*PrependPath))
+                      : Optional<StringRef>(None));
+  if (!MaybeParser)
+    return MaybeParser.takeError();
+
+  RemarkParser &Parser = **MaybeParser;
+
+  while (true) {
+    Expected<std::unique_ptr<Remark>> Next = Parser.next();
+    if (Error E = Next.takeError()) {
+      if (E.isA<EndOfFileError>()) {
+        consumeError(std::move(E));
+        break;
+      }
+      return E;
+    }
+
+    assert(*Next != nullptr);
+
+    if (shouldKeepRemark(**Next))
+      keep(std::move(*Next));
+  }
+  return Error::success();
+}
+
+Error RemarkLinker::link(const object::ObjectFile &Obj,
+                         Optional<Format> RemarkFormat) {
+  Expected<Optional<StringRef>> SectionOrErr = getRemarksSectionContents(Obj);
+  if (!SectionOrErr)
+    return SectionOrErr.takeError();
+
+  if (Optional<StringRef> Section = *SectionOrErr)
+    return link(*Section, RemarkFormat);
+  return Error::success();
+}
+
+Error RemarkLinker::serialize(raw_ostream &OS, Format RemarksFormat) const {
+  Expected<std::unique_ptr<RemarkSerializer>> MaybeSerializer =
+      createRemarkSerializer(RemarksFormat, SerializerMode::Standalone, OS,
+                             std::move(const_cast<StringTable &>(StrTab)));
+  if (!MaybeSerializer)
+    return MaybeSerializer.takeError();
+
+  std::unique_ptr<remarks::RemarkSerializer> Serializer =
+      std::move(*MaybeSerializer);
+
+  for (const Remark &R : remarks())
+    Serializer->emit(R);
+  return Error::success();
+}

unittests/Remarks/CMakeLists.txt

@@ -9,6 +9,7 @@ add_llvm_unittest(RemarksTests
   BitstreamRemarksParsingTest.cpp
   BitstreamRemarksSerializerTest.cpp
   RemarksAPITest.cpp
+  RemarksLinkingTest.cpp
   RemarksStrTabParsingTest.cpp
   YAMLRemarksParsingTest.cpp
   YAMLRemarksSerializerTest.cpp

unittests/Remarks/RemarksLinkingTest.cpp

@@ -0,0 +1,217 @@
+//===- unittest/Support/RemarksLinkingTest.cpp - Linking tests ------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "llvm/Bitcode/BitcodeAnalyzer.h"
+#include "llvm/Remarks/RemarkLinker.h"
+#include "llvm/Remarks/RemarkSerializer.h"
+#include "llvm/Support/raw_ostream.h"
+#include "gtest/gtest.h"
+#include <string>
+
+using namespace llvm;
+
+static void serializeAndCheck(remarks::RemarkLinker &RL,
+                              remarks::Format OutputFormat,
+                              StringRef ExpectedOutput) {
+  // 1. Create a serializer.
+  // 2. Serialize all the remarks from the linker.
+  // 3. Check that it matches the output.
+  std::string Buf;
+  raw_string_ostream OS(Buf);
+  Error E = RL.serialize(OS, OutputFormat);
+  EXPECT_FALSE(static_cast<bool>(E));
+
+  // For bitstream, run it through the analyzer.
+  if (OutputFormat == remarks::Format::Bitstream) {
+    std::string AnalyzeBuf;
+    raw_string_ostream AnalyzeOS(AnalyzeBuf);
+    BCDumpOptions O(AnalyzeOS);
+    O.ShowBinaryBlobs = true;
+    BitcodeAnalyzer BA(OS.str());
+    EXPECT_FALSE(BA.analyze(O)); // Expect no errors.
+    EXPECT_EQ(AnalyzeOS.str(), ExpectedOutput);
+  } else {
+    EXPECT_EQ(OS.str(), ExpectedOutput);
+  }
+}
+
+static void check(remarks::Format InputFormat, StringRef Input,
+                  remarks::Format OutputFormat, StringRef ExpectedOutput) {
+  remarks::RemarkLinker RL;
+  EXPECT_FALSE(RL.link(Input, InputFormat));
+  serializeAndCheck(RL, OutputFormat, ExpectedOutput);
+}
+
+static void check(remarks::Format InputFormat, StringRef Input,
+                  remarks::Format InputFormat2, StringRef Input2,
+                  remarks::Format OutputFormat, StringRef ExpectedOutput) {
+  remarks::RemarkLinker RL;
+  EXPECT_FALSE(RL.link(Input, InputFormat));
+  EXPECT_FALSE(RL.link(Input2, InputFormat2));
+  serializeAndCheck(RL, OutputFormat, ExpectedOutput);
+}
+
+TEST(Remarks, LinkingGoodYAML) {
+  // One YAML remark.
+  check(remarks::Format::YAML,
+        "--- !Missed\n"
+        "Pass:            inline\n"
+        "Name:            NoDefinition\n"
+        "DebugLoc:        { File: file.c, Line: 3, Column: 12 }\n"
+        "Function:        foo\n"
+        "...\n",
+        remarks::Format::YAML,
+        "--- !Missed\n"
+        "Pass:            inline\n"
+        "Name:            NoDefinition\n"
+        "DebugLoc:        { File: file.c, Line: 3, Column: 12 }\n"
+        "Function:        foo\n"
+        "...\n");
+
+  // Check that we don't keep remarks without debug locations.
+  check(remarks::Format::YAML,
+        "--- !Missed\n"
+        "Pass:            inline\n"
+        "Name:            NoDefinition\n"
+        "Function:        foo\n"
+        "...\n",
+        remarks::Format::YAML, "");
+
+  // Check that we deduplicate remarks.
+  check(remarks::Format::YAML,
+        "--- !Missed\n"
+        "Pass:            inline\n"
+        "Name:            NoDefinition\n"
+        "DebugLoc:        { File: file.c, Line: 3, Column: 12 }\n"
+        "Function:        foo\n"
+        "...\n"
+        "--- !Missed\n"
+        "Pass:            inline\n"
+        "Name:            NoDefinition\n"
+        "DebugLoc:        { File: file.c, Line: 3, Column: 12 }\n"
+        "Function:        foo\n"
+        "...\n",
+        remarks::Format::YAML,
+        "--- !Missed\n"
+        "Pass:            inline\n"
+        "Name:            NoDefinition\n"
+        "DebugLoc:        { File: file.c, Line: 3, Column: 12 }\n"
+        "Function:        foo\n"
+        "...\n");
+}
+
+TEST(Remarks, LinkingGoodBitstream) {
+  // One YAML remark.
+  check(remarks::Format::YAML,
+        "--- !Missed\n"
+        "Pass:            inline\n"
+        "Name:            NoDefinition\n"
+        "DebugLoc:        { File: file.c, Line: 3, Column: 12 }\n"
+        "Function:        foo\n"
+        "...\n",
+        remarks::Format::Bitstream,
+        "<BLOCKINFO_BLOCK/>\n"
+        "<Meta BlockID=8 NumWords=12 BlockCodeSize=3>\n"
+        "  <Container info codeid=1 abbrevid=4 op0=0 op1=2/>\n"
+        "  <Remark version codeid=2 abbrevid=5 op0=0/>\n"
+        "  <String table codeid=3 abbrevid=6/> blob data = "
+        "'inline\\x00NoDefinition\\x00foo\\x00file.c\\x00'\n"
+        "</Meta>\n"
+        "<Remark BlockID=9 NumWords=4 BlockCodeSize=4>\n"
+        "  <Remark header codeid=5 abbrevid=4 op0=2 op1=1 op2=0 op3=2/>\n"
+        "  <Remark debug location codeid=6 abbrevid=5 op0=3 op1=3 op2=12/>\n"
+        "</Remark>\n");
+
+  // Check that we deduplicate remarks.
+  check(remarks::Format::YAML,
+        "--- !Missed\n"
+        "Pass:            inline\n"
+        "Name:            NoDefinition\n"
+        "DebugLoc:        { File: file.c, Line: 3, Column: 12 }\n"
+        "Function:        foo\n"
+        "...\n"
+        "--- !Missed\n"
+        "Pass:            inline\n"
+        "Name:            NoDefinition\n"
+        "DebugLoc:        { File: file.c, Line: 3, Column: 12 }\n"
+        "Function:        foo\n"
+        "...\n",
+        remarks::Format::Bitstream,
+        "<BLOCKINFO_BLOCK/>\n"
+        "<Meta BlockID=8 NumWords=12 BlockCodeSize=3>\n"
+        "  <Container info codeid=1 abbrevid=4 op0=0 op1=2/>\n"
+        "  <Remark version codeid=2 abbrevid=5 op0=0/>\n"
+        "  <String table codeid=3 abbrevid=6/> blob data = "
+        "'inline\\x00NoDefinition\\x00foo\\x00file.c\\x00'\n"
+        "</Meta>\n"
+        "<Remark BlockID=9 NumWords=4 BlockCodeSize=4>\n"
+        "  <Remark header codeid=5 abbrevid=4 op0=2 op1=1 op2=0 op3=2/>\n"
+        "  <Remark debug location codeid=6 abbrevid=5 op0=3 op1=3 op2=12/>\n"
+        "</Remark>\n");
+}
+
+TEST(Remarks, LinkingGoodStrTab) {
+  // Check that remarks from different entries use the same strtab.
+  check(remarks::Format::YAML,
+        "--- !Missed\n"
+        "Pass:            inline\n"
+        "Name:            NoDefinition\n"
+        "DebugLoc:        { File: file.c, Line: 3, Column: 12 }\n"
+        "Function:        foo\n"
+        "...\n",
+        remarks::Format::YAML,
+        "--- !Passed\n"
+        "Pass:            inline\n"
+        "Name:            Ok\n"
+        "DebugLoc:        { File: file.c, Line: 3, Column: 12 }\n"
+        "Function:        foo\n"
+        "...\n",
+        remarks::Format::YAMLStrTab,
+        StringRef("REMARKS\0\0\0\0\0\0\0\0\0\x22\0\0\0\0\0\0\0"
+                  "inline\0NoDefinition\0foo\0file.c\0Ok\0"
+                  "--- !Passed\n"
+                  "Pass:            0\n"
+                  "Name:            4\n"
+                  "DebugLoc:        { File: 3, Line: 3, Column: 12 }\n"
+                  "Function:        2\n"
+                  "...\n"
+                  "--- !Missed\n"
+                  "Pass:            0\n"
+                  "Name:            1\n"
+                  "DebugLoc:        { File: 3, Line: 3, Column: 12 }\n"
+                  "Function:        2\n"
+                  "...\n",
+                  304));
+}
+
+// Check that we propagate parsing errors.
+TEST(Remarks, LinkingError) {
+  remarks::RemarkLinker RL;
+  {
+    Error E = RL.link("badyaml", remarks::Format::YAML);
+    EXPECT_TRUE(static_cast<bool>(E));
+    EXPECT_EQ(toString(std::move(E)),
+              "YAML:1:1: error: document root is not of mapping type.\n"
+              "\n"
+              "badyaml\n"
+              "^~~~~~~\n"
+              "\n");
+  }
+
+  {
+    // Check that the prepend path is propagated and fails with the full path.
+    RL.setExternalFilePrependPath("/baddir/");
+    Error E = RL.link(
+        StringRef("REMARKS\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0badfile.opt.yaml",
+                  40),
+        remarks::Format::YAMLStrTab);
+    EXPECT_TRUE(static_cast<bool>(E));
+    EXPECT_EQ(toString(std::move(E)),
+              "'/baddir/badfile.opt.yaml': No such file or directory");
+  }
+}