Save and restore previous terminal after setting the terminal for checking if terminal supports colors.

[augusto2112]

Summary

The call to "set_curterm" inside the "terminalHasColors" function breaks the EditLine configuration on some Linux distributions, causing certain characters that have functions bound to them to not show up and backspace to stop deleting characters (only visually). This patch ensures that term struct is restored after the routine for cheking if terminal supports colors is done, which fixes the aforementioned issue.

[augusto2112]

This is the second (and better) solution that addresses this issue. Previous discussions can be found here.

[labath]

This change seems obviously correct. We'll still need to be careful about using this functionality on the lldb side though. This fix will only help if there is proper synchronization between using this function and libedit initialization. If we still run into issues, we'll need to either add more synchronization, or do something different altogether.

Regarding testing, I suppose it should be possible to write a unit test which checks that this function really preserves the terminal value, but it's not clear to me how useful would that be....

llvm/lib/Support/Unix/Process.inc

361–362

Now that its purpose is also to restore the terminal value, this code is no longer that "silly".
I'd update this to something like:

// Restore the current terminal to the original value. Be sure to also free the structure allocated by setupterm.

[augusto2112]

We'll still need to be careful about using this functionality on the lldb side though. This fix will only help if there is proper synchronization between using this function and libedit initialization

The only way I think this could have adverse effects is if some component is relying on this function to clear the current settings, which would stop happening after this change.

@labath can this be merged or do we need someone else to approve it? I tagged everyone I found that could review it, but maybe we should add more people?

[labath]

Ownership of various Support components is a little bit unclear, but I think this is sufficiently obvious that I'll just go ahead and commit it for you. Thanks for your patience.

llvm/lib/Support/Unix/Process.inc

 static ManagedStatic<std::mutex> TermColorMutex;
 #endif
 
 static bool terminalHasColors(int fd) {
 #ifdef LLVM_ENABLE_TERMINFO
   // First, acquire a global lock because these C routines are thread hostile.
   std::lock_guard<std::mutex> G(*TermColorMutex);
 
+  struct term *previous_term = set_curterm(nullptr);
   int errret = 0;
   if (setupterm(nullptr, fd, &errret) != 0)
     // Regardless of why, if we can't get terminfo, we shouldn't try to print
     // colors.
     return false;
 
   // Test whether the terminal as set up supports color output. How to do this
   // isn't entirely obvious. We can use the curses routine 'has_colors' but it
   // would be nice to avoid a dependency on curses proper when we can make do
   // with a minimal terminfo parsing library. Also, we don't really care whether
   // the terminal supports the curses-specific color changing routines, merely
   // if it will interpret ANSI color escape codes in a reasonable way. Thus, the
   // strategy here is just to query the baseline colors capability and if it
   // supports colors at all to assume it will translate the escape codes into
   // whatever range of colors it does support. We can add more detailed tests
   // here if users report them as necessary.
   //
   // The 'tigetnum' routine returns -2 or -1 on errors, and might return 0 if
   // the terminfo says that no colors are supported.
   bool HasColors = tigetnum(const_cast<char *>("colors")) > 0;
 
   // Now extract the structure allocated by setupterm and free its memory
   // through a really silly dance.
-  struct term *termp = set_curterm(nullptr);
+  struct term *termp = set_curterm(previous_term);
   (void)del_curterm(termp); // Drop any errors here.
 
   // Return true if we found a color capabilities for the current terminal.
   if (HasColors)
     return true;
 #else
   // When the terminfo database is not available, check if the current terminal
   // is one of terminals that are known to support ANSI color escape codes.