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

3 files changed, 124 insertions, 71 deletions

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(