[Lldb-commits] [lldb] d79273c - [lldb/ScriptInterpreter] Extract IO redirection logic

Jonas Devlieghere via lldb-commits lldb-commits at lists.llvm.org
Thu Jun 25 09:43:34 PDT 2020


Author: Jonas Devlieghere
Date: 2020-06-25T09:43:28-07:00
New Revision: d79273c941d58486d09c020eb7767a5246a9c24d

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

LOG: [lldb/ScriptInterpreter] Extract IO redirection logic

This patch takes the IO redirection logic from ScriptInterpreterPython
and moves it into the interpreter library so that it can be used by
other script interpreters. I've turned it into a RAII object so that we
don't have to worry about cleaning up in the calling code.

Differential revision: https://reviews.llvm.org/D82396

Added: 
    

Modified: 
    lldb/include/lldb/Interpreter/ScriptInterpreter.h
    lldb/source/Interpreter/ScriptInterpreter.cpp
    lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp

Removed: 
    


################################################################################
diff  --git a/lldb/include/lldb/Interpreter/ScriptInterpreter.h b/lldb/include/lldb/Interpreter/ScriptInterpreter.h
index d008e18e7b6f..8086886149f8 100644
--- a/lldb/include/lldb/Interpreter/ScriptInterpreter.h
+++ b/lldb/include/lldb/Interpreter/ScriptInterpreter.h
@@ -9,16 +9,16 @@
 #ifndef LLDB_INTERPRETER_SCRIPTINTERPRETER_H
 #define LLDB_INTERPRETER_SCRIPTINTERPRETER_H
 
-#include "lldb/lldb-private.h"
-
 #include "lldb/Breakpoint/BreakpointOptions.h"
+#include "lldb/Core/Communication.h"
 #include "lldb/Core/PluginInterface.h"
 #include "lldb/Core/SearchFilter.h"
+#include "lldb/Core/StreamFile.h"
+#include "lldb/Host/PseudoTerminal.h"
 #include "lldb/Utility/Broadcaster.h"
 #include "lldb/Utility/Status.h"
 #include "lldb/Utility/StructuredData.h"
