https://github.com/clayborg created 
https://github.com/llvm/llvm-project/pull/129166

When we don't specify the size of an ELF image when loading from memory, our 
data for the ELF file might end up being truncated to only include the ELF 
header and the program headers. The ObjectFileELF class was able to load 
program header contents, this patch modifies the class so the section headers 
can still be loaded even if they are not included in the data buffer.

>From 4d528d5930e5dfff6e5d832cf9836faf0bbd1871 Mon Sep 17 00:00:00 2001
From: Greg Clayton <clayb...@gmail.com>
Date: Fri, 14 Feb 2025 10:55:06 -0800
Subject: [PATCH 1/2] [lldb] Make ELF files able to load section headers from
 memory.

When we don't specify the size of an ELF image when loading from memory, our 
data for the ELF file might end up being truncated to only include the ELF 
header and the program headers. The ObjectFileELF class was able to load 
program header contents, this patch modifies the class so the section headers 
can still be loaded even if they are not included in the data buffer.
---
 .../source/Plugins/ObjectFile/ELF/ELFHeader.h |   6 +
 .../Plugins/ObjectFile/ELF/ObjectFileELF.cpp  | 133 +++++++++++++-----
 .../Plugins/ObjectFile/ELF/ObjectFileELF.h    |  44 +++++-
 3 files changed, 140 insertions(+), 43 deletions(-)

