[llvm] [Remarks] Escape control characters in YAML remark arguments and decode them when parsing (PR #227654)
Henrich Lauko via llvm-commits
llvm-commits at lists.llvm.org
Wed Sep 30 05:04:10 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] [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);
+}
More information about the llvm-commits
mailing list