Index: lldb/include/lldb/Core/ArchSpec.h =================================================================== --- lldb/include/lldb/Core/ArchSpec.h +++ lldb/include/lldb/Core/ArchSpec.h @@ -14,6 +14,7 @@ #include "lldb/Utility/ConstString.h" #include "lldb/lldb-forward.h" +#include "llvm/ADT/StringSet.h" #include "llvm/ADT/Triple.h" namespace lldb_private { @@ -239,8 +240,6 @@ }; - typedef void (*StopInfoOverrideCallbackType)(lldb_private::Thread &thread); - //------------------------------------------------------------------ /// Default constructor. /// @@ -283,7 +282,7 @@ //------------------------------------------------------------------ const ArchSpec &operator=(const ArchSpec &rhs); - static size_t AutoComplete(llvm::StringRef name, StringList &matches); + static size_t AutoComplete(llvm::StringRef name, llvm::StringSet<> &matches); //------------------------------------------------------------------ /// Returns a static string representing the current architecture. @@ -555,29 +554,6 @@ //------------------------------------------------------------------ bool IsCompatibleMatch(const ArchSpec &rhs) const; - //------------------------------------------------------------------ - /// Get a stop info override callback for the current architecture. - /// - /// Most platform specific code should go in lldb_private::Platform, - /// but there are cases where no matter which platform you are on - /// certain things hold true. - /// - /// This callback is currently intended to handle cases where a - /// program stops at an instruction that won't get executed and it - /// allows the stop reasonm, like "breakpoint hit", to be replaced - /// with a different stop reason like "no stop reason". - /// - /// This is specifically used for ARM in Thumb code when we stop in - /// an IT instruction (if/then/else) where the instruction won't get - /// executed and therefore it wouldn't be correct to show the program - /// stopped at the current PC. The code is generic and applies to all - /// ARM CPUs. - /// - /// @return NULL or a valid stop info override callback for the - /// current architecture. - //------------------------------------------------------------------ - StopInfoOverrideCallbackType GetStopInfoOverrideCallback() const; - bool IsFullySpecifiedTriple() const; void PiecewiseTripleCompare(const ArchSpec &other, bool &arch_different, Index: lldb/include/lldb/Target/Process.h =================================================================== --- lldb/include/lldb/Target/Process.h +++ lldb/include/lldb/Target/Process.h @@ -2512,9 +2512,29 @@ OperatingSystem *GetOperatingSystem() { return m_os_ap.get(); } - ArchSpec::StopInfoOverrideCallbackType GetStopInfoOverrideCallback() const { - return m_stop_info_override_callback; - } + //------------------------------------------------------------------ + /// Get a stop info override callback for the current architecture. + /// + /// Most platform specific code should go in lldb_private::Platform, + /// but there are cases where no matter which platform you are on + /// certain things hold true. + /// + /// This callback is currently intended to handle cases where a + /// program stops at an instruction that won't get executed and it + /// allows the stop reasonm, like "breakpoint hit", to be replaced + /// with a different stop reason like "no stop reason". + /// + /// This is specifically used for ARM in Thumb code when we stop in + /// an IT instruction (if/then/else) where the instruction won't get + /// executed and therefore it wouldn't be correct to show the program + /// stopped at the current PC. The code is generic and applies to all + /// ARM CPUs. + /// + /// @return NULL or a valid stop info override callback for the + /// current architecture. + //------------------------------------------------------------------ + typedef void (*StopInfoOverrideCallbackType)(lldb_private::Thread &); + void InvokeStopInfoOverrideCallback(lldb_private::Thread &Thread); virtual LanguageRuntime *GetLanguageRuntime(lldb::LanguageType language, bool retry_if_null = true); @@ -3036,7 +3056,7 @@ std::vector m_pre_resume_actions; ProcessRunLock m_public_run_lock; ProcessRunLock m_private_run_lock; - ArchSpec::StopInfoOverrideCallbackType m_stop_info_override_callback; + StopInfoOverrideCallbackType m_stop_info_override_callback; bool m_currently_handling_do_on_removals; bool m_resume_requested; // If m_currently_handling_event or // m_currently_handling_do_on_removals are true, @@ -3081,6 +3101,8 @@ void ResumePrivateStateThread(); private: + void SetStopInfoOverrideCallback(const ArchSpec &Arch); + struct PrivateStateThreadArgs { PrivateStateThreadArgs(Process *p, bool s) : process(p), is_secondary_thread(s){}; Index: lldb/source/Core/ArchSpec.cpp =================================================================== --- lldb/source/Core/ArchSpec.cpp +++ lldb/source/Core/ArchSpec.cpp @@ -22,14 +22,8 @@ #include "llvm/Support/Host.h" // Project includes -#include "Plugins/Process/Utility/ARMDefines.h" -#include "Plugins/Process/Utility/InstructionUtils.h" -#include "lldb/Core/StringList.h" #include "lldb/Host/HostInfo.h" #include "lldb/Target/Platform.h" -#include "lldb/Target/Process.h" -#include "lldb/Target/RegisterContext.h" -#include "lldb/Target/Thread.h" #include "lldb/Utility/Endian.h" #include "lldb/Utility/NameMatches.h" #include "lldb/Utility/RegularExpression.h" @@ -256,17 +250,18 @@ const char *name; }; -size_t ArchSpec::AutoComplete(llvm::StringRef name, StringList &matches) { +size_t ArchSpec::AutoComplete(llvm::StringRef name, + llvm::StringSet<> &matches) { if (!name.empty()) { for (uint32_t i = 0; i < llvm::array_lengthof(g_core_definitions); ++i) { if (NameMatches(g_core_definitions[i].name, NameMatch::StartsWith, name)) - matches.AppendString(g_core_definitions[i].name); + matches.insert(g_core_definitions[i].name); } } else { for (uint32_t i = 0; i < llvm::array_lengthof(g_core_definitions); ++i) - matches.AppendString(g_core_definitions[i].name); + matches.insert(g_core_definitions[i].name); } - return matches.GetSize(); + return matches.size(); } #define CPU_ANY (UINT32_MAX) @@ -1499,102 +1494,6 @@ return lhs_core < rhs_core; } -static void StopInfoOverrideCallbackTypeARM(lldb_private::Thread &thread) { - // We need to check if we are stopped in Thumb mode in a IT instruction - // and detect if the condition doesn't pass. If this is the case it means - // we won't actually execute this instruction. If this happens we need to - // clear the stop reason to no thread plans think we are stopped for a - // reason and the plans should keep going. - // - // We do this because when single stepping many ARM processes, debuggers - // often use the BVR/BCR registers that says "stop when the PC is not - // equal to its current value". This method of stepping means we can end - // up stopping on instructions inside an if/then block that wouldn't get - // executed. By fixing this we can stop the debugger from seeming like - // you stepped through both the "if" _and_ the "else" clause when source - // level stepping because the debugger stops regardless due to the BVR/BCR - // triggering a stop. - // - // It also means we can set breakpoints on instructions inside an an - // if/then block and correctly skip them if we use the BKPT instruction. - // The ARM and Thumb BKPT instructions are unconditional even when executed - // in a Thumb IT block. - // - // If your debugger inserts software traps in ARM/Thumb code, it will - // need to use 16 and 32 bit instruction for 16 and 32 bit thumb - // instructions respectively. If your debugger inserts a 16 bit thumb - // trap on top of a 32 bit thumb instruction for an opcode that is inside - // an if/then, it will change the it/then to conditionally execute your - // 16 bit trap and then cause your program to crash if it executes the - // trailing 16 bits (the second half of the 32 bit thumb instruction you - // partially overwrote). - - RegisterContextSP reg_ctx_sp(thread.GetRegisterContext()); - if (reg_ctx_sp) { - const uint32_t cpsr = reg_ctx_sp->GetFlags(0); - if (cpsr != 0) { - // Read the J and T bits to get the ISETSTATE - const uint32_t J = Bit32(cpsr, 24); - const uint32_t T = Bit32(cpsr, 5); - const uint32_t ISETSTATE = J << 1 | T; - if (ISETSTATE == 0) { -// NOTE: I am pretty sure we want to enable the code below -// that detects when we stop on an instruction in ARM mode -// that is conditional and the condition doesn't pass. This -// can happen if you set a breakpoint on an instruction that -// is conditional. We currently will _always_ stop on the -// instruction which is bad. You can also run into this while -// single stepping and you could appear to run code in the "if" -// and in the "else" clause because it would stop at all of the -// conditional instructions in both. -// In such cases, we really don't want to stop at this location. -// I will check with the lldb-dev list first before I enable this. -#if 0 - // ARM mode: check for condition on intsruction - const addr_t pc = reg_ctx_sp->GetPC(); - Error error; - // If we fail to read the opcode we will get UINT64_MAX as the - // result in "opcode" which we can use to detect if we read a - // valid opcode. - const uint64_t opcode = thread.GetProcess()->ReadUnsignedIntegerFromMemory(pc, 4, UINT64_MAX, error); - if (opcode <= UINT32_MAX) - { - const uint32_t condition = Bits32((uint32_t)opcode, 31, 28); - if (!ARMConditionPassed(condition, cpsr)) - { - // We ARE stopped on an ARM instruction whose condition doesn't - // pass so this instruction won't get executed. - // Regardless of why it stopped, we need to clear the stop info - thread.SetStopInfo (StopInfoSP()); - } - } -#endif - } else if (ISETSTATE == 1) { - // Thumb mode - const uint32_t ITSTATE = - Bits32(cpsr, 15, 10) << 2 | Bits32(cpsr, 26, 25); - if (ITSTATE != 0) { - const uint32_t condition = Bits32(ITSTATE, 7, 4); - if (!ARMConditionPassed(condition, cpsr)) { - // We ARE stopped in a Thumb IT instruction on an instruction whose - // condition doesn't pass so this instruction won't get executed. - // Regardless of why it stopped, we need to clear the stop info - thread.SetStopInfo(StopInfoSP()); - } - } - } - } - } -} - -ArchSpec::StopInfoOverrideCallbackType -ArchSpec::GetStopInfoOverrideCallback() const { - const llvm::Triple::ArchType machine = GetMachine(); - if (machine == llvm::Triple::arm) - return StopInfoOverrideCallbackTypeARM; - return nullptr; -} - bool ArchSpec::IsFullySpecifiedTriple() const { const auto &user_specified_triple = GetTriple(); Index: lldb/source/Interpreter/CommandObject.cpp =================================================================== --- lldb/source/Interpreter/CommandObject.cpp +++ lldb/source/Interpreter/CommandObject.cpp @@ -33,6 +33,9 @@ #include "lldb/Interpreter/CommandInterpreter.h" #include "lldb/Interpreter/CommandReturnObject.h" +#include "llvm/ADT/StringExtras.h" +#include "llvm/ADT/StringSet.h" + using namespace lldb; using namespace lldb_private; @@ -1017,15 +1020,19 @@ return handled; } +static std::string computeArchHelp() { + llvm::StringSet<> Archs; + ArchSpec::AutoComplete("", Archs); + std::string Result; + llvm::raw_string_ostream OS(Result); + OS << "These are the supported architecture names:\n"; + OS << llvm::join_range(Archs.keys(), "\n"); + return OS.str(); +} + static llvm::StringRef arch_helper() { - static StreamString g_archs_help; - if (g_archs_help.Empty()) { - StringList archs; - ArchSpec::AutoComplete(llvm::StringRef(), archs); - g_archs_help.Printf("These are the supported architecture names:\n"); - archs.Join("\n", g_archs_help); - } - return g_archs_help.GetString(); + static std::string Help = computeArchHelp(); + return Help; } CommandObject::ArgumentTableEntry CommandObject::g_arguments_data[] = { Index: lldb/source/Target/Process.cpp =================================================================== --- lldb/source/Target/Process.cpp +++ lldb/source/Target/Process.cpp @@ -17,7 +17,9 @@ #include "llvm/Support/Threading.h" // Project includes +#include "Plugins/Process/Utility/ARMDefines.h" #include "Plugins/Process/Utility/InferiorCallPOSIX.h" +#include "Plugins/Process/Utility/InstructionUtils.h" #include "lldb/Breakpoint/BreakpointLocation.h" #include "lldb/Breakpoint/StoppointCallbackContext.h" #include "lldb/Core/Debugger.h" @@ -2397,6 +2399,104 @@ return bytes_written; } +static void StopInfoOverrideCallbackTypeARM(lldb_private::Thread &thread) { + // We need to check if we are stopped in Thumb mode in a IT instruction + // and detect if the condition doesn't pass. If this is the case it means + // we won't actually execute this instruction. If this happens we need to + // clear the stop reason to no thread plans think we are stopped for a + // reason and the plans should keep going. + // + // We do this because when single stepping many ARM processes, debuggers + // often use the BVR/BCR registers that says "stop when the PC is not + // equal to its current value". This method of stepping means we can end + // up stopping on instructions inside an if/then block that wouldn't get + // executed. By fixing this we can stop the debugger from seeming like + // you stepped through both the "if" _and_ the "else" clause when source + // level stepping because the debugger stops regardless due to the BVR/BCR + // triggering a stop. + // + // It also means we can set breakpoints on instructions inside an an + // if/then block and correctly skip them if we use the BKPT instruction. + // The ARM and Thumb BKPT instructions are unconditional even when executed + // in a Thumb IT block. + // + // If your debugger inserts software traps in ARM/Thumb code, it will + // need to use 16 and 32 bit instruction for 16 and 32 bit thumb + // instructions respectively. If your debugger inserts a 16 bit thumb + // trap on top of a 32 bit thumb instruction for an opcode that is inside + // an if/then, it will change the it/then to conditionally execute your + // 16 bit trap and then cause your program to crash if it executes the + // trailing 16 bits (the second half of the 32 bit thumb instruction you + // partially overwrote). + + RegisterContextSP reg_ctx_sp(thread.GetRegisterContext()); + if (reg_ctx_sp) { + const uint32_t cpsr = reg_ctx_sp->GetFlags(0); + if (cpsr != 0) { + // Read the J and T bits to get the ISETSTATE + const uint32_t J = Bit32(cpsr, 24); + const uint32_t T = Bit32(cpsr, 5); + const uint32_t ISETSTATE = J << 1 | T; + if (ISETSTATE == 0) { +// NOTE: I am pretty sure we want to enable the code below +// that detects when we stop on an instruction in ARM mode +// that is conditional and the condition doesn't pass. This +// can happen if you set a breakpoint on an instruction that +// is conditional. We currently will _always_ stop on the +// instruction which is bad. You can also run into this while +// single stepping and you could appear to run code in the "if" +// and in the "else" clause because it would stop at all of the +// conditional instructions in both. +// In such cases, we really don't want to stop at this location. +// I will check with the lldb-dev list first before I enable this. +#if 0 + // ARM mode: check for condition on intsruction + const addr_t pc = reg_ctx_sp->GetPC(); + Error error; + // If we fail to read the opcode we will get UINT64_MAX as the + // result in "opcode" which we can use to detect if we read a + // valid opcode. + const uint64_t opcode = thread.GetProcess()->ReadUnsignedIntegerFromMemory(pc, 4, UINT64_MAX, error); + if (opcode <= UINT32_MAX) + { + const uint32_t condition = Bits32((uint32_t)opcode, 31, 28); + if (!ARMConditionPassed(condition, cpsr)) + { + // We ARE stopped on an ARM instruction whose condition doesn't + // pass so this instruction won't get executed. + // Regardless of why it stopped, we need to clear the stop info + thread.SetStopInfo(StopInfoSP()); + } + } +#endif + } else if (ISETSTATE == 1) { + // Thumb mode + const uint32_t ITSTATE = + Bits32(cpsr, 15, 10) << 2 | Bits32(cpsr, 26, 25); + if (ITSTATE != 0) { + const uint32_t condition = Bits32(ITSTATE, 7, 4); + if (!ARMConditionPassed(condition, cpsr)) { + // We ARE stopped in a Thumb IT instruction on an instruction whose + // condition doesn't pass so this instruction won't get executed. + // Regardless of why it stopped, we need to clear the stop info + thread.SetStopInfo(StopInfoSP()); + } + } + } + } + } +} + +void Process::SetStopInfoOverrideCallback(const ArchSpec &Arch) { + if (Arch.GetMachine() == llvm::Triple::arm) + m_stop_info_override_callback = StopInfoOverrideCallbackTypeARM; +} + +void Process::InvokeStopInfoOverrideCallback(lldb_private::Thread &Thread) { + if (m_stop_info_override_callback) + m_stop_info_override_callback(Thread); +} + size_t Process::WriteMemory(addr_t addr, const void *buf, size_t size, Error &error) { #if defined(ENABLE_MEMORY_CACHING) @@ -2797,8 +2897,7 @@ else StartPrivateStateThread(); - m_stop_info_override_callback = - GetTarget().GetArchitecture().GetStopInfoOverrideCallback(); + SetStopInfoOverrideCallback(GetTarget().GetArchitecture()); // Target was stopped at entry as was intended. Need to notify the // listeners @@ -3217,7 +3316,7 @@ } } - m_stop_info_override_callback = process_arch.GetStopInfoOverrideCallback(); + SetStopInfoOverrideCallback(process_arch); } Error Process::ConnectRemote(Stream *strm, llvm::StringRef remote_url) { Index: lldb/source/Target/Thread.cpp =================================================================== --- lldb/source/Target/Thread.cpp +++ lldb/source/Target/Thread.cpp @@ -441,10 +441,7 @@ if (m_stop_info_override_stop_id != process_stop_id) { m_stop_info_override_stop_id = process_stop_id; if (m_stop_info_sp) { - ArchSpec::StopInfoOverrideCallbackType callback = - GetProcess()->GetStopInfoOverrideCallback(); - if (callback) - callback(*this); + GetProcess()->InvokeStopInfoOverrideCallback(*this); } } }