[Lldb-commits] [lldb] fac3f20 - Reland "[lldb] Remove non address bits when looking up memory regions"

David Spickett via lldb-commits lldb-commits at lists.llvm.org
Wed Nov 3 06:57:01 PDT 2021


Author: David Spickett
Date: 2021-11-03T13:56:51Z
New Revision: fac3f20de55769d028bd92220e74f22fa57dd4b2

URL: https://github.com/llvm/llvm-project/commit/fac3f20de55769d028bd92220e74f22fa57dd4b2
DIFF: https://github.com/llvm/llvm-project/commit/fac3f20de55769d028bd92220e74f22fa57dd4b2.diff

LOG: Reland "[lldb] Remove non address bits when looking up memory regions"

This reverts commit 5fbcf677347e38718461496d9e9e184a7a30c3fb.

ProcessDebugger is used in ProcessWindows and NativeProcessWindows.
I thought I was simplifying things by renaming to DoGetMemoryRegionInfo
in ProcessDebugger but the Native process side expects "GetMemoryRegionInfo".

Follow the pattern that WriteMemory uses. So:
* ProcessWindows::DoGetMemoryRegioninfo calls ProcessDebugger::GetMemoryRegionInfo
* NativeProcessWindows::GetMemoryRegionInfo does the same

Added: 
    lldb/test/API/linux/aarch64/tagged_memory_region/Makefile
    lldb/test/API/linux/aarch64/tagged_memory_region/TestAArch64LinuxTaggedMemoryRegion.py
    lldb/test/API/linux/aarch64/tagged_memory_region/main.c

Modified: 
    lldb/include/lldb/Target/Process.h
    lldb/source/Plugins/Process/Windows/Common/ProcessWindows.cpp
    lldb/source/Plugins/Process/Windows/Common/ProcessWindows.h
    lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp
    lldb/source/Plugins/Process/elf-core/ProcessElfCore.h
    lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
    lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
    lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp
    lldb/source/Plugins/Process/mach-core/ProcessMachCore.h
    lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp
    lldb/source/Plugins/Process/minidump/ProcessMinidump.h
    lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp
    lldb/source/Plugins/Process/scripted/ScriptedProcess.h
    lldb/source/Target/Process.cpp

Removed: 
    


################################################################################
diff  --git a/lldb/include/lldb/Target/Process.h b/lldb/include/lldb/Target/Process.h
index e9743ed17dd46..d36c60e24887d 100644
--- a/lldb/include/lldb/Target/Process.h
+++ b/lldb/include/lldb/Target/Process.h
@@ -1792,7 +1792,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
@@ -1801,23 +1801,21 @@ 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 will only return an error if it is unimplemented for
-  /// the current process.
+  /// GetMemoryRegionInfo calls DoGetMemoryRegionInfo. Override that function in
+  /// process subclasses.
   ///
   /// \param[in] load_addr
-  ///     The load address to query the range_info for.
+  ///     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.
   ///
   /// \param[out] range_info
   ///     An range_info value containing the details of the range.
   ///
   /// \return
   ///     An error value.
-  virtual Status GetMemoryRegionInfo(lldb::addr_t load_addr,
-                                     MemoryRegionInfo &range_info) {
-    Status error;
-    error.SetErrorString("Process::GetMemoryRegionInfo() not supported");
-    return error;
-  }
+  Status GetMemoryRegionInfo(lldb::addr_t load_addr,
+                             MemoryRegionInfo &range_info);
 
   /// Obtain all the mapped memory regions within this process.
   ///
