Author: Jonas Devlieghere Date: 2026-06-25T08:16:26-07:00 New Revision: 77b0b29955125c6caae1f88f710c4b3b41a0d37f
URL: https://github.com/llvm/llvm-project/commit/77b0b29955125c6caae1f88f710c4b3b41a0d37f DIFF: https://github.com/llvm/llvm-project/commit/77b0b29955125c6caae1f88f710c4b3b41a0d37f.diff LOG: [lldb] Return an empty extractor for unmappable subset requests (#205596) DataExtractor::GetSubsetExtractorSP always returns a valid, non-null extractor, and every caller relies on that: they dereference the result directly rather than checking for null. The VirtualDataExtractor override broke this contract. When the requested range had no backing in the lookup table (e.g. the virtual offset fell in a gap, or the range crossed an entry boundary) it returned an empty shared_ptr, turning each caller's dereference into a null-pointer access. It also asserted on those cases. But the offsets passed to a subset request are derived from parsed file contents, which may be truncated or inconsistent. A range that cannot be mapped is therefore ordinary input, not a programming error, and must not abort an assertions-enabled build. Return a valid but empty extractor instead. Reads see zero bytes and the caller's existing size checks skip the data, matching the graceful degradation the base class already provides for out-of-range offsets. Assisted-by: Claude Added: Modified: lldb/source/Utility/VirtualDataExtractor.cpp lldb/unittests/Utility/VirtualDataExtractorTest.cpp Removed: ################################################################################ diff --git a/lldb/source/Utility/VirtualDataExtractor.cpp b/lldb/source/Utility/VirtualDataExtractor.cpp index fe7ed963d0bdd..167560bb08344 100644 --- a/lldb/source/Utility/VirtualDataExtractor.cpp +++ b/lldb/source/Utility/VirtualDataExtractor.cpp @@ -246,24 +246,30 @@ uint64_t VirtualDataExtractor::GetU64_unchecked(offset_t *offset_ptr) const { return result; } +/// A subset request that can't be mapped still returns a valid, non-null +/// extractor, because most callers dereference the result directly. +static DataExtractorSP MakeEmptySubsetExtractor(ByteOrder byte_order, + uint32_t addr_size) { + auto empty_sp = std::make_shared<DataExtractor>(); + empty_sp->SetByteOrder(byte_order); + empty_sp->SetAddressByteSize(addr_size); + return empty_sp; +} + DataExtractorSP VirtualDataExtractor::GetSubsetExtractorSP(offset_t virtual_offset, offset_t virtual_length) { const LookupTable::Entry *entry = FindEntry(virtual_offset); - assert( - entry && - "VirtualDataExtractor subset extractor requires valid virtual address"); if (!entry) - return {}; + return MakeEmptySubsetExtractor(GetByteOrder(), GetAddressByteSize()); // Entry::data is the offset into the DataBuffer's actual start/end range // Entry::base is the virtual address at the start of this region of data offset_t offset_into_entry_range = virtual_offset - entry->base; - assert( - offset_into_entry_range + virtual_length <= entry->size && - "VirtualDataExtractor subset may not span multiple LookupTable entries"); + // A subset may not span more than one entry; a range that overflows this + // entry is unmappable. if (offset_into_entry_range + virtual_length > entry->size) - return {}; + return MakeEmptySubsetExtractor(GetByteOrder(), GetAddressByteSize()); // We could support a Subset VirtualDataExtractor which covered // multiple LookupTable virtual entries, but we'd need to mutate @@ -283,11 +289,8 @@ VirtualDataExtractor::GetSubsetExtractorSP(offset_t virtual_offset, DataExtractorSP VirtualDataExtractor::GetSubsetExtractorSP(offset_t virtual_offset) { const LookupTable::Entry *entry = FindEntry(virtual_offset); - assert( - entry && - "VirtualDataExtractor subset extractor requires valid virtual address"); if (!entry) - return {}; + return MakeEmptySubsetExtractor(GetByteOrder(), GetAddressByteSize()); // Entry::data is the offset into the DataBuffer's actual start/end range // Entry::base is the virtual address at the start of this region of data diff --git a/lldb/unittests/Utility/VirtualDataExtractorTest.cpp b/lldb/unittests/Utility/VirtualDataExtractorTest.cpp index 5a1ec9d6332de..89c1f27fdbc87 100644 --- a/lldb/unittests/Utility/VirtualDataExtractorTest.cpp +++ b/lldb/unittests/Utility/VirtualDataExtractorTest.cpp @@ -661,3 +661,40 @@ TEST(VirtualDataExtractorTest, SubsetExtractorGetU32) { // Entry(0x0, 4*sizeof(uint32_t), 12*sizeof(uint32_t)) EXPECT_EQ(extractor->GetU32(&virtual_offset), 0xccccccccU); } + +TEST(VirtualDataExtractorTest, SubsetExtractorUnmappedRangeReturnsEmpty) { + // Two mapped regions with a gap in the virtual address space between them. + uint8_t buffer[] = {0x01, 0x02, 0x03, 0x04, 0x11, 0x12, 0x13, 0x14}; + DataBufferSP buffer_sp = + std::make_shared<DataBufferUnowned>(buffer, sizeof(buffer)); + lldb::DataExtractorSP extractor = std::make_shared<VirtualDataExtractor>( + buffer_sp, eByteOrderLittle, 8, + Table{Entry(0x1000, 4, 0), Entry(0x2000, 4, 4)}); + + // An offset in the gap between entries must not produce a null shared_ptr -- + // callers dereference the result -- so it returns a valid empty extractor. + lldb::DataExtractorSP gap_subset = extractor->GetSubsetExtractorSP(0x1800, 4); + ASSERT_NE(gap_subset, nullptr); + EXPECT_EQ(gap_subset->GetByteSize(), 0U); + + // A range starting inside an entry but running past its end (into the + // unmapped gap) is likewise unsatisfiable and returns an empty extractor. + lldb::DataExtractorSP overrun_subset = + extractor->GetSubsetExtractorSP(0x1002, 8); + ASSERT_NE(overrun_subset, nullptr); + EXPECT_EQ(overrun_subset->GetByteSize(), 0U); + + // The single-argument overload behaves the same for an unmapped offset. + lldb::DataExtractorSP gap_subset_single = + extractor->GetSubsetExtractorSP(0x1800); + ASSERT_NE(gap_subset_single, nullptr); + EXPECT_EQ(gap_subset_single->GetByteSize(), 0U); + + // A fully-contained range still produces a real subset with the right bytes. + lldb::DataExtractorSP valid_subset = + extractor->GetSubsetExtractorSP(0x2000, 4); + ASSERT_NE(valid_subset, nullptr); + EXPECT_EQ(valid_subset->GetByteSize(), 4U); + offset_t physical_offset = 0; + EXPECT_EQ(valid_subset->GetU32(&physical_offset), 0x14131211U); +} _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
