update ScriptInterpreterPython to use File, not FILE*

[lawrence_danna]

Summary

ScriptInterpreterPython needs to save and restore sys.stdout and
friends when LLDB runs a python script.

It currently does this using FILE*, which is not optimal. If
whatever was in sys.stdout can not be represented as a FILE*, then
it will not be restored correctly when the script is finished.

It also means that if the debugger's own output stream is not
representable as a file, ScriptInterpreterPython will not be able
to redirect python's output correctly.

This patch updates ScriptInterpreterPython to represent files with
lldb_private::File, and to represent whatever the user had in
sys.stdout as simply a PythonObject.

This will make lldb interoperate better with other scripts or programs
that need to manipulate sys.stdout.

[labath]

lldb/source/Core/Debugger.cpp

977

The call to reset seems superfluous, as the value will be overwritten anyway.

[lawrence_danna]

remove superfluous resets

lldb/packages/Python/lldbsuite/test/python_api/file_handle/TestFileHandle.py

def test_fileno_error(self):
 
             self.debugger.GetErrorFile().Write(b'\nzork\n')
 
         with open(self.out_filename, 'r') as f:
             errors = f.read()
             self.assertTrue(re.search(r'error:.*lolwut', errors))
             self.assertTrue(re.search(r'zork', errors))
 
-    #FIXME This shouldn't fail for python2 either.
+
     @add_test_categories(['pyapi'])
-    @skipIf(py_version=['<', (3,)])
     def test_replace_stdout(self):
         f = io.StringIO()
         with replace_stdout(f):
             self.assertEqual(sys.stdout, f)
             self.handleCmd('script sys.stdout.write("lol")',
                 collect_result=False, check=False)
             self.assertEqual(sys.stdout, f)
 
 
     @add_test_categories(['pyapi'])
-    @expectedFailureAll() #FIXME bug in ScriptInterpreterPython
     def test_replace_stdout_with_nonfile(self):
         debugger = self.debugger
         f = io.StringIO()
         with replace_stdout(f):
             class Nothing():
                 pass
             with replace_stdout(Nothing):
                 self.assertEqual(sys.stdout, Nothing)

lldb/source/Core/Debugger.cpp