@@ -2637,6 +2635,26 @@ 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
index 97ba1bdc9ee9e..f6e89bb0662a3 100644
--- 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::GetMemoryRegionInfo(lldb::addr_t vm_addr,
-                                           MemoryRegionInfo &info) {
+Status ProcessWindows::DoGetMemoryRegionInfo(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
index fc83649818edd..6f6f93f588e33 100644
--- a/lldb/source/Plugins/Process/Windows/Common/ProcessWindows.h
+++ b/lldb/source/Plugins/Process/Windows/Common/ProcessWindows.h
@@ -78,8 +78,6 @@ 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;
 
@@ -103,6 +101,10 @@ 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
index b852a01643753..23b346d5c17f1 100644
--- 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::GetMemoryRegionInfo(lldb::addr_t load_addr,
-                                           MemoryRegionInfo &region_info) {
+Status ProcessElfCore::DoGetMemoryRegionInfo(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
index 67df3c5fac76a..fd36e50278168 100644
--- a/lldb/source/Plugins/Process/elf-core/ProcessElfCore.h
+++ b/lldb/source/Plugins/Process/elf-core/ProcessElfCore.h
@@ -86,10 +86,6 @@ 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();
@@ -105,6 +101,10 @@ 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
index 4f78ae428147c..ce97240e8d905 100644
--- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -2903,8 +2903,8 @@ lldb::addr_t ProcessGDBRemote::DoAllocateMemory(size_t size,
   return allocated_addr;
 }
 
-Status ProcessGDBRemote::GetMemoryRegionInfo(addr_t load_addr,
-                                             MemoryRegionInfo &region_info) {
+Status ProcessGDBRemote::DoGetMemoryRegionInfo(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
index 9e0583408e42a..8134bc6b530d6 100644
--- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
+++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
@@ -144,9 +144,6 @@ 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
@@ -424,6 +421,9 @@ 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
index 6aed04565eb01..59c04590672d3 100644
--- 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::GetMemoryRegionInfo(addr_t load_addr,
-                                            MemoryRegionInfo &region_info) {
+Status ProcessMachCore::DoGetMemoryRegionInfo(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
index e55bfcfcc723b..b5ca515a7d3fb 100644
--- a/lldb/source/Plugins/Process/mach-core/ProcessMachCore.h
+++ b/lldb/source/Plugins/Process/mach-core/ProcessMachCore.h
@@ -68,10 +68,6 @@ 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:
@@ -84,6 +80,10 @@ 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
index 08cf58b0a84a1..736cfa0700886 100644
--- 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::GetMemoryRegionInfo(lldb::addr_t load_addr,
-                                            MemoryRegionInfo &region) {
+Status ProcessMinidump::DoGetMemoryRegionInfo(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
index 3501d38a0f27e..5360269199cdd 100644
--- a/lldb/source/Plugins/Process/minidump/ProcessMinidump.h
+++ b/lldb/source/Plugins/Process/minidump/ProcessMinidump.h
@@ -75,9 +75,6 @@ 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;
 
@@ -98,6 +95,9 @@ 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
index c0eefbf60337c..63c68c2a20236 100644
--- a/lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp
+++ b/lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp
@@ -249,8 +249,8 @@ ArchSpec ScriptedProcess::GetArchitecture() {
   return GetTarget().GetArchitecture();
 }
 
-Status ScriptedProcess::GetMemoryRegionInfo(lldb::addr_t load_addr,
-                                            MemoryRegionInfo &region) {
+Status ScriptedProcess::DoGetMemoryRegionInfo(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
index 68cc6aa372104..fd4a94b9b6adb 100644
--- a/lldb/source/Plugins/Process/scripted/ScriptedProcess.h
+++ b/lldb/source/Plugins/Process/scripted/ScriptedProcess.h
@@ -86,9 +86,6 @@ 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;
 
@@ -102,6 +99,9 @@ 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
index d901d523a1036..1ae82289bdbfe 100644
--- a/lldb/source/Target/Process.cpp
+++ b/lldb/source/Target/Process.cpp
@@ -5896,6 +5896,13 @@ 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
new file mode 100644
index 0000000000000..10495940055b6
--- /dev/null
+++ b/lldb/test/API/linux/aarch64/tagged_memory_region/Makefile
@@ -0,0 +1,3 @@
+C_SOURCES := main.c
+
+include Makefile.rules

diff  --git a/lldb/test/API/linux/aarch64/tagged_memory_region/TestAArch64LinuxTaggedMemoryRegion.py b/lldb/test/API/linux/aarch64/tagged_memory_region/TestAArch64LinuxTaggedMemoryRegion.py
new file mode 100644
index 0000000000000..b175f6234b10b
--- /dev/null
+++ b/lldb/test/API/linux/aarch64/tagged_memory_region/TestAArch64LinuxTaggedMemoryRegion.py
@@ -0,0 +1,42 @@
+"""
+Test that "memory region" lookup uses the ABI plugin to remove
+non address bits from addresses before lookup.
+"""
+
+
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class AArch64LinuxTaggedMemoryRegionTestCase(TestBase):
+
+    mydir = TestBase.compute_mydir(__file__)
+
+    NO_DEBUG_INFO_TESTCASE = True
+
+    # AArch64 Linux always enables the top byte ignore feature
+    @skipUnlessArch("aarch64")
+    @skipUnlessPlatform(["linux"])
+    def test_mte_regions(self):
+        self.build()
+        self.runCmd("file " + self.getBuildArtifact("a.out"), CURRENT_EXECUTABLE_SET)
+
+        lldbutil.run_break_set_by_file_and_line(self, "main.c",
+            line_number('main.c', '// Set break point at this line.'),
+            num_expected_locations=1)
+
+        self.runCmd("run", RUN_SUCCEEDED)
+
+        if self.process().GetState() == lldb.eStateExited:
+            self.fail("Test program failed to run.")
+
+        self.expect("thread list", STOPPED_DUE_TO_BREAKPOINT,
+            substrs=['stopped',
+                     'stop reason = breakpoint'])
+
+        # Despite the non address bits we should find a region
+        self.expect("memory region the_page", patterns=[
+            "\[0x[0-9A-Fa-f]+-0x[0-9A-Fa-f]+\) r-x"])

diff  --git a/lldb/test/API/linux/aarch64/tagged_memory_region/main.c b/lldb/test/API/linux/aarch64/tagged_memory_region/main.c
new file mode 100644
index 0000000000000..29f99d73e12d1
--- /dev/null
+++ b/lldb/test/API/linux/aarch64/tagged_memory_region/main.c
@@ -0,0 +1,17 @@
+#include <asm/hwcap.h>
+#include <sys/auxv.h>
+#include <sys/mman.h>
+#include <unistd.h>
+
+int main(int argc, char const *argv[]) {
+  void *the_page = mmap(0, sysconf(_SC_PAGESIZE), PROT_READ | PROT_EXEC,
+                        MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
+  if (the_page == MAP_FAILED)
+    return 1;
+
+  // Put something in the top byte (AArch64 Linux always enables top byte
+  // ignore)
+  the_page = (void *)((size_t)the_page | ((size_t)0x34 << 56));
+
+  return 0; // Set break point at this line.
+}


        


More information about the lldb-commits mailing list