llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-lldb
Author: David Spickett (DavidSpickett)
<details>
<summary>Changes</summary>
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.
---
Full diff: https://github.com/llvm/llvm-project/pull/145338.diff
6 Files Affected:
- (modified) lldb/include/lldb/Target/MemoryTagManager.h (+8-4)
- (modified) lldb/source/Plugins/Process/Utility/MemoryTagManagerAArch64MTE.cpp
(+7-3)
- (modified) lldb/source/Plugins/Process/Utility/MemoryTagManagerAArch64MTE.h
(+1-1)
- (modified)
lldb/test/API/linux/aarch64/mte_core_file/TestAArch64LinuxMTEMemoryTagCoreFile.py
(+24)
- (added) lldb/test/API/linux/aarch64/mte_core_file/core.mte.notags ()
- (modified) lldb/unittests/Process/Utility/MemoryTagManagerAArch64MTETest.cpp
(+41-10)
``````````diff
diff --git a/lldb/include/lldb/Target/MemoryTagManager.h
b/lldb/include/lldb/Target/MemoryTagManager.h
index 6bd4180fff703..587e5ef4199d4 100644
--- a/lldb/include/lldb/Target/MemoryTagManager.h
+++ b/lldb/include/lldb/Target/MemoryTagManager.h
@@ -122,11 +122,15 @@ class MemoryTagManager {
//
// 'reader' will always be a wrapper around a CoreFile in real use
// but allows testing without having to mock a CoreFile.
+ //
+ // This call may 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.
diff --git a/lldb/source/Plugins/Process/Utility/MemoryTagManagerAArch64MTE.cpp
b/lldb/source/Plugins/Process/Utility/MemoryTagManagerAArch64MTE.cpp
index 7e25bc4ea2a28..9f60675e51904 100644
--- a/lldb/source/Plugins/Process/Utility/MemoryTagManagerAArch64MTE.cpp
+++ b/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());
diff --git a/lldb/source/Plugins/Process/Utility/MemoryTagManagerAArch64MTE.h
b/lldb/source/Plugins/Process/Utility/MemoryTagManagerAArch64MTE.h
index 365e176e5b1da..79d24ce78ecee 100644
--- a/lldb/source/Plugins/Process/Utility/MemoryTagManagerAArch64MTE.h
+++ b/lldb/source/Plugins/Process/Utility/MemoryTagManagerAArch64MTE.h
@@ -44,7 +44,7 @@ class MemoryTagManagerAArch64MTE : public MemoryTagManager {
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,
diff --git
a/lldb/test/API/linux/aarch64/mte_core_file/TestAArch64LinuxMTEMemoryTagCoreFile.py
b/lldb/test/API/linux/aarch64/mte_core_file/TestAArch64LinuxMTEMemoryTagCoreFile.py
index a9879f67d8b8f..467881430e778 100644
---
a/lldb/test/API/linux/aarch64/mte_core_file/TestAArch64LinuxMTEMemoryTagCoreFile.py
+++
b/lldb/test/API/linux/aarch64/mte_core_file/TestAArch64LinuxMTEMemoryTagCoreFile.py
@@ -248,3 +248,27 @@ def test_mte_ctrl_register(self):
"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")
+
+ # TODO: regenerate all the core files once MTE4 support has landed.
+ 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,
+ )
diff --git a/lldb/test/API/linux/aarch64/mte_core_file/core.mte.notags
b/lldb/test/API/linux/aarch64/mte_core_file/core.mte.notags
new file mode 100644
index 0000000000000..8f9d60668a84d
Binary files /dev/null and
b/lldb/test/API/linux/aarch64/mte_core_file/core.mte.notags differ
diff --git a/lldb/unittests/Process/Utility/MemoryTagManagerAArch64MTETest.cpp
b/lldb/unittests/Process/Utility/MemoryTagManagerAArch64MTETest.cpp
index 40d7c3601ccfd..30199bfe5c254 100644
--- a/lldb/unittests/Process/Utility/MemoryTagManagerAArch64MTETest.cpp
+++ b/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) {
``````````
</details>
https://github.com/llvm/llvm-project/pull/145338
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