void Debugger::AdoptTopIOHandlerFilesIfInvalid(FileSP &in, StreamFileSP &out,
   // Before an IOHandler runs, it must have in/out/err streams. This function
   // is called when one ore more of the streams are nullptr. We use the top
   // input reader's in/out/err streams, or fall back to the debugger file
   // handles, or we fall back onto stdin/stdout/stderr as a last resort.
 
   std::lock_guard<std::recursive_mutex> guard(m_input_reader_stack.GetMutex());
   IOHandlerSP top_reader_sp(m_input_reader_stack.Top());
   // If no STDIN has been set, then set it appropriately
-  if (!in) {
+  if (!in || !in->IsValid()) {
     if (top_reader_sp)
       in = top_reader_sp->GetInputFileSP();
     else
       in = GetInputFileSP();
-
     // If there is nothing, use stdin
     if (!in)
       in = std::make_shared<NativeFile>(stdin, false);
   }
   // If no STDOUT has been set, then set it appropriately
-  if (!out) {
+  if (!out || !out->GetFile().IsValid()) {
     if (top_reader_sp)
       out = top_reader_sp->GetOutputStreamFileSP();
     else
       out = GetOutputStreamSP();
-
     // If there is nothing, use stdout
     if (!out)
       out = std::make_shared<StreamFile>(stdout, false);
   }
   // If no STDERR has been set, then set it appropriately
-  if (!err) {
+  if (!err || !err->GetFile().IsValid()) {
     if (top_reader_sp)
       err = top_reader_sp->GetErrorStreamFileSP();
     else
       err = GetErrorStreamSP();
-
     // If there is nothing, use stderr
     if (!err)
       err = std::make_shared<StreamFile>(stderr, false);
   }
 }
 
 void Debugger::PushIOHandler(const IOHandlerSP &reader_sp,
                              bool cancel_top_handler) {

lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp

PluginManager::RegisterPlugin(GetPluginNameStatic(),
                                   ScriptInterpreterPythonImpl::CreateInstance);
   });
 }
 
 void ScriptInterpreterPython::Terminate() {}
 
 ScriptInterpreterPythonImpl::Locker::Locker(
     ScriptInterpreterPythonImpl *py_interpreter, uint16_t on_entry,
-    uint16_t on_leave, FILE *in, FILE *out, FILE *err)
+    uint16_t on_leave, FileSP in, FileSP out, FileSP err)
     : ScriptInterpreterLocker(),
       m_teardown_session((on_leave & TearDownSession) == TearDownSession),
       m_python_interpreter(py_interpreter) {
   DoAcquireLock();
   if ((on_entry & InitSession) == InitSession) {
     if (!DoInitSession(on_entry, in, out, err)) {
       // Don't teardown the session if we didn't init it.
       m_teardown_session = false;
bool ScriptInterpreterPythonImpl::Locker::DoAcquireLock() {
   // case, _PyThreadState_Current will be NULL - and we would be unable to set
   // the asynchronous exception - not a desirable situation
   m_python_interpreter->SetThreadState(PyThreadState_Get());
   m_python_interpreter->IncrementLockCount();
   return true;
 }
 
 bool ScriptInterpreterPythonImpl::Locker::DoInitSession(uint16_t on_entry_flags,
-                                                        FILE *in, FILE *out,
-                                                        FILE *err) {
+                                                        FileSP in, FileSP out,
+                                                        FileSP err) {
   if (!m_python_interpreter)
     return false;
   return m_python_interpreter->EnterSession(on_entry_flags, in, out, err);
 }
 
 bool ScriptInterpreterPythonImpl::Locker::DoFreeLock() {
   Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_SCRIPT));
   LLDB_LOGV(log, "Releasing PyGILState. Returning to state = {0}locked",
if (sys_module_dict.IsValid()) {
         m_saved_stderr.Reset();
       }
     }
   }
 
   m_session_is_active = false;
 }
 
