DavidSpickett updated this revision to Diff 402898. DavidSpickett added a comment.
Since output of GetMemoryRegions will be sorted and not overlap, switch the checks to asserts and don't sort the input regions. Updated the testing to reflect that and added some comment diagrams showing what regions are used for the later tests. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112824/new/ https://reviews.llvm.org/D112824 Files: lldb/include/lldb/Target/MemoryTagManager.h lldb/source/Plugins/Process/Utility/MemoryTagManagerAArch64MTE.cpp lldb/source/Plugins/Process/Utility/MemoryTagManagerAArch64MTE.h lldb/unittests/Process/Utility/MemoryTagManagerAArch64MTETest.cpp
Index: lldb/unittests/Process/Utility/MemoryTagManagerAArch64MTETest.cpp =================================================================== --- lldb/unittests/Process/Utility/MemoryTagManagerAArch64MTETest.cpp +++ lldb/unittests/Process/Utility/MemoryTagManagerAArch64MTETest.cpp @@ -165,6 +165,14 @@ llvm::FailedWithMessage( "End address (0x0) must be greater than the start address (0x1)")); + // The inversion check ignores tags in the addresses (MTE tags start at bit + // 56). + ASSERT_THAT_EXPECTED( + manager.MakeTaggedRange((lldb::addr_t)1 << 56, + ((lldb::addr_t)2 << 56) + 0x10, memory_regions), + llvm::FailedWithMessage( + "Address range 0x0:0x10 is not in a memory tagged region")); + // Adding a single region to cover the whole range memory_regions.push_back(MakeRegionInfo(0, 0x1000, true)); @@ -247,6 +255,122 @@ ASSERT_EQ(*got, expected_range); } +TEST(MemoryTagManagerAArch64MTETest, MakeTaggedRanges) { + MemoryTagManagerAArch64MTE manager; + MemoryRegionInfos memory_regions; + + // Note that MakeTaggedRanges takes start/end address. + // Whereas TagRanges and regions take start address and size. + + // Range must not be inverted + ASSERT_THAT_EXPECTED( + manager.MakeTaggedRanges(1, 0, memory_regions), + llvm::FailedWithMessage( + "End address (0x0) must be greater than the start address (0x1)")); + + // We remove tags before doing the inversion check, so this is not an error. + // Also no regions means no tagged regions returned. + // (bit 56 is where MTE tags begin) + llvm::Expected<std::vector<MemoryTagManager::TagRange>> got = + manager.MakeTaggedRanges((lldb::addr_t)2 << 56, + ((lldb::addr_t)1 << 56) + 0x10, memory_regions); + ASSERT_THAT_EXPECTED(got, llvm::Succeeded()); + ASSERT_EQ(*got, std::vector<MemoryTagManager::TagRange>{}); + + // Cover whole range, untagged. No ranges returned. + memory_regions.push_back(MakeRegionInfo(0, 0x20, false)); + got = manager.MakeTaggedRanges(0, 0x20, memory_regions); + ASSERT_THAT_EXPECTED(got, llvm::Succeeded()); + ASSERT_EQ(*got, std::vector<MemoryTagManager::TagRange>{}); + + // Make the region tagged and it'll be the one range returned. + memory_regions.back().SetMemoryTagged(MemoryRegionInfo::eYes); + got = manager.MakeTaggedRanges(0, 0x20, memory_regions); + ASSERT_THAT_EXPECTED(got, llvm::Succeeded()); + ASSERT_EQ(*got, std::vector<MemoryTagManager::TagRange>{ + MemoryTagManager::TagRange(0, 0x20)}); + + // This region will be trimmed if it's larger than the whole range. + memory_regions.clear(); + memory_regions.push_back(MakeRegionInfo(0, 0x40, true)); + got = manager.MakeTaggedRanges(0x10, 0x30, memory_regions); + ASSERT_THAT_EXPECTED(got, llvm::Succeeded()); + ASSERT_EQ(*got, std::vector<MemoryTagManager::TagRange>{ + MemoryTagManager::TagRange(0x10, 0x20)}); + + memory_regions.clear(); + + // For the following tests we keep the input regions + // in ascending order as MakeTaggedRanges expects. + + // Only start of range is tagged, only that is returned. + // Start the region just before the requested range to check + // we limit the result to the requested range. + memory_regions.push_back(MakeRegionInfo(0, 0x20, true)); + got = manager.MakeTaggedRanges(0x10, 0x100, memory_regions); + ASSERT_THAT_EXPECTED(got, llvm::Succeeded()); + ASSERT_EQ(*got, std::vector<MemoryTagManager::TagRange>{ + MemoryTagManager::TagRange(0x10, 0x10)}); + + // Add a tagged region at the end, now we get both + // and the middle is untagged. + // <tagged: [0x0, 0x20)> + // <...> + // <tagged: [0xE0, 0x120)> + // The range added here is deliberately over the end of the + // requested range to show that we trim the end. + memory_regions.push_back(MakeRegionInfo(0xE0, 0x40, true)); + got = manager.MakeTaggedRanges(0x10, 0x110, memory_regions); + ASSERT_THAT_EXPECTED(got, llvm::Succeeded()); + + std::vector<MemoryTagManager::TagRange> expected{ + MemoryTagManager::TagRange(0x10, 0x10), + MemoryTagManager::TagRange(0xE0, 0x30)}; + ASSERT_EQ(*got, expected); + + // Now add a middle tagged region. + // <tagged: [0x0, 0x20)> + // <...> + // <tagged: [0x90, 0xB0)> + // <...> + // <tagged: [0xE0, 0x120)> + memory_regions.insert(std::next(memory_regions.begin()), + MakeRegionInfo(0x90, 0x20, true)); + + // As the given regions are in ascending order, the resulting + // tagged ranges are also. So this new range goes in the middle. + expected.insert(std::next(expected.begin()), + MemoryTagManager::TagRange(0x90, 0x20)); + got = manager.MakeTaggedRanges(0x10, 0x110, memory_regions); + ASSERT_THAT_EXPECTED(got, llvm::Succeeded()); + ASSERT_EQ(*got, expected); + + // Then if we add untagged regions in between the tagged, + // the output should stay the same. + // <tagged: [0x0, 0x20)> + // <untagged: [0x20, 0x90)> + // <tagged: [0x90, 0xB0)> + // <untagged: [0xB0, 0xE0)> + // <tagged: [0xE0, 0x120)> + memory_regions.insert(std::next(memory_regions.begin()), + MakeRegionInfo(0x20, 0x70, false)); + memory_regions.insert(std::prev(memory_regions.end()), + MakeRegionInfo(0xB0, 0x30, false)); + got = manager.MakeTaggedRanges(0x10, 0x110, memory_regions); + ASSERT_THAT_EXPECTED(got, llvm::Succeeded()); + ASSERT_EQ(*got, expected); + + // Finally check that we handle only having the end of the range. + memory_regions.clear(); + expected.clear(); + + memory_regions.push_back(MakeRegionInfo(0x100, 0x10, true)); + expected.push_back(MemoryTagManager::TagRange(0x100, 0x10)); + got = manager.MakeTaggedRanges(0x10, 0x110, memory_regions); + ASSERT_THAT_EXPECTED(got, llvm::Succeeded()); + ASSERT_EQ(*got, expected); +} + TEST(MemoryTagManagerAArch64MTETest, RemoveTagBits) { MemoryTagManagerAArch64MTE manager; Index: lldb/source/Plugins/Process/Utility/MemoryTagManagerAArch64MTE.h =================================================================== --- lldb/source/Plugins/Process/Utility/MemoryTagManagerAArch64MTE.h +++ lldb/source/Plugins/Process/Utility/MemoryTagManagerAArch64MTE.h @@ -36,6 +36,10 @@ lldb::addr_t addr, lldb::addr_t end_addr, const lldb_private::MemoryRegionInfos &memory_regions) const override; + llvm::Expected<std::vector<TagRange>> MakeTaggedRanges( + lldb::addr_t addr, lldb::addr_t end_addr, + lldb_private::MemoryRegionInfos memory_regions) const override; + llvm::Expected<std::vector<lldb::addr_t>> UnpackTagsData(const std::vector<uint8_t> &tags, size_t granules = 0) const override; Index: lldb/source/Plugins/Process/Utility/MemoryTagManagerAArch64MTE.cpp =================================================================== --- lldb/source/Plugins/Process/Utility/MemoryTagManagerAArch64MTE.cpp +++ lldb/source/Plugins/Process/Utility/MemoryTagManagerAArch64MTE.cpp @@ -7,6 +7,8 @@ //===----------------------------------------------------------------------===// #include "MemoryTagManagerAArch64MTE.h" +#include "llvm/Support/Error.h" +#include <assert.h> using namespace lldb_private; @@ -66,6 +68,15 @@ return TagRange(new_start, new_len); } +static llvm::Error MakeInvalidRangeErr(lldb::addr_t addr, + lldb::addr_t end_addr) { + return llvm::createStringError( + llvm::inconvertibleErrorCode(), + "End address (0x%" PRIx64 + ") must be greater than the start address (0x%" PRIx64 ")", + end_addr, addr); +} + llvm::Expected<MemoryTagManager::TagRange> MemoryTagManagerAArch64MTE::MakeTaggedRange( lldb::addr_t addr, lldb::addr_t end_addr, @@ -74,13 +85,8 @@ // We must remove tags here otherwise an address with a higher // tag value will always be > the other. ptrdiff_t len = AddressDiff(end_addr, addr); - if (len <= 0) { - return llvm::createStringError( - llvm::inconvertibleErrorCode(), - "End address (0x%" PRIx64 - ") must be greater than the start address (0x%" PRIx64 ")", - end_addr, addr); - } + if (len <= 0) + return MakeInvalidRangeErr(addr, end_addr); // Region addresses will not have memory tags. So when searching // we must use an untagged address. @@ -123,6 +129,91 @@ return tag_range; } +llvm::Expected<std::vector<MemoryTagManager::TagRange>> +MemoryTagManagerAArch64MTE::MakeTaggedRanges( + lldb::addr_t addr, lldb::addr_t end_addr, + lldb_private::MemoryRegionInfos memory_regions) const { + // First check that the range is not inverted. + // We must remove tags here otherwise an address with a higher + // tag value will always be > the other. + ptrdiff_t len = AddressDiff(end_addr, addr); + if (len <= 0) + return MakeInvalidRangeErr(addr, end_addr); + + std::vector<MemoryTagManager::TagRange> tagged_ranges; + // No memory regions means no tagged memory at all + if (memory_regions.empty()) + return tagged_ranges; + + // For the logic to work regions must be in ascending order + // which is what you'd have if you used GetMemoryRegions. + assert(std::is_sorted( + memory_regions.begin(), memory_regions.end(), + [](const MemoryRegionInfo &lhs, const MemoryRegionInfo &rhs) { + return lhs.GetRange().GetRangeBase() < rhs.GetRange().GetRangeBase(); + })); + + // If we're debugging userspace in an OS like Linux that uses an MMU, + // the only reason we'd get overlapping regions is incorrect data. + // It is possible that won't hold for embedded with memory protection + // units (MPUs) that allow overlaps. + // + // For now we're going to assume the former, as there is no good way + // to handle overlaps. For example: + // < requested range > + // [-- region 1 --] + // [-- region 2--] + // Where the first region will reduce the requested range to nothing + // and exit early before it sees the overlap. + MemoryRegionInfos::const_iterator overlap = std::adjacent_find( + memory_regions.begin(), memory_regions.end(), + [](const MemoryRegionInfo &lhs, const MemoryRegionInfo &rhs) { + return rhs.GetRange().DoesIntersect(lhs.GetRange()); + }); + UNUSED_IF_ASSERT_DISABLED(overlap); + assert(overlap == memory_regions.end()); + + // Region addresses will not have memory tags so when searching + // we must use an untagged address. + MemoryRegionInfo::RangeType range(RemoveTagBits(addr), len); + range = ExpandToGranule(range); + + // While there are regions to check and the range has non zero length + for (const MemoryRegionInfo ®ion : memory_regions) { + // If range we're checking has been reduced to zero length, exit early + if (!range.IsValid()) + break; + + // If the region doesn't overlap the range at all, ignore it. + if (!region.GetRange().DoesIntersect(range)) + continue; + + // If it's tagged record this sub-range. + // (assuming that it's already granule aligned) + if (region.GetMemoryTagged()) { + // The region found may extend outside the requested range. + // For example the first region might start before the range. + // We must only add what covers the requested range. + lldb::addr_t start = + std::max(range.GetRangeBase(), region.GetRange().GetRangeBase()); + lldb::addr_t end = + std::min(range.GetRangeEnd(), region.GetRange().GetRangeEnd()); + tagged_ranges.push_back(MemoryTagManager::TagRange(start, end - start)); + } + + // Move the range up to start at the end of the region. + lldb::addr_t old_end = range.GetRangeEnd(); + // This "slides" the range so it moves the end as well. + range.SetRangeBase(region.GetRange().GetRangeEnd()); + // So we set the end back to the original end address after sliding it up. + range.SetRangeEnd(old_end); + // (if the above were to try to set end < begin the range will just be set + // to 0 size) + } + + return tagged_ranges; +} + llvm::Expected<std::vector<lldb::addr_t>> MemoryTagManagerAArch64MTE::UnpackTagsData(const std::vector<uint8_t> &tags, size_t granules /*=0*/) const { Index: lldb/include/lldb/Target/MemoryTagManager.h =================================================================== --- lldb/include/lldb/Target/MemoryTagManager.h +++ lldb/include/lldb/Target/MemoryTagManager.h @@ -64,7 +64,7 @@ // (which may include one or more memory regions) // // If so, return a modified range which will have been expanded - // to be granule aligned. + // to be granule aligned. Otherwise return an error. // // Tags in the input addresses are ignored and not present // in the returned range. @@ -72,6 +72,23 @@ lldb::addr_t addr, lldb::addr_t end_addr, const lldb_private::MemoryRegionInfos &memory_regions) const = 0; + // Given a range addr to end_addr, check that end_addr >= addr. + // If it is not, return an error saying so. + // Otherwise, granule align it and return a set of ranges representing + // subsections of the aligned range that have memory tagging enabled. + // + // Basically a sparse version of MakeTaggedRange. Use this when you + // want to know which parts of a larger range have memory tagging. + // + // Regions in memory_regions should be sorted in ascending order and + // not overlap. (use Process GetMemoryRegions) + // + // Tags in the input addresses are ignored and not present + // in the returned ranges. + virtual llvm::Expected<std::vector<TagRange>> + MakeTaggedRanges(lldb::addr_t addr, lldb::addr_t end_addr, + lldb_private::MemoryRegionInfos memory_regions) const = 0; + // Return the type value to use in GDB protocol qMemTags packets to read // allocation tags. This is named "Allocation" specifically because the spec // allows for logical tags to be read the same way, though we do not use that.
_______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits