mib updated this revision to Diff 527575.
mib marked an inline comment as done.
mib added a subscriber: jingham.
mib added a comment.

Address @bulbazord & @jingham's comments:

- Rename name -> label
- Add test
- Add sanity check to avoid having integer only labels
- Add target index when label already exist for another target


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D151849/new/

https://reviews.llvm.org/D151849

Files:
  lldb/include/lldb/API/SBTarget.h
  lldb/include/lldb/Target/Target.h
  lldb/include/lldb/Target/TargetList.h
  lldb/source/API/SBTarget.cpp
  lldb/source/Commands/CommandObjectTarget.cpp
  lldb/source/Target/Target.cpp
  lldb/source/Target/TargetList.cpp
  lldb/test/Shell/Target/target-label.test

Index: lldb/test/Shell/Target/target-label.test
===================================================================
--- /dev/null
+++ lldb/test/Shell/Target/target-label.test
@@ -0,0 +1,44 @@
+# RUN: %lldb -b -s %s  | FileCheck %s
+
+target create -l "ls" /bin/ls
+target list
+# CHECK: * target #0 (ls): /bin/ls
+
+script lldb.target.SetLabel("")
+target list
+# CHECK: * target #0: /bin/ls
+
+target create -l "cat" /bin/cat
+target list
+# CHECK: target #0 (ls): /bin/ls
+# CHECK-NEXT: * target #1 (cat): /bin/cat
+
+target create -l "cat" /bin/cat
+target list
+# CHECK: target #0 (ls): /bin/ls
+# CHECK-NEXT: target #1 (cat): /bin/cat
+# CHECK-NEXT: * target #2 (cat 2): /bin/cat
+
+target create -l 42 /bin/cat
+# CHECK: error: Cannot use integer as target label.
+
+target select 0
+# CHECK: * target #0 (ls): /bin/ls
+# CHECK-NEXT: target #1 (cat): /bin/cat
+# CHECK-NEXT: target #2 (cat 2): /bin/cat
+
+target select cat
+# CHECK: target #0 (ls): /bin/ls
+# CHECK-NEXT: * target #1 (cat): /bin/cat
+# CHECK-NEXT: target #2 (cat 2): /bin/cat
+
+script lldb.target.GetLabel()
+# CHECK: 'cat'
+
+script lldb.debugger.GetTargetAtIndex(2).SetLabel("The other cat")
+# CHECK: 'The other cat'
+
+target list
+# CHECK: target #0 (ls): /bin/ls
+# CHECK-NEXT: * target #1 (cat): /bin/cat
+# CHECK-NEXT: target #2 (The other cat): /bin/cat
Index: lldb/source/Target/TargetList.cpp
===================================================================
--- lldb/source/Target/TargetList.cpp
+++ lldb/source/Target/TargetList.cpp
@@ -489,7 +489,7 @@
   return num_signals_sent;
 }
 
-int TargetList::GetNumTargets() const {
+size_t TargetList::GetNumTargets() const {
   std::lock_guard<std::recursive_mutex> guard(m_target_list_mutex);
   return m_target_list.size();
 }
Index: lldb/source/Target/Target.cpp
===================================================================
--- lldb/source/Target/Target.cpp
+++ lldb/source/Target/Target.cpp
@@ -69,6 +69,7 @@
 #include <memory>
 #include <mutex>
 #include <optional>
+#include <sstream>
 
 using namespace lldb;
 using namespace lldb_private;
@@ -2536,6 +2537,21 @@
   Target::GetGlobalProperties().SetDefaultArchitecture(arch);
 }
 