-bool ScriptInterpreterPythonImpl::SetStdHandle(File &file, const char *py_name,
-                                               PythonFile &save_file,
+bool ScriptInterpreterPythonImpl::SetStdHandle(FileSP file_sp,
+                                               const char *py_name,
+                                               PythonObject &save_file,
                                                const char *mode) {
-  if (file.IsValid()) {
-    // Flush the file before giving it to python to avoid interleaved output.
-    file.Flush();
+  if (!file_sp || !*file_sp) {
+    save_file.Reset();
+    return false;
+  }
+  File &file = *file_sp;
+
 
-    PythonDictionary &sys_module_dict = GetSysModuleDictionary();
 
-    save_file = sys_module_dict.GetItemForKey(PythonString(py_name))
-                    .AsType<PythonFile>();
+  save_file = sys_module_dict.GetItemForKey(PythonString(py_name));
 
-    PythonFile new_file(file, mode);
-    sys_module_dict.SetItemForKey(PythonString(py_name), new_file);
-    return true;
-  } else
-    save_file.Reset();
-  return false;
 }
 
 bool ScriptInterpreterPythonImpl::EnterSession(uint16_t on_entry_flags,
-                                               FILE *in, FILE *out, FILE *err) {
+                                               FileSP in_sp, FileSP out_sp,
+                                               FileSP err_sp) {
   // If we have already entered the session, without having officially 'left'
   // it, then there is no need to 'enter' it again.
   Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_SCRIPT));
   if (m_session_is_active) {
     LLDB_LOGF(
         log,
         "ScriptInterpreterPythonImpl::EnterSession(on_entry_flags=0x%" PRIx16
         ") session is already active, returning without doing anything",
if (on_entry_flags & Locker::InitGlobals) {
     run_string.PutCString("')");
   }
 
   PyRun_SimpleString(run_string.GetData());
   run_string.Clear();
 
   PythonDictionary &sys_module_dict = GetSysModuleDictionary();
   if (sys_module_dict.IsValid()) {
-    NativeFile in_file(in, false);
-    NativeFile out_file(out, false);
-    NativeFile err_file(err, false);
-
-    lldb::FileSP in_sp;
-    lldb::StreamFileSP out_sp;
-    lldb::StreamFileSP err_sp;
-    if (!in_file.IsValid() || !out_file.IsValid() || !err_file.IsValid())
-      m_debugger.AdoptTopIOHandlerFilesIfInvalid(in_sp, out_sp, err_sp);
+    lldb::FileSP top_in_sp;
+    lldb::StreamFileSP top_out_sp, top_err_sp;
+    if (!in_sp || !out_sp || !err_sp || !*in_sp || !*out_sp || !*err_sp)
+      m_debugger.AdoptTopIOHandlerFilesIfInvalid(top_in_sp, top_out_sp,
+                                                 top_err_sp);
 
     if (on_entry_flags & Locker::NoSTDIN) {
       m_saved_stdin.Reset();
     } else {
-      if (!SetStdHandle(in_file, "stdin", m_saved_stdin, "r")) {
-        if (in_sp)
-          SetStdHandle(*in_sp, "stdin", m_saved_stdin, "r");
+      if (!SetStdHandle(in_sp, "stdin", m_saved_stdin, "r")) {
+        if (top_in_sp)
+          SetStdHandle(top_in_sp, "stdin", m_saved_stdin, "r");
       }
     }
 
-    if (!SetStdHandle(out_file, "stdout", m_saved_stdout, "w")) {
-      if (out_sp)
-        SetStdHandle(out_sp->GetFile(), "stdout", m_saved_stdout, "w");
+    if (!SetStdHandle(out_sp, "stdout", m_saved_stdout, "w")) {
+      if (top_out_sp)
+        SetStdHandle(top_out_sp->GetFileSP(), "stdout", m_saved_stdout, "w");
     }
 
-    if (!SetStdHandle(err_file, "stderr", m_saved_stderr, "w")) {
-      if (err_sp)
-        SetStdHandle(err_sp->GetFile(), "stderr", m_saved_stderr, "w");
+    if (!SetStdHandle(err_sp, "stderr", m_saved_stderr, "w")) {
+      if (top_err_sp)
+        SetStdHandle(top_err_sp->GetFileSP(), "stderr", m_saved_stderr, "w");
     }
   }
 
   if (PyErr_Occurred())
     PyErr_Clear();
 
   return true;
 }
#endif
         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()));
     }
 
-    FILE *in_file = input_file_sp->GetStream();
-    FILE *out_file = output_file_sp->GetFile().GetStream();
-    FILE *err_file = error_file_sp->GetFile().GetStream();
     bool success = false;
     {
       // WARNING!  It's imperative that this RAII scope be as tight as
       // possible. In particular, the scope must end *before* we try to join
       // the read thread.  The reason for this is that a pre-requisite for
       // joining the read thread is that we close the write handle (to break
       // the pipe and cause it to wake up and exit).  But acquiring the GIL as
       // below will redirect Python's stdio to use this same handle.  If we
       // close the handle while Python is still using it, bad things will
       // happen.
       Locker locker(
           this,
           Locker::AcquireLock | Locker::InitSession |
               (options.GetSetLLDBGlobals() ? Locker::InitGlobals : 0) |
               ((result && result->GetInteractive()) ? 0 : Locker::NoSTDIN),
-          Locker::FreeAcquiredLock | Locker::TearDownSession, in_file, out_file,
-          err_file);
+          Locker::FreeAcquiredLock | Locker::TearDownSession, input_file_sp,
+          output_file_sp->GetFileSP(), error_file_sp->GetFileSP());
 
       // Find the correct script interpreter dictionary in the main module.
       PythonDictionary &session_dict = GetSessionDictionary();
       if (session_dict.IsValid()) {
         if (GetEmbeddedInterpreterModuleObjects()) {
           if (PyCallable_Check(m_run_one_line_function.get())) {
             PythonObject pargs(
                 PyRefType::Owned,
bool success = false;
                 PyErr_Clear();
               }
             }
           }
         }
       }
 
       // Flush our output and error file handles
