llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-lldb Author: David Spickett (DavidSpickett) <details> <summary>Changes</summary> Reverts llvm/llvm-project#<!-- -->174348 due to reported failures on MacOS and Arm 32-bit Linux. --- Patch is 27.45 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/180944.diff 16 Files Affected: - (modified) lldb/include/lldb/Core/Architecture.h (-12) - (modified) lldb/include/lldb/Target/Platform.h (-29) - (modified) lldb/include/lldb/Target/StopInfo.h (-6) - (modified) lldb/source/Plugins/Architecture/AArch64/ArchitectureAArch64.cpp (-17) - (modified) lldb/source/Plugins/Architecture/AArch64/ArchitectureAArch64.h (-3) - (modified) lldb/source/Plugins/Architecture/Arm/ArchitectureArm.cpp (-30) - (modified) lldb/source/Plugins/Architecture/Arm/ArchitectureArm.h (-3) - (modified) lldb/source/Plugins/Architecture/Mips/ArchitectureMips.cpp (-11) - (modified) lldb/source/Plugins/Architecture/Mips/ArchitectureMips.h (-3) - (modified) lldb/source/Plugins/Process/Utility/StopInfoMachException.cpp (-7) - (modified) lldb/source/Plugins/Process/Utility/StopInfoMachException.h (-4) - (modified) lldb/source/Target/Platform.cpp (+61-87) - (modified) lldb/source/Target/StopInfo.cpp (-42) - (renamed) lldb/test/API/macosx/builtin-debugtrap/Makefile (+1-1) - (renamed) lldb/test/API/macosx/builtin-debugtrap/TestBuiltinDebugTrap.py (+10-15) - (renamed) lldb/test/API/macosx/builtin-debugtrap/main.cpp (+2-1) ``````````diff diff --git a/lldb/include/lldb/Core/Architecture.h b/lldb/include/lldb/Core/Architecture.h index f31f23347fb54..ed64a895717a1 100644 --- a/lldb/include/lldb/Core/Architecture.h +++ b/lldb/include/lldb/Core/Architecture.h @@ -138,18 +138,6 @@ class Architecture : public PluginInterface { std::shared_ptr<const UnwindPlan> current_unwindplan) { return lldb::UnwindPlanSP(); } - - /// Returns whether a given byte sequence is a valid trap instruction for the - /// architecture. Some architectures feature instructions that have immediates - /// that can take on any value, resulting in a family of valid byte sequences. - /// If the observed byte sequence is shorter than the reference then they are - /// considered not to match, even if the initial bytes would match. - virtual bool IsValidTrapInstruction(llvm::ArrayRef<uint8_t> reference, - llvm::ArrayRef<uint8_t> observed) const { - if (reference.size() > observed.size()) - return false; - return !std::memcmp(reference.data(), observed.data(), reference.size()); - } }; } // namespace lldb_private diff --git a/lldb/include/lldb/Target/Platform.h b/lldb/include/lldb/Target/Platform.h index 47642ba80eb92..637d4c37b90bc 100644 --- a/lldb/include/lldb/Target/Platform.h +++ b/lldb/include/lldb/Target/Platform.h @@ -330,35 +330,6 @@ class Platform : public PluginInterface { virtual std::vector<ArchSpec> GetSupportedArchitectures(const ArchSpec &process_host_arch) = 0; - /// Get the bytes of the platform's software interrupt instruction. If there - /// are multiple possible encodings, for example where there are immediate - /// values encoded in the instruction, this will return the instruction with - /// those bits set as 0. - /// - /// \param[in] arch - /// The architecture of the inferior. - /// \param size_hint - /// A hint to disambiguate which instruction is used on platforms where - /// there are multiple interrupts with different sizes in the ISA (e.g - /// ARM Thumb, RISC-V). - /// - /// \return - /// The bytes of the interrupt instruction, with any immediate value - /// bits set to 0. - llvm::ArrayRef<uint8_t> SoftwareTrapOpcodeBytes(const ArchSpec &arch, - size_t size_hint = 0); - - /// Get the suggested size hint for a trap instruction on the given target. - /// Some platforms have a compressed instruction set which can be used - /// instead of the "normal" encoding. This function attempts to determine - /// a size hint for the size of the instruction at address \a addr, and - /// return 0, 2 or 4, with 2 and 4 corresponding to the estimated size - /// and zero meaning no applicable hint. Returns the estimated size in bytes - /// of the instruction for this target at the given address, or 0 if no - /// estimate is available. - size_t GetTrapOpcodeSizeHint(Target &target, Address addr, - llvm::ArrayRef<uint8_t> bytes); - virtual size_t GetSoftwareBreakpointTrapOpcode(Target &target, BreakpointSite *bp_site); diff --git a/lldb/include/lldb/Target/StopInfo.h b/lldb/include/lldb/Target/StopInfo.h index fbbad7c91fba5..cdd6a6fbe6aa4 100644 --- a/lldb/include/lldb/Target/StopInfo.h +++ b/lldb/include/lldb/Target/StopInfo.h @@ -218,12 +218,6 @@ class StopInfo : public std::enable_shared_from_this<StopInfo> { // to consult this later on. virtual bool ShouldStop(Event *event_ptr) { return true; } - // Shared implementation for when a trap instruction leaves the CPU PC at - // the trap instruction, instead of just after it. Currently the subclasses - // StopInfoMachException and StopInfoUnixSignal use this to skip over the - // instruction at the PC if it is a matching trap instruction. - void SkipOverTrapInstruction(); - // Classes that inherit from StackID can see and modify these lldb::ThreadWP m_thread_wp; // The thread corresponding to the stop reason. uint32_t m_stop_id; // The process stop ID for which this stop info is valid diff --git a/lldb/source/Plugins/Architecture/AArch64/ArchitectureAArch64.cpp b/lldb/source/Plugins/Architecture/AArch64/ArchitectureAArch64.cpp index 29fb9ce65ab0b..6a072354972ac 100644 --- a/lldb/source/Plugins/Architecture/AArch64/ArchitectureAArch64.cpp +++ b/lldb/source/Plugins/Architecture/AArch64/ArchitectureAArch64.cpp @@ -13,8 +13,6 @@ #include "lldb/Utility/DataBufferHeap.h" #include "lldb/Utility/DataExtractor.h" -#include "llvm/Support/Endian.h" - using namespace lldb_private; using namespace lldb; @@ -143,18 +141,3 @@ bool ArchitectureAArch64::ReconfigureRegisterInfo(DynamicRegisterInfo ®_info, return true; } - -bool ArchitectureAArch64::IsValidTrapInstruction( - llvm::ArrayRef<uint8_t> reference, llvm::ArrayRef<uint8_t> observed) const { - if (reference.size() < 4 || observed.size() < 4) - return false; - auto ref_bytes = llvm::support::endian::read32le(reference.data()); - auto bytes = llvm::support::endian::read32le(observed.data()); - // Only the 11 highest bits define the breakpoint instruction, the others - // include an immediate value that we will explicitly check against. - uint32_t mask = 0xFFE00000; - // Check that the masked bytes match the reference, but also check that the - // immediate in the instruction is the default output by llvm.debugtrap. - // The reference has the immediate set as all-zero, so mask and check here. - return (ref_bytes == (bytes & mask)) && ((bytes & ~mask) >> 5 == 0xF000); -} diff --git a/lldb/source/Plugins/Architecture/AArch64/ArchitectureAArch64.h b/lldb/source/Plugins/Architecture/AArch64/ArchitectureAArch64.h index 3bf4b66e28e0a..ba409428c951e 100644 --- a/lldb/source/Plugins/Architecture/AArch64/ArchitectureAArch64.h +++ b/lldb/source/Plugins/Architecture/AArch64/ArchitectureAArch64.h @@ -39,9 +39,6 @@ class ArchitectureAArch64 : public Architecture { DataExtractor ®_data, RegisterContext ®_context) const override; - bool IsValidTrapInstruction(llvm::ArrayRef<uint8_t> reference, - llvm::ArrayRef<uint8_t> observed) const override; - private: static std::unique_ptr<Architecture> Create(const ArchSpec &arch); ArchitectureAArch64() = default; diff --git a/lldb/source/Plugins/Architecture/Arm/ArchitectureArm.cpp b/lldb/source/Plugins/Architecture/Arm/ArchitectureArm.cpp index 5e53b58f1472b..721c4bcfe6342 100644 --- a/lldb/source/Plugins/Architecture/Arm/ArchitectureArm.cpp +++ b/lldb/source/Plugins/Architecture/Arm/ArchitectureArm.cpp @@ -22,8 +22,6 @@ #include "lldb/Utility/Log.h" #include "lldb/Utility/RegisterValue.h" -#include "llvm/Support/Endian.h" - using namespace lldb_private; using namespace lldb; @@ -338,31 +336,3 @@ UnwindPlanSP ArchitectureArm::GetArchitectureUnwindPlan( } return new_plan; } - -bool ArchitectureArm::IsValidTrapInstruction( - llvm::ArrayRef<uint8_t> reference, llvm::ArrayRef<uint8_t> observed) const { - if (reference.size() > observed.size()) - return false; - - if (reference.size() == 2) { - auto ref_bytes = llvm::support::endian::read16le(reference.data()); - auto obs_bytes = llvm::support::endian::read16le(observed.data()); - if (ref_bytes == obs_bytes) - return true; - // LLDB uses an undef instruction encoding for breakpoints - - // perhaps we have an actual BKPT in the inferior. - uint16_t mask = 0xF0; - if ((obs_bytes & mask) == 0xBE00) - return true; - } else if (reference.size() == 4) { - auto ref_bytes = llvm::support::endian::read32le(reference.data()); - auto obs_bytes = llvm::support::endian::read32le(observed.data()); - if (ref_bytes == obs_bytes) - return true; - uint32_t mask = 0x0FF000F0; - uint32_t bkpt_pattern = 0xE1200070; - if ((obs_bytes & mask) == bkpt_pattern) - return true; - } - return false; -} diff --git a/lldb/source/Plugins/Architecture/Arm/ArchitectureArm.h b/lldb/source/Plugins/Architecture/Arm/ArchitectureArm.h index dfd7b7b651767..52277dc5dbae0 100644 --- a/lldb/source/Plugins/Architecture/Arm/ArchitectureArm.h +++ b/lldb/source/Plugins/Architecture/Arm/ArchitectureArm.h @@ -34,9 +34,6 @@ class ArchitectureArm : public Architecture { lldb_private::Thread &thread, lldb_private::RegisterContextUnwind *regctx, std::shared_ptr<const UnwindPlan> current_unwindplan) override; - bool IsValidTrapInstruction(llvm::ArrayRef<uint8_t> reference, - llvm::ArrayRef<uint8_t> observed) const override; - private: static std::unique_ptr<Architecture> Create(const ArchSpec &arch); ArchitectureArm() = default; diff --git a/lldb/source/Plugins/Architecture/Mips/ArchitectureMips.cpp b/lldb/source/Plugins/Architecture/Mips/ArchitectureMips.cpp index f4c7a19531ac3..3748be0533ad7 100644 --- a/lldb/source/Plugins/Architecture/Mips/ArchitectureMips.cpp +++ b/lldb/source/Plugins/Architecture/Mips/ArchitectureMips.cpp @@ -19,8 +19,6 @@ #include "lldb/Utility/LLDBLog.h" #include "lldb/Utility/Log.h" -#include "llvm/Support/Endian.h" - using namespace lldb_private; using namespace lldb; @@ -231,12 +229,3 @@ Instruction *ArchitectureMips::GetInstructionAtAddress( return nullptr; } - -bool ArchitectureMips::IsValidTrapInstruction( - llvm::ArrayRef<uint8_t> reference, llvm::ArrayRef<uint8_t> observed) const { - // The middle twenty bits of BREAK can be anything, so zero them - uint32_t mask = 0xFC00003F; - auto ref_bytes = llvm::support::endian::read32le(reference.data()); - auto bytes = llvm::support::endian::read32le(observed.data()); - return (ref_bytes & mask) == (bytes & mask); -} diff --git a/lldb/source/Plugins/Architecture/Mips/ArchitectureMips.h b/lldb/source/Plugins/Architecture/Mips/ArchitectureMips.h index d2475d21b8de6..cedd4127afcb6 100644 --- a/lldb/source/Plugins/Architecture/Mips/ArchitectureMips.h +++ b/lldb/source/Plugins/Architecture/Mips/ArchitectureMips.h @@ -33,9 +33,6 @@ class ArchitectureMips : public Architecture { lldb::addr_t GetOpcodeLoadAddress(lldb::addr_t load_addr, AddressClass addr_class) const override; - bool IsValidTrapInstruction(llvm::ArrayRef<uint8_t> reference, - llvm::ArrayRef<uint8_t> observed) const override; - private: Instruction *GetInstructionAtAddress(Target &target, const Address &resolved_addr, diff --git a/lldb/source/Plugins/Process/Utility/StopInfoMachException.cpp b/lldb/source/Plugins/Process/Utility/StopInfoMachException.cpp index f523dc80248a0..d374742b5722c 100644 --- a/lldb/source/Plugins/Process/Utility/StopInfoMachException.cpp +++ b/lldb/source/Plugins/Process/Utility/StopInfoMachException.cpp @@ -827,13 +827,6 @@ StopInfoSP StopInfoMachException::CreateStopReasonWithMachException( not_stepping_but_got_singlestep_exception); } -void StopInfoMachException::PerformAction([[maybe_unused]] Event *event_ptr) { - if (!(m_value == 6 /*EXC_BREAKPOINT*/ && - m_exc_code == 1 /*EXC_ARM_BREAKPOINT*/)) - return; - SkipOverTrapInstruction(); -} - // Detect an unusual situation on Darwin where: // // 0. We did an instruction-step before this. diff --git a/lldb/source/Plugins/Process/Utility/StopInfoMachException.h b/lldb/source/Plugins/Process/Utility/StopInfoMachException.h index bac5e9777289f..147d53ad31e7e 100644 --- a/lldb/source/Plugins/Process/Utility/StopInfoMachException.h +++ b/lldb/source/Plugins/Process/Utility/StopInfoMachException.h @@ -86,10 +86,6 @@ class StopInfoMachException : public StopInfo { }; #endif - /// Allow this plugin to respond to stop events to enable skip-over-trap - /// behaviour on AArch64. - void PerformAction(Event *event_ptr) override; - // Since some mach exceptions will be reported as breakpoints, signals, // or trace, we use this static accessor which will translate the mach // exception into the correct StopInfo. diff --git a/lldb/source/Target/Platform.cpp b/lldb/source/Target/Platform.cpp index 8047d0b3ad146..4068210cc1632 100644 --- a/lldb/source/Target/Platform.cpp +++ b/lldb/source/Target/Platform.cpp @@ -39,7 +39,6 @@ #include "lldb/Utility/Log.h" #include "lldb/Utility/Status.h" #include "lldb/Utility/StructuredData.h" -#include "llvm/ADT/ArrayRef.h" #include "llvm/ADT/STLExtras.h" #include "llvm/Support/FileSystem.h" #include "llvm/Support/Path.h" @@ -1947,99 +1946,121 @@ size_t Platform::ConnectToWaitingProcesses(lldb_private::Debugger &debugger, return 0; } -llvm::ArrayRef<uint8_t> Platform::SoftwareTrapOpcodeBytes(const ArchSpec &arch, - size_t size_hint) { - llvm::ArrayRef<uint8_t> trap_opcode; +size_t Platform::GetSoftwareBreakpointTrapOpcode(Target &target, + BreakpointSite *bp_site) { + ArchSpec arch = target.GetArchitecture(); + assert(arch.IsValid()); + const uint8_t *trap_opcode = nullptr; + size_t trap_opcode_size = 0; switch (arch.GetMachine()) { case llvm::Triple::aarch64_32: case llvm::Triple::aarch64: { static const uint8_t g_aarch64_opcode[] = {0x00, 0x00, 0x20, 0xd4}; - trap_opcode = - llvm::ArrayRef<uint8_t>(g_aarch64_opcode, sizeof(g_aarch64_opcode)); + trap_opcode = g_aarch64_opcode; + trap_opcode_size = sizeof(g_aarch64_opcode); } break; case llvm::Triple::arc: { - static const uint8_t g_hex_opcode[] = {0xff, 0x7f}; - trap_opcode = llvm::ArrayRef<uint8_t>(g_hex_opcode, sizeof(g_hex_opcode)); + static const uint8_t g_hex_opcode[] = { 0xff, 0x7f }; + trap_opcode = g_hex_opcode; + trap_opcode_size = sizeof(g_hex_opcode); } break; + // TODO: support big-endian arm and thumb trap codes. case llvm::Triple::arm: { - // ARM CPUs have dedicated BKPT instructions: 0xe7fddefe and 0xdefe. - // However, the linux kernel recognizes two different sequences based on - // undefined instruction encodings (linux/arch/arm/kernel/ptrace.c) + // The ARM reference recommends the use of 0xe7fddefe and 0xdefe but the + // linux kernel does otherwise. static const uint8_t g_arm_breakpoint_opcode[] = {0xf0, 0x01, 0xf0, 0xe7}; static const uint8_t g_thumb_breakpoint_opcode[] = {0x01, 0xde}; - if (size_hint == 2) { - trap_opcode = llvm::ArrayRef<uint8_t>(g_thumb_breakpoint_opcode, - sizeof(g_thumb_breakpoint_opcode)); + lldb::BreakpointLocationSP bp_loc_sp(bp_site->GetConstituentAtIndex(0)); + AddressClass addr_class = AddressClass::eUnknown; + + if (bp_loc_sp) { + addr_class = bp_loc_sp->GetAddress().GetAddressClass(); + if (addr_class == AddressClass::eUnknown && + (bp_loc_sp->GetAddress().GetFileAddress() & 1)) + addr_class = AddressClass::eCodeAlternateISA; + } + + if (addr_class == AddressClass::eCodeAlternateISA) { + trap_opcode = g_thumb_breakpoint_opcode; + trap_opcode_size = sizeof(g_thumb_breakpoint_opcode); } else { - trap_opcode = llvm::ArrayRef<uint8_t>(g_arm_breakpoint_opcode, - sizeof(g_arm_breakpoint_opcode)); + trap_opcode = g_arm_breakpoint_opcode; + trap_opcode_size = sizeof(g_arm_breakpoint_opcode); } } break; case llvm::Triple::avr: { static const uint8_t g_hex_opcode[] = {0x98, 0x95}; - trap_opcode = llvm::ArrayRef<uint8_t>(g_hex_opcode, sizeof(g_hex_opcode)); + trap_opcode = g_hex_opcode; + trap_opcode_size = sizeof(g_hex_opcode); } break; case llvm::Triple::mips: case llvm::Triple::mips64: { static const uint8_t g_hex_opcode[] = {0x00, 0x00, 0x00, 0x0d}; - trap_opcode = llvm::ArrayRef<uint8_t>(g_hex_opcode, sizeof(g_hex_opcode)); + trap_opcode = g_hex_opcode; + trap_opcode_size = sizeof(g_hex_opcode); } break; case llvm::Triple::mipsel: case llvm::Triple::mips64el: { static const uint8_t g_hex_opcode[] = {0x0d, 0x00, 0x00, 0x00}; - trap_opcode = llvm::ArrayRef<uint8_t>(g_hex_opcode, sizeof(g_hex_opcode)); + trap_opcode = g_hex_opcode; + trap_opcode_size = sizeof(g_hex_opcode); } break; case llvm::Triple::msp430: { static const uint8_t g_msp430_opcode[] = {0x43, 0x43}; - trap_opcode = - llvm::ArrayRef<uint8_t>(g_msp430_opcode, sizeof(g_msp430_opcode)); + trap_opcode = g_msp430_opcode; + trap_opcode_size = sizeof(g_msp430_opcode); } break; case llvm::Triple::systemz: { static const uint8_t g_hex_opcode[] = {0x00, 0x01}; - trap_opcode = llvm::ArrayRef<uint8_t>(g_hex_opcode, sizeof(g_hex_opcode)); + trap_opcode = g_hex_opcode; + trap_opcode_size = sizeof(g_hex_opcode); } break; case llvm::Triple::hexagon: { static const uint8_t g_hex_opcode[] = {0x0c, 0xdb, 0x00, 0x54}; - trap_opcode = llvm::ArrayRef<uint8_t>(g_hex_opcode, sizeof(g_hex_opcode)); + trap_opcode = g_hex_opcode; + trap_opcode_size = sizeof(g_hex_opcode); } break; case llvm::Triple::ppc: case llvm::Triple::ppc64: { static const uint8_t g_ppc_opcode[] = {0x7f, 0xe0, 0x00, 0x08}; - trap_opcode = llvm::ArrayRef<uint8_t>(g_ppc_opcode, sizeof(g_ppc_opcode)); + trap_opcode = g_ppc_opcode; + trap_opcode_size = sizeof(g_ppc_opcode); } break; case llvm::Triple::ppc64le: { static const uint8_t g_ppc64le_opcode[] = {0x08, 0x00, 0xe0, 0x7f}; // trap - trap_opcode = - llvm::ArrayRef<uint8_t>(g_ppc64le_opcode, sizeof(g_ppc64le_opcode)); + trap_opcode = g_ppc64le_opcode; + trap_opcode_size = sizeof(g_ppc64le_opcode); } break; case llvm::Triple::x86: case llvm::Triple::x86_64: { static const uint8_t g_i386_opcode[] = {0xCC}; - trap_opcode = llvm::ArrayRef<uint8_t>(g_i386_opcode, sizeof(g_i386_opcode)); + trap_opcode = g_i386_opcode; + trap_opcode_size = sizeof(g_i386_opcode); } break; case llvm::Triple::riscv32: case llvm::Triple::riscv64: { static const uint8_t g_riscv_opcode[] = {0x73, 0x00, 0x10, 0x00}; // ebreak static const uint8_t g_riscv_opcode_c[] = {0x02, 0x90}; // c.ebreak - if (size_hint == 2) { + if (arch.GetFlags() & ArchSpec::eRISCV_rvc) { trap_opcode = g_riscv_opcode_c; + trap_opcode_size = sizeof(g_riscv_opcode_c); } else { - trap_opcode = - llvm::ArrayRef<uint8_t>(g_riscv_opcode, sizeof(g_riscv_opcode)); + trap_opcode = g_riscv_opcode; + trap_opcode_size = sizeof(g_riscv_opcode); } } break; @@ -2047,71 +2068,24 @@ llvm::ArrayRef<uint8_t> Platform::SoftwareTrapOpcodeBytes(const ArchSpec &arch, case llvm::Triple::loongarch64: { static const uint8_t g_loongarch_opcode[] = {0x05, 0x00, 0x2a, 0x00}; // break 0x5 - trap_opcode = - llvm::ArrayRef<uint8_t>(g_loongarch_opcode, sizeof(g_loongarch_opcode)); + trap_opcode = g_loongarch_opcode; + trap_opcode_size = sizeof(g_loongarch_opcode); } break; - // Unreachable (0x00) triggers an unconditional trap. case llvm::Triple::wasm32: { + // Unreachable (0x00) triggers an unconditional trap. static const uint8_t g_wasm_opcode[] = {0x00}; - trap_opcode = llvm::ArrayRef<uint8_t>(g_wasm_opcode, sizeof(g_wasm_opcode)); + trap_opcode = g_wasm_opcode; + trap_opcode_size = sizeof(g_wasm_opcode); } break; - // The default case should not match against anything, so return empty Array. - default: { - trap_opcode = llvm::ArrayRef<uint8_t>{}; - }; - } - return trap_opcode; -} - -size_t Platform::GetTrapOpcodeSizeHint(Target &target, Address addr, - llvm::ArrayRef<uint8_t> bytes) { - ArchSpec arch = target.GetArchitecture(); - assert(arch.IsValid()); - const auto &triple = arch.GetTriple(); - - if (bytes.size() && triple.isRISCV()) { - // RISC-V instructions have the two LSB as 0b11 if they are four-byte. - return (bytes[0] & 0b11) == 0b11 ? 4 : 2; - } - if (triple.isARM()) { - if (auto addr_class = addr.GetAddressClass(); - addr_class == AddressClass::eCodeAlternateISA) { - return 2; - } else { - return 4; - } - } - return 0; -} - -size_t Platform::GetSoftwareBreakpointTrapOpcode(Target &target, - Breakpoi... [truncated] `````````` </details> https://github.com/llvm/llvm-project/pull/180944 _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
