[Lldb-commits] [lldb] 7d19566 - [lldb] Ignore non-address bits in "memory find" arguments

[David Spickett via lldb-commits]

Author: David Spickett
Date: 2022-01-24T10:42:49Z
New Revision: 7d19566c3bfb3efacb629d18839e2d85761156ab

URL: https://github.com/llvm/llvm-project/commit/7d19566c3bfb3efacb629d18839e2d85761156ab
DIFF: https://github.com/llvm/llvm-project/commit/7d19566c3bfb3efacb629d18839e2d85761156ab.diff

LOG: [lldb] Ignore non-address bits in "memory find" arguments

This removes the non-address bits before we try to use
the addresses.

Meaning that when results are shown, those results won't
show non-address bits either. This follows what "memory read"
has done. On the grounds that non-address bits are a property
of a pointer, not the memory pointed to.

I've added testing and merged the find and read tests into one
file.

Note that there are no API side changes because "memory find"
does not have an equivalent API call.

Reviewed By: omjavaid

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

Added: 
    lldb/test/API/linux/aarch64/tagged_memory_access/Makefile
    lldb/test/API/linux/aarch64/tagged_memory_access/TestAArch64LinuxTaggedMemoryAccess.py
    lldb/test/API/linux/aarch64/tagged_memory_access/main.c

Modified: 
    lldb/source/Commands/CommandObjectMemory.cpp

Removed: 
    lldb/test/API/linux/aarch64/tagged_memory_read/Makefile
    lldb/test/API/linux/aarch64/tagged_memory_read/TestAArch64LinuxTaggedMemoryRead.py
    lldb/test/API/linux/aarch64/tagged_memory_read/main.c


################################################################################
diff  --git a/lldb/source/Commands/CommandObjectMemory.cpp b/lldb/source/Commands/CommandObjectMemory.cpp
index 0b5f39bc7a8f4..f5e2cb4ed44ea 100644
--- a/lldb/source/Commands/CommandObjectMemory.cpp
+++ b/lldb/source/Commands/CommandObjectMemory.cpp
@@ -1032,6 +1032,12 @@ class CommandObjectMemoryFind : public CommandObjectParsed {
       return false;
     }
 
+    ABISP abi = m_exe_ctx.GetProcessPtr()->GetABI();
+    if (abi) {
+      low_addr = abi->FixDataAddress(low_addr);
+      high_addr = abi->FixDataAddress(high_addr);
+    }
+
     if (high_addr <= low_addr) {
       result.AppendError(
           "starting address must be smaller than ending address");

diff  --git a/lldb/test/API/linux/aarch64/tagged_memory_read/Makefile b/lldb/test/API/linux/aarch64/tagged_memory_access/Makefile
similarity index 100%
rename from lldb/test/API/linux/aarch64/tagged_memory_read/Makefile
rename to lldb/test/API/linux/aarch64/tagged_memory_access/Makefile

diff  --git a/lldb/test/API/linux/aarch64/tagged_memory_read/TestAArch64LinuxTaggedMemoryRead.py b/lldb/test/API/linux/aarch64/tagged_memory_access/TestAArch64LinuxTaggedMemoryAccess.py
similarity index 62%
rename from lldb/test/API/linux/aarch64/tagged_memory_read/TestAArch64LinuxTaggedMemoryRead.py
rename to lldb/test/API/linux/aarch64/tagged_memory_access/TestAArch64LinuxTaggedMemoryAccess.py
index 2f55b951a7548..3cc0d70a3ce34 100644
--- a/lldb/test/API/linux/aarch64/tagged_memory_read/TestAArch64LinuxTaggedMemoryRead.py
+++ b/lldb/test/API/linux/aarch64/tagged_memory_access/TestAArch64LinuxTaggedMemoryAccess.py
@@ -1,6 +1,9 @@
 """
-Test that "memory read" removes non address bits from
-memory read arguments.
+Test that "memory read" and "memory find" remove non address bits from
+address arguments.
+
+These tests use the top byte ignore feature of AArch64. Which Linux
+always enables.
 """
 
 
@@ -17,10 +20,7 @@ class AArch64LinuxTaggedMemoryReadTestCase(TestBase):
 
     NO_DEBUG_INFO_TESTCASE = True
 
-    # AArch64 Linux always enables top byte ignore
-    @skipUnlessArch("aarch64")
-    @skipUnlessPlatform(["linux"])
-    def test_tagged_memory_read(self):
+    def setup_test(self):
         self.build()
         self.runCmd("file " + self.getBuildArtifact("a.out"), CURRENT_EXECUTABLE_SET)
 
@@ -37,6 +37,11 @@ def test_tagged_memory_read(self):
             substrs=['stopped',
                      'stop reason = breakpoint'])
 
+    @skipUnlessArch("aarch64")
+    @skipUnlessPlatform(["linux"])
+    def test_tagged_memory_read(self):
+        self.setup_test()
+
         # If we do not remove non address bits, this can fail in two ways.
         # 1. We attempt to read much more than 16 bytes, probably more than
         #    the default 1024 byte read size. Which will error.
@@ -53,3 +58,26 @@ def test_tagged_memory_read(self):
         # Would fail if we don't remove non address bits because 0x56... > 0x34...
         self.expect("memory read ptr2 ptr1+16", patterns=[tagged_addr_pattern], matching=False)
         self.expect("memory read", patterns=[tagged_addr_pattern], matching=False)
+
+    @skipUnlessArch("aarch64")
+    @skipUnlessPlatform(["linux"])
+    def test_tagged_memory_find(self):
+        self.setup_test()
+
+        # If memory find doesn't remove non-address bits one of two
+        # things happen.
+        # 1. It tries to search a gigantic amount of memory.
+        #    We're not going to test for this because a failure
+        #    would take a very long time and perhaps even find the
+        #    target value randomly.
+        # 2. It thinks high address <= low address, which we check below.
+
+        self.runCmd("memory find -s '?' ptr2 ptr1+32")
+
+        self.assertTrue(self.res.Succeeded())
+        out = self.res.GetOutput()
+        # memory find does not fail when it doesn't find the data.
+        # First check we actually got something.
+        self.assertRegexpMatches(out, "data found at location: 0x[0-9A-Fa-f]+")
+        # Then that the location found does not display the tag bits.
+        self.assertNotRegexpMatches(out, "data found at location: 0x(34|56)[0-9A-Fa-f]+")

diff  --git a/lldb/test/API/linux/aarch64/tagged_memory_read/main.c b/lldb/test/API/linux/aarch64/tagged_memory_access/main.c
similarity index 80%
rename from lldb/test/API/linux/aarch64/tagged_memory_read/main.c
rename to lldb/test/API/linux/aarch64/tagged_memory_access/main.c
index 72ee30cef7869..3dd064b00bd31 100644
--- a/lldb/test/API/linux/aarch64/tagged_memory_read/main.c
+++ b/lldb/test/API/linux/aarch64/tagged_memory_access/main.c
@@ -5,11 +5,15 @@ static char *set_non_address_bits(char *ptr, size_t tag) {
   return (char *)((size_t)ptr | (tag << 56));
 }
 
-int main(int argc, char const *argv[]) {
-  char buf[32];
+// Global to zero init
+char buf[32];
 
+int main(int argc, char const *argv[]) {
   char *ptr1 = set_non_address_bits(buf, 0x34);
   char *ptr2 = set_non_address_bits(buf, 0x56);
 
+  // Target value for "memory find"
+  buf[15] = '?';
+
   return 0; // Set break point at this line.
 }