bulbazord created this revision.
bulbazord added reviewers: JDevlieghere, mib, jasonmolenda, jingham.
Herald added a project: All.
bulbazord requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

m_source_name is currently a ConstString. Comparing the source names are
not a performance-critical operation, so that portion isn't really
necessary. It may benefit from deduplication (since there are only
a handful of possible values) but this means they will stick around
forever in the StringPool, even if they are never used again. As such, I
think it would be reasonable to change its type to `std::string`
instead.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D154759

Files:
  lldb/include/lldb/Symbol/UnwindPlan.h
  lldb/source/Commands/CommandObjectTarget.cpp
  lldb/source/Plugins/UnwindAssembly/x86/x86AssemblyInspectionEngine.cpp
  lldb/source/Symbol/UnwindPlan.cpp
  lldb/source/Target/RegisterContextUnwind.cpp

Index: lldb/source/Target/RegisterContextUnwind.cpp
===================================================================
--- lldb/source/Target/RegisterContextUnwind.cpp
+++ lldb/source/Target/RegisterContextUnwind.cpp
@@ -300,9 +300,8 @@
   UnwindLogMsg("initialized frame current pc is 0x%" PRIx64 " cfa is 0x%" PRIx64
                " afa is 0x%" PRIx64 " using %s UnwindPlan",
                (uint64_t)m_current_pc.GetLoadAddress(exe_ctx.GetTargetPtr()),
-               (uint64_t)m_cfa,
-               (uint64_t)m_afa,
-               m_full_unwind_plan_sp->GetSourceName().GetCString());
+               (uint64_t)m_cfa, (uint64_t)m_afa,
+               m_full_unwind_plan_sp->GetSourceName().c_str());
 }
 
 // Initialize a RegisterContextUnwind for the non-zeroth frame -- rely on the
@@ -645,7 +644,7 @@
       active_row->Dump(active_row_strm, m_fast_unwind_plan_sp.get(), &m_thread,
                        m_start_pc.GetLoadAddress(exe_ctx.GetTargetPtr()));
       UnwindLogMsg("Using fast unwind plan '%s'",
-                   m_fast_unwind_plan_sp->GetSourceName().AsCString());
+                   m_fast_unwind_plan_sp->GetSourceName().c_str());
       UnwindLogMsg("active row: %s", active_row_strm.GetData());
     }
   } else {
@@ -661,7 +660,7 @@
                          &m_thread,
                          m_start_pc.GetLoadAddress(exe_ctx.GetTargetPtr()));
         UnwindLogMsg("Using full unwind plan '%s'",
-                     m_full_unwind_plan_sp->GetSourceName().AsCString());
+                     m_full_unwind_plan_sp->GetSourceName().c_str());
         UnwindLogMsg("active row: %s", active_row_strm.GetData());
       }
     }
@@ -957,7 +956,7 @@
     if (unwind_plan_sp && unwind_plan_sp->PlanValidAtAddress(m_current_pc)) {
       UnwindLogMsgVerbose("frame uses %s for full UnwindPlan because the "
                           "DynamicLoader suggested we prefer it",
-                          unwind_plan_sp->GetSourceName().GetCString());
+                          unwind_plan_sp->GetSourceName().c_str());
       return unwind_plan_sp;
     }
   }
@@ -995,7 +994,7 @@
       UnwindLogMsgVerbose("frame uses %s for full UnwindPlan because this "
                           "is the non-call site unwind plan and this is a "
                           "zeroth frame",
-                          unwind_plan_sp->GetSourceName().GetCString());
+                          unwind_plan_sp->GetSourceName().c_str());
       return unwind_plan_sp;
     }
 
@@ -1007,9 +1006,10 @@
           func_unwinders_sp->GetUnwindPlanArchitectureDefaultAtFunctionEntry(
               m_thread);
       if (unwind_plan_sp) {
-        UnwindLogMsgVerbose("frame uses %s for full UnwindPlan because we are at "
-                            "the first instruction of a function",
-                            unwind_plan_sp->GetSourceName().GetCString());
+        UnwindLogMsgVerbose(
+            "frame uses %s for full UnwindPlan because we are at "
+            "the first instruction of a function",
+            unwind_plan_sp->GetSourceName().c_str());
         return unwind_plan_sp;
       }
     }
