[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

diff --git a/lldb/include/lldb/Core/IOHandler.h b/lldb/include/lldb/Core/IOHandler.h
index ff75bfb..539ba04 100644 (file)
--- a/lldb/include/lldb/Core/IOHandler.h
+++ b/lldb/include/lldb/Core/IOHandler.h
@@ -95,6 +95,8 @@ public:
 
   virtual void Deactivate() { m_active = false; }
 
+  virtual void TerminalSizeChanged() {}
+
   virtual const char *GetPrompt() {
     // Prompt support isn't mandatory
     return nullptr;
@@ -369,6 +371,8 @@ public:
 
   void Deactivate() override;
 
+  void TerminalSizeChanged() override;
+
   ConstString GetControlSequence(char ch) override {
     return m_delegate.IOHandlerGetControlSequence(ch);
   }

diff --git a/lldb/include/lldb/Host/Editline.h b/lldb/include/lldb/Host/Editline.h
index 6590a16..6e9daae 100644 (file)
--- a/lldb/include/lldb/Host/Editline.h
+++ b/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 LLDB_HOST_EDITLINE_H
@@ -50,6 +47,7 @@
 #include <histedit.h>
 #endif
 
+#include <csignal>
 #include <mutex>
 #include <string>
 #include <vector>
@@ -171,9 +169,7 @@ public:
   /// 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 @@ private:
 
   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 @@ private:
   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;

diff --git a/lldb/source/Core/Debugger.cpp b/lldb/source/Core/Debugger.cpp
index 1d4bf0d..546dc9e 100644 (file)
--- a/lldb/source/Core/Debugger.cpp
+++ b/lldb/source/Core/Debugger.cpp
@@ -315,6 +315,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);
 }

diff --git a/lldb/source/Core/IOHandler.cpp b/lldb/source/Core/IOHandler.cpp
index 933da6b..e29e70c 100644 (file)
--- a/lldb/source/Core/IOHandler.cpp
+++ b/lldb/source/Core/IOHandler.cpp
@@ -289,6 +289,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');

diff --git a/lldb/source/Host/common/Editline.cpp b/lldb/source/Host/common/Editline.cpp
index b72e951..56203e3 100644 (file)
--- a/lldb/source/Host/common/Editline.cpp
+++ b/lldb/source/Host/common/Editline.cpp
@@ -560,6 +560,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
@@ -1052,7 +1055,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()) {
@@ -1305,28 +1308,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;
   }
 }
 

diff --git a/lldb/test/API/iohandler/resize/TestIOHandlerResize.py b/lldb/test/API/iohandler/resize/TestIOHandlerResize.py
new file mode 100644 (file)
index 0000000..488553f
--- /dev/null
+++ b/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()