[Lldb-commits] [lldb] [lldb][bytecode] Improve testing of bytecode section loading (PR #212568)

[Dave Lee via lldb-commits]

================
@@ -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
----------------
kastiglione wrote:

I removed it. Honest meaning correct/valid, vs a faked/invalid value for the purpose of testing edge cases.

https://github.com/llvm/llvm-project/pull/212568