DavidSpickett updated this revision to Diff 330684. DavidSpickett added a comment.
Correctly handle the ptrace call by looping until we get all tags as an error. I've gone ahead and treated anything but all tags as an error as far as lldb-server is concerned. Tell me what you think of that. 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/packages/Python/lldbsuite/test/decorators.py 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 @@ -220,6 +220,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 @@ -201,6 +201,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" @@ -207,6 +208,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) { @@ -3511,6 +3516,72 @@ return SendOKResponse(); } +GDBRemoteCommunication::PacketResult +GDBRemoteCommunicationServerLLGS::Handle_qMemTags( + StringExtractorGDBRemote &packet) { + Log *log(GetLogIfAnyCategoriesSet(LIBLLDB_LOG_PROCESS)); + + // Ensure we have a process. + if (!m_debugged_process_up || + (m_debugged_process_up->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_debugged_process_up->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 @@ -52,6 +52,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 @@ -736,4 +737,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 { @@ -55,6 +57,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: lldb::ByteOrder GetByteOrder() const; Index: lldb/source/Plugins/Process/Linux/NativeProcessLinux.h =================================================================== --- lldb/source/Plugins/Process/Linux/NativeProcessLinux.h +++ lldb/source/Plugins/Process/Linux/NativeProcessLinux.h @@ -78,6 +78,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 @@ -1322,6 +1322,59 @@ 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->GetBytesPerTag()); + + struct iovec tags_vec; + uint8_t *dest = &tags[0]; + 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 > 0) { + tags_vec.iov_base = dest; + tags_vec.iov_len = num_tags; + + Status error = NativeProcessLinux::PtraceWrapper( + details->ptrace_read_req, GetID(), reinterpret_cast<void *>(read_addr), + static_cast<void *>(&tags_vec), 0, nullptr); + + if (error.Fail()) { + // Discard partial reads + tags.resize(0); + return error; + } + + size_t tags_read = tags_vec.iov_len; + dest += tags_read * details->manager->GetBytesPerTag(); + 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 @@ -54,6 +54,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/packages/Python/lldbsuite/test/decorators.py =================================================================== --- lldb/packages/Python/lldbsuite/test/decorators.py +++ lldb/packages/Python/lldbsuite/test/decorators.py @@ -823,6 +823,40 @@ """Skip this test if the environment is set up to run LLDB *itself* under ASAN.""" return skipTestIfFn(is_running_under_asan)(func) +def skipUnlessAArch64MTELinuxCompiler(func): + """Decorate the item to skip test unless MTE is supported by the test compiler.""" + + def is_toolchain_with_mte(self): + compiler_path = self.getCompiler() + compiler = os.path.basename(compiler_path) + f = tempfile.NamedTemporaryFile() + if lldbplatformutil.getPlatform() == 'windows': + return "MTE tests are not compatible with 'windows'" + + cmd = "echo 'int main() {}' | %s -x c -o %s -" % (compiler_path, f.name) + if os.popen(cmd).close() is not None: + # Cannot compile at all, don't skip the test + # so that we report the broken compiler normally. + return None + + # We need the Linux headers and ACLE MTE intrinsics + test_src = """ + #include <asm/hwcap.h> + #include <arm_acle.h> + #ifndef HWCAP2_MTE + #error + #endif + int main() { + void* ptr = __arm_mte_create_random_tag((void*)(0), 0); + }""" + cmd = "echo '%s' | %s -march=armv8.5-a+memtag -x c -o %s -" % (test_src, compiler_path, f.name) + if os.popen(cmd).close() is not None: + return "Toolchain does not support MTE" + return None + + return skipTestIfFn(is_toolchain_with_mte)(func) + + def _get_bool_config(key, fail_value = True): """ Returns the current LLDB's build config value. Index: lldb/include/lldb/Utility/StringExtractorGDBRemote.h =================================================================== --- lldb/include/lldb/Utility/StringExtractorGDBRemote.h +++ lldb/include/lldb/Utility/StringExtractorGDBRemote.h @@ -169,6 +169,8 @@ eServerPacketType_jTraceConfigRead, // deprecated eServerPacketType_jLLDBTraceSupportedType, + + 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 @@ -85,6 +85,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