[llvm] Fix for invalid JSON produced by ELFAttributeParser. (PR #120592)
Paul Bowen-Huggett via llvm-commits
llvm-commits at lists.llvm.org
Thu Dec 19 07:38:21 PST 2024
https://github.com/paulhuggett created https://github.com/llvm/llvm-project/pull/120592
Start with a minimal source file, compile to a RISCV-32 ELF object file, and use readobj to dump the RISC-V attributes section:
```bash
echo "void f(){}" > t.c
clang -c -target riscv32-none t.c
llvm-readobj --elf-output-style=JSON --arch-specific t.o
```
What we get looks something like:
```
[{"FileSummary":{"File":"t.o","Format":"elf32-littleriscv","Arch":"riscv32","AddressSize":"32bit","LoadName":"<Not found>"},"BuildAttributes":{"FormatVersion":65Section 1 {
,"SectionLength":70,"Vendor":"riscv","Tag":{"Name":"Tag_File","Value":1},"Size":60,"FileAttributes":{"Attribute":{"Tag":4,"Value":16,"TagName":"stack_align","Description":"Stack alignment is 16-bytes"},"Attribute":{"Tag":5,"TagName":"arch","Value":"rv32i2p1_m2p0_a2p1_c2p0_zmmul1p0_zaamo1p0_zalrsc1p0"}}}
}}]
```
This is not legal JSON. If you look at the portion containing `{"FormatVersion":65Section 1 {`, you'll see that:
- there is no comma separator between `"FormatVersion":65` and `Section 1`
- the second of these is an object key missing its surrounding quotation marks and colon
- we have a stray comma after the value object's openning brace
This is easier to see if readobj's `--pretty-print` mode is enabled:
```
"Arch": "riscv64",
"AddressSize": "64bit",
"LoadName": "<Not found>"
},
"BuildAttributes": {
"FormatVersion": 65Section 1 {
,
"SectionLength": 70,
"Vendor": "riscv",
"Tag": {
"Name": "Tag_File",
"Value": 1
},
```
This is pretty obviously wrong! (For clarity I've removed the unrelated portions of the output.)
The fix uses the objectBegin() and objectEnd() members of the ScopedPrinter rather than directly emitting the text. A unit test exercises and checks the output produced by the combination of the two classes with pretty-printing enabled and disabled.
>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] 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));
More information about the llvm-commits
mailing list