@@ -1024,7 +1024,7 @@
   if (IsUnwindPlanValidForCurrentPC(unwind_plan_sp)) {
     UnwindLogMsgVerbose("frame uses %s for full UnwindPlan because this "
                         "is the call-site unwind plan",
-                        unwind_plan_sp->GetSourceName().GetCString());
+                        unwind_plan_sp->GetSourceName().c_str());
     return unwind_plan_sp;
   }
 
@@ -1061,9 +1061,10 @@
   }
 
   if (IsUnwindPlanValidForCurrentPC(unwind_plan_sp)) {
-    UnwindLogMsgVerbose("frame uses %s for full UnwindPlan because we "
-                        "failed to find a call-site unwind plan that would work",
-                        unwind_plan_sp->GetSourceName().GetCString());
+    UnwindLogMsgVerbose(
+        "frame uses %s for full UnwindPlan because we "
+        "failed to find a call-site unwind plan that would work",
+        unwind_plan_sp->GetSourceName().c_str());
     return unwind_plan_sp;
   }
 
@@ -1073,7 +1074,7 @@
     UnwindLogMsgVerbose(
         "frame uses %s for full UnwindPlan because we are falling back "
         "to the arch default plan",
-        arch_default_unwind_plan_sp->GetSourceName().GetCString());
+        arch_default_unwind_plan_sp->GetSourceName().c_str());
   else
     UnwindLogMsg(
         "Unable to find any UnwindPlan for full unwind of this frame.");
@@ -1337,7 +1338,7 @@
                          &m_thread,
                          m_start_pc.GetLoadAddress(exe_ctx.GetTargetPtr()));
         UnwindLogMsg("Using full unwind plan '%s'",
-                     m_full_unwind_plan_sp->GetSourceName().AsCString());
+                     m_full_unwind_plan_sp->GetSourceName().c_str());
         UnwindLogMsg("active row: %s", active_row_strm.GetData());
       }
       RegisterNumber return_address_reg;
@@ -1393,7 +1394,7 @@
         UnwindLogMsg(
             "supplying caller's saved %s (%d)'s location using %s UnwindPlan",
             regnum.GetName(), regnum.GetAsKind(eRegisterKindLLDB),
-            m_full_unwind_plan_sp->GetSourceName().GetCString());
+            m_full_unwind_plan_sp->GetSourceName().c_str());
       }
 
       // This is frame 0 and we're retrieving the PC and it's saved in a Return
@@ -1448,7 +1449,7 @@
           !m_all_registers_available) {
         UnwindLogMsg("%s UnwindPlan tried to restore the pc from the link "
                      "register but this is a non-zero frame",
-                     m_full_unwind_plan_sp->GetSourceName().GetCString());
+                     m_full_unwind_plan_sp->GetSourceName().c_str());
 
         // Throw away the full unwindplan; install the arch default unwindplan
         if (ForceSwitchToFallbackUnwindPlan()) {
@@ -1525,11 +1526,9 @@
       return UnwindLLDB::RegisterSearchResult::eRegisterFound;
     } else {
       std::string unwindplan_name;
-      if (m_full_unwind_plan_sp) {
-        unwindplan_name += "via '";
-        unwindplan_name += m_full_unwind_plan_sp->GetSourceName().AsCString();
-        unwindplan_name += "'";
-      }
+      if (m_full_unwind_plan_sp)
+        unwindplan_name +=
+            "via '" + m_full_unwind_plan_sp->GetSourceName() + "'";
       UnwindLogMsg("no save location for %s (%d) %s", regnum.GetName(),
                    regnum.GetAsKind(eRegisterKindLLDB),
                    unwindplan_name.c_str());
@@ -1845,8 +1844,8 @@
 
     UnwindLogMsg("trying to unwind from this function with the UnwindPlan '%s' "
                  "because UnwindPlan '%s' failed.",
-                 m_fallback_unwind_plan_sp->GetSourceName().GetCString(),
-                 original_full_unwind_plan_sp->GetSourceName().GetCString());
+                 m_fallback_unwind_plan_sp->GetSourceName().c_str(),
+                 original_full_unwind_plan_sp->GetSourceName().c_str());
 
     // We've copied the fallback unwind plan into the full - now clear the
     // fallback.
@@ -1898,7 +1897,7 @@
     PropagateTrapHandlerFlagFromUnwindPlan(m_full_unwind_plan_sp);
 
     UnwindLogMsg("switched unconditionally to the fallback unwindplan %s",
-                 m_full_unwind_plan_sp->GetSourceName().GetCString());
+                 m_full_unwind_plan_sp->GetSourceName().c_str());
     return true;
   }
   return false;
Index: lldb/source/Symbol/UnwindPlan.cpp
===================================================================
--- lldb/source/Symbol/UnwindPlan.cpp
+++ lldb/source/Symbol/UnwindPlan.cpp
@@ -12,7 +12,6 @@
 #include "lldb/Target/RegisterContext.h"
 #include "lldb/Target/Target.h"
 #include "lldb/Target/Thread.h"
-#include "lldb/Utility/ConstString.h"
 #include "lldb/Utility/LLDBLog.h"
 #include "lldb/Utility/Log.h"
 #include "llvm/DebugInfo/DIContext.h"
@@ -443,11 +442,11 @@
         LLDB_LOGF(log,
                   "UnwindPlan is invalid -- no unwind rows for UnwindPlan "
                   "'%s' at address %s",
-                  m_source_name.GetCString(), s.GetData());
+                  m_source_name.c_str(), s.GetData());
       } else {
         LLDB_LOGF(log,
                   "UnwindPlan is invalid -- no unwind rows for UnwindPlan '%s'",
-                  m_source_name.GetCString());
+                  m_source_name.c_str());
       }
     }
     return false;
@@ -466,12 +465,12 @@
         LLDB_LOGF(log,
                   "UnwindPlan is invalid -- no CFA register defined in row 0 "
                   "for UnwindPlan '%s' at address %s",
-                  m_source_name.GetCString(), s.GetData());
+                  m_source_name.c_str(), s.GetData());
       } else {
         LLDB_LOGF(log,
                   "UnwindPlan is invalid -- no CFA register defined in row 0 "
                   "for UnwindPlan '%s'",
-                  m_source_name.GetCString());
+                  m_source_name.c_str());
       }
     }
     return false;
@@ -491,9 +490,9 @@
 }
 
 void UnwindPlan::Dump(Stream &s, Thread *thread, lldb::addr_t base_addr) const {
-  if (!m_source_name.IsEmpty()) {
+  if (!m_source_name.empty()) {
     s.Printf("This UnwindPlan originally sourced from %s\n",
-             m_source_name.GetCString());
+             m_source_name.c_str());
   }
   if (m_lsda_address.IsValid() && m_personality_func_addr.IsValid()) {
     TargetSP target_sp(thread->CalculateTarget());
@@ -561,11 +560,11 @@
   }
 }
 
-void UnwindPlan::SetSourceName(const char *source) {
-  m_source_name = ConstString(source);
+void UnwindPlan::SetSourceName(std::string name) {
+  m_source_name = std::move(name);
 }
 
-ConstString UnwindPlan::GetSourceName() const { return m_source_name; }
+const std::string &UnwindPlan::GetSourceName() const { return m_source_name; }
 
 const RegisterInfo *UnwindPlan::GetRegisterInfo(Thread *thread,
                                                 uint32_t unwind_reg) const {
Index: lldb/source/Plugins/UnwindAssembly/x86/x86AssemblyInspectionEngine.cpp
===================================================================
--- lldb/source/Plugins/UnwindAssembly/x86/x86AssemblyInspectionEngine.cpp
+++ lldb/source/Plugins/UnwindAssembly/x86/x86AssemblyInspectionEngine.cpp
@@ -1554,9 +1554,9 @@
 
   unwind_plan.SetPlanValidAddressRange(func_range);
   if (unwind_plan_updated) {
-    std::string unwind_plan_source(unwind_plan.GetSourceName().AsCString());
+    std::string unwind_plan_source(unwind_plan.GetSourceName());
     unwind_plan_source += " plus augmentation from assembly parsing";
-    unwind_plan.SetSourceName(unwind_plan_source.c_str());
+    unwind_plan.SetSourceName(std::move(unwind_plan_source));
     unwind_plan.SetSourcedFromCompiler(eLazyBoolNo);
     unwind_plan.SetUnwindPlanValidAtAllInstructions(eLazyBoolYes);
   }
Index: lldb/source/Commands/CommandObjectTarget.cpp
===================================================================
--- lldb/source/Commands/CommandObjectTarget.cpp
+++ lldb/source/Commands/CommandObjectTarget.cpp
@@ -3458,21 +3458,21 @@
       if (non_callsite_unwind_plan) {
         result.GetOutputStream().Printf(
             "Asynchronous (not restricted to call-sites) UnwindPlan is '%s'\n",
-            non_callsite_unwind_plan->GetSourceName().AsCString());
+            non_callsite_unwind_plan->GetSourceName().c_str());
       }
       UnwindPlanSP callsite_unwind_plan =
           func_unwinders_sp->GetUnwindPlanAtCallSite(*target, *thread);
       if (callsite_unwind_plan) {
         result.GetOutputStream().Printf(
             "Synchronous (restricted to call-sites) UnwindPlan is '%s'\n",
-            callsite_unwind_plan->GetSourceName().AsCString());
+            callsite_unwind_plan->GetSourceName().c_str());
       }
       UnwindPlanSP fast_unwind_plan =
           func_unwinders_sp->GetUnwindPlanFastUnwind(*target, *thread);
       if (fast_unwind_plan) {
         result.GetOutputStream().Printf(
             "Fast UnwindPlan is '%s'\n",
-            fast_unwind_plan->GetSourceName().AsCString());
+            fast_unwind_plan->GetSourceName().c_str());
       }
 
       result.GetOutputStream().Printf("\n");
Index: lldb/include/lldb/Symbol/UnwindPlan.h
===================================================================
--- lldb/include/lldb/Symbol/UnwindPlan.h
+++ lldb/include/lldb/Symbol/UnwindPlan.h
@@ -11,10 +11,10 @@
 
 #include <map>
 #include <memory>
+#include <string>
 #include <vector>
 
 #include "lldb/Core/AddressRange.h"
-#include "lldb/Utility/ConstString.h"
 #include "lldb/Utility/Stream.h"
 #include "lldb/lldb-private.h"
 
@@ -465,9 +465,9 @@
 
   const UnwindPlan::RowSP GetLastRow() const;
 
-  lldb_private::ConstString GetSourceName() const;
+  const std::string &GetSourceName() const;
 
-  void SetSourceName(const char *);
+  void SetSourceName(std::string name);
 
   // Was this UnwindPlan emitted by a compiler?
   lldb_private::LazyBool GetSourcedFromCompiler() const {
@@ -509,7 +509,7 @@
     m_row_list.clear();
     m_plan_valid_address_range.Clear();
     m_register_kind = lldb::eRegisterKindDWARF;
-    m_source_name.Clear();
+    m_source_name.clear();
     m_plan_is_sourced_from_compiler = eLazyBoolCalculate;
     m_plan_is_valid_at_all_instruction_locations = eLazyBoolCalculate;
     m_plan_is_for_signal_trap = eLazyBoolCalculate;
@@ -539,7 +539,7 @@
   uint32_t m_return_addr_register; // The register that has the return address
                                    // for the caller frame
                                    // e.g. the lr on arm
-  lldb_private::ConstString
+  std::string
       m_source_name; // for logging, where this UnwindPlan originated from
   lldb_private::LazyBool m_plan_is_sourced_from_compiler;
   lldb_private::LazyBool m_plan_is_valid_at_all_instruction_locations;
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to