Skip to content

Revert "Read and store gnu build id from loaded core file" #92181

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
May 14, 2024

Conversation

GeorgeHuyubo
Copy link
Contributor

Reverts #92078

@GeorgeHuyubo GeorgeHuyubo merged commit 5bf653c into main May 14, 2024
4 of 5 checks passed
@GeorgeHuyubo GeorgeHuyubo deleted the revert-92078-main branch May 14, 2024 21:36
@llvmbot llvmbot added the lldb label May 14, 2024
@llvmbot
Copy link
Member

llvmbot commented May 14, 2024

@llvm/pr-subscribers-lldb

Author: None (GeorgeHuyubo)

Changes

Reverts llvm/llvm-project#92078


Full diff: https://github.com/llvm/llvm-project/pull/92181.diff

5 Files Affected:

  • (modified) lldb/include/lldb/Target/Process.h (-50)
  • (modified) lldb/source/Commands/CommandObjectMemory.cpp (+59-2)
  • (modified) lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp (-50)
  • (modified) lldb/source/Plugins/Process/elf-core/ProcessElfCore.h (-11)
  • (modified) lldb/source/Target/Process.cpp (-27)
diff --git a/lldb/include/lldb/Target/Process.h b/lldb/include/lldb/Target/Process.h
index c8a49edc5c78d..aac0cf51680a9 100644
--- a/lldb/include/lldb/Target/Process.h
+++ b/lldb/include/lldb/Target/Process.h
@@ -406,36 +406,6 @@ class Process : public std::enable_shared_from_this<Process>,
                                   lldb::StateType state);
   } Notifications;
 
