[lldb] reject `.debug_arange` sections with nonzero segment size

[ldrumm]

Summary

If a producer emits a nonzero segment size, `lldb` will silently read
incorrect values and crash, or do something worse later, as the tuple
size is expected to be 2, rather than 3.

Neither LLVM, nor GCC produce segmented aranges, but this dangerous case
should still be checked and handled.

[clayborg]

Change looks good, just needs a test. Should be easy to take a simple binary that has a .debug_aranges, and run obj2yaml on it, and tweak the segment size as needed?

[labath]

In D75925#1915693, @clayborg wrote:

Change looks good, just needs a test. Should be easy to take a simple binary that has a .debug_aranges, and run obj2yaml on it, and tweak the segment size as needed?

In this case I think the cleanest solution would be to write a c++ unit test: create a simple debug_aranges header (just hardcode bytes, nothing fancy), pass it to DWARFDebugArangesSet::extract, and check the result.

[ldrumm]

I found similar unittests for other DWARF entries which allow me to check more about the parser state, so I went with Pavel's suggestion for the testcase

[clayborg]

lgtm once all formatting issues are taken care of. Pavel?

[labath]

Looks good, just run clang-format before committing.

lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugArangeSet.cpp

llvm::Error DWARFDebugArangeSet::extract(const DWARFDataExtractor &data,
   m_header.cu_offset = data.GetDWARFOffset(offset_ptr);
   m_header.addr_size = data.GetU8(offset_ptr);
   m_header.seg_size = data.GetU8(offset_ptr);
 
   // Try to avoid reading invalid arange sets by making sure:
   // 1 - the version looks good
   // 2 - the address byte size looks plausible
   // 3 - the length seems to make sense
-  // size looks plausible
+  // 4 - size looks plausible
+  // 5 - the arange tuples do not contain a segment field
   if (m_header.version < 2 || m_header.version > 5)
     return llvm::make_error<llvm::object::GenericBinaryError>(
         "Invalid arange header version");
 
   if (m_header.addr_size != 4 && m_header.addr_size != 8)
     return llvm::make_error<llvm::object::GenericBinaryError>(
         "Invalid arange header address size");
 
   if (m_header.length == 0)
     return llvm::make_error<llvm::object::GenericBinaryError>(
         "Invalid arange header length");
 
   if (!data.ValidOffset(m_offset + sizeof(m_header.length) + m_header.length -
                         1))
     return llvm::make_error<llvm::object::GenericBinaryError>(
         "Invalid arange header length");
 
+  if (m_header.seg_size)
+    return llvm::make_error<llvm::object::GenericBinaryError>(
+        "segmented arange entries are not supported");
+
   // The first tuple following the header in each set begins at an offset
   // that is a multiple of the size of a single tuple (that is, twice the
   // size of an address). The header is padded, if necessary, to the
   // appropriate boundary.
   const uint32_t header_size = *offset_ptr - m_offset;
   const uint32_t tuple_size = m_header.addr_size << 1;
   uint32_t first_tuple_offset = 0;
   while (first_tuple_offset < header_size)

lldb/unittests/SymbolFile/DWARF/SymbolFileDWARFTests.cpp

 #include "llvm/DebugInfo/PDB/PDBSymbolExe.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Path.h"
 
 #include "Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h"
 #include "Plugins/SymbolFile/DWARF/DWARFAbbreviationDeclaration.h"
 #include "Plugins/SymbolFile/DWARF/DWARFDataExtractor.h"
 #include "Plugins/SymbolFile/DWARF/DWARFDebugAbbrev.h"
+#include "Plugins/SymbolFile/DWARF/DWARFDebugArangeSet.h"
 #include "Plugins/SymbolFile/DWARF/SymbolFileDWARF.h"
 #include "Plugins/SymbolFile/PDB/SymbolFilePDB.h"
 #include "Plugins/TypeSystem/Clang/TypeSystemClang.h"
 #include "TestingSupport/SubsystemRAII.h"
 #include "TestingSupport/TestUtilities.h"
 #include "lldb/Core/Address.h"
 #include "lldb/Core/Module.h"
 #include "lldb/Core/ModuleSpec.h"
TEST_F(SymbolFileDWARFTests, TestAbbrevMissingTerminator) {
   DWARFAbbreviationDeclarationSet abbrev_set;
   lldb::offset_t data_offset = 0;
   llvm::Error error = abbrev_set.extract(data, &data_offset);
   // Verify we get an error
   EXPECT_TRUE(bool(error));
   EXPECT_EQ("abbreviation declaration attribute list not terminated with a "
             "null entry", llvm::toString(std::move(error)));
 }
+
+TEST_F(SymbolFileDWARFTests, ParseArangesNonzeroSegmentSize) {
+  // This `.debug_aranges` table header is a valid 32bit big-endian section
+  // according to the DWARFv5 spec:6.2.1, but contains segment selectors which
+  // are not supported by lldb, and should be gracefully rejected
+  const unsigned char binary_data[] = {
+    0, 0, 0, 41, // unit_length (length field not including this field itself)
+    0, 2, // DWARF version number (half)
+    0, 0, 0, 0, // offset into the .debug_info_table (ignored for the purposes of this test
+    4, // address size
+    1, // segment size
+    // alignment for the first tuple which "begins at an offset that is a
+    // multiple of the size of a single tuple". Tuples are nine bytes in this example.
+    0, 0, 0, 0, 0, 0,
+    // BEGIN TUPLES
+    1, 0, 0, 0, 4, 0, 0, 0, 1, // a 1byte object starting at address 4 in segment 1
+    0, 0, 0, 0, 4, 0, 0, 0, 1, // a 1byte object starting at address 4 in segment 0
+    // END TUPLES
+    0, 0, 0, 0, 0, 0, 0, 0, 0 // terminator
+  };
+  DWARFDataExtractor data;
+  data.SetData(static_cast<const void *>(binary_data), sizeof binary_data,
+               lldb::ByteOrder::eByteOrderBig);
+  DWARFDebugArangeSet debug_aranges;
+  offset_t off = 0;
+  llvm::Error error = debug_aranges.extract(data, &off);
+  EXPECT_TRUE(bool(error));
+  EXPECT_EQ("segmented arange entries are not supported",
+            llvm::toString(std::move(error)));
+  EXPECT_EQ(off, 12); // Parser should read no further than the segment size
+}