-      ::fflush(out_file);
-      if (out_file != err_file)
-        ::fflush(err_file);
+      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

lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h

public:
 
     enum OnLeave {
       FreeLock = 0x0001,
       FreeAcquiredLock = 0x0002, // do not free the lock if we already held it
                                  // when calling constructor
       TearDownSession = 0x0004
     };
 
-    Locker(ScriptInterpreterPythonImpl *py_interpreter = nullptr,
+    Locker(ScriptInterpreterPythonImpl *py_interpreter,
            uint16_t on_entry = AcquireLock | InitSession,
-           uint16_t on_leave = FreeLock | TearDownSession, FILE *in = nullptr,
-           FILE *out = nullptr, FILE *err = nullptr);
+           uint16_t on_leave = FreeLock | TearDownSession,
+           lldb::FileSP in = nullptr, lldb::FileSP out = nullptr,
+           lldb::FileSP err = nullptr);
 
     ~Locker() override;
 
   private:
     bool DoAcquireLock();
 
-    bool DoInitSession(uint16_t on_entry_flags, FILE *in, FILE *out, FILE *err);
+    bool DoInitSession(uint16_t on_entry_flags, lldb::FileSP in,
+                       lldb::FileSP out, lldb::FileSP err);
 
     bool DoFreeLock();
 
     bool DoTearDownSession();
 
     bool m_teardown_session;
     ScriptInterpreterPythonImpl *m_python_interpreter;
-    //      FILE*                    m_tmp_fh;
     PyGILState_STATE m_GILState;
   };
 
   static bool BreakpointCallbackFunction(void *baton,
                                          StoppointCallbackContext *context,
                                          lldb::user_id_t break_id,
                                          lldb::user_id_t break_loc_id);
   static bool WatchpointCallbackFunction(void *baton,
public:
 
     ~SynchronicityHandler();
   };
 
   enum class AddLocation { Beginning, End };
 
   static void AddToSysPath(AddLocation location, std::string path);
 
-  bool EnterSession(uint16_t on_entry_flags, FILE *in, FILE *out, FILE *err);
+  bool EnterSession(uint16_t on_entry_flags, lldb::FileSP in, lldb::FileSP out,
+                    lldb::FileSP err);
 
   void LeaveSession();
 
   uint32_t IsExecutingPython() const { return m_lock_count > 0; }
 
   uint32_t IncrementLockCount() { return ++m_lock_count; }
 
   uint32_t DecrementLockCount() {
public:
   PythonObject &GetMainModule();
 
   PythonDictionary &GetSessionDictionary();
 
   PythonDictionary &GetSysModuleDictionary();
 
   bool GetEmbeddedInterpreterModuleObjects();
 
-  bool SetStdHandle(File &file, const char *py_name, PythonFile &save_file,
-                    const char *mode);
+  bool SetStdHandle(lldb::FileSP file, const char *py_name,
+                    PythonObject &save_file, const char *mode);
 
-  PythonFile m_saved_stdin;
-  PythonFile m_saved_stdout;
-  PythonFile m_saved_stderr;
+  PythonObject m_saved_stdin;
+  PythonObject m_saved_stdout;
+  PythonObject m_saved_stderr;
   PythonObject m_main_module;
   PythonDictionary m_session_dict;
   PythonDictionary m_sys_module_dict;
   PythonObject m_run_one_line_function;
   PythonObject m_run_one_line_str_global;
   std::string m_dictionary_name;
   ActiveIOHandler m_active_io_handler;
   bool m_session_is_active;