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

[DavidSpickett]

Summary

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.

[omjavaid]

lldb/test/API/linux/aarch64/tagged_memory_access/TestAArch64LinuxTaggedMemoryAccess.py

70

to to => to

83

so I am thinking do we actually need to omit tags from location address. Internally we want LLDB to ignore tags thats fine but user must be seeing the tagged address of buf so why omit tags in output of memory find?

[DavidSpickett]

Fix "to to" in comment.

[DavidSpickett]

lldb/test/API/linux/aarch64/tagged_memory_access/TestAArch64LinuxTaggedMemoryAccess.py

83

TLDR: The logic here is that the tag (or more generally non-address) bits of the pointer are a property of the pointer, not the memory pointed to.

More detail, since I've thought about this a lot. Please challenge if it doesn't make sense.

You could argue that you're showing the user a "view" of memory through that pointer. My problem there is that pointer auth and memory tagging break that argument because you cannot assume they stay the same as the memory location increments. Since you can't just assume that the signature for ptr and ptr+16 would be the same, or the memory tag for those locations would be the same.

The only minor benefit there is the user can copy a tagged pointer to a later location, but those tags may not actually let the program access the memory. (though the debugger can always but it didn't need tags to do that in any case)

This is what the full output looks like. If you wanted to display the tags in the output which tag would you use? Well, simple choice is the start address tag. However, it's really an arbitrary choice that I think doesn't help much.

(lldb) p ptr1
(char *) $2 = 0x3400000000411029 ""
(lldb) p ptr2
(char *) $3 = 0x5600000000411029 ""
(lldb) memory find -s '?' ptr2 ptr1+16
data found at location: 0x411038
0x00411038: 3f 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ?...............
0x00411048: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................

I agree there is an initial "why did my pointer get changed" reaction but think I not showing non-address bits when showing actual memory is going to be clearer in the long run. Granted this is mostly intuition, we don't have a lot of user feedback about this at this stage.

Actual "memory tags" do have a tag that is attached to the memory location in hardware, so you may want to show that. That's something I've been addressing for "memory read" and will probably port that over to memory find as well. It'll look something like:

(lldb) memory find -s '?' ptr2 ptr1+16
data found at location: 0x411038
0x00411038: 3f 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ?............... (tag: 0x1)
0x00411048: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................ (tag: 0x2)

Crucially here we're showing the real tag for each line, instead of assuming that the one in the pointer can be applied to them all.

[omjavaid]

lldb/test/API/linux/aarch64/tagged_memory_access/TestAArch64LinuxTaggedMemoryAccess.py

83

This actually makes me think if we should try adding a feature where we can enable/disable viewing off non-address-bits. The purpose is to allow average programmer debug their simple applications without any surprise of ever changing memory addresses. May be with time people will realize the non-address bits technology but for programmers switching back n forth between x64 and AArch64 may feel things happening differently unless they figure there is this non-address bits phenomenon.

Anyways your patch does make sense for now unless we hear any user complaints.

Moreover additional tag information that you suggested above would be a good adon so +1 for that.

lldb/source/Commands/CommandObjectMemory.cpp

bool DoExecute(Args &command, CommandReturnObject &result) override {
     }
     lldb::addr_t high_addr = OptionArgParser::ToAddress(
         &m_exe_ctx, command[1].ref(), LLDB_INVALID_ADDRESS, &error);
     if (high_addr == LLDB_INVALID_ADDRESS || error.Fail()) {
       result.AppendError("invalid high address");
       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");
       return false;
     }
 
     lldb::addr_t found_location = LLDB_INVALID_ADDRESS;
 

lldb/test/API/linux/aarch64/tagged_memory_access/TestAArch64LinuxTaggedMemoryAccess.py

 """
-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.
 """
 
 
 
 import lldb
 from lldbsuite.test.decorators import *
 from lldbsuite.test.lldbtest import *
 from lldbsuite.test import lldbutil
 
 
 class AArch64LinuxTaggedMemoryReadTestCase(TestBase):
 
     mydir = TestBase.compute_mydir(__file__)
 
     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)
 
         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'])
 
+    @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.
         # 2. We error because end address is < start address since end's
         #    tag is < start's tag.
         #
         # Each time we check that the printed line addresses do not include
         # either of the tags we set. Those bits are a property of the
         # pointer not of the memory it points to.
         tagged_addr_pattern = "0x(34|46)[0-9A-Fa-f]{14}:.*"
         self.expect("memory read ptr1 ptr2+16", patterns=[tagged_addr_pattern], matching=False)
         # Check that the stored previous end address is stripped
         self.expect("memory read", patterns=[tagged_addr_pattern], matching=False)
         # 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]+")

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

 #include <stddef.h>
 
 static char *set_non_address_bits(char *ptr, size_t tag) {
   // Set top byte tag (AArch64 Linux always enables top byte ignore)
   return (char *)((size_t)ptr | (tag << 56));
 }
 
-int main(int argc, char const *argv[]) {
-  char buf[32];
+// Global to zero init
 
+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.
 }