[Lldb-commits] [lldb] d9398a9 - [lldb] Remove non-address bits from read/write addresses in lldb
David Spickett via lldb-commits
lldb-commits at lists.llvm.org
Wed May 18 04:59:39 PDT 2022
Author: David Spickett
Date: 2022-05-18T12:59:34+01:00
New Revision: d9398a91e2a6b8837a47a5fda2164c9160e86199
URL: https://github.com/llvm/llvm-project/commit/d9398a91e2a6b8837a47a5fda2164c9160e86199
DIFF: https://github.com/llvm/llvm-project/commit/d9398a91e2a6b8837a47a5fda2164c9160e86199.diff
LOG: [lldb] Remove non-address bits from read/write addresses in lldb
Non-address bits are not part of the virtual address in a pointer.
So they must be removed before passing to interfaces like ptrace.
Some of them we get way with not removing, like AArch64's top byte.
However this is only because of a hardware feature that ignores them.
This change updates all the Process/Target Read/Write memory methods
to remove non-address bits before using addresses.
Doing it in this way keeps lldb-server simple and also fixes the
memory caching when differently tagged pointers for the same location
are read.
Removing the bits is done at the ReadMemory level not DoReadMemory
because particualrly for process, many subclasses override DoReadMemory.
Tests have been added for read/write at the command and API level,
for process and target. This includes variants like
Read<sometype>FromMemory. Commands are tested to make sure we remove
at the command and API level.
"memory find" is not included because:
* There is no API for it.
* It already has its own address handling tests.
Software breakpoints do use these methods but they are not tested
here because there are bigger issues to fix with those. This will
happen in another change.
Reviewed By: omjavaid
Differential Revision: https://reviews.llvm.org/D118794
Added:
lldb/test/API/linux/aarch64/non_address_bit_memory_access/Makefile
lldb/test/API/linux/aarch64/non_address_bit_memory_access/TestAArch64LinuxNonAddressBitMemoryAccess.py
lldb/test/API/linux/aarch64/non_address_bit_memory_access/main.c
Modified:
lldb/source/Target/Process.cpp
lldb/source/Target/ProcessTrace.cpp
lldb/source/Target/Target.cpp
Removed:
################################################################################
diff --git a/lldb/source/Target/Process.cpp b/lldb/source/Target/Process.cpp
index 79adb2c9d155..1f62e95377f4 100644
--- a/lldb/source/Target/Process.cpp
+++ b/lldb/source/Target/Process.cpp
@@ -1919,6 +1919,9 @@ Status Process::DisableSoftwareBreakpoint(BreakpointSite *bp_site) {
//#define VERIFY_MEMORY_READS
size_t Process::ReadMemory(addr_t addr, void *buf, size_t size, Status &error) {
+ if (ABISP abi_sp = GetABI())
+ addr = abi_sp->FixAnyAddress(addr);
+
error.Clear();
if (!GetDisableMemoryCache()) {
#if defined(VERIFY_MEMORY_READS)
@@ -2031,6 +2034,9 @@ size_t Process::ReadMemoryFromInferior(addr_t addr, void *buf, size_t size,
Status &error) {
LLDB_SCOPED_TIMER();
+ if (ABISP abi_sp = GetABI())
+ addr = abi_sp->FixAnyAddress(addr);
+
if (buf == nullptr || size == 0)
return 0;
@@ -2113,6 +2119,9 @@ size_t Process::WriteMemoryPrivate(addr_t addr, const void *buf, size_t size,
size_t Process::WriteMemory(addr_t addr, const void *buf, size_t size,
Status &error) {
+ if (ABISP abi_sp = GetABI())
+ addr = abi_sp->FixAnyAddress(addr);
+
#if defined(ENABLE_MEMORY_CACHING)
m_memory_cache.Flush(addr, size);
#endif
diff --git a/lldb/source/Target/ProcessTrace.cpp b/lldb/source/Target/ProcessTrace.cpp
index 41d5b01b61d8..6b147cb285a7 100644
--- a/lldb/source/Target/ProcessTrace.cpp
+++ b/lldb/source/Target/ProcessTrace.cpp
@@ -13,6 +13,7 @@
#include "lldb/Core/Module.h"
#include "lldb/Core/PluginManager.h"
#include "lldb/Core/Section.h"
+#include "lldb/Target/ABI.h"
#include "lldb/Target/SectionLoadList.h"
#include "lldb/Target/Target.h"
@@ -80,6 +81,9 @@ Status ProcessTrace::DoDestroy() { return Status(); }
size_t ProcessTrace::ReadMemory(addr_t addr, void *buf, size_t size,
Status &error) {
+ if (const ABISP &abi = GetABI())
+ addr = abi->FixAnyAddress(addr);
+
// Don't allow the caching that lldb_private::Process::ReadMemory does since
// we have it all cached in the trace files.
return DoReadMemory(addr, buf, size, error);
diff --git a/lldb/source/Target/Target.cpp b/lldb/source/Target/Target.cpp
index ed733f2645c3..993e7efd02ba 100644
--- a/lldb/source/Target/Target.cpp
+++ b/lldb/source/Target/Target.cpp
@@ -1732,6 +1732,12 @@ size_t Target::ReadMemory(const Address &addr, void *dst, size_t dst_len,
lldb::addr_t *load_addr_ptr) {
error.Clear();
+ Address fixed_addr = addr;
+ if (ProcessIsValid())
+ if (const ABISP &abi = m_process_sp->GetABI())
+ fixed_addr.SetLoadAddress(abi->FixAnyAddress(addr.GetLoadAddress(this)),
+ this);
+
// if we end up reading this from process memory, we will fill this with the
// actual load address
if (load_addr_ptr)
@@ -1742,26 +1748,28 @@ size_t Target::ReadMemory(const Address &addr, void *dst, size_t dst_len,
addr_t load_addr = LLDB_INVALID_ADDRESS;
addr_t file_addr = LLDB_INVALID_ADDRESS;
Address resolved_addr;
- if (!addr.IsSectionOffset()) {
+ if (!fixed_addr.IsSectionOffset()) {
SectionLoadList §ion_load_list = GetSectionLoadList();
if (section_load_list.IsEmpty()) {
// No sections are loaded, so we must assume we are not running yet and
// anything we are given is a file address.
- file_addr = addr.GetOffset(); // "addr" doesn't have a section, so its
- // offset is the file address
+ file_addr =
+ fixed_addr.GetOffset(); // "fixed_addr" doesn't have a section, so
+ // its offset is the file address
m_images.ResolveFileAddress(file_addr, resolved_addr);
} else {
// We have at least one section loaded. This can be because we have
// manually loaded some sections with "target modules load ..." or
// because we have have a live process that has sections loaded through
// the dynamic loader
- load_addr = addr.GetOffset(); // "addr" doesn't have a section, so its
- // offset is the load address
+ load_addr =
+ fixed_addr.GetOffset(); // "fixed_addr" doesn't have a section, so
+ // its offset is the load address
section_load_list.ResolveLoadAddress(load_addr, resolved_addr);
}
}
if (!resolved_addr.IsValid())
- resolved_addr = addr;
+ resolved_addr = fixed_addr;
// If we read from the file cache but can't get as many bytes as requested,
// we keep the result around in this buffer, in case this result is the
diff --git a/lldb/test/API/linux/aarch64/non_address_bit_memory_access/Makefile b/lldb/test/API/linux/aarch64/non_address_bit_memory_access/Makefile
new file mode 100644
index 000000000000..01745f1889ea
--- /dev/null
+++ b/lldb/test/API/linux/aarch64/non_address_bit_memory_access/Makefile
@@ -0,0 +1,4 @@
+C_SOURCES := main.c
+CFLAGS_EXTRAS := -march=armv8.5-a+memtag
+
+include Makefile.rules
diff --git a/lldb/test/API/linux/aarch64/non_address_bit_memory_access/TestAArch64LinuxNonAddressBitMemoryAccess.py b/lldb/test/API/linux/aarch64/non_address_bit_memory_access/TestAArch64LinuxNonAddressBitMemoryAccess.py
new file mode 100644
index 000000000000..b9b5b54d4a30
--- /dev/null
+++ b/lldb/test/API/linux/aarch64/non_address_bit_memory_access/TestAArch64LinuxNonAddressBitMemoryAccess.py
@@ -0,0 +1,182 @@
+"""
+Test that lldb removes non-address bits in situations where they would cause
+failures if not removed. Like when reading memory. Tests are done at command
+and API level because commands may remove non-address bits for display
+reasons which can make it seem like the operation as a whole works but at the
+API level it won't if we don't remove them there also.
+"""
+
+
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class AArch64LinuxNonAddressBitMemoryAccessTestCase(TestBase):
+
+ mydir = TestBase.compute_mydir(__file__)
+
+ NO_DEBUG_INFO_TESTCASE = True
+
+ def setup_test(self):
+ if not self.isAArch64PAuth():
+ self.skipTest('Target must support pointer authentication.')
+
+ 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'])
+
+ def check_cmd_read_write(self, write_to, read_from, data):
+ self.runCmd("memory write {} {}".format(write_to, data))
+ self.expect("memory read {}".format(read_from),
+ substrs=[data])
+
+ @skipUnlessArch("aarch64")
+ @skipUnlessPlatform(["linux"])
+ def test_non_address_bit_memory_read_write_cmds(self):
+ self.setup_test()
+
+ # Writes should be visible through either pointer
+ self.check_cmd_read_write("buf", "buf", "01 02 03 04")
+ self.check_cmd_read_write("buf_with_non_address", "buf_with_non_address", "02 03 04 05")
+ self.check_cmd_read_write("buf", "buf_with_non_address", "03 04 05 06")
+ self.check_cmd_read_write("buf_with_non_address", "buf", "04 05 06 07")
+
+ # Printing either should get the same result
+ self.expect("expression -f hex -- *(uint32_t*)buf", substrs=["0x07060504"])
+ self.expect("expression -f hex -- *(uint32_t*)buf_with_non_address",
+ substrs=["0x07060504"])
+
+ def get_ptr_values(self):
+ frame = self.process().GetThreadAtIndex(0).GetFrameAtIndex(0)
+ buf = frame.FindVariable("buf").GetValueAsUnsigned()
+ buf_with_non_address = frame.FindVariable("buf_with_non_address").GetValueAsUnsigned()
+ return buf, buf_with_non_address
+
+ def check_api_read_write(self, write_to, read_from, data):
+ error = lldb.SBError()
+ written = self.process().WriteMemory(write_to, data, error)
+ self.assertTrue(error.Success())
+ self.assertEqual(len(data), written)
+ buf_content = self.process().ReadMemory(read_from, 4, error)
+ self.assertTrue(error.Success())
+ self.assertEqual(data, buf_content)
+
+ @skipUnlessArch("aarch64")
+ @skipUnlessPlatform(["linux"])
+ def test_non_address_bit_memory_read_write_api_process(self):
+ self.setup_test()
+ buf, buf_with_non_address = self.get_ptr_values()
+
+ # Writes are visible through either pointer
+ self.check_api_read_write(buf, buf, bytes([0, 1, 2, 3]))
+ self.check_api_read_write(buf_with_non_address, buf_with_non_address, bytes([1, 2, 3, 4]))
+ self.check_api_read_write(buf, buf_with_non_address, bytes([2, 3, 4, 5]))
+ self.check_api_read_write(buf_with_non_address, buf, bytes([3, 4, 5, 6]))
+
+ # Now check all the "Read<type>FromMemory" don't fail
+ error = lldb.SBError()
+ # Last 4 bytes are just for the pointer read
+ data = bytes([0x4C, 0x4C, 0x44, 0x42, 0x00, 0x12, 0x34, 0x56])
+ written = self.process().WriteMemory(buf, data, error)
+ self.assertTrue(error.Success())
+ self.assertEqual(len(data), written)
+
+ # C string
+ c_string = self.process().ReadCStringFromMemory(buf_with_non_address, 5, error)
+ self.assertTrue(error.Success())
+ self.assertEqual("LLDB", c_string)
+
+ # Unsigned
+ unsigned_num = self.process().ReadUnsignedFromMemory(buf_with_non_address, 4, error)
+ self.assertTrue(error.Success())
+ self.assertEqual(0x42444c4c, unsigned_num)
+
+ # Pointer
+ ptr = self.process().ReadPointerFromMemory(buf_with_non_address, error)
+ self.assertTrue(error.Success())
+ self.assertEqual(0x5634120042444c4c, ptr)
+
+ @skipUnlessArch("aarch64")
+ @skipUnlessPlatform(["linux"])
+ def test_non_address_bit_memory_read_write_api_target(self):
+ self.setup_test()
+ buf, buf_with_non_address = self.get_ptr_values()
+
+ # Target only has ReadMemory
+ error = lldb.SBError()
+ data = bytes([1, 2, 3, 4])
+ written = self.process().WriteMemory(buf, data, error)
+ self.assertTrue(error.Success())
+ self.assertEqual(len(data), written)
+
+ addr = lldb.SBAddress()
+ addr.SetLoadAddress(buf, self.target())
+ buf_read = self.target().ReadMemory(addr, 4, error)
+ self.assertTrue(error.Success())
+ self.assertEqual(data, buf_read)
+
+ addr.SetLoadAddress(buf_with_non_address, self.target())
+ buf_non_address_read = self.target().ReadMemory(addr, 4, error)
+ self.assertTrue(error.Success())
+ self.assertEqual(data, buf_non_address_read)
+
+ # Read<type>FromMemory are in Target but not SBTarget so no tests for those.
+
+ @skipUnlessArch("aarch64")
+ @skipUnlessPlatform(["linux"])
+ def test_non_address_bit_memory_caching(self):
+ # The read/write tests above do exercise the cache but this test
+ # only cares that the cache sees buf and buf_with_non_address
+ # as the same location.
+ self.setup_test()
+ buf, buf_with_non_address = self.get_ptr_values()
+
+ # Enable packet logging so we can see when reads actually
+ # happen.
+ log_file = self.getBuildArtifact("lldb-non-address-bit-log.txt")
+ # This defaults to overwriting the file so we don't need to delete
+ # any existing files.
+ self.runCmd("log enable gdb-remote packets -f '%s'" % log_file)
+
+ # This should fill the cache by doing a read of buf_with_non_address
+ # with the non-address bits removed (which is == buf).
+ self.runCmd("p buf_with_non_address")
+ # This will read from the cache since the two pointers point to the
+ # same place.
+ self.runCmd("p buf")
+
+ # Open log ignoring utf-8 decode errors
+ with open(log_file, 'r', errors='ignore') as f:
+ read_packet = "send packet: $x{:x}"
+ read_buf_packet = read_packet.format(buf)
+ read_buf_with_non_address_packet = read_packet.format(buf_with_non_address)
+
+ # We expect to find 1 and only 1 read of buf.
+ # We expect to find no reads using buf_with_no_address.
+ found_read_buf = False
+ for line in f:
+ if read_buf_packet in line:
+ if found_read_buf:
+ self.fail("Expected 1 read of buf but found more than one.")
+ found_read_buf = True
+
+ if read_buf_with_non_address_packet in line:
+ self.fail("Unexpected read of buf_with_non_address found.")
+
+ if not found_read_buf:
+ self.fail("Did not find any reads of buf.")
diff --git a/lldb/test/API/linux/aarch64/non_address_bit_memory_access/main.c b/lldb/test/API/linux/aarch64/non_address_bit_memory_access/main.c
new file mode 100644
index 000000000000..222781e72457
--- /dev/null
+++ b/lldb/test/API/linux/aarch64/non_address_bit_memory_access/main.c
@@ -0,0 +1,25 @@
+#include <linux/mman.h>
+#include <sys/mman.h>
+#include <unistd.h>
+
+int main(int argc, char const *argv[]) {
+ size_t page_size = sysconf(_SC_PAGESIZE);
+ // Note that we allocate memory here because if we used
+ // stack or globals lldb might read it in the course of
+ // running to the breakpoint. Before the test can look
+ // for those reads.
+ char *buf = mmap(0, page_size, PROT_READ | PROT_WRITE,
+ MAP_ANONYMOUS | MAP_SHARED, -1, 0);
+ if (buf == MAP_FAILED)
+ return 1;
+
+#define sign_ptr(ptr) __asm__ __volatile__("pacdza %0" : "=r"(ptr) : "r"(ptr))
+
+ // Set top byte to something.
+ char *buf_with_non_address = (char *)((size_t)buf | (size_t)0xff << 56);
+ sign_ptr(buf_with_non_address);
+ // Address is now:
+ // <8 bit top byte tag><pointer signature><virtual address>
+
+ return 0; // Set break point at this line.
+}
More information about the lldb-commits
mailing list