diff --git a/lldb/source/Plugins/ObjectFile/ELF/ELFHeader.h 
b/lldb/source/Plugins/ObjectFile/ELF/ELFHeader.h
index 963cc850736ff..b35929835c6e0 100644
--- a/lldb/source/Plugins/ObjectFile/ELF/ELFHeader.h
+++ b/lldb/source/Plugins/ObjectFile/ELF/ELFHeader.h
@@ -100,6 +100,12 @@ struct ELFHeader {
   ///    The byte order of this ELF file as described by the header.
   lldb::ByteOrder GetByteOrder() const;
 
+  /// Get the size in bytes of the section header data.
+  ///
+  /// \return
+  ///   The byte size of the section header data.
+  uint64_t GetSectionHeaderByteSize() const { return e_shnum * e_shentsize; }
+
   /// The jump slot relocation type of this ELF.
   unsigned GetRelocationJumpSlotType() const;
 
diff --git a/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp 
b/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
index 13e1198516f78..9225fa9b757bf 100644
--- a/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
+++ b/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
@@ -556,6 +556,28 @@ static bool GetOsFromOSABI(unsigned char osabi_byte,
   return ostype != llvm::Triple::OSType::UnknownOS;
 }
 
+/// Read the bytes for the section headers from the ELF object file data.
+static DataExtractor GetSectionHeadersFromELFData(
+  const elf::ELFHeader &header, const DataExtractor &object_data) {
+  DataExtractor sh_data;
+  const elf_off sh_offset = header.e_shoff;
+  const size_t sh_size = header.GetSectionHeaderByteSize();
+  sh_data.SetData(object_data, sh_offset, sh_size);
+  return sh_data;
+}
+
+/// Read the section data bytes for the section from the ELF object file data.
+bool ObjectFileELF::GetSectionContentsFromELFData(
+  const elf::ELFSectionHeader &sh, const DataExtractor &object_data,
+  lldb_private::DataExtractor &section_data) {
+  if (sh.sh_type == SHT_NOBITS || sh.sh_size == 0) {
+    section_data.Clear();
+    return false;
+  }
+  return section_data.SetData(object_data,
+                              sh.sh_offset, sh.sh_size) == sh.sh_size;
+}
+
 size_t ObjectFileELF::GetModuleSpecifications(
     const lldb_private::FileSpec &file, lldb::DataBufferSP &data_sp,
     lldb::offset_t data_offset, lldb::offset_t file_offset,
@@ -633,9 +655,16 @@ size_t ObjectFileELF::GetModuleSpecifications(
           SectionHeaderColl section_headers;
           lldb_private::UUID &uuid = spec.GetUUID();
 
-          GetSectionHeaderInfo(section_headers, data, header, uuid,
-                               gnu_debuglink_file, gnu_debuglink_crc,
-                               spec.GetArchitecture());
+          // Get the section header data from the object file.
+          DataExtractor sh_data = GetSectionHeadersFromELFData(header, data);
+
+          auto read_sect_callback = [&](const elf::ELFSectionHeader &sh,
+                                        lldb_private::DataExtractor &sh_data) 
-> bool {
+            return GetSectionContentsFromELFData(sh, data, sh_data);
+          };
+          GetSectionHeaderInfo(header, sh_data, section_headers,
+                               read_sect_callback, uuid, gnu_debuglink_file,
+                               gnu_debuglink_crc, spec.GetArchitecture());
 
           llvm::Triple &spec_triple = spec.GetArchitecture().GetTriple();
 
@@ -1284,10 +1313,10 @@ 
ObjectFileELF::RefineModuleDetailsFromNote(lldb_private::DataExtractor &data,
   return error;
 }
 
-void ObjectFileELF::ParseARMAttributes(DataExtractor &data, uint64_t length,
+void ObjectFileELF::ParseARMAttributes(DataExtractor &data,
                                        ArchSpec &arch_spec) {
   lldb::offset_t Offset = 0;
-
+  const uint64_t length = data.GetByteSize();
   uint8_t FormatVersion = data.GetU8(&Offset);
   if (FormatVersion != llvm::ELFAttrs::Format_Version)
     return;
@@ -1355,9 +1384,10 @@ void ObjectFileELF::ParseARMAttributes(DataExtractor 
&data, uint64_t length,
 }
 
 // GetSectionHeaderInfo
-size_t ObjectFileELF::GetSectionHeaderInfo(SectionHeaderColl &section_headers,
-                                           DataExtractor &object_data,
-                                           const elf::ELFHeader &header,
+size_t ObjectFileELF::GetSectionHeaderInfo(const elf::ELFHeader &header,
+                                           const DataExtractor &sh_data,
+                                           SectionHeaderColl &section_headers,
+                                           ReadSectionDataCallback read_sect,
                                            lldb_private::UUID &uuid,
                                            std::string &gnu_debuglink_file,
                                            uint32_t &gnu_debuglink_crc,
@@ -1459,14 +1489,12 @@ size_t 
ObjectFileELF::GetSectionHeaderInfo(SectionHeaderColl &section_headers,
 
   Log *log = GetLog(LLDBLog::Modules);
 
-  section_headers.resize(header.e_shnum);
-  if (section_headers.size() != header.e_shnum)
+  if (sh_data.GetByteSize() != header.GetSectionHeaderByteSize())
     return 0;
 
-  const size_t sh_size = header.e_shnum * header.e_shentsize;
-  const elf_off sh_offset = header.e_shoff;
-  DataExtractor sh_data;
-  if (sh_data.SetData(object_data, sh_offset, sh_size) != sh_size)
+  // Only resize our section headers if we got valid section header data.
+  section_headers.resize(header.e_shnum);
+  if (section_headers.size() != header.e_shnum)
     return 0;
 
   uint32_t idx;
@@ -1481,28 +1509,21 @@ size_t 
ObjectFileELF::GetSectionHeaderInfo(SectionHeaderColl &section_headers,
   const unsigned strtab_idx = header.e_shstrndx;
   if (strtab_idx && strtab_idx < section_headers.size()) {
     const ELFSectionHeaderInfo &sheader = section_headers[strtab_idx];
-    const size_t byte_size = sheader.sh_size;
-    const Elf64_Off offset = sheader.sh_offset;
     lldb_private::DataExtractor shstr_data;
-
-    if (shstr_data.SetData(object_data, offset, byte_size) == byte_size) {
+    if (read_sect(sheader, shstr_data)) {
       for (SectionHeaderCollIter I = section_headers.begin();
            I != section_headers.end(); ++I) {
         static ConstString g_sect_name_gnu_debuglink(".gnu_debuglink");
         const ELFSectionHeaderInfo &sheader = *I;
-        const uint64_t section_size =
-            sheader.sh_type == SHT_NOBITS ? 0 : sheader.sh_size;
         ConstString name(shstr_data.PeekCStr(I->sh_name));
 
         I->section_name = name;
 
         if (arch_spec.IsMIPS()) {
           uint32_t arch_flags = arch_spec.GetFlags();
-          DataExtractor data;
           if (sheader.sh_type == SHT_MIPS_ABIFLAGS) {
-
-            if (section_size && (data.SetData(object_data, sheader.sh_offset,
-                                              section_size) == section_size)) {
+            DataExtractor data;
+            if (read_sect(sheader, data)) {
               // MIPS ASE Mask is at offset 12 in MIPS.abiflags section
               lldb::offset_t offset = 12; // MIPS ABI Flags Version: 0
               arch_flags |= data.GetU32(&offset);
@@ -1565,16 +1586,14 @@ size_t 
ObjectFileELF::GetSectionHeaderInfo(SectionHeaderColl &section_headers,
         if (arch_spec.GetMachine() == llvm::Triple::arm ||
             arch_spec.GetMachine() == llvm::Triple::thumb) {
           DataExtractor data;
-
-          if (sheader.sh_type == SHT_ARM_ATTRIBUTES && section_size != 0 &&
-              data.SetData(object_data, sheader.sh_offset, section_size) == 
section_size)
-            ParseARMAttributes(data, section_size, arch_spec);
+          if (sheader.sh_type == SHT_ARM_ATTRIBUTES &&
+            read_sect(sheader, data))
+            ParseARMAttributes(data, arch_spec);
         }
 
         if (name == g_sect_name_gnu_debuglink) {
           DataExtractor data;
-          if (section_size && (data.SetData(object_data, sheader.sh_offset,
-                                            section_size) == section_size)) {
+          if (read_sect(sheader, data)) {
             lldb::offset_t gnu_debuglink_offset = 0;
             gnu_debuglink_file = data.GetCStr(&gnu_debuglink_offset);
             gnu_debuglink_offset = llvm::alignTo(gnu_debuglink_offset, 4);
@@ -1594,8 +1613,7 @@ size_t 
ObjectFileELF::GetSectionHeaderInfo(SectionHeaderColl &section_headers,
         if (is_note_header) {
           // Allow notes to refine module info.
           DataExtractor data;
-          if (section_size && (data.SetData(object_data, sheader.sh_offset,
-                                            section_size) == section_size)) {
+          if (read_sect(sheader, data)) {
             Status error = RefineModuleDetailsFromNote(data, arch_spec, uuid);
             if (error.Fail()) {
               LLDB_LOGF(log, "ObjectFileELF::%s ELF note processing failed: 
%s",
@@ -1627,9 +1645,28 @@ 
ObjectFileELF::StripLinkerSymbolAnnotations(llvm::StringRef symbol_name) const {
 
 // ParseSectionHeaders
 size_t ObjectFileELF::ParseSectionHeaders() {
-  return GetSectionHeaderInfo(m_section_headers, m_data, m_header, m_uuid,
-                              m_gnu_debuglink_file, m_gnu_debuglink_crc,
-                              m_arch_spec);
+  DataExtractor sh_data = GetSectionHeadersFromELFData(m_header, m_data);
+  const size_t sh_size = m_header.GetSectionHeaderByteSize();
+  if (sh_data.GetByteSize() != sh_size) {
+    if (IsInMemory()) {
+      // We have a ELF file in process memory, read the program header data 
from
+      // the process.
+      if (ProcessSP process_sp = m_process_wp.lock()) {
+        const addr_t addr = m_memory_addr + m_header.e_shoff;
+        if (DataBufferSP data_sp = ReadMemory(process_sp, addr, sh_size))
+          sh_data = DataExtractor(data_sp, GetByteOrder(), 
GetAddressByteSize());
+      }
+    }
+  }
+  auto read_sect_callback = [&](const elf::ELFSectionHeader &sh,
+                                lldb_private::DataExtractor &sh_data) -> bool {
+    sh_data = this->GetSectionData(sh);
+    return sh_data.GetByteSize() == sh.sh_size;
+  };
+
+  return GetSectionHeaderInfo(m_header, sh_data, m_section_headers,
+                              read_sect_callback, m_uuid, m_gnu_debuglink_file,
+                              m_gnu_debuglink_crc, m_arch_spec);
 }
 
 const ObjectFileELF::ELFSectionHeaderInfo *
@@ -3795,9 +3832,7 @@ DataExtractor ObjectFileELF::GetSegmentData(const 
ELFProgramHeader &H) {
     // We have a ELF file in process memory, read the program header data from
     // the process.
     if (ProcessSP process_sp = m_process_wp.lock()) {
-      const lldb::offset_t base_file_addr = GetBaseAddress().GetFileAddress();
-      const addr_t load_bias = m_memory_addr - base_file_addr;
-      const addr_t data_addr = H.p_vaddr + load_bias;
+      const addr_t data_addr = m_memory_addr + H.p_offset;
       if (DataBufferSP data_sp = ReadMemory(process_sp, data_addr, H.p_memsz))
         return DataExtractor(data_sp, GetByteOrder(), GetAddressByteSize());
     }
@@ -3805,6 +3840,28 @@ DataExtractor ObjectFileELF::GetSegmentData(const 
ELFProgramHeader &H) {
   return DataExtractor();
 }
 
+DataExtractor ObjectFileELF::GetSectionData(const elf::ELFSectionHeader &sh) {
+  // Try and read the section contents from our cached m_data which can come
+  // from the file on disk being mmap'ed or from the initial part of the ELF
+  // file we read from memory and cached.
+  DataExtractor data;
+  if (GetSectionContentsFromELFData(sh, m_data, data))
+    return data;
+  if (IsInMemory()) {
+    // We have a ELF file in process memory, read the program header data from
+    // the process.
+    if (ProcessSP process_sp = m_process_wp.lock()) {
+      const addr_t data_addr = m_memory_addr + sh.sh_offset;
+      if (DataBufferSP data_sp = ReadMemory(process_sp, data_addr, 
sh.sh_size)) {
+        data = DataExtractor(data_sp, GetByteOrder(), GetAddressByteSize());
+        if (data.GetByteSize() == sh.sh_size)
+          return data;
+      }
+    }
+  }
+  return DataExtractor();
+}
+
 bool ObjectFileELF::AnySegmentHasPhysicalAddress() {
   for (const ELFProgramHeader &H : ProgramHeaders()) {
     if (H.p_paddr != 0)
diff --git a/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.h 
b/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.h
index 41b8ce189e41d..039909f7d39c0 100644
--- a/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.h
+++ b/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.h
@@ -149,7 +149,7 @@ class ObjectFileELF : public lldb_private::ObjectFile {
 
   llvm::ArrayRef<elf::ELFProgramHeader> ProgramHeaders();
   lldb_private::DataExtractor GetSegmentData(const elf::ELFProgramHeader &H);
-
+  lldb_private::DataExtractor GetSectionData(const elf::ELFSectionHeader &H);
   llvm::StringRef
   StripLinkerSymbolAnnotations(llvm::StringRef symbol_name) const override;
 
@@ -270,14 +270,48 @@ class ObjectFileELF : public lldb_private::ObjectFile {
   lldb::SectionType GetSectionType(const ELFSectionHeaderInfo &H) const;
 
   static void ParseARMAttributes(lldb_private::DataExtractor &data,
-                                 uint64_t length,
                                  lldb_private::ArchSpec &arch_spec);
 
+  /// Read the section contents for a given section header.
+  ///
+  /// Section contents are available in the ELF file data if we have all of the
+  /// data mapped into \a object_data.
+  ///
+  /// \param[in] sh
+  ///    The section header of the section we want to read.
+  ///
+  /// \param[in] object_data
+  ///    The object data that contains all of the contiguous bytes which starts
+  ///    with the ELF header and also contains the program headers. This data
+  ///    will contain the full ELF file if the ELF file is read from disk. If
+  ///    the ELF file is read from memory, then it might only contain the ELF
+  ///    header and program header data.
+  ///
+  /// \param[out] section_data
+  ///    The section data to fill in if we are able to read the section 
contents
+  ///    completely.
+  ///
+  /// \return True if we are able to read all of the section data from
+  ///    \a object_data. False otherwise.
+  static bool GetSectionContentsFromELFData(
+    const elf::ELFSectionHeader &sh,
+    const lldb_private::DataExtractor &object_data,
+    lldb_private::DataExtractor &section_data);
+
+  /// Callback that can be used to read the data for a section.
+  ///
+  /// \return True if the section has a size and all bytes can be read,
+  ///         False otherwise.
+  using ReadSectionDataCallback =
+  std::function<bool(const elf::ELFSectionHeader &sh,
+                     lldb_private::DataExtractor &data)>;
+
   /// Parses the elf section headers and returns the uuid, debug link name,
   /// crc, archspec.
-  static size_t GetSectionHeaderInfo(SectionHeaderColl &section_headers,
-                                     lldb_private::DataExtractor &object_data,
-                                     const elf::ELFHeader &header,
+  static size_t GetSectionHeaderInfo(const elf::ELFHeader &header,
+                                     const lldb_private::DataExtractor 
&sh_data,
+                                     SectionHeaderColl &section_headers,
+                                     ReadSectionDataCallback 
read_sect_callback,
                                      lldb_private::UUID &uuid,
                                      std::string &gnu_debuglink_file,
                                      uint32_t &gnu_debuglink_crc,

>From d653c41231ab7dd2ee649ea6ebef3d12aed4d4b9 Mon Sep 17 00:00:00 2001
From: Greg Clayton <clayb...@gmail.com>
Date: Thu, 27 Feb 2025 16:52:03 -0800
Subject: [PATCH 2/2] Add a test and revert previous segment data fix that
 didn't work.

---
 .../Plugins/ObjectFile/ELF/ObjectFileELF.cpp  | 16 ++++++--
 .../test/Shell/ObjectFile/ELF/elf-memory.test | 40 +++++++++++++++++++
 2 files changed, 53 insertions(+), 3 deletions(-)

diff --git a/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp 
b/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
index 9225fa9b757bf..ba96f9fad61ff 100644
--- a/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
+++ b/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
@@ -658,8 +658,9 @@ size_t ObjectFileELF::GetModuleSpecifications(
           // Get the section header data from the object file.
           DataExtractor sh_data = GetSectionHeadersFromELFData(header, data);
 
-          auto read_sect_callback = [&](const elf::ELFSectionHeader &sh,
-                                        lldb_private::DataExtractor &sh_data) 
-> bool {
+          auto read_sect_callback =
+              [&](const elf::ELFSectionHeader &sh,
+                  lldb_private::DataExtractor &sh_data) -> bool {
             return GetSectionContentsFromELFData(sh, data, sh_data);
           };
           GetSectionHeaderInfo(header, sh_data, section_headers,
@@ -1505,6 +1506,13 @@ size_t ObjectFileELF::GetSectionHeaderInfo(const 
elf::ELFHeader &header,
   }
   if (idx < section_headers.size())
     section_headers.resize(idx);
+  // Sometimes we are able to read the section header memory from an in memory
+  // ELF file, but all section header data has been set to zeroes. Remove any
+  // SHT_NULL sections if we have more than 1. The first entry in the section
+  // headers should always be a SHT_NULL section, but none of the others should
+  // be.
+  if (section_headers.size() > 1 && section_headers[1].sh_type == SHT_NULL)
+    section_headers.erase(section_headers.begin() + 1);
 
   const unsigned strtab_idx = header.e_shstrndx;
   if (strtab_idx && strtab_idx < section_headers.size()) {
@@ -3832,7 +3840,9 @@ DataExtractor ObjectFileELF::GetSegmentData(const 
ELFProgramHeader &H) {
     // We have a ELF file in process memory, read the program header data from
     // the process.
     if (ProcessSP process_sp = m_process_wp.lock()) {
-      const addr_t data_addr = m_memory_addr + H.p_offset;
+      const lldb::offset_t base_file_addr = GetBaseAddress().GetFileAddress();
+      const addr_t load_bias = m_memory_addr - base_file_addr;
+      const addr_t data_addr = H.p_vaddr + load_bias;
       if (DataBufferSP data_sp = ReadMemory(process_sp, data_addr, H.p_memsz))
         return DataExtractor(data_sp, GetByteOrder(), GetAddressByteSize());
     }
diff --git a/lldb/test/Shell/ObjectFile/ELF/elf-memory.test 
b/lldb/test/Shell/ObjectFile/ELF/elf-memory.test
index 75a68edd2d349..2b0c61469c600 100644
--- a/lldb/test/Shell/ObjectFile/ELF/elf-memory.test
+++ b/lldb/test/Shell/ObjectFile/ELF/elf-memory.test
@@ -19,10 +19,15 @@
 // RUN:   -o "script real_module = lldb.target.module[0]" \
 // RUN:   -o "script base_addr = 
real_module.GetObjectFileHeaderAddress().GetLoadAddress(lldb.target)" \
 // RUN:   -o "script mem_module = lldb.SBModule(lldb.process, base_addr)" \
+// RUN:   -o 'script vsdo_module = [m for m in lldb.target.modules if 
m.GetFileSpec().fullpath == "[vdso]"][0]' \
 // RUN:   -o "script target2 = lldb.debugger.CreateTarget('')" \
 // RUN:   -o "script target2.AddModule(mem_module)" \
 // RUN:   -o "target select 1" \
 // RUN:   -o "image dump objfile" \
+// RUN:   -o "script target3 = lldb.debugger.CreateTarget('')" \
+// RUN:   -o "script target3.AddModule(vsdo_module)" \
+// RUN:   -o "target select 2" \
+// RUN:   -o "image dump objfile" \
 // RUN:   | FileCheck %s --check-prefix=MAIN --dump-input=always
 // MAIN: (lldb) image dump objfile
 // MAIN: Dumping headers for 1 module(s).
@@ -33,6 +38,9 @@
 // MAIN: Program Headers
 // MAIN: ] PT_DYNAMIC
 
+// Make sure there are no section headers for when they aren't loaded
+// MAIN-NOT: Section Headers
+
 // Make sure we see some sections created from the program headers
 // MAIN: SectID
 // MAIN: PT_LOAD[0]
@@ -53,3 +61,35 @@
 // MAIN-DAG: SYMENT {{0x[0-9a-f]+}}
 // MAIN-DAG: DEBUG {{0x[0-9a-f]+}}
 // MAIN:     NULL {{0x[0-9a-f]+}}
+
+// Watch for the dump of the VSDO image. This should have section headers
+// MAIN: ObjectFileELF, file = '', arch = {{.*, addr = 0x[0-9a-f]+}}
+// MAIN: ELF Header
+
+// Make sure we find the program headers and see a PT_DYNAMIC entry.
+// MAIN: Program Headers
+// MAIN: ] PT_DYNAMIC
+
+// Make sure we find the section headers with at least a .text section to
+// ensure we found the sections and their names.
+// MAIN: Section Headers
+// MAIN: .text
+
+// Make sure we see some sections created from the program headers
+// MAIN: SectID
+// MAIN: PT_LOAD[0]
+
+// Ensure we find some dependent modules as won't find these if we aren't able
+// to load the .dynamic section from the PT_DYNAMIC program header.
+// MAIN-NOT: Dependent Modules:
+
+// Check for the .dynamic dump and ensure we find all dynamic entries that are
+// required to be there and needed to get the .dynstr section and the symbol
+// table, and the DT_DEBUG entry to find the list of shared libraries.
+// MAIN: .dynamic:
+// Make sure we found the .dynstr section by checking for valid strings after 
NEEDED
+// MAIN-DAG: STRTAB {{0x[0-9a-f]+}}
+// MAIN-DAG: SYMTAB {{0x[0-9a-f]+}}
+// MAIN-DAG: STRSZ {{0x[0-9a-f]+}}
+// MAIN-DAG: SYMENT {{0x[0-9a-f]+}}
+// MAIN:     NULL {{0x[0-9a-f]+}}

_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to