[PATCH] D151755: [DebugInfo] Add error checking around data extraction in DWARFAbbreviationDeclaration::extract

[Alex Langford via Phabricator via llvm-commits]

bulbazord created this revision.
bulbazord added reviewers: JDevlieghere, aprantl, fdeazeve, jhenderson.
Herald added a subscriber: hiraditya.
Herald added a project: All.
bulbazord requested review of this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

In trying to hoist errors further up this callstack, I discovered that
if the data in the debug_abbrev section is invalid entirely, the code
that parses the debug_abbrev section may do strange or unpredictable
things. The underlying issue is that DataExtractor will return a value
of 0 when it encounters an error in extracting a LEB128 value. It's thus
difficult to determine if there was an error just by looking at the
return value. This patch aims to bail at the first sight of an error in
the debug_abbrev parsing code.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D151755

Files:
  llvm/lib/DebugInfo/DWARF/DWARFAbbreviationDeclaration.cpp
  llvm/unittests/DebugInfo/DWARF/DWARFDebugAbbrevTest.cpp


Index: llvm/unittests/DebugInfo/DWARF/DWARFDebugAbbrevTest.cpp
===================================================================
--- llvm/unittests/DebugInfo/DWARF/DWARFDebugAbbrevTest.cpp
+++ llvm/unittests/DebugInfo/DWARF/DWARFDebugAbbrevTest.cpp
@@ -42,6 +42,12 @@
   encodeULEB128(0, OS);
 }
 
+void writeBadAbbreviationDeclarationData(raw_ostream &OS, size_t Size,
+                                         uint32_t Data) {
+  std::vector<uint32_t> BadData((Size / 4) + 1, Data);
+  OS.write(reinterpret_cast<char *>(BadData.data()), Size);
+}
+
 TEST(DWARFDebugAbbrevTest, DWARFAbbrevDeclSetExtractSuccess) {
   SmallString<64> RawData;
   raw_svector_ostream OS(RawData);
@@ -104,3 +110,18 @@
   EXPECT_FALSE(Abbrev1->hasChildren());
   EXPECT_EQ(Abbrev1->getNumAttributes(), 1u);
 }
+
+TEST(DWARFDebugAbbrevTest, DWARFAbbreviationDeclSetBadData) {
+  SmallString<64> RawData;
+  raw_svector_ostream OS(RawData);
+  const uint32_t DataSize = 72; // This number is arbitrary.
+  const uint32_t DataContents = 0xDEADBEEFu;
+
+  writeBadAbbreviationDeclarationData(OS, DataSize, DataContents);
+
+  uint64_t Offset = 0;
+  DataExtractor Data(RawData, sys::IsLittleEndianHost, sizeof(uint64_t));
+  DWARFAbbreviationDeclarationSet AbbrevSet;
+  const bool DataWasExtracted = AbbrevSet.extract(Data, &Offset);
+  EXPECT_FALSE(DataWasExtracted);
+}
Index: llvm/lib/DebugInfo/DWARF/DWARFAbbreviationDeclaration.cpp
===================================================================
--- llvm/lib/DebugInfo/DWARF/DWARFAbbreviationDeclaration.cpp
+++ llvm/lib/DebugInfo/DWARF/DWARFAbbreviationDeclaration.cpp
@@ -38,18 +38,28 @@
 DWARFAbbreviationDeclaration::extract(DataExtractor Data, uint64_t *OffsetPtr) {
   clear();
   const uint64_t Offset = *OffsetPtr;
-  Code = Data.getULEB128(OffsetPtr);
+  Error Err = Error::success();
+  Code = Data.getULEB128(OffsetPtr, &Err);
+  if (Err)
+    return Err;
+
   if (Code == 0)
     return ExtractState::Complete;
 
   CodeByteSize = *OffsetPtr - Offset;
-  Tag = static_cast<llvm::dwarf::Tag>(Data.getULEB128(OffsetPtr));
+  Tag = static_cast<llvm::dwarf::Tag>(Data.getULEB128(OffsetPtr, &Err));
+  if (Err)
+    return Err;
+
   if (Tag == DW_TAG_null) {
     clear();
     return make_error<llvm::object::GenericBinaryError>(
         "abbreviation declaration requires a non-null tag");
   }
-  uint8_t ChildrenByte = Data.getU8(OffsetPtr);
+  uint8_t ChildrenByte = Data.getU8(OffsetPtr, &Err);
+  if (Err)
+    return Err;
+
   HasChildren = (ChildrenByte == DW_CHILDREN_yes);
   // Assign a value to our optional FixedAttributeSize member variable. If
   // this member variable still has a value after the while loop below, then
@@ -58,8 +68,13 @@
 
   // Read all of the abbreviation attributes and forms.
   while (Data.isValidOffset(*OffsetPtr)) {
-    auto A = static_cast<Attribute>(Data.getULEB128(OffsetPtr));
-    auto F = static_cast<Form>(Data.getULEB128(OffsetPtr));
+    auto A = static_cast<Attribute>(Data.getULEB128(OffsetPtr, &Err));
+    if (Err)
+      return Err;
+
+    auto F = static_cast<Form>(Data.getULEB128(OffsetPtr, &Err));
+    if (Err)
+      return Err;
 
     // We successfully reached the end of this abbreviation declaration
     // since both attribute and form are zero. There may be more abbreviation


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D151755.526800.patch
Type: text/x-patch
Size: 3295 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20230530/cf0c0c57/attachment.bin>