DavidSpickett updated this revision to Diff 417960.
DavidSpickett added a comment.
Herald added a project: All.
Switch to removing non-address bits in lldb instead of lldb-server.
The breakpoint issues I mention only really happen if you try to break on a
tagged
function pointer. Which is pretty niche, but I hope to address it later anyway.
On the issue of whether to use FixData vs FixCode there's 2 maybe 3 ways to go:
- Assume that they're the same, which does work for Linux, for now.
- Add a method that does both fixes, on the assumption that the virtual address
size for code and data is the same so no harm done and all bits will be removed
either way.
- Extensively track whether addresses refer to code or data. In some situations
this is possible (looking at the exec bits of a memory mapping for example) but
I don't have a great idea what that looks like at this time.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D118794/new/
https://reviews.llvm.org/D118794
Files:
lldb/source/Target/Process.cpp
lldb/source/Target/ProcessTrace.cpp
lldb/source/Target/Target.cpp
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
Index: lldb/test/API/linux/aarch64/non_address_bit_memory_access/main.c
===================================================================
--- /dev/null
+++ 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.
+}
Index: lldb/test/API/linux/aarch64/non_address_bit_memory_access/TestAArch64LinuxNonAddressBitMemoryAccess.py
===================================================================
--- /dev/null
+++ lldb/test/API/linux/aarch64/non_address_bit_memory_access/TestAArch64LinuxNonAddressBitMemoryAccess.py
@@ -0,0 +1,177 @@
+"""
+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")
+
+ 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.")
Index: lldb/test/API/linux/aarch64/non_address_bit_memory_access/Makefile
===================================================================
--- /dev/null
+++ 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
Index: lldb/source/Target/Target.cpp
===================================================================
--- lldb/source/Target/Target.cpp
+++ lldb/source/Target/Target.cpp
@@ -1732,6 +1732,12 @@
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->FixDataAddress(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,26 @@
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(); // "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(); // "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
Index: lldb/source/Target/ProcessTrace.cpp
===================================================================
--- lldb/source/Target/ProcessTrace.cpp
+++ 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 @@
size_t ProcessTrace::ReadMemory(addr_t addr, void *buf, size_t size,
Status &error) {
+ if (const ABISP &abi = GetABI())
+ addr = abi->FixDataAddress(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);
Index: lldb/source/Target/Process.cpp
===================================================================
--- lldb/source/Target/Process.cpp
+++ lldb/source/Target/Process.cpp
@@ -1919,6 +1919,11 @@
//#define VERIFY_MEMORY_READS
size_t Process::ReadMemory(addr_t addr, void *buf, size_t size, Status &error) {
+ // We are assuming that fixing a code or data address is the same. This
+ // may not be true in future.
+ if (ABISP abi_sp = GetABI())
+ addr = abi_sp->FixDataAddress(addr);
+
error.Clear();
if (!GetDisableMemoryCache()) {
#if defined(VERIFY_MEMORY_READS)
@@ -2031,6 +2036,9 @@
Status &error) {
LLDB_SCOPED_TIMER();
+ if (ABISP abi_sp = GetABI())
+ addr = abi_sp->FixDataAddress(addr);
+
if (buf == nullptr || size == 0)
return 0;
@@ -2113,6 +2121,9 @@
size_t Process::WriteMemory(addr_t addr, const void *buf, size_t size,
Status &error) {
+ if (ABISP abi_sp = GetABI())
+ addr = abi_sp->FixDataAddress(addr);
+
#if defined(ENABLE_MEMORY_CACHING)
m_memory_cache.Flush(addr, size);
#endif
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
