Revert "Reland "[lldb] Remove non address bits when looking up memory…

… regions"" This reverts commit fac3f20. I found this has broken how we detect the last memory region in GetMemoryRegions/"memory region" command. When you're debugging an AArch64 system with pointer authentication, the ABI plugin will remove the top bit from the end address of the last user mapped area. (lldb) [0x0000fffffffdf000-0x0001000000000000) rw- [stack] ABI plugin removes anything above the 48th bit (48 bit virtual addresses by default on AArch64, leaving an address of 0. (lldb) [0x0000000000000000-0x0000000000400000) --- You get back a mapping for 0 and get into an infinite loop.
llvm · Nov 26, 2021 · 0df5229 · 0df5229
1 parent 34cc210
commit 0df5229
Show file tree

Hide file tree

Showing 17 changed files with 41 additions and 130 deletions.
diff --git a/lldb/include/lldb/Target/Process.h b/lldb/include/lldb/Target/Process.h
@@ -1762,7 +1762,7 @@ class Process : public std::enable_shared_from_this<Process>,
   ///
   /// If load_addr is within the address space the process has mapped
   /// range_info will be filled in with the start and end of that range as
-  /// well as the permissions for that range and range_info. GetMapped will
+  /// well as the permissions for that range and range_info.GetMapped will
   /// return true.
   ///
   /// If load_addr is outside any mapped region then range_info will have its
@@ -1771,21 +1771,23 @@ class Process : public std::enable_shared_from_this<Process>,
   /// there are no valid mapped ranges between load_addr and the end of the
   /// process address space.
   ///
-  /// GetMemoryRegionInfo calls DoGetMemoryRegionInfo. Override that function in
-  /// process subclasses.
+  /// GetMemoryRegionInfo will only return an error if it is unimplemented for
+  /// the current process.
   ///
   /// \param[in] load_addr
-  ///     The load address to query the range_info for. May include non
-  ///     address bits, these will be removed by the the ABI plugin if there is
-  ///     one.
+  ///     The load address to query the range_info for.
   ///
   /// \param[out] range_info
   ///     An range_info value containing the details of the range.
   ///
   /// \return
   ///     An error value.
-  Status GetMemoryRegionInfo(lldb::addr_t load_addr,
-                             MemoryRegionInfo &range_info);
+  virtual Status GetMemoryRegionInfo(lldb::addr_t load_addr,
+                                     MemoryRegionInfo &range_info) {
+    Status error;
+    error.SetErrorString("Process::GetMemoryRegionInfo() not supported");
+    return error;
+  }
 
   /// Obtain all the mapped memory regions within this process.
   ///
@@ -2605,26 +2607,6 @@ void PruneThreadPlans();
   virtual size_t DoReadMemory(lldb::addr_t vm_addr, void *buf, size_t size,
                               Status &error) = 0;
 
-  /// DoGetMemoryRegionInfo is called by GetMemoryRegionInfo after it has
-  /// removed non address bits from load_addr. Override this method in
-  /// subclasses of Process.
-  ///
-  /// See GetMemoryRegionInfo for details of the logic.
-  ///
-  /// \param[in] load_addr
-  ///     The load address to query the range_info for. (non address bits
-  ///     removed)
-  ///
-  /// \param[out] range_info
-  ///     An range_info value containing the details of the range.
-  ///
-  /// \return
-  ///     An error value.
-  virtual Status DoGetMemoryRegionInfo(lldb::addr_t load_addr,
-                                       MemoryRegionInfo &range_info) {
-    return Status("Process::DoGetMemoryRegionInfo() not supported");
-  }
-
   lldb::StateType GetPrivateState();
 
   /// The "private" side of resuming a process.  This doesn't alter the state

diff --git a/lldb/source/Plugins/Process/Windows/Common/ProcessWindows.cpp b/lldb/source/Plugins/Process/Windows/Common/ProcessWindows.cpp
@@ -601,8 +601,8 @@ Status ProcessWindows::DoDeallocateMemory(lldb::addr_t ptr) {
   return ProcessDebugger::DeallocateMemory(ptr);
 }
 