-
-#include "lldb/Host/PseudoTerminal.h"
+#include "lldb/lldb-private.h"
 
 namespace lldb_private {
 
@@ -34,6 +34,37 @@ class ScriptInterpreterLocker {
   operator=(const ScriptInterpreterLocker &) = delete;
 };
 
+class ScriptInterpreterIORedirect {
+public:
+  /// Create an IO redirect with /dev/null as input, output and error file.
+  static llvm::Expected<std::unique_ptr<ScriptInterpreterIORedirect>> Create();
+
+  /// Create an IO redirect that redirects the output to the command return
+  /// object if set or to the debugger otherwise.
+  static llvm::Expected<std::unique_ptr<ScriptInterpreterIORedirect>>
+  Create(Debugger &debugger, CommandReturnObject *result);
+
+  ~ScriptInterpreterIORedirect();
+
+  lldb::FileSP GetInputFile() const { return m_input_file_sp; }
+  lldb::FileSP GetOutputFile() const { return m_output_file_sp->GetFileSP(); }
+  lldb::FileSP GetErrorFile() const { return m_error_file_sp->GetFileSP(); }
+
+  /// Flush our output and error file handles.
+  void Flush();
+
+private:
+  ScriptInterpreterIORedirect(std::unique_ptr<File> input,
+                              std::unique_ptr<File> output);
+  ScriptInterpreterIORedirect(Debugger &debugger, CommandReturnObject *result);
+
+  lldb::FileSP m_input_file_sp;
+  lldb::StreamFileSP m_output_file_sp;
+  lldb::StreamFileSP m_error_file_sp;
+  Communication m_communication;
+  bool m_disconnect;
+};
+
 class ScriptInterpreter : public PluginInterface {
 public:
   enum ScriptReturnType {

diff  --git a/lldb/source/Interpreter/ScriptInterpreter.cpp b/lldb/source/Interpreter/ScriptInterpreter.cpp
index 2dda1bbbff47..d46a104fa25c 100644
--- a/lldb/source/Interpreter/ScriptInterpreter.cpp
+++ b/lldb/source/Interpreter/ScriptInterpreter.cpp
@@ -8,10 +8,14 @@
 
 #include "lldb/Interpreter/ScriptInterpreter.h"
 
+#include <memory>
 #include <stdio.h>
 #include <stdlib.h>
 #include <string>
 
+#include "lldb/Core/Debugger.h"
+#include "lldb/Host/ConnectionFileDescriptor.h"
+#include "lldb/Host/Pipe.h"
 #include "lldb/Host/PseudoTerminal.h"
 #include "lldb/Interpreter/CommandReturnObject.h"
 #include "lldb/Utility/Status.h"
@@ -104,3 +108,113 @@ std::unique_ptr<ScriptInterpreterLocker>
 ScriptInterpreter::AcquireInterpreterLock() {
   return std::make_unique<ScriptInterpreterLocker>();
 }
+
+static void ReadThreadBytesReceived(void *baton, const void *src,
+                                    size_t src_len) {
+  if (src && src_len) {
+    Stream *strm = (Stream *)baton;
+    strm->Write(src, src_len);
+    strm->Flush();
+  }
+}
+
+llvm::Expected<std::unique_ptr<ScriptInterpreterIORedirect>>
+ScriptInterpreterIORedirect::Create() {
+  auto nullin = FileSystem::Instance().Open(FileSpec(FileSystem::DEV_NULL),
+                                            File::eOpenOptionRead);
+  if (!nullin)
+    return nullin.takeError();
+
+  auto nullout = FileSystem::Instance().Open(FileSpec(FileSystem::DEV_NULL),
+                                             File::eOpenOptionWrite);
+  if (!nullout)
+    return nullin.takeError();
+
+  return std::unique_ptr<ScriptInterpreterIORedirect>(
+      new ScriptInterpreterIORedirect(std::move(*nullin), std::move(*nullout)));
+}
+
+llvm::Expected<std::unique_ptr<ScriptInterpreterIORedirect>>
+ScriptInterpreterIORedirect::Create(Debugger &debugger,
+                                    CommandReturnObject *result) {
+  return std::unique_ptr<ScriptInterpreterIORedirect>(
+      new ScriptInterpreterIORedirect(debugger, result));
+}
+
+ScriptInterpreterIORedirect::ScriptInterpreterIORedirect(
+    std::unique_ptr<File> input, std::unique_ptr<File> output)
+    : m_input_file_sp(std::move(input)),
+      m_output_file_sp(std::make_shared<StreamFile>(std::move(output))),
+      m_error_file_sp(m_output_file_sp),
+      m_communication("lldb.ScriptInterpreterIORedirect.comm"),
+      m_disconnect(false) {}
+
+ScriptInterpreterIORedirect::ScriptInterpreterIORedirect(
+    Debugger &debugger, CommandReturnObject *result)
+    : m_communication("lldb.ScriptInterpreterIORedirect.comm"),
+      m_disconnect(false) {
+
+  if (result) {
+    m_input_file_sp = debugger.GetInputFileSP();
+
+    Pipe pipe;
+    Status pipe_result = pipe.CreateNew(false);
+#if defined(_WIN32)
+    lldb::file_t read_file = pipe.GetReadNativeHandle();
+    pipe.ReleaseReadFileDescriptor();
+    std::unique_ptr<ConnectionGenericFile> conn_up =
+        std::make_unique<ConnectionGenericFile>(read_file, true);
+#else
+    std::unique_ptr<ConnectionFileDescriptor> conn_up =
+        std::make_unique<ConnectionFileDescriptor>(
+            pipe.ReleaseReadFileDescriptor(), true);
+#endif
+
+    if (conn_up->IsConnected()) {
+      m_communication.SetConnection(std::move(conn_up));
+      m_communication.SetReadThreadBytesReceivedCallback(
+          ReadThreadBytesReceived, &result->GetOutputStream());
+      m_communication.StartReadThread();
+      m_disconnect = true;
+
+      FILE *outfile_handle = fdopen(pipe.ReleaseWriteFileDescriptor(), "w");
+      m_output_file_sp = std::make_shared<StreamFile>(outfile_handle, true);
+      m_error_file_sp = m_output_file_sp;
+      if (outfile_handle)
+        ::setbuf(outfile_handle, nullptr);
+
+      result->SetImmediateOutputFile(debugger.GetOutputStream().GetFileSP());
+      result->SetImmediateErrorFile(debugger.GetErrorStream().GetFileSP());
+    }
+  }
+
+  if (!m_input_file_sp || !m_output_file_sp || !m_error_file_sp)
+    debugger.AdoptTopIOHandlerFilesIfInvalid(m_input_file_sp, m_output_file_sp,
+                                             m_error_file_sp);
+}
+
+void ScriptInterpreterIORedirect::Flush() {
+  if (m_output_file_sp)
+    m_output_file_sp->Flush();
+  if (m_error_file_sp)
+    m_error_file_sp->Flush();
+}
+
+ScriptInterpreterIORedirect::~ScriptInterpreterIORedirect() {
+  if (!m_disconnect)
+    return;
+
+  assert(m_output_file_sp);
+  assert(m_error_file_sp);
+  assert(m_output_file_sp == m_error_file_sp);
+
+  // Close the write end of the pipe since we are done with our one line
+  // script. This should cause the read thread that output_comm is using to
+  // exit.
+  m_output_file_sp->GetFile().Close();
+  // The close above should cause this thread to exit when it gets to the end
+  // of file, so let it get all its data.
+  m_communication.JoinReadThread();
+  // Now we can close the read end of the pipe.
+  m_communication.Disconnect();
+}

diff  --git a/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp b/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
index 39f76e7c5bb5..40ce7d997800 100644
--- a/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
+++ b/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
@@ -42,6 +42,7 @@
 
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/StringRef.h"
+#include "llvm/Support/Error.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/FormatAdapters.h"
 
@@ -895,15 +896,6 @@ bool ScriptInterpreterPythonImpl::GetEmbeddedInterpreterModuleObjects() {
   return m_run_one_line_function.IsValid();
 }
 
-static void ReadThreadBytesReceived(void *baton, const void *src,
-                                    size_t src_len) {
-  if (src && src_len) {
-    Stream *strm = (Stream *)baton;
-    strm->Write(src, src_len);
-    strm->Flush();
-  }
-}
-
 bool ScriptInterpreterPythonImpl::ExecuteOneLine(
     llvm::StringRef command, CommandReturnObject *result,
     const ExecuteScriptOptions &options) {
@@ -919,77 +911,23 @@ bool ScriptInterpreterPythonImpl::ExecuteOneLine(
     // another string to pass to PyRun_SimpleString messes up the escaping.  So
     // we use the following more complicated method to pass the command string
     // directly down to Python.
-    Debugger &debugger = m_debugger;
-
-    FileSP input_file_sp;
-    StreamFileSP output_file_sp;
-    StreamFileSP error_file_sp;
-    Communication output_comm(
-        "lldb.ScriptInterpreterPythonImpl.ExecuteOneLine.comm");
-    bool join_read_thread = false;
-    if (options.GetEnableIO()) {
-      if (result) {
-        input_file_sp = debugger.GetInputFileSP();
-        // Set output to a temporary file so we can forward the results on to
-        // the result object
-
-        Pipe pipe;
-        Status pipe_result = pipe.CreateNew(false);
-        if (pipe_result.Success()) {
-#if defined(_WIN32)
-          lldb::file_t read_file = pipe.GetReadNativeHandle();
-          pipe.ReleaseReadFileDescriptor();
-          std::unique_ptr<ConnectionGenericFile> conn_up(
-              new ConnectionGenericFile(read_file, true));
-#else
-          std::unique_ptr<ConnectionFileDescriptor> conn_up(
-              new ConnectionFileDescriptor(pipe.ReleaseReadFileDescriptor(),
-                                           true));
-#endif
-          if (conn_up->IsConnected()) {
-            output_comm.SetConnection(std::move(conn_up));
-            output_comm.SetReadThreadBytesReceivedCallback(
-                ReadThreadBytesReceived, &result->GetOutputStream());
-            output_comm.StartReadThread();
-            join_read_thread = true;
-            FILE *outfile_handle =
-                fdopen(pipe.ReleaseWriteFileDescriptor(), "w");
-            output_file_sp = std::make_shared<StreamFile>(outfile_handle, true);
-            error_file_sp = output_file_sp;
-            if (outfile_handle)
-              ::setbuf(outfile_handle, nullptr);
-
-            result->SetImmediateOutputFile(
-                debugger.GetOutputStream().GetFileSP());
-            result->SetImmediateErrorFile(
-                debugger.GetErrorStream().GetFileSP());
-          }
-        }
-      }
-      if (!input_file_sp || !output_file_sp || !error_file_sp)
-        debugger.AdoptTopIOHandlerFilesIfInvalid(input_file_sp, output_file_sp,
-                                                 error_file_sp);
-    } else {
-      auto nullin = FileSystem::Instance().Open(
-                                  FileSpec(FileSystem::DEV_NULL),
-                                  File::eOpenOptionRead);
-      auto nullout = FileSystem::Instance().Open(
-                                  FileSpec(FileSystem::DEV_NULL),
-                                  File::eOpenOptionWrite);
-      if (!nullin) {
-        result->AppendErrorWithFormatv("failed to open /dev/null: {0}\n",
-                                       llvm::fmt_consume(nullin.takeError()));
-        return false;
-      }
-      if (!nullout) {
-        result->AppendErrorWithFormatv("failed to open /dev/null: {0}\n",
-                                       llvm::fmt_consume(nullout.takeError()));
-        return false;
-      }
-      input_file_sp = std::move(nullin.get());
-      error_file_sp = output_file_sp = std::make_shared<StreamFile>(std::move(nullout.get()));
+    llvm::Expected<std::unique_ptr<ScriptInterpreterIORedirect>>
+        io_redirect_or_error =
+            options.GetEnableIO()
+                ? ScriptInterpreterIORedirect::Create(m_debugger, result)
+                : ScriptInterpreterIORedirect::Create();
+    if (!io_redirect_or_error) {
+      if (result)
+        result->AppendErrorWithFormatv(
+            "failed to redirect I/O: {0}\n",
+            llvm::fmt_consume(io_redirect_or_error.takeError()));
+      else
+        llvm::consumeError(io_redirect_or_error.takeError());
+      return false;
     }
 
+    ScriptInterpreterIORedirect &io_redirect = **io_redirect_or_error;
+
     bool success = false;
     {
       // WARNING!  It's imperative that this RAII scope be as tight as
@@ -1005,8 +943,9 @@ bool ScriptInterpreterPythonImpl::ExecuteOneLine(
           Locker::AcquireLock | Locker::InitSession |
               (options.GetSetLLDBGlobals() ? Locker::InitGlobals : 0) |
               ((result && result->GetInteractive()) ? 0 : Locker::NoSTDIN),
-          Locker::FreeAcquiredLock | Locker::TearDownSession, input_file_sp,
-          output_file_sp->GetFileSP(), error_file_sp->GetFileSP());
+          Locker::FreeAcquiredLock | Locker::TearDownSession,
+          io_redirect.GetInputFile(), io_redirect.GetOutputFile(),
+          io_redirect.GetErrorFile());
 
       // Find the correct script interpreter dictionary in the main module.
       PythonDictionary &session_dict = GetSessionDictionary();
@@ -1032,21 +971,7 @@ bool ScriptInterpreterPythonImpl::ExecuteOneLine(
         }
       }
 
-      // Flush our output and error file handles
-      output_file_sp->Flush();
-      error_file_sp->Flush();
-    }
-
-    if (join_read_thread) {
-      // Close the write end of the pipe since we are done with our one line
-      // script. This should cause the read thread that output_comm is using to
-      // exit
-      output_file_sp->GetFile().Close();
-      // The close above should cause this thread to exit when it gets to the
-      // end of file, so let it get all its data
-      output_comm.JoinReadThread();
-      // Now we can close the read end of the pipe
-      output_comm.Disconnect();
+      io_redirect.Flush();
     }
 
     if (success)


        


More information about the lldb-commits mailing list