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>

llvm-svn: 309772

13 files changed, 378 insertions, 128 deletions

diff --git a/lldb/include/lldb/API/SBBreakpointLocation.h b/lldb/include/lldb/API/SBBreakpointLocation.h
index 900541b0201..c8fa4a1934d 100644
--- a/lldb/include/lldb/API/SBBreakpointLocation.h
+++ b/lldb/include/lldb/API/SBBreakpointLocation.h
@@ -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();
diff --git a/lldb/include/lldb/API/SBStringList.h b/lldb/include/lldb/API/SBStringList.h
index 66b5172a0b3..c50594db07f 100644
--- a/lldb/include/lldb/API/SBStringList.h
+++ b/lldb/include/lldb/API/SBStringList.h
@@ -44,6 +44,7 @@ protected:
   friend class SBCommandInterpreter;
   friend class SBDebugger;
   friend class SBBreakpoint;
+  friend class SBBreakpointLocation;
 
   SBStringList(const lldb_private::StringList *lldb_strings);
 
diff --git a/lldb/include/lldb/Breakpoint/BreakpointLocation.h b/lldb/include/lldb/Breakpoint/BreakpointLocation.h
index a1086aa3fe5..2a94f3c2a2b 100644
--- a/lldb/include/lldb/Breakpoint/BreakpointLocation.h
+++ b/lldb/include/lldb/Breakpoint/BreakpointLocation.h
@@ -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);
 
diff --git a/lldb/include/lldb/Breakpoint/BreakpointOptions.h b/lldb/include/lldb/Breakpoint/BreakpointOptions.h
index 420d5915fd4..cb379a9103a 100644
--- a/lldb/include/lldb/Breakpoint/BreakpointOptions.h
+++ b/lldb/include/lldb/Breakpoint/BreakpointOptions.h
@@ -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
diff --git a/lldb/lldb.xcodeproj/xcshareddata/xcschemes/desktop.xcscheme b/lldb/lldb.xcodeproj/xcshareddata/xcschemes/desktop.xcscheme
index 17aedff2ce5..7512a281357 100644
--- a/lldb/lldb.xcodeproj/xcshareddata/xcschemes/desktop.xcscheme
+++ b/lldb/lldb.xcodeproj/xcshareddata/xcschemes/desktop.xcscheme
@@ -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"
diff --git a/lldb/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_locations/TestBreakpointLocations.py b/lldb/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_locations/TestBreakpointLocations.py
index fdbd622f767..de24d8b002d 100644
--- a/lldb/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_locations/TestBreakpointLocations.py
+++ b/lldb/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_locations/TestBreakpointLocations.py
@@ -18,27 +18,40 @@ class BreakpointLocationsTestCase(TestBase):
     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(TestBase):
                 "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(TestBase):
             "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)
 
diff --git a/lldb/scripts/interface/SBBreakpoint.i b/lldb/scripts/interface/SBBreakpoint.i
index 95bc0cda005..e3b1eea0630 100644
--- a/lldb/scripts/interface/SBBreakpoint.i
+++ b/lldb/scripts/interface/SBBreakpoint.i
@@ -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.''')
             
diff --git a/lldb/scripts/interface/SBBreakpointLocation.i b/lldb/scripts/interface/SBBreakpointLocation.i
index 4a5aac031b4..5609c1f84f8 100644
--- a/lldb/scripts/interface/SBBreakpointLocation.i
+++ b/lldb/scripts/interface/SBBreakpointLocation.i
@@ -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);
 
diff --git a/lldb/source/API/SBBreakpointLocation.cpp b/lldb/source/API/SBBreakpointLocation.cpp
index 3678d1d33e0..f5e0b91483e 100644
--- a/lldb/source/API/SBBreakpointLocation.cpp
+++ b/lldb/source/API/SBBreakpointLocation.cpp
@@ -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::SetScriptCallbackBody(const char *callback_body_text) {
   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) {
diff --git a/lldb/source/Breakpoint/Breakpoint.cpp b/lldb/source/Breakpoint/Breakpoint.cpp
index 17c104ba0c6..043e5e8f591 100644
--- a/lldb/source/Breakpoint/Breakpoint.cpp
+++ b/lldb/source/Breakpoint/Breakpoint.cpp
@@ -53,7 +53,7 @@ Breakpoint::Breakpoint(Target &target, SearchFilterSP &filter_sp,
                        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;
 }
diff --git a/lldb/source/Breakpoint/BreakpointLocation.cpp b/lldb/source/Breakpoint/BreakpointLocation.cpp
index ec8f141e8d3..15865be3f9a 100644
--- a/lldb/source/Breakpoint/BreakpointLocation.cpp
+++ b/lldb/source/Breakpoint/BreakpointLocation.cpp
@@ -58,6 +58,15 @@ lldb::addr_t BreakpointLocation::GetLoadAddress() const {
   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(lldb::tid_t thread_id) {
 }
 
 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 index) {
 }
 
 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(const char *thread_name) {
 }
 
 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(const char *queue_name) {
 }
 
 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(const char *condition) {
 }
 
 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::ConditionSaysStop(ExecutionContext &exe_ctx,
 }
 
 uint32_t BreakpointLocation::GetIgnoreCount() {
-  return GetOptionsNoCreate()->GetIgnoreCount();
+  return GetOptionsSpecifyingKind(BreakpointOptions::eIgnoreCount)
+      ->GetIgnoreCount();
 }
 
 void BreakpointLocation::SetIgnoreCount(uint32_t n) {
@@ -335,26 +358,21 @@ bool BreakpointLocation::IgnoreCountShouldStop() {
   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) const {
   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(
diff --git a/lldb/source/Breakpoint/BreakpointOptions.cpp b/lldb/source/Breakpoint/BreakpointOptions.cpp
index bef63cc0f22..3e8d3dd41f3 100644
--- a/lldb/source/Breakpoint/BreakpointOptions.cpp
+++ b/lldb/source/Breakpoint/BreakpointOptions.cpp
@@ -126,11 +126,15 @@ bool BreakpointOptions::NullCallback(void *baton,
 //----------------------------------------------------------------------
 // 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(const char *condition, bool enabled,
       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(const BreakpointOptions &rhs)
       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> BreakpointOptions::CreateFromStructuredData(
   bool enabled = true;
   bool one_shot = false;
   int32_t ignore_count = 0;
-  std::string condition_text;
-
-  bool success = options_dict.GetValueForKeyAsBoolean(
-      GetKey(OptionNames::EnabledState), enabled);
-  if (!success) {
-    error.SetErrorStringWithFormat("%s key is not a boolean.",
+  llvm::StringRef condition_ref("");
+  Flags set_options;
+
+  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> BreakpointOptions::CreateFromStructuredData(
   }
 
   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> BreakpointOptions::CreateFromStructuredData(
 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 BreakpointOptions::SerializeToStructuredData() {
           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(BreakpointHitCallback callback,
   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::GetCommandLineCallbacks(StringList &command_list) {
 }
 
 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::GetThreadSpec() {
 
 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::SetCommandDataCallback(
   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(
diff --git a/lldb/source/Commands/CommandObjectBreakpointCommand.cpp b/lldb/source/Commands/CommandObjectBreakpointCommand.cpp
index de491195310..ecb00f9a339 100644
--- a/lldb/source/Commands/CommandObjectBreakpointCommand.cpp
+++ b/lldb/source/Commands/CommandObjectBreakpointCommand.cpp
@@ -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);
         }
       }
     }