+const std::string &Target::SetLabel(llvm::StringRef label) {
+  std::ostringstream formatted_label(label.str(), std::ios_base::ate);
+  TargetList &targets = GetDebugger().GetTargetList();
+  for (size_t i = 0; i < targets.GetNumTargets(); i++) {
+    TargetSP target_sp = targets.GetTargetAtIndex(i);
+    if (target_sp && target_sp->GetLabel() == label) {
+        formatted_label << " " << targets.GetIndexOfTarget(shared_from_this());
+        break;
+    }
+  }
+
+  m_label = formatted_label.str();
+  return m_label;
+}
+
 Target *Target::GetTargetFromContexts(const ExecutionContext *exe_ctx_ptr,
                                       const SymbolContext *sc_ptr) {
   // The target can either exist in the "process" of ExecutionContext, or in
Index: lldb/source/Commands/CommandObjectTarget.cpp
===================================================================
--- lldb/source/Commands/CommandObjectTarget.cpp
+++ lldb/source/Commands/CommandObjectTarget.cpp
@@ -82,8 +82,14 @@
   if (!exe_valid)
     ::strcpy(exe_path, "<none>");
 
-  strm.Printf("%starget #%u: %s", prefix_cstr ? prefix_cstr : "", target_idx,
-              exe_path);
+  std::string formatted_label = "";
+  const std::string &label = target->GetLabel();
+  if (!label.empty()) {
+    formatted_label = " (" + label + ")";
+  }
+
+  strm.Printf("%starget #%u%s: %s", prefix_cstr ? prefix_cstr : "", target_idx,
+              formatted_label.data(), exe_path);
 
   uint32_t properties = 0;
   if (target_arch.IsValid()) {
@@ -209,6 +215,8 @@
         m_platform_options(true), // Include the --platform option.
         m_core_file(LLDB_OPT_SET_1, false, "core", 'c', 0, eArgTypeFilename,
                     "Fullpath to a core file to use for this target."),
+        m_label(LLDB_OPT_SET_1, false, "label", 'l', 0, eArgTypeName,
+                "Optional name for this target.", nullptr),
         m_symbol_file(LLDB_OPT_SET_1, false, "symfile", 's', 0,
                       eArgTypeFilename,
                       "Fullpath to a stand alone debug "
@@ -234,6 +242,7 @@
     m_option_group.Append(&m_arch_option, LLDB_OPT_SET_ALL, LLDB_OPT_SET_1);
     m_option_group.Append(&m_platform_options, LLDB_OPT_SET_ALL, 1);
     m_option_group.Append(&m_core_file, LLDB_OPT_SET_ALL, LLDB_OPT_SET_1);
+    m_option_group.Append(&m_label, LLDB_OPT_SET_ALL, LLDB_OPT_SET_1);
     m_option_group.Append(&m_symbol_file, LLDB_OPT_SET_ALL, LLDB_OPT_SET_1);
     m_option_group.Append(&m_remote_file, LLDB_OPT_SET_ALL, LLDB_OPT_SET_1);
     m_option_group.Append(&m_add_dependents, LLDB_OPT_SET_ALL, LLDB_OPT_SET_1);
@@ -270,6 +279,14 @@
       }
     }
 
+    size_t label_is_integral = LLDB_INVALID_INDEX32;
+    const llvm::StringRef label =
+        m_label.GetOptionValue().GetCurrentValueAsRef();
+    if (!label.empty() && llvm::to_integer(label, label_is_integral)) {
+      result.AppendError("Cannot use integer as target label.");
+      return false;
+    }
+
     if (argc == 1 || core_file || remote_file) {
       FileSpec symfile(m_symbol_file.GetOptionValue().GetCurrentValue());
       if (symfile) {
@@ -303,6 +320,9 @@
         return false;
       }
 
+      if (!label.empty())
+        target_sp->SetLabel(label);
+
       auto on_error = llvm::make_scope_exit(
           [&target_list = debugger.GetTargetList(), &target_sp]() {
             target_list.DeleteTarget(target_sp);
@@ -455,6 +475,7 @@
   OptionGroupArchitecture m_arch_option;
   OptionGroupPlatform m_platform_options;
   OptionGroupFile m_core_file;
+  OptionGroupString m_label;
   OptionGroupFile m_symbol_file;
   OptionGroupFile m_remote_file;
   OptionGroupDependents m_add_dependents;
@@ -503,11 +524,11 @@
 protected:
   bool DoExecute(Args &args, CommandReturnObject &result) override {
     if (args.GetArgumentCount() == 1) {
-      const char *target_idx_arg = args.GetArgumentAtIndex(0);
-      uint32_t target_idx;
-      if (llvm::to_integer(target_idx_arg, target_idx)) {
-        TargetList &target_list = GetDebugger().GetTargetList();
-        const uint32_t num_targets = target_list.GetNumTargets();
+      const char *target_identifier = args.GetArgumentAtIndex(0);
+      uint32_t target_idx = LLDB_INVALID_INDEX32;
+      TargetList &target_list = GetDebugger().GetTargetList();
+      const uint32_t num_targets = target_list.GetNumTargets();
+      if (llvm::to_integer(target_identifier, target_idx)) {
         if (target_idx < num_targets) {
           target_list.SetSelectedTarget(target_idx);
           Stream &strm = result.GetOutputStream();
@@ -526,8 +547,26 @@
           }
         }
       } else {
-        result.AppendErrorWithFormat("invalid index string value '%s'\n",
-                                     target_idx_arg);
+        for (size_t i = 0; i < num_targets; i++) {
+          if (TargetSP target_sp = target_list.GetTargetAtIndex(i)) {
+            const std::string &label = target_sp->GetLabel();
+            if (!label.empty() && label == target_identifier) {
+              target_idx = i;
+              break;
+            }
+          }
+        }
+
+        if (target_idx != LLDB_INVALID_INDEX32) {
+          target_list.SetSelectedTarget(target_idx);
+          Stream &strm = result.GetOutputStream();
+          bool show_stopped_process_status = false;
+          DumpTargetList(target_list, show_stopped_process_status, strm);
+          result.SetStatus(eReturnStatusSuccessFinishResult);
+        } else {
+          result.AppendErrorWithFormat("invalid index string value '%s'\n",
+                                       target_identifier);
+        }
       }
     } else {
       result.AppendError(
@@ -576,7 +615,7 @@
     TargetSP target_sp;
 
     if (m_all_option.GetOptionValue()) {
-      for (int i = 0; i < target_list.GetNumTargets(); ++i)
+      for (size_t i = 0; i < target_list.GetNumTargets(); ++i)
         delete_target_list.push_back(target_list.GetTargetAtIndex(i));
     } else if (argc > 0) {
       const uint32_t num_targets = target_list.GetNumTargets();
Index: lldb/source/API/SBTarget.cpp
===================================================================
--- lldb/source/API/SBTarget.cpp
+++ lldb/source/API/SBTarget.cpp
@@ -1601,6 +1601,31 @@
   return const_name.GetCString();
 }
 
+const char *SBTarget::GetLabel() const {
+  LLDB_INSTRUMENT_VA(this);
+
+  TargetSP target_sp(GetSP());
+  if (!target_sp)
+    return nullptr;
+
+  return ConstString(target_sp->GetLabel().data()).AsCString();
+}
+
+const char *SBTarget::SetLabel(const char *label) {
+  LLDB_INSTRUMENT_VA(this, label);
+
+  llvm::StringRef label_ref(label);
+  size_t label_is_integral = LLDB_INVALID_INDEX32;
+  if (!label_ref.empty() && llvm::to_integer(label_ref, label_is_integral))
+    return nullptr;
+
+  TargetSP target_sp(GetSP());
+  if (!target_sp)
+    return nullptr;
+
+  return ConstString(target_sp->SetLabel(label_ref)).AsCString();
+}
+
 uint32_t SBTarget::GetDataByteSize() {
   LLDB_INSTRUMENT_VA(this);
 
Index: lldb/include/lldb/Target/TargetList.h
===================================================================
--- lldb/include/lldb/Target/TargetList.h
+++ lldb/include/lldb/Target/TargetList.h
@@ -115,7 +115,7 @@
   ///     in \a target_sp which can then be properly released.
   bool DeleteTarget(lldb::TargetSP &target_sp);
 
-  int GetNumTargets() const;
+  size_t GetNumTargets() const;
 
   lldb::TargetSP GetTargetAtIndex(uint32_t index) const;
 
Index: lldb/include/lldb/Target/Target.h
===================================================================
--- lldb/include/lldb/Target/Target.h
+++ lldb/include/lldb/Target/Target.h
@@ -554,6 +554,17 @@
 
   bool IsDummyTarget() const { return m_is_dummy_target; }
 
+  const std::string &GetLabel() const { return m_label; }
+
+  /// Set a label for a target.
+  ///
+  /// If another target already have a label, appends target index to the given
+  /// label.
+  ///
+  /// \return
+  ///     The final label for this target.
+  const std::string &SetLabel(llvm::StringRef label);
+
   /// Find a binary on the system and return its Module,
   /// or return an existing Module that is already in the Target.
   ///
@@ -1520,6 +1531,7 @@
   /// detect that code is running on the private state thread.
   std::recursive_mutex m_private_mutex;
   Arch m_arch;
+  std::string m_label;
   ModuleList m_images; ///< The list of images for this process (shared
                        /// libraries and anything dynamically loaded).
   SectionLoadHistory m_section_load_history;
Index: lldb/include/lldb/API/SBTarget.h
===================================================================
--- lldb/include/lldb/API/SBTarget.h
+++ lldb/include/lldb/API/SBTarget.h
@@ -328,6 +328,10 @@
   
   const char *GetABIName();
 
+  const char *GetLabel() const;
+
+  const char *SetLabel(const char *label);
+
   /// Architecture data byte width accessor
   ///
   /// \return
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to