[Lldb-commits] [lldb] r230654 - Fix handling of double quotes (MI)

Ilia K ki.stfu at gmail.com
Thu Feb 26 10:21:23 PST 2015


Author: ki.stfu
Date: Thu Feb 26 12:21:22 2015
New Revision: 230654

URL: http://llvm.org/viewvc/llvm-project?rev=230654&view=rev
Log:
Fix handling of double quotes (MI)

Summary:
* Clean CMICmdArgValString::Validate: now it's based on CMIUtilString::SplitConsiderQuotes method:
A bit of introduction:
# Command line is wrapped into CMICmdArgContext.
# CMICmdArgSet is a set of arguments to be parsed. This class contains CMICmdArgContext as a private member.
# MI command is class which is inhereted from CMICmdBase. It contains CMICmdArgSet as a private member.

When command is executed CMICmdBase::ParseArgs() is called. This method adds args for parsing using CMICmdArgSet::Add(). Then CMICmdBase::ParseValidateCmdOptions() is called, which calls CMICmdArgSet::Validate(). Then it gets a number of arguments (using SplitConsiderQuotes().array_length) and for each arguments registered in ParseArgs() tries to validate it using CMICmdArgValBase::Validate(). Every user commands parses this string again (first time it was made in SplitConsiderQuotes) and in case of CMICmdArgValString it was made incorrectly. It searches the first and last quotes (but it should be first and next after first). Besides, it was splitted into 4 cases. 
I'm just using SplitConsiderQuotes directly, and I don't split them by hand again. 

Actually, I think we should do so in every CMICmdArgVal_XXX::Validate() method.

* Enable MiInterpreterExecTestCase.test_lldbmi_target_create test
* Fix MiExecTestCase.test_lldbmi_exec_arguments_set test

All tests pass on OS X.

Reviewers: abidh, emaste, zturner, clayborg

Reviewed By: clayborg

Subscribers: lldb-commits, zturner, emaste, clayborg, abidh

Differential Revision: http://reviews.llvm.org/D7860

Modified:
    lldb/trunk/test/tools/lldb-mi/TestMiInterpreterExec.py
    lldb/trunk/test/tools/lldb-mi/control/TestMiExec.py
    lldb/trunk/tools/lldb-mi/MICmdArgValString.cpp
    lldb/trunk/tools/lldb-mi/MICmdCmdSymbol.cpp
    lldb/trunk/tools/lldb-mi/MIDriver.cpp
    lldb/trunk/tools/lldb-mi/MIUtilString.cpp
    lldb/trunk/tools/lldb-mi/MIUtilString.h

