[Lldb-commits] [lldb] b624b7d - [lldb] Make shared_from_this-related code safer

Tatyana Krasnukha via lldb-commits lldb-commits at lists.llvm.org
Tue Feb 18 02:49:33 PST 2020


Author: Tatyana Krasnukha
Date: 2020-02-18T13:49:07+03:00
New Revision: b624b7dfd087809fb58bff0737750e75375fe450

URL: https://github.com/llvm/llvm-project/commit/b624b7dfd087809fb58bff0737750e75375fe450
DIFF: https://github.com/llvm/llvm-project/commit/b624b7dfd087809fb58bff0737750e75375fe450.diff

LOG: [lldb] Make shared_from_this-related code safer

Pass TargetSP to filters' CreateFromStructuredData, don't let them guess
whether target object is managed by a shared_ptr.

Make Breakpoint sure that m_target.shared_from_this() is safe by passing TargetSP
to all its static Create*** member-functions. This should be enough, since Breakpoint's
constructors are private/protected and never called directly (except by Target itself).

Added: 
    

Modified: 
    lldb/include/lldb/Breakpoint/Breakpoint.h
    lldb/include/lldb/Core/SearchFilter.h
    lldb/source/Breakpoint/Breakpoint.cpp
    lldb/source/Core/SearchFilter.cpp
    lldb/source/Target/Target.cpp

Removed: 
    


