[lldb/Driver] Support terminal resizing

Summary:
The comment in the Editine.h header made it sound like editline was
just unable to handle terminal resizing. We were not ever telling
editline that the terminal had changed size, which might explain why
it wasn't working.

This patch threads a `TerminalSizeChanged()` callback through the
IOHandler and invokes it from the SIGWINCH handler in the driver. Our
`Editline` class already had a `TerminalSizeChanged()` method which
was invoked only when editline was configured.

This patch also changes `Editline` to not apply the changes right away
in `TerminalSizeChanged()`, but instead defer that to the next
character read. During my testing, it happened once that the signal
was received while our `ConnectionFileDescriptor::Read` was allocating
memory. As `el_resize` seems to allocate memory too, this crashed.

Reviewers: labath, teemperor

Subscribers: lldb-commits

Tags: #lldb

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

(cherry picked from commit d9166ad)

Author: fredriss

lldb/include/lldb/Core/IOHandler.h

@@ -95,6 +95,8 @@ class IOHandler {
 
   virtual void Deactivate() { m_active = false; }
 
+  virtual void TerminalSizeChanged() {}
+
   virtual const char *GetPrompt() {
     // Prompt support isn't mandatory
     return nullptr;
@@ -369,6 +371,8 @@ class IOHandlerEditline : public IOHandler {
 
   void Deactivate() override;
 
+  void TerminalSizeChanged() override;
+
   ConstString GetControlSequence(char ch) override {
     return m_delegate.IOHandlerGetControlSequence(ch);
   }

lldb/include/lldb/Host/Editline.h

@@ -19,13 +19,10 @@
 //    good amount of the text will
 //    disappear.  It's still in the buffer, just invisible.
 // b) The prompt printing logic for dealing with ANSI formatting characters is
-// broken, which is why we're
-//    working around it here.
-// c) When resizing the terminal window, if the cursor moves between rows
-// libedit will get confused. d) The incremental search uses escape to cancel
-// input, so it's confused by
+// broken, which is why we're working around it here.
+// c) The incremental search uses escape to cancel input, so it's confused by
 // ANSI sequences starting with escape.
-// e) Emoji support is fairly terrible, presumably it doesn't understand
+// d) Emoji support is fairly terrible, presumably it doesn't understand
 // composed characters?
 
 #ifndef liblldb_Editline_h_
@@ -50,6 +47,7 @@
 #include <histedit.h>
 #endif
 
+#include <csignal>
 #include <mutex>
 #include <string>
 #include <vector>
@@ -171,9 +169,7 @@ class Editline {
   /// editing scenarios.
   void SetContinuationPrompt(const char *continuation_prompt);
 
-  /// Required to update the width of the terminal registered for I/O.  It is
-  /// critical that this
-  /// be correct at all times.
+  /// Call when the terminal size changes
   void TerminalSizeChanged();
 
   /// Returns the prompt established by SetPrompt()
@@ -328,6 +324,8 @@ class Editline {
 
   bool CompleteCharacter(char ch, EditLineGetCharType &out);
 
+  void ApplyTerminalSizeChange();
+
 private:
 #if LLDB_EDITLINE_USE_WCHAR
   std::wstring_convert<std::codecvt_utf8<wchar_t>> m_utf8conv;
@@ -350,6 +348,7 @@ class Editline {
   std::string m_set_continuation_prompt;
   std::string m_current_prompt;
   bool m_needs_prompt_repaint = false;
+  volatile std::sig_atomic_t m_terminal_size_has_changed = 0;
   std::string m_editor_name;
   FILE *m_input_file;
   FILE *m_output_file;

lldb/source/Core/Debugger.cpp

@@ -388,6 +388,9 @@ uint32_t Debugger::GetTerminalWidth() const {
 }
 
 bool Debugger::SetTerminalWidth(uint32_t term_width) {
+  if (auto handler_sp = m_io_handler_stack.Top())
+    handler_sp->TerminalSizeChanged();
+
   const uint32_t idx = ePropertyTerminalWidth;
   return m_collection_sp->SetPropertyAtIndexAsSInt64(nullptr, idx, term_width);
 }

lldb/source/Core/IOHandler.cpp

@@ -287,6 +287,12 @@ void IOHandlerEditline::Deactivate() {
   m_delegate.IOHandlerDeactivated(*this);
 }
 
+void IOHandlerEditline::TerminalSizeChanged() {
+#if LLDB_ENABLE_LIBEDIT
+  m_editline_up->TerminalSizeChanged();
+#endif
+}
+
 // 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');

lldb/source/Host/common/Editline.cpp

@@ -563,6 +563,9 @@ int Editline::GetCharacter(EditLineGetCharType *c) {
     lldb::ConnectionStatus status = lldb::eConnectionStatusSuccess;
     char ch = 0;
 
+    if (m_terminal_size_has_changed)
+      ApplyTerminalSizeChange();
+
     // This mutex is locked by our caller (GetLine). Unlock it while we read a
     // character (blocking operation), so we do not hold the mutex
     // indefinitely. This gives a chance for someone to interrupt us. After
@@ -1055,7 +1058,7 @@ void Editline::ConfigureEditor(bool multiline) {
 
   m_editline =
       el_init(m_editor_name.c_str(), m_input_file, m_output_file, m_error_file);
-  TerminalSizeChanged();
+  ApplyTerminalSizeChange();
 
   if (m_history_sp && m_history_sp->IsValid()) {
     if (!m_history_sp->Load()) {
@@ -1308,28 +1311,32 @@ void Editline::SetContinuationPrompt(const char *continuation_prompt) {
       continuation_prompt == nullptr ? "" : continuation_prompt;
 }
 
-void Editline::TerminalSizeChanged() {
-  if (m_editline != nullptr) {
-    el_resize(m_editline);
-    int columns;
-    // This function is documenting as taking (const char *, void *) for the
-    // vararg part, but in reality in was consuming arguments until the first
-    // null pointer. This was fixed in libedit in April 2019
-    // <http://mail-index.netbsd.org/source-changes/2019/04/26/msg105454.html>,
-    // but we're keeping the workaround until a version with that fix is more
-    // widely available.
-    if (el_get(m_editline, EL_GETTC, "co", &columns, nullptr) == 0) {
-      m_terminal_width = columns;
-      if (m_current_line_rows != -1) {
-        const LineInfoW *info = el_wline(m_editline);
-        int lineLength =
-            (int)((info->lastchar - info->buffer) + GetPromptWidth());
-        m_current_line_rows = (lineLength / columns) + 1;
-      }
-    } else {
-      m_terminal_width = INT_MAX;
-      m_current_line_rows = 1;
+void Editline::TerminalSizeChanged() { m_terminal_size_has_changed = 1; }
+
+void Editline::ApplyTerminalSizeChange() {
+  if (!m_editline)
+    return;
+
+  m_terminal_size_has_changed = 0;
+  el_resize(m_editline);
+  int columns;
+  // This function is documenting as taking (const char *, void *) for the
+  // vararg part, but in reality in was consuming arguments until the first
+  // null pointer. This was fixed in libedit in April 2019
+  // <http://mail-index.netbsd.org/source-changes/2019/04/26/msg105454.html>,
+  // but we're keeping the workaround until a version with that fix is more
+  // widely available.
+  if (el_get(m_editline, EL_GETTC, "co", &columns, nullptr) == 0) {
+    m_terminal_width = columns;
+    if (m_current_line_rows != -1) {
+      const LineInfoW *info = el_wline(m_editline);
+      int lineLength =
+          (int)((info->lastchar - info->buffer) + GetPromptWidth());
+      m_current_line_rows = (lineLength / columns) + 1;
     }
+  } else {
+    m_terminal_width = INT_MAX;
+    m_current_line_rows = 1;
   }
 }
 

lldb/test/API/iohandler/resize/TestIOHandlerResize.py

@@ -0,0 +1,36 @@
+"""
+Test resizing in our IOHandlers.
+"""
+
+import os
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test.lldbpexpect import PExpectTest
+
+class IOHandlerCompletionTest(PExpectTest):
+
+    mydir = TestBase.compute_mydir(__file__)
+
+    # PExpect uses many timeouts internally and doesn't play well
+    # under ASAN on a loaded machine..
+    @skipIfAsan
+    @skipIfEditlineSupportMissing
+    def test_resize(self):
+
+        # Start with a small window
+        self.launch(dimensions=(10,10))
+
+        self.child.send("his is a long sentence missing its first letter.")
+
+        # Now resize to something bigger
+        self.child.setwinsize(100,500)
+
+        # Hit "left" 60 times (to go to the beginning of the line) and insert
+        # a character.
+        self.child.send(60 * "\033[D")
+        self.child.send("T")
+
+        self.child.expect_exact("(lldb) This is a long sentence missing its first letter.")
+        self.quit()