[lldb-mi] Check raw pointers before passing them to std::string ctor/assignment

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

llvm-svn: 354798

12 files changed, 78 insertions, 23 deletions

diff --git a/lldb/tools/lldb-mi/MICmdCmdMiscellanous.cpp b/lldb/tools/lldb-mi/MICmdCmdMiscellanous.cpp
index 037f3731aee..ac25cece4e5 100644
--- a/lldb/tools/lldb-mi/MICmdCmdMiscellanous.cpp
+++ b/lldb/tools/lldb-mi/MICmdCmdMiscellanous.cpp
@@ -339,7 +339,9 @@ bool CMICmdCmdListThreadGroups::Acknowledge() {
       const char *pDir = sbTrgt.GetExecutable().GetDirectory();
       const char *pFileName = sbTrgt.GetExecutable().GetFilename();
       const CMIUtilString strFile(
-          CMIUtilString::Format("%s/%s", pDir, pFileName));
+          CMIUtilString::Format("%s/%s",
+                                CMIUtilString::WithNullAsEmpty(pDir),
+                                CMIUtilString::WithNullAsEmpty(pFileName)));
       const CMICmnMIValueConst miValueConst4(strFile);
       const CMICmnMIValueResult miValueResult4("executable", miValueConst4);
       miTuple.Add(miValueResult4);