-Status ProcessWindows::DoGetMemoryRegionInfo(lldb::addr_t vm_addr,
-                                             MemoryRegionInfo &info) {
+Status ProcessWindows::GetMemoryRegionInfo(lldb::addr_t vm_addr,
+                                           MemoryRegionInfo &info) {
   return ProcessDebugger::GetMemoryRegionInfo(vm_addr, info);
 }
 

diff --git a/lldb/source/Plugins/Process/Windows/Common/ProcessWindows.h b/lldb/source/Plugins/Process/Windows/Common/ProcessWindows.h
@@ -78,6 +78,8 @@ class ProcessWindows : public Process, public ProcessDebugger {
   lldb::addr_t DoAllocateMemory(size_t size, uint32_t permissions,
                                 Status &error) override;
   Status DoDeallocateMemory(lldb::addr_t ptr) override;
+  Status GetMemoryRegionInfo(lldb::addr_t vm_addr,
+                             MemoryRegionInfo &info) override;
 
   lldb::addr_t GetImageInfoAddress() override;
 
@@ -101,10 +103,6 @@ class ProcessWindows : public Process, public ProcessDebugger {
   Status EnableWatchpoint(Watchpoint *wp, bool notify = true) override;
   Status DisableWatchpoint(Watchpoint *wp, bool notify = true) override;
 
-protected:
-  Status DoGetMemoryRegionInfo(lldb::addr_t vm_addr,
-                               MemoryRegionInfo &info) override;
-
 private:
   struct WatchpointInfo {
     uint32_t slot_id;

diff --git a/lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp b/lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp
@@ -281,8 +281,8 @@ size_t ProcessElfCore::ReadMemory(lldb::addr_t addr, void *buf, size_t size,
   return DoReadMemory(addr, buf, size, error);
 }
 
-Status ProcessElfCore::DoGetMemoryRegionInfo(lldb::addr_t load_addr,
-                                             MemoryRegionInfo &region_info) {
+Status ProcessElfCore::GetMemoryRegionInfo(lldb::addr_t load_addr,
+                                           MemoryRegionInfo &region_info) {
   region_info.Clear();
   const VMRangeToPermissions::Entry *permission_entry =
       m_core_range_infos.FindEntryThatContainsOrFollows(load_addr);

diff --git a/lldb/source/Plugins/Process/elf-core/ProcessElfCore.h b/lldb/source/Plugins/Process/elf-core/ProcessElfCore.h
@@ -86,6 +86,10 @@ class ProcessElfCore : public lldb_private::PostMortemProcess {
   size_t DoReadMemory(lldb::addr_t addr, void *buf, size_t size,
                       lldb_private::Status &error) override;
 
+  lldb_private::Status
+  GetMemoryRegionInfo(lldb::addr_t load_addr,
+                      lldb_private::MemoryRegionInfo &region_info) override;
+
   lldb::addr_t GetImageInfoAddress() override;
 
   lldb_private::ArchSpec GetArchitecture();
@@ -101,10 +105,6 @@ class ProcessElfCore : public lldb_private::PostMortemProcess {
   bool DoUpdateThreadList(lldb_private::ThreadList &old_thread_list,
                           lldb_private::ThreadList &new_thread_list) override;
 
-  lldb_private::Status
-  DoGetMemoryRegionInfo(lldb::addr_t load_addr,
-                        lldb_private::MemoryRegionInfo &region_info) override;
-
 private:
   struct NT_FILE_Entry {
     lldb::addr_t start;

diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -2877,8 +2877,8 @@ lldb::addr_t ProcessGDBRemote::DoAllocateMemory(size_t size,
   return allocated_addr;
 }
 
-Status ProcessGDBRemote::DoGetMemoryRegionInfo(addr_t load_addr,
-                                               MemoryRegionInfo &region_info) {
+Status ProcessGDBRemote::GetMemoryRegionInfo(addr_t load_addr,
+                                             MemoryRegionInfo &region_info) {
 
   Status error(m_gdb_comm.GetMemoryRegionInfo(load_addr, region_info));
   return error;

diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
@@ -144,6 +144,9 @@ class ProcessGDBRemote : public Process,
   lldb::addr_t DoAllocateMemory(size_t size, uint32_t permissions,
                                 Status &error) override;
 
+  Status GetMemoryRegionInfo(lldb::addr_t load_addr,
+                             MemoryRegionInfo &region_info) override;
+
   Status DoDeallocateMemory(lldb::addr_t ptr) override;
 
   // Process STDIO
@@ -417,9 +420,6 @@ class ProcessGDBRemote : public Process,
   Status DoWriteMemoryTags(lldb::addr_t addr, size_t len, int32_t type,
                            const std::vector<uint8_t> &tags) override;
 
-  Status DoGetMemoryRegionInfo(lldb::addr_t load_addr,
-                               MemoryRegionInfo &region_info) override;
-
 private:
   // For ProcessGDBRemote only
   std::string m_partial_profile_data;

diff --git a/lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp b/lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp
@@ -633,8 +633,8 @@ size_t ProcessMachCore::DoReadMemory(addr_t addr, void *buf, size_t size,
   return bytes_read;
 }
 
-Status ProcessMachCore::DoGetMemoryRegionInfo(addr_t load_addr,
-                                              MemoryRegionInfo &region_info) {
+Status ProcessMachCore::GetMemoryRegionInfo(addr_t load_addr,
+                                            MemoryRegionInfo &region_info) {
   region_info.Clear();
   const VMRangeToPermissions::Entry *permission_entry =
       m_core_range_infos.FindEntryThatContainsOrFollows(load_addr);

diff --git a/lldb/source/Plugins/Process/mach-core/ProcessMachCore.h b/lldb/source/Plugins/Process/mach-core/ProcessMachCore.h
@@ -68,6 +68,10 @@ class ProcessMachCore : public lldb_private::PostMortemProcess {
   size_t DoReadMemory(lldb::addr_t addr, void *buf, size_t size,
                       lldb_private::Status &error) override;
 
+  lldb_private::Status
+  GetMemoryRegionInfo(lldb::addr_t load_addr,
+                      lldb_private::MemoryRegionInfo &region_info) override;
+
   lldb::addr_t GetImageInfoAddress() override;
 
 protected:
@@ -80,10 +84,6 @@ class ProcessMachCore : public lldb_private::PostMortemProcess {
 
   lldb_private::ObjectFile *GetCoreObjectFile();
 
-  lldb_private::Status
-  DoGetMemoryRegionInfo(lldb::addr_t load_addr,
-                        lldb_private::MemoryRegionInfo &region_info) override;
-
 private:
   bool GetDynamicLoaderAddress(lldb::addr_t addr);
 

diff --git a/lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp b/lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp
@@ -439,8 +439,8 @@ void ProcessMinidump::BuildMemoryRegions() {
   llvm::sort(*m_memory_regions);
 }
 
-Status ProcessMinidump::DoGetMemoryRegionInfo(lldb::addr_t load_addr,
-                                              MemoryRegionInfo &region) {
+Status ProcessMinidump::GetMemoryRegionInfo(lldb::addr_t load_addr,
+                                            MemoryRegionInfo &region) {
   BuildMemoryRegions();
   region = MinidumpParser::GetMemoryRegionInfo(*m_memory_regions, load_addr);
   return Status();

diff --git a/lldb/source/Plugins/Process/minidump/ProcessMinidump.h b/lldb/source/Plugins/Process/minidump/ProcessMinidump.h
@@ -75,6 +75,9 @@ class ProcessMinidump : public PostMortemProcess {
 
   ArchSpec GetArchitecture();
 
+  Status GetMemoryRegionInfo(lldb::addr_t load_addr,
+                             MemoryRegionInfo &range_info) override;
+
   Status GetMemoryRegions(
       lldb_private::MemoryRegionInfos &region_list) override;
 
@@ -95,9 +98,6 @@ class ProcessMinidump : public PostMortemProcess {
   bool DoUpdateThreadList(ThreadList &old_thread_list,
                           ThreadList &new_thread_list) override;
 
-  Status DoGetMemoryRegionInfo(lldb::addr_t load_addr,
-                               MemoryRegionInfo &range_info) override;
-
   void ReadModuleList();
 
   lldb::ModuleSP GetOrCreateModule(lldb_private::UUID minidump_uuid,

diff --git a/lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp b/lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp
@@ -248,8 +248,8 @@ ArchSpec ScriptedProcess::GetArchitecture() {
   return GetTarget().GetArchitecture();
 }
 
-Status ScriptedProcess::DoGetMemoryRegionInfo(lldb::addr_t load_addr,
-                                              MemoryRegionInfo &region) {
+Status ScriptedProcess::GetMemoryRegionInfo(lldb::addr_t load_addr,
+                                            MemoryRegionInfo &region) {
   CheckInterpreterAndScriptObject();
 
   Status error;

diff --git a/lldb/source/Plugins/Process/scripted/ScriptedProcess.h b/lldb/source/Plugins/Process/scripted/ScriptedProcess.h
@@ -84,6 +84,9 @@ class ScriptedProcess : public Process {
 
   ArchSpec GetArchitecture();
 
+  Status GetMemoryRegionInfo(lldb::addr_t load_addr,
+                             MemoryRegionInfo &range_info) override;
+
   Status
   GetMemoryRegions(lldb_private::MemoryRegionInfos &region_list) override;
 
@@ -97,9 +100,6 @@ class ScriptedProcess : public Process {
   bool DoUpdateThreadList(ThreadList &old_thread_list,
                           ThreadList &new_thread_list) override;
 
-  Status DoGetMemoryRegionInfo(lldb::addr_t load_addr,
-                               MemoryRegionInfo &range_info) override;
-
 private:
   friend class ScriptedThread;
 

diff --git a/lldb/source/Target/Process.cpp b/lldb/source/Target/Process.cpp
@@ -5853,13 +5853,6 @@ Process::AdvanceAddressToNextBranchInstruction(Address default_stop_addr,
   return retval;
 }
 
-Status Process::GetMemoryRegionInfo(lldb::addr_t load_addr,
-                                    MemoryRegionInfo &range_info) {
-  if (auto abi = GetABI())
-    load_addr = abi->FixDataAddress(load_addr);
-  return DoGetMemoryRegionInfo(load_addr, range_info);
-}
-
 Status
 Process::GetMemoryRegions(lldb_private::MemoryRegionInfos &region_list) {
 

diff --git a/lldb/test/API/linux/aarch64/tagged_memory_region/Makefile b/lldb/test/API/linux/aarch64/tagged_memory_region/Makefile
diff --git a/lldb/test/API/linux/aarch64/tagged_memory_region/TestAArch64LinuxTaggedMemoryRegion.py b/lldb/test/API/linux/aarch64/tagged_memory_region/TestAArch64LinuxTaggedMemoryRegion.py
diff --git a/lldb/test/API/linux/aarch64/tagged_memory_region/main.c b/lldb/test/API/linux/aarch64/tagged_memory_region/main.c