[llvm] r367155 - Reland: [Remarks] Support parsing remark metadata in the YAML remark parser

Francis Visoiu Mistrih via llvm-commits llvm-commits at lists.llvm.org
Fri Jul 26 14:02:02 PDT 2019


Author: thegameg
Date: Fri Jul 26 14:02:02 2019
New Revision: 367155

URL: http://llvm.org/viewvc/llvm-project?rev=367155&view=rev
Log:
Reland: [Remarks] Support parsing remark metadata in the YAML remark parser

This adds support to the yaml remark parser to be able to parse remarks
directly from the metadata.

This supports parsing separate metadata and following the external file
with the associated metadata, and also a standalone file containing
metadata + remarks all together.

Original llvm-svn: 367148
Revert llvm-svn: 367151

This has a fix for gcc builds.

Modified:
    llvm/trunk/docs/Remarks.rst
    llvm/trunk/include/llvm/Remarks/RemarkParser.h
    llvm/trunk/lib/Remarks/RemarkParser.cpp
    llvm/trunk/lib/Remarks/YAMLRemarkParser.cpp
    llvm/trunk/lib/Remarks/YAMLRemarkParser.h
    llvm/trunk/unittests/Remarks/YAMLRemarksParsingTest.cpp

Modified: llvm/trunk/docs/Remarks.rst
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/docs/Remarks.rst?rev=367155&r1=367154&r2=367155&view=diff
==============================================================================
--- llvm/trunk/docs/Remarks.rst (original)
+++ llvm/trunk/docs/Remarks.rst Fri Jul 26 14:02:02 2019
@@ -218,6 +218,28 @@ support this format.
 
 .. _optviewer:
 
+YAML metadata
+-------------
+
+The metadata used together with the YAML format is:
+
+* a magic number: "REMARKS\\0"
+* the version number: a little-endian uint64_t
+* the total size of the string table (the size itself excluded):
+  little-endian uint64_t
+* a list of null-terminated strings
+
+Optional:
+
+* the absolute file path to the serialized remark diagnostics: a
+  null-terminated string.
+
+When the metadata is serialized separately from the remarks, the file path
+should be present and point to the file where the remarks are serialized to.
+
+In case the metadata only acts as a header to the remarks, the file path can be
+omitted.
+
 opt-viewer
 ==========
 
@@ -295,15 +317,8 @@ Emitting remark diagnostics in the objec
 ==============================================
 
 A section containing metadata on remark diagnostics will be emitted when
--remarks-section is passed. The section contains:
-
-* a magic number: "REMARKS\\0"
-* the version number: a little-endian uint64_t
-* the total size of the string table (the size itself excluded):
-  little-endian uint64_t
-* a list of null-terminated strings
-* the absolute file path to the serialized remark diagnostics: a
-  null-terminated string.
+-remarks-section is passed. The section contains the metadata associated to the
+format used to serialize the remarks.
 
 The section is named:
 

Modified: llvm/trunk/include/llvm/Remarks/RemarkParser.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Remarks/RemarkParser.h?rev=367155&r1=367154&r2=367155&view=diff
==============================================================================
--- llvm/trunk/include/llvm/Remarks/RemarkParser.h (original)
+++ llvm/trunk/include/llvm/Remarks/RemarkParser.h Fri Jul 26 14:02:02 2019
@@ -80,6 +80,10 @@ Expected<std::unique_ptr<RemarkParser>>
 createRemarkParser(Format ParserFormat, StringRef Buf,
                    ParsedStringTable StrTab);
 
+Expected<std::unique_ptr<RemarkParser>>
+createRemarkParserFromMeta(Format ParserFormat, StringRef Buf,
+                           Optional<ParsedStringTable> StrTab = None);
+
 } // end namespace remarks
 } // end namespace llvm
 

Modified: llvm/trunk/lib/Remarks/RemarkParser.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Remarks/RemarkParser.cpp?rev=367155&r1=367154&r2=367155&view=diff
==============================================================================
--- llvm/trunk/lib/Remarks/RemarkParser.cpp (original)
+++ llvm/trunk/lib/Remarks/RemarkParser.cpp Fri Jul 26 14:02:02 2019
@@ -80,6 +80,21 @@ llvm::remarks::createRemarkParser(Format
   llvm_unreachable("unhandled ParseFormat");
 }
 
