DavidSpickett created this revision. Herald added a subscriber: kristof.beyls. DavidSpickett requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits.
This reverts commit 0df522969a7a0128052bd79182c8d58e00556e2f <https://reviews.llvm.org/rG0df522969a7a0128052bd79182c8d58e00556e2f>. Additional checks are added to fix the detection of the last memory region in GetMemoryRegions or repeating the "memory region" command when the target has non-address bits. Normally you keep reading from address 0, looking up each region's end address until you get LLDB_INVALID_ADDR as the region end address. (0xffffffffffffffff) This is what the remote will return once you go beyond the last mapped region: [0x0000fffffffdf000-0x0001000000000000) rw- [stack] [0x0001000000000000-0xffffffffffffffff) --- Problem is that when we "fix" the lookup address, we remove some bits from it. On an AArch64 system we have 48 bit virtual addresses, so when we fix the end address of the [stack] region the result is 0. So we loop back to the start. [0x0000fffffffdf000-0x0001000000000000) rw- [stack] [0x0000000000000000-0x0000000000400000) --- To fix this I added an additional check for the last range. If the end address of the region is different once you apply FixDataAddress, we are at the last region. Since the end of the last region will be the last valid mappable address, plus 1. That 1 will be removed by the ABI plugin. The only side effect is that on systems with non-address bits, you won't get that last catch all unmapped region from the max virtual address up to 0xf...f. [0x0000fffff8000000-0x0000fffffffdf000) --- [0x0000fffffffdf000-0x0001000000000000) rw- [stack] <ends here> Though in some way this is more correct because that region is not just unmapped, it's not mappable at all. No extra testing is needed because this is already covered by TestMemoryRegion.py, I simply forgot to run it on system that had both top byte ignore and pointer authentication. This change has been tested on a qemu VM with top byte ignore, memory tagging and pointer authentication enabled. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D115508 Files: lldb/include/lldb/Target/Process.h lldb/source/Commands/CommandObjectMemory.cpp 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 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
Index: lldb/test/API/linux/aarch64/tagged_memory_region/main.c =================================================================== --- /dev/null +++ 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. +} Index: lldb/test/API/linux/aarch64/tagged_memory_region/TestAArch64LinuxTaggedMemoryRegion.py =================================================================== --- /dev/null +++ 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"]) Index: lldb/test/API/linux/aarch64/tagged_memory_region/Makefile =================================================================== --- /dev/null +++ lldb/test/API/linux/aarch64/tagged_memory_region/Makefile @@ -0,0 +1,3 @@ +C_SOURCES := main.c + +include Makefile.rules Index: lldb/source/Target/Process.cpp =================================================================== --- lldb/source/Target/Process.cpp +++ lldb/source/Target/Process.cpp @@ -5853,15 +5853,23 @@ return retval; } +Status Process::GetMemoryRegionInfo(lldb::addr_t load_addr, + MemoryRegionInfo &range_info) { + if (const lldb::ABISP &abi = GetABI()) + load_addr = abi->FixDataAddress(load_addr); + return DoGetMemoryRegionInfo(load_addr, range_info); +} + Status Process::GetMemoryRegions(lldb_private::MemoryRegionInfos ®ion_list) { Status error; lldb::addr_t range_end = 0; + const lldb::ABISP &abi = GetABI(); region_list.clear(); - do { + while (1) { lldb_private::MemoryRegionInfo region_info; error = GetMemoryRegionInfo(range_end, region_info); // GetMemoryRegionInfo should only return an error if it is unimplemented. @@ -5870,11 +5878,22 @@ break; } - range_end = region_info.GetRange().GetRangeEnd(); if (region_info.GetMapped() == MemoryRegionInfo::eYes) { region_list.push_back(std::move(region_info)); } - } while (range_end != LLDB_INVALID_ADDRESS); + + // We detect the last region in 2 ways. + // 1. Its end address has all bits set. + // (this happens when a target doesn't have any "non-address" bits) + // 2. Its end address includes bits set that are known to be + // "non-address" bits. Meaning that no process can expect + // to map memory with an address with those bits set. + range_end = region_info.GetRange().GetRangeEnd(); + if (range_end == LLDB_INVALID_ADDRESS || + (abi && (abi->FixDataAddress(range_end) != range_end))) { + break; + } + } return error; } Index: lldb/source/Plugins/Process/scripted/ScriptedProcess.h =================================================================== --- lldb/source/Plugins/Process/scripted/ScriptedProcess.h +++ lldb/source/Plugins/Process/scripted/ScriptedProcess.h @@ -84,9 +84,6 @@ ArchSpec GetArchitecture(); - Status GetMemoryRegionInfo(lldb::addr_t load_addr, - MemoryRegionInfo &range_info) override; - Status GetMemoryRegions(lldb_private::MemoryRegionInfos ®ion_list) override; @@ -100,6 +97,9 @@ 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; Index: lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp =================================================================== --- lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp +++ lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp @@ -248,8 +248,8 @@ return GetTarget().GetArchitecture(); } -Status ScriptedProcess::GetMemoryRegionInfo(lldb::addr_t load_addr, - MemoryRegionInfo ®ion) { +Status ScriptedProcess::DoGetMemoryRegionInfo(lldb::addr_t load_addr, + MemoryRegionInfo ®ion) { CheckInterpreterAndScriptObject(); Status error; Index: lldb/source/Plugins/Process/minidump/ProcessMinidump.h =================================================================== --- lldb/source/Plugins/Process/minidump/ProcessMinidump.h +++ lldb/source/Plugins/Process/minidump/ProcessMinidump.h @@ -75,9 +75,6 @@ ArchSpec GetArchitecture(); - Status GetMemoryRegionInfo(lldb::addr_t load_addr, - MemoryRegionInfo &range_info) override; - Status GetMemoryRegions( lldb_private::MemoryRegionInfos ®ion_list) override; @@ -98,6 +95,9 @@ 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, Index: lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp =================================================================== --- lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp +++ lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp @@ -439,8 +439,8 @@ llvm::sort(*m_memory_regions); } -Status ProcessMinidump::GetMemoryRegionInfo(lldb::addr_t load_addr, - MemoryRegionInfo ®ion) { +Status ProcessMinidump::DoGetMemoryRegionInfo(lldb::addr_t load_addr, + MemoryRegionInfo ®ion) { BuildMemoryRegions(); region = MinidumpParser::GetMemoryRegionInfo(*m_memory_regions, load_addr); return Status(); Index: lldb/source/Plugins/Process/mach-core/ProcessMachCore.h =================================================================== --- lldb/source/Plugins/Process/mach-core/ProcessMachCore.h +++ lldb/source/Plugins/Process/mach-core/ProcessMachCore.h @@ -68,10 +68,6 @@ 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 ®ion_info) override; - lldb::addr_t GetImageInfoAddress() override; protected: @@ -84,6 +80,10 @@ lldb_private::ObjectFile *GetCoreObjectFile(); + lldb_private::Status + DoGetMemoryRegionInfo(lldb::addr_t load_addr, + lldb_private::MemoryRegionInfo ®ion_info) override; + private: bool GetDynamicLoaderAddress(lldb::addr_t addr); Index: lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp =================================================================== --- lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp +++ lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp @@ -633,8 +633,8 @@ return bytes_read; } -Status ProcessMachCore::GetMemoryRegionInfo(addr_t load_addr, - MemoryRegionInfo ®ion_info) { +Status ProcessMachCore::DoGetMemoryRegionInfo(addr_t load_addr, + MemoryRegionInfo ®ion_info) { region_info.Clear(); const VMRangeToPermissions::Entry *permission_entry = m_core_range_infos.FindEntryThatContainsOrFollows(load_addr); Index: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h =================================================================== --- lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h +++ lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h @@ -144,9 +144,6 @@ lldb::addr_t DoAllocateMemory(size_t size, uint32_t permissions, Status &error) override; - Status GetMemoryRegionInfo(lldb::addr_t load_addr, - MemoryRegionInfo ®ion_info) override; - Status DoDeallocateMemory(lldb::addr_t ptr) override; // Process STDIO @@ -420,6 +417,9 @@ 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 ®ion_info) override; + private: // For ProcessGDBRemote only std::string m_partial_profile_data; Index: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp =================================================================== --- lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp +++ lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp @@ -2877,8 +2877,8 @@ return allocated_addr; } -Status ProcessGDBRemote::GetMemoryRegionInfo(addr_t load_addr, - MemoryRegionInfo ®ion_info) { +Status ProcessGDBRemote::DoGetMemoryRegionInfo(addr_t load_addr, + MemoryRegionInfo ®ion_info) { Status error(m_gdb_comm.GetMemoryRegionInfo(load_addr, region_info)); return error; Index: lldb/source/Plugins/Process/elf-core/ProcessElfCore.h =================================================================== --- lldb/source/Plugins/Process/elf-core/ProcessElfCore.h +++ lldb/source/Plugins/Process/elf-core/ProcessElfCore.h @@ -86,10 +86,6 @@ 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 ®ion_info) override; - lldb::addr_t GetImageInfoAddress() override; lldb_private::ArchSpec GetArchitecture(); @@ -105,6 +101,10 @@ 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 ®ion_info) override; + private: struct NT_FILE_Entry { lldb::addr_t start; Index: lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp =================================================================== --- lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp +++ lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp @@ -285,8 +285,8 @@ return DoReadMemory(addr, buf, size, error); } -Status ProcessElfCore::GetMemoryRegionInfo(lldb::addr_t load_addr, - MemoryRegionInfo ®ion_info) { +Status ProcessElfCore::DoGetMemoryRegionInfo(lldb::addr_t load_addr, + MemoryRegionInfo ®ion_info) { region_info.Clear(); const VMRangeToPermissions::Entry *permission_entry = m_core_range_infos.FindEntryThatContainsOrFollows(load_addr); Index: lldb/source/Plugins/Process/Windows/Common/ProcessWindows.h =================================================================== --- lldb/source/Plugins/Process/Windows/Common/ProcessWindows.h +++ lldb/source/Plugins/Process/Windows/Common/ProcessWindows.h @@ -78,8 +78,6 @@ 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 @@ 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; Index: lldb/source/Plugins/Process/Windows/Common/ProcessWindows.cpp =================================================================== --- lldb/source/Plugins/Process/Windows/Common/ProcessWindows.cpp +++ lldb/source/Plugins/Process/Windows/Common/ProcessWindows.cpp @@ -601,8 +601,8 @@ 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); } Index: lldb/source/Commands/CommandObjectMemory.cpp =================================================================== --- lldb/source/Commands/CommandObjectMemory.cpp +++ lldb/source/Commands/CommandObjectMemory.cpp @@ -23,6 +23,7 @@ #include "lldb/Interpreter/Options.h" #include "lldb/Symbol/SymbolFile.h" #include "lldb/Symbol/TypeList.h" +#include "lldb/Target/ABI.h" #include "lldb/Target/Language.h" #include "lldb/Target/MemoryHistory.h" #include "lldb/Target/MemoryRegionInfo.h" @@ -1650,7 +1651,21 @@ m_prev_end_addr = LLDB_INVALID_ADDRESS; const size_t argc = command.GetArgumentCount(); - if (argc > 1 || (argc == 0 && load_addr == LLDB_INVALID_ADDRESS)) { + const lldb::ABISP &abi = process_sp->GetABI(); + // When we're repeating the command, the previous end address is used for + // load_addr. If that was 0xF...F then we must have reached the end of + // memory. + // + // If the target has non-address bits (tags, limited virtual address + // size, etc.), the end of mappable memory will be lower than that. So if we + // find any non-address bit set, then we must be at the end of the mappable + // range. + // + // We do not apply this when you supply a load address argument because + // those are pointers that can legitimately contain non-address bits, + // which we'll handle in GetMemoryRegion. + if (argc > 1 || (argc == 0 && load_addr == LLDB_INVALID_ADDRESS) || + (abi && (abi->FixDataAddress(load_addr) != load_addr))) { result.AppendErrorWithFormat("'%s' takes one argument:\nUsage: %s\n", m_cmd_name.c_str(), m_cmd_syntax.c_str()); return false; Index: lldb/include/lldb/Target/Process.h =================================================================== --- lldb/include/lldb/Target/Process.h +++ lldb/include/lldb/Target/Process.h @@ -1762,7 +1762,7 @@ /// /// 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,23 +1771,21 @@ /// 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. /// @@ -2607,6 +2605,26 @@ 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
_______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits