IOHandler: fall back on File::Read if a FILE* isn't available.

Summary:
IOHandler needs to read lines of input from a lldb::File.
The way it currently does this using, FILE*, which is something
we want to avoid now.   I'd prefer to just replace the FILE* code
with calls to File::Read, but it contains an awkward and
delicate workaround specific to ctrl-C handling on windows, and
it's not clear if or how that workaround would translate to
lldb::File.

So in this patch, we use use the FILE* if it's available, and only
fall back on File::Read if that's the only option.

I think this is a reasonable approach here for two reasons.  First
is that interactive terminal support is the one area where FILE*
can't be avoided.   We need them for libedit and curses anyway,
and using them here as well is consistent with that pattern.

The second reason is that the comments express a hope that the
underlying windows bug that's being worked around will be fixed one
day, so hopefully when that happens, that whole path can be deleted.

Reviewers: JDevlieghere, jasonmolenda, labath, lanza

Reviewed By: labath

Subscribers: lldb-commits

Tags: #lldb

Differential Revision: https://reviews.llvm.org/D68622

llvm-svn: 374576

lldb/include/lldb/Core/IOHandler.h

@@ -431,6 +431,7 @@ protected:
   bool m_interrupt_exits;
   bool m_editing; // Set to true when fetching a line manually (not using
                   // libedit)
+  std::string m_line_buffer;
 };
 
 // The order of base classes is important. Look at the constructor of

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

@@ -399,9 +399,9 @@ class FileHandleTestCase(lldbtest.TestBase):
 
 
     @add_test_categories(['pyapi'])
-    @expectedFailureAll() # FIXME IOHandler still using FILE*
+    @skipIf(py_version=['<', (3,)])
     def test_string_inout(self):
-        inf = io.StringIO("help help\n")
+        inf = io.StringIO("help help\np/x ~0\n")
         outf = io.StringIO()
         status = self.debugger.SetOutputFile(lldb.SBFile(outf))
         self.assertTrue(status.Success())
@@ -412,10 +412,11 @@ class FileHandleTestCase(lldbtest.TestBase):
         self.debugger.GetOutputFile().Flush()
         output = outf.getvalue()
         self.assertIn('Show a list of all debugger commands', output)
+        self.assertIn('0xfff', output)
 
 
     @add_test_categories(['pyapi'])
-    @expectedFailureAll() # FIXME IOHandler still using FILE*
+    @skipIf(py_version=['<', (3,)])
     def test_bytes_inout(self):
         inf = io.BytesIO(b"help help\nhelp b\n")
         outf = io.BytesIO()

lldb/source/Core/IOHandler.cpp

@@ -70,6 +70,10 @@
 
 using namespace lldb;
 using namespace lldb_private;
+using llvm::None;
+using llvm::Optional;
+using llvm::StringRef;
+
 
 IOHandler::IOHandler(Debugger &debugger, IOHandler::Type type)
     : IOHandler(debugger, type,
@@ -306,94 +310,119 @@ void IOHandlerEditline::Deactivate() {
   m_delegate.IOHandlerDeactivated(*this);
 }
 
+// Split out a line from the buffer, if there is a full one to get.
+static Optional<std::string> SplitLine(std::string &line_buffer) {
+  size_t pos = line_buffer.find('\n');
+  if (pos == std::string::npos)
+    return None;
+  std::string line = StringRef(line_buffer.c_str(), pos).rtrim("\n\r");
+  line_buffer = line_buffer.substr(pos + 1);
+  return line;
+}
+
+// If the final line of the file ends without a end-of-line, return
+// it as a line anyway.
+static Optional<std::string> SplitLineEOF(std::string &line_buffer) {
+  if (llvm::all_of(line_buffer, isspace))
+    return None;
+  std::string line = std::move(line_buffer);
+  line_buffer.clear();
+  return line;
+}
+
 bool IOHandlerEditline::GetLine(std::string &line, bool &interrupted) {
 #ifndef LLDB_DISABLE_LIBEDIT
   if (m_editline_up) {
     bool b = m_editline_up->GetLine(line, interrupted);
-    if (m_data_recorder)
+    if (b && m_data_recorder)
       m_data_recorder->Record(line, true);
     return b;
-  } else {
-#endif
-    line.clear();
-
-    FILE *in = GetInputFILE();
-    if (in) {
-      if (GetIsInteractive()) {
-        const char *prompt = nullptr;
-
-        if (m_multi_line && m_curr_line_idx > 0)
-          prompt = GetContinuationPrompt();
-
-        if (prompt == nullptr)
-          prompt = GetPrompt();
-
-        if (prompt && prompt[0]) {
-          if (m_output_sp) {
-            m_output_sp->Printf("%s", prompt);
-            m_output_sp->Flush();
-          }
-        }
-      }
-      char buffer[256];
-      bool done = false;
-      bool got_line = false;
-      m_editing = true;
-      while (!done) {
-#ifdef _WIN32
-        // ReadFile on Windows is supposed to set ERROR_OPERATION_ABORTED
-        // according to the docs on MSDN. However, this has evidently been a
-        // known bug since Windows 8. Therefore, we can't detect if a signal
-        // interrupted in the fgets. So pressing ctrl-c causes the repl to end
-        // and the process to exit. A temporary workaround is just to attempt to
-        // fgets twice until this bug is fixed.
-        if (fgets(buffer, sizeof(buffer), in) == nullptr &&
-            fgets(buffer, sizeof(buffer), in) == nullptr) {
-          // this is the equivalent of EINTR for Windows
-          if (GetLastError() == ERROR_OPERATION_ABORTED)
-            continue;
-#else
-      if (fgets(buffer, sizeof(buffer), in) == nullptr) {
-#endif
-          const int saved_errno = errno;
-          if (feof(in))
-            done = true;
-          else if (ferror(in)) {
-            if (saved_errno != EINTR)
-              done = true;
-          }
-        } else {
-          got_line = true;
-          size_t buffer_len = strlen(buffer);
-          assert(buffer[buffer_len] == '\0');
-          char last_char = buffer[buffer_len - 1];
-          if (last_char == '\r' || last_char == '\n') {
-            done = true;
-            // Strip trailing newlines
-            while (last_char == '\r' || last_char == '\n') {
-              --buffer_len;
-              if (buffer_len == 0)
-                break;
-              last_char = buffer[buffer_len - 1];
-            }
-          }
-          line.append(buffer, buffer_len);
-        }
-      }
-      m_editing = false;
-      if (m_data_recorder && got_line)
-        m_data_recorder->Record(line, true);
-      // We might have gotten a newline on a line by itself make sure to return
-      // true in this case.
-      return got_line;
-    } else {
-      // No more input file, we are done...
-      SetIsDone(true);
-    }
-    return false;
-#ifndef LLDB_DISABLE_LIBEDIT
   }
 #endif
+
+  line.clear();
+
+  if (GetIsInteractive()) {
+    const char *prompt = nullptr;
+
+    if (m_multi_line && m_curr_line_idx > 0)
+      prompt = GetContinuationPrompt();
+
+    if (prompt == nullptr)
+      prompt = GetPrompt();
+
+    if (prompt && prompt[0]) {
+      if (m_output_sp) {
+        m_output_sp->Printf("%s", prompt);
+        m_output_sp->Flush();
+      }
+    }
+  }
+
+  Optional<std::string> got_line = SplitLine(m_line_buffer);
+
+  if (!got_line && !m_input_sp) {
+    // No more input file, we are done...
+    SetIsDone(true);
+    return false;
+  }
+
+  FILE *in = GetInputFILE();
+  char buffer[256];
+
+  if (!got_line && !in && m_input_sp) {
+    // there is no FILE*, fall back on just reading bytes from the stream.
+    while (!got_line) {
+      size_t bytes_read = sizeof(buffer);
+      Status error = m_input_sp->Read((void *)buffer, bytes_read);
+      if (error.Success() && !bytes_read) {
+        got_line = SplitLineEOF(m_line_buffer);
+        break;
+      }
+      if (error.Fail())
+        break;
+      m_line_buffer += StringRef(buffer, bytes_read);
+      got_line = SplitLine(m_line_buffer);
+    }
+  }
+
+  if (!got_line && in) {
+    m_editing = true;
+    while (!got_line) {
+      char *r = fgets(buffer, sizeof(buffer), in);
+#ifdef _WIN32
+      // ReadFile on Windows is supposed to set ERROR_OPERATION_ABORTED
+      // according to the docs on MSDN. However, this has evidently been a
+      // known bug since Windows 8. Therefore, we can't detect if a signal
+      // interrupted in the fgets. So pressing ctrl-c causes the repl to end
+      // and the process to exit. A temporary workaround is just to attempt to
+      // fgets twice until this bug is fixed.
+      if (r == nullptr)
+        r = fgets(buffer, sizeof(buffer), in);
+      // this is the equivalent of EINTR for Windows
+      if (r == nullptr && GetLastError() == ERROR_OPERATION_ABORTED)
+        continue;
+#endif
+      if (r == nullptr) {
+        if (ferror(in) && errno == EINTR)
+          continue;
+        if (feof(in))
+          got_line = SplitLineEOF(m_line_buffer);
+        break;
+      }
+      m_line_buffer += buffer;
+      got_line = SplitLine(m_line_buffer);
+    }
+    m_editing = false;
+  }
+
+  if (got_line) {
+    line = got_line.getValue();
+    if (m_data_recorder)
+      m_data_recorder->Record(line, true);
+  }
+
+  return (bool)got_line;
 }
 
 #ifndef LLDB_DISABLE_LIBEDIT