+Expected<std::unique_ptr<RemarkParser>>
+llvm::remarks::createRemarkParserFromMeta(Format ParserFormat, StringRef Buf,
+                                          Optional<ParsedStringTable> StrTab) {
+  switch (ParserFormat) {
+  // Depending on the metadata, the format can be either yaml or yaml-strtab,
+  // regardless of the input argument.
+  case Format::YAML:
+  case Format::YAMLStrTab:
+    return createYAMLParserFromMeta(Buf, std::move(StrTab));
+  case Format::Unknown:
+    return createStringError(std::make_error_code(std::errc::invalid_argument),
+                             "Unknown remark parser format.");
+  }
+}
+
 // Wrapper that holds the state needed to interact with the C API.
 struct CParser {
   std::unique_ptr<RemarkParser> TheParser;

Modified: llvm/trunk/lib/Remarks/YAMLRemarkParser.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Remarks/YAMLRemarkParser.cpp?rev=367155&r1=367154&r2=367155&view=diff
==============================================================================
--- llvm/trunk/lib/Remarks/YAMLRemarkParser.cpp (original)
+++ llvm/trunk/lib/Remarks/YAMLRemarkParser.cpp Fri Jul 26 14:02:02 2019
@@ -14,6 +14,7 @@
 #include "YAMLRemarkParser.h"
 #include "llvm/ADT/StringSwitch.h"
 #include "llvm/Remarks/RemarkParser.h"
+#include "llvm/Support/Endian.h"
 
 using namespace llvm;
 using namespace llvm::remarks;
@@ -54,6 +55,110 @@ static SourceMgr setupSM(std::string &La
   return SM;
 }
 
+// Parse the magic number. This function returns true if this represents remark
+// metadata, false otherwise.
+static Expected<bool> parseMagic(StringRef &Buf) {
+  if (!Buf.consume_front(remarks::Magic))
+    return false;
+
+  if (Buf.size() < 1 || !Buf.consume_front(StringRef("\0", 1)))
+    return createStringError(std::errc::illegal_byte_sequence,
+                             "Expecting \\0 after magic number.");
+  return true;
+}
+
+static Expected<uint64_t> parseVersion(StringRef &Buf) {
+  if (Buf.size() < sizeof(uint64_t))
+    return createStringError(std::errc::illegal_byte_sequence,
+                             "Expecting version number.");
+
+  uint64_t Version =
+      support::endian::read<uint64_t, support::little, support::unaligned>(
+          Buf.data());
+  if (Version != remarks::Version)
+    return createStringError(
+        std::errc::illegal_byte_sequence,
+        "Mismatching remark version. Got %u, expected %u.", Version,
+        remarks::Version);
+  Buf = Buf.drop_front(sizeof(uint64_t));
+  return Version;
+}
+
+static Expected<uint64_t> parseStrTabSize(StringRef &Buf) {
+  if (Buf.size() < sizeof(uint64_t))
+    return createStringError(std::errc::illegal_byte_sequence,
+                             "Expecting string table size.");
+  uint64_t StrTabSize =
+      support::endian::read<uint64_t, support::little, support::unaligned>(
+          Buf.data());
+  Buf = Buf.drop_front(sizeof(uint64_t));
+  return StrTabSize;
+}
+
+static Expected<ParsedStringTable> parseStrTab(StringRef &Buf,
+                                               uint64_t StrTabSize) {
+  if (Buf.size() < StrTabSize)
+    return createStringError(std::errc::illegal_byte_sequence,
+                             "Expecting string table.");
+
+  // Attach the string table to the parser.
+  ParsedStringTable Result(StringRef(Buf.data(), StrTabSize));
+  Buf = Buf.drop_front(StrTabSize);
+  return Expected<ParsedStringTable>(std::move(Result));
+}
+
+Expected<std::unique_ptr<YAMLRemarkParser>>
+remarks::createYAMLParserFromMeta(StringRef Buf,
+                                  Optional<ParsedStringTable> StrTab) {
+  // We now have a magic number. The metadata has to be correct.
+  Expected<bool> isMeta = parseMagic(Buf);
+  if (!isMeta)
+    return isMeta.takeError();
+  // If it's not recognized as metadata, roll back.
+  std::unique_ptr<MemoryBuffer> SeparateBuf;
+  if (*isMeta) {
+    Expected<uint64_t> Version = parseVersion(Buf);
+    if (!Version)
+      return Version.takeError();
+
+    Expected<uint64_t> StrTabSize = parseStrTabSize(Buf);
+    if (!StrTabSize)
+      return StrTabSize.takeError();
+
+    // If the size of string table is not 0, try to build one.
+    if (*StrTabSize != 0) {
+      if (StrTab)
+        return createStringError(std::errc::illegal_byte_sequence,
+                                 "String table already provided.");
+      Expected<ParsedStringTable> MaybeStrTab = parseStrTab(Buf, *StrTabSize);
+      if (!MaybeStrTab)
+        return MaybeStrTab.takeError();
+      StrTab = std::move(*MaybeStrTab);
+    }
+    // If it starts with "---", there is no external file.
+    if (!Buf.startswith("---")) {
+      // At this point, we expect Buf to contain the external file path.
+      // Try to open the file and start parsing from there.
+      ErrorOr<std::unique_ptr<MemoryBuffer>> BufferOrErr =
+          MemoryBuffer::getFile(Buf);
+      if (std::error_code EC = BufferOrErr.getError())
+        return errorCodeToError(EC);
+
+      // Keep the buffer alive.
+      SeparateBuf = std::move(*BufferOrErr);
+      Buf = SeparateBuf->getBuffer();
+    }
+  }
+
+  std::unique_ptr<YAMLRemarkParser> Result =
+      StrTab
+          ? llvm::make_unique<YAMLStrTabRemarkParser>(Buf, std::move(*StrTab))
+          : llvm::make_unique<YAMLRemarkParser>(Buf);
+  if (SeparateBuf)
+    Result->SeparateBuf = std::move(SeparateBuf);
+  return std::move(Result);
+}
+
 YAMLRemarkParser::YAMLRemarkParser(StringRef Buf)
     : YAMLRemarkParser(Buf, None) {}
 

Modified: llvm/trunk/lib/Remarks/YAMLRemarkParser.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Remarks/YAMLRemarkParser.h?rev=367155&r1=367154&r2=367155&view=diff
==============================================================================
--- llvm/trunk/lib/Remarks/YAMLRemarkParser.h (original)
+++ llvm/trunk/lib/Remarks/YAMLRemarkParser.h Fri Jul 26 14:02:02 2019
@@ -18,6 +18,7 @@
 #include "llvm/Remarks/Remark.h"
 #include "llvm/Remarks/RemarkParser.h"
 #include "llvm/Support/Error.h"
+#include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/SourceMgr.h"
 #include "llvm/Support/YAMLParser.h"
 #include "llvm/Support/YAMLTraits.h"
@@ -58,6 +59,9 @@ struct YAMLRemarkParser : public RemarkP
   yaml::Stream Stream;
   /// Iterator in the YAML stream.
   yaml::document_iterator YAMLIt;
+  /// If we parse remark metadata in separate mode, we need to open a new file
+  /// and parse that.
+  std::unique_ptr<MemoryBuffer> SeparateBuf;
 
   YAMLRemarkParser(StringRef Buf);
 
@@ -104,6 +108,11 @@ protected:
   /// Parse one value to a string.
   Expected<StringRef> parseStr(yaml::KeyValueNode &Node) override;
 };
+
+Expected<std::unique_ptr<YAMLRemarkParser>>
+createYAMLParserFromMeta(StringRef Buf,
+                         Optional<ParsedStringTable> StrTab = None);
+
 } // end namespace remarks
 } // end namespace llvm
 

Modified: llvm/trunk/unittests/Remarks/YAMLRemarksParsingTest.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/Remarks/YAMLRemarksParsingTest.cpp?rev=367155&r1=367154&r2=367155&view=diff
==============================================================================
--- llvm/trunk/unittests/Remarks/YAMLRemarksParsingTest.cpp (original)
+++ llvm/trunk/unittests/Remarks/YAMLRemarksParsingTest.cpp Fri Jul 26 14:02:02 2019
@@ -29,6 +29,22 @@ template <size_t N> void parseGood(const
   EXPECT_TRUE(errorToBool(std::move(E))); // Check for parsing errors.
 }
 
+void parseGoodMeta(StringRef Buf) {
+  Expected<std::unique_ptr<remarks::RemarkParser>> MaybeParser =
+      remarks::createRemarkParserFromMeta(remarks::Format::YAML, Buf);
+  EXPECT_FALSE(errorToBool(MaybeParser.takeError()));
+  EXPECT_TRUE(*MaybeParser != nullptr);
+
+  remarks::RemarkParser &Parser = **MaybeParser;
+  Expected<std::unique_ptr<remarks::Remark>> Remark = Parser.next();
+  EXPECT_FALSE(errorToBool(Remark.takeError())); // Check for parsing errors.
+  EXPECT_TRUE(*Remark != nullptr);               // At least one remark.
+  Remark = Parser.next();
+  Error E = Remark.takeError();
+  EXPECT_TRUE(E.isA<remarks::EndOfFileError>());
+  EXPECT_TRUE(errorToBool(std::move(E))); // Check for parsing errors.
+}
+
 template <size_t N>
 bool parseExpectError(const char (&Buf)[N], const char *Error) {
   Expected<std::unique_ptr<remarks::RemarkParser>> MaybeParser =
@@ -47,6 +63,17 @@ bool parseExpectError(const char (&Buf)[
   return StringRef(Stream.str()).contains(Error);
 }
 
+void parseExpectErrorMeta(StringRef Buf, const char *Error) {
+  std::string ErrorStr;
+  raw_string_ostream Stream(ErrorStr);
+
+  Expected<std::unique_ptr<remarks::RemarkParser>> MaybeParser =
+      remarks::createRemarkParserFromMeta(remarks::Format::YAML, Buf);
+  handleAllErrors(MaybeParser.takeError(),
+                  [&](const ErrorInfoBase &EIB) { EIB.log(Stream); });
+  EXPECT_EQ(Stream.str(), Error);
+}
+
 TEST(YAMLRemarks, ParsingEmpty) {
   EXPECT_TRUE(parseExpectError("\n\n", "document root is not of mapping type."));
 }
@@ -619,3 +646,62 @@ TEST(YAMLRemarks, ParsingBadStringTableI
       StringRef(Stream.str())
           .contains("String with index 50 is out of bounds (size = 1)."));
 }
+
+TEST(YAMLRemarks, ParsingGoodMeta) {
+  // No metadata should also work.
+  parseGoodMeta("--- !Missed\n"
+                "Pass: inline\n"
+                "Name: NoDefinition\n"
+                "Function: foo\n");
+
+  // No string table.
+  parseGoodMeta(StringRef("REMARKS\0"
+                          "\0\0\0\0\0\0\0\0"
+                          "\0\0\0\0\0\0\0\0"
+                          "--- !Missed\n"
+                          "Pass: inline\n"
+                          "Name: NoDefinition\n"
+                          "Function: foo\n",
+                          82));
+
+  // Use the string table from the metadata.
+  parseGoodMeta(StringRef("REMARKS\0"
+                          "\0\0\0\0\0\0\0\0"
+                          "\x02\0\0\0\0\0\0\0"
+                          "a\0"
+                          "--- !Missed\n"
+                          "Pass: 0\n"
+                          "Name: 0\n"
+                          "Function: 0\n",
+                          66));
+}
+
+TEST(YAMLRemarks, ParsingBadMeta) {
+  parseExpectErrorMeta(StringRef("REMARKSS", 9),
+                       "Expecting \\0 after magic number.");
+
+  parseExpectErrorMeta(StringRef("REMARKS\0", 8), "Expecting version number.");
+
+  parseExpectErrorMeta(StringRef("REMARKS\0"
+                                 "\x09\0\0\0\0\0\0\0",
+                                 16),
+                       "Mismatching remark version. Got 9, expected 0.");
+
+  parseExpectErrorMeta(StringRef("REMARKS\0"
+                                 "\0\0\0\0\0\0\0\0",
+                                 16),
+                       "Expecting string table size.");
+
+  parseExpectErrorMeta(StringRef("REMARKS\0"
+                                 "\0\0\0\0\0\0\0\0"
+                                 "\x01\0\0\0\0\0\0\0",
+                                 24),
+                       "Expecting string table.");
+
+  parseExpectErrorMeta(StringRef("REMARKS\0"
+                                 "\0\0\0\0\0\0\0\0"
+                                 "\0\0\0\0\0\0\0\0"
+                                 "/path/",
+                                 28),
+                       "No such file or directory");
+}




More information about the llvm-commits mailing list