Index: include/lldb/Core/ArchSpec.h =================================================================== --- include/lldb/Core/ArchSpec.h +++ include/lldb/Core/ArchSpec.h @@ -15,6 +15,7 @@ #include "lldb/Utility/ConstString.h" #include "lldb/lldb-enumerations.h" #include "lldb/lldb-private-enumerations.h" +#include "lldb/lldb-forward.h" #include "llvm/ADT/StringRef.h" // for StringRef #include "llvm/ADT/Triple.h" @@ -23,19 +24,6 @@ #include // for size_t #include // for uint32_t -namespace lldb_private { -class Platform; -} -namespace lldb_private { -class Stream; -} -namespace lldb_private { -class StringList; -} -namespace lldb_private { -class Thread; -} - namespace lldb_private { //---------------------------------------------------------------------- @@ -257,8 +245,6 @@ }; - typedef void (*StopInfoOverrideCallbackType)(lldb_private::Thread &thread); - //------------------------------------------------------------------ /// Default constructor. /// @@ -573,34 +559,11 @@ //------------------------------------------------------------------ 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, bool &vendor_different, bool &os_different, - bool &os_version_different, bool &env_different); + bool &os_version_different, bool &env_different) const; //------------------------------------------------------------------ /// Detect whether this architecture uses thumb code exclusively Index: include/lldb/Core/Architecture.h =================================================================== --- /dev/null +++ include/lldb/Core/Architecture.h @@ -0,0 +1,43 @@ +//===-- Architecture.h ------------------------------------------*- C++ -*-===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// + +#ifndef LLDB_CORE_ARCHITECTURE_H +#define LLDB_CORE_ARCHITECTURE_H + +#include "lldb/Core/PluginInterface.h" + +namespace lldb_private { + +class Architecture : public PluginInterface { +public: + Architecture() = default; + virtual ~Architecture() = default; + + //------------------------------------------------------------------ + /// This 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. + //------------------------------------------------------------------ + virtual void OverrideStopInfo(Thread &thread) = 0; + +private: + Architecture(const Architecture &) = delete; + void operator=(const Architecture &) = delete; +}; + +} // namespace lldb_private + +#endif // LLDB_CORE_ARCHITECTURE_H Index: include/lldb/Core/PluginManager.h =================================================================== --- include/lldb/Core/PluginManager.h +++ include/lldb/Core/PluginManager.h @@ -10,6 +10,7 @@ #ifndef liblldb_PluginManager_h_ #define liblldb_PluginManager_h_ +#include "lldb/Core/Architecture.h" #include "lldb/Utility/FileSpec.h" #include "lldb/Utility/Status.h" // for Status #include "lldb/lldb-enumerations.h" // for ScriptLanguage @@ -53,6 +54,21 @@ static ABICreateInstance GetABICreateCallbackForPluginName(const ConstString &name); + //------------------------------------------------------------------ + // Architecture + //------------------------------------------------------------------ + using ArchitectureCreateInstance = + std::unique_ptr (*)(const ArchSpec &); + + static void RegisterPlugin(const ConstString &name, + llvm::StringRef description, + ArchitectureCreateInstance create_callback); + + static void UnregisterPlugin(ArchitectureCreateInstance create_callback); + + static std::unique_ptr + CreateArchitectureInstance(const ArchSpec &arch); + //------------------------------------------------------------------ // Disassembler //------------------------------------------------------------------ Index: include/lldb/Target/Process.h =================================================================== --- include/lldb/Target/Process.h +++ include/lldb/Target/Process.h @@ -2514,10 +2514,6 @@ OperatingSystem *GetOperatingSystem() { return m_os_ap.get(); } - ArchSpec::StopInfoOverrideCallbackType GetStopInfoOverrideCallback() const { - return m_stop_info_override_callback; - } - virtual LanguageRuntime *GetLanguageRuntime(lldb::LanguageType language, bool retry_if_null = true); @@ -3106,7 +3102,6 @@ std::vector m_pre_resume_actions; ProcessRunLock m_public_run_lock; ProcessRunLock m_private_run_lock; - ArchSpec::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, Index: include/lldb/Target/Target.h =================================================================== --- include/lldb/Target/Target.h +++ include/lldb/Target/Target.h @@ -23,6 +23,7 @@ #include "lldb/Breakpoint/BreakpointList.h" #include "lldb/Breakpoint/WatchpointList.h" #include "lldb/Core/ArchSpec.h" +#include "lldb/Core/Architecture.h" #include "lldb/Core/Broadcaster.h" #include "lldb/Core/Disassembler.h" #include "lldb/Core/ModuleList.h" @@ -881,7 +882,7 @@ bool ModuleIsExcludedForUnconstrainedSearches(const lldb::ModuleSP &module_sp); - const ArchSpec &GetArchitecture() const { return m_arch; } + const ArchSpec &GetArchitecture() const { return m_arch.GetSpec(); } //------------------------------------------------------------------ /// Set the architecture for this target. @@ -912,6 +913,8 @@ bool MergeArchitecture(const ArchSpec &arch_spec); + Architecture *GetArchitecturePlugin() { return m_arch.GetPlugin(); } + Debugger &GetDebugger() { return m_debugger; } size_t ReadMemoryFromFileCache(const Address &addr, void *dst, size_t dst_len, @@ -1205,6 +1208,18 @@ const lldb::ModuleSP &new_module_sp) override; void WillClearList(const ModuleList &module_list) override; + class Arch { + public: + explicit Arch(const ArchSpec &spec); + const Arch &operator=(const ArchSpec &spec); + + const ArchSpec &GetSpec() const { return m_spec; } + Architecture *GetPlugin() const { return m_plugin_up.get(); } + + private: + ArchSpec m_spec; + std::unique_ptr m_plugin_up; + }; //------------------------------------------------------------------ // Member variables. //------------------------------------------------------------------ @@ -1212,7 +1227,7 @@ lldb::PlatformSP m_platform_sp; ///< The platform for this target. std::recursive_mutex m_mutex; ///< An API mutex that is used by the lldb::SB* /// classes make the SB interface thread safe - ArchSpec m_arch; + Arch m_arch; ModuleList m_images; ///< The list of images for this process (shared /// libraries and anything dynamically loaded). SectionLoadHistory m_section_load_history; Index: packages/Python/lldbsuite/test/arm/breakpoint-it/Makefile =================================================================== --- /dev/null +++ packages/Python/lldbsuite/test/arm/breakpoint-it/Makefile @@ -0,0 +1,6 @@ +LEVEL = ../../make + +C_SOURCES := main.c +CFLAGS_EXTRAS = -mthumb + +include $(LEVEL)/Makefile.rules Index: packages/Python/lldbsuite/test/arm/breakpoint-it/TestBreakpointIt.py =================================================================== --- /dev/null +++ packages/Python/lldbsuite/test/arm/breakpoint-it/TestBreakpointIt.py @@ -0,0 +1,45 @@ +""" +Test that breakpoints in an IT instruction don't fire if their condition is +false. +""" +from __future__ import print_function + + +import lldb +import os +import time +from lldbsuite.test.decorators import * +from lldbsuite.test.lldbtest import * +from lldbsuite.test import lldbutil + + +class TestBreakpointIt(TestBase): + + mydir = TestBase.compute_mydir(__file__) + NO_DEBUG_INFO_TESTCASE = True + + @skipIf(archs=no_match(["arm"])) + def test_false(self): + self.build() + exe = os.path.join(os.getcwd(), "a.out") + + self.runCmd("target create %s" % exe) + lldbutil.run_break_set_by_symbol(self, "bkpt_false", + extra_options="--skip-prologue 0") + + self.runCmd("run") + self.assertEqual(self.process().GetState(), lldb.eStateExited, + "Breakpoint does not get hit") + + @skipIf(archs=no_match(["arm"])) + def test_true(self): + self.build() + exe = os.path.join(os.getcwd(), "a.out") + + self.runCmd("target create %s" % exe) + bpid = lldbutil.run_break_set_by_symbol(self, "bkpt_true", + extra_options="--skip-prologue 0") + + self.runCmd("run") + self.assertIsNotNone(lldbutil.get_one_thread_stopped_at_breakpoint_id( + self.process(), bpid)) Index: packages/Python/lldbsuite/test/arm/breakpoint-it/main.c =================================================================== --- /dev/null +++ packages/Python/lldbsuite/test/arm/breakpoint-it/main.c @@ -0,0 +1,14 @@ +int main() { + int value; + asm ( + "cmp %1, %2\n\t" + "ite ne\n\t" + ".thumb_func\n\t" + "bkpt_true:\n\t" + "movne %0, %1\n\t" + ".thumb_func\n\t" + "bkpt_false:\n\t" + "moveq %0, %2\n\t" + : "=r" (value) : "r"(42), "r"(47)); + return value; +} Index: source/API/SystemInitializerFull.cpp =================================================================== --- source/API/SystemInitializerFull.cpp +++ source/API/SystemInitializerFull.cpp @@ -42,6 +42,7 @@ #include "Plugins/ABI/SysV-ppc64/ABISysV_ppc64.h" #include "Plugins/ABI/SysV-s390x/ABISysV_s390x.h" #include "Plugins/ABI/SysV-x86_64/ABISysV_x86_64.h" +#include "Plugins/Architecture/Arm/ArchitectureArm.h" #include "Plugins/Disassembler/llvm/DisassemblerLLVMC.h" #include "Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOS.h" #include "Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOSXDYLD.h" @@ -50,9 +51,9 @@ #include "Plugins/DynamicLoader/Windows-DYLD/DynamicLoaderWindowsDYLD.h" #include "Plugins/Instruction/ARM64/EmulateInstructionARM64.h" #include "Plugins/InstrumentationRuntime/ASan/ASanRuntime.h" +#include "Plugins/InstrumentationRuntime/MainThreadChecker/MainThreadCheckerRuntime.h" #include "Plugins/InstrumentationRuntime/TSan/TSanRuntime.h" #include "Plugins/InstrumentationRuntime/UBSan/UBSanRuntime.h" -#include "Plugins/InstrumentationRuntime/MainThreadChecker/MainThreadCheckerRuntime.h" #include "Plugins/JITLoader/GDB/JITLoaderGDB.h" #include "Plugins/Language/CPlusPlus/CPlusPlusLanguage.h" #include "Plugins/Language/Go/GoLanguage.h" @@ -304,6 +305,9 @@ ABISysV_mips::Initialize(); ABISysV_mips64::Initialize(); ABISysV_s390x::Initialize(); + + ArchitectureArm::Initialize(); + DisassemblerLLVMC::Initialize(); JITLoaderGDB::Initialize(); Index: source/Core/ArchSpec.cpp =================================================================== --- source/Core/ArchSpec.cpp +++ source/Core/ArchSpec.cpp @@ -11,17 +11,12 @@ #include "lldb/Host/HostInfo.h" #include "lldb/Target/Platform.h" -#include "lldb/Target/RegisterContext.h" -#include "lldb/Target/Thread.h" #include "lldb/Utility/NameMatches.h" #include "lldb/Utility/Stream.h" // for Stream #include "lldb/Utility/StringList.h" #include "lldb/lldb-defines.h" // for LLDB_INVALID_C... #include "lldb/lldb-forward.h" // for RegisterContextSP -#include "Plugins/Process/Utility/ARMDefines.h" -#include "Plugins/Process/Utility/InstructionUtils.h" - #include "llvm/ADT/STLExtras.h" #include "llvm/ADT/Twine.h" // for Twine #include "llvm/BinaryFormat/COFF.h" @@ -1502,102 +1497,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(); - Status 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(); @@ -1619,7 +1518,7 @@ void ArchSpec::PiecewiseTripleCompare( const ArchSpec &other, bool &arch_different, bool &vendor_different, - bool &os_different, bool &os_version_different, bool &env_different) { + bool &os_different, bool &os_version_different, bool &env_different) const { const llvm::Triple &me(GetTriple()); const llvm::Triple &them(other.GetTriple()); Index: source/Core/PluginManager.cpp =================================================================== --- source/Core/PluginManager.cpp +++ source/Core/PluginManager.cpp @@ -275,6 +275,54 @@ return nullptr; } +#pragma mark Architecture + +struct ArchitectureInstance { + ConstString name; + std::string description; + PluginManager::ArchitectureCreateInstance create_callback; +}; + +typedef std::vector ArchitectureInstances; + +static std::mutex g_architecture_mutex; + +static ArchitectureInstances &GetArchitectureInstances() { + static ArchitectureInstances g_instances; + return g_instances; +} + +void PluginManager::RegisterPlugin(const ConstString &name, + llvm::StringRef description, + ArchitectureCreateInstance create_callback) { + std::lock_guard guard(g_architecture_mutex); + GetArchitectureInstances().push_back({name, description, create_callback}); +} + +void PluginManager::UnregisterPlugin( + ArchitectureCreateInstance create_callback) { + std::lock_guard guard(g_architecture_mutex); + auto &instances = GetArchitectureInstances(); + + for (auto pos = instances.begin(), end = instances.end(); pos != end; ++pos) { + if (pos->create_callback == create_callback) { + instances.erase(pos); + return; + } + } + llvm_unreachable("Plugin not found"); +} + +std::unique_ptr +PluginManager::CreateArchitectureInstance(const ArchSpec &arch) { + std::lock_guard guard(g_architecture_mutex); + for (const auto &instances : GetArchitectureInstances()) { + if (auto plugin_up = instances.create_callback(arch)) + return plugin_up; + } + return nullptr; +} + #pragma mark Disassembler struct DisassemblerInstance { Index: source/Plugins/Architecture/Arm/ArchitectureArm.h =================================================================== --- /dev/null +++ source/Plugins/Architecture/Arm/ArchitectureArm.h @@ -0,0 +1,35 @@ +//===-- ArchitectureArm.h ---------------------------------------*- C++ -*-===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// + +#ifndef LLDB_PLUGIN_ARCHITECTURE_ARM_H +#define LLDB_PLUGIN_ARCHITECTURE_ARM_H + +#include "lldb/Core/Architecture.h" + +namespace lldb_private { + +class ArchitectureArm : public Architecture { +public: + static ConstString GetPluginNameStatic(); + static void Initialize(); + static void Terminate(); + + ConstString GetPluginName() override; + uint32_t GetPluginVersion() override; + + void OverrideStopInfo(Thread &thread) override; + +private: + static std::unique_ptr Create(const ArchSpec &arch); + ArchitectureArm() = default; +}; + +} // namespace lldb_private + +#endif // LLDB_PLUGIN_ARCHITECTURE_ARM_H Index: source/Plugins/Architecture/Arm/ArchitectureArm.cpp =================================================================== --- /dev/null +++ source/Plugins/Architecture/Arm/ArchitectureArm.cpp @@ -0,0 +1,131 @@ +//===-- ArchitectureArm.cpp -------------------------------------*- C++ -*-===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// + +#include "Plugins/Architecture/Arm/ArchitectureArm.h" +#include "Plugins/Process/Utility/ARMDefines.h" +#include "Plugins/Process/Utility/InstructionUtils.h" +#include "lldb/Core/ArchSpec.h" +#include "lldb/Core/PluginManager.h" +#include "lldb/Target/RegisterContext.h" +#include "lldb/Target/Thread.h" + +using namespace lldb_private; +using namespace lldb; + +ConstString ArchitectureArm::GetPluginNameStatic() { + return ConstString("arm"); +} + +void ArchitectureArm::Initialize() { + PluginManager::RegisterPlugin(GetPluginNameStatic(), + "Arm-specific algorithms", + &ArchitectureArm::Create); +} + +void ArchitectureArm::Terminate() { + PluginManager::UnregisterPlugin(&ArchitectureArm::Create); +} + +std::unique_ptr ArchitectureArm::Create(const ArchSpec &arch) { + if (arch.GetMachine() != llvm::Triple::arm) + return nullptr; + return std::unique_ptr(new ArchitectureArm()); +} + +ConstString ArchitectureArm::GetPluginName() { return GetPluginNameStatic(); } +uint32_t ArchitectureArm::GetPluginVersion() { return 1; } + +void ArchitectureArm::OverrideStopInfo(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) + return; + + const uint32_t cpsr = reg_ctx_sp->GetFlags(0); + if (cpsr == 0) + return; + + // 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(); + Status 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()); + } + } + } +} Index: source/Plugins/Architecture/Arm/CMakeLists.txt =================================================================== --- /dev/null +++ source/Plugins/Architecture/Arm/CMakeLists.txt @@ -0,0 +1,10 @@ +add_lldb_library(lldbPluginArchitectureArm PLUGIN + ArchitectureArm.cpp + + LINK_LIBS + lldbPluginProcessUtility + lldbCore + lldbTarget + LINK_COMPONENTS + Support + ) Index: source/Plugins/Architecture/CMakeLists.txt =================================================================== --- /dev/null +++ source/Plugins/Architecture/CMakeLists.txt @@ -0,0 +1 @@ +add_subdirectory(Arm) Index: source/Plugins/CMakeLists.txt =================================================================== --- source/Plugins/CMakeLists.txt +++ source/Plugins/CMakeLists.txt @@ -1,4 +1,5 @@ add_subdirectory(ABI) +add_subdirectory(Architecture) add_subdirectory(Disassembler) add_subdirectory(DynamicLoader) add_subdirectory(ExpressionParser) Index: source/Target/Process.cpp =================================================================== --- source/Target/Process.cpp +++ source/Target/Process.cpp @@ -743,8 +743,7 @@ m_profile_data_comm_mutex(), m_profile_data(), m_iohandler_sync(0), m_memory_cache(*this), m_allocated_memory_cache(*this), m_should_detach(false), m_next_event_action_ap(), m_public_run_lock(), - m_private_run_lock(), m_stop_info_override_callback(nullptr), - m_finalizing(false), m_finalize_called(false), + m_private_run_lock(), m_finalizing(false), m_finalize_called(false), m_clear_thread_plans_on_stop(false), m_force_next_event_delivery(false), m_last_broadcast_state(eStateInvalid), m_destroy_in_process(false), m_can_interpret_function_calls(false), m_warnings_issued(), @@ -871,7 +870,6 @@ m_language_runtimes.clear(); m_instrumentation_runtimes.clear(); m_next_event_action_ap.reset(); - m_stop_info_override_callback = nullptr; // Clear the last natural stop ID since it has a strong // reference to this process m_mod_id.SetStopEventForLastNaturalStopID(EventSP()); @@ -2712,7 +2710,6 @@ m_system_runtime_ap.reset(); m_os_ap.reset(); m_process_input_reader.reset(); - m_stop_info_override_callback = nullptr; Module *exe_module = GetTarget().GetExecutableModulePointer(); if (exe_module) { @@ -2800,9 +2797,6 @@ else StartPrivateStateThread(); - m_stop_info_override_callback = - GetTarget().GetArchitecture().GetStopInfoOverrideCallback(); - // Target was stopped at entry as was intended. Need to notify the // listeners // about it. @@ -2986,7 +2980,6 @@ m_jit_loaders_ap.reset(); m_system_runtime_ap.reset(); m_os_ap.reset(); - m_stop_info_override_callback = nullptr; lldb::pid_t attach_pid = attach_info.GetProcessID(); Status error; @@ -3219,8 +3212,6 @@ : ""); } } - - m_stop_info_override_callback = process_arch.GetStopInfoOverrideCallback(); } Status Process::ConnectRemote(Stream *strm, llvm::StringRef remote_url) { @@ -5849,7 +5840,6 @@ m_instrumentation_runtimes.clear(); m_thread_list.DiscardThreadPlans(); m_memory_cache.Clear(true); - m_stop_info_override_callback = nullptr; DoDidExec(); CompleteAttach(); // Flush the process (threads and all stack frames) after running Index: source/Target/Target.cpp =================================================================== --- source/Target/Target.cpp +++ source/Target/Target.cpp @@ -26,6 +26,7 @@ #include "lldb/Core/Event.h" #include "lldb/Core/Module.h" #include "lldb/Core/ModuleSpec.h" +#include "lldb/Core/PluginManager.h" #include "lldb/Core/Section.h" #include "lldb/Core/SourceManager.h" #include "lldb/Core/State.h" @@ -65,6 +66,16 @@ constexpr std::chrono::milliseconds EvaluateExpressionOptions::default_timeout; +Target::Arch::Arch(const ArchSpec &spec) + : m_spec(spec), + m_plugin_up(PluginManager::CreateArchitectureInstance(spec)) {} + +const Target::Arch& Target::Arch::operator=(const ArchSpec &spec) { + m_spec = spec; + m_plugin_up = PluginManager::CreateArchitectureInstance(spec); + return *this; +} + ConstString &Target::GetStaticBroadcasterClass() { static ConstString class_name("lldb.target"); return class_name; @@ -76,12 +87,12 @@ Broadcaster(debugger.GetBroadcasterManager(), Target::GetStaticBroadcasterClass().AsCString()), ExecutionContextScope(), m_debugger(debugger), m_platform_sp(platform_sp), - m_mutex(), m_arch(target_arch), m_images(this), m_section_load_history(), - m_breakpoint_list(false), m_internal_breakpoint_list(true), - m_watchpoint_list(), m_process_sp(), m_search_filter_sp(), - m_image_search_paths(ImageSearchPathsChanged, this), m_ast_importer_sp(), - m_source_manager_ap(), m_stop_hooks(), m_stop_hook_next_id(0), - m_valid(true), m_suppress_stop_hooks(false), + m_mutex(), m_arch(target_arch), + m_images(this), m_section_load_history(), m_breakpoint_list(false), + m_internal_breakpoint_list(true), m_watchpoint_list(), m_process_sp(), + m_search_filter_sp(), m_image_search_paths(ImageSearchPathsChanged, this), + m_ast_importer_sp(), m_source_manager_ap(), m_stop_hooks(), + m_stop_hook_next_id(0), m_valid(true), m_suppress_stop_hooks(false), m_is_dummy_target(is_dummy_target) { @@ -96,10 +107,11 @@ Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_OBJECT)); if (log) log->Printf("%p Target::Target()", static_cast(this)); - if (m_arch.IsValid()) { - LogIfAnyCategoriesSet( - LIBLLDB_LOG_TARGET, "Target::Target created with architecture %s (%s)", - m_arch.GetArchitectureName(), m_arch.GetTriple().getTriple().c_str()); + if (target_arch.IsValid()) { + LogIfAnyCategoriesSet(LIBLLDB_LOG_TARGET, + "Target::Target created with architecture %s (%s)", + target_arch.GetArchitectureName(), + target_arch.GetTriple().getTriple().c_str()); } } @@ -243,7 +255,7 @@ m_valid = false; DeleteCurrentProcess(); m_platform_sp.reset(); - m_arch.Clear(); + m_arch = ArchSpec(); ClearModules(true); m_section_load_history.Clear(); const bool notify = false; @@ -1226,8 +1238,8 @@ void Target::DidExec() { // When a process exec's we need to know about it so we can do some cleanup. - m_breakpoint_list.RemoveInvalidLocations(m_arch); - m_internal_breakpoint_list.RemoveInvalidLocations(m_arch); + m_breakpoint_list.RemoveInvalidLocations(m_arch.GetSpec()); + m_internal_breakpoint_list.RemoveInvalidLocations(m_arch.GetSpec()); } void Target::SetExecutableModule(ModuleSP &executable_sp, @@ -1245,13 +1257,12 @@ // If we haven't set an architecture yet, reset our architecture based on // what we found in the executable module. - if (!m_arch.IsValid()) { + if (!m_arch.GetSpec().IsValid()) { m_arch = executable_sp->GetArchitecture(); - if (log) - log->Printf("Target::SetExecutableModule setting architecture to %s " - "(%s) based on executable file", - m_arch.GetArchitectureName(), - m_arch.GetTriple().getTriple().c_str()); + LLDB_LOG(log, + "setting architecture to {0} ({1}) based on executable file", + m_arch.GetSpec().GetArchitectureName(), + m_arch.GetSpec().GetTriple().getTriple()); } FileSpecList dependent_files; @@ -1269,7 +1280,7 @@ else platform_dependent_file_spec = dependent_file_spec; - ModuleSpec module_spec(platform_dependent_file_spec, m_arch); + ModuleSpec module_spec(platform_dependent_file_spec, m_arch.GetSpec()); ModuleSP image_module_sp(GetSharedModule(module_spec)); if (image_module_sp) { ObjectFile *objfile = image_module_sp->GetObjectFile(); @@ -1283,21 +1294,21 @@ bool Target::SetArchitecture(const ArchSpec &arch_spec) { Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_TARGET)); - bool missing_local_arch = !m_arch.IsValid(); + bool missing_local_arch = !m_arch.GetSpec().IsValid(); bool replace_local_arch = true; bool compatible_local_arch = false; ArchSpec other(arch_spec); if (!missing_local_arch) { - if (m_arch.IsCompatibleMatch(arch_spec)) { - other.MergeFrom(m_arch); + if (m_arch.GetSpec().IsCompatibleMatch(arch_spec)) { + other.MergeFrom(m_arch.GetSpec()); - if (m_arch.IsCompatibleMatch(other)) { + if (m_arch.GetSpec().IsCompatibleMatch(other)) { compatible_local_arch = true; bool arch_changed, vendor_changed, os_changed, os_ver_changed, env_changed; - m_arch.PiecewiseTripleCompare(other, arch_changed, vendor_changed, + m_arch.GetSpec().PiecewiseTripleCompare(other, arch_changed, vendor_changed, os_changed, os_ver_changed, env_changed); if (!arch_changed && !vendor_changed && !os_changed && !env_changed) @@ -1311,10 +1322,9 @@ // update the architecture, unless the one we already have is more specified if (replace_local_arch) m_arch = other; - if (log) - log->Printf("Target::SetArchitecture set architecture to %s (%s)", - m_arch.GetArchitectureName(), - m_arch.GetTriple().getTriple().c_str()); + LLDB_LOG(log, "set architecture to {0} ({1})", + m_arch.GetSpec().GetArchitectureName(), + m_arch.GetSpec().GetTriple().getTriple()); return true; } @@ -1351,12 +1361,12 @@ bool Target::MergeArchitecture(const ArchSpec &arch_spec) { if (arch_spec.IsValid()) { - if (m_arch.IsCompatibleMatch(arch_spec)) { + if (m_arch.GetSpec().IsCompatibleMatch(arch_spec)) { // The current target arch is compatible with "arch_spec", see if we // can improve our current architecture using bits from "arch_spec" // Merge bits from arch_spec into "merged_arch" and set our architecture - ArchSpec merged_arch(m_arch); + ArchSpec merged_arch(m_arch.GetSpec()); merged_arch.MergeFrom(arch_spec); return SetArchitecture(merged_arch); } else { @@ -1684,8 +1694,8 @@ size_t bytes_read = ReadMemory(addr, prefer_file_cache, &uval, byte_size, error); if (bytes_read == byte_size) { - DataExtractor data(&uval, sizeof(uval), m_arch.GetByteOrder(), - m_arch.GetAddressByteSize()); + DataExtractor data(&uval, sizeof(uval), m_arch.GetSpec().GetByteOrder(), + m_arch.GetSpec().GetAddressByteSize()); lldb::offset_t offset = 0; if (byte_size <= 4) scalar = data.GetMaxU32(&offset, byte_size); @@ -1719,7 +1729,7 @@ Status &error, Address &pointer_addr) { Scalar scalar; if (ReadScalarIntegerFromMemory(addr, prefer_file_cache, - m_arch.GetAddressByteSize(), false, scalar, + m_arch.GetSpec().GetAddressByteSize(), false, scalar, error)) { addr_t pointer_vm_addr = scalar.ULongLong(LLDB_INVALID_ADDRESS); if (pointer_vm_addr != LLDB_INVALID_ADDRESS) { @@ -2212,7 +2222,7 @@ lldb::addr_t Target::GetCallableLoadAddress(lldb::addr_t load_addr, AddressClass addr_class) const { addr_t code_addr = load_addr; - switch (m_arch.GetMachine()) { + switch (m_arch.GetSpec().GetMachine()) { case llvm::Triple::mips: case llvm::Triple::mipsel: case llvm::Triple::mips64: @@ -2271,7 +2281,7 @@ lldb::addr_t Target::GetOpcodeLoadAddress(lldb::addr_t load_addr, AddressClass addr_class) const { addr_t opcode_addr = load_addr; - switch (m_arch.GetMachine()) { + switch (m_arch.GetSpec().GetMachine()) { case llvm::Triple::mips: case llvm::Triple::mipsel: case llvm::Triple::mips64: @@ -2303,7 +2313,7 @@ addr_t breakable_addr = addr; Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_BREAKPOINTS)); - switch (m_arch.GetMachine()) { + switch (m_arch.GetSpec().GetMachine()) { default: break; case llvm::Triple::mips: @@ -2314,7 +2324,7 @@ addr_t current_offset = 0; uint32_t loop_count = 0; Address resolved_addr; - uint32_t arch_flags = m_arch.GetFlags(); + uint32_t arch_flags = m_arch.GetSpec().GetFlags(); bool IsMips16 = arch_flags & ArchSpec::eMIPSAse_mips16; bool IsMicromips = arch_flags & ArchSpec::eMIPSAse_micromips; SectionLoadList §ion_load_list = GetSectionLoadList(); @@ -2367,7 +2377,7 @@ // Create Disassembler Instance lldb::DisassemblerSP disasm_sp( - Disassembler::FindPlugin(m_arch, nullptr, nullptr)); + Disassembler::FindPlugin(m_arch.GetSpec(), nullptr, nullptr)); ExecutionContext exe_ctx; CalculateExecutionContext(exe_ctx); Index: source/Target/Thread.cpp =================================================================== --- source/Target/Thread.cpp +++ source/Target/Thread.cpp @@ -442,10 +442,9 @@ 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); + if (Architecture *arch = + process_sp->GetTarget().GetArchitecturePlugin()) + arch->OverrideStopInfo(*this); } } }