apolyakov created this revision.
apolyakov added reviewers: aprantl, clayborg, labath.
Herald added a subscriber: ki.stfu.

In this patch I suggest a way to deal with the problem started in 
https://reviews.llvm.org/D47302 and use it to re-implement MI target-select 
command. You are welcome to comment this patch, especially the new methods of 
Target and SBTarget classes and the bash script(do we need to find a random 
port or should we hardcode it?).


https://reviews.llvm.org/D49739

Files:
  include/lldb/API/SBTarget.h
  include/lldb/Target/Target.h
  lit/lit.cfg
  lit/tools/lldb-mi/target/inputs/main.c
  lit/tools/lldb-mi/target/inputs/target-select-so-path.sh
  lit/tools/lldb-mi/target/lit.local.cfg
  lit/tools/lldb-mi/target/target-select-so-path.test
  source/API/SBTarget.cpp
  source/Target/Target.cpp
  tools/lldb-mi/MICmdCmdTarget.cpp

Index: tools/lldb-mi/MICmdCmdTarget.cpp
===================================================================
--- tools/lldb-mi/MICmdCmdTarget.cpp
+++ tools/lldb-mi/MICmdCmdTarget.cpp
@@ -10,9 +10,8 @@
 // Overview:    CMICmdCmdTargetSelect           implementation.
 
 // Third Party Headers:
-#include "lldb/API/SBCommandInterpreter.h"
-#include "lldb/API/SBCommandReturnObject.h"
 #include "lldb/API/SBStream.h"
+#include "lldb/API/SBError.h"
 
 // In-house headers:
 #include "MICmdArgValNumber.h"
@@ -52,7 +51,7 @@
 // Return:  None.
 // Throws:  None.
 //--
-CMICmdCmdTargetSelect::~CMICmdCmdTargetSelect() {}
+CMICmdCmdTargetSelect::~CMICmdCmdTargetSelect() = default;
 
 //++
 //------------------------------------------------------------------------------------
@@ -93,51 +92,44 @@
 
   CMICmnLLDBDebugSessionInfo &rSessionInfo(
       CMICmnLLDBDebugSessionInfo::Instance());
+  lldb::SBTarget target = rSessionInfo.GetTarget();
 
-  // Check we have a valid target
-  // Note: target created via 'file-exec-and-symbols' command
-  if (!rSessionInfo.GetTarget().IsValid()) {
+  // Check we have a valid target.
+  // Note: target created via 'file-exec-and-symbols' command.
+  if (!target.IsValid()) {
     SetError(CMIUtilString::Format(MIRSRC(IDS_CMD_ERR_INVALID_TARGET_CURRENT),
                                    m_cmdData.strMiCmd.c_str()));
     return MIstatus::failure;
   }
 
-  // Verify that we are executing remotely
+  // Verify that we are executing remotely.
   const CMIUtilString &rRemoteType(pArgType->GetValue());
   if (rRemoteType != "remote") {
     SetError(CMIUtilString::Format(MIRSRC(IDS_CMD_ERR_INVALID_TARGET_TYPE),
                                    m_cmdData.strMiCmd.c_str(),
                                    rRemoteType.c_str()));
     return MIstatus::failure;
   }
 
-  // Create a URL pointing to the remote gdb stub
+  // Create a URL pointing to the remote gdb stub.
   const CMIUtilString strUrl =
       CMIUtilString::Format("connect://%s", pArgParameters->GetValue().c_str());
 
-  // Ask LLDB to connect to the target port
-  const char *pPlugin("gdb-remote");
   lldb::SBError error;
-  lldb::SBProcess process = rSessionInfo.GetTarget().ConnectRemote(
+  // Ask LLDB to connect to the target port.
+  const char *pPlugin("gdb-remote");
+  lldb::SBProcess process = target.ConnectRemote(
       rSessionInfo.GetListener(), strUrl.c_str(), pPlugin, error);
 
-  // Verify that we have managed to connect successfully
-  lldb::SBStream errMsg;
-  error.GetDescription(errMsg);
+  // Verify that we have managed to connect successfully.
   if (!process.IsValid()) {
     SetError(CMIUtilString::Format(MIRSRC(IDS_CMD_ERR_INVALID_TARGET_PLUGIN),
                                    m_cmdData.strMiCmd.c_str(),
-                                   errMsg.GetData()));
-    return MIstatus::failure;
-  }
-  if (error.Fail()) {
-    SetError(CMIUtilString::Format(MIRSRC(IDS_CMD_ERR_CONNECT_TO_TARGET),
-                                   m_cmdData.strMiCmd.c_str(),
-                                   errMsg.GetData()));
+                                   error.GetCString()));
     return MIstatus::failure;
   }
 
