[llvm] Fix for invalid JSON produced by ELFAttributeParser. (PR #120592)
Paul Bowen-Huggett via llvm-commits
llvm-commits at lists.llvm.org
Fri Dec 20 00:50:54 PST 2024
https://github.com/paulhuggett updated https://github.com/llvm/llvm-project/pull/120592
>From e738123b8a3786e093293ddaca3205599594d277 Mon Sep 17 00:00:00 2001
From: Paul Bowen-Huggett <paulhuggett at mac.com>
Date: Thu, 19 Dec 2024 16:29:10 +0100
Subject: [PATCH 1/2] Fix for invalid JSON produced by ELFAttributeParser.
If JSONScopedPrinter is used as the output printer for the
ELFAttributeParser class, the output may not be valid. The "Section N"
key was not correctly quoted and did not include the colon separator
before the associated value.
A unit test exercises and checks the output produced by the
combination of the two classes with pretty-printing enabled and
disabled.
---
llvm/lib/Support/ELFAttributeParser.cpp | 7 +-
.../Support/ELFAttributeParserTest.cpp | 64 +++++++++++++++++++
2 files changed, 67 insertions(+), 4 deletions(-)
diff --git a/llvm/lib/Support/ELFAttributeParser.cpp b/llvm/lib/Support/ELFAttributeParser.cpp
index 26c3d54e17ade8..b61507d1f55ed5 100644
--- a/llvm/lib/Support/ELFAttributeParser.cpp
+++ b/llvm/lib/Support/ELFAttributeParser.cpp
@@ -213,8 +213,8 @@ Error ELFAttributeParser::parse(ArrayRef<uint8_t> section,
return cursor.takeError();
if (sw) {
- sw->startLine() << "Section " << ++sectionNumber << " {\n";
- sw->indent();
+ ++sectionNumber;
+ sw->objectBegin((Twine("Section ") + Twine(sectionNumber)).str());
}
if (sectionLength < 4 || cursor.tell() - 4 + sectionLength > section.size())
@@ -226,8 +226,7 @@ Error ELFAttributeParser::parse(ArrayRef<uint8_t> section,
if (Error e = parseSubsection(sectionLength))
return e;
if (sw) {
- sw->unindent();
- sw->startLine() << "}\n";
+ sw->objectEnd();
}
}
diff --git a/llvm/unittests/Support/ELFAttributeParserTest.cpp b/llvm/unittests/Support/ELFAttributeParserTest.cpp
index 38e7b09cc3c7d5..82b3f8d8c81e7e 100644
--- a/llvm/unittests/Support/ELFAttributeParserTest.cpp
+++ b/llvm/unittests/Support/ELFAttributeParserTest.cpp
@@ -8,6 +8,7 @@
#include "llvm/Support/ELFAttributeParser.h"
#include "llvm/Support/ELFAttributes.h"
+#include "llvm/Support/ScopedPrinter.h"
#include "gtest/gtest.h"
#include <string>
@@ -56,3 +57,66 @@ TEST(AttributeHeaderParser, InvalidAttributeSize) {
't', 0, 1, 4, 0, 0, 0};
testParseError(bytes, "invalid attribute size 4 at offset 0xa");
}
+
+class AttributeParserJSONOutput : public testing::TestWithParam<bool> {
+public:
+ // Accepts the contents of an ELF attribute section and parses it to
+ // JSON-formatted output. The error value from the attribute parse as
+ // well as its formatted output are returned.
+ std::pair<Error, std::string> parse(ArrayRef<uint8_t> Section,
+ bool PrettyPrint) {
+ std::string Output;
+ raw_string_ostream OS{Output};
+ JSONScopedPrinter Printer{OS, PrettyPrint};
+ Parser P{&Printer, emptyTagNameMap, "vendor"};
+ Error Err = P.parse(Section, endianness::little);
+ return std::make_pair(std::move(Err), std::move(Output));
+ }
+
+private:
+ class Parser : public ELFAttributeParser {
+ public:
+ using ELFAttributeParser::ELFAttributeParser;
+
+ private:
+ Error handler(uint64_t, bool &Handled) override {
+ Handled = false; // No custom attributes are handled.
+ return Error::success();
+ }
+ };
+};
+
+TEST_P(AttributeParserJSONOutput, Empty) {
+ const uint8_t Section[] = {
+ 'A', // format magic number
+ 11, 0, 0, 0, // section length
+ 'v', 'e', 'n', 'd', 'o', 'r', '\0', // vendor name
+ };
+ // Parse and emit JSON. Pretty-printing is controlled by the
+ // test parameter.
+ auto [Err, Output] = this->parse(Section, GetParam());
+ EXPECT_FALSE(bool{Err}) << toString(std::move(Err));
+ // Check that 'Output' is valid JSON.
+ Error JsonErr = json::parse(Output).takeError();
+ EXPECT_FALSE(bool{JsonErr}) << Output << '\n' << toString(std::move(JsonErr));
+}
+
+TEST_P(AttributeParserJSONOutput, SingleSubsection) {
+ const uint8_t Section[] = {
+ 'A', // format magic number
+ 18, 0, 0, 0, // section length
+ 'v', 'e', 'n', 'd', 'o', 'r', '\0', // vendor name
+ 1, // tag (File=1, Section=2, Symbol=3)
+ 7, 0, 0, 0, // size
+ 32, // tag (uleb128)
+ 16, // value (16 uleb128)
+ };
+ auto [Err, Output] = this->parse(Section, GetParam());
+ EXPECT_FALSE(bool{Err}) << toString(std::move(Err));
+ // Check that 'Output' is valid JSON.
+ Error JsonErr = json::parse(Output).takeError();
+ EXPECT_FALSE(bool{JsonErr}) << Output << '\n' << toString(std::move(JsonErr));
+}
+
+INSTANTIATE_TEST_SUITE_P(AttributeParserJSONOutput, AttributeParserJSONOutput,
+ testing::Values(false, true));
>From cbe6f9de52bb6cc68a962e099f44cf349048eedc Mon Sep 17 00:00:00 2001
From: Paul Bowen-Huggett <paulhuggett at mac.com>
Date: Fri, 20 Dec 2024 09:49:40 +0100
Subject: [PATCH 2/2] Include any JSON output in the message if the initial
parse() fails.
---
llvm/unittests/Support/ELFAttributeParserTest.cpp | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/llvm/unittests/Support/ELFAttributeParserTest.cpp b/llvm/unittests/Support/ELFAttributeParserTest.cpp
index 82b3f8d8c81e7e..d66578bdd9f4bd 100644
--- a/llvm/unittests/Support/ELFAttributeParserTest.cpp
+++ b/llvm/unittests/Support/ELFAttributeParserTest.cpp
@@ -95,7 +95,7 @@ TEST_P(AttributeParserJSONOutput, Empty) {
// Parse and emit JSON. Pretty-printing is controlled by the
// test parameter.
auto [Err, Output] = this->parse(Section, GetParam());
- EXPECT_FALSE(bool{Err}) << toString(std::move(Err));
+ EXPECT_FALSE(bool{Err}) << Output << '\n' << toString(std::move(Err));
// Check that 'Output' is valid JSON.
Error JsonErr = json::parse(Output).takeError();
EXPECT_FALSE(bool{JsonErr}) << Output << '\n' << toString(std::move(JsonErr));
@@ -112,7 +112,7 @@ TEST_P(AttributeParserJSONOutput, SingleSubsection) {
16, // value (16 uleb128)
};
auto [Err, Output] = this->parse(Section, GetParam());
- EXPECT_FALSE(bool{Err}) << toString(std::move(Err));
+ EXPECT_FALSE(bool{Err}) << Output << '\n' << toString(std::move(Err));
// Check that 'Output' is valid JSON.
Error JsonErr = json::parse(Output).takeError();
EXPECT_FALSE(bool{JsonErr}) << Output << '\n' << toString(std::move(JsonErr));
More information about the llvm-commits
mailing list