[Lldb-commits] [lldb] 787f91b - [lldb] Remove non-address bits from addresses given to memory tag commands

[David Spickett via lldb-commits]

Author: David Spickett
Date: 2022-01-20T10:48:14Z
New Revision: 787f91b0bb34abeca86ad5c8a60ef044a7942996

URL: https://github.com/llvm/llvm-project/commit/787f91b0bb34abeca86ad5c8a60ef044a7942996
DIFF: https://github.com/llvm/llvm-project/commit/787f91b0bb34abeca86ad5c8a60ef044a7942996.diff

LOG: [lldb] Remove non-address bits from addresses given to memory tag commands

Although the memory tag commands use a memory tag manager to handle
addresses, that only removes the top byte.

That top byte is 4 bits of memory tag and 4 free bits, which is more
than it should strictly remove but that's how it is for now.

There are other non-address bit uses like pointer authentication.
To ensure the memory tag manager only has to deal with memory tags,
use the ABI plugin to remove the rest.

The tag access test has been updated to sign all the relevant pointers
and require that we're running on a system with pointer authentication
in addition to memory tagging.

The pointers will look like:
<4 bit user tag><4 bit memory tag><signature><bit virtual address>

Note that there is currently no API for reading memory tags. It will
also have to consider this when it arrives.

Reviewed By: omjavaid

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

Added: 
    

Modified: 
    lldb/source/Commands/CommandObjectMemoryTag.cpp
    lldb/test/API/linux/aarch64/mte_tag_access/TestAArch64LinuxMTEMemoryTagAccess.py
    lldb/test/API/linux/aarch64/mte_tag_access/main.c

Removed: 
    


################################################################################
diff  --git a/lldb/source/Commands/CommandObjectMemoryTag.cpp b/lldb/source/Commands/CommandObjectMemoryTag.cpp
index 840f81719d7dc..666d5c21b8714 100644
--- a/lldb/source/Commands/CommandObjectMemoryTag.cpp
+++ b/lldb/source/Commands/CommandObjectMemoryTag.cpp
@@ -12,6 +12,7 @@
 #include "lldb/Interpreter/OptionArgParser.h"
 #include "lldb/Interpreter/OptionGroupFormat.h"
 #include "lldb/Interpreter/OptionValueString.h"
+#include "lldb/Target/ABI.h"
 #include "lldb/Target/Process.h"
 
 using namespace lldb;
@@ -85,6 +86,17 @@ class CommandObjectMemoryTagRead : public CommandObjectParsed {
     // If this fails the list of regions is cleared, so we don't need to read
     // the return status here.
     process->GetMemoryRegions(memory_regions);
+
+    lldb::addr_t logical_tag = tag_manager->GetLogicalTag(start_addr);
+
+    // The tag manager only removes tag bits. These addresses may include other
+    // non-address bits that must also be ignored.
+    ABISP abi = process->GetABI();
+    if (abi) {
+      start_addr = abi->FixDataAddress(start_addr);
+      end_addr = abi->FixDataAddress(end_addr);
+    }
+
     llvm::Expected<MemoryTagManager::TagRange> tagged_range =
         tag_manager->MakeTaggedRange(start_addr, end_addr, memory_regions);
 
@@ -101,7 +113,6 @@ class CommandObjectMemoryTagRead : public CommandObjectParsed {
       return false;
     }
 
-    lldb::addr_t logical_tag = tag_manager->GetLogicalTag(start_addr);
     result.AppendMessageWithFormatv("Logical tag: {0:x}", logical_tag);
     result.AppendMessage("Allocation tags:");
 
@@ -231,6 +242,12 @@ class CommandObjectMemoryTagWrite : public CommandObjectParsed {
     // the return status here.
     process->GetMemoryRegions(memory_regions);
 
+    // The tag manager only removes tag bits. These addresses may include other
+    // non-address bits that must also be ignored.
+    ABISP abi = process->GetABI();
+    if (abi)
+      start_addr = abi->FixDataAddress(start_addr);
+
     // We have to assume start_addr is not granule aligned.
     // So if we simply made a range:
     // (start_addr, start_addr + (N * granule_size))
@@ -254,6 +271,10 @@ class CommandObjectMemoryTagWrite : public CommandObjectParsed {
       end_addr =
           aligned_start_addr + (tags.size() * tag_manager->GetGranuleSize());
 
+    // Remove non-address bits that aren't memory tags
+    if (abi)
+      end_addr = abi->FixDataAddress(end_addr);
+
     // Now we've aligned the start address so if we ask for another range
     // using the number of tags N, we'll get back a range that is also N
     // granules in size.

diff  --git a/lldb/test/API/linux/aarch64/mte_tag_access/TestAArch64LinuxMTEMemoryTagAccess.py b/lldb/test/API/linux/aarch64/mte_tag_access/TestAArch64LinuxMTEMemoryTagAccess.py
index 29c19aa68fb72..983f283444a46 100644
--- a/lldb/test/API/linux/aarch64/mte_tag_access/TestAArch64LinuxMTEMemoryTagAccess.py
+++ b/lldb/test/API/linux/aarch64/mte_tag_access/TestAArch64LinuxMTEMemoryTagAccess.py
@@ -20,6 +20,11 @@ def setup_mte_test(self):
         if not self.isAArch64MTE():
             self.skipTest('Target must support MTE.')
 
+        # Required to check that commands remove non-address bits
+        # other than the memory tags.
+        if not self.isAArch64PAuth():
+            self.skipTest('Target must support pointer authentication')
+
         self.build()
         self.runCmd("file " + self.getBuildArtifact("a.out"), CURRENT_EXECUTABLE_SET)
 

diff  --git a/lldb/test/API/linux/aarch64/mte_tag_access/main.c b/lldb/test/API/linux/aarch64/mte_tag_access/main.c
index 493733d09562c..5a466b1660090 100644
--- a/lldb/test/API/linux/aarch64/mte_tag_access/main.c
+++ b/lldb/test/API/linux/aarch64/mte_tag_access/main.c
@@ -71,8 +71,9 @@ int main(int argc, char const *argv[]) {
   // tag
   char *mte_buf_alt_tag = __arm_mte_create_random_tag(mte_buf, ~(1 << 10));
 
-  // lldb should be removing the whole top byte, not just the tags.
-  // So fill 63-60 with something non zero so we'll fail if we only remove tags.
+  // The memory tag manager should be removing the whole top byte, not just the
+  // tags. So fill 63-60 with something non zero so we'll fail if we only remove
+  // tags.
 #define SET_TOP_NIBBLE(ptr, value)                                             \
   (char *)((size_t)(ptr) | ((size_t)((value)&0xf) << 60))
   // mte_buf_alt_tag's nibble > mte_buf to check that lldb isn't just removing
@@ -82,6 +83,15 @@ int main(int argc, char const *argv[]) {
   mte_buf_2 = SET_TOP_NIBBLE(mte_buf_2, 0xC);
   mte_read_only = SET_TOP_NIBBLE(mte_read_only, 0xD);
 
+// The top level commands should be removing all non-address bits, including
+// pointer signatures. This signs ptr with PAC key A. That signature goes
+// in some bits other than the top byte.
+#define sign_ptr(ptr) __asm__ __volatile__("pacdza %0" : "=r"(ptr) : "r"(ptr))
+  sign_ptr(mte_buf);
+  sign_ptr(mte_buf_alt_tag);
+  sign_ptr(mte_buf_2);
+  sign_ptr(mte_read_only);
+
   // Breakpoint here
   return 0;
 }