-  class ProcessMemoryIterator {
-  public:
-    ProcessMemoryIterator(lldb::ProcessSP process_sp, lldb::addr_t base)
-        : m_process_sp(process_sp), m_base_addr(base) {
-      lldbassert(process_sp.get() != nullptr);
-    }
-
-    bool IsValid() { return m_is_valid; }
-
-    uint8_t operator[](lldb::addr_t offset) {
-      if (!IsValid())
-        return 0;
-
-      uint8_t retval = 0;
-      Status error;
-      if (0 ==
-          m_process_sp->ReadMemory(m_base_addr + offset, &retval, 1, error)) {
-        m_is_valid = false;
-        return 0;
-      }
-
-      return retval;
-    }
-
-  private:
-    lldb::ProcessSP m_process_sp;
-    lldb::addr_t m_base_addr;
-    bool m_is_valid = true;
-  };
-
   class ProcessEventData : public EventData {
     friend class Process;
 
@@ -1679,26 +1649,6 @@ class Process : public std::enable_shared_from_this<Process>,
 
   lldb::addr_t ReadPointerFromMemory(lldb::addr_t vm_addr, Status &error);
 
-  /// Find a string within a memory region.
-  ///
-  /// This function searches for the string represented by the provided buffer
-  /// within the memory range specified by the low and high addresses. It uses
-  /// a bad character heuristic to optimize the search process.
-  ///
-  /// \param[in] low The starting address of the memory region to be searched.
-  ///
-  /// \param[in] high The ending address of the memory region to be searched.
-  ///
-  /// \param[in] buffer A pointer to the buffer containing the string to be
-  /// searched.
-  ///
-  /// \param[in] buffer_size The size of the buffer in bytes.
-  ///
-  /// \return The address where the string was found or LLDB_INVALID_ADDRESS if
-  /// not found.
-  lldb::addr_t FindInMemory(lldb::addr_t low, lldb::addr_t high,
-                            uint8_t *buffer, size_t buffer_size);
-
   bool WritePointerToMemory(lldb::addr_t vm_addr, lldb::addr_t ptr_value,
                             Status &error);
 
diff --git a/lldb/source/Commands/CommandObjectMemory.cpp b/lldb/source/Commands/CommandObjectMemory.cpp
index 1c13484dede64..b78a0492cca55 100644
--- a/lldb/source/Commands/CommandObjectMemory.cpp
+++ b/lldb/source/Commands/CommandObjectMemory.cpp
@@ -977,6 +977,35 @@ class CommandObjectMemoryFind : public CommandObjectParsed {
   Options *GetOptions() override { return &m_option_group; }
 
 protected:
+  class ProcessMemoryIterator {
+  public:
+    ProcessMemoryIterator(ProcessSP process_sp, lldb::addr_t base)
+        : m_process_sp(process_sp), m_base_addr(base) {
+      lldbassert(process_sp.get() != nullptr);
+    }
+
+    bool IsValid() { return m_is_valid; }
+
+    uint8_t operator[](lldb::addr_t offset) {
+      if (!IsValid())
+        return 0;
+
+      uint8_t retval = 0;
+      Status error;
+      if (0 ==
+          m_process_sp->ReadMemory(m_base_addr + offset, &retval, 1, error)) {
+        m_is_valid = false;
+        return 0;
+      }
+
+      return retval;
+    }
+
+  private:
+    ProcessSP m_process_sp;
+    lldb::addr_t m_base_addr;
+    bool m_is_valid = true;
+  };
   void DoExecute(Args &command, CommandReturnObject &result) override {
     // No need to check "process" for validity as eCommandRequiresProcess
     // ensures it is valid
@@ -1077,8 +1106,8 @@ class CommandObjectMemoryFind : public CommandObjectParsed {
     found_location = low_addr;
     bool ever_found = false;
     while (count) {
-      found_location = process->FindInMemory(
-          found_location, high_addr, buffer.GetBytes(), buffer.GetByteSize());
+      found_location = FastSearch(found_location, high_addr, buffer.GetBytes(),
+                                  buffer.GetByteSize());
       if (found_location == LLDB_INVALID_ADDRESS) {
         if (!ever_found) {
           result.AppendMessage("data not found within the range.\n");
@@ -1115,6 +1144,34 @@ class CommandObjectMemoryFind : public CommandObjectParsed {
     result.SetStatus(lldb::eReturnStatusSuccessFinishResult);
   }
 
+  lldb::addr_t FastSearch(lldb::addr_t low, lldb::addr_t high, uint8_t *buffer,
+                          size_t buffer_size) {
+    const size_t region_size = high - low;
+
+    if (region_size < buffer_size)
+      return LLDB_INVALID_ADDRESS;
+
+    std::vector<size_t> bad_char_heuristic(256, buffer_size);
+    ProcessSP process_sp = m_exe_ctx.GetProcessSP();
+    ProcessMemoryIterator iterator(process_sp, low);
+
+    for (size_t idx = 0; idx < buffer_size - 1; idx++) {
+      decltype(bad_char_heuristic)::size_type bcu_idx = buffer[idx];
+      bad_char_heuristic[bcu_idx] = buffer_size - idx - 1;
+    }
+    for (size_t s = 0; s <= (region_size - buffer_size);) {
+      int64_t j = buffer_size - 1;
+      while (j >= 0 && buffer[j] == iterator[s + j])
+        j--;
+      if (j < 0)
+        return low + s;
+      else
+        s += bad_char_heuristic[iterator[s + buffer_size - 1]];
+    }
+
+    return LLDB_INVALID_ADDRESS;
+  }
+
   OptionGroupOptions m_option_group;
   OptionGroupFindMemory m_memory_options;
   OptionGroupMemoryTag m_memory_tag_options;
diff --git a/lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp b/lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp
index 4ff03eb8ab485..36812c27a5b6d 100644
--- a/lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp
+++ b/lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp
@@ -6,12 +6,10 @@
 //
 //===----------------------------------------------------------------------===//
 
-#include <cstddef>
 #include <cstdlib>
 
 #include <memory>
 #include <mutex>
-#include <tuple>
 
 #include "lldb/Core/Module.h"
 #include "lldb/Core/ModuleSpec.h"
@@ -212,9 +210,6 @@ Status ProcessElfCore::DoLoadCore() {
     }
   }
 
-  // We need to update uuid after address range is populated.
-  UpdateBuildIdForNTFileEntries();
-
   if (!ranges_are_sorted) {
     m_core_aranges.Sort();
     m_core_range_infos.Sort();
@@ -263,7 +258,6 @@ Status ProcessElfCore::DoLoadCore() {
     if (!m_nt_file_entries.empty()) {
       ModuleSpec exe_module_spec;
       exe_module_spec.GetArchitecture() = arch;
-      exe_module_spec.GetUUID() = m_nt_file_entries[0].uuid;
       exe_module_spec.GetFileSpec().SetFile(m_nt_file_entries[0].path,
                                             FileSpec::Style::native);
       if (exe_module_spec.GetFileSpec()) {
@@ -277,16 +271,6 @@ Status ProcessElfCore::DoLoadCore() {
   return error;
 }
 
-void ProcessElfCore::UpdateBuildIdForNTFileEntries() {
-  if (!m_nt_file_entries.empty()) {
-    for (NT_FILE_Entry &entry : m_nt_file_entries) {
-      std::optional<UUID> uuid = FindBuildId(entry);
-      if (uuid)
-        entry.uuid = uuid.value();
-    }
-  }
-}
-
 lldb_private::DynamicLoader *ProcessElfCore::GetDynamicLoader() {
   if (m_dyld_up.get() == nullptr)
     m_dyld_up.reset(DynamicLoader::FindPlugin(
@@ -999,40 +983,6 @@ llvm::Error ProcessElfCore::ParseThreadContextsFromNoteSegment(
   }
 }
 
-bool ProcessElfCore::IsElf(const NT_FILE_Entry entry) {
-  size_t size = strlen(llvm::ELF::ElfMagic);
-  uint8_t buf[size];
-  Status error;
-  size_t byte_read = ReadMemory(entry.start, buf, size, error);
-  if (byte_read == size)
-    return memcmp(llvm::ELF::ElfMagic, buf, size) == 0;
-  else
-    return false;
-}
-
-std::optional<UUID> ProcessElfCore::FindBuildId(const NT_FILE_Entry entry) {
-  if (!IsElf(entry))
-    return std::nullopt;
-  // Build ID is stored in the ELF file as a section named ".note.gnu.build-id"
-  uint8_t gnu_build_id_bytes[8] = {0x03, 0x00, 0x00, 0x00,
-                                   0x47, 0x4e, 0x55, 0x00};
-  lldb::addr_t gnu_build_id_addr =
-      FindInMemory(entry.start, entry.end, gnu_build_id_bytes, 8);
-  if (gnu_build_id_addr == LLDB_INVALID_ADDRESS)
-    return std::nullopt;
-  uint8_t buf[36];
-  Status error;
-  size_t byte_read = ReadMemory(gnu_build_id_addr - 8, buf, 36, error);
-  // .note.gnu.build-id starts with 04 00 00 00 {id_byte_size} 00 00 00 03 00 00
-  // 00 47 4e 55 00
-  if (byte_read == 36) {
-    if (buf[0] == 0x04) {
-      return UUID(llvm::ArrayRef<uint8_t>(buf + 16, buf[4] /*byte size*/));
-    }
-  }
-  return std::nullopt;
-}
-
 uint32_t ProcessElfCore::GetNumThreadContexts() {
   if (!m_thread_data_valid)
     DoLoadCore();
diff --git a/lldb/source/Plugins/Process/elf-core/ProcessElfCore.h b/lldb/source/Plugins/Process/elf-core/ProcessElfCore.h
index ae827f3df002c..2cec635bbacfe 100644
--- a/lldb/source/Plugins/Process/elf-core/ProcessElfCore.h
+++ b/lldb/source/Plugins/Process/elf-core/ProcessElfCore.h
@@ -117,8 +117,6 @@ class ProcessElfCore : public lldb_private::PostMortemProcess {
     lldb::addr_t end;
     lldb::addr_t file_ofs;
     std::string path;
-    lldb_private::UUID
-        uuid; // extracted from .note.gnu.build-id section from core file
   };
 
   // For ProcessElfCore only
@@ -160,15 +158,6 @@ class ProcessElfCore : public lldb_private::PostMortemProcess {
   // Returns number of thread contexts stored in the core file
   uint32_t GetNumThreadContexts();
 
-  // Populate gnu uuid for each NT_FILE entry
-  void UpdateBuildIdForNTFileEntries();
-
-  // Returns the UUID of a given NT_FILE entry
-  std::optional<lldb_private::UUID> FindBuildId(const NT_FILE_Entry entry);
-
-  // Returns true if the given NT_FILE entry is an ELF file
-  bool IsElf(const NT_FILE_Entry entry);
-
   // Parse a contiguous address range of the process from LOAD segment
   lldb::addr_t
   AddAddressRangeFromLoadSegment(const elf::ELFProgramHeader &header);
diff --git a/lldb/source/Target/Process.cpp b/lldb/source/Target/Process.cpp
index 6f5c43bc41082..25afade9a8275 100644
--- a/lldb/source/Target/Process.cpp
+++ b/lldb/source/Target/Process.cpp
@@ -3191,33 +3191,6 @@ Status Process::Halt(bool clear_thread_plans, bool use_run_lock) {
   return Status();
 }
 
-lldb::addr_t Process::FindInMemory(lldb::addr_t low, lldb::addr_t high,
-                                   uint8_t *buffer, size_t buffer_size) {
-  const size_t region_size = high - low;
-
-  if (region_size < buffer_size)
-    return LLDB_INVALID_ADDRESS;
-
-  std::vector<size_t> bad_char_heuristic(256, buffer_size);
-  ProcessMemoryIterator iterator(shared_from_this(), low);
-
-  for (size_t idx = 0; idx < buffer_size - 1; idx++) {
-    decltype(bad_char_heuristic)::size_type bcu_idx = buffer[idx];
-    bad_char_heuristic[bcu_idx] = buffer_size - idx - 1;
-  }
-  for (size_t s = 0; s <= (region_size - buffer_size);) {
-    int64_t j = buffer_size - 1;
-    while (j >= 0 && buffer[j] == iterator[s + j])
-      j--;
-    if (j < 0)
-      return low + s;
-    else
-      s += bad_char_heuristic[iterator[s + buffer_size - 1]];
-  }
-
-  return LLDB_INVALID_ADDRESS;
-}
-
 Status Process::StopForDestroyOrDetach(lldb::EventSP &exit_event_sp) {
   Status error;
 

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants