[Lldb-commits] [PATCH] D102445: Switch from using member_clang_type.GetByteSize() to member_type->GetByteSize() in ParseSingleMember
aprantl added a comment. I guess we could add least add a comment like this? // By calling member_type instead of member_clang_type, the size comes from DWARF, which avoids premature record layout. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D102445/new/ https://reviews.llvm.org/D102445 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D102445: Switch from using member_clang_type.GetByteSize() to member_type->GetByteSize() in ParseSingleMember
aprantl accepted this revision. aprantl added a comment. This revision is now accepted and ready to land. The change itself seems good to me, I wonder what we can do to prevent a similar mistake in the future. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D102445/new/ https://reviews.llvm.org/D102445 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D102445: Switch from using member_clang_type.GetByteSize() to member_type->GetByteSize() in ParseSingleMember
shafik created this revision. shafik added reviewers: aprantl, teemperor, labath. shafik requested review of this revision. We have a bug in which using `member_clang_type.GetByteSize()` triggers record layout and during this process since the record was not yet complete we ended up reaching a record that had not been layed out yet. Using `member_type->GetByteSize() ` avoids this situation since it relies on size from DWARF and will not trigger record layout. We were not able to reduce this to minimal reproducer. The crash goes away in this case when we have a dSYM and we have seen that ordering in symbol processing can change the behavior as well. For reference: rdar://77293040 https://reviews.llvm.org/D102445 Files: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp === --- lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp +++ lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp @@ -2671,7 +2671,7 @@ last_field_info.bit_offset = field_bit_offset; if (llvm::Optional clang_type_size = - member_clang_type.GetByteSize(nullptr)) { + member_type->GetByteSize(nullptr)) { last_field_info.bit_size = *clang_type_size * character_width; } Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp === --- lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp +++ lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp @@ -2671,7 +2671,7 @@ last_field_info.bit_offset = field_bit_offset; if (llvm::Optional clang_type_size = - member_clang_type.GetByteSize(nullptr)) { + member_type->GetByteSize(nullptr)) { last_field_info.bit_size = *clang_type_size * character_width; } ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D102428: [StopInfoMachException] Summarize arm64e BLRAx/LDRAx auth failures
vsk created this revision. vsk added reviewers: JDevlieghere, jingham, jasonmolenda. Herald added subscribers: omjavaid, kristof.beyls. vsk requested review of this revision. Herald added a project: LLDB. Upstream lldb support for summarizing BLRAx and LDRAx auth failures. rdar://41615322 Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D102428 Files: lldb/include/lldb/Core/Address.h lldb/include/lldb/Core/Disassembler.h lldb/packages/Python/lldbsuite/test/functionalities/ptrauth_diagnostics/BLRAA_error/Makefile lldb/packages/Python/lldbsuite/test/functionalities/ptrauth_diagnostics/BLRAA_error/TestPtrauthBLRAADiagnostic.py lldb/packages/Python/lldbsuite/test/functionalities/ptrauth_diagnostics/BLRAA_error/blraa.c lldb/packages/Python/lldbsuite/test/functionalities/ptrauth_diagnostics/BRAA_error/Makefile lldb/packages/Python/lldbsuite/test/functionalities/ptrauth_diagnostics/BRAA_error/TestPtrauthBRAADiagnostic.py lldb/packages/Python/lldbsuite/test/functionalities/ptrauth_diagnostics/BRAA_error/braa.c lldb/packages/Python/lldbsuite/test/functionalities/ptrauth_diagnostics/LDRAA_error/Makefile lldb/packages/Python/lldbsuite/test/functionalities/ptrauth_diagnostics/LDRAA_error/TestPtrauthLDRAADiagnostic.py lldb/packages/Python/lldbsuite/test/functionalities/ptrauth_diagnostics/LDRAA_error/ldraa.c lldb/packages/Python/lldbsuite/test/functionalities/ptrauth_diagnostics/brkC47x_code/Makefile lldb/packages/Python/lldbsuite/test/functionalities/ptrauth_diagnostics/brkC47x_code/TestPtrauthBRKc47xDiagnostic.py lldb/packages/Python/lldbsuite/test/functionalities/ptrauth_diagnostics/brkC47x_code/brkC47x.c lldb/source/Core/Address.cpp lldb/source/Core/Disassembler.cpp lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp lldb/source/Plugins/Process/Utility/StopInfoMachException.cpp lldb/source/Plugins/Process/Utility/StopInfoMachException.h Index: lldb/source/Plugins/Process/Utility/StopInfoMachException.h === --- lldb/source/Plugins/Process/Utility/StopInfoMachException.h +++ lldb/source/Plugins/Process/Utility/StopInfoMachException.h @@ -16,6 +16,11 @@ namespace lldb_private { class StopInfoMachException : public StopInfo { + /// Determine the pointer-authentication related failure that caused this + /// exception. Returns true and fills out the failure description if there + /// is auth-related failure, and returns false otherwise. + bool DeterminePtrauthFailure(ExecutionContext _ctx); + public: // Constructors and Destructors StopInfoMachException(Thread , uint32_t exc_type, Index: lldb/source/Plugins/Process/Utility/StopInfoMachException.cpp === --- lldb/source/Plugins/Process/Utility/StopInfoMachException.cpp +++ lldb/source/Plugins/Process/Utility/StopInfoMachException.cpp @@ -17,6 +17,7 @@ #include "lldb/Breakpoint/Watchpoint.h" #include "lldb/Symbol/Symbol.h" +#include "lldb/Target/ABI.h" #include "lldb/Target/DynamicLoader.h" #include "lldb/Target/ExecutionContext.h" #include "lldb/Target/Process.h" @@ -30,6 +31,181 @@ using namespace lldb; using namespace lldb_private; +/// Information about a pointer-authentication related instruction. +struct PtrauthInstructionInfo { + bool IsAuthenticated; + bool IsLoad; + bool DoesBranch; +}; + +/// Get any pointer-authentication related information about the instruction +/// at address \p at_addr. +static llvm::Optional +GetPtrauthInstructionInfo(Target , const ArchSpec , + const Address _addr) { + const char *plugin_name = nullptr; + const char *flavor = nullptr; + AddressRange range_bounds(at_addr, 4); + const bool prefer_file_cache = true; + DisassemblerSP disassembler_sp = Disassembler::DisassembleRange( + arch, plugin_name, flavor, target, range_bounds, prefer_file_cache); + if (!disassembler_sp) +return llvm::None; + + InstructionList _list = disassembler_sp->GetInstructionList(); + InstructionSP insn = insn_list.GetInstructionAtIndex(0); + if (!insn) +return llvm::None; + + return PtrauthInstructionInfo{insn->IsAuthenticated(), insn->IsLoad(), +insn->DoesBranch()}; +} + +/// Describe the load address of \p addr using the format filename:line:col. +static void DescribeAddressBriefly(Stream , const Address , + Target ) { + strm.Printf("at address=0x%" PRIx64, addr.GetLoadAddress()); + StreamString s; + if (addr.GetDescription(s, target, eDescriptionLevelBrief)) +strm.Printf(" %s", s.GetString().data()); + strm.Printf(".\n"); +} + +bool StopInfoMachException::DeterminePtrauthFailure(ExecutionContext _ctx) { + bool IsBreakpoint = m_value == 6; // EXC_BREAKPOINT + bool IsBadAccess = m_value == 1; // EXC_BAD_ACCESS + if (!IsBreakpoint && !IsBadAccess) +return false; + + // Check that we have a live process.
[Lldb-commits] [lldb] f93e9c1 - [lldb] Fixup indirect symbols as they are signed.
Author: Jonas Devlieghere Date: 2021-05-13T10:27:22-07:00 New Revision: f93e9c12bf482dbfe3d4d00fcf8bbc251500dd99 URL: https://github.com/llvm/llvm-project/commit/f93e9c12bf482dbfe3d4d00fcf8bbc251500dd99 DIFF: https://github.com/llvm/llvm-project/commit/f93e9c12bf482dbfe3d4d00fcf8bbc251500dd99.diff LOG: [lldb] Fixup indirect symbols as they are signed. This fixes a bunch of test failures in Apple Silicon (arm64e). Added: Modified: lldb/source/Target/Process.cpp Removed: diff --git a/lldb/source/Target/Process.cpp b/lldb/source/Target/Process.cpp index 514a8f7dbbca..480759e6e3fe 100644 --- a/lldb/source/Target/Process.cpp +++ b/lldb/source/Target/Process.cpp @@ -5643,6 +5643,8 @@ addr_t Process::ResolveIndirectFunction(const Address *address, Status ) { symbol ? symbol->GetName().AsCString() : ""); function_addr = LLDB_INVALID_ADDRESS; } else { + if (ABISP abi_sp = GetABI()) +function_addr = abi_sp->FixCodeAddress(function_addr); m_resolved_indirect_addresses.insert( std::pair(addr, function_addr)); } ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] ce12b52 - [lldb] Fixup more code addresses
Author: Jonas Devlieghere Date: 2021-05-13T10:27:22-07:00 New Revision: ce12b52de2fb3f319ff18effc4ea9ff4d369f328 URL: https://github.com/llvm/llvm-project/commit/ce12b52de2fb3f319ff18effc4ea9ff4d369f328 DIFF: https://github.com/llvm/llvm-project/commit/ce12b52de2fb3f319ff18effc4ea9ff4d369f328.diff LOG: [lldb] Fixup more code addresses The Swift async task pointers are signed on arm64e and we need to fixup the addresses in the CFA and DWARF expressions. Added: Modified: lldb/source/Expression/DWARFExpression.cpp lldb/source/Target/RegisterContextUnwind.cpp Removed: diff --git a/lldb/source/Expression/DWARFExpression.cpp b/lldb/source/Expression/DWARFExpression.cpp index 12fe76ee95d2f..776d7f253a8ff 100644 --- a/lldb/source/Expression/DWARFExpression.cpp +++ b/lldb/source/Expression/DWARFExpression.cpp @@ -1130,6 +1130,8 @@ bool DWARFExpression::Evaluate( lldb::addr_t pointer_value = process->ReadPointerFromMemory(pointer_addr, error); if (pointer_value != LLDB_INVALID_ADDRESS) { + if (ABISP abi_sp = process->GetABI()) +pointer_value = abi_sp->FixCodeAddress(pointer_value); stack.back().GetScalar() = pointer_value; stack.back().ClearContext(); } else { diff --git a/lldb/source/Target/RegisterContextUnwind.cpp b/lldb/source/Target/RegisterContextUnwind.cpp index b8d9926ac77ba..1ce21e6306e09 100644 --- a/lldb/source/Target/RegisterContextUnwind.cpp +++ b/lldb/source/Target/RegisterContextUnwind.cpp @@ -1946,6 +1946,8 @@ bool RegisterContextUnwind::ReadFrameAddress( reg_info, cfa_reg_contents, reg_info->byte_size, reg_value); if (error.Success()) { address = reg_value.GetAsUInt64(); + if (ABISP abi_sp = m_thread.GetProcess()->GetABI()) +address = abi_sp->FixCodeAddress(address); UnwindLogMsg( "CFA value via dereferencing reg %s (%d): reg has val 0x%" PRIx64 ", CFA value is 0x%" PRIx64, @@ -2000,6 +2002,8 @@ bool RegisterContextUnwind::ReadFrameAddress( if (dwarfexpr.Evaluate(_ctx, this, 0, nullptr, nullptr, result, )) { address = result.GetScalar().ULongLong(); + if (ABISP abi_sp = m_thread.GetProcess()->GetABI()) +address = abi_sp->FixCodeAddress(address); UnwindLogMsg("CFA value set by DWARF expression is 0x%" PRIx64, address); ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D97285: [lldb][AArch64] Add "memory tag read" command
DavidSpickett updated this revision to Diff 345117. DavidSpickett added a comment. Rebase onto earlier patches. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D97285/new/ https://reviews.llvm.org/D97285 Files: lldb/source/Commands/CMakeLists.txt lldb/source/Commands/CommandObjectMemory.cpp lldb/source/Commands/CommandObjectMemoryTag.cpp lldb/source/Commands/CommandObjectMemoryTag.h lldb/test/API/functionalities/memory/tag/Makefile lldb/test/API/functionalities/memory/tag/TestMemoryTag.py lldb/test/API/functionalities/memory/tag/main.cpp lldb/test/API/linux/aarch64/mte_tag_read/Makefile lldb/test/API/linux/aarch64/mte_tag_read/TestAArch64LinuxMTEMemoryTagRead.py lldb/test/API/linux/aarch64/mte_tag_read/main.c Index: lldb/test/API/linux/aarch64/mte_tag_read/main.c === --- /dev/null +++ lldb/test/API/linux/aarch64/mte_tag_read/main.c @@ -0,0 +1,77 @@ +#include +#include +#include +#include +#include +#include +#include + +int main(int argc, char const *argv[]) { + // We assume that the test runner has checked we're on an MTE system + + 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. +(0x << PR_MTE_TAG_SHIFT), +0, 0, 0)) { +return 1; + } + + size_t page_size = sysconf(_SC_PAGESIZE); + + // Allocate memory with MTE + // We ask for two pages. One is read only so that we get + // 2 mappings in /proc/.../smaps so we can check reading + // a range across mappings. + // The first allocation will start at the highest address, + // so we allocate buf2 first to get: + // | buf | buf2 | + int prot = PROT_READ | PROT_MTE; + int flags = MAP_PRIVATE | MAP_ANONYMOUS; + + char *buf2 = mmap(0, page_size, prot, flags, -1, 0); + if (buf2 == MAP_FAILED) +return 1; + + // Writeable so we can set tags on it later + char *buf = mmap(0, page_size, prot | PROT_WRITE, flags, -1, 0); + if (buf == MAP_FAILED) +return 1; + + // We expect the mappings to be next to each other + if (buf2 - buf != page_size) +return 1; + + // And without MTE + char *non_mte_buf = mmap(0, page_size, PROT_READ | PROT_WRITE, flags, -1, 0); + if (non_mte_buf == MAP_FAILED) +return 1; + + // Set incrementing tags until end of the first page + char *tagged_ptr = buf; + // This ignores tag bits when subtracting the addresses + while (__arm_mte_ptrdiff(tagged_ptr, buf) < page_size) { +// Set the allocation tag for this location +__arm_mte_set_tag(tagged_ptr); +// + 16 for 16 byte granules +// Earlier we allowed all tag values, so this will give us an +// incrementing pattern 0-0xF wrapping back to 0. +tagged_ptr = __arm_mte_increment_tag(tagged_ptr + 16, 1); + } + + // Tag the original pointer with 9 + buf = __arm_mte_create_random_tag(buf, ~(1 << 9)); + // A different tag so that buf_alt_tag > buf if you don't handle the tag + char *buf_alt_tag = __arm_mte_create_random_tag(buf, ~(1 << 10)); + + // lldb should be removing the whole top byte, not just the tags. + // So fill 63-60 with something non zero so we'll fail if we only remove tags. +#define SET_TOP_NIBBLE(ptr) (char *)((size_t)(ptr) | (0xA << 60)) + buf = SET_TOP_NIBBLE(buf); + buf_alt_tag = SET_TOP_NIBBLE(buf_alt_tag); + buf2 = SET_TOP_NIBBLE(buf2); + + // Breakpoint here + return 0; +} Index: lldb/test/API/linux/aarch64/mte_tag_read/TestAArch64LinuxMTEMemoryTagRead.py === --- /dev/null +++ lldb/test/API/linux/aarch64/mte_tag_read/TestAArch64LinuxMTEMemoryTagRead.py @@ -0,0 +1,126 @@ +""" +Test "memory tag read" command on AArch64 Linux with MTE. +""" + + +import lldb +from lldbsuite.test.decorators import * +from lldbsuite.test.lldbtest import * +from lldbsuite.test import lldbutil + + +class AArch64LinuxMTEMemoryTagReadTestCase(TestBase): + +mydir = TestBase.compute_mydir(__file__) + +NO_DEBUG_INFO_TESTCASE = True + +@skipUnlessArch("aarch64") +@skipUnlessPlatform(["linux"]) +@skipUnlessAArch64MTELinuxCompiler +def test_mte_tag_read(self): +if not self.isAArch64MTE(): +self.skipTest('Target must support MTE.') + +self.build() +self.runCmd("file " + self.getBuildArtifact("a.out"), CURRENT_EXECUTABLE_SET) + +lldbutil.run_break_set_by_file_and_line(self, "main.c", +line_number('main.c', '// Breakpoint here'), +num_expected_locations=1) + +self.runCmd("run", RUN_SUCCEEDED) + +if self.process().GetState() == lldb.eStateExited: +self.fail("Test program failed to run.") + +self.expect("thread list", STOPPED_DUE_TO_BREAKPOINT, +substrs=['stopped', + 'stop reason =
[Lldb-commits] [PATCH] D95602: [lldb][AArch64] Add MTE memory tag reading to lldb
DavidSpickett updated this revision to Diff 345113. DavidSpickett added a comment. Rebase, fix use of SendPacketAndWaitForResponse. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95602/new/ https://reviews.llvm.org/D95602 Files: lldb/include/lldb/Core/Architecture.h lldb/include/lldb/Target/Process.h lldb/source/Plugins/Architecture/AArch64/ArchitectureAArch64.cpp lldb/source/Plugins/Architecture/AArch64/ArchitectureAArch64.h lldb/source/Plugins/Architecture/AArch64/CMakeLists.txt lldb/source/Plugins/Architecture/CMakeLists.txt lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h lldb/source/Target/Process.cpp lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp Index: lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp === --- lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp +++ lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp @@ -467,3 +467,68 @@ EXPECT_EQ(llvm::None, GetQOffsets("TextSeg=0x1234")); EXPECT_EQ(llvm::None, GetQOffsets("TextSeg=12345678123456789")); } + +static void +check_qmemtags(TestClient , MockServer , size_t read_len, + const char *packet, llvm::StringRef response, + llvm::Optional> expected_tag_data) { + const auto = [&](size_t len, const char *packet, + llvm::StringRef response) { +std::future result = std::async(std::launch::async, [&] { + return client.ReadMemoryTags(0xDEF0, read_len, 1); +}); + +HandlePacket(server, packet, response); +return result.get(); + }; + + auto result = ReadMemoryTags(0, packet, response); + if (expected_tag_data) { +ASSERT_TRUE(result); +llvm::ArrayRef expected(*expected_tag_data); +llvm::ArrayRef got = result->GetData(); +ASSERT_THAT(expected, testing::ContainerEq(got)); + } else { +ASSERT_FALSE(result); + } +} + +TEST_F(GDBRemoteCommunicationClientTest, ReadMemoryTags) { + // Zero length reads are valid + check_qmemtags(client, server, 0, "qMemTags:def0,0:1", "m", + std::vector{}); + + // The client layer does not check the length of the received data. + // All we need is the "m" and for the decode to use all of the chars + check_qmemtags(client, server, 32, "qMemTags:def0,20:1", "m09", + std::vector{0x9}); + + // Zero length response is fine as long as the "m" is present + check_qmemtags(client, server, 0, "qMemTags:def0,0:1", "m", + std::vector{}); + + // Normal responses + check_qmemtags(client, server, 16, "qMemTags:def0,10:1", "m66", + std::vector{0x66}); + check_qmemtags(client, server, 32, "qMemTags:def0,20:1", "m0102", + std::vector{0x1, 0x2}); + + // Empty response is an error + check_qmemtags(client, server, 17, "qMemTags:def0,11:1", "", llvm::None); + // Usual error response + check_qmemtags(client, server, 17, "qMemTags:def0,11:1", "E01", llvm::None); + // Leading m missing + check_qmemtags(client, server, 17, "qMemTags:def0,11:1", "01", llvm::None); + // Anything other than m is an error + check_qmemtags(client, server, 17, "qMemTags:def0,11:1", "z01", llvm::None); + // Decoding tag data doesn't use all the chars in the packet + check_qmemtags(client, server, 32, "qMemTags:def0,20:1", "m09zz", llvm::None); + // Data that is not hex bytes + check_qmemtags(client, server, 32, "qMemTags:def0,20:1", "mhello", + llvm::None); + // Data is not a complete hex char + check_qmemtags(client, server, 32, "qMemTags:def0,20:1", "m9", llvm::None); + // Data has a trailing hex char + check_qmemtags(client, server, 32, "qMemTags:def0,20:1", "m01020", + llvm::None); +} Index: lldb/source/Target/Process.cpp === --- lldb/source/Target/Process.cpp +++ lldb/source/Target/Process.cpp @@ -6073,3 +6073,84 @@ return false; } + +llvm::Expected +Process::GetMemoryTagManager(lldb::addr_t addr, lldb::addr_t end_addr) { + Architecture *arch = GetTarget().GetArchitecturePlugin(); + const MemoryTagManager *tag_manager = + arch ? arch->GetMemoryTagManager() : nullptr; + if (!arch || !tag_manager) { +return llvm::createStringError( +llvm::inconvertibleErrorCode(), +"This architecture does not support memory tagging", +GetPluginName().GetCString()); + } + + if (!SupportsMemoryTagging()) { +return llvm::createStringError(llvm::inconvertibleErrorCode(), + "Process does not support memory tagging"); + } + + ptrdiff_t len = tag_manager->AddressDiff(end_addr,
[Lldb-commits] [PATCH] D95601: [lldb][AArch64] Add memory tag reading to lldb-server
DavidSpickett marked 2 inline comments as done. DavidSpickett added inline comments. Comment at: lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp:1464 + // get all tags back. + while (num_tags > 0) { +tags_vec.iov_base = dest; omjavaid wrote: > this loop condition is is a little fishy. num_tags is unsigned which means > if by chance it doesnt end up going to zero we ll keep looping for ever. I couldn't see another loop condition that made sense to use, so I've added an assert below: ``` assert(tags_read && (tags_read <= num_tags)); ``` If num_tags was 0 we'd never enter the loop in the first place. Then we assert that if there was no error, the kernel returned at least 1 tag and no more tags than we asked for. This should prevent num_tags wrapping around 0 and causing an infinite loop. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95601/new/ https://reviews.llvm.org/D95601 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D95601: [lldb][AArch64] Add memory tag reading to lldb-server
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 +#include +#include +#include +#include +#include +#include +#include + +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. +(0x << 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":
[Lldb-commits] [PATCH] D97282: [lldb][AArch64] Add memory-tagging qSupported feature
DavidSpickett updated this revision to Diff 345105. DavidSpickett added a comment. Rebase, fix missed clang-format-ing. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D97282/new/ https://reviews.llvm.org/D97282 Files: lldb/include/lldb/Host/common/NativeProcessProtocol.h lldb/include/lldb/Target/Process.h lldb/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h lldb/test/API/tools/lldb-server/TestLldbGdbServer.py Index: lldb/test/API/tools/lldb-server/TestLldbGdbServer.py === --- lldb/test/API/tools/lldb-server/TestLldbGdbServer.py +++ lldb/test/API/tools/lldb-server/TestLldbGdbServer.py @@ -1025,6 +1025,14 @@ self.assertEqual(supported_dict.get('fork-events', '-'), '-') self.assertEqual(supported_dict.get('vfork-events', '-'), '-') +# We need to be able to self.runCmd to get cpuinfo, +# which is not possible when using a remote platform. +@skipIfRemote +def test_qSupported_memory_tagging(self): +supported_dict = self.get_qSupported_dict() +self.assertEqual(supported_dict.get("memory-tagging", '-'), + '+' if self.isAArch64MTE() else '-') + @skipIfWindows # No pty support to test any inferior output def test_written_M_content_reads_back_correctly(self): self.build() Index: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h === --- lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h +++ lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h @@ -235,6 +235,8 @@ friend class GDBRemoteCommunicationClient; friend class GDBRemoteRegisterContext; + bool SupportsMemoryTagging() override; + /// Broadcaster event bits definitions. enum { eBroadcastBitAsyncContinue = (1 << 0), Index: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp === --- lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp +++ lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp @@ -2769,6 +2769,10 @@ return 0; } +bool ProcessGDBRemote::SupportsMemoryTagging() { + return m_gdb_comm.GetMemoryTaggingSupported(); +} + Status ProcessGDBRemote::WriteObjectFile( std::vector entries) { Status error; 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 @@ -3608,6 +3608,8 @@ ret.push_back("qXfer:auxv:read+"); if (bool(plugin_features & Extension::libraries_svr4)) ret.push_back("qXfer:libraries-svr4:read+"); + if (bool(plugin_features & Extension::memory_tagging)) +ret.push_back("memory-tagging+"); // check for client features m_extensions_supported = {}; Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h === --- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h +++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h @@ -452,6 +452,8 @@ bool GetSharedCacheInfoSupported(); + bool GetMemoryTaggingSupported(); + /// Use qOffsets to query the offset used when relocating the target /// executable. If successful, the returned structure will contain at least /// one value in the offsets field. @@ -565,6 +567,7 @@ LazyBool m_supports_QPassSignals; LazyBool m_supports_error_string_reply; LazyBool m_supports_multiprocess; + LazyBool m_supports_memory_tagging; bool m_supports_qProcessInfoPID : 1, m_supports_qfProcessInfo : 1, m_supports_qUserName : 1, m_supports_qGroupName : 1, Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp === --- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp +++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp @@ -90,6 +90,7 @@ m_supports_QPassSignals(eLazyBoolCalculate), m_supports_error_string_reply(eLazyBoolCalculate), m_supports_multiprocess(eLazyBoolCalculate), + m_supports_memory_tagging(eLazyBoolCalculate), m_supports_qProcessInfoPID(true), m_supports_qfProcessInfo(true),
[Lldb-commits] [PATCH] D97281: [lldb][AArch64] Add class for managing memory tags
DavidSpickett updated this revision to Diff 345099. DavidSpickett added a comment. Address all comments apart from address handling that I need to spend some time thinking about. It may end up that the best way to do this is have the tag manager provide a way to remove only the tags and let the other parts of lldb deal with addresses using the ABI methods you're adding. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D97281/new/ https://reviews.llvm.org/D97281 Files: lldb/include/lldb/Target/MemoryTagManager.h lldb/source/Plugins/Process/Utility/CMakeLists.txt lldb/source/Plugins/Process/Utility/MemoryTagManagerAArch64MTE.cpp lldb/source/Plugins/Process/Utility/MemoryTagManagerAArch64MTE.h lldb/unittests/Process/Utility/CMakeLists.txt lldb/unittests/Process/Utility/MemoryTagManagerAArch64MTETest.cpp Index: lldb/unittests/Process/Utility/MemoryTagManagerAArch64MTETest.cpp === --- /dev/null +++ lldb/unittests/Process/Utility/MemoryTagManagerAArch64MTETest.cpp @@ -0,0 +1,120 @@ +//===-- MemoryTagManagerAArch64MTETest.cpp ===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===--===// + +#include "Plugins/Process/Utility/MemoryTagManagerAArch64MTE.h" +#include "llvm/Testing/Support/Error.h" +#include "gtest/gtest.h" + +using namespace lldb_private; + +TEST(MemoryTagManagerAArch64MTETest, UnpackTagsData) { + MemoryTagManagerAArch64MTE manager; + + // Error for insufficient tag data + std::vector input; + ASSERT_THAT_EXPECTED( + manager.UnpackTagsData(input, 2), + llvm::FailedWithMessage( + "Packed tag data size does not match expected number of tags. " + "Expected 2 tag(s) for 2 granules, got 0 tag(s).")); + + // This is out of the valid tag range + input.push_back(0x1f); + ASSERT_THAT_EXPECTED( + manager.UnpackTagsData(input, 1), + llvm::FailedWithMessage( + "Found tag 0x1f which is > max MTE tag value of 0xf.")); + + // MTE tags are 1 per byte + input.pop_back(); + input.push_back(0xe); + input.push_back(0xf); + + std::vector expected{0xe, 0xf}; + + llvm::Expected> got = + manager.UnpackTagsData(input, 2); + ASSERT_THAT_EXPECTED(got, llvm::Succeeded()); + ASSERT_THAT(expected, testing::ContainerEq(*got)); +} + +TEST(MemoryTagManagerAArch64MTETest, GetLogicalTag) { + MemoryTagManagerAArch64MTE manager; + + // Set surrounding bits to check shift is correct + ASSERT_EQ((lldb::addr_t)0, manager.GetLogicalTag(0xe0e0)); + // Max tag value + ASSERT_EQ((lldb::addr_t)0xf, manager.GetLogicalTag(0x0f00)); + ASSERT_EQ((lldb::addr_t)2, manager.GetLogicalTag(0x0200)); +} + +TEST(MemoryTagManagerAArch64MTETest, ExpandToGranule) { + MemoryTagManagerAArch64MTE manager; + // Reading nothing, no alignment needed + ASSERT_EQ( + MemoryTagManagerAArch64MTE::TagRange(0, 0), + manager.ExpandToGranule(MemoryTagManagerAArch64MTE::TagRange(0, 0))); + + // Ranges with 0 size are unchanged even if address is non 0 + // (normally 0x1234 would be aligned to 0x1230) + ASSERT_EQ( + MemoryTagManagerAArch64MTE::TagRange(0x1234, 0), + manager.ExpandToGranule(MemoryTagManagerAArch64MTE::TagRange(0x1234, 0))); + + // Ranges already aligned don't change + ASSERT_EQ( + MemoryTagManagerAArch64MTE::TagRange(0x100, 64), + manager.ExpandToGranule(MemoryTagManagerAArch64MTE::TagRange(0x100, 64))); + + // Any read of less than 1 granule is rounded up to reading 1 granule + ASSERT_EQ( + MemoryTagManagerAArch64MTE::TagRange(0, 16), + manager.ExpandToGranule(MemoryTagManagerAArch64MTE::TagRange(0, 1))); + + // Start address is aligned down, and length modified accordingly + // Here bytes 8 through 24 straddle 2 granules. So the resulting range starts + // at 0 and covers 32 bytes. + ASSERT_EQ( + MemoryTagManagerAArch64MTE::TagRange(0, 32), + manager.ExpandToGranule(MemoryTagManagerAArch64MTE::TagRange(8, 16))); + + // Here only the size of the range needs aligning + ASSERT_EQ( + MemoryTagManagerAArch64MTE::TagRange(16, 32), + manager.ExpandToGranule(MemoryTagManagerAArch64MTE::TagRange(16, 24))); + + // Start and size need aligning here but we only need 1 granule to cover it + ASSERT_EQ( + MemoryTagManagerAArch64MTE::TagRange(16, 16), + manager.ExpandToGranule(MemoryTagManagerAArch64MTE::TagRange(18, 4))); +} + +TEST(MemoryTagManagerAArch64MTETest, RemoveNonAddressBits) { + MemoryTagManagerAArch64MTE manager; + + ASSERT_EQ(0, 0); + ASSERT_EQ((lldb::addr_t)0x00ffeedd11223344, +manager.RemoveNonAddressBits(0x00ffeedd11223344)); +
[Lldb-commits] [lldb] afee097 - [NFC] Add GetInferiorAddrSize method, unify code to compute
Author: Jason Molenda Date: 2021-05-13T00:47:58-07:00 New Revision: afee09751d2d744a753ef4d3e8d83857dcd0f682 URL: https://github.com/llvm/llvm-project/commit/afee09751d2d744a753ef4d3e8d83857dcd0f682 DIFF: https://github.com/llvm/llvm-project/commit/afee09751d2d744a753ef4d3e8d83857dcd0f682.diff LOG: [NFC] Add GetInferiorAddrSize method, unify code to compute MachProcess.mm has a sequence to get the address size in the inferior in three places; and I'm about to add a fourth in a future patch. Not a fan. Added: Modified: lldb/tools/debugserver/source/MacOSX/MachProcess.h lldb/tools/debugserver/source/MacOSX/MachProcess.mm Removed: diff --git a/lldb/tools/debugserver/source/MacOSX/MachProcess.h b/lldb/tools/debugserver/source/MacOSX/MachProcess.h index 282366d073887..b295dfecec41a 100644 --- a/lldb/tools/debugserver/source/MacOSX/MachProcess.h +++ b/lldb/tools/debugserver/source/MacOSX/MachProcess.h @@ -343,6 +343,9 @@ class MachProcess { bool ProcessUsingFrontBoard(); + // Size of addresses in the inferior process (4 or 8). + int GetInferiorAddrSize(pid_t pid); + Genealogy::ThreadActivitySP GetGenealogyInfoForThread(nub_thread_t tid, bool _out); diff --git a/lldb/tools/debugserver/source/MacOSX/MachProcess.mm b/lldb/tools/debugserver/source/MacOSX/MachProcess.mm index 921e4ffb8ae5e..0a6ef61617117 100644 --- a/lldb/tools/debugserver/source/MacOSX/MachProcess.mm +++ b/lldb/tools/debugserver/source/MacOSX/MachProcess.mm @@ -987,23 +987,15 @@ static bool FBSAddEventDataToOptions(NSMutableDictionary *options, nub_process_t pid, nub_addr_t image_list_address, nub_addr_t image_count) { JSONGenerator::DictionarySP reply_sp; - int mib[4] = {CTL_KERN, KERN_PROC, KERN_PROC_PID, pid}; - struct kinfo_proc processInfo; - size_t bufsize = sizeof(processInfo); - if (sysctl(mib, (unsigned)(sizeof(mib) / sizeof(int)), , , - NULL, 0) == 0 && - bufsize > 0) { -uint32_t pointer_size = 4; -if (processInfo.kp_proc.p_flag & P_LP64) - pointer_size = 8; + int pointer_size = GetInferiorAddrSize(pid); -std::vector image_infos; -size_t image_infos_size = image_count * 3 * pointer_size; + std::vector image_infos; + size_t image_infos_size = image_count * 3 * pointer_size; -uint8_t *image_info_buf = (uint8_t *)malloc(image_infos_size); -if (image_info_buf == NULL) { - return reply_sp; -} + uint8_t *image_info_buf = (uint8_t *)malloc(image_infos_size); + if (image_info_buf == NULL) { +return reply_sp; + } if (ReadMemory(image_list_address, image_infos_size, image_info_buf) != image_infos_size) { return reply_sp; @@ -1085,7 +1077,6 @@ static bool FBSAddEventDataToOptions(NSMutableDictionary *options, Third, format all of the above in the JSONGenerator object. return FormatDynamicLibrariesIntoJSON(image_infos); - } return reply_sp; } @@ -1146,28 +1137,16 @@ static bool FBSAddEventDataToOptions(NSMutableDictionary *options, MachProcess::GetAllLoadedLibrariesInfos(nub_process_t pid) { JSONGenerator::DictionarySP reply_sp; - int mib[4] = {CTL_KERN, KERN_PROC, KERN_PROC_PID, pid}; - struct kinfo_proc processInfo; - size_t bufsize = sizeof(processInfo); - if (sysctl(mib, (unsigned)(sizeof(mib) / sizeof(int)), , , - NULL, 0) == 0 && - bufsize > 0) { -uint32_t pointer_size = 4; -if (processInfo.kp_proc.p_flag & P_LP64) - pointer_size = 8; - -std::vector image_infos; -GetAllLoadedBinariesViaDYLDSPI(image_infos); -uint32_t platform = GetProcessPlatformViaDYLDSPI(); -const size_t image_count = image_infos.size(); -for (size_t i = 0; i < image_count; i++) { - GetMachOInformationFromMemory(platform, -image_infos[i].load_address, pointer_size, -image_infos[i].macho_info); -} -return FormatDynamicLibrariesIntoJSON(image_infos); + int pointer_size = GetInferiorAddrSize(pid); + std::vector image_infos; + GetAllLoadedBinariesViaDYLDSPI(image_infos); + uint32_t platform = GetProcessPlatformViaDYLDSPI(); + const size_t image_count = image_infos.size(); + for (size_t i = 0; i < image_count; i++) { +GetMachOInformationFromMemory(platform, image_infos[i].load_address, + pointer_size, image_infos[i].macho_info); } - return reply_sp; +return FormatDynamicLibrariesIntoJSON(image_infos); } // Fetch information about the shared libraries at the given load addresses @@ -1177,30 +1156,22 @@ static bool FBSAddEventDataToOptions(NSMutableDictionary *options, nub_process_t pid, std::vector _addresses) { JSONGenerator::DictionarySP reply_sp; - int mib[4] = {CTL_KERN, KERN_PROC, KERN_PROC_PID, pid}; - struct kinfo_proc processInfo; - size_t