DavidSpickett updated this revision to Diff 345106. DavidSpickett added a comment.
Address comments in general. Added an assert in the loop where we call ptrace so that we catch any unexpected condition that would make it an infinite loop. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95601/new/ https://reviews.llvm.org/D95601 Files: lldb/include/lldb/Host/common/NativeProcessProtocol.h lldb/include/lldb/Host/linux/Ptrace.h lldb/include/lldb/Utility/StringExtractorGDBRemote.h lldb/source/Host/common/NativeProcessProtocol.cpp lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp lldb/source/Plugins/Process/Linux/NativeProcessLinux.h lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux.h lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.h lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h lldb/source/Utility/StringExtractorGDBRemote.cpp lldb/test/API/tools/lldb-server/memory-tagging/Makefile lldb/test/API/tools/lldb-server/memory-tagging/TestGdbRemoteMemoryTagging.py lldb/test/API/tools/lldb-server/memory-tagging/main.c
Index: lldb/test/API/tools/lldb-server/memory-tagging/main.c =================================================================== --- /dev/null +++ lldb/test/API/tools/lldb-server/memory-tagging/main.c @@ -0,0 +1,55 @@ +#include <arm_acle.h> +#include <asm/hwcap.h> +#include <linux/mman.h> +#include <stdio.h> +#include <sys/auxv.h> +#include <sys/mman.h> +#include <sys/prctl.h> +#include <unistd.h> + +int print_result(char *ptr) { + // Page size allows the test to try reading off of the end of the page + printf("buffer: %p page_size: 0x%x\n", ptr, sysconf(_SC_PAGESIZE)); + + // Exit after some time, so we don't leave a zombie process + // if the test framework lost track of us. + sleep(60); + return 0; +} + +int main(int argc, char const *argv[]) { + if (prctl(PR_SET_TAGGED_ADDR_CTRL, + PR_TAGGED_ADDR_ENABLE | PR_MTE_TCF_SYNC | + // Allow all tags to be generated by the addg + // instruction __arm_mte_increment_tag produces. + (0xffff << PR_MTE_TAG_SHIFT), + 0, 0, 0)) { + return print_result(NULL); + } + + size_t page_size = sysconf(_SC_PAGESIZE); + char *buf = mmap(0, page_size, PROT_READ | PROT_WRITE | PROT_MTE, + MAP_PRIVATE | MAP_ANONYMOUS, -1, 0); + if (buf == MAP_FAILED) + return print_result(NULL); + + // Set incrementing tags until end of the page + char *tagged_ptr = buf; + // This intrinsic treats the addresses as if they were untagged + while (__arm_mte_ptrdiff(tagged_ptr, buf) < page_size) { + // This sets the allocation tag + __arm_mte_set_tag(tagged_ptr); + // Set the tag of the next granule (hence +16) to the next + // tag value. Returns a new pointer with the new logical tag. + // Tag values wrap at 0xF so it'll cycle. + tagged_ptr = __arm_mte_increment_tag(tagged_ptr + 16, 1); + } + + // lldb-server should be removing the top byte from addresses passed + // to ptrace. So put some random bits in there. + // ptrace expects you to remove them but it can still succeed if you + // don't. So this isn't proof that we're removing them, it's just a + // smoke test in case something didn't account for them. + buf = (char *)((size_t)buf | ((size_t)0xAA << 56)); + return print_result(buf); +} Index: lldb/test/API/tools/lldb-server/memory-tagging/TestGdbRemoteMemoryTagging.py =================================================================== --- /dev/null +++ lldb/test/API/tools/lldb-server/memory-tagging/TestGdbRemoteMemoryTagging.py @@ -0,0 +1,116 @@ +import gdbremote_testcase +from lldbsuite.test.decorators import * +from lldbsuite.test.lldbtest import * +from lldbsuite.test import lldbutil + +class TestGdbRemoteMemoryTagging(gdbremote_testcase.GdbRemoteTestCaseBase): + + mydir = TestBase.compute_mydir(__file__) + + def check_qmemtags_response(self, body, expected): + self.test_sequence.add_log_lines(["read packet: $qMemTags:{}#00".format(body), + "send packet: ${}#00".format(expected), + ], + True) + self.expect_gdbremote_sequence() + + @skipUnlessArch("aarch64") + @skipUnlessPlatform(["linux"]) + @skipUnlessAArch64MTELinuxCompiler + def test_qmemtags_packets(self): + """ Test that qMemTags packets are parsed correctly and/or rejected. """ + + self.build() + self.set_inferior_startup_launch() + procs = self.prep_debug_monitor_and_inferior() + + # Run the process + self.test_sequence.add_log_lines( + [ + # Start running after initial stop + "read packet: $c#63", + # Match the address of the MTE page + {"type": "output_match", + "regex": self.maybe_strict_output_regex(r"buffer: (.+) page_size: (.+)\r\n"), + "capture": {1: "buffer", 2: "page_size"}}, + # Now stop the inferior + "read packet: {}".format(chr(3)), + # And wait for the stop notification + {"direction": "send", "regex": r"^\$T[0-9a-fA-F]{2}thread:[0-9a-fA-F]+;"}], + True) + + # Run the packet stream + context = self.expect_gdbremote_sequence() + self.assertIsNotNone(context) + + buf_address = context.get("buffer") + self.assertIsNotNone(buf_address) + page_size = context.get("page_size") + self.assertIsNotNone(page_size) + + # nil means we couldn't set up a tagged page because the + # target doesn't support it. + if buf_address == "(nil)": + self.skipTest("Target must support MTE.") + + buf_address = int(buf_address, 16) + page_size = int(page_size, 16) + + # In the tests below E03 means the packet wasn't formed correctly + # and E01 means it was but we had some other error acting upon it. + + # Sanity check that address is correct + self.check_qmemtags_response("{:x},20:1".format(buf_address), "m0001") + + # Invalid packets + + # No content + self.check_qmemtags_response("", "E03") + # Only address + self.check_qmemtags_response("{:x}".format(buf_address), "E03") + # Only address and length + self.check_qmemtags_response("{:x},20".format(buf_address), "E03") + # Empty address + self.check_qmemtags_response(",20:1", "E03") + # Invalid addresses + self.check_qmemtags_response("aardvark,20:1", "E03") + self.check_qmemtags_response("-100,20:1", "E03") + self.check_qmemtags_response("cafe?,20:1", "E03") + # Empty length + self.check_qmemtags_response("{:x},:1".format(buf_address), "E03") + # Invalid lengths + self.check_qmemtags_response("{:x},food:1".format(buf_address), "E03") + self.check_qmemtags_response("{:x},-1:1".format(buf_address), "E03") + self.check_qmemtags_response("{:x},12??:1".format(buf_address), "E03") + # Empty type + self.check_qmemtags_response("{:x},10:".format(buf_address), "E03") + # Types we don't support + self.check_qmemtags_response("{:x},10:FF".format(buf_address), "E01") + # (even if the length of the read is zero) + self.check_qmemtags_response("{:x},0:FF".format(buf_address), "E01") + self.check_qmemtags_response("{:x},10:-1".format(buf_address), "E01") + self.check_qmemtags_response("{:x},10:+20".format(buf_address), "E01") + # Invalid type format + self.check_qmemtags_response("{:x},10:cat".format(buf_address), "E03") + self.check_qmemtags_response("{:x},10:?11".format(buf_address), "E03") + + # Valid packets + + # Reading nothing is allowed + self.check_qmemtags_response("{:x},0:1".format(buf_address), "m") + # A range that's already aligned + self.check_qmemtags_response("{:x},20:1".format(buf_address), "m0001") + # lldb-server should re-align the range + # Here we read from 1/2 way through a granule, into the next. Expands to 2 granules + self.check_qmemtags_response("{:x},10:1".format(buf_address+64-8), "m0304") + # Read up to the end of an MTE page. + # We know that the last tag should be 0xF since page size will always be a + # multiple of 256 bytes, which is 16 granules and we have 16 tags to use. + self.check_qmemtags_response("{:x},10:1".format(buf_address+page_size-16), "m0f") + # Here we read off of the end of the MTE range so ptrace gives us one tag, + # then fails on the second call. To lldb-server this means the response + # should just be an error, not a partial read. + self.check_qmemtags_response("{:x},20:1".format(buf_address+page_size-16), "E01") + # Note that we do not test reading over a page boundary within the same + # mapping. That logic is handled in the kernel itself so it's just a single + # ptrace call for lldb-server. Index: lldb/test/API/tools/lldb-server/memory-tagging/Makefile =================================================================== --- /dev/null +++ lldb/test/API/tools/lldb-server/memory-tagging/Makefile @@ -0,0 +1,4 @@ +C_SOURCES := main.c +CFLAGS_EXTRAS := -march=armv8.5-a+memtag + +include Makefile.rules Index: lldb/source/Utility/StringExtractorGDBRemote.cpp =================================================================== --- lldb/source/Utility/StringExtractorGDBRemote.cpp +++ lldb/source/Utility/StringExtractorGDBRemote.cpp @@ -223,6 +223,8 @@ return eServerPacketType_qMemoryRegionInfoSupported; if (PACKET_STARTS_WITH("qModuleInfo:")) return eServerPacketType_qModuleInfo; + if (PACKET_STARTS_WITH("qMemTags:")) + return eServerPacketType_qMemTags; break; case 'P': Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h =================================================================== --- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h +++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h @@ -216,6 +216,8 @@ PacketResult Handle_g(StringExtractorGDBRemote &packet); + PacketResult Handle_qMemTags(StringExtractorGDBRemote &packet); + void SetCurrentThreadID(lldb::tid_t tid); lldb::tid_t GetCurrentThreadID() const; Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp =================================================================== --- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp +++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp @@ -13,6 +13,7 @@ #include <chrono> #include <cstring> +#include <limits> #include <thread> #include "GDBRemoteCommunicationServerLLGS.h" @@ -211,6 +212,10 @@ RegisterMemberFunctionHandler(StringExtractorGDBRemote::eServerPacketType_g, &GDBRemoteCommunicationServerLLGS::Handle_g); + RegisterMemberFunctionHandler( + StringExtractorGDBRemote::eServerPacketType_qMemTags, + &GDBRemoteCommunicationServerLLGS::Handle_qMemTags); + RegisterPacketHandler(StringExtractorGDBRemote::eServerPacketType_k, [this](StringExtractorGDBRemote packet, Status &error, bool &interrupt, bool &quit) { @@ -3414,6 +3419,71 @@ return SendOKResponse(); } +GDBRemoteCommunication::PacketResult +GDBRemoteCommunicationServerLLGS::Handle_qMemTags( + StringExtractorGDBRemote &packet) { + Log *log(GetLogIfAnyCategoriesSet(LIBLLDB_LOG_PROCESS)); + + // Ensure we have a process. + if (!m_current_process || + (m_current_process->GetID() == LLDB_INVALID_PROCESS_ID)) { + LLDB_LOGF( + log, + "GDBRemoteCommunicationServerLLGS::%s failed, no process available", + __FUNCTION__); + return SendErrorResponse(1); + } + + // We are expecting + // qMemTags:<hex address>,<hex length>:<hex type> + + // Address + packet.SetFilePos(strlen("qMemTags:")); + const char *current_char = packet.Peek(); + if (!current_char || *current_char == ',') + return SendIllFormedResponse(packet, "Missing address in qMemTags packet"); + const lldb::addr_t addr = packet.GetHexMaxU64(/*little_endian=*/false, 0); + + // Length + char previous_char = packet.GetChar(); + current_char = packet.Peek(); + // If we don't have a separator or the length field is empty + if (previous_char != ',' || (current_char && *current_char == ':')) + return SendIllFormedResponse(packet, + "Invalid addr,length pair in qMemTags packet"); + + if (packet.GetBytesLeft() < 1) + return SendIllFormedResponse( + packet, "Too short qMemtags: packet (looking for length)"); + const size_t length = packet.GetHexMaxU64(/*little_endian=*/false, 0); + + // Type + const char *invalid_type_err = "Invalid type field in qMemTags: packet"; + if (packet.GetBytesLeft() < 1 || packet.GetChar() != ':') + return SendIllFormedResponse(packet, invalid_type_err); + + int32_t type = + packet.GetS32(std::numeric_limits<int32_t>::max(), /*base=*/16); + if (type == std::numeric_limits<int32_t>::max() || + // To catch inputs like "123aardvark" that will parse but clearly aren't + // valid in this case. + packet.GetBytesLeft()) { + return SendIllFormedResponse(packet, invalid_type_err); + } + + StreamGDBRemote response; + std::vector<uint8_t> tags; + Status error = m_current_process->ReadMemoryTags(type, addr, length, tags); + if (error.Fail()) + return SendErrorResponse(1); + + // This m is here in case we want to support multi part replies in the future. + // In the same manner as qfThreadInfo/qsThreadInfo. + response.PutChar('m'); + response.PutBytesAsRawHex8(tags.data(), tags.size()); + return SendPacketNoLock(response.GetString()); +} + void GDBRemoteCommunicationServerLLGS::MaybeCloseInferiorTerminalConnection() { Log *log(GetLogIfAnyCategoriesSet(LIBLLDB_LOG_PROCESS)); Index: lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.h =================================================================== --- lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.h +++ lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.h @@ -53,6 +53,9 @@ bool RegisterOffsetIsDynamic() const override { return true; } + llvm::Expected<MemoryTaggingDetails> + GetMemoryTaggingDetails(int32_t type) override; + protected: Status ReadGPR() override; Index: lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp =================================================================== --- lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp +++ lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp @@ -21,6 +21,7 @@ #include "Plugins/Process/Linux/NativeProcessLinux.h" #include "Plugins/Process/Linux/Procfs.h" #include "Plugins/Process/POSIX/ProcessPOSIXLog.h" +#include "Plugins/Process/Utility/MemoryTagManagerAArch64MTE.h" #include "Plugins/Process/Utility/RegisterInfoPOSIX_arm64.h" // System includes - They have to be included after framework includes because @@ -877,4 +878,15 @@ return expedited_reg_nums; } +llvm::Expected<NativeRegisterContextLinux::MemoryTaggingDetails> +NativeRegisterContextLinux_arm64::GetMemoryTaggingDetails(int32_t type) { + if (type == MemoryTagManagerAArch64MTE::eMTE_allocation) { + return MemoryTaggingDetails{std::make_unique<MemoryTagManagerAArch64MTE>(), + PTRACE_PEEKMTETAGS, PTRACE_POKEMTETAGS}; + } + + return llvm::createStringError(llvm::inconvertibleErrorCode(), + "Unknown AArch64 memory tag type %d", type); +} + #endif // defined (__arm64__) || defined (__aarch64__) Index: lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux.h =================================================================== --- lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux.h +++ lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux.h @@ -11,6 +11,8 @@ #include "Plugins/Process/Utility/NativeRegisterContextRegisterInfo.h" #include "lldb/Host/common/NativeThreadProtocol.h" +#include "lldb/Target/MemoryTagManager.h" +#include "llvm/Support/Error.h" namespace lldb_private { namespace process_linux { @@ -57,6 +59,22 @@ /// they are supported. virtual llvm::Optional<MmapData> GetMmapData() { return llvm::None; } + struct MemoryTaggingDetails { + /// Object with tag handling utilities. If the function below returns + /// a valid structure, you can assume that this pointer is valid. + std::unique_ptr<MemoryTagManager> manager; + int ptrace_read_req; /// ptrace operation number for memory tag read + int ptrace_write_req; /// ptrace operation number for memory tag write + }; + /// Return architecture specific data needed to use memory tags, + /// if they are supported. + virtual llvm::Expected<MemoryTaggingDetails> + GetMemoryTaggingDetails(int32_t type) { + return llvm::createStringError( + llvm::inconvertibleErrorCode(), + "Architecture does not support memory tagging"); + } + protected: // NB: This constructor is here only because gcc<=6.5 requires a virtual base // class initializer on abstract class (even though it is never used). It can Index: lldb/source/Plugins/Process/Linux/NativeProcessLinux.h =================================================================== --- lldb/source/Plugins/Process/Linux/NativeProcessLinux.h +++ lldb/source/Plugins/Process/Linux/NativeProcessLinux.h @@ -80,6 +80,9 @@ llvm::Error DeallocateMemory(lldb::addr_t addr) override; + Status ReadMemoryTags(int32_t type, lldb::addr_t addr, size_t len, + std::vector<uint8_t> &tags) override; + size_t UpdateThreads() override; const ArchSpec &GetArchitecture() const override { return m_arch; } Index: lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp =================================================================== --- lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp +++ lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp @@ -1431,6 +1431,61 @@ return llvm::Error::success(); } +Status NativeProcessLinux::ReadMemoryTags(int32_t type, lldb::addr_t addr, + size_t len, + std::vector<uint8_t> &tags) { + llvm::Expected<NativeRegisterContextLinux::MemoryTaggingDetails> details = + GetCurrentThread()->GetRegisterContext().GetMemoryTaggingDetails(type); + if (!details) + return Status(details.takeError()); + + // Ignore 0 length read + if (!len) + return Status(); + + // lldb will align the range it requests but it is not required to by + // the protocol so we'll do it again just in case. + // Remove non address bits too. Ptrace calls may work regardless but that + // is not a guarantee. + MemoryTagManager::TagRange range(details->manager->RemoveNonAddressBits(addr), + len); + range = details->manager->ExpandToGranule(range); + + // Allocate enough space for all tags to be read + size_t num_tags = range.GetByteSize() / details->manager->GetGranuleSize(); + tags.resize(num_tags * details->manager->GetTagSizeInBytes()); + + struct iovec tags_iovec; + uint8_t *dest = tags.data(); + lldb::addr_t read_addr = range.GetRangeBase(); + + // This call can return partial data so loop until we error or + // get all tags back. + while (num_tags) { + tags_iovec.iov_base = dest; + tags_iovec.iov_len = num_tags; + + Status error = NativeProcessLinux::PtraceWrapper( + details->ptrace_read_req, GetID(), reinterpret_cast<void *>(read_addr), + static_cast<void *>(&tags_iovec), 0, nullptr); + + if (error.Fail()) { + // Discard partial reads + tags.resize(0); + return error; + } + + size_t tags_read = tags_iovec.iov_len; + assert(tags_read && (tags_read <= num_tags)); + + dest += tags_read * details->manager->GetTagSizeInBytes(); + read_addr += details->manager->GetGranuleSize() * tags_read; + num_tags -= tags_read; + } + + return Status(); +} + size_t NativeProcessLinux::UpdateThreads() { // The NativeProcessLinux monitoring threads are always up to date with // respect to thread state and they keep the thread list populated properly. Index: lldb/source/Host/common/NativeProcessProtocol.cpp =================================================================== --- lldb/source/Host/common/NativeProcessProtocol.cpp +++ lldb/source/Host/common/NativeProcessProtocol.cpp @@ -52,6 +52,12 @@ return Status("not implemented"); } +lldb_private::Status +NativeProcessProtocol::ReadMemoryTags(int32_t type, lldb::addr_t addr, + size_t len, std::vector<uint8_t> &tags) { + return Status("not implemented"); +} + llvm::Optional<WaitStatus> NativeProcessProtocol::GetExitStatus() { if (m_state == lldb::eStateExited) return m_exit_status; Index: lldb/include/lldb/Utility/StringExtractorGDBRemote.h =================================================================== --- lldb/include/lldb/Utility/StringExtractorGDBRemote.h +++ lldb/include/lldb/Utility/StringExtractorGDBRemote.h @@ -167,6 +167,8 @@ eServerPacketType_jLLDBTraceStop, eServerPacketType_jLLDBTraceGetState, eServerPacketType_jLLDBTraceGetBinaryData, + + eServerPacketType_qMemTags, }; ServerPacketType GetServerPacketType() const; Index: lldb/include/lldb/Host/linux/Ptrace.h =================================================================== --- lldb/include/lldb/Host/linux/Ptrace.h +++ lldb/include/lldb/Host/linux/Ptrace.h @@ -50,6 +50,12 @@ #define ARCH_GET_FS 0x1003 #define ARCH_GET_GS 0x1004 #endif +#ifndef PTRACE_PEEKMTETAGS +#define PTRACE_PEEKMTETAGS 33 +#endif +#ifndef PTRACE_POKEMTETAGS +#define PTRACE_POKEMTETAGS 34 +#endif #define LLDB_PTRACE_NT_ARM_TLS 0x401 // ARM TLS register Index: lldb/include/lldb/Host/common/NativeProcessProtocol.h =================================================================== --- lldb/include/lldb/Host/common/NativeProcessProtocol.h +++ lldb/include/lldb/Host/common/NativeProcessProtocol.h @@ -87,6 +87,9 @@ Status ReadMemoryWithoutTrap(lldb::addr_t addr, void *buf, size_t size, size_t &bytes_read); + virtual Status ReadMemoryTags(int32_t type, lldb::addr_t addr, size_t len, + std::vector<uint8_t> &tags); + /// Reads a null terminated string from memory. /// /// Reads up to \p max_size bytes of memory until it finds a '\0'.
_______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits