From fb8a68898266a591f944d4bc2f878234786fe2c0 Mon Sep 17 00:00:00 2001 From: Francis Visoiu Mistrih Date: Fri, 26 Jul 2019 20:54:44 +0000 Subject: [PATCH] Revert "[Remarks] Support parsing remark metadata in the YAML remark parser" This reverts r367148. Seems to fail on http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-fuzzer/builds/27768. llvm-svn: 367151 --- docs/Remarks.rst | 33 ++---- include/llvm/Remarks/RemarkParser.h | 4 - lib/Remarks/RemarkParser.cpp | 15 --- lib/Remarks/YAMLRemarkParser.cpp | 105 ------------------- lib/Remarks/YAMLRemarkParser.h | 9 -- unittests/Remarks/YAMLRemarksParsingTest.cpp | 86 --------------- 6 files changed, 9 insertions(+), 243 deletions(-) diff --git a/docs/Remarks.rst b/docs/Remarks.rst index c573eb2dec5..56465476d01 100644 --- a/docs/Remarks.rst +++ b/docs/Remarks.rst @@ -218,28 +218,6 @@ 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 ========== @@ -317,8 +295,15 @@ Emitting remark diagnostics in the object file ============================================== A section containing metadata on remark diagnostics will be emitted when --remarks-section is passed. The section contains the metadata associated to the -format used to serialize the remarks. +-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. The section is named: diff --git a/include/llvm/Remarks/RemarkParser.h b/include/llvm/Remarks/RemarkParser.h index 551d20a8a0a..f73671214f0 100644 --- a/include/llvm/Remarks/RemarkParser.h +++ b/include/llvm/Remarks/RemarkParser.h @@ -80,10 +80,6 @@ Expected> createRemarkParser(Format ParserFormat, StringRef Buf, ParsedStringTable StrTab); -Expected> -createRemarkParserFromMeta(Format ParserFormat, StringRef Buf, - Optional StrTab = None); - } // end namespace remarks } // end namespace llvm diff --git a/lib/Remarks/RemarkParser.cpp b/lib/Remarks/RemarkParser.cpp index 1ff9cf626b6..e7f174ce02c 100644 --- a/lib/Remarks/RemarkParser.cpp +++ b/lib/Remarks/RemarkParser.cpp @@ -80,21 +80,6 @@ llvm::remarks::createRemarkParser(Format ParserFormat, StringRef Buf, llvm_unreachable("unhandled ParseFormat"); } -Expected> -llvm::remarks::createRemarkParserFromMeta(Format ParserFormat, StringRef Buf, - Optional 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 TheParser; diff --git a/lib/Remarks/YAMLRemarkParser.cpp b/lib/Remarks/YAMLRemarkParser.cpp index fdd059c7037..f677bfc04ce 100644 --- a/lib/Remarks/YAMLRemarkParser.cpp +++ b/lib/Remarks/YAMLRemarkParser.cpp @@ -14,7 +14,6 @@ #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; @@ -55,110 +54,6 @@ static SourceMgr setupSM(std::string &LastErrorMessage) { return SM; } -// Parse the magic number. This function returns true if this represents remark -// metadata, false otherwise. -static Expected 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 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( - 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 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( - Buf.data()); - Buf = Buf.drop_front(sizeof(uint64_t)); - return StrTabSize; -} - -static Expected 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 Result; -} - -Expected> -remarks::createYAMLParserFromMeta(StringRef Buf, - Optional StrTab) { - // We now have a magic number. The metadata has to be correct. - Expected isMeta = parseMagic(Buf); - if (!isMeta) - return isMeta.takeError(); - // If it's not recognized as metadata, roll back. - std::unique_ptr SeparateBuf; - if (*isMeta) { - Expected Version = parseVersion(Buf); - if (!Version) - return Version.takeError(); - - Expected 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 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> 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 Result = - StrTab - ? llvm::make_unique(Buf, std::move(*StrTab)) - : llvm::make_unique(Buf); - if (SeparateBuf) - Result->SeparateBuf = std::move(SeparateBuf); - return std::move(Result); -} - YAMLRemarkParser::YAMLRemarkParser(StringRef Buf) : YAMLRemarkParser(Buf, None) {} diff --git a/lib/Remarks/YAMLRemarkParser.h b/lib/Remarks/YAMLRemarkParser.h index ff03abbb576..e8f0edc50f4 100644 --- a/lib/Remarks/YAMLRemarkParser.h +++ b/lib/Remarks/YAMLRemarkParser.h @@ -18,7 +18,6 @@ #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" @@ -59,9 +58,6 @@ struct YAMLRemarkParser : public RemarkParser { 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 SeparateBuf; YAMLRemarkParser(StringRef Buf); @@ -108,11 +104,6 @@ protected: /// Parse one value to a string. Expected parseStr(yaml::KeyValueNode &Node) override; }; - -Expected> -createYAMLParserFromMeta(StringRef Buf, - Optional StrTab = None); - } // end namespace remarks } // end namespace llvm diff --git a/unittests/Remarks/YAMLRemarksParsingTest.cpp b/unittests/Remarks/YAMLRemarksParsingTest.cpp index eb5f24ecb98..967ac965069 100644 --- a/unittests/Remarks/YAMLRemarksParsingTest.cpp +++ b/unittests/Remarks/YAMLRemarksParsingTest.cpp @@ -29,22 +29,6 @@ template void parseGood(const char (&Buf)[N]) { EXPECT_TRUE(errorToBool(std::move(E))); // Check for parsing errors. } -void parseGoodMeta(StringRef Buf) { - Expected> MaybeParser = - remarks::createRemarkParserFromMeta(remarks::Format::YAML, Buf); - EXPECT_FALSE(errorToBool(MaybeParser.takeError())); - EXPECT_TRUE(*MaybeParser != nullptr); - - remarks::RemarkParser &Parser = **MaybeParser; - Expected> 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()); - EXPECT_TRUE(errorToBool(std::move(E))); // Check for parsing errors. -} - template bool parseExpectError(const char (&Buf)[N], const char *Error) { Expected> MaybeParser = @@ -63,17 +47,6 @@ bool parseExpectError(const char (&Buf)[N], const char *Error) { return StringRef(Stream.str()).contains(Error); } -void parseExpectErrorMeta(StringRef Buf, const char *Error) { - std::string ErrorStr; - raw_string_ostream Stream(ErrorStr); - - Expected> 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.")); } @@ -646,62 +619,3 @@ TEST(YAMLRemarks, ParsingBadStringTableIndex) { 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"); -}