[lldb][AArch64] Handle core file tag segments missing tag data (#145338)

In the same way that memory regions may be known from a core file but
not readable, tag segments can also have no content. For example:
```
$ readelf --segments core
<...>
Program Headers:
  Type           Offset             VirtAddr           PhysAddr
                 FileSiz            MemSiz              Flags  Align
<...>
  LOAD           0x0000000000002000 0x0000ffff93899000 0x0000000000000000
                 0x0000000000000000 0x0000000000001000  RW     0x1000
<...>
  LOPROC+0x2     0x0000000000008000 0x0000ffff93899000 0x0000000000000000
                 0x0000000000000000 0x0000000000001000         0x0
```
This happens if you have a restricted coredump filter or size limit.

The area of virtual memory this segment covers is 0x1000, or 4096 bytes
aka one tagged page. It's FileSiz would normally be 0x80. Tags are
packed 2 per byte and granules are 16 bytes. 4096 / 16 / 2 = 128 or
0x80.

But here it has no data, and in theory a corrupt file might have some
data but not all. This triggered an assert in
UnpackTagsFromCoreFileSegment and crashed lldb.

To fix this I have made UnpackTagsFromCoreFileSegment return an expected
and returned an error in this case instead of asserting. This will be
seen by the user, as shown in the added API test.

lldb/include/lldb/Target/MemoryTagManager.h

@@ -122,11 +122,15 @@ public:
   //
   // 'reader' will always be a wrapper around a CoreFile in real use
   // but allows testing without having to mock a CoreFile.
+  //
+  // This call will fail in the case that the core file segment does not contain
+  // enough data to read all the tags.
   typedef std::function<size_t(lldb::offset_t, size_t, void *)> CoreReaderFn;
-  std::vector<lldb::addr_t> virtual UnpackTagsFromCoreFileSegment(
-      CoreReaderFn reader, lldb::addr_t tag_segment_virtual_address,
-      lldb::addr_t tag_segment_data_address, lldb::addr_t addr,
-      size_t len) const = 0;
+  llvm::
+      Expected<std::vector<lldb::addr_t>> virtual UnpackTagsFromCoreFileSegment(
+          CoreReaderFn reader, lldb::addr_t tag_segment_virtual_address,
+          lldb::addr_t tag_segment_data_address, lldb::addr_t addr,
+          size_t len) const = 0;
 
   // Pack uncompressed tags into their storage format (e.g. for gdb QMemTags).
   // Checks that each tag is within the expected value range.

lldb/source/Plugins/Process/Utility/MemoryTagManagerAArch64MTE.cpp

@@ -247,7 +247,7 @@ MemoryTagManagerAArch64MTE::UnpackTagsData(const std::vector<uint8_t> &tags,
   return unpacked;
 }
 
-std::vector<lldb::addr_t>
+llvm::Expected<std::vector<lldb::addr_t>>
 MemoryTagManagerAArch64MTE::UnpackTagsFromCoreFileSegment(
     CoreReaderFn reader, lldb::addr_t tag_segment_virtual_address,
     lldb::addr_t tag_segment_data_address, lldb::addr_t addr,
@@ -290,8 +290,12 @@ MemoryTagManagerAArch64MTE::UnpackTagsFromCoreFileSegment(
   const size_t bytes_copied =
       reader(tag_segment_data_address + file_offset_in_bytes, tag_bytes_to_read,
              tag_data.data());
-  UNUSED_IF_ASSERT_DISABLED(bytes_copied);
-  assert(bytes_copied == tag_bytes_to_read);
+  if (bytes_copied != tag_bytes_to_read) {
+    return llvm::createStringError(
+        llvm::inconvertibleErrorCode(),
+        "Could not read tags from core file segment. Segment "
+        "is missing some or all tag data.");
+  }
 
   std::vector<lldb::addr_t> tags;
   tags.reserve(2 * tag_data.size());

lldb/source/Plugins/Process/Utility/MemoryTagManagerAArch64MTE.h

@@ -44,7 +44,7 @@ public:
   UnpackTagsData(const std::vector<uint8_t> &tags,
                  size_t granules = 0) const override;
 
-  std::vector<lldb::addr_t>
+  llvm::Expected<std::vector<lldb::addr_t>>
   UnpackTagsFromCoreFileSegment(CoreReaderFn reader,
                                 lldb::addr_t tag_segment_virtual_address,
                                 lldb::addr_t tag_segment_data_address,

lldb/test/API/linux/aarch64/mte_core_file/TestAArch64LinuxMTEMemoryTagCoreFile.py

@@ -248,3 +248,26 @@ class AArch64LinuxMTEMemoryTagCoreFileTestCase(TestBase):
                     "TCF: 0 = TCF_NONE, 1 = TCF_SYNC, 2 = TCF_ASYNC, 3 = TCF_ASYMM"
                 ],
             )
+
+    @skipIfLLVMTargetMissing("AArch64")
+    def test_mte_no_tags(self):
+        """Test that we handle there being a tag segment but that segment does
+        not contain any tag data. This can happen when the core is dumped
+        with a restrictive limit or filter."""
+        self.runCmd("target create --core core.mte.notags")
+
+        mte_buf_addr = 0xFFFFA4AF3000
+
+        # We can see which memory was tagged.
+        self.expect(
+            f"memory region {mte_buf_addr}", substrs=["memory tagging: enabled"]
+        )
+
+        # We cannot read those tags.
+        self.expect(
+            f"memory tag read {mte_buf_addr}",
+            substrs=[
+                "Could not read tags from core file segment. Segment is missing some or all tag data."
+            ],
+            error=True,
+        )

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

@@ -5,9 +5,14 @@
 //
 // Compile with:
 // <gcc or clang> -march=armv8.5-a+memtag -g main.c -o a.out.mte
+// (use a.out.mte to generate core.mte.notags and core.mte)
 // <gcc or clang> -march=armv8.5-a+memtag -g main.c -DNO_MTE -o a.out.nomte
 //
-// /proc/self/coredump_filter was set to 2 when the core files were made.
+// Set /proc/self/coredump_filter to the following values when generating the
+// core files:
+// * core.mte - 3
+// * core.mte.notags - 2
+// * core.nomte - 3
 
 #include <arm_acle.h>
 #include <asm/mman.h>

lldb/unittests/Process/Utility/MemoryTagManagerAArch64MTETest.cpp

@@ -87,31 +87,38 @@ TEST(MemoryTagManagerAArch64MTETest, UnpackTagsFromCoreFileSegment) {
   std::vector<uint8_t> tags_data;
   MemoryTagManager::CoreReaderFn reader =
       [&tags_data](lldb::offset_t offset, size_t length, void *dst) {
+        if ((offset + length) >= tags_data.size())
+          length = tags_data.size() - offset;
+
         std::memcpy(dst, tags_data.data() + offset, length);
         return length;
       };
 
   // Zero length is ok.
-  std::vector<lldb::addr_t> tags =
+  llvm::Expected<std::vector<lldb::addr_t>> tags =
       manager.UnpackTagsFromCoreFileSegment(reader, 0, 0, 0, 0);
-  ASSERT_EQ(tags.size(), (size_t)0);
+  ASSERT_THAT_EXPECTED(tags, llvm::Succeeded());
+  ASSERT_EQ(tags->size(), (size_t)0);
 
   // In the simplest case we read 2 tags which are in the same byte.
   tags_data.push_back(0x21);
   // The least significant bits are the first tag in memory.
   std::vector<lldb::addr_t> expected{1, 2};
   tags = manager.UnpackTagsFromCoreFileSegment(reader, 0, 0, 0, 32);
-  ASSERT_THAT(expected, testing::ContainerEq(tags));
+  ASSERT_THAT_EXPECTED(tags, llvm::Succeeded());
+  ASSERT_THAT(expected, testing::ContainerEq(*tags));
 
   // If we read just one then it will have to trim off the second one.
   expected = std::vector<lldb::addr_t>{1};
   tags = manager.UnpackTagsFromCoreFileSegment(reader, 0, 0, 0, 16);
-  ASSERT_THAT(expected, testing::ContainerEq(tags));
+  ASSERT_THAT_EXPECTED(tags, llvm::Succeeded());
+  ASSERT_THAT(expected, testing::ContainerEq(*tags));
 
   // If we read the second tag only then the first one must be trimmed.
   expected = std::vector<lldb::addr_t>{2};
   tags = manager.UnpackTagsFromCoreFileSegment(reader, 0, 0, 16, 16);
-  ASSERT_THAT(expected, testing::ContainerEq(tags));
+  ASSERT_THAT_EXPECTED(tags, llvm::Succeeded());
+  ASSERT_THAT(expected, testing::ContainerEq(*tags));
 
   // This trimming logic applies if you read a larger set of tags.
   tags_data = std::vector<uint8_t>{0x21, 0x43, 0x65, 0x87};
@@ -119,31 +126,55 @@ TEST(MemoryTagManagerAArch64MTETest, UnpackTagsFromCoreFileSegment) {
   // Trailing tag should be trimmed.
   expected = std::vector<lldb::addr_t>{1, 2, 3};
   tags = manager.UnpackTagsFromCoreFileSegment(reader, 0, 0, 0, 48);
-  ASSERT_THAT(expected, testing::ContainerEq(tags));
+  ASSERT_THAT_EXPECTED(tags, llvm::Succeeded());
+  ASSERT_THAT(expected, testing::ContainerEq(*tags));
 
   // Leading tag should be trimmed.
   expected = std::vector<lldb::addr_t>{2, 3, 4};
   tags = manager.UnpackTagsFromCoreFileSegment(reader, 0, 0, 16, 48);
-  ASSERT_THAT(expected, testing::ContainerEq(tags));
+  ASSERT_THAT_EXPECTED(tags, llvm::Succeeded());
+  ASSERT_THAT(expected, testing::ContainerEq(*tags));
 
   // Leading and trailing trimmmed.
   expected = std::vector<lldb::addr_t>{2, 3, 4, 5};
   tags = manager.UnpackTagsFromCoreFileSegment(reader, 0, 0, 16, 64);
-  ASSERT_THAT(expected, testing::ContainerEq(tags));
+  ASSERT_THAT_EXPECTED(tags, llvm::Succeeded());
+  ASSERT_THAT(expected, testing::ContainerEq(*tags));
 
   // The address given is an offset into the whole file so the address requested
   // from the reader should be beyond that.
   tags_data = std::vector<uint8_t>{0xFF, 0xFF, 0x21, 0x43, 0x65, 0x87};
   expected = std::vector<lldb::addr_t>{1, 2};
   tags = manager.UnpackTagsFromCoreFileSegment(reader, 0, 2, 0, 32);
-  ASSERT_THAT(expected, testing::ContainerEq(tags));
+  ASSERT_THAT_EXPECTED(tags, llvm::Succeeded());
+  ASSERT_THAT(expected, testing::ContainerEq(*tags));
 
   // addr is a virtual address that we expect to be >= the tag segment's
   // starting virtual address. So again an offset must be made from the
   // difference.
   expected = std::vector<lldb::addr_t>{3, 4};
   tags = manager.UnpackTagsFromCoreFileSegment(reader, 32, 2, 64, 32);
-  ASSERT_THAT(expected, testing::ContainerEq(tags));
+  ASSERT_THAT_EXPECTED(tags, llvm::Succeeded());
+  ASSERT_THAT(expected, testing::ContainerEq(*tags));
+
+  // Error when there is not enough data to decode tags.
+
+  // Read 1 tag from an offset just outside the segment's data.
+  tags_data = {0xAB};
+  tags = manager.UnpackTagsFromCoreFileSegment(reader, 0, 0, 32, 16);
+  const char *expected_err = "Could not read tags from core file segment. "
+                             "Segment is missing some or all tag data.";
+  EXPECT_THAT_EXPECTED(tags, llvm::FailedWithMessage(expected_err));
+
+  // First 2 tags come from the segment, second 2 cannot be read.
+  tags_data.push_back(0xCD);
+  tags = manager.UnpackTagsFromCoreFileSegment(reader, 0, 0, 32, 64);
+  EXPECT_THAT_EXPECTED(tags, llvm::FailedWithMessage(expected_err));
+
+  // Segment is completely empty.
+  tags_data.clear();
+  tags = manager.UnpackTagsFromCoreFileSegment(reader, 0, 0, 0, 16);
+  EXPECT_THAT_EXPECTED(tags, llvm::FailedWithMessage(expected_err));
 }
 
 TEST(MemoryTagManagerAArch64MTETest, GetLogicalTag) {