Merge pull request #10523 from swiftlang/cherrypick-swift/release/6.2-5f99e0d4b9ea-83116209331a

[🍒 swift/release/6.2] [lldb] Fix lock inversion between statusline mutex and output mutex (llvm#135956)

Author: JDevlieghere

lldb/source/Core/Statusline.cpp

@@ -28,6 +28,7 @@
 #define ANSI_CLEAR_LINE ESCAPE "[2K"
 #define ANSI_SET_SCROLL_ROWS ESCAPE "[0;%ur"
 #define ANSI_TO_START_OF_ROW ESCAPE "[%u;0f"
+#define ANSI_REVERSE_VIDEO ESCAPE "[7m"
 #define ANSI_UP_ROWS ESCAPE "[%dA"
 
 using namespace lldb;
@@ -75,6 +76,12 @@ void Statusline::Draw(std::string str) {
   locked_stream << ANSI_SAVE_CURSOR;
   locked_stream.Printf(ANSI_TO_START_OF_ROW,
                        static_cast<unsigned>(m_terminal_height));
+
+  // Use "reverse video" to make sure the statusline has a background. Only do
+  // this when colors are disabled, and rely on the statusline format otherwise.
+  if (!m_debugger.GetUseColor())
+    locked_stream << ANSI_REVERSE_VIDEO;
+
   locked_stream << str;
   locked_stream << ANSI_NORMAL;
   locked_stream << ANSI_RESTORE_CURSOR;

lldb/source/Host/common/Editline.cpp

@@ -567,9 +567,6 @@ int Editline::GetCharacter(EditLineGetCharType *c) {
     m_needs_prompt_repaint = false;
   }
 
-  if (m_redraw_callback)
-    m_redraw_callback();
-
   if (m_multiline_enabled) {
     // Detect when the number of rows used for this input line changes due to
     // an edit
@@ -585,54 +582,56 @@ int Editline::GetCharacter(EditLineGetCharType *c) {
     m_current_line_rows = new_line_rows;
   }
 
+  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
+  // Read returns, immediately lock the mutex again and check if we were
+  // interrupted.
+  m_locked_output.reset();
+
+  if (m_redraw_callback)
+    m_redraw_callback();
+
   // Read an actual character
-  while (true) {
-    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
-    // Read returns, immediately lock the mutex again and check if we were
-    // interrupted.
-    m_locked_output.reset();
-    int read_count =
-        m_input_connection.Read(&ch, 1, std::nullopt, status, nullptr);
-    m_locked_output.emplace(m_output_stream_sp->Lock());
-    if (m_editor_status == EditorStatus::Interrupted) {
-      while (read_count > 0 && status == lldb::eConnectionStatusSuccess)
-        read_count =
-            m_input_connection.Read(&ch, 1, std::nullopt, status, nullptr);
-      lldbassert(status == lldb::eConnectionStatusInterrupted);
-      return 0;
-    }
+  lldb::ConnectionStatus status = lldb::eConnectionStatusSuccess;
+  char ch = 0;
+  int read_count =
+      m_input_connection.Read(&ch, 1, std::nullopt, status, nullptr);
+
+  // Re-lock the output mutex to protected m_editor_status here and in the
+  // switch below.
+  m_locked_output.emplace(m_output_stream_sp->Lock());
+  if (m_editor_status == EditorStatus::Interrupted) {
+    while (read_count > 0 && status == lldb::eConnectionStatusSuccess)
+      read_count =
+          m_input_connection.Read(&ch, 1, std::nullopt, status, nullptr);
+    lldbassert(status == lldb::eConnectionStatusInterrupted);
+    return 0;
+  }
 
-    if (read_count) {
-      if (CompleteCharacter(ch, *c))
-        return 1;
-    } else {
-      switch (status) {
-      case lldb::eConnectionStatusSuccess: // Success
-        break;
+  if (read_count) {
+    if (CompleteCharacter(ch, *c))
+      return 1;
+    return 0;
+  }
 
-      case lldb::eConnectionStatusInterrupted:
-        llvm_unreachable("Interrupts should have been handled above.");
-
-      case lldb::eConnectionStatusError:        // Check GetError() for details
-      case lldb::eConnectionStatusTimedOut:     // Request timed out
-      case lldb::eConnectionStatusEndOfFile:    // End-of-file encountered
-      case lldb::eConnectionStatusNoConnection: // No connection
-      case lldb::eConnectionStatusLostConnection: // Lost connection while
-                                                  // connected to a valid
-                                                  // connection
-        m_editor_status = EditorStatus::EndOfInput;
-        return 0;
-      }
-    }
+  switch (status) {
+  case lldb::eConnectionStatusSuccess:
+    llvm_unreachable("Success should have resulted in positive read_count.");
+  case lldb::eConnectionStatusInterrupted:
+    llvm_unreachable("Interrupts should have been handled above.");
+  case lldb::eConnectionStatusError:
+  case lldb::eConnectionStatusTimedOut:
+  case lldb::eConnectionStatusEndOfFile:
+  case lldb::eConnectionStatusNoConnection:
+  case lldb::eConnectionStatusLostConnection:
+    m_editor_status = EditorStatus::EndOfInput;
   }
+
+  return 0;
 }
 
 const char *Editline::Prompt() {

lldb/test/API/functionalities/statusline/TestStatusline.py

@@ -55,3 +55,42 @@ def test(self):
         self.expect(
             "set set show-statusline false", ["\x1b[0;{}r".format(terminal_height)]
         )
+
+    # PExpect uses many timeouts internally and doesn't play well
+    # under ASAN on a loaded machine..
+    @skipIfAsan
+    def test_no_color(self):
+        """Basic test for the statusline with colors disabled."""
+        self.build()
+        self.launch(use_colors=False)
+        self.do_setup()
+
+        # Change the terminal dimensions.
+        terminal_height = 10
+        terminal_width = 60
+        self.child.setwinsize(terminal_height, terminal_width)
+
+        # Enable the statusline and check for the "reverse video" control character.
+        self.expect(
+            "set set show-statusline true",
+            [
+                "\x1b[7m",
+            ],
+        )
+
+    def test_deadlock(self):
+        """Regression test for lock inversion between the statusline mutex and
+        the output mutex."""
+        self.build()
+        self.launch(extra_args=["-o", "settings set use-color false"])
+        self.child.expect("(lldb)")
+
+        # Change the terminal dimensions.
+        terminal_height = 10
+        terminal_width = 60
+        self.child.setwinsize(terminal_height, terminal_width)
+
+        exe = self.getBuildArtifact("a.out")
+
+        self.expect("file {}".format(exe), ["Current executable"])
+        self.expect("help", ["Debugger commands"])