[lldb][AArch64] Fix corefile memory reads when there are non-address bits

Previously if you read a code/data mask before there was a valid thread
you would get the top byte mask. This meant the value was "valid" as in,
don't read it again.

When using a corefile we ask for the data mask very early on and this
meant that later once you did have a thread it wouldn't read the
register to get the rest of the mask.

This fixes that and adds a corefile test generated from the same program
as in my previous change on this theme.

Depends on D118794

Reviewed By: omjavaid

Differential Revision: https://reviews.llvm.org/D122411

lldb/source/Plugins/ABI/AArch64/ABISysV_arm64.cpp

@@ -794,14 +794,20 @@ lldb::addr_t ABISysV_arm64::FixAddress(addr_t pc, addr_t mask) {
 // Reads code or data address mask for the current Linux process.
 static lldb::addr_t ReadLinuxProcessAddressMask(lldb::ProcessSP process_sp,
                                                 llvm::StringRef reg_name) {
-  // Linux configures user-space virtual addresses with top byte ignored.
-  // We set default value of mask such that top byte is masked out.
-  uint64_t address_mask = ~((1ULL << 56) - 1);
-  // If Pointer Authentication feature is enabled then Linux exposes
-  // PAC data and code mask register. Try reading relevant register
-  // below and merge it with default address mask calculated above.
+  // 0 means there isn't a mask or it has not been read yet.
+  // We do not return the top byte mask unless thread_sp is valid.
+  // This prevents calls to this function before the thread is setup locking
+  // in the value to just the top byte mask, in cases where pointer
+  // authentication might also be active.
+  uint64_t address_mask = 0;
   lldb::ThreadSP thread_sp = process_sp->GetThreadList().GetSelectedThread();
   if (thread_sp) {
+    // Linux configures user-space virtual addresses with top byte ignored.
+    // We set default value of mask such that top byte is masked out.
+    address_mask = ~((1ULL << 56) - 1);
+    // If Pointer Authentication feature is enabled then Linux exposes
+    // PAC data and code mask register. Try reading relevant register
+    // below and merge it with default address mask calculated above.
     lldb::RegisterContextSP reg_ctx_sp = thread_sp->GetRegisterContext();
     if (reg_ctx_sp) {
       const RegisterInfo *reg_info =

lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp

@@ -15,6 +15,7 @@
 #include "lldb/Core/ModuleSpec.h"
 #include "lldb/Core/PluginManager.h"
 #include "lldb/Core/Section.h"
+#include "lldb/Target/ABI.h"
 #include "lldb/Target/DynamicLoader.h"
 #include "lldb/Target/MemoryRegionInfo.h"
 #include "lldb/Target/Target.h"
@@ -281,6 +282,9 @@ bool ProcessElfCore::IsAlive() { return true; }
 // Process Memory
 size_t ProcessElfCore::ReadMemory(lldb::addr_t addr, void *buf, size_t size,
                                   Status &error) {
+  if (lldb::ABISP abi_sp = GetABI())
+    addr = abi_sp->FixAnyAddress(addr);
+
   // Don't allow the caching that lldb_private::Process::ReadMemory does since
   // in core files we have it all cached our our core file anyway.
   return DoReadMemory(addr, buf, size, error);

lldb/test/API/linux/aarch64/non_address_bit_memory_access/TestAArch64LinuxNonAddressBitMemoryAccess.py

@@ -163,6 +163,10 @@ class AArch64LinuxNonAddressBitMemoryAccessTestCase(TestBase):
         # Open log ignoring utf-8 decode errors
         with open(log_file, 'r', errors='ignore') as f:
             read_packet = "send packet: $x{:x}"
+
+            # Since we allocated a 4k page that page will be aligned to 4k, which
+            # also fits the 512 byte line size of the memory cache. So we can expect
+            # to find a packet with its exact address.
             read_buf_packet = read_packet.format(buf)
             read_buf_with_non_address_packet = read_packet.format(buf_with_non_address)
 
@@ -180,3 +184,51 @@ class AArch64LinuxNonAddressBitMemoryAccessTestCase(TestBase):
 
             if not found_read_buf:
                 self.fail("Did not find any reads of buf.")
+
+    @skipIfLLVMTargetMissing("AArch64")
+    def test_non_address_bit_memory_corefile(self):
+        self.runCmd("target create --core corefile")
+
+        self.expect("thread list", substrs=['stopped',
+                                            'stop reason = signal SIGSEGV'])
+
+        # No caching (the program/corefile are the cache) and no writing
+        # to memory. So just check that tagged/untagged addresses read
+        # the same location.
+
+        # These are known addresses in the corefile, since we won't have symbols.
+        buf = 0x0000ffffa75a5000
+        buf_with_non_address = 0xff0bffffa75a5000
+
+        expected = ["4c 4c 44 42", "LLDB"]
+        self.expect("memory read 0x{:x}".format(buf), substrs=expected)
+        self.expect("memory read 0x{:x}".format(buf_with_non_address), substrs=expected)
+
+        # This takes a more direct route to ReadMemory. As opposed to "memory read"
+        # above that might fix the addresses up front for display reasons.
+        self.expect("expression (char*)0x{:x}".format(buf), substrs=["LLDB"])
+        self.expect("expression (char*)0x{:x}".format(buf_with_non_address), substrs=["LLDB"])
+
+        def check_reads(addrs, method, num_bytes, expected):
+            error = lldb.SBError()
+            for addr in addrs:
+                if num_bytes is None:
+                    got = method(addr, error)
+                else:
+                    got = method(addr, num_bytes, error)
+
+            self.assertTrue(error.Success())
+            self.assertEqual(expected, got)
+
+        addr_buf = lldb.SBAddress()
+        addr_buf.SetLoadAddress(buf, self.target())
+        addr_buf_with_non_address = lldb.SBAddress()
+        addr_buf_with_non_address.SetLoadAddress(buf_with_non_address, self.target())
+        check_reads([addr_buf, addr_buf_with_non_address], self.target().ReadMemory,
+                    4, b'LLDB')
+
+        addrs = [buf, buf_with_non_address]
+        check_reads(addrs, self.process().ReadMemory, 4, b'LLDB')
+        check_reads(addrs, self.process().ReadCStringFromMemory, 5, "LLDB")
+        check_reads(addrs, self.process().ReadUnsignedFromMemory, 4, 0x42444c4c)
+        check_reads(addrs, self.process().ReadPointerFromMemory, None, 0x0000000042444c4c)

lldb/test/API/linux/aarch64/non_address_bit_memory_access/main.c

@@ -13,6 +13,13 @@ int main(int argc, char const *argv[]) {
   if (buf == MAP_FAILED)
     return 1;
 
+  // Some known values to go in the corefile, since we cannot
+  // write to corefile memory.
+  buf[0] = 'L';
+  buf[1] = 'L';
+  buf[2] = 'D';
+  buf[3] = 'B';
+
 #define sign_ptr(ptr) __asm__ __volatile__("pacdza %0" : "=r"(ptr) : "r"(ptr))
 
   // Set top byte to something.
@@ -21,5 +28,11 @@ int main(int argc, char const *argv[]) {
   // Address is now:
   // <8 bit top byte tag><pointer signature><virtual address>
 
+  // Uncomment this line to crash and generate a corefile.
+  // Prints so we know what fixed address to look for in testing.
+  // printf("buf: %p\n", buf);
+  // printf("buf_with_non_address: %p\n", buf_with_non_address);
+  // *(char*)0 = 0;
+
   return 0; // Set break point at this line.
 }