[llvm-branch-commits] [llvm] [Remarks] Escape control characters in YAML remark arguments and decode them when parsing (PR #227654)
Henrich Lauko via llvm-branch-commits
llvm-branch-commits at lists.llvm.org
Thu Oct 1 06:34:16 PDT 2026
https://github.com/xlauko updated https://github.com/llvm/llvm-project/pull/227654
>From ffcd3f19df5bc8e989c31bd7aa840dcb38edea6d Mon Sep 17 00:00:00 2001
From: Henrich Lauko <hlauko at nvidia.com>
Date: Wed, 30 Sep 2026 10:41:35 +0000
Subject: [PATCH 1/3] [Remarks] Escape control characters in YAML remark
arguments and decode them when parsing
The serializer writes argument values with more than one newline as
literal block scalars. A block scalar has no escapes, so a value that
also contained a control character other than tab or newline was
written with the raw byte, which strict YAML readers reject. Such
values now use the double-quoted form, as do values whose first line
starts with a space, which a block scalar would read as indentation.
The parser took the raw scalar text and stripped only single quotes, so
double-quoted values came back with their quotes and escapes, and ''
inside single quotes was not unescaped. Decode scalars with
ScalarNode::getValue instead and report escape errors. Unescaped values
and block scalar values are copied into storage owned by the parser.
Block scalar values used to point into the YAML document, which next()
frees before returning the remark.
Assisted-by: Claude
---
llvm/lib/Remarks/YAMLRemarkParser.cpp | 31 +++++-----
llvm/lib/Remarks/YAMLRemarkParser.h | 4 ++
llvm/lib/Remarks/YAMLRemarkSerializer.cpp | 16 ++++-
.../Remarks/YAMLRemarksParsingTest.cpp | 51 ++++++++++++++++
.../Remarks/YAMLRemarksSerializerTest.cpp | 61 +++++++++++++++++++
5 files changed, 146 insertions(+), 17 deletions(-)
diff --git a/llvm/lib/Remarks/YAMLRemarkParser.cpp b/llvm/lib/Remarks/YAMLRemarkParser.cpp
index 33f659eaaa49b..b1ba56c042a6f 100644
--- a/llvm/lib/Remarks/YAMLRemarkParser.cpp
+++ b/llvm/lib/Remarks/YAMLRemarkParser.cpp
@@ -265,22 +265,21 @@ Expected<StringRef> YAMLRemarkParser::parseKey(yaml::KeyValueNode &Node) {
}
Expected<StringRef> YAMLRemarkParser::parseStr(yaml::KeyValueNode &Node) {
- auto *Value = dyn_cast_if_present<yaml::ScalarNode>(Node.getValue());
- yaml::BlockScalarNode *ValueBlock;
- StringRef Result;
- if (!Value) {
- // Try to parse the value as a block node.
- ValueBlock = dyn_cast_if_present<yaml::BlockScalarNode>(Node.getValue());
- if (!ValueBlock)
- return error("expected a value of scalar type.", Node);
- Result = ValueBlock->getValue();
- } else
- Result = Value->getRawValue();
-
- Result.consume_front("\'");
- Result.consume_back("\'");
-
- return Result;
+ yaml::Node *Value = Node.getValue();
+ // A block value lives in the YAML document, which next() frees before
+ // returning the remark.
+ if (auto *Block = dyn_cast_if_present<yaml::BlockScalarNode>(Value))
+ return Block->getValue().copy(Alloc);
+
+ auto *Scalar = dyn_cast_if_present<yaml::ScalarNode>(Value);
+ if (!Scalar)
+ return error("expected a value of scalar type.", Node);
+ SmallString<32> Storage;
+ StringRef Result = Scalar->getValue(Storage);
+ if (Error E = error())
+ return std::move(E);
+ // getValue only fills Storage when it had to rewrite the value.
+ return Storage.empty() ? Result : Result.copy(Alloc);
}
Expected<unsigned> YAMLRemarkParser::parseUnsigned(yaml::KeyValueNode &Node) {
diff --git a/llvm/lib/Remarks/YAMLRemarkParser.h b/llvm/lib/Remarks/YAMLRemarkParser.h
index 9a30e9e295cb2..7de905b2a5bf8 100644
--- a/llvm/lib/Remarks/YAMLRemarkParser.h
+++ b/llvm/lib/Remarks/YAMLRemarkParser.h
@@ -15,6 +15,7 @@
#include "llvm/Remarks/Remark.h"
#include "llvm/Remarks/RemarkParser.h"
+#include "llvm/Support/Allocator.h"
#include "llvm/Support/Error.h"
#include "llvm/Support/MemoryBuffer.h"
#include "llvm/Support/SourceMgr.h"
@@ -58,6 +59,9 @@ struct YAMLRemarkParser : public RemarkParser {
/// If we parse remark metadata in separate mode, we need to open a new file
/// and parse that.
std::unique_ptr<MemoryBuffer> SeparateBuf;
+ /// Storage for values that do not point into the input buffer: unescaped
+ /// scalars and block scalars. Remarks point into it.
+ BumpPtrAllocator Alloc;
YAMLRemarkParser(StringRef Buf);
diff --git a/llvm/lib/Remarks/YAMLRemarkSerializer.cpp b/llvm/lib/Remarks/YAMLRemarkSerializer.cpp
index 22e297040575c..39107f665e73f 100644
--- a/llvm/lib/Remarks/YAMLRemarkSerializer.cpp
+++ b/llvm/lib/Remarks/YAMLRemarkSerializer.cpp
@@ -12,6 +12,8 @@
//===----------------------------------------------------------------------===//
#include "llvm/Remarks/YAMLRemarkSerializer.h"
+#include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/StringExtras.h"
#include "llvm/Remarks/Remark.h"
#include "llvm/Support/FileSystem.h"
#include <optional>
@@ -109,6 +111,18 @@ template <typename T> struct SequenceTraits<ArrayRef<T>> {
}
};
+/// A literal block scalar has no escapes, so it cannot hold control characters
+/// other than tab and line feed. UTF-8 is written as-is. Readers take the
+/// indentation from the first non-empty line, so that line cannot start with a
+/// space.
+static bool canUseBlockScalar(StringRef S) {
+ if (S.ltrim('\n').starts_with(' '))
+ return false;
+ return all_of(S, [](unsigned char C) {
+ return isPrint(C) || C == '\t' || C == '\n' || C >= 0x80;
+ });
+}
+
/// Implement this as a mapping for now to get proper quotation for the value.
template <> struct MappingTraits<Argument> {
static void mapping(IO &io, Argument &A) {
@@ -116,7 +130,7 @@ template <> struct MappingTraits<Argument> {
// NB: A.Key.data() is not necessarily null-terminated, as the StringRef may
// be a span into the middle of a string.
- if (StringRef(A.Val).count('\n') > 1) {
+ if (StringRef(A.Val).count('\n') > 1 && canUseBlockScalar(A.Val)) {
StringBlockVal S(A.Val);
io.mapRequired(A.Key, S);
} else {
diff --git a/llvm/unittests/Remarks/YAMLRemarksParsingTest.cpp b/llvm/unittests/Remarks/YAMLRemarksParsingTest.cpp
index 824813aa5af7c..29d378058717c 100644
--- a/llvm/unittests/Remarks/YAMLRemarksParsingTest.cpp
+++ b/llvm/unittests/Remarks/YAMLRemarksParsingTest.cpp
@@ -384,6 +384,16 @@ TEST(YAMLRemarks, ParsingWrongArgs) {
" - DebugLoc: { File: a, Line: 1, Column: 2 }\n"
"",
"argument key is missing."));
+ // Bad escape in a double-quoted value.
+ EXPECT_TRUE(parseExpectError("\n"
+ "--- !Missed\n"
+ "Pass: inline\n"
+ "Name: NoDefinition\n"
+ "Function: foo\n"
+ "Args:\n"
+ " - Str: \"a\\qb\"\n"
+ "",
+ "Unrecognized escape code"));
}
static inline StringRef checkStr(StringRef Str, unsigned ExpectedLen) {
@@ -475,6 +485,47 @@ TEST(YAMLRemarks, Contents) {
EXPECT_TRUE(errorToBool(std::move(E))); // Check for parsing errors.
}
+TEST(YAMLRemarks, ContentsQuoted) {
+ StringRef Buf = "--- !Missed\n"
+ "Pass: pass\n"
+ "Name: name\n"
+ "Function: func\n"
+ "Args:\n"
+ " - Single: 'it''s'\n"
+ " - Double: \"abc\\ndef\\n\\x01ghi\"\n"
+ " - Block: |\n"
+ " 'abc'\n"
+ " def\n"
+ "--- !Missed\n"
+ "Pass: pass\n"
+ "Name: name\n"
+ "Function: func\n"
+ "Args:\n"
+ " - Block: |\n"
+ " xxxxxxxxxx\n"
+ " xxxxxxxxxx\n"
+ "\n";
+
+ Expected<std::unique_ptr<remarks::RemarkParser>> MaybeParser =
+ remarks::createRemarkParser(remarks::Format::YAML, Buf);
+ EXPECT_FALSE(errorToBool(MaybeParser.takeError()));
+ EXPECT_TRUE(*MaybeParser != nullptr);
+
+ remarks::RemarkParser &Parser = **MaybeParser;
+ Expected<std::unique_ptr<remarks::Remark>> MaybeRemark = Parser.next();
+ EXPECT_FALSE(errorToBool(MaybeRemark.takeError()));
+ EXPECT_TRUE(*MaybeRemark != nullptr);
+ // The values must outlive the YAML document they were parsed from.
+ Expected<std::unique_ptr<remarks::Remark>> MaybeNext = Parser.next();
+ EXPECT_FALSE(errorToBool(MaybeNext.takeError()));
+
+ const remarks::Remark &Remark = **MaybeRemark;
+ ASSERT_EQ(Remark.Args.size(), 3U);
+ EXPECT_EQ(checkStr(Remark.Args[0].Val, 4), "it's");
+ EXPECT_EQ(checkStr(Remark.Args[1].Val, 12), "abc\ndef\n\x01ghi");
+ EXPECT_EQ(checkStr(Remark.Args[2].Val, 10), "'abc'\ndef\n");
+}
+
static inline StringRef checkStr(LLVMRemarkStringRef Str,
unsigned ExpectedLen) {
const char *StrData = LLVMRemarkStringGetData(Str);
diff --git a/llvm/unittests/Remarks/YAMLRemarksSerializerTest.cpp b/llvm/unittests/Remarks/YAMLRemarksSerializerTest.cpp
index 974356d9cf30a..7da6e8ed4f1c8 100644
--- a/llvm/unittests/Remarks/YAMLRemarksSerializerTest.cpp
+++ b/llvm/unittests/Remarks/YAMLRemarksSerializerTest.cpp
@@ -193,3 +193,64 @@ TEST(YAMLRemarks, SerializerRemarkStringRefOOBRead) {
" DebugLoc: { File: argpath, Line: 6, Column: 7 }\n"
"...\n");
}
+
+TEST(YAMLRemarks, SerializerRemarkMultiLineArg) {
+ remarks::Remark R;
+ R.RemarkType = remarks::Type::Missed;
+ R.PassName = "pass";
+ R.RemarkName = "name";
+ R.FunctionName = "func";
+ R.Args.emplace_back();
+ R.Args.back().Key = "block";
+ R.Args.back().Val = "abc\ndef\nghi";
+ // A literal block scalar cannot hold control characters, so this has to be
+ // escaped in a double-quoted scalar instead.
+ R.Args.emplace_back();
+ R.Args.back().Key = "control";
+ R.Args.back().Val = "abc\ndef\n\x01ghi";
+ checkStandalone(remarks::Format::YAML, R,
+ "--- !Missed\n"
+ "Pass: pass\n"
+ "Name: name\n"
+ "Function: func\n"
+ "Args:\n"
+ " - block: |\n"
+ " abc\n"
+ " def\n"
+ " ghi\n"
+ " - control: \"abc\\ndef\\n\\x01ghi\"\n"
+ "...\n");
+}
+
+TEST(YAMLRemarks, SerializerRemarkRoundTrip) {
+ // Values that the literal block form cannot hold must survive a round trip.
+ StringRef Vals[] = {"abc\ndef\n\x01ghi", "abc\r\ndef\r\nghi",
+ StringRef("abc\ndef\n\0ghi", 12), " abc\ndef\nghi",
+ "abc\ndef\nghi\n"};
+ remarks::Remark R;
+ R.RemarkType = remarks::Type::Missed;
+ R.PassName = "pass";
+ R.RemarkName = "name";
+ R.FunctionName = "func";
+ for (StringRef Val : Vals) {
+ R.Args.emplace_back();
+ R.Args.back().Key = "key";
+ R.Args.back().Val = Val;
+ }
+
+ std::string Buf;
+ raw_string_ostream OS(Buf);
+ Expected<std::unique_ptr<remarks::RemarkSerializer>> MaybeS =
+ createRemarkSerializer(remarks::Format::YAML, OS);
+ ASSERT_FALSE(errorToBool(MaybeS.takeError()));
+ (*MaybeS)->emit(R);
+ (*MaybeS)->finalize();
+
+ Expected<std::unique_ptr<remarks::RemarkParser>> MaybeParser =
+ remarks::createRemarkParser(remarks::Format::YAML, Buf);
+ ASSERT_FALSE(errorToBool(MaybeParser.takeError()));
+ Expected<std::unique_ptr<remarks::Remark>> MaybeRemark =
+ (*MaybeParser)->next();
+ ASSERT_FALSE(errorToBool(MaybeRemark.takeError()));
+ EXPECT_EQ(**MaybeRemark, R);
+}
>From 459416d5065b10d42abef0e308545c7d7cf61763 Mon Sep 17 00:00:00 2001
From: Henrich Lauko <hlauko at nvidia.com>
Date: Thu, 1 Oct 2026 13:20:12 +0000
Subject: [PATCH 2/3] [Remarks] Reject null characters in YAML remark strings
The bitstream string table stores NUL-terminated strings, so a string
with an embedded NUL splits into several entries and every later string
is read back at the wrong index. Now that the YAML parser decodes
escapes, "\0" in YAML input reaches that path. Reject it with a parse
error.
Assisted-by: Claude
---
llvm/lib/Remarks/YAMLRemarkParser.cpp | 3 +++
.../tools/llvm-remarkutil/yaml2bitstream-null.test | 12 ++++++++++++
llvm/unittests/Remarks/YAMLRemarksParsingTest.cpp | 11 +++++++++++
llvm/unittests/Remarks/YAMLRemarksSerializerTest.cpp | 3 +--
4 files changed, 27 insertions(+), 2 deletions(-)
create mode 100644 llvm/test/tools/llvm-remarkutil/yaml2bitstream-null.test
diff --git a/llvm/lib/Remarks/YAMLRemarkParser.cpp b/llvm/lib/Remarks/YAMLRemarkParser.cpp
index b1ba56c042a6f..31d255b56d55b 100644
--- a/llvm/lib/Remarks/YAMLRemarkParser.cpp
+++ b/llvm/lib/Remarks/YAMLRemarkParser.cpp
@@ -278,6 +278,9 @@ Expected<StringRef> YAMLRemarkParser::parseStr(yaml::KeyValueNode &Node) {
StringRef Result = Scalar->getValue(Storage);
if (Error E = error())
return std::move(E);
+ // The bitstream format cannot hold a null character in a string.
+ if (Result.contains('\0'))
+ return error("remark strings cannot contain null characters.", *Scalar);
// getValue only fills Storage when it had to rewrite the value.
return Storage.empty() ? Result : Result.copy(Alloc);
}
diff --git a/llvm/test/tools/llvm-remarkutil/yaml2bitstream-null.test b/llvm/test/tools/llvm-remarkutil/yaml2bitstream-null.test
new file mode 100644
index 0000000000000..6cd09e374f412
--- /dev/null
+++ b/llvm/test/tools/llvm-remarkutil/yaml2bitstream-null.test
@@ -0,0 +1,12 @@
+# The bitstream string table cannot hold a null character, so reject one in
+# YAML input instead of writing a corrupt string table.
+# RUN: not llvm-remarkutil yaml2bitstream %s -o /dev/null 2>&1 | FileCheck %s
+# CHECK: error: {{.*}}remark strings cannot contain null characters.
+
+--- !Missed
+Pass: pass
+Name: name
+Function: func
+Args:
+ - String: "abc\0def"
+...
diff --git a/llvm/unittests/Remarks/YAMLRemarksParsingTest.cpp b/llvm/unittests/Remarks/YAMLRemarksParsingTest.cpp
index c3bbdb7f971a3..f74c9958e0ff3 100644
--- a/llvm/unittests/Remarks/YAMLRemarksParsingTest.cpp
+++ b/llvm/unittests/Remarks/YAMLRemarksParsingTest.cpp
@@ -394,6 +394,17 @@ TEST(YAMLRemarks, ParsingWrongArgs) {
" - Str: \"a\\qb\"\n"
"",
"Unrecognized escape code"));
+ // Null character in a value.
+ EXPECT_TRUE(
+ parseExpectError("\n"
+ "--- !Missed\n"
+ "Pass: inline\n"
+ "Name: NoDefinition\n"
+ "Function: foo\n"
+ "Args:\n"
+ " - Str: \"a\\0b\"\n"
+ "",
+ "remark strings cannot contain null characters."));
}
static inline StringRef checkStr(StringRef Str, unsigned ExpectedLen) {
diff --git a/llvm/unittests/Remarks/YAMLRemarksSerializerTest.cpp b/llvm/unittests/Remarks/YAMLRemarksSerializerTest.cpp
index 7da6e8ed4f1c8..53ca5d2fe2562 100644
--- a/llvm/unittests/Remarks/YAMLRemarksSerializerTest.cpp
+++ b/llvm/unittests/Remarks/YAMLRemarksSerializerTest.cpp
@@ -225,8 +225,7 @@ TEST(YAMLRemarks, SerializerRemarkMultiLineArg) {
TEST(YAMLRemarks, SerializerRemarkRoundTrip) {
// Values that the literal block form cannot hold must survive a round trip.
StringRef Vals[] = {"abc\ndef\n\x01ghi", "abc\r\ndef\r\nghi",
- StringRef("abc\ndef\n\0ghi", 12), " abc\ndef\nghi",
- "abc\ndef\nghi\n"};
+ " abc\ndef\nghi", "abc\ndef\nghi\n"};
remarks::Remark R;
R.RemarkType = remarks::Type::Missed;
R.PassName = "pass";
>From c3d85f713cd665f68142c6912315c02c9b498d49 Mon Sep 17 00:00:00 2001
From: Henrich Lauko <hlauko at nvidia.com>
Date: Thu, 1 Oct 2026 13:30:00 +0000
Subject: [PATCH 3/3] [Support] Preserve trailing newlines in YAML block scalar
output
Output::blockScalarString always wrote "|", whose default clip chomping
keeps exactly one trailing line break. A value without a trailing
newline came back with one, and extra trailing newlines were dropped.
Write "|-" or "|+" when the value does not end in exactly one newline.
MIR modules and bodies end in exactly one newline, so their output does
not change.
Assisted-by: Claude
---
llvm/lib/Support/YAMLTraits.cpp | 9 +++++++++
.../Remarks/YAMLRemarksSerializerTest.cpp | 9 ++++++---
llvm/unittests/Support/YAMLIOTest.cpp | 14 +++++++-------
3 files changed, 22 insertions(+), 10 deletions(-)
diff --git a/llvm/lib/Support/YAMLTraits.cpp b/llvm/lib/Support/YAMLTraits.cpp
index 622806423635f..7c5ab1ef67712 100644
--- a/llvm/lib/Support/YAMLTraits.cpp
+++ b/llvm/lib/Support/YAMLTraits.cpp
@@ -756,6 +756,15 @@ void Output::blockScalarString(StringRef &S) {
if (!StateStack.empty())
newLineCheck();
output(" |");
+ // The default chomping keeps exactly one trailing line break, so strip or
+ // keep them explicitly to preserve the value.
+ if (!S.empty()) {
+ size_t LineBreaks = S.size() - S.rtrim('\n').size();
+ if (LineBreaks == 0)
+ output("-");
+ else if (LineBreaks > 1)
+ output("+");
+ }
unsigned Indent = StateStack.empty() ? 1 : StateStack.size();
diff --git a/llvm/unittests/Remarks/YAMLRemarksSerializerTest.cpp b/llvm/unittests/Remarks/YAMLRemarksSerializerTest.cpp
index 53ca5d2fe2562..1950c01dfb034 100644
--- a/llvm/unittests/Remarks/YAMLRemarksSerializerTest.cpp
+++ b/llvm/unittests/Remarks/YAMLRemarksSerializerTest.cpp
@@ -214,7 +214,7 @@ TEST(YAMLRemarks, SerializerRemarkMultiLineArg) {
"Name: name\n"
"Function: func\n"
"Args:\n"
- " - block: |\n"
+ " - block: |-\n"
" abc\n"
" def\n"
" ghi\n"
@@ -223,9 +223,12 @@ TEST(YAMLRemarks, SerializerRemarkMultiLineArg) {
}
TEST(YAMLRemarks, SerializerRemarkRoundTrip) {
- // Values that the literal block form cannot hold must survive a round trip.
+ // Values must survive a round trip, whether they are written as a block or
+ // as a quoted scalar.
StringRef Vals[] = {"abc\ndef\n\x01ghi", "abc\r\ndef\r\nghi",
- " abc\ndef\nghi", "abc\ndef\nghi\n"};
+ " abc\ndef\nghi", "abc\ndef\nghi",
+ "abc\ndef\nghi\n", "abc\ndef\nghi\n\n",
+ "\nabc\ndef\nghi"};
remarks::Remark R;
R.RemarkType = remarks::Type::Missed;
R.PassName = "pass";
diff --git a/llvm/unittests/Support/YAMLIOTest.cpp b/llvm/unittests/Support/YAMLIOTest.cpp
index 78e19c5ff4f2c..216ee353555fc 100644
--- a/llvm/unittests/Support/YAMLIOTest.cpp
+++ b/llvm/unittests/Support/YAMLIOTest.cpp
@@ -1211,12 +1211,12 @@ TEST(YAMLIO, TestReadWriteMultilineStringType) {
yin >> map2;
EXPECT_FALSE(yin.error());
- EXPECT_EQ(map2.name.str, "An Item\n");
- EXPECT_EQ(map2.description.str, "Hello\nWorld\n");
+ EXPECT_EQ(map2.name.str, "An Item");
+ EXPECT_EQ(map2.description.str, "Hello\nWorld");
EXPECT_EQ(map2.ingredients.str, "SubItem 1\nSub Item 2\n\nSub Item 3\n");
- EXPECT_EQ(map2.recipes.str, "\n\nTest 1\n");
+ EXPECT_EQ(map2.recipes.str, "\n\nTest 1\n\n\n");
EXPECT_TRUE(map2.warningLabels.str.empty());
- EXPECT_TRUE(map2.documentation.str.empty());
+ EXPECT_EQ(map2.documentation.str, "\n\n");
EXPECT_EQ(map2.price, 350);
}
}
@@ -1248,7 +1248,7 @@ TEST(YAMLIO, TestReadWriteBlockScalarDocuments) {
EXPECT_FALSE(yin.error());
EXPECT_EQ(documents2.size(), size_t(1));
- EXPECT_EQ(documents2[0].str, "Hello\nWorld\n");
+ EXPECT_EQ(documents2[0].str, "Hello\nWorld");
}
}
@@ -1268,7 +1268,7 @@ TEST(YAMLIO, TestReadWriteBlockScalarValue) {
yin >> doc;
EXPECT_FALSE(yin.error());
- EXPECT_EQ(doc.str, "Just a block\nscalar doc\n");
+ EXPECT_EQ(doc.str, "Just a block\nscalar doc");
}
}
@@ -1294,7 +1294,7 @@ TEST(YAMLIO, TestScalarAfterBlockScalar) {
Output yout(ostr);
yout << v;
EXPECT_EQ(output, R"(---
-- block_scalac: |
+- block_scalac: |-
AA
BB
scalar: a
More information about the llvm-branch-commits
mailing list