Fix some crashes and deadlocks in FormatAnsiTerminalCodes

[teemperor]

Summary

This patch fixes a few problems with the FormatAnsiTerminalCodes function:

• It does an infinite loop on an unknown color value.
• It crashes when the color value is at the end of the string.
• It deletes the first character behind the color token.

Also added a few tests that reproduce those problems (and test some other corner cases).

[labath]

This looks straight-forward enough.

unittests/Utility/AnsiTerminalTest.cpp

18

I would suggest getting rid of the temporary format variable. That way, if this assertion fails, the error message will immediately tell you what the failing command was.
(you can also use EXPECT_EQ here as operator== will do the right thing when one of the arguments is a std::string)

[teemperor]

• Removed temp string variables.

include/lldb/Utility/AnsiTerminal.h

while (!format.empty()) {
 
     fmt.append(left);
 
     if (left == format && right.empty()) {
       // The header was not found.  Just exit.
       break;
     }
 
+    bool found_code = false;
     for (const auto &code : codes) {
       if (!right.consume_front(code.name))
         continue;
 
       if (do_color)
         fmt.append(code.value);
-      format = right;
+      found_code = true;
       break;
     }
-
-    format = format.drop_front();
+    format = right;
+    // If we haven't found a valid replacement value, we just copy the string
+    // to the result without any modifications.
+    if (!found_code)
+      fmt.append(tok_hdr);
   }
   return fmt;
 }
 }
 }

unittests/Utility/AnsiTerminalTest.cpp

+//===-- AnsiTerminalTest.cpp ------------------------------------*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "gtest/gtest.h"
+
+#include "lldb/Utility/AnsiTerminal.h"
+
+using namespace lldb_utility;
+
+TEST(AnsiTerminal, Empty) {
+  std::string format = ansi::FormatAnsiTerminalCodes("");
+  EXPECT_STREQ("", format.c_str());
+}
+
+TEST(AnsiTerminal, WhiteSpace) {
+  std::string format = ansi::FormatAnsiTerminalCodes(" ");
+  EXPECT_STREQ(" ", format.c_str());
+}
+
+TEST(AnsiTerminal, AtEnd) {
+  std::string format = ansi::FormatAnsiTerminalCodes("abc${ansi.fg.black}");
+  EXPECT_STREQ("abc\x1B[30m", format.c_str());
+}
+
+TEST(AnsiTerminal, AtStart) {
+  std::string format = ansi::FormatAnsiTerminalCodes("${ansi.fg.black}abc");
+  EXPECT_STREQ("\x1B[30mabc", format.c_str());
+}
+
+TEST(AnsiTerminal, KnownPrefix) {
+  std::string format = ansi::FormatAnsiTerminalCodes("${ansi.fg.redish}abc");
+  EXPECT_STREQ("${ansi.fg.redish}abc", format.c_str());
+}
+
+TEST(AnsiTerminal, Unknown) {
+  std::string format = ansi::FormatAnsiTerminalCodes("${ansi.fg.foo}abc");
+  EXPECT_STREQ("${ansi.fg.foo}abc", format.c_str());
+}
+
+TEST(AnsiTerminal, Incomplete) {
+  std::string format = ansi::FormatAnsiTerminalCodes("abc${ansi.");
+  EXPECT_STREQ("abc${ansi.", format.c_str());
+}
+
+TEST(AnsiTerminal, Twice) {
+  std::string format =
+      ansi::FormatAnsiTerminalCodes("${ansi.fg.black}${ansi.fg.red}abc");
+  EXPECT_STREQ("\x1B[30m\x1B[31mabc", format.c_str());
+}
+
+TEST(AnsiTerminal, Basic) {
+  std::string format =
+      ansi::FormatAnsiTerminalCodes("abc${ansi.fg.red}abc${ansi.normal}abc");
+  EXPECT_STREQ("abc\x1B[31mabc\x1B[0mabc", format.c_str());
+}

unittests/Utility/CMakeLists.txt

 add_lldb_unittest(UtilityTests
+  AnsiTerminalTest.cpp
   ArgsTest.cpp
   OptionsWithRawTest.cpp
   ArchSpecTest.cpp
   CleanUpTest.cpp
   ConstStringTest.cpp
   CompletionRequestTest.cpp
   EnvironmentTest.cpp
   FileSpecTest.cpp