Move StopInfoOverride callback to the new architecture plugin

This creates a new Architecture plugin and moves the stop info override
callback to this place. The motivation for this is to remove complex
dependencies from the ArchSpec class because it is used in a lot of
places that (should) know nothing about Process instances and StopInfo
objects.

I also add a test for the functionality covered by the override
callback.

Differential Revision: https://reviews.llvm.org/D31172

llvm-svn: 316609

lldb/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 <stddef.h> // for size_t
 #include <stdint.h> // 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 {
 
 //----------------------------------------------------------------------
@@ -258,8 +246,6 @@ class ArchSpec {
 
   };
 
-  typedef void (*StopInfoOverrideCallbackType)(lldb_private::Thread &thread);
-
   //------------------------------------------------------------------
   /// Default constructor.
   ///
@@ -574,34 +560,11 @@ class ArchSpec {
   //------------------------------------------------------------------
   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

lldb/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

lldb/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 @@ class PluginManager {
   static ABICreateInstance
   GetABICreateCallbackForPluginName(const ConstString &name);
 
+  //------------------------------------------------------------------
+  // Architecture
+  //------------------------------------------------------------------
+  using ArchitectureCreateInstance =
+      std::unique_ptr<Architecture> (*)(const ArchSpec &);
+
+  static void RegisterPlugin(const ConstString &name,
+                             llvm::StringRef description,
+                             ArchitectureCreateInstance create_callback);
+
+  static void UnregisterPlugin(ArchitectureCreateInstance create_callback);
+
+  static std::unique_ptr<Architecture>
+  CreateArchitectureInstance(const ArchSpec &arch);
+
   //------------------------------------------------------------------
   // Disassembler
   //------------------------------------------------------------------

lldb/include/lldb/Target/Process.h

@@ -2514,10 +2514,6 @@ class Process : public std::enable_shared_from_this<Process>,
 
   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 @@ class Process : public std::enable_shared_from_this<Process>,
   std::vector<PreResumeCallbackAndBaton> 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,

lldb/include/lldb/Target/Target.h

@@ -24,6 +24,7 @@
 #include "lldb/Breakpoint/BreakpointName.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"
@@ -917,7 +918,7 @@ class Target : public std::enable_shared_from_this<Target>,
   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.
@@ -948,6 +949,8 @@ class Target : public std::enable_shared_from_this<Target>,
 
   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,
@@ -1241,14 +1244,26 @@ class Target : public std::enable_shared_from_this<Target>,
                      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<Architecture> m_plugin_up;
+  };
   //------------------------------------------------------------------
   // Member variables.
   //------------------------------------------------------------------
   Debugger &m_debugger;
   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;

lldb/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

lldb/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))

lldb/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;
+}

lldb/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 @@ void SystemInitializerFull::Initialize() {
   ABISysV_mips::Initialize();
   ABISysV_mips64::Initialize();
   ABISysV_s390x::Initialize();
+
+  ArchitectureArm::Initialize();
+
   DisassemblerLLVMC::Initialize();
 
   JITLoaderGDB::Initialize();