################################################################################
diff  --git a/lldb/include/lldb/Breakpoint/Breakpoint.h b/lldb/include/lldb/Breakpoint/Breakpoint.h
index c48a0ad73d4e..bdff7772fc6b 100644
--- a/lldb/include/lldb/Breakpoint/Breakpoint.h
+++ b/lldb/include/lldb/Breakpoint/Breakpoint.h
@@ -142,7 +142,8 @@ class Breakpoint : public std::enable_shared_from_this<Breakpoint>,
 
   // Saving & restoring breakpoints:
   static lldb::BreakpointSP CreateFromStructuredData(
-      Target &target, StructuredData::ObjectSP &data_object_sp, Status &error);
+      lldb::TargetSP target_sp, StructuredData::ObjectSP &data_object_sp,
+      Status &error);
 
   static bool
   SerializedBreakpointMatchesNames(StructuredData::ObjectSP &bkpt_object_sp,
@@ -570,7 +571,7 @@ class Breakpoint : public std::enable_shared_from_this<Breakpoint>,
 
   // This one should only be used by Target to copy breakpoints from target to
   // target - primarily from the dummy target to prime new targets.
-  static lldb::BreakpointSP CopyFromBreakpoint(Target& new_target,
+  static lldb::BreakpointSP CopyFromBreakpoint(lldb::TargetSP new_target,
       const Breakpoint &bp_to_copy_from);
 
 protected:

diff  --git a/lldb/include/lldb/Core/SearchFilter.h b/lldb/include/lldb/Core/SearchFilter.h
index ab5dc962230f..8846ec0cf922 100644
--- a/lldb/include/lldb/Core/SearchFilter.h
+++ b/lldb/include/lldb/Core/SearchFilter.h
@@ -190,7 +190,7 @@ class SearchFilter {
   lldb::SearchFilterSP CopyForBreakpoint(Breakpoint &breakpoint);
 
   static lldb::SearchFilterSP
-  CreateFromStructuredData(Target &target,
+  CreateFromStructuredData(const lldb::TargetSP& target_sp,
                            const StructuredData::Dictionary &data_dict,
                            Status &error);
 
@@ -288,7 +288,7 @@ class SearchFilterForUnconstrainedSearches : public SearchFilter {
   bool ModulePasses(const lldb::ModuleSP &module_sp) override;
 
   static lldb::SearchFilterSP
-  CreateFromStructuredData(Target &target,
+  CreateFromStructuredData(const lldb::TargetSP& target_sp,
                            const StructuredData::Dictionary &data_dict,
                            Status &error);
 
@@ -334,7 +334,7 @@ class SearchFilterByModule : public SearchFilter {
   void Search(Searcher &searcher) override;
 
   static lldb::SearchFilterSP
-  CreateFromStructuredData(Target &target,
+  CreateFromStructuredData(const lldb::TargetSP& target_sp,
                            const StructuredData::Dictionary &data_dict,
                            Status &error);
 
@@ -385,7 +385,7 @@ class SearchFilterByModuleList : public SearchFilter {
   void Search(Searcher &searcher) override;
 
   static lldb::SearchFilterSP
-  CreateFromStructuredData(Target &target,
+  CreateFromStructuredData(const lldb::TargetSP& target_sp,
                            const StructuredData::Dictionary &data_dict,
                            Status &error);
 
@@ -425,7 +425,7 @@ class SearchFilterByModuleListAndCU : public SearchFilterByModuleList {
   void Search(Searcher &searcher) override;
 
   static lldb::SearchFilterSP
-  CreateFromStructuredData(Target &target,
+  CreateFromStructuredData(const lldb::TargetSP& target_sp,
                            const StructuredData::Dictionary &data_dict,
                            Status &error);
 

diff  --git a/lldb/source/Breakpoint/Breakpoint.cpp b/lldb/source/Breakpoint/Breakpoint.cpp
index 39c6828d3f09..09122cfc2bfa 100644
--- a/lldb/source/Breakpoint/Breakpoint.cpp
+++ b/lldb/source/Breakpoint/Breakpoint.cpp
@@ -66,9 +66,12 @@ Breakpoint::Breakpoint(Target &new_target, const Breakpoint &source_bp)
 // Destructor
 Breakpoint::~Breakpoint() = default;
 
-BreakpointSP Breakpoint::CopyFromBreakpoint(Target& new_target,
+BreakpointSP Breakpoint::CopyFromBreakpoint(TargetSP new_target,
     const Breakpoint& bp_to_copy_from) {
-  BreakpointSP bp(new Breakpoint(new_target, bp_to_copy_from));
+  if (!new_target)
+    return BreakpointSP();
+
+  BreakpointSP bp(new Breakpoint(*new_target, bp_to_copy_from));
   // Now go through and copy the filter & resolver:
   bp->m_resolver_sp = bp_to_copy_from.m_resolver_sp->CopyForBreakpoint(*bp);
   bp->m_filter_sp = bp_to_copy_from.m_filter_sp->CopyForBreakpoint(*bp);
@@ -125,8 +128,10 @@ StructuredData::ObjectSP Breakpoint::SerializeToStructuredData() {
 }
 
 lldb::BreakpointSP Breakpoint::CreateFromStructuredData(
-    Target &target, StructuredData::ObjectSP &object_data, Status &error) {
+    TargetSP target_sp, StructuredData::ObjectSP &object_data, Status &error) {
   BreakpointSP result_sp;
+  if (!target_sp)
+    return result_sp;
 
   StructuredData::Dictionary *breakpoint_dict = object_data->GetAsDictionary();
 
@@ -160,11 +165,11 @@ lldb::BreakpointSP Breakpoint::CreateFromStructuredData(
       SearchFilter::GetSerializationKey(), filter_dict);
   SearchFilterSP filter_sp;
   if (!success)
-    filter_sp = std::make_shared<SearchFilterForUnconstrainedSearches>(
-        target.shared_from_this());
+    filter_sp =
+        std::make_shared<SearchFilterForUnconstrainedSearches>(target_sp);
   else {
-    filter_sp = SearchFilter::CreateFromStructuredData(target, *filter_dict,
-                                                       create_error);
+    filter_sp = SearchFilter::CreateFromStructuredData(target_sp, *filter_dict,
+        create_error);
     if (create_error.Fail()) {
       error.SetErrorStringWithFormat(
           "Error creating breakpoint filter from data: %s.",
@@ -175,6 +180,7 @@ lldb::BreakpointSP Breakpoint::CreateFromStructuredData(
 
   std::unique_ptr<BreakpointOptions> options_up;
   StructuredData::Dictionary *options_dict;
+  Target& target = *target_sp;
   success = breakpoint_dict->GetValueForKeyAsDictionary(
       BreakpointOptions::GetSerializationKey(), options_dict);
   if (success) {
@@ -192,8 +198,8 @@ lldb::BreakpointSP Breakpoint::CreateFromStructuredData(
   success = breakpoint_dict->GetValueForKeyAsBoolean(
       Breakpoint::GetKey(OptionNames::Hardware), hardware);
 
-  result_sp =
-      target.CreateBreakpoint(filter_sp, resolver_sp, false, hardware, true);
+  result_sp = target.CreateBreakpoint(filter_sp, resolver_sp, false,
+                                      hardware, true);
 
   if (result_sp && options_up) {
     result_sp->m_options_up = std::move(options_up);

diff  --git a/lldb/source/Core/SearchFilter.cpp b/lldb/source/Core/SearchFilter.cpp
index 64d22797a224..a42fa968656f 100644
--- a/lldb/source/Core/SearchFilter.cpp
+++ b/lldb/source/Core/SearchFilter.cpp
@@ -75,7 +75,8 @@ SearchFilter::SearchFilter(const TargetSP &target_sp, unsigned char filterType)
 SearchFilter::~SearchFilter() = default;
 
 SearchFilterSP SearchFilter::CreateFromStructuredData(
-    Target &target, const StructuredData::Dictionary &filter_dict,
+    const lldb::TargetSP& target_sp,
+    const StructuredData::Dictionary &filter_dict,
     Status &error) {
   SearchFilterSP result_sp;
   if (!filter_dict.IsValid()) {
@@ -109,19 +110,19 @@ SearchFilterSP SearchFilter::CreateFromStructuredData(
   switch (filter_type) {
   case Unconstrained:
     result_sp = SearchFilterForUnconstrainedSearches::CreateFromStructuredData(
-        target, *subclass_options, error);
+        target_sp, *subclass_options, error);
     break;
   case ByModule:
     result_sp = SearchFilterByModule::CreateFromStructuredData(
-        target, *subclass_options, error);
+        target_sp, *subclass_options, error);
     break;
   case ByModules:
     result_sp = SearchFilterByModuleList::CreateFromStructuredData(
-        target, *subclass_options, error);
+        target_sp, *subclass_options, error);
     break;
   case ByModulesAndCU:
     result_sp = SearchFilterByModuleListAndCU::CreateFromStructuredData(
-        target, *subclass_options, error);
+        target_sp, *subclass_options, error);
     break;
   case Exception:
     error.SetErrorString("Can't serialize exception breakpoints yet.");
@@ -212,7 +213,7 @@ void SearchFilter::Search(Searcher &searcher) {
     searcher.SearchCallback(*this, empty_sc, nullptr);
     return;
   }
-  
+
   DoModuleIteration(empty_sc, searcher);
 }
 
@@ -362,11 +363,11 @@ Searcher::CallbackReturn SearchFilter::DoFunctionIteration(
 //  Selects a shared library matching a given file spec, consulting the targets
 //  "black list".
 SearchFilterSP SearchFilterForUnconstrainedSearches::CreateFromStructuredData(
-    Target &target, const StructuredData::Dictionary &data_dict,
+    const lldb::TargetSP& target_sp,
+    const StructuredData::Dictionary &data_dict,
     Status &error) {
   // No options for an unconstrained search.
-  return std::make_shared<SearchFilterForUnconstrainedSearches>(
-      target.shared_from_this());
+  return std::make_shared<SearchFilterForUnconstrainedSearches>(target_sp);
 }
 
 StructuredData::ObjectSP
@@ -472,7 +473,8 @@ SearchFilterByModule::DoCopyForBreakpoint(Breakpoint &breakpoint) {
 }
 
 SearchFilterSP SearchFilterByModule::CreateFromStructuredData(
-    Target &target, const StructuredData::Dictionary &data_dict,
+    const lldb::TargetSP& target_sp,
+    const StructuredData::Dictionary &data_dict,
     Status &error) {
   StructuredData::Array *modules_array;
   bool success = data_dict.GetValueForKeyAsArray(GetKey(OptionNames::ModList),
@@ -497,8 +499,7 @@ SearchFilterSP SearchFilterByModule::CreateFromStructuredData(
   }
   FileSpec module_spec(module);
 
-  return std::make_shared<SearchFilterByModule>(target.shared_from_this(),
-                                                module_spec);
+  return std::make_shared<SearchFilterByModule>(target_sp, module_spec);
 }
 
 StructuredData::ObjectSP SearchFilterByModule::SerializeToStructuredData() {
@@ -617,14 +618,15 @@ SearchFilterByModuleList::DoCopyForBreakpoint(Breakpoint &breakpoint) {
 }
 
 SearchFilterSP SearchFilterByModuleList::CreateFromStructuredData(
-    Target &target, const StructuredData::Dictionary &data_dict,
+    const lldb::TargetSP& target_sp,
+    const StructuredData::Dictionary &data_dict,
     Status &error) {
   StructuredData::Array *modules_array;
   bool success = data_dict.GetValueForKeyAsArray(GetKey(OptionNames::ModList),
                                                  modules_array);
 
   if (!success)
-    return std::make_shared<SearchFilterByModuleList>(target.shared_from_this(),
+    return std::make_shared<SearchFilterByModuleList>(target_sp,
                                                       FileSpecList{});
   FileSpecList modules;
   size_t num_modules = modules_array->GetSize();
@@ -638,8 +640,7 @@ SearchFilterSP SearchFilterByModuleList::CreateFromStructuredData(
     }
     modules.EmplaceBack(module);
   }
-  return std::make_shared<SearchFilterByModuleList>(target.shared_from_this(),
-                                                    modules);
+  return std::make_shared<SearchFilterByModuleList>(target_sp, modules);
 }
 
 void SearchFilterByModuleList::SerializeUnwrapped(
@@ -667,7 +668,8 @@ SearchFilterByModuleListAndCU::SearchFilterByModuleListAndCU(
 SearchFilterByModuleListAndCU::~SearchFilterByModuleListAndCU() = default;
 
 lldb::SearchFilterSP SearchFilterByModuleListAndCU::CreateFromStructuredData(
-    Target &target, const StructuredData::Dictionary &data_dict,
+    const lldb::TargetSP& target_sp,
+    const StructuredData::Dictionary &data_dict,
     Status &error) {
   StructuredData::Array *modules_array = nullptr;
   SearchFilterSP result_sp;
@@ -710,7 +712,7 @@ lldb::SearchFilterSP SearchFilterByModuleListAndCU::CreateFromStructuredData(
   }
 
   return std::make_shared<SearchFilterByModuleListAndCU>(
-      target.shared_from_this(), modules, cus);
+      target_sp, modules, cus);
 }
 
 StructuredData::ObjectSP

diff  --git a/lldb/source/Target/Target.cpp b/lldb/source/Target/Target.cpp
index 49cb52bb0498..a867d6916f2d 100644
--- a/lldb/source/Target/Target.cpp
+++ b/lldb/source/Target/Target.cpp
@@ -131,7 +131,8 @@ void Target::PrimeFromDummyTarget(Target *target) {
     if (breakpoint_sp->IsInternal())
       continue;
 
-    BreakpointSP new_bp(Breakpoint::CopyFromBreakpoint(*this, *breakpoint_sp));
+    BreakpointSP new_bp(
+        Breakpoint::CopyFromBreakpoint(shared_from_this(), *breakpoint_sp));
     AddBreakpoint(std::move(new_bp), false);
   }
 
@@ -1108,8 +1109,8 @@ Status Target::CreateBreakpointsFromFile(const FileSpec &file,
         !Breakpoint::SerializedBreakpointMatchesNames(bkpt_data_sp, names))
       continue;
 
-    BreakpointSP bkpt_sp =
-        Breakpoint::CreateFromStructuredData(*this, bkpt_data_sp, error);
+    BreakpointSP bkpt_sp = Breakpoint::CreateFromStructuredData(
+        shared_from_this(), bkpt_data_sp, error);
     if (!error.Success()) {
       error.SetErrorStringWithFormat(
           "Error restoring breakpoint %zu from %s: %s.", i,


        


More information about the lldb-commits mailing list