Modified: lldb/trunk/test/tools/lldb-mi/TestMiInterpreterExec.py
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/test/tools/lldb-mi/TestMiInterpreterExec.py?rev=230654&r1=230653&r2=230654&view=diff
==============================================================================
--- lldb/trunk/test/tools/lldb-mi/TestMiInterpreterExec.py (original)
+++ lldb/trunk/test/tools/lldb-mi/TestMiInterpreterExec.py Thu Feb 26 12:21:22 2015
@@ -12,7 +12,6 @@ class MiInterpreterExecTestCase(lldbmi_t
 
     @lldbmi_test
     @expectedFailureWindows("llvm.org/pr22274: need a pexpect replacement for windows")
-    @unittest2.skip("lldb-mi handles double quotes in passed app path incorrectly")
     def test_lldbmi_target_create(self):
         """Test that 'lldb-mi --interpreter' can create target by 'target create' command."""
 

Modified: lldb/trunk/test/tools/lldb-mi/control/TestMiExec.py
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/test/tools/lldb-mi/control/TestMiExec.py?rev=230654&r1=230653&r2=230654&view=diff
==============================================================================
--- lldb/trunk/test/tools/lldb-mi/control/TestMiExec.py (original)
+++ lldb/trunk/test/tools/lldb-mi/control/TestMiExec.py Thu Feb 26 12:21:22 2015
@@ -97,8 +97,7 @@ class MiExecTestCase(lldbmi_testcase.MiT
         #self.runCmd("-data-evaluate-expression argv[2]")
         #self.expect("\^done,value=\"2nd arg\"")
         self.runCmd("-interpreter-exec command \"print argv[2]\"")
-        #FIXME: lldb-mi doesn't handle inner quotes
-        self.expect("\"\\\\\\\"2nd arg\\\\\\\"\"") #FIXME: self.expect("\"2nd arg\"")
+        self.expect("\"2nd arg\"")
         #self.runCmd("-data-evaluate-expression argv[3]")
         #self.expect("\^done,value=\"third_arg\"")
         self.runCmd("-interpreter-exec command \"print argv[3]\"")

Modified: lldb/trunk/tools/lldb-mi/MICmdArgValString.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/tools/lldb-mi/MICmdArgValString.cpp?rev=230654&r1=230653&r2=230654&view=diff
==============================================================================
--- lldb/trunk/tools/lldb-mi/MICmdArgValString.cpp (original)
+++ lldb/trunk/tools/lldb-mi/MICmdArgValString.cpp Thu Feb 26 12:21:22 2015
@@ -46,7 +46,7 @@ CMICmdArgValString::CMICmdArgValString(v
 // Throws:  None.
 //--
 CMICmdArgValString::CMICmdArgValString(const bool vbAnything)
-    : m_bHandleQuotedString(false)
+    : m_bHandleQuotedString(vbAnything ? true : false)
     , m_bAcceptNumbers(false)
     , m_bHandleDirPaths(false)
     , m_bHandleAnything(vbAnything)
@@ -123,7 +123,7 @@ CMICmdArgValString::Validate(CMICmdArgCo
         return MIstatus::success;
 
     if (m_bHandleQuotedString)
-        return (ValidateQuotedText(vrwArgContext) || ValidateQuotedTextEmbedded(vrwArgContext));
+        return ValidateQuotedText(vrwArgContext);
 
     return ValidateSingleText(vrwArgContext);
 }
@@ -140,22 +140,6 @@ CMICmdArgValString::Validate(CMICmdArgCo
 bool
 CMICmdArgValString::ValidateSingleText(CMICmdArgContext &vrwArgContext)
 {
-    if (vrwArgContext.GetNumberArgsPresent() == 1)
-    {
-        const CMIUtilString &rArg(vrwArgContext.GetArgsLeftToParse());
-        if (IsStringArg(rArg))
-        {
-            m_bFound = true;
-            m_bValid = true;
-            m_argValue = rArg;
-            vrwArgContext.RemoveArg(rArg);
-            return MIstatus::success;
-        }
-        else
-            return MIstatus::failure;
-    }
-
-    // More than one option...
     const CMIUtilString::VecString_t vecOptions(vrwArgContext.GetArgs());
     CMIUtilString::VecString_t::const_iterator it = vecOptions.begin();
     while (it != vecOptions.end())
@@ -168,7 +152,7 @@ CMICmdArgValString::ValidateSingleText(C
             if (vrwArgContext.RemoveArg(rArg))
             {
                 m_bValid = true;
-                m_argValue = rArg;
+                m_argValue = rArg.StripSlashes();
                 return MIstatus::success;
             }
             else
@@ -184,8 +168,7 @@ CMICmdArgValString::ValidateSingleText(C
 
 //++ ------------------------------------------------------------------------------------
 // Details: Parse the command's argument options string and try to extract all the words
-//          between quotes then delimited by the next space. Can fall through to
-//          ValidateSingleText() or ValidateQuotedQuotedTextEmbedded().
+//          between quotes then delimited by the next space.
 // Type:    Method.
 // Args:    vrwArgContext   - (RW) The command's argument options string.
 // Return:  MIstatus::success - Functional succeeded.
@@ -195,148 +178,21 @@ CMICmdArgValString::ValidateSingleText(C
 bool
 CMICmdArgValString::ValidateQuotedText(CMICmdArgContext &vrwArgContext)
 {
-    // CODETAG_QUOTEDTEXT_SIMILAR_CODE
-    CMIUtilString strOptions = vrwArgContext.GetArgsLeftToParse();
-    const MIchar cQuote = '"';
-
-    // Look for first quote of two
-    MIint nPos = strOptions.find(cQuote);
-    if (nPos == (MIint)std::string::npos)
-        return ValidateSingleText(vrwArgContext);
-
-    // Is one and only quote at end of the string
-    const MIint nLen = strOptions.length();
-    if (nPos == (MIint)(nLen - 1))
-        return MIstatus::failure;
-
-    // Quote must be the first character in the string or be preceeded by a space
-    if ((nPos > 0) && (strOptions[nPos - 1] != ' '))
-        return MIstatus::failure;
-
-    // Need to find the other quote
-    const MIint nPos2 = strOptions.rfind(cQuote);
-    if (nPos2 == (MIint)std::string::npos)
-        return MIstatus::failure;
-
-    // Is there quotes surrounding string formatting embedded quotes
-    if (IsStringArgQuotedQuotedTextEmbedded(strOptions))
-        return ValidateQuotedQuotedTextEmbedded(vrwArgContext);
-
-    // Make sure not same back quote, need two quotes
-    if (nPos == nPos2)
-        return MIstatus::failure;
-
-    // Extract quoted text
-    const CMIUtilString strQuotedTxt = strOptions.substr(nPos, nPos2 - nPos + 1).c_str();
-    if (vrwArgContext.RemoveArg(strQuotedTxt))
-    {
-        m_bFound = true;
-        m_bValid = true;
-        m_argValue = strOptions.substr(nPos + 1, nPos2 - nPos - 1).c_str();
-        return MIstatus::success;
-    }
-
-    return MIstatus::failure;
-}
-
-//++ ------------------------------------------------------------------------------------
-// Details: Parse the command's argument options string and try to extract all the words
-//          between quotes then delimited by the next space. If there any string format
-//          characters '\\' used to embed quotes these are ignored i.e. "\\\"%5d\\\""
-//          becomes "%5d". Can fall through to ValidateQuotedText().
-// Type:    Method.
-// Args:    vrwArgContext   - (RW) The command's argument options string.
-// Return:  MIstatus::success - Functional succeeded.
-//          MIstatus::failure - Functional failed.
-// Throws:  None.
-//--
-bool
-CMICmdArgValString::ValidateQuotedTextEmbedded(CMICmdArgContext &vrwArgContext)
-{
-    // CODETAG_QUOTEDTEXT_SIMILAR_CODE
-    CMIUtilString strOptions = vrwArgContext.GetArgsLeftToParse();
-    const MIchar cBckSlash = '\\';
-    const MIint nPos = strOptions.find(cBckSlash);
-    if (nPos == (MIint)std::string::npos)
-        return ValidateQuotedText(vrwArgContext);
-
-    // Back slash must be the first character in the string or be preceeded by a space
-    // or '\\'
-    const MIchar cSpace = ' ';
-    if ((nPos > 0) && (strOptions[nPos - 1] != cSpace))
-        return MIstatus::failure;
-
-    // Need to find the other back slash
-    const MIint nPos2 = strOptions.rfind(cBckSlash);
-    if (nPos2 == (MIint)std::string::npos)
-        return MIstatus::failure;
-
-    // Make sure not same back slash, need two slashs
-    if (nPos == nPos2)
-        return MIstatus::failure;
-
-    // Look for the two quotes
-    const MIint nLen = strOptions.length();
-    const MIchar cQuote = '"';
-    const MIint nPosQuote1 = nPos + 1;
-    const MIint nPosQuote2 = (nPos2 < nLen) ? nPos2 + 1 : nPos2;
-    if ((nPosQuote1 != nPosQuote2) && (strOptions[nPosQuote1] != cQuote) && (strOptions[nPosQuote2] != cQuote))
-        return MIstatus::failure;
-
-    // Extract quoted text
-    const CMIUtilString strQuotedTxt = strOptions.substr(nPos, nPosQuote2 - nPos + 1).c_str();
-    if (vrwArgContext.RemoveArg(strQuotedTxt))
-    {
-        m_bFound = true;
-        m_bValid = true;
-        m_argValue = strQuotedTxt;
-        return MIstatus::success;
-    }
-
-    return MIstatus::failure;
-}
-
-//++ ------------------------------------------------------------------------------------
-// Details: Parse the command's argument options string and try to extract all the words
-//          between quotes then delimited by the next space. If there any string format
-//          characters '\\' used to embed quotes these are ignored i.e. "\\\"%5d\\\""
-//          becomes "%5d".
-// Type:    Method.
-// Args:    vrwArgContext   - (RW) The command's argument options string.
-// Return:  MIstatus::success - Functional succeeded.
-//          MIstatus::failure - Functional failed.
-// Throws:  None.
-//--
-bool
-CMICmdArgValString::ValidateQuotedQuotedTextEmbedded(CMICmdArgContext &vrwArgContext)
-{
-    // CODETAG_QUOTEDTEXT_SIMILAR_CODE
-    CMIUtilString strOptions = vrwArgContext.GetArgsLeftToParse();
-    const MIint nPos = strOptions.find("\"\\\"");
-    if (nPos == (MIint)std::string::npos)
-        return MIstatus::failure;
-
-    const MIint nPos2 = strOptions.rfind("\\\"\"");
-    if (nPos2 == (MIint)std::string::npos)
+    const CMIUtilString::VecString_t vecOptions(vrwArgContext.GetArgs());
+    if (vecOptions.size() == 0)
         return MIstatus::failure;
 
-    const MIint nLen = strOptions.length();
-    if ((nLen > 5) && ((nPos + 2) == (nPos2 - 2)))
+    const CMIUtilString &rArg(vecOptions[0]);
+    if (!IsStringArg(rArg))
         return MIstatus::failure;
 
-    // Quote must be the first character in the string or be preceeded by a space
-    // or '\\'
-    const MIchar cSpace = ' ';
-    if ((nPos > 0) && (strOptions[nPos - 1] != cSpace))
-        return MIstatus::failure;
+    m_bFound = true;
 
-    // Extract quoted text
-    const CMIUtilString strQuotedTxt = strOptions.substr(nPos, nPos2 - nPos + 3).c_str();
-    if (vrwArgContext.RemoveArg(strQuotedTxt))
+    if (vrwArgContext.RemoveArg(rArg))
     {
-        m_bFound = true;
         m_bValid = true;
-        m_argValue = strQuotedTxt;
+        const MIchar cQuote = '"';
+        m_argValue = rArg.Trim(cQuote).StripSlashes();
         return MIstatus::success;
     }
 
@@ -373,10 +229,6 @@ CMICmdArgValString::IsStringArg(const CM
 bool
 CMICmdArgValString::IsStringArgSingleText(const CMIUtilString &vrTxt) const
 {
-    // Accept anything as string word
-    if (m_bHandleAnything)
-        return true;
-
     if (!m_bHandleDirPaths)
     {
         // Look for directory file paths, if found reject
@@ -417,6 +269,10 @@ CMICmdArgValString::IsStringArgSingleTex
 bool
 CMICmdArgValString::IsStringArgQuotedText(const CMIUtilString &vrTxt) const
 {
+    // Accept anything as string word
+    if (m_bHandleAnything)
+        return true;
+
     // CODETAG_QUOTEDTEXT_SIMILAR_CODE
     const MIchar cQuote = '"';
     const MIint nPos = vrTxt.find(cQuote);

Modified: lldb/trunk/tools/lldb-mi/MICmdCmdSymbol.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/tools/lldb-mi/MICmdCmdSymbol.cpp?rev=230654&r1=230653&r2=230654&view=diff
==============================================================================
--- lldb/trunk/tools/lldb-mi/MICmdCmdSymbol.cpp (original)
+++ lldb/trunk/tools/lldb-mi/MICmdCmdSymbol.cpp Thu Feb 26 12:21:22 2015
@@ -91,7 +91,7 @@ CMICmdCmdSymbolListLines::Execute(void)
     CMICMDBASE_GETOPTION(pArgFile, File, m_constStrArgNameFile);
 
     const CMIUtilString &strFilePath(pArgFile->GetValue());
-    const CMIUtilString strCmd(CMIUtilString::Format("target modules dump line-table \"%s\"", strFilePath.c_str()));
+    const CMIUtilString strCmd(CMIUtilString::Format("target modules dump line-table \"%s\"", strFilePath.AddSlashes().c_str()));
 
     CMICmnLLDBDebugSessionInfo &rSessionInfo(CMICmnLLDBDebugSessionInfo::Instance());
     const lldb::ReturnStatus rtn = rSessionInfo.GetDebugger().GetCommandInterpreter().HandleCommand(strCmd.c_str(), m_lldbResult);

Modified: lldb/trunk/tools/lldb-mi/MIDriver.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/tools/lldb-mi/MIDriver.cpp?rev=230654&r1=230653&r2=230654&view=diff
==============================================================================
--- lldb/trunk/tools/lldb-mi/MIDriver.cpp (original)
+++ lldb/trunk/tools/lldb-mi/MIDriver.cpp Thu Feb 26 12:21:22 2015
@@ -1172,7 +1172,7 @@ CMIDriver::GetExecutableFileNamePathOnCm
 bool
 CMIDriver::LocalDebugSessionStartupInjectCommands(void)
 {
-    const CMIUtilString strCmd(CMIUtilString::Format("-file-exec-and-symbols \"%s\"", m_strCmdLineArgExecuteableFileNamePath.c_str()));
+    const CMIUtilString strCmd(CMIUtilString::Format("-file-exec-and-symbols \"%s\"", m_strCmdLineArgExecuteableFileNamePath.AddSlashes().c_str()));
     const bool bOk = CMICmnStreamStdout::TextToStdout(strCmd);
     return (bOk && InterpretCommand(strCmd));
 }

Modified: lldb/trunk/tools/lldb-mi/MIUtilString.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/tools/lldb-mi/MIUtilString.cpp?rev=230654&r1=230653&r2=230654&view=diff
==============================================================================
--- lldb/trunk/tools/lldb-mi/MIUtilString.cpp (original)
+++ lldb/trunk/tools/lldb-mi/MIUtilString.cpp Thu Feb 26 12:21:22 2015
@@ -865,3 +865,87 @@ CMIUtilString::Escape(const bool vbEscap
     }
     return strNew;
 }
+
+//++ ------------------------------------------------------------------------------------
+// Details: Get string with backslashes in front of double quote '"' and backslash '\\'
+//          characters.
+// Type:    Method.
+// Args:    None.
+// Return:  CMIUtilString - The wrapped version of the initial string.
+// Throws:  None.
+//--
+CMIUtilString
+CMIUtilString::AddSlashes(void) const
+{
+    const MIchar cBckSlash('\\');
+    const MIuint nLen(length());
+    CMIUtilString strNew;
+    strNew.reserve(nLen);
+
+    MIuint nOffset(0);
+    while (nOffset < nLen)
+    {
+        const MIuint nUnescapedCharPos(find_first_of("\"\\", nOffset));
+        const bool bUnescapedCharNotFound(nUnescapedCharPos == (MIuint)std::string::npos);
+        if (bUnescapedCharNotFound)
+        {
+            const MIuint nAppendAll((MIuint)std::string::npos);
+            strNew.append(*this, nOffset, nAppendAll);
+            break;
+        }
+        const MIuint nAppendLen(nUnescapedCharPos - nOffset);
+        strNew.append(*this, nOffset, nAppendLen);
+        strNew.push_back(cBckSlash);
+        const MIchar cUnescapedChar((*this)[nUnescapedCharPos]);
+        strNew.push_back(cUnescapedChar);
+        nOffset = nUnescapedCharPos + 1;
+    }
+
+    return strNew;
+}
+
+//++ ------------------------------------------------------------------------------------
+// Details: Remove backslashes added by CMIUtilString::AddSlashes.
+// Type:    Method.
+// Args:    None.
+// Return:  CMIUtilString - The initial version of wrapped string.
+// Throws:  None.
+//--
+CMIUtilString
+CMIUtilString::StripSlashes(void) const
+{
+    const MIchar cBckSlash('\\');
+    const MIuint nLen(length());
+    CMIUtilString strNew;
+    strNew.reserve(nLen);
+
+    MIuint nOffset(0);
+    while (nOffset < nLen)
+    {
+        const MIuint nBckSlashPos(find(cBckSlash, nOffset));
+        const bool bBckSlashNotFound(nBckSlashPos == (MIuint)std::string::npos);
+        if (bBckSlashNotFound)
+        {
+            const MIuint nAppendAll((MIuint)std::string::npos);
+            strNew.append(*this, nOffset, nAppendAll);
+            break;
+        }
+        const MIuint nAppendLen(nBckSlashPos - nOffset);
+        strNew.append(*this, nOffset, nAppendLen);
+        const bool bBckSlashIsLast(nBckSlashPos == nLen);
+        if (bBckSlashIsLast)
+        {
+            strNew.push_back(cBckSlash);
+            break;
+        }
+        const MIchar cEscapedChar((*this)[nBckSlashPos + 1]);
+        const MIuint nEscapedCharPos(std::string("\"\\").find(cEscapedChar));
+        const bool bEscapedCharNotFound(nEscapedCharPos == (MIuint)std::string::npos);
+        if (bEscapedCharNotFound)
+            strNew.push_back(cBckSlash);
+        strNew.push_back(cEscapedChar);
+        nOffset = nBckSlashPos + 2;
+    }
+
+    return strNew;
+}

Modified: lldb/trunk/tools/lldb-mi/MIUtilString.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/tools/lldb-mi/MIUtilString.h?rev=230654&r1=230653&r2=230654&view=diff
==============================================================================
--- lldb/trunk/tools/lldb-mi/MIUtilString.h (original)
+++ lldb/trunk/tools/lldb-mi/MIUtilString.h Thu Feb 26 12:21:22 2015
@@ -73,6 +73,8 @@ class CMIUtilString : public std::string
                      const MIuint vnPos = 0) const;
     MIuint FindFirstNot(const CMIUtilString &vrPattern, const MIuint vnPos = 0) const;
     CMIUtilString Escape(const bool vbEscapeQuotes = false) const;
+    CMIUtilString AddSlashes(void) const;
+    CMIUtilString StripSlashes(void) const;
     //
     CMIUtilString &operator=(const MIchar *vpRhs);
     CMIUtilString &operator=(const std::string &vrRhs);





More information about the lldb-commits mailing list