-  // Set the environment path if we were given one
+  // Set the environment path if we were given one.
   CMIUtilString strWkDir;
   if (rSessionInfo.SharedDataRetrieve<CMIUtilString>(
           rSessionInfo.m_constStrSharedDataKeyWkDir, strWkDir)) {
@@ -150,28 +142,13 @@
     }
   }
 
-  // Set the shared object path if we were given one
+  // Set the shared object path if we were given one.
   CMIUtilString strSolibPath;
   if (rSessionInfo.SharedDataRetrieve<CMIUtilString>(
-          rSessionInfo.m_constStrSharedDataSolibPath, strSolibPath)) {
-    lldb::SBDebugger &rDbgr = rSessionInfo.GetDebugger();
-    lldb::SBCommandInterpreter cmdIterpreter = rDbgr.GetCommandInterpreter();
-
-    CMIUtilString strCmdString = CMIUtilString::Format(
-        "target modules search-paths add . %s", strSolibPath.c_str());
-
-    lldb::SBCommandReturnObject retObj;
-    cmdIterpreter.HandleCommand(strCmdString.c_str(), retObj, false);
+          rSessionInfo.m_constStrSharedDataSolibPath, strSolibPath))
+    target.AppendImageSearchPath(".", strSolibPath.c_str(), error);
 
-    if (!retObj.Succeeded()) {
-      SetError(CMIUtilString::Format(MIRSRC(IDS_CMD_ERR_FNFAILED),
-                                     m_cmdData.strMiCmd.c_str(),
-                                     "target-select"));
-      return MIstatus::failure;
-    }
-  }
-
-  return MIstatus::success;
+  return HandleSBError(error);
 }
 
 //++
Index: source/Target/Target.cpp
===================================================================
--- source/Target/Target.cpp
+++ source/Target/Target.cpp
@@ -2052,6 +2052,11 @@
   return m_image_search_paths;
 }
 
+void Target::AppendImageSearchPath(const ConstString &from,
+                                   const ConstString &to) {
+  m_image_search_paths.Append(from, to, true);
+}
+
 void Target::ImageSearchPathsChanged(const PathMappingList &path_list,
                                      void *baton) {
   Target *target = (Target *)baton;
Index: source/API/SBTarget.cpp
===================================================================
--- source/API/SBTarget.cpp
+++ source/API/SBTarget.cpp
@@ -1457,6 +1457,30 @@
   return false;
 }
 
