DavidSpickett updated this revision to Diff 399654.
DavidSpickett added a comment.
- Fix bug where the previous end address was checked for non-address bits even
when the user gave a new address.
- Added testing for that situation.
- Added a comment to explain that non-address bits in the start/base of a
memory region are possible but we can ignore them.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D115508/new/
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
llvm/docs/ReleaseNotes.rst
Index: llvm/docs/ReleaseNotes.rst
===================================================================
--- llvm/docs/ReleaseNotes.rst
+++ llvm/docs/ReleaseNotes.rst
@@ -166,6 +166,11 @@
* The ``memory read`` command now ignores non-address bits in start and end
addresses. In addition, non-address bits will not be shown in the addresses
in the output.
+* The ``memory region`` command and ``GetMemoryRegionInfo`` API method now
+ ignore non-address bits in the address parameter. This means that on those
+ systems the last (usually unmapped) memory region will not extend to 0xF...F.
+ Instead it will end at the last address that the process could possibly map
+ if you account for the non-address bits.
Changes to Sanitizers
---------------------
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,70 @@
+"""
+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"])
+
+ # Check that the usual error message is displayed after repeating
+ # the command until the last region.
+ self.runCmd("memory region 0")
+
+ # Count the number of repeats for use in the next check
+ repeats = 0
+ interp = self.dbg.GetCommandInterpreter()
+ result = lldb.SBCommandReturnObject()
+
+ while True:
+ interp.HandleCommand("memory region", result)
+ if result.Succeeded():
+ repeats += 1
+ else:
+ self.assertRegexpMatches(result.GetError(), "Usage: memory region ADDR")
+ break
+
+ # This time repeat until we get the last region. At that
+ # point the previous address will have non-address bits in it.
+ self.runCmd("memory region 0")
+ for i in range(repeats):
+ self.runCmd("memory region")
+
+ # This should not error, since the user supplied address overrides
+ # the previous end address.
+ 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,12 +5853,18 @@
return retval;
}
-Status
-Process::GetMemoryRegions(lldb_private::MemoryRegionInfos ®ion_list) {
+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 {
@@ -5870,11 +5876,22 @@
break;
}
+ // We only check the end address, not start and end, because we assume that
+ // the start will not have non-address bits until the first unmappable
+ // region. We will have exited the loop by that point because the previous
+ // region, the last mappable region, will have non-address bits in its end
+ // address.
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);
+ } while (
+ // For a process with no non-address bits, all address bits
+ // set means the end of memory.
+ range_end != LLDB_INVALID_ADDRESS &&
+ // If we have non-address bits and some are set then the end
+ // is at or beyond the end of mappable memory.
+ !(abi && (abi->FixDataAddress(range_end) != range_end)));
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
@@ -659,8 +659,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
@@ -143,9 +143,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
@@ -415,6 +412,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
@@ -2961,8 +2961,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
@@ -1658,14 +1658,11 @@
m_prev_end_addr = LLDB_INVALID_ADDRESS;
const size_t argc = command.GetArgumentCount();
- if (argc > 1 || (argc == 0 && load_addr == LLDB_INVALID_ADDRESS)) {
- result.AppendErrorWithFormat("'%s' takes one argument:\nUsage: %s\n",
- m_cmd_name.c_str(), m_cmd_syntax.c_str());
- return false;
- }
+ const lldb::ABISP &abi = process_sp->GetABI();
if (argc == 1) {
auto load_addr_str = command[0].ref();
+ // Non-address bits in this will be handled later by GetMemoryRegion
load_addr = OptionArgParser::ToAddress(&m_exe_ctx, load_addr_str,
LLDB_INVALID_ADDRESS, &error);
if (error.Fail() || load_addr == LLDB_INVALID_ADDRESS) {
@@ -1673,6 +1670,19 @@
command[0].c_str(), error.AsCString());
return false;
}
+ } else if (argc > 1 ||
+ // 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.
+ (argc == 0 && load_addr == LLDB_INVALID_ADDRESS) ||
+ // 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, we must be
+ // at the end of the mappable range.
+ (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;
}
lldb_private::MemoryRegionInfo range_info;
Index: lldb/include/lldb/Target/Process.h
===================================================================
--- lldb/include/lldb/Target/Process.h
+++ lldb/include/lldb/Target/Process.h
@@ -1759,7 +1759,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
@@ -1768,23 +1768,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.
///
@@ -2604,6 +2602,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
