[Lldb-commits] [lldb] r309772 - Fix a mis-feature with propagation of breakpoint options -> location options.

Jim Ingham via lldb-commits lldb-commits at lists.llvm.org
Tue Aug 1 17:16:10 PDT 2017


Author: jingham
Date: Tue Aug  1 17:16:10 2017
New Revision: 309772

URL: http://llvm.org/viewvc/llvm-project?rev=309772&view=rev
Log:
Fix a mis-feature with propagation of breakpoint options -> location options.

When an option was set at on a location, I was just copying the whole option set 
to the location, and letting it shadow the breakpoint options.  That was wrong since
it meant changes to unrelated options on the breakpoint would no longer take on this
location.  I added a mask of set options and use that for option propagation.

I also added a "location" property to breakpoints, and added SBBreakpointLocation.{G,S}etCommandLineCommands
since I wanted to use them to write some more test cases.

<rdar://problem/24397798>

Modified:
    lldb/trunk/include/lldb/API/SBBreakpointLocation.h
    lldb/trunk/include/lldb/API/SBStringList.h
    lldb/trunk/include/lldb/Breakpoint/BreakpointLocation.h
    lldb/trunk/include/lldb/Breakpoint/BreakpointOptions.h
    lldb/trunk/lldb.xcodeproj/xcshareddata/xcschemes/desktop.xcscheme
    lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_locations/TestBreakpointLocations.py
    lldb/trunk/scripts/interface/SBBreakpoint.i
    lldb/trunk/scripts/interface/SBBreakpointLocation.i
    lldb/trunk/source/API/SBBreakpointLocation.cpp
    lldb/trunk/source/Breakpoint/Breakpoint.cpp
    lldb/trunk/source/Breakpoint/BreakpointLocation.cpp
    lldb/trunk/source/Breakpoint/BreakpointOptions.cpp
    lldb/trunk/source/Commands/CommandObjectBreakpointCommand.cpp

Modified: lldb/trunk/include/lldb/API/SBBreakpointLocation.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/API/SBBreakpointLocation.h?rev=309772&r1=309771&r2=309772&view=diff
==============================================================================
--- lldb/trunk/include/lldb/API/SBBreakpointLocation.h (original)
+++ lldb/trunk/include/lldb/API/SBBreakpointLocation.h Tue Aug  1 17:16:10 2017
@@ -51,7 +51,11 @@ public:
   void SetScriptCallbackFunction(const char *callback_function_name);
 
   SBError SetScriptCallbackBody(const char *script_body_text);
+  
+  void SetCommandLineCommands(SBStringList &commands);
 
+  bool GetCommandLineCommands(SBStringList &commands);
+ 
   void SetThreadID(lldb::tid_t sb_thread_id);
 
   lldb::tid_t GetThreadID();

Modified: lldb/trunk/include/lldb/API/SBStringList.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/API/SBStringList.h?rev=309772&r1=309771&r2=309772&view=diff
==============================================================================
--- lldb/trunk/include/lldb/API/SBStringList.h (original)
+++ lldb/trunk/include/lldb/API/SBStringList.h Tue Aug  1 17:16:10 2017
@@ -44,6 +44,7 @@ protected:
   friend class SBCommandInterpreter;
   friend class SBDebugger;
   friend class SBBreakpoint;
+  friend class SBBreakpointLocation;
 
   SBStringList(const lldb_private::StringList *lldb_strings);
 

Modified: lldb/trunk/include/lldb/Breakpoint/BreakpointLocation.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Breakpoint/BreakpointLocation.h?rev=309772&r1=309771&r2=309772&view=diff
==============================================================================
--- lldb/trunk/include/lldb/Breakpoint/BreakpointLocation.h (original)
+++ lldb/trunk/include/lldb/Breakpoint/BreakpointLocation.h Tue Aug  1 17:16:10 2017
@@ -17,6 +17,7 @@
 
 // Other libraries and framework includes
 // Project includes
+#include "lldb/Breakpoint/BreakpointOptions.h"
 #include "lldb/Breakpoint/StoppointLocation.h"
 #include "lldb/Core/Address.h"
 #include "lldb/Utility/UserID.h"
@@ -255,14 +256,17 @@ public:
 
   //------------------------------------------------------------------
   /// Use this to access breakpoint options from this breakpoint location.
-  /// This will point to the owning breakpoint's options unless options have
-  /// been set specifically on this location.
+  /// This will return the options that have a setting for the specified
+  /// BreakpointOptions kind.
   ///
+  /// @param[in] kind
+  ///     The particular option you are looking up.
   /// @return
   ///     A pointer to the containing breakpoint's options if this
   ///     location doesn't have its own copy.
   //------------------------------------------------------------------
-  const BreakpointOptions *GetOptionsNoCreate() const;
+  const BreakpointOptions *GetOptionsSpecifyingKind(
+      BreakpointOptions::OptionKind kind) const;
 
   bool ValidForThisThread(Thread *thread);
 

Modified: lldb/trunk/include/lldb/Breakpoint/BreakpointOptions.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Breakpoint/BreakpointOptions.h?rev=309772&r1=309771&r2=309772&view=diff
==============================================================================
--- lldb/trunk/include/lldb/Breakpoint/BreakpointOptions.h (original)
+++ lldb/trunk/include/lldb/Breakpoint/BreakpointOptions.h Tue Aug  1 17:16:10 2017
@@ -18,6 +18,7 @@
 // Other libraries and framework includes
 // Project includes
 #include "lldb/Utility/Baton.h"
+#include "lldb/Utility/Flags.h"
 #include "lldb/Utility/StringList.h"
 #include "lldb/Utility/StructuredData.h"
 #include "lldb/lldb-private.h"