diff --git a/lldb/tools/lldb-mi/MICmnLLDBDebugSessionInfo.cpp b/lldb/tools/lldb-mi/MICmnLLDBDebugSessionInfo.cpp
index c24ac94beb0..0ffb967b968 100644
--- a/lldb/tools/lldb-mi/MICmnLLDBDebugSessionInfo.cpp
+++ b/lldb/tools/lldb-mi/MICmnLLDBDebugSessionInfo.cpp
@@ -375,11 +375,9 @@ bool CMICmnLLDBDebugSessionInfo::MIResponseFormThreadInfo(
 
   // Add "target-id"
   const char *pThreadName = rThread.GetName();
-  const MIuint len =
-      (pThreadName != nullptr) ? CMIUtilString(pThreadName).length() : 0;
-  const bool bHaveName = ((pThreadName != nullptr) && (len > 0) && (len < 32) &&
-                          CMIUtilString::IsAllValidAlphaAndNumeric(
-                              pThreadName)); // 32 is arbitrary number
+  const MIuint len = CMIUtilString(pThreadName).length();
+  const bool bHaveName = (len > 0) && (len < 32) && // 32 is arbitrary number
+                         CMIUtilString::IsAllValidAlphaAndNumeric(pThreadName);
   const char *pThrdFmt = bHaveName ? "%s" : "Thread %d";
   CMIUtilString strThread;
   if (bHaveName)
diff --git a/lldb/tools/lldb-mi/MICmnLLDBDebugger.cpp b/lldb/tools/lldb-mi/MICmnLLDBDebugger.cpp
index 1f4fe13dadc..293ba8642b6 100644
--- a/lldb/tools/lldb-mi/MICmnLLDBDebugger.cpp
+++ b/lldb/tools/lldb-mi/MICmnLLDBDebugger.cpp
@@ -43,10 +43,11 @@ static inline bool MI_char_summary_provider(lldb::SBValue value,
 
   lldb::BasicType type_code = value_type.GetBasicType();
   if (type_code == lldb::eBasicTypeSignedChar)
-    stream.Printf("%d %s", (int)value.GetValueAsSigned(), value.GetValue());
+    stream.Printf("%d %s", (int)value.GetValueAsSigned(),
+                  CMIUtilString::WithNullAsEmpty(value.GetValue()));
   else if (type_code == lldb::eBasicTypeUnsignedChar)
     stream.Printf("%u %s", (unsigned)value.GetValueAsUnsigned(),
-                  value.GetValue());
+                  CMIUtilString::WithNullAsEmpty(value.GetValue()));
   else
     return false;
 
diff --git a/lldb/tools/lldb-mi/MICmnMIOutOfBandRecord.cpp b/lldb/tools/lldb-mi/MICmnMIOutOfBandRecord.cpp
index 7a1246b1ae7..778043aeb82 100644
--- a/lldb/tools/lldb-mi/MICmnMIOutOfBandRecord.cpp
+++ b/lldb/tools/lldb-mi/MICmnMIOutOfBandRecord.cpp
@@ -109,8 +109,10 @@ MapOutOfBandToToken(CMICmnMIOutOfBandRecord::OutOfBand_e veType) {
 //--
 static CMIUtilString
 BuildAsyncRecord(CMICmnMIOutOfBandRecord::OutOfBand_e veType) {
-  return CMIUtilString::Format("%s%s", MapOutOfBandToToken(veType),
-                               MapOutOfBandToText(veType));
+  auto Token = MapOutOfBandToToken(veType);
+  auto Text = MapOutOfBandToText(veType);
+  return CMIUtilString::Format("%s%s", CMIUtilString::WithNullAsEmpty(Token),
+                               CMIUtilString::WithNullAsEmpty(Text));
 }
 
 //++
diff --git a/lldb/tools/lldb-mi/MICmnMIResultRecord.cpp b/lldb/tools/lldb-mi/MICmnMIResultRecord.cpp
index 4dffb0b2365..b56c3ad84c1 100644
--- a/lldb/tools/lldb-mi/MICmnMIResultRecord.cpp
+++ b/lldb/tools/lldb-mi/MICmnMIResultRecord.cpp
@@ -53,7 +53,8 @@ static const CMIUtilString
 BuildResultRecord(const CMIUtilString &vrToken,
                   CMICmnMIResultRecord::ResultClass_e veType) {
   const char *pStrResultRecord = MapResultClassToResultClassText(veType);
-  return CMIUtilString::Format("%s^%s", vrToken.c_str(), pStrResultRecord);
+  return CMIUtilString::Format("%s^%s", vrToken.c_str(),
+                               CMIUtilString::WithNullAsEmpty(pStrResultRecord));
 }
 
 //++
diff --git a/lldb/tools/lldb-mi/MIDriverMgr.cpp b/lldb/tools/lldb-mi/MIDriverMgr.cpp
index c46d018c6d6..8dc1c648929 100644
--- a/lldb/tools/lldb-mi/MIDriverMgr.cpp
+++ b/lldb/tools/lldb-mi/MIDriverMgr.cpp
@@ -493,7 +493,8 @@ bool CMIDriverMgr::ParseArgs(const int argc, const char *argv[],
     bOk = bOk && m_pLog->Write(strArgs, CMICmnLog::eLogVerbosity_Log);
   } else {
     for (MIint i = 1; i < argc; i++) {
-      strArgs += CMIUtilString::Format("%d:'%s' ", i, argv[i]);
+      strArgs += CMIUtilString::Format("%d:'%s' ", i,
+                                       CMIUtilString::WithNullAsEmpty(argv[i]));
     }
     bOk = bOk && m_pLog->Write(strArgs, CMICmnLog::eLogVerbosity_Log);
   }
diff --git a/lldb/tools/lldb-mi/MIUtilString.cpp b/lldb/tools/lldb-mi/MIUtilString.cpp
index 6e3766d215f..27a39da1f06 100644
--- a/lldb/tools/lldb-mi/MIUtilString.cpp
+++ b/lldb/tools/lldb-mi/MIUtilString.cpp
@@ -37,7 +37,8 @@ CMIUtilString::CMIUtilString() : std::string() {}
 // Return:  None.
 // Throws:  None.
 //--
-CMIUtilString::CMIUtilString(const char *vpData) : std::string(vpData) {}
+CMIUtilString::CMIUtilString(const char *vpData)
+  : std::string(WithNullAsEmpty(vpData)) {}
 
 //++
 //------------------------------------------------------------------------------------
@@ -58,7 +59,7 @@ CMIUtilString::CMIUtilString(const std::string &vrStr) : std::string(vrStr) {}
 // Throws:  None.
 //--
 CMIUtilString &CMIUtilString::operator=(const char *vpRhs) {
-  assign(vpRhs);
+  assign(WithNullAsEmpty(vpRhs));
   return *this;
 }
 
@@ -103,12 +104,10 @@ CMIUtilString CMIUtilString::FormatPriv(const CMIUtilString &vrFormat,
   MIint n = vrFormat.size();
 
   // IOR: mysterious crash in this function on some windows builds not able to
-  // duplicate
-  // but found article which may be related. Crash occurs in vsnprintf() or
-  // va_copy()
+  // duplicate but found article which may be related. Crash occurs in
+  // vsnprintf() or va_copy().
   // Duplicate vArgs va_list argument pointer to ensure that it can be safely
-  // used in
-  // a new frame
+  // used in a new frame.
   // http://julipedia.meroh.net/2011/09/using-vacopy-to-safely-pass-ap.html
   va_list argsDup;
   va_copy(argsDup, vArgs);
@@ -127,8 +126,8 @@ CMIUtilString CMIUtilString::FormatPriv(const CMIUtilString &vrFormat,
     pFormatted.reset(new char[n + 1]); // +1 for safety margin
     ::strncpy(&pFormatted[0], vrFormat.c_str(), n);
 
-    //  We need to restore the variable argument list pointer to the start again
-    //  before running vsnprintf() more then once
+    // We need to restore the variable argument list pointer to the start again
+    // before running vsnprintf() more then once
     va_copy(argsDup, argsCpy);
 
     nFinal = ::vsnprintf(&pFormatted[0], n, vrFormat.c_str(), argsDup);
@@ -160,7 +159,8 @@ CMIUtilString CMIUtilString::FormatPriv(const CMIUtilString &vrFormat,
 CMIUtilString CMIUtilString::Format(const char *vFormating, ...) {
   va_list args;
   va_start(args, vFormating);
-  CMIUtilString strResult = CMIUtilString::FormatPriv(vFormating, args);
+  CMIUtilString strResult =
+      CMIUtilString::FormatPriv(WithNullAsEmpty(vFormating), args);
   va_end(args);
 
   return strResult;
@@ -457,7 +457,7 @@ bool CMIUtilString::ExtractNumberFromHexadecimal(MIint64 &vwrNumber) const {
 // Throws:  None.
 //--
 bool CMIUtilString::IsAllValidAlphaAndNumeric(const char *vpText) {
-  const size_t len = ::strlen(vpText);
+  const size_t len = ::strlen(WithNullAsEmpty(vpText));
   if (len == 0)
     return false;
 
diff --git a/lldb/tools/lldb-mi/MIUtilString.h b/lldb/tools/lldb-mi/MIUtilString.h
index 8685544cf29..e9d3af7a622 100644
--- a/lldb/tools/lldb-mi/MIUtilString.h
+++ b/lldb/tools/lldb-mi/MIUtilString.h
@@ -34,6 +34,9 @@ public:
   static CMIUtilString FormatValist(const CMIUtilString &vrFormating,
                                     va_list vArgs);
   static bool IsAllValidAlphaAndNumeric(const char *vpText);
+  static const char *WithNullAsEmpty(const char *vpText) {
+    return vpText ? vpText : "";
+  }
   static bool Compare(const CMIUtilString &vrLhs, const CMIUtilString &vrRhs);
   static CMIUtilString ConvertToPrintableASCII(const char vChar,
                                                bool bEscapeQuotes = false);
diff --git a/lldb/unittests/tools/CMakeLists.txt b/lldb/unittests/tools/CMakeLists.txt
index d180ebe8987..59b87864da5 100644
--- a/lldb/unittests/tools/CMakeLists.txt
+++ b/lldb/unittests/tools/CMakeLists.txt
@@ -1,3 +1,4 @@
+add_subdirectory(lldb-mi)
 if(CMAKE_SYSTEM_NAME MATCHES "Android|Darwin|Linux|NetBSD")
   if ((CMAKE_SYSTEM_NAME MATCHES "Darwin" AND SKIP_TEST_DEBUGSERVER) OR (NOT CMAKE_SYSTEM_NAME MATCHES "Darwin" AND SKIP_LLDB_SERVER_BUILD))
     # These tests are meant to test lldb-server/debugserver in isolation, and
diff --git a/lldb/unittests/tools/lldb-mi/CMakeLists.txt b/lldb/unittests/tools/lldb-mi/CMakeLists.txt
new file mode 100644
index 00000000000..1d211628bba
--- /dev/null
+++ b/lldb/unittests/tools/lldb-mi/CMakeLists.txt
@@ -0,0 +1,2 @@
+include_directories(${LLDB_SOURCE_DIR}/tools/lldb-mi)
+add_subdirectory(utils)
diff --git a/lldb/unittests/tools/lldb-mi/utils/CMakeLists.txt b/lldb/unittests/tools/lldb-mi/utils/CMakeLists.txt
new file mode 100644
index 00000000000..d6fb56abe58
--- /dev/null
+++ b/lldb/unittests/tools/lldb-mi/utils/CMakeLists.txt
@@ -0,0 +1,12 @@
+add_library(lldb-mi-utils OBJECT
+  ${LLDB_SOURCE_DIR}/tools/lldb-mi/MIUtilString.cpp
+  )
+
+add_lldb_unittest(LLDBMiUtilTests
+  StringTest.cpp
+
+  LINK_COMPONENTS
+    Support
+  )
+
+target_sources(LLDBMiUtilTests PRIVATE $<TARGET_OBJECTS:lldb-mi-utils>)
diff --git a/lldb/unittests/tools/lldb-mi/utils/StringTest.cpp b/lldb/unittests/tools/lldb-mi/utils/StringTest.cpp
new file mode 100644
index 00000000000..9f76f4601a0
--- /dev/null
+++ b/lldb/unittests/tools/lldb-mi/utils/StringTest.cpp
@@ -0,0 +1,32 @@
+//===-- StringTest.cpp ------------------------------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "MIUtilString.h"
+#include "gtest/gtest.h"
+
+TEST(StringTest, ConstructFromNullptr) {
+  auto str = CMIUtilString(nullptr);
+  EXPECT_TRUE(str.empty());
+  EXPECT_NE(nullptr, str.c_str());
+  EXPECT_EQ(CMIUtilString(""), str);
+}
+
+TEST(StringTest, AssignNullptr) {
+  CMIUtilString str;
+  str = nullptr;
+  EXPECT_TRUE(str.empty());
+  EXPECT_NE(nullptr, str.c_str());
+  EXPECT_EQ(CMIUtilString(""), str);
+}
+
+TEST(StringTest, IsAllValidAlphaAndNumeric) {
+  EXPECT_TRUE(CMIUtilString::IsAllValidAlphaAndNumeric("123abc"));
+  EXPECT_FALSE(CMIUtilString::IsAllValidAlphaAndNumeric(""));
+  EXPECT_FALSE(CMIUtilString::IsAllValidAlphaAndNumeric(nullptr));
+}
+