[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

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

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);

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 &section_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

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

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.")

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.
+}