@@ -32,7 +33,18 @@ namespace lldb_private {
 //----------------------------------------------------------------------
 
 class BreakpointOptions {
+friend class BreakpointLocation;
+friend class Breakpoint;
+
 public:
+  enum OptionKind {
+    eCallback =    1 << 0,
+    eEnabled  =    1 << 1,
+    eOneShot  =    1 << 2,
+    eIgnoreCount = 1 << 3,
+    eThreadSpec  = 1 << 4,
+    eCondition   = 1 << 5
+  };
   struct CommandData {
     CommandData()
         : user_source(), script_source(),
@@ -85,18 +97,6 @@ public:
   typedef std::shared_ptr<CommandBaton> CommandBatonSP;
 
   //------------------------------------------------------------------
-  // Constructors and Destructors
-  //------------------------------------------------------------------
-  //------------------------------------------------------------------
-  /// Default constructor.  The breakpoint is enabled, and has no condition,
-  /// callback, ignore count, etc...
-  //------------------------------------------------------------------
-  BreakpointOptions();
-  BreakpointOptions(const BreakpointOptions &rhs);
-
-  static BreakpointOptions *CopyOptionsNoCallback(BreakpointOptions &rhs);
-
-  //------------------------------------------------------------------
   /// This constructor allows you to specify all the breakpoint options
   /// except the callback.  That one is more complicated, and better
   /// to do by hand.
@@ -290,7 +290,10 @@ public:
   //------------------------------------------------------------------
   /// If \a enable is \b true, enable the breakpoint, if \b false disable it.
   //------------------------------------------------------------------
-  void SetEnabled(bool enabled) { m_enabled = enabled; }
+  void SetEnabled(bool enabled) { 
+    m_enabled = enabled;
+    m_set_flags.Set(eEnabled);
+  }
 
   //------------------------------------------------------------------
   /// Check the One-shot state.
@@ -302,7 +305,10 @@ public:
   //------------------------------------------------------------------
   /// If \a enable is \b true, enable the breakpoint, if \b false disable it.
   //------------------------------------------------------------------
-  void SetOneShot(bool one_shot) { m_one_shot = one_shot; }
+  void SetOneShot(bool one_shot) { 
+    m_one_shot = one_shot; 
+    m_set_flags.Set(eOneShot); 
+  }
 
   //------------------------------------------------------------------
   /// Set the breakpoint to ignore the next \a count breakpoint hits.
@@ -310,7 +316,10 @@ public:
   ///    The number of breakpoint hits to ignore.
   //------------------------------------------------------------------
 
-  void SetIgnoreCount(uint32_t n) { m_ignore_count = n; }
+  void SetIgnoreCount(uint32_t n) { 
+    m_ignore_count = n; 
+    m_set_flags.Set(eIgnoreCount);
+  }
 
   //------------------------------------------------------------------
   /// Return the current Ignore Count.
@@ -360,11 +369,26 @@ public:
   ///     The breakpoint will take ownership of pointer held by this object.
   //------------------------------------------------------------------
   void SetCommandDataCallback(std::unique_ptr<CommandData> &cmd_data);
-
+  
 protected:
   //------------------------------------------------------------------
+  // Constructors and Destructors
+  //------------------------------------------------------------------
+  //------------------------------------------------------------------
+  /// Breakpoints make options with all flags set.  Locations make options
+  /// with no flags set.  Nobody else should make breakpoint options.
+  //------------------------------------------------------------------
+  BreakpointOptions(bool all_flags_set);
+  BreakpointOptions(const BreakpointOptions &rhs);
+
+//------------------------------------------------------------------
   // Classes that inherit from BreakpointOptions can see and modify these
   //------------------------------------------------------------------
+  bool IsOptionSet(OptionKind kind)
+  {
+    return m_set_flags.Test(kind);
+  }
+
   enum class OptionNames {
     ConditionText = 0,
     IgnoreCount,
@@ -400,6 +424,8 @@ private:
   std::string m_condition_text; // The condition to test.
   size_t m_condition_text_hash; // Its hash, so that locations know when the
                                 // condition is updated.
+  Flags m_set_flags;            // Which options are set at this level.  Drawn
+                                // from BreakpointOptions::SetOptionsFlags.
 };
 
 } // namespace lldb_private

Modified: lldb/trunk/lldb.xcodeproj/xcshareddata/xcschemes/desktop.xcscheme
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/lldb.xcodeproj/xcshareddata/xcschemes/desktop.xcscheme?rev=309772&r1=309771&r2=309772&view=diff
==============================================================================
--- lldb/trunk/lldb.xcodeproj/xcshareddata/xcschemes/desktop.xcscheme (original)
+++ lldb/trunk/lldb.xcodeproj/xcshareddata/xcschemes/desktop.xcscheme Tue Aug  1 17:16:10 2017
@@ -26,6 +26,7 @@
       buildConfiguration = "Debug"
       selectedDebuggerIdentifier = "Xcode.DebuggerFoundation.Debugger.LLDB"
       selectedLauncherIdentifier = "Xcode.DebuggerFoundation.Launcher.LLDB"
+      language = ""
       shouldUseLaunchSchemeArgsEnv = "YES">
       <Testables>
       </Testables>
@@ -36,6 +37,7 @@
       buildConfiguration = "DebugClang"
       selectedDebuggerIdentifier = "Xcode.DebuggerFoundation.Debugger.LLDB"
       selectedLauncherIdentifier = "Xcode.DebuggerFoundation.Launcher.LLDB"
+      language = ""
       launchStyle = "0"
       useCustomWorkingDirectory = "NO"
       ignoresPersistentStateOnLaunch = "NO"

Modified: lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_locations/TestBreakpointLocations.py
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_locations/TestBreakpointLocations.py?rev=309772&r1=309771&r2=309772&view=diff
==============================================================================
--- lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_locations/TestBreakpointLocations.py (original)
+++ lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_locations/TestBreakpointLocations.py Tue Aug  1 17:16:10 2017
@@ -18,27 +18,40 @@ class BreakpointLocationsTestCase(TestBa
     mydir = TestBase.compute_mydir(__file__)
 
     @expectedFailureAll(oslist=["windows"], bugnumber="llvm.org/pr24528")
-    def test(self):
+    def test_enable(self):
         """Test breakpoint enable/disable for a breakpoint ID with multiple locations."""
         self.build()
         self.breakpoint_locations_test()
 
+    def test_shadowed_cond_options(self):
+        """Test that options set on the breakpoint and location behave correctly."""
+        self.build()
+        self.shadowed_bkpt_cond_test()
+
+
+    def test_shadowed_command_options(self):
+        """Test that options set on the breakpoint and location behave correctly."""
+        self.build()
+        self.shadowed_bkpt_command_test()
+
     def setUp(self):
         # Call super's setUp().
         TestBase.setUp(self)
         # Find the line number to break inside main().
         self.line = line_number('main.c', '// Set break point at this line.')
 
-    def breakpoint_locations_test(self):
-        """Test breakpoint enable/disable for a breakpoint ID with multiple locations."""
+    def set_breakpoint (self):
         exe = os.path.join(os.getcwd(), "a.out")
-        self.runCmd("file " + exe, CURRENT_EXECUTABLE_SET)
+        target = self.dbg.CreateTarget(exe)
+        self.assertTrue(target, "Target %s is not valid"%(exe))
 
         # This should create a breakpoint with 3 locations.
-        lldbutil.run_break_set_by_file_and_line(
-            self, "main.c", self.line, num_expected_locations=3)
+        
+        bkpt = target.BreakpointCreateByLocation("main.c", self.line)
 
         # The breakpoint list should show 3 locations.
+        self.assertEqual(bkpt.GetNumLocations(), 3, "Wrong number of locations")
+        
         self.expect(
             "breakpoint list -f",
             "Breakpoint locations shown correctly",
@@ -49,6 +62,87 @@ class BreakpointLocationsTestCase(TestBa
                 "where = a.out`func_inlined .+unresolved, hit count = 0",
                 "where = a.out`main .+\[inlined\].+unresolved, hit count = 0"])
 
+        return bkpt        
+
+    def shadowed_bkpt_cond_test(self):
+        """Test that options set on the breakpoint and location behave correctly."""
+        # Breakpoint option propagation from bkpt to loc used to be done the first time
+        # a breakpoint location option was specifically set.  After that the other options
+        # on that location would stop tracking the breakpoint.  That got fixed, and this test
+        # makes sure only the option touched is affected.
+
+        bkpt = self.set_breakpoint()
+        bkpt_cond = "1 == 0"
+        bkpt.SetCondition(bkpt_cond)
+        self.assertEqual(bkpt.GetCondition(), bkpt_cond,"Successfully set condition")
+        self.assertTrue(bkpt.location[0].GetCondition() == bkpt.GetCondition(), "Conditions are the same")
+
+        # Now set a condition on the locations, make sure that this doesn't effect the bkpt:
+        bkpt_loc_1_cond = "1 == 1"
+        bkpt.location[0].SetCondition(bkpt_loc_1_cond)
+        self.assertEqual(bkpt.location[0].GetCondition(), bkpt_loc_1_cond, "Successfully changed location condition")
+        self.assertNotEqual(bkpt.GetCondition(), bkpt_loc_1_cond, "Changed location changed Breakpoint condition")
+        self.assertEqual(bkpt.location[1].GetCondition(), bkpt_cond, "Changed another location's condition")
+
+        # Now make sure that setting one options doesn't fix the value of another:
+        bkpt.SetIgnoreCount(10)
+        self.assertEqual(bkpt.GetIgnoreCount(), 10, "Set the ignore count successfully")
+        self.assertEqual(bkpt.location[0].GetIgnoreCount(), 10, "Location doesn't track top-level bkpt.")
+
+        # Now make sure resetting the condition to "" resets the tracking:
+        bkpt.location[0].SetCondition("")
+        bkpt_new_cond = "1 == 3"
+        bkpt.SetCondition(bkpt_new_cond)
+        self.assertEqual(bkpt.location[0].GetCondition(), bkpt_new_cond, "Didn't go back to tracking condition")
+
+    def shadowed_bkpt_command_test(self):
+        """Test that options set on the breakpoint and location behave correctly."""
+        # Breakpoint option propagation from bkpt to loc used to be done the first time
+        # a breakpoint location option was specifically set.  After that the other options
+        # on that location would stop tracking the breakpoint.  That got fixed, and this test
+        # makes sure only the option touched is affected.
+
+        bkpt = self.set_breakpoint()
+        commands = ["AAAAAA", "BBBBBB", "CCCCCC"]
+        str_list = lldb.SBStringList()
+        str_list.AppendList(commands, len(commands))
+        
+        bkpt.SetCommandLineCommands(str_list)
+        cmd_list = lldb.SBStringList()
+        bkpt.GetCommandLineCommands(cmd_list)
+        list_size = str_list.GetSize()
+        self.assertEqual(cmd_list.GetSize() , list_size, "Added the right number of commands")
+        for i in range(0,list_size):
+            self.assertEqual(str_list.GetStringAtIndex(i), cmd_list.GetStringAtIndex(i), "Mismatched commands.")
+
+        commands = ["DDDDDD", "EEEEEE", "FFFFFF", "GGGGGG"]
+        loc_list = lldb.SBStringList()
+        loc_list.AppendList(commands, len(commands))
+        bkpt.location[1].SetCommandLineCommands(loc_list)
+        loc_cmd_list = lldb.SBStringList()
+        bkpt.location[1].GetCommandLineCommands(loc_cmd_list)
+
+        loc_list_size = loc_list.GetSize()
+        
+        # Check that the location has the right commands:
+        self.assertEqual(loc_cmd_list.GetSize() , loc_list_size, "Added the right number of commands to location")
+        for i in range(0,loc_list_size):
+            self.assertEqual(loc_list.GetStringAtIndex(i), loc_cmd_list.GetStringAtIndex(i), "Mismatched commands.")
+
+        # Check that we didn't mess up the breakpoint level commands:
+        self.assertEqual(cmd_list.GetSize() , list_size, "Added the right number of commands")
+        for i in range(0,list_size):
+            self.assertEqual(str_list.GetStringAtIndex(i), cmd_list.GetStringAtIndex(i), "Mismatched commands.")
+
+        # And check we didn't mess up another location:
+        untouched_loc_cmds = lldb.SBStringList()
+        bkpt.location[0].GetCommandLineCommands(untouched_loc_cmds)
+        self.assertEqual(untouched_loc_cmds.GetSize() , 0, "Changed the wrong location")
+
+    def breakpoint_locations_test(self):
+        """Test breakpoint enable/disable for a breakpoint ID with multiple locations."""
+        self.set_breakpoint()
+
         # The 'breakpoint disable 3.*' command should fail gracefully.
         self.expect("breakpoint disable 3.*",
                     "Disabling an invalid breakpoint should fail gracefully",
@@ -80,7 +174,7 @@ class BreakpointLocationsTestCase(TestBa
             "1 breakpoint locatons disabled correctly",
             startstr="1 breakpoints disabled.")
 
-        # Run the program againt.  We should stop on the two breakpoint
+        # Run the program again.  We should stop on the two breakpoint
         # locations.
         self.runCmd("run", RUN_SUCCEEDED)
 

Modified: lldb/trunk/scripts/interface/SBBreakpoint.i
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/scripts/interface/SBBreakpoint.i?rev=309772&r1=309771&r2=309772&view=diff
==============================================================================
--- lldb/trunk/scripts/interface/SBBreakpoint.i (original)
+++ lldb/trunk/scripts/interface/SBBreakpoint.i Tue Aug  1 17:16:10 2017
@@ -251,6 +251,39 @@ public:
     
     %pythoncode %{
         
+        class locations_access(object):
+            '''A helper object that will lazily hand out locations for a breakpoint when supplied an index.'''
+            def __init__(self, sbbreakpoint):
+                self.sbbreakpoint = sbbreakpoint
+        
+            def __len__(self):
+                if self.sbbreakpoint:
+                    return int(self.sbbreakpoint.GetNumLocations())
+                return 0
+        
+            def __getitem__(self, key):
+                if type(key) is int and key < len(self):
+                    return self.sbbreakpoint.GetLocationAtIndex(key)
+                return None
+        
+        def get_locations_access_object(self):
+            '''An accessor function that returns a locations_access() object which allows lazy location access from a lldb.SBBreakpoint object.'''
+            return self.locations_access (self)
+        
+        def get_breakpoint_location_list(self):
+            '''An accessor function that returns a list() that contains all locations in a lldb.SBBreakpoint object.'''
+            locations = []
+            accessor = self.get_locations_access_object()
+            for idx in range(len(accessor)):
+                locations.append(accessor[idx])
+            return locations
+        
+        __swig_getmethods__["locations"] = get_breakpoint_location_list
+        if _newclass: locations = property(get_breakpoint_location_list, None, doc='''A read only property that returns a list() of lldb.SBBreakpointLocation objects for this breakpoint.''')
+        
+        __swig_getmethods__["location"] = get_locations_access_object
+        if _newclass: location = property(get_locations_access_object, None, doc='''A read only property that returns an object that can access locations by index (not location ID) (location = bkpt.location[12]).''')
+
         __swig_getmethods__["id"] = GetID
         if _newclass: id = property(GetID, None, doc='''A read only property that returns the ID of this breakpoint.''')
             

Modified: lldb/trunk/scripts/interface/SBBreakpointLocation.i
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/scripts/interface/SBBreakpointLocation.i?rev=309772&r1=309771&r2=309772&view=diff
==============================================================================
--- lldb/trunk/scripts/interface/SBBreakpointLocation.i (original)
+++ lldb/trunk/scripts/interface/SBBreakpointLocation.i Tue Aug  1 17:16:10 2017
@@ -96,6 +96,10 @@ public:
     SBError
     SetScriptCallbackBody (const char *script_body_text);
     
+    void SetCommandLineCommands(SBStringList &commands);
+
+    bool GetCommandLineCommands(SBStringList &commands);
+
     void
     SetThreadID (lldb::tid_t sb_thread_id);
 

Modified: lldb/trunk/source/API/SBBreakpointLocation.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/API/SBBreakpointLocation.cpp?rev=309772&r1=309771&r2=309772&view=diff
==============================================================================
--- lldb/trunk/source/API/SBBreakpointLocation.cpp (original)
+++ lldb/trunk/source/API/SBBreakpointLocation.cpp Tue Aug  1 17:16:10 2017
@@ -12,6 +12,7 @@
 #include "lldb/API/SBDebugger.h"
 #include "lldb/API/SBDefines.h"
 #include "lldb/API/SBStream.h"
+#include "lldb/API/SBStringList.h"
 
 #include "lldb/Breakpoint/Breakpoint.h"
 #include "lldb/Breakpoint/BreakpointLocation.h"
@@ -195,6 +196,33 @@ SBBreakpointLocation::SetScriptCallbackB
   return sb_error;
 }
 
+void SBBreakpointLocation::SetCommandLineCommands(SBStringList &commands) {
+  BreakpointLocationSP loc_sp = GetSP();
+  if (!loc_sp)
+    return;
+  if (commands.GetSize() == 0)
+    return;
+
+  std::lock_guard<std::recursive_mutex> guard(
+      loc_sp->GetTarget().GetAPIMutex());
+  std::unique_ptr<BreakpointOptions::CommandData> cmd_data_up(
+      new BreakpointOptions::CommandData(*commands, eScriptLanguageNone));
+
+  loc_sp->GetLocationOptions()->SetCommandDataCallback(cmd_data_up);
+}
+
+bool SBBreakpointLocation::GetCommandLineCommands(SBStringList &commands) {
+  BreakpointLocationSP loc_sp = GetSP();
+  if (!loc_sp)
+    return false;
+  StringList command_list;
+  bool has_commands =
+      loc_sp->GetLocationOptions()->GetCommandLineCallbacks(command_list);
+  if (has_commands)
+    commands.AppendList(command_list);
+  return has_commands;
+}
+
 void SBBreakpointLocation::SetThreadID(tid_t thread_id) {
   BreakpointLocationSP loc_sp = GetSP();
   if (loc_sp) {

Modified: lldb/trunk/source/Breakpoint/Breakpoint.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Breakpoint/Breakpoint.cpp?rev=309772&r1=309771&r2=309772&view=diff
==============================================================================
--- lldb/trunk/source/Breakpoint/Breakpoint.cpp (original)
+++ lldb/trunk/source/Breakpoint/Breakpoint.cpp Tue Aug  1 17:16:10 2017
@@ -53,7 +53,7 @@ Breakpoint::Breakpoint(Target &target, S
                        bool resolve_indirect_symbols)
     : m_being_created(true), m_hardware(hardware), m_target(target),
       m_filter_sp(filter_sp), m_resolver_sp(resolver_sp),
-      m_options_up(new BreakpointOptions()), m_locations(*this),
+      m_options_up(new BreakpointOptions(true)), m_locations(*this),
       m_resolve_indirect_symbols(resolve_indirect_symbols), m_hit_count(0) {
   m_being_created = false;
 }

Modified: lldb/trunk/source/Breakpoint/BreakpointLocation.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Breakpoint/BreakpointLocation.cpp?rev=309772&r1=309771&r2=309772&view=diff
==============================================================================
--- lldb/trunk/source/Breakpoint/BreakpointLocation.cpp (original)
+++ lldb/trunk/source/Breakpoint/BreakpointLocation.cpp Tue Aug  1 17:16:10 2017
@@ -58,6 +58,15 @@ lldb::addr_t BreakpointLocation::GetLoad
   return m_address.GetOpcodeLoadAddress(&m_owner.GetTarget());
 }
 
+const BreakpointOptions *
+BreakpointLocation::GetOptionsSpecifyingKind(BreakpointOptions::OptionKind kind)
+const {
+    if (m_options_ap && m_options_ap->IsOptionSet(kind))
+      return m_options_ap.get();
+    else
+      return m_owner.GetOptions();
+}
+
 Address &BreakpointLocation::GetAddress() { return m_address; }
 
 Breakpoint &BreakpointLocation::GetBreakpoint() { return m_owner; }
@@ -97,8 +106,11 @@ void BreakpointLocation::SetThreadID(lld
 }
 
 lldb::tid_t BreakpointLocation::GetThreadID() {
-  if (GetOptionsNoCreate()->GetThreadSpecNoCreate())
-    return GetOptionsNoCreate()->GetThreadSpecNoCreate()->GetTID();
+  const ThreadSpec *thread_spec = 
+      GetOptionsSpecifyingKind(BreakpointOptions::eThreadSpec)
+          ->GetThreadSpecNoCreate();
+  if (thread_spec)
+    return thread_spec->GetTID();
   else
     return LLDB_INVALID_THREAD_ID;
 }
@@ -116,8 +128,11 @@ void BreakpointLocation::SetThreadIndex(
 }
 
 uint32_t BreakpointLocation::GetThreadIndex() const {
-  if (GetOptionsNoCreate()->GetThreadSpecNoCreate())
-    return GetOptionsNoCreate()->GetThreadSpecNoCreate()->GetIndex();
+  const ThreadSpec *thread_spec = 
+      GetOptionsSpecifyingKind(BreakpointOptions::eThreadSpec)
+          ->GetThreadSpecNoCreate();
+  if (thread_spec)
+    return thread_spec->GetIndex();
   else
     return 0;
 }
@@ -135,8 +150,11 @@ void BreakpointLocation::SetThreadName(c
 }
 
 const char *BreakpointLocation::GetThreadName() const {
-  if (GetOptionsNoCreate()->GetThreadSpecNoCreate())
-    return GetOptionsNoCreate()->GetThreadSpecNoCreate()->GetName();
+  const ThreadSpec *thread_spec = 
+      GetOptionsSpecifyingKind(BreakpointOptions::eThreadSpec)
+          ->GetThreadSpecNoCreate();
+  if (thread_spec)
+    return thread_spec->GetName();
   else
     return nullptr;
 }
@@ -154,8 +172,11 @@ void BreakpointLocation::SetQueueName(co
 }
 
 const char *BreakpointLocation::GetQueueName() const {
-  if (GetOptionsNoCreate()->GetThreadSpecNoCreate())
-    return GetOptionsNoCreate()->GetThreadSpecNoCreate()->GetQueueName();
+  const ThreadSpec *thread_spec = 
+      GetOptionsSpecifyingKind(BreakpointOptions::eThreadSpec)
+          ->GetThreadSpecNoCreate();
+  if (thread_spec)
+    return thread_spec->GetQueueName();
   else
     return nullptr;
 }
@@ -193,7 +214,8 @@ void BreakpointLocation::SetCondition(co
 }
 
 const char *BreakpointLocation::GetConditionText(size_t *hash) const {
-  return GetOptionsNoCreate()->GetConditionText(hash);
+  return GetOptionsSpecifyingKind(BreakpointOptions::eCondition)
+      ->GetConditionText(hash);
 }
 
 bool BreakpointLocation::ConditionSaysStop(ExecutionContext &exe_ctx,
@@ -305,7 +327,8 @@ bool BreakpointLocation::ConditionSaysSt
 }
 
 uint32_t BreakpointLocation::GetIgnoreCount() {
-  return GetOptionsNoCreate()->GetIgnoreCount();
+  return GetOptionsSpecifyingKind(BreakpointOptions::eIgnoreCount)
+      ->GetIgnoreCount();
 }
 
 void BreakpointLocation::SetIgnoreCount(uint32_t n) {
@@ -335,26 +358,21 @@ bool BreakpointLocation::IgnoreCountShou
   return true;
 }
 
-const BreakpointOptions *BreakpointLocation::GetOptionsNoCreate() const {
-  if (m_options_ap.get() != nullptr)
-    return m_options_ap.get();
-  else
-    return m_owner.GetOptions();
-}
-
 BreakpointOptions *BreakpointLocation::GetLocationOptions() {
   // If we make the copy we don't copy the callbacks because that is potentially
   // expensive and we don't want to do that for the simple case where someone is
   // just disabling the location.
   if (m_options_ap.get() == nullptr)
     m_options_ap.reset(
-        BreakpointOptions::CopyOptionsNoCallback(*m_owner.GetOptions()));
+        new BreakpointOptions(false));
 
   return m_options_ap.get();
 }
 
 bool BreakpointLocation::ValidForThisThread(Thread *thread) {
-  return thread->MatchesSpec(GetOptionsNoCreate()->GetThreadSpecNoCreate());
+  return thread
+      ->MatchesSpec(GetOptionsSpecifyingKind(BreakpointOptions::eThreadSpec)
+      ->GetThreadSpecNoCreate());
 }
 
 // RETURNS - true if we should stop at this breakpoint, false if we
@@ -600,17 +618,20 @@ void BreakpointLocation::Dump(Stream *s)
   if (s == nullptr)
     return;
 
+  lldb::tid_t tid = GetOptionsSpecifyingKind(BreakpointOptions::eThreadSpec)
+      ->GetThreadSpecNoCreate()->GetTID();
   s->Printf(
       "BreakpointLocation %u: tid = %4.4" PRIx64 "  load addr = 0x%8.8" PRIx64
       "  state = %s  type = %s breakpoint  "
       "hw_index = %i  hit_count = %-4u  ignore_count = %-4u",
-      GetID(), GetOptionsNoCreate()->GetThreadSpecNoCreate()->GetTID(),
+      GetID(), tid,
       (uint64_t)m_address.GetOpcodeLoadAddress(&m_owner.GetTarget()),
       (m_options_ap.get() ? m_options_ap->IsEnabled() : m_owner.IsEnabled())
           ? "enabled "
           : "disabled",
       IsHardware() ? "hardware" : "software", GetHardwareIndex(), GetHitCount(),
-      GetOptionsNoCreate()->GetIgnoreCount());
+      GetOptionsSpecifyingKind(BreakpointOptions::eIgnoreCount)
+          ->GetIgnoreCount());
 }
 
 void BreakpointLocation::SendBreakpointLocationChangedEvent(

Modified: lldb/trunk/source/Breakpoint/BreakpointOptions.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Breakpoint/BreakpointOptions.cpp?rev=309772&r1=309771&r2=309772&view=diff
==============================================================================
--- lldb/trunk/source/Breakpoint/BreakpointOptions.cpp (original)
+++ lldb/trunk/source/Breakpoint/BreakpointOptions.cpp Tue Aug  1 17:16:10 2017
@@ -126,11 +126,15 @@ bool BreakpointOptions::NullCallback(voi
 //----------------------------------------------------------------------
 // BreakpointOptions constructor
 //----------------------------------------------------------------------
-BreakpointOptions::BreakpointOptions()
+BreakpointOptions::BreakpointOptions(bool all_flags_set)
     : m_callback(BreakpointOptions::NullCallback), m_callback_baton_sp(),
       m_baton_is_command_baton(false), m_callback_is_synchronous(false),
       m_enabled(true), m_one_shot(false), m_ignore_count(0), m_thread_spec_ap(),
-      m_condition_text(), m_condition_text_hash(0) {}
+      m_condition_text(), m_condition_text_hash(0), 
+      m_set_flags() {
+        if (all_flags_set)
+          m_set_flags.Set(~((Flags::ValueType) 0));
+      }
 
 BreakpointOptions::BreakpointOptions(const char *condition, bool enabled,
                                      int32_t ignore, bool one_shot)
@@ -138,8 +142,9 @@ BreakpointOptions::BreakpointOptions(con
       m_callback_is_synchronous(false), m_enabled(enabled),
       m_one_shot(one_shot), m_ignore_count(ignore), m_condition_text(condition),
       m_condition_text_hash(0)
-
-{}
+{
+    m_set_flags.Set(eEnabled | eIgnoreCount | eOneShot | eCondition);
+}
 
 //----------------------------------------------------------------------
 // BreakpointOptions copy constructor
@@ -149,7 +154,8 @@ BreakpointOptions::BreakpointOptions(con
       m_baton_is_command_baton(rhs.m_baton_is_command_baton),
       m_callback_is_synchronous(rhs.m_callback_is_synchronous),
       m_enabled(rhs.m_enabled), m_one_shot(rhs.m_one_shot),
-      m_ignore_count(rhs.m_ignore_count), m_thread_spec_ap() {
+      m_ignore_count(rhs.m_ignore_count), m_thread_spec_ap(),
+      m_set_flags(rhs.m_set_flags) {
   if (rhs.m_thread_spec_ap.get() != nullptr)
     m_thread_spec_ap.reset(new ThreadSpec(*rhs.m_thread_spec_ap.get()));
   m_condition_text = rhs.m_condition_text;
@@ -172,23 +178,10 @@ operator=(const BreakpointOptions &rhs)
     m_thread_spec_ap.reset(new ThreadSpec(*rhs.m_thread_spec_ap.get()));
   m_condition_text = rhs.m_condition_text;
   m_condition_text_hash = rhs.m_condition_text_hash;
+  m_set_flags = rhs.m_set_flags;
   return *this;
 }
 
-BreakpointOptions *
-BreakpointOptions::CopyOptionsNoCallback(BreakpointOptions &orig) {
-  BreakpointHitCallback orig_callback = orig.m_callback;
-  lldb::BatonSP orig_callback_baton_sp = orig.m_callback_baton_sp;
-  bool orig_is_sync = orig.m_callback_is_synchronous;
-
-  orig.ClearCallback();
-  BreakpointOptions *ret_val = new BreakpointOptions(orig);
-
-  orig.SetCallback(orig_callback, orig_callback_baton_sp, orig_is_sync);
-
-  return ret_val;
-}
-
 //----------------------------------------------------------------------
 // Destructor
 //----------------------------------------------------------------------
@@ -200,29 +193,52 @@ std::unique_ptr<BreakpointOptions> Break
   bool enabled = true;
   bool one_shot = false;
   int32_t ignore_count = 0;
-  std::string condition_text;
+  llvm::StringRef condition_ref("");
+  Flags set_options;
 
-  bool success = options_dict.GetValueForKeyAsBoolean(
-      GetKey(OptionNames::EnabledState), enabled);
-  if (!success) {
-    error.SetErrorStringWithFormat("%s key is not a boolean.",
+  const char *key = GetKey(OptionNames::EnabledState);
+  bool success;
+  if (key) {
+    success = options_dict.GetValueForKeyAsBoolean(key, enabled);
+    if (!success) {
+      error.SetErrorStringWithFormat("%s key is not a boolean.",
                                    GetKey(OptionNames::EnabledState));
-    return nullptr;
+      return nullptr;
+    }
+    set_options.Set(eEnabled);
   }
 
-  success = options_dict.GetValueForKeyAsBoolean(
-      GetKey(OptionNames::OneShotState), one_shot);
-  if (!success) {
-    error.SetErrorStringWithFormat("%s key is not a boolean.",
-                                   GetKey(OptionNames::OneShotState));
-    return nullptr;
+  key = GetKey(OptionNames::OneShotState);
+  if (key) {
+    success = options_dict.GetValueForKeyAsBoolean(key, one_shot);
+    if (!success) {
+      error.SetErrorStringWithFormat("%s key is not a boolean.",
+                                     GetKey(OptionNames::OneShotState));
+      return nullptr;
+      }
+      set_options.Set(eOneShot);
   }
-  success = options_dict.GetValueForKeyAsInteger(
-      GetKey(OptionNames::IgnoreCount), ignore_count);
-  if (!success) {
-    error.SetErrorStringWithFormat("%s key is not an integer.",
-                                   GetKey(OptionNames::IgnoreCount));
-    return nullptr;
+  
+  key = GetKey(OptionNames::IgnoreCount);
+  if (key) {
+    success = options_dict.GetValueForKeyAsInteger(key, ignore_count);
+    if (!success) {
+      error.SetErrorStringWithFormat("%s key is not an integer.",
+                                     GetKey(OptionNames::IgnoreCount));
+      return nullptr;
+    }
+    set_options.Set(eIgnoreCount);
+  }
+
+  key = GetKey(OptionNames::ConditionText);
+  if (key) {
+    success = options_dict.GetValueForKeyAsString(key, condition_ref);
+    if (!success) {
+      error.SetErrorStringWithFormat("%s key is not an string.",
+                                     GetKey(OptionNames::ConditionText));
+      return nullptr;
+    }
+    set_options.Set(eCondition);
   }
 
   std::unique_ptr<CommandData> cmd_data_up;
@@ -241,7 +257,7 @@ std::unique_ptr<BreakpointOptions> Break
   }
 
   auto bp_options = llvm::make_unique<BreakpointOptions>(
-      condition_text.c_str(), enabled, ignore_count, one_shot);
+      condition_ref.str().c_str(), enabled, ignore_count, one_shot);
   if (cmd_data_up.get()) {
     if (cmd_data_up->interpreter == eScriptLanguageNone)
       bp_options->SetCommandDataCallback(cmd_data_up);
@@ -293,14 +309,20 @@ std::unique_ptr<BreakpointOptions> Break
 StructuredData::ObjectSP BreakpointOptions::SerializeToStructuredData() {
   StructuredData::DictionarySP options_dict_sp(
       new StructuredData::Dictionary());
-  options_dict_sp->AddBooleanItem(GetKey(OptionNames::EnabledState), m_enabled);
-  options_dict_sp->AddBooleanItem(GetKey(OptionNames::OneShotState),
-                                  m_one_shot);
-  options_dict_sp->AddIntegerItem(GetKey(OptionNames::IgnoreCount),
-                                  m_ignore_count);
-  options_dict_sp->AddStringItem(GetKey(OptionNames::ConditionText),
-                                 m_condition_text);
-  if (m_baton_is_command_baton) {
+  if (m_set_flags.Set(eEnabled))
+    options_dict_sp->AddBooleanItem(GetKey(OptionNames::EnabledState),
+                                    m_enabled);
+  if (m_set_flags.Set(eOneShot))
+    options_dict_sp->AddBooleanItem(GetKey(OptionNames::OneShotState),
+                               m_one_shot);
+  if (m_set_flags.Set(eIgnoreCount))
+    options_dict_sp->AddIntegerItem(GetKey(OptionNames::IgnoreCount),
+                                    m_ignore_count);
+  if (m_set_flags.Set(eCondition))
+    options_dict_sp->AddStringItem(GetKey(OptionNames::ConditionText),
+                                   m_condition_text);
+         
+  if (m_set_flags.Set(eCallback) && m_baton_is_command_baton) {
     auto cmd_baton =
         std::static_pointer_cast<CommandBaton>(m_callback_baton_sp);
     StructuredData::ObjectSP commands_sp =
@@ -310,7 +332,7 @@ StructuredData::ObjectSP BreakpointOptio
           BreakpointOptions::CommandData::GetSerializationKey(), commands_sp);
     }
   }
-  if (m_thread_spec_ap) {
+  if (m_set_flags.Set(eThreadSpec) && m_thread_spec_ap) {
     StructuredData::ObjectSP thread_spec_sp =
         m_thread_spec_ap->SerializeToStructuredData();
     options_dict_sp->AddItem(ThreadSpec::GetSerializationKey(), thread_spec_sp);
@@ -340,6 +362,7 @@ void BreakpointOptions::SetCallback(Brea
   m_callback = callback;
   m_callback_baton_sp = callback_baton_sp;
   m_baton_is_command_baton = false;
+  m_set_flags.Set(eCallback);
 }
 
 void BreakpointOptions::SetCallback(
@@ -350,6 +373,7 @@ void BreakpointOptions::SetCallback(
   m_callback = callback;
   m_callback_baton_sp = callback_baton_sp;
   m_baton_is_command_baton = true;
+  m_set_flags.Set(eCallback);
 }
 
 void BreakpointOptions::ClearCallback() {
@@ -357,6 +381,7 @@ void BreakpointOptions::ClearCallback()
   m_callback_is_synchronous = false;
   m_callback_baton_sp.reset();
   m_baton_is_command_baton = false;
+  m_set_flags.Clear(eCallback);
 }
 
 Baton *BreakpointOptions::GetBaton() { return m_callback_baton_sp.get(); }
@@ -395,8 +420,12 @@ bool BreakpointOptions::GetCommandLineCa
 }
 
 void BreakpointOptions::SetCondition(const char *condition) {
-  if (!condition)
+  if (!condition || condition[0] == '\0') {
     condition = "";
+    m_set_flags.Clear(eCondition);
+  }
+  else
+    m_set_flags.Set(eCondition);
 
   m_condition_text.assign(condition);
   std::hash<std::string> hasher;
@@ -427,11 +456,13 @@ ThreadSpec *BreakpointOptions::GetThread
 
 void BreakpointOptions::SetThreadID(lldb::tid_t thread_id) {
   GetThreadSpec()->SetTID(thread_id);
+  m_set_flags.Set(eThreadSpec);
 }
 
 void BreakpointOptions::SetThreadSpec(
     std::unique_ptr<ThreadSpec> &thread_spec_up) {
   m_thread_spec_ap = std::move(thread_spec_up);
+  m_set_flags.Set(eThreadSpec);
 }
 
 void BreakpointOptions::GetDescription(Stream *s,
@@ -520,6 +551,7 @@ void BreakpointOptions::SetCommandDataCa
   cmd_data->interpreter = eScriptLanguageNone;
   auto baton_sp = std::make_shared<CommandBaton>(std::move(cmd_data));
   SetCallback(BreakpointOptions::BreakpointOptionsCallbackFunction, baton_sp);
+  m_set_flags.Set(eCallback);
 }
 
 bool BreakpointOptions::BreakpointOptionsCallbackFunction(

Modified: lldb/trunk/source/Commands/CommandObjectBreakpointCommand.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Commands/CommandObjectBreakpointCommand.cpp?rev=309772&r1=309771&r2=309772&view=diff
==============================================================================
--- lldb/trunk/source/Commands/CommandObjectBreakpointCommand.cpp (original)
+++ lldb/trunk/source/Commands/CommandObjectBreakpointCommand.cpp Tue Aug  1 17:16:10 2017
@@ -673,48 +673,49 @@ protected:
               target->GetBreakpointByID(cur_bp_id.GetBreakpointID()).get();
 
           if (bp) {
-            const BreakpointOptions *bp_options = nullptr;
+            BreakpointLocationSP bp_loc_sp;
             if (cur_bp_id.GetLocationID() != LLDB_INVALID_BREAK_ID) {
-              BreakpointLocationSP bp_loc_sp(
-                  bp->FindLocationByID(cur_bp_id.GetLocationID()));
-              if (bp_loc_sp)
-                bp_options = bp_loc_sp->GetOptionsNoCreate();
-              else {
+                  bp_loc_sp = bp->FindLocationByID(cur_bp_id.GetLocationID());
+              if (!bp_loc_sp)
+              {
                 result.AppendErrorWithFormat("Invalid breakpoint ID: %u.%u.\n",
                                              cur_bp_id.GetBreakpointID(),
                                              cur_bp_id.GetLocationID());
                 result.SetStatus(eReturnStatusFailed);
                 return false;
               }
-            } else {
-              bp_options = bp->GetOptions();
             }
 
-            if (bp_options) {
-              StreamString id_str;
-              BreakpointID::GetCanonicalReference(&id_str,
-                                                  cur_bp_id.GetBreakpointID(),
-                                                  cur_bp_id.GetLocationID());
-              const Baton *baton = bp_options->GetBaton();
-              if (baton) {
-                result.GetOutputStream().Printf("Breakpoint %s:\n",
-                                                id_str.GetData());
-                result.GetOutputStream().IndentMore();
-                baton->GetDescription(&result.GetOutputStream(),
-                                      eDescriptionLevelFull);
-                result.GetOutputStream().IndentLess();
-              } else {
-                result.AppendMessageWithFormat(
-                    "Breakpoint %s does not have an associated command.\n",
-                    id_str.GetData());
-              }
+            StreamString id_str;
+            BreakpointID::GetCanonicalReference(&id_str,
+                                                cur_bp_id.GetBreakpointID(),
+                                                cur_bp_id.GetLocationID());
+            const Baton *baton = nullptr;
+            if (bp_loc_sp)
+              baton = bp_loc_sp
+               ->GetOptionsSpecifyingKind(BreakpointOptions::eCallback)
+               ->GetBaton();
+            else
+              baton = bp->GetOptions()->GetBaton();
+
+            if (baton) {
+              result.GetOutputStream().Printf("Breakpoint %s:\n",
+                                              id_str.GetData());
+              result.GetOutputStream().IndentMore();
+              baton->GetDescription(&result.GetOutputStream(),
+                                    eDescriptionLevelFull);
+              result.GetOutputStream().IndentLess();
+            } else {
+              result.AppendMessageWithFormat(
+                  "Breakpoint %s does not have an associated command.\n",
+                  id_str.GetData());
             }
-            result.SetStatus(eReturnStatusSuccessFinishResult);
-          } else {
-            result.AppendErrorWithFormat("Invalid breakpoint ID: %u.\n",
-                                         cur_bp_id.GetBreakpointID());
-            result.SetStatus(eReturnStatusFailed);
           }
+          result.SetStatus(eReturnStatusSuccessFinishResult);
+        } else {
+          result.AppendErrorWithFormat("Invalid breakpoint ID: %u.\n",
+                                       cur_bp_id.GetBreakpointID());
+          result.SetStatus(eReturnStatusFailed);
         }
       }
     }




More information about the lldb-commits mailing list