[Lldb-commits] [lldb] [lldb][bytecode] Improve testing of bytecode section loading (PR #212568)
Dave Lee via lldb-commits
lldb-commits at lists.llvm.org
Fri Jul 31 18:19:12 PDT 2026
https://github.com/kastiglione updated https://github.com/llvm/llvm-project/pull/212568
>From 78b429be5118a7b2ed831c8c88b3b5b7b6f7ebc3 Mon Sep 17 00:00:00 2001
From: Dave Lee <davelee.com at gmail.com>
Date: Tue, 28 Jul 2026 10:57:59 -0700
Subject: [PATCH 1/2] [lldb][bytecode] Improve testing of bytecode section
loading
---
.../DataFormatters/FormatterSection.cpp | 30 +-
.../DataFormatter/FormatterSectionTest.cpp | 345 ++++++++++++++++++
2 files changed, 368 insertions(+), 7 deletions(-)
diff --git a/lldb/source/DataFormatters/FormatterSection.cpp b/lldb/source/DataFormatters/FormatterSection.cpp
index 7ddfdc6ec41d0..c0e495806522c 100644
--- a/lldb/source/DataFormatters/FormatterSection.cpp
+++ b/lldb/source/DataFormatters/FormatterSection.cpp
@@ -73,6 +73,13 @@ static void ForEachFormatterInModule(
uint64_t version = section.getULEB128(cursor);
uint64_t record_size = section.getULEB128(cursor);
+ if (cursor && record_size > section_size - cursor.tell()) {
+ LLDB_LOG(GetLog(LLDBLog::DataFormatters),
+ "Record size {0} exceeds the remaining size of the embedded "
+ "formatter section in {1}; ignoring the rest of the section.",
+ record_size, module.GetFileSpec());
+ break;
+ }
if (version == 1) {
llvm::DataExtractor record(
section.getData().drop_front(cursor.tell()).take_front(record_size),
@@ -173,26 +180,30 @@ void LoadFormattersForModule(ModuleSP module_sp) {
std::unique_ptr<llvm::MemoryBuffer> summary_func_up;
std::array<std::unique_ptr<llvm::MemoryBuffer>, kSignatureCount>
synthetic_methods;
+ bool has_synthetic_method = false;
using Signatures = FormatterBytecode::Signatures;
while (cursor && cursor.tell() < extractor.size()) {
auto signature = static_cast<Signatures>(extractor.getU8(cursor));
uint64_t size = extractor.getULEB128(cursor);
llvm::StringRef bytecode = extractor.getBytes(cursor, size);
- if (!cursor) {
- LLDB_LOG_ERROR(GetLog(LLDBLog::DataFormatters), cursor.takeError(),
- "{0}");
+ if (!cursor)
break;
- }
auto buffer_up = llvm::MemoryBuffer::getMemBufferCopy(bytecode);
if (signature == Signatures::sig_summary)
summary_func_up = std::move(buffer_up);
- else if (signature <= Signatures::sig_update)
+ else if (signature <= Signatures::sig_update) {
synthetic_methods[signature] = std::move(buffer_up);
- else
+ has_synthetic_method = true;
+ } else
LLDB_LOG(GetLog(LLDBLog::DataFormatters),
"Unsupported formatter signature {0} for '{1}' in {2}",
signature, type_name, module_sp->GetFileSpec());
}
+ if (!cursor) {
+ LLDB_LOG_ERROR(GetLog(LLDBLog::DataFormatters), cursor.takeError(),
+ "{0}");
+ return;
+ }
FormatterMatchType match_type = eFormatterMatchExact;
if (type_name.front() == '^')
@@ -205,7 +216,7 @@ void LoadFormattersForModule(ModuleSP module_sp) {
LLDB_LOG(GetLog(LLDBLog::DataFormatters),
"Loaded embedded type summary for '{0}' from {1}.",
type_name, module_sp->GetFileSpec());
- } else {
+ } else if (has_synthetic_method) {
BytecodeSyntheticChildren::SyntheticBytecodeImplementation impl =
CreateSyntheticImpl(synthetic_methods);
auto synthetic_children_sp =
@@ -215,6 +226,11 @@ void LoadFormattersForModule(ModuleSP module_sp) {
LLDB_LOG(GetLog(LLDBLog::DataFormatters),
"Loaded embedded type synthetic for '{0}' from {1}.",
type_name, module_sp->GetFileSpec());
+ } else {
+ LLDB_LOG(GetLog(LLDBLog::DataFormatters),
+ "No summary or synthetic methods found for '{0}' in {1}, "
+ "not registering a formatter.",
+ type_name, module_sp->GetFileSpec());
}
});
}
diff --git a/lldb/unittests/DataFormatter/FormatterSectionTest.cpp b/lldb/unittests/DataFormatter/FormatterSectionTest.cpp
index 7da7a34d84ef9..4ee0bf8e68c66 100644
--- a/lldb/unittests/DataFormatter/FormatterSectionTest.cpp
+++ b/lldb/unittests/DataFormatter/FormatterSectionTest.cpp
@@ -15,17 +15,100 @@
#include "lldb/Core/Debugger.h"
#include "lldb/Core/Module.h"
#include "lldb/DataFormatters/DataVisualization.h"
+#include "lldb/DataFormatters/FormatterBytecode.h"
#include "lldb/Host/FileSystem.h"
#include "lldb/Host/HostInfo.h"
#include "lldb/Target/Platform.h"
#include "lldb/ValueObject/ValueObjectConstResult.h"
+#include "llvm/Support/LEB128.h"
#include "gtest/gtest.h"
+#include <optional>
+#include <string>
+#include <vector>
using namespace lldb;
using namespace lldb_private;
namespace {
+// --- Helpers for hand-assembling the embedded formatter/summary record
+// format read by FormatterSection.cpp, so malformed inputs can be expressed
+// as typed fields instead of raw hex blobs. ---
+
+void AppendULEB(std::vector<uint8_t> &bytes, uint64_t value) {
+ uint8_t buf[10];
+ unsigned len = llvm::encodeULEB128(value, buf);
+ bytes.insert(bytes.end(), buf, buf + len);
+}
+
+void AppendBytes(std::vector<uint8_t> &bytes, llvm::StringRef data) {
+ bytes.insert(bytes.end(), data.begin(), data.end());
+}
+
+void AppendBytes(std::vector<uint8_t> &bytes, llvm::ArrayRef<uint8_t> data) {
+ bytes.insert(bytes.end(), data.begin(), data.end());
+}
+
+// Appends one length-framed record: [version][record_size][type_size]
+// [type_name][entry]. `record_size` is declared honestly as the size of
+// [type_size][type_name][entry] unless a test overrides it to exercise a
+// mismatched/corrupt size.
+void AppendRecord(std::vector<uint8_t> §ion, uint64_t version,
+ llvm::StringRef type_name,
+ llvm::ArrayRef<uint8_t> entry,
+ std::optional<uint64_t> record_size_override = {}) {
+ std::vector<uint8_t> body;
+ AppendULEB(body, type_name.size());
+ AppendBytes(body, type_name);
+ AppendBytes(body, entry);
+
+ AppendULEB(section, version);
+ AppendULEB(section, record_size_override.value_or(body.size()));
+ AppendBytes(section, llvm::ArrayRef<uint8_t>(body));
+}
+
+std::string ToHex(llvm::ArrayRef<uint8_t> bytes) {
+ static const char digits[] = "0123456789ABCDEF";
+ std::string hex;
+ hex.reserve(bytes.size() * 2);
+ for (uint8_t b : bytes) {
+ hex.push_back(digits[b >> 4]);
+ hex.push_back(digits[b & 0xF]);
+ }
+ return hex;
+}
+
+// Builds a minimal ELF with a single section named `section_name` whose
+// contents are exactly `content` (no implicit padding).
+std::string BuildSectionYaml(llvm::StringRef section_name,
+ llvm::ArrayRef<uint8_t> content) {
+ return ("--- !ELF\n"
+ "FileHeader:\n"
+ " Class: ELFCLASS64\n"
+ " Data: ELFDATA2LSB\n"
+ " Type: ET_DYN\n"
+ " Machine: EM_X86_64\n"
+ "Sections:\n"
+ " - Name: " +
+ section_name.str() +
+ "\n"
+ " Type: SHT_PROGBITS\n"
+ " Flags: [ ]\n"
+ " Address: 0x2010\n"
+ " AddressAlign: 0x10\n"
+ " Content: " +
+ ToHex(content) +
+ "\n"
+ " Size: " +
+ std::to_string(content.size()) +
+ "\n"
+ "...\n");
+}
+
+} // namespace
+
+namespace {
+
struct MockProcess : Process {
MockProcess(TargetSP target_sp, ListenerSP listener_sp)
: Process(target_sp, listener_sp) {}
@@ -54,6 +137,15 @@ struct MockProcess : Process {
class FormatterSectionTest : public ::testing::Test {
public:
void SetUp() override {
+ // The "default" category lives in a process-wide FormatManager, so start
+ // each test from a clean slate regardless of what earlier tests in this
+ // binary registered.
+ TypeCategoryImplSP category;
+ DataVisualization::Categories::GetCategory(ConstString("default"),
+ category);
+ if (category)
+ category->Clear();
+
ArchSpec arch("x86_64-pc-linux");
Platform::SetHostPlatform(
platform_linux::PlatformLinux::CreateInstance(true, &arch));
@@ -142,3 +234,256 @@ TEST_F(FormatterSectionTest, LoadFormattersForModule) {
rect_summary_sp->FormatObject(valobj.get(), dest, TypeSummaryOptions()));
ASSERT_EQ(dest, "BBBBB");
}
+
+/// A lone continuation byte (high bit set) is not a complete ULEB128 value,
+/// so even the leading version number can't be decoded. This must not read
+/// out of bounds or crash.
+TEST_F(FormatterSectionTest, MalformedULEBAtStart) {
+ std::vector<uint8_t> section = {0x80};
+
+ auto ExpectedFile =
+ TestFile::fromYaml(BuildSectionYaml(".lldbformatters", section));
+ ASSERT_THAT_EXPECTED(ExpectedFile, llvm::Succeeded());
+ auto module_sp = std::make_shared<Module>(ExpectedFile->moduleSpec());
+
+ LoadFormattersForModule(module_sp);
+
+ TypeCategoryImplSP category;
+ DataVisualization::Categories::GetCategory(ConstString("default"), category);
+ ASSERT_TRUE(category != nullptr);
+ EXPECT_EQ(category->GetCount(), 0u);
+}
+
+/// A record whose version isn't 1 is unsupported and should be skipped over
+/// (using its honestly-declared record_size) without disturbing a
+/// well-formed record that follows it.
+TEST_F(FormatterSectionTest, SkipsRecordWithUnsupportedVersion) {
+ std::vector<uint8_t> entry;
+ AppendULEB(entry, /*flags=*/0);
+ entry.push_back(FormatterBytecode::Signatures::sig_summary);
+ AppendULEB(entry, /*bytecode_size=*/2);
+ AppendBytes(entry, llvm::ArrayRef<uint8_t>({0xAA, 0xBB}));
+
+ std::vector<uint8_t> section;
+ AppendRecord(section, /*version=*/2, "Bogus", entry);
+ AppendRecord(section, /*version=*/1, "Good", entry);
+
+ auto ExpectedFile =
+ TestFile::fromYaml(BuildSectionYaml(".lldbformatters", section));
+ ASSERT_THAT_EXPECTED(ExpectedFile, llvm::Succeeded());
+ auto module_sp = std::make_shared<Module>(ExpectedFile->moduleSpec());
+
+ LoadFormattersForModule(module_sp);
+
+ TypeCategoryImplSP category;
+ DataVisualization::Categories::GetCategory(ConstString("default"), category);
+ ASSERT_TRUE(category != nullptr);
+ EXPECT_EQ(category->GetSummaryForType(std::make_shared<TypeNameSpecifierImpl>(
+ "Bogus", lldb::eFormatterMatchExact)),
+ nullptr);
+ EXPECT_NE(category->GetSummaryForType(std::make_shared<TypeNameSpecifierImpl>(
+ "Good", lldb::eFormatterMatchExact)),
+ nullptr);
+}
+
+/// The record declares a type name of length 10, but the record itself
+/// (honestly sized by the outer record_size field) only has room for 3
+/// bytes of name and nothing else. The type name read must fail cleanly
+/// instead of reading past the record's bounds, and a well-formed record
+/// that follows must still be reached.
+TEST_F(FormatterSectionTest, InnerTypeSizeExceedsRecordBounds) {
+ std::vector<uint8_t> body;
+ AppendULEB(body, /*type_size=*/10);
+ AppendBytes(body, llvm::StringRef("Foo"));
+
+ std::vector<uint8_t> section;
+ AppendULEB(section, /*version=*/1);
+ AppendULEB(section, /*record_size=*/body.size());
+ AppendBytes(section, llvm::ArrayRef<uint8_t>(body));
+
+ std::vector<uint8_t> entry;
+ AppendULEB(entry, /*flags=*/0);
+ entry.push_back(FormatterBytecode::Signatures::sig_summary);
+ AppendULEB(entry, /*bytecode_size=*/2);
+ AppendBytes(entry, llvm::ArrayRef<uint8_t>({0xAA, 0xBB}));
+ AppendRecord(section, /*version=*/1, "Good", entry);
+
+ auto ExpectedFile =
+ TestFile::fromYaml(BuildSectionYaml(".lldbformatters", section));
+ ASSERT_THAT_EXPECTED(ExpectedFile, llvm::Succeeded());
+ auto module_sp = std::make_shared<Module>(ExpectedFile->moduleSpec());
+
+ LoadFormattersForModule(module_sp);
+
+ TypeCategoryImplSP category;
+ DataVisualization::Categories::GetCategory(ConstString("default"), category);
+ ASSERT_TRUE(category != nullptr);
+ EXPECT_EQ(category->GetCount(), 1u);
+ EXPECT_NE(category->GetSummaryForType(std::make_shared<TypeNameSpecifierImpl>(
+ "Good", lldb::eFormatterMatchExact)),
+ nullptr);
+}
+
+/// A record_size far larger than the number of bytes actually remaining in
+/// the section is an internally-inconsistent (corrupt/truncated) record: its
+/// own declared size can't be trusted to locate the next record, so it must
+/// be rejected rather than silently parsed from whatever bytes happen to be
+/// left. A well-formed record preceding it is unaffected.
+TEST_F(FormatterSectionTest, RecordSizeExceedsRemainingSectionIsRejected) {
+ std::vector<uint8_t> entry;
+ AppendULEB(entry, /*flags=*/0);
+ entry.push_back(FormatterBytecode::Signatures::sig_summary);
+ AppendULEB(entry, /*bytecode_size=*/2);
+ AppendBytes(entry, llvm::ArrayRef<uint8_t>({0xAA, 0xBB}));
+
+ std::vector<uint8_t> section;
+ AppendRecord(section, /*version=*/1, "Good", entry);
+ AppendRecord(section, /*version=*/1, "Oversized", entry,
+ /*record_size_override=*/1000000);
+
+ auto ExpectedFile =
+ TestFile::fromYaml(BuildSectionYaml(".lldbformatters", section));
+ ASSERT_THAT_EXPECTED(ExpectedFile, llvm::Succeeded());
+ auto module_sp = std::make_shared<Module>(ExpectedFile->moduleSpec());
+
+ LoadFormattersForModule(module_sp);
+
+ TypeCategoryImplSP category;
+ DataVisualization::Categories::GetCategory(ConstString("default"), category);
+ ASSERT_TRUE(category != nullptr);
+ EXPECT_EQ(category->GetCount(), 1u);
+ EXPECT_NE(category->GetSummaryForType(std::make_shared<TypeNameSpecifierImpl>(
+ "Good", lldb::eFormatterMatchExact)),
+ nullptr);
+ EXPECT_EQ(category->GetSummaryForType(std::make_shared<TypeNameSpecifierImpl>(
+ "Oversized", lldb::eFormatterMatchExact)),
+ nullptr);
+}
+
+/// An unrecognized signature byte (with otherwise well-formed size/bytecode
+/// framing) is logged and skipped without preventing a later, valid
+/// signature in the same entry from being picked up.
+TEST_F(FormatterSectionTest, UnsupportedSignatureByteIsSkippedWithinEntry) {
+ std::vector<uint8_t> entry;
+ AppendULEB(entry, /*flags=*/0);
+ entry.push_back(0xFF);
+ AppendULEB(entry, /*size=*/2);
+ AppendBytes(entry, llvm::ArrayRef<uint8_t>({0x11, 0x22}));
+ entry.push_back(FormatterBytecode::Signatures::sig_summary);
+ AppendULEB(entry, /*size=*/2);
+ AppendBytes(entry, llvm::ArrayRef<uint8_t>({0xAA, 0xBB}));
+
+ std::vector<uint8_t> section;
+ AppendRecord(section, /*version=*/1, "Widget", entry);
+
+ auto ExpectedFile =
+ TestFile::fromYaml(BuildSectionYaml(".lldbformatters", section));
+ ASSERT_THAT_EXPECTED(ExpectedFile, llvm::Succeeded());
+ auto module_sp = std::make_shared<Module>(ExpectedFile->moduleSpec());
+
+ LoadFormattersForModule(module_sp);
+
+ TypeCategoryImplSP category;
+ DataVisualization::Categories::GetCategory(ConstString("default"), category);
+ ASSERT_TRUE(category != nullptr);
+ EXPECT_NE(category->GetSummaryForType(std::make_shared<TypeNameSpecifierImpl>(
+ "Widget", lldb::eFormatterMatchExact)),
+ nullptr);
+}
+
+/// The declared bytecode size (500) is far larger than the 0 bytes that
+/// actually remain in the entry, so reading it fails cleanly rather than
+/// reading out of bounds. Since no summary and no synthetic method was
+/// successfully parsed, nothing should be registered for the type.
+TEST_F(FormatterSectionTest, TruncatedBytecodeSizeAbortsEntryParsing) {
+ std::vector<uint8_t> entry;
+ AppendULEB(entry, /*flags=*/0);
+ entry.push_back(FormatterBytecode::Signatures::sig_init);
+ AppendULEB(entry, /*size=*/500);
+
+ std::vector<uint8_t> section;
+ AppendRecord(section, /*version=*/1, "Broken", entry);
+
+ auto ExpectedFile =
+ TestFile::fromYaml(BuildSectionYaml(".lldbformatters", section));
+ ASSERT_THAT_EXPECTED(ExpectedFile, llvm::Succeeded());
+ auto module_sp = std::make_shared<Module>(ExpectedFile->moduleSpec());
+
+ LoadFormattersForModule(module_sp);
+
+ TypeCategoryImplSP category;
+ DataVisualization::Categories::GetCategory(ConstString("default"), category);
+ ASSERT_TRUE(category != nullptr);
+ EXPECT_EQ(category->GetCount(), 0u);
+ EXPECT_EQ(category->GetSyntheticForType(std::make_shared<TypeNameSpecifierImpl>(
+ "Broken", lldb::eFormatterMatchExact)),
+ nullptr);
+}
+
+/// An entry that has flags but no summary or synthetic-method sub-entries
+/// at all (valid framing, just empty) must not register a formatter either.
+TEST_F(FormatterSectionTest, EmptyEntryRegistersNothing) {
+ std::vector<uint8_t> entry;
+ AppendULEB(entry, /*flags=*/0);
+
+ std::vector<uint8_t> section;
+ AppendRecord(section, /*version=*/1, "Empty", entry);
+
+ auto ExpectedFile =
+ TestFile::fromYaml(BuildSectionYaml(".lldbformatters", section));
+ ASSERT_THAT_EXPECTED(ExpectedFile, llvm::Succeeded());
+ auto module_sp = std::make_shared<Module>(ExpectedFile->moduleSpec());
+
+ LoadFormattersForModule(module_sp);
+
+ TypeCategoryImplSP category;
+ DataVisualization::Categories::GetCategory(ConstString("default"), category);
+ ASSERT_TRUE(category != nullptr);
+ EXPECT_EQ(category->GetCount(), 0u);
+}
+
+/// An embedded type summary with an empty summary string is dropped instead
+/// of being registered.
+TEST_F(FormatterSectionTest, EmptySummaryStringIsNotRegistered) {
+ std::vector<uint8_t> entry;
+ AppendULEB(entry, /*summary_size=*/0);
+
+ std::vector<uint8_t> section;
+ AppendRecord(section, /*version=*/1, "Empty", entry);
+
+ auto ExpectedFile =
+ TestFile::fromYaml(BuildSectionYaml(".lldbsummaries", section));
+ ASSERT_THAT_EXPECTED(ExpectedFile, llvm::Succeeded());
+ auto module_sp = std::make_shared<Module>(ExpectedFile->moduleSpec());
+
+ LoadTypeSummariesForModule(module_sp);
+
+ TypeCategoryImplSP category;
+ DataVisualization::Categories::GetCategory(ConstString("default"), category);
+ ASSERT_TRUE(category != nullptr);
+ EXPECT_EQ(category->GetCount(), 0u);
+}
+
+/// A declared summary_size larger than the bytes actually available in the
+/// entry must fail cleanly instead of reading out of bounds, and the
+/// summary must not be registered.
+TEST_F(FormatterSectionTest, SummarySizeExceedsAvailableBytes) {
+ std::vector<uint8_t> entry;
+ AppendULEB(entry, /*summary_size=*/50);
+ AppendBytes(entry, llvm::StringRef("short"));
+
+ std::vector<uint8_t> section;
+ AppendRecord(section, /*version=*/1, "Oops", entry);
+
+ auto ExpectedFile =
+ TestFile::fromYaml(BuildSectionYaml(".lldbsummaries", section));
+ ASSERT_THAT_EXPECTED(ExpectedFile, llvm::Succeeded());
+ auto module_sp = std::make_shared<Module>(ExpectedFile->moduleSpec());
+
+ LoadTypeSummariesForModule(module_sp);
+
+ TypeCategoryImplSP category;
+ DataVisualization::Categories::GetCategory(ConstString("default"), category);
+ ASSERT_TRUE(category != nullptr);
+ EXPECT_EQ(category->GetCount(), 0u);
+}
>From 04bfbb320a8ccaf5c564b49bb61712762e39bd51 Mon Sep 17 00:00:00 2001
From: Dave Lee <davelee.com at gmail.com>
Date: Fri, 31 Jul 2026 18:18:37 -0700
Subject: [PATCH 2/2] Review updates
---
.../DataFormatters/FormatterSection.cpp | 6 +-
.../data-formatter/bytecode-summary/main.cpp | 4 +-
.../DataFormatter/FormatterSectionTest.cpp | 138 +++++++-----------
3 files changed, 58 insertions(+), 90 deletions(-)
diff --git a/lldb/source/DataFormatters/FormatterSection.cpp b/lldb/source/DataFormatters/FormatterSection.cpp
index c0e495806522c..c1e715ae498b3 100644
--- a/lldb/source/DataFormatters/FormatterSection.cpp
+++ b/lldb/source/DataFormatters/FormatterSection.cpp
@@ -223,9 +223,9 @@ void LoadFormattersForModule(ModuleSP module_sp) {
std::make_shared<BytecodeSyntheticChildren>(std::move(impl));
category->AddTypeSynthetic(type_name, match_type,
synthetic_children_sp);
- LLDB_LOG(GetLog(LLDBLog::DataFormatters),
- "Loaded embedded type synthetic for '{0}' from {1}.",
- type_name, module_sp->GetFileSpec());
+ LLDB_LOG_VERBOSE(GetLog(LLDBLog::DataFormatters),
+ "Loaded embedded type synthetic for '{0}' from {1}.",
+ type_name, module_sp->GetFileSpec());
} else {
LLDB_LOG(GetLog(LLDBLog::DataFormatters),
"No summary or synthetic methods found for '{0}' in {1}, "
diff --git a/lldb/test/API/functionalities/data-formatter/bytecode-summary/main.cpp b/lldb/test/API/functionalities/data-formatter/bytecode-summary/main.cpp
index eba0f57149f73..8508813edd822 100644
--- a/lldb/test/API/functionalities/data-formatter/bytecode-summary/main.cpp
+++ b/lldb/test/API/functionalities/data-formatter/bytecode-summary/main.cpp
@@ -34,13 +34,13 @@ int main(int argc, char **argv) {
__attribute__((used, section(FORMATTER_SECTION)))
unsigned char _MyOptional_type_summary[] =
"\x01" // version
- "\xa4" // record size
+ "\xa2" // record size
"\x01" // record size
"\x10" // type name size
"^MyOptional<.+>$" // type name
"\x00" // flags
"\x00" // sig_summary
- "\x8e" // program size
+ "\x8d" // program size
"\x01" // program size
"\x1\x22\x7Storage#\x12\x60\x1,C\x10\x1\x5\x11\x2\x1\x22\x6hasVal#"
"\x12\x60\x1,\x10\x1e\x2\x22\x1b<could not read MyOptional>\x10G#!\x60 "
diff --git a/lldb/unittests/DataFormatter/FormatterSectionTest.cpp b/lldb/unittests/DataFormatter/FormatterSectionTest.cpp
index 4ee0bf8e68c66..35a24bef7b0aa 100644
--- a/lldb/unittests/DataFormatter/FormatterSectionTest.cpp
+++ b/lldb/unittests/DataFormatter/FormatterSectionTest.cpp
@@ -20,6 +20,8 @@
#include "lldb/Host/HostInfo.h"
#include "lldb/Target/Platform.h"
#include "lldb/ValueObject/ValueObjectConstResult.h"
+#include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/StringExtras.h"
#include "llvm/Support/LEB128.h"
#include "gtest/gtest.h"
#include <optional>
@@ -29,34 +31,25 @@
using namespace lldb;
using namespace lldb_private;
-namespace {
-
-// --- Helpers for hand-assembling the embedded formatter/summary record
-// format read by FormatterSection.cpp, so malformed inputs can be expressed
-// as typed fields instead of raw hex blobs. ---
-
-void AppendULEB(std::vector<uint8_t> &bytes, uint64_t value) {
- uint8_t buf[10];
- unsigned len = llvm::encodeULEB128(value, buf);
- bytes.insert(bytes.end(), buf, buf + len);
-}
+// Helpers for building bytecode formatter records, embedded into a binary and
+// then read by LoadFormattersForModule.
-void AppendBytes(std::vector<uint8_t> &bytes, llvm::StringRef data) {
+template <typename T>
+static void AppendBytes(std::vector<uint8_t> &bytes, T data) {
bytes.insert(bytes.end(), data.begin(), data.end());
}
-void AppendBytes(std::vector<uint8_t> &bytes, llvm::ArrayRef<uint8_t> data) {
- bytes.insert(bytes.end(), data.begin(), data.end());
+static void AppendULEB(std::vector<uint8_t> &bytes, uint64_t value) {
+ uint8_t buf[10];
+ unsigned len = llvm::encodeULEB128(value, buf);
+ AppendBytes(bytes, llvm::ArrayRef(buf, len));
}
-// Appends one length-framed record: [version][record_size][type_size]
-// [type_name][entry]. `record_size` is declared honestly as the size of
-// [type_size][type_name][entry] unless a test overrides it to exercise a
-// mismatched/corrupt size.
-void AppendRecord(std::vector<uint8_t> §ion, uint64_t version,
- llvm::StringRef type_name,
- llvm::ArrayRef<uint8_t> entry,
- std::optional<uint64_t> record_size_override = {}) {
+/// Append a bytecode formatter record to a section.
+static void AppendRecord(std::vector<uint8_t> §ion, uint64_t version,
+ llvm::StringRef type_name,
+ llvm::ArrayRef<uint8_t> entry,
+ std::optional<uint64_t> record_size_override = {}) {
std::vector<uint8_t> body;
AppendULEB(body, type_name.size());
AppendBytes(body, type_name);
@@ -67,21 +60,10 @@ void AppendRecord(std::vector<uint8_t> §ion, uint64_t version,
AppendBytes(section, llvm::ArrayRef<uint8_t>(body));
}
-std::string ToHex(llvm::ArrayRef<uint8_t> bytes) {
- static const char digits[] = "0123456789ABCDEF";
- std::string hex;
- hex.reserve(bytes.size() * 2);
- for (uint8_t b : bytes) {
- hex.push_back(digits[b >> 4]);
- hex.push_back(digits[b & 0xF]);
- }
- return hex;
-}
-
-// Builds a minimal ELF with a single section named `section_name` whose
-// contents are exactly `content` (no implicit padding).
-std::string BuildSectionYaml(llvm::StringRef section_name,
- llvm::ArrayRef<uint8_t> content) {
+/// Build a minimal ELF binary with a single named section with the given
+/// contents.
+static std::string BuildBinaryYaml(llvm::StringRef section_name,
+ llvm::ArrayRef<uint8_t> content) {
return ("--- !ELF\n"
"FileHeader:\n"
" Class: ELFCLASS64\n"
@@ -97,7 +79,7 @@ std::string BuildSectionYaml(llvm::StringRef section_name,
" Address: 0x2010\n"
" AddressAlign: 0x10\n"
" Content: " +
- ToHex(content) +
+ llvm::toHex(content) +
"\n"
" Size: " +
std::to_string(content.size()) +
@@ -105,8 +87,6 @@ std::string BuildSectionYaml(llvm::StringRef section_name,
"...\n");
}
-} // namespace
-
namespace {
struct MockProcess : Process {
@@ -142,7 +122,7 @@ class FormatterSectionTest : public ::testing::Test {
// binary registered.
TypeCategoryImplSP category;
DataVisualization::Categories::GetCategory(ConstString("default"),
- category);
+ category);
if (category)
category->Clear();
@@ -235,14 +215,13 @@ TEST_F(FormatterSectionTest, LoadFormattersForModule) {
ASSERT_EQ(dest, "BBBBB");
}
-/// A lone continuation byte (high bit set) is not a complete ULEB128 value,
-/// so even the leading version number can't be decoded. This must not read
-/// out of bounds or crash.
+/// Test an invalid leading version number can't be decoded.
TEST_F(FormatterSectionTest, MalformedULEBAtStart) {
+ // A lone continuation byte (high bit set) is not a complete ULEB128 value.
std::vector<uint8_t> section = {0x80};
auto ExpectedFile =
- TestFile::fromYaml(BuildSectionYaml(".lldbformatters", section));
+ TestFile::fromYaml(BuildBinaryYaml(".lldbformatters", section));
ASSERT_THAT_EXPECTED(ExpectedFile, llvm::Succeeded());
auto module_sp = std::make_shared<Module>(ExpectedFile->moduleSpec());
@@ -254,9 +233,7 @@ TEST_F(FormatterSectionTest, MalformedULEBAtStart) {
EXPECT_EQ(category->GetCount(), 0u);
}
-/// A record whose version isn't 1 is unsupported and should be skipped over
-/// (using its honestly-declared record_size) without disturbing a
-/// well-formed record that follows it.
+/// A record whose version isn't 1 is unsupported and should be skipped.
TEST_F(FormatterSectionTest, SkipsRecordWithUnsupportedVersion) {
std::vector<uint8_t> entry;
AppendULEB(entry, /*flags=*/0);
@@ -269,7 +246,7 @@ TEST_F(FormatterSectionTest, SkipsRecordWithUnsupportedVersion) {
AppendRecord(section, /*version=*/1, "Good", entry);
auto ExpectedFile =
- TestFile::fromYaml(BuildSectionYaml(".lldbformatters", section));
+ TestFile::fromYaml(BuildBinaryYaml(".lldbformatters", section));
ASSERT_THAT_EXPECTED(ExpectedFile, llvm::Succeeded());
auto module_sp = std::make_shared<Module>(ExpectedFile->moduleSpec());
@@ -286,13 +263,10 @@ TEST_F(FormatterSectionTest, SkipsRecordWithUnsupportedVersion) {
nullptr);
}
-/// The record declares a type name of length 10, but the record itself
-/// (honestly sized by the outer record_size field) only has room for 3
-/// bytes of name and nothing else. The type name read must fail cleanly
-/// instead of reading past the record's bounds, and a well-formed record
-/// that follows must still be reached.
-TEST_F(FormatterSectionTest, InnerTypeSizeExceedsRecordBounds) {
+/// Test mismatch of decalred type name size and actual length of type name.
+TEST_F(FormatterSectionTest, TypeNameSizeExceedsLengthOfTypeName) {
std::vector<uint8_t> body;
+ // Declare a type name of incorrect length (name: "Foo", length: 10).
AppendULEB(body, /*type_size=*/10);
AppendBytes(body, llvm::StringRef("Foo"));
@@ -309,7 +283,7 @@ TEST_F(FormatterSectionTest, InnerTypeSizeExceedsRecordBounds) {
AppendRecord(section, /*version=*/1, "Good", entry);
auto ExpectedFile =
- TestFile::fromYaml(BuildSectionYaml(".lldbformatters", section));
+ TestFile::fromYaml(BuildBinaryYaml(".lldbformatters", section));
ASSERT_THAT_EXPECTED(ExpectedFile, llvm::Succeeded());
auto module_sp = std::make_shared<Module>(ExpectedFile->moduleSpec());
@@ -324,11 +298,7 @@ TEST_F(FormatterSectionTest, InnerTypeSizeExceedsRecordBounds) {
nullptr);
}
-/// A record_size far larger than the number of bytes actually remaining in
-/// the section is an internally-inconsistent (corrupt/truncated) record: its
-/// own declared size can't be trusted to locate the next record, so it must
-/// be rejected rather than silently parsed from whatever bytes happen to be
-/// left. A well-formed record preceding it is unaffected.
+// Test that a record does not extend past the section it is within.
TEST_F(FormatterSectionTest, RecordSizeExceedsRemainingSectionIsRejected) {
std::vector<uint8_t> entry;
AppendULEB(entry, /*flags=*/0);
@@ -339,10 +309,10 @@ TEST_F(FormatterSectionTest, RecordSizeExceedsRemainingSectionIsRejected) {
std::vector<uint8_t> section;
AppendRecord(section, /*version=*/1, "Good", entry);
AppendRecord(section, /*version=*/1, "Oversized", entry,
- /*record_size_override=*/1000000);
+ /*record_size_override=*/1000000);
auto ExpectedFile =
- TestFile::fromYaml(BuildSectionYaml(".lldbformatters", section));
+ TestFile::fromYaml(BuildBinaryYaml(".lldbformatters", section));
ASSERT_THAT_EXPECTED(ExpectedFile, llvm::Succeeded());
auto module_sp = std::make_shared<Module>(ExpectedFile->moduleSpec());
@@ -360,12 +330,11 @@ TEST_F(FormatterSectionTest, RecordSizeExceedsRemainingSectionIsRejected) {
nullptr);
}
-/// An unrecognized signature byte (with otherwise well-formed size/bytecode
-/// framing) is logged and skipped without preventing a later, valid
-/// signature in the same entry from being picked up.
-TEST_F(FormatterSectionTest, UnsupportedSignatureByteIsSkippedWithinEntry) {
+// Test that an unrecognized signature skips the current formatter entry.
+TEST_F(FormatterSectionTest, UnsupportedSignatureSkipsEntry) {
std::vector<uint8_t> entry;
AppendULEB(entry, /*flags=*/0);
+ // Invalid signature.
entry.push_back(0xFF);
AppendULEB(entry, /*size=*/2);
AppendBytes(entry, llvm::ArrayRef<uint8_t>({0x11, 0x22}));
@@ -377,7 +346,7 @@ TEST_F(FormatterSectionTest, UnsupportedSignatureByteIsSkippedWithinEntry) {
AppendRecord(section, /*version=*/1, "Widget", entry);
auto ExpectedFile =
- TestFile::fromYaml(BuildSectionYaml(".lldbformatters", section));
+ TestFile::fromYaml(BuildBinaryYaml(".lldbformatters", section));
ASSERT_THAT_EXPECTED(ExpectedFile, llvm::Succeeded());
auto module_sp = std::make_shared<Module>(ExpectedFile->moduleSpec());
@@ -391,21 +360,19 @@ TEST_F(FormatterSectionTest, UnsupportedSignatureByteIsSkippedWithinEntry) {
nullptr);
}
-/// The declared bytecode size (500) is far larger than the 0 bytes that
-/// actually remain in the entry, so reading it fails cleanly rather than
-/// reading out of bounds. Since no summary and no synthetic method was
-/// successfully parsed, nothing should be registered for the type.
+/// Test a signature body being declared with too large a size.
TEST_F(FormatterSectionTest, TruncatedBytecodeSizeAbortsEntryParsing) {
std::vector<uint8_t> entry;
AppendULEB(entry, /*flags=*/0);
entry.push_back(FormatterBytecode::Signatures::sig_init);
+ // Declared bytecode size is larger than the 0 bytes of the entry.
AppendULEB(entry, /*size=*/500);
std::vector<uint8_t> section;
AppendRecord(section, /*version=*/1, "Broken", entry);
auto ExpectedFile =
- TestFile::fromYaml(BuildSectionYaml(".lldbformatters", section));
+ TestFile::fromYaml(BuildBinaryYaml(".lldbformatters", section));
ASSERT_THAT_EXPECTED(ExpectedFile, llvm::Succeeded());
auto module_sp = std::make_shared<Module>(ExpectedFile->moduleSpec());
@@ -415,13 +382,14 @@ TEST_F(FormatterSectionTest, TruncatedBytecodeSizeAbortsEntryParsing) {
DataVisualization::Categories::GetCategory(ConstString("default"), category);
ASSERT_TRUE(category != nullptr);
EXPECT_EQ(category->GetCount(), 0u);
- EXPECT_EQ(category->GetSyntheticForType(std::make_shared<TypeNameSpecifierImpl>(
- "Broken", lldb::eFormatterMatchExact)),
- nullptr);
+ EXPECT_EQ(
+ category->GetSyntheticForType(std::make_shared<TypeNameSpecifierImpl>(
+ "Broken", lldb::eFormatterMatchExact)),
+ nullptr);
}
-/// An entry that has flags but no summary or synthetic-method sub-entries
-/// at all (valid framing, just empty) must not register a formatter either.
+/// Test that an entry which has flags but neither summary or synthetic
+/// signature (valid framing, but empty) must not register a formatter either.
TEST_F(FormatterSectionTest, EmptyEntryRegistersNothing) {
std::vector<uint8_t> entry;
AppendULEB(entry, /*flags=*/0);
@@ -430,7 +398,7 @@ TEST_F(FormatterSectionTest, EmptyEntryRegistersNothing) {
AppendRecord(section, /*version=*/1, "Empty", entry);
auto ExpectedFile =
- TestFile::fromYaml(BuildSectionYaml(".lldbformatters", section));
+ TestFile::fromYaml(BuildBinaryYaml(".lldbformatters", section));
ASSERT_THAT_EXPECTED(ExpectedFile, llvm::Succeeded());
auto module_sp = std::make_shared<Module>(ExpectedFile->moduleSpec());
@@ -442,8 +410,8 @@ TEST_F(FormatterSectionTest, EmptyEntryRegistersNothing) {
EXPECT_EQ(category->GetCount(), 0u);
}
-/// An embedded type summary with an empty summary string is dropped instead
-/// of being registered.
+/// Test that an embedded type summary with an empty summary string is dropped
+/// instead of being registered.
TEST_F(FormatterSectionTest, EmptySummaryStringIsNotRegistered) {
std::vector<uint8_t> entry;
AppendULEB(entry, /*summary_size=*/0);
@@ -452,7 +420,7 @@ TEST_F(FormatterSectionTest, EmptySummaryStringIsNotRegistered) {
AppendRecord(section, /*version=*/1, "Empty", entry);
auto ExpectedFile =
- TestFile::fromYaml(BuildSectionYaml(".lldbsummaries", section));
+ TestFile::fromYaml(BuildBinaryYaml(".lldbsummaries", section));
ASSERT_THAT_EXPECTED(ExpectedFile, llvm::Succeeded());
auto module_sp = std::make_shared<Module>(ExpectedFile->moduleSpec());
@@ -464,8 +432,8 @@ TEST_F(FormatterSectionTest, EmptySummaryStringIsNotRegistered) {
EXPECT_EQ(category->GetCount(), 0u);
}
-/// A declared summary_size larger than the bytes actually available in the
-/// entry must fail cleanly instead of reading out of bounds, and the
+/// Test that a declared summary size larger than the bytes actually available
+/// in the entry must fail cleanly instead of reading out of bounds, and the
/// summary must not be registered.
TEST_F(FormatterSectionTest, SummarySizeExceedsAvailableBytes) {
std::vector<uint8_t> entry;
@@ -476,7 +444,7 @@ TEST_F(FormatterSectionTest, SummarySizeExceedsAvailableBytes) {
AppendRecord(section, /*version=*/1, "Oops", entry);
auto ExpectedFile =
- TestFile::fromYaml(BuildSectionYaml(".lldbsummaries", section));
+ TestFile::fromYaml(BuildBinaryYaml(".lldbsummaries", section));
ASSERT_THAT_EXPECTED(ExpectedFile, llvm::Succeeded());
auto module_sp = std::make_shared<Module>(ExpectedFile->moduleSpec());
More information about the lldb-commits
mailing list