+void SBTarget::AppendImageSearchPath(const char *from, const char *to) {
+  lldb::SBError error; // Ignored
+  AppendImageSearchPath(from, to, error);
+}
+
+void SBTarget::AppendImageSearchPath(const char *from, const char *to,
+                                     lldb::SBError &error) {
+  TargetSP target_sp(GetSP());
+  if (!target_sp) {
+    error.SetErrorStringWithFormat("invalid target "
+                                   "(SBTarget::%s) failed", __FUNCTION__);
+    return;
+  }
+  if (from[0] && to[0]) {
+    Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_API));
+    if (log)
+      log->Printf("SBTarget::%s: '%s' -> '%s'",
+                  __FUNCTION__, from, to);
+    target_sp->AppendImageSearchPath(ConstString(from), ConstString(to));
+  } else
+    error.SetErrorStringWithFormat("<path-prefix> can't be empty "
+                                   "(SBTarget::%s) failed", __FUNCTION__);
+}
+
 lldb::SBModule SBTarget::AddModule(const char *path, const char *triple,
                                    const char *uuid_cstr) {
   return AddModule(path, triple, uuid_cstr, NULL);
Index: lit/tools/lldb-mi/target/target-select-so-path.test
===================================================================
--- /dev/null
+++ lit/tools/lldb-mi/target/target-select-so-path.test
@@ -0,0 +1,24 @@
+# REQUIRES: nowindows
+#
+# RUN: %cc -o %t %p/inputs/main.c -g
+# RUN: %p/inputs/./target-select-so-path.sh "%debugserver" "%lldbmi %t" %s
+# CHECK: 0
+
+# Test that -target-select command can hook up a path
+# added by gdb-set solib-search-path.
+
+# Check that we have a valid target created via file-exec-and-symbols.
+# CHECK: ^done
+
+-interpreter-exec console "target modules search-paths list"
+# CHECK ^done
+
+-gdb-set solib-search-path /example/dir
+# CHECK: ^done
+
+-target-select remote localhost:$PORT
+# CHECK: ^connected
+
+-interpreter-exec console "target modules search-paths list"
+# CHECK: ~"[0] \".\" -> \"/example/dir\"\n"
+# CHECK-NEXT: ^done
Index: lit/tools/lldb-mi/target/lit.local.cfg
===================================================================
--- /dev/null
+++ lit/tools/lldb-mi/target/lit.local.cfg
@@ -0,0 +1 @@
+config.suffixes = ['.test']
Index: lit/tools/lldb-mi/target/inputs/target-select-so-path.sh
===================================================================
--- /dev/null
+++ lit/tools/lldb-mi/target/inputs/target-select-so-path.sh
@@ -0,0 +1,25 @@
+#!/bin/bash
+
+function get_random_unused_port {
+    local port=$(shuf -i 2000-65000 -n 1)
+    netstat -lat | grep $port > /dev/null
+    if [[ $? == 1 ]] ; then
+        echo $port
+    else
+        get_random_unused_port
+    fi
+}
+
+PORT=$(get_random_unused_port)
+DEBUGSERVER=$1
+LLDB_MI=$2
+FILE_TO_CHECK=$3
+
+$DEBUGSERVER :$PORT &
+
+# Replace substring $PORT in a lldb-mi input with the found
+# unused port.
+cat $FILE_TO_CHECK | sed --expression='s/$PORT/'$PORT'/g' | $LLDB_MI \
+| FileCheck $FILE_TO_CHECK
+
+exit $?
Index: lit/tools/lldb-mi/target/inputs/main.c
===================================================================
--- /dev/null
+++ lit/tools/lldb-mi/target/inputs/main.c
@@ -0,0 +1,4 @@
+int main(void) {
+  int x = 0;
+  return x;
+}
Index: lit/lit.cfg
===================================================================
--- lit/lit.cfg
+++ lit/lit.cfg
@@ -56,7 +56,10 @@
 # Register substitutions
 config.substitutions.append(('%python', config.python_executable))
 
-debugserver = lit.util.which('debugserver', lldb_tools_dir)
+if platform.system() in ['Darwin']:
+    debugserver = lit.util.which('debugserver', lldb_tools_dir)
+else:
+    debugserver = lit.util.which('lldb-server', lldb_tools_dir)
 lldb = "%s -S %s/lit-lldb-init" % (lit.util.which('lldb', lldb_tools_dir),
                                config.test_source_root)
 
@@ -91,7 +94,10 @@
 config.substitutions.append(('%lldb', lldb))
 
 if debugserver is not None:
-    config.substitutions.append(('%debugserver', debugserver))
+    if platform.system() in ['Darwin']:
+        config.substitutions.append(('%debugserver', debugserver))
+    else:
+        config.substitutions.append(('%debugserver', debugserver + ' gdbserver'))
 
 for pattern in [r"\bFileCheck\b",
                 r"\blldb-test\b",
Index: include/lldb/Target/Target.h
===================================================================
--- include/lldb/Target/Target.h
+++ include/lldb/Target/Target.h
@@ -1000,6 +1000,9 @@
 
   PathMappingList &GetImageSearchPathList();
 
+  void AppendImageSearchPath(const ConstString &from,
+                             const ConstString &to);
+
   TypeSystem *GetScratchTypeSystemForLanguage(Status *error,
                                               lldb::LanguageType language,
                                               bool create_on_demand = true);
Index: include/lldb/API/SBTarget.h
===================================================================
--- include/lldb/API/SBTarget.h
+++ include/lldb/API/SBTarget.h
@@ -272,6 +272,11 @@
 
   lldb::SBFileSpec GetExecutable();
 
+  void AppendImageSearchPath(const char *from, const char *to);
+
+  void AppendImageSearchPath(const char *from, const char *to,
+                             lldb::SBError &error);
+
   bool AddModule(lldb::SBModule &module);
 
   lldb::SBModule AddModule(const char *path, const char *triple,
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to