[Lldb-commits] [lldb] r239297 - [LLDB-MI] Properly detect missing mandatory arguments to MI commands
Bruce Mitchener
bruce.mitchener at gmail.com
Mon Jun 8 04:15:10 PDT 2015
Author: brucem
Date: Mon Jun 8 06:15:09 2015
New Revision: 239297
URL: http://llvm.org/viewvc/llvm-project?rev=239297&view=rev
Log:
[LLDB-MI] Properly detect missing mandatory arguments to MI commands
Summary:
Previously if an MI command had **X** mandatory and **Y** optional arguments you could provide **X** or more optional arguments without providing any of the mandatory arguments, and the argument validation code wouldn't complain.
For example this would pass argument validation even though the mandatory **address** and **count** arguments are missing:
-data-read-memory-bytes --thread 1 --frame 0
Part of the problem was that an empty string was considered a valid value for a mandatory argument, which didn't make much sense.
Patch by Vadim Macagon. Thanks!
Test Plan:
./dotest.py -A x86_64 -C clang --executable $BUILDDIR/bin/lldb tools/lldb-mi/
No unexpected failures on my Ubuntu 14.10 64bit Virtualbox VM.
Reviewers: domipheus, ki.stfu, abidh
Reviewed By: ki.stfu, abidh
Subscribers: brucem, lldb-commits
Differential Revision: http://reviews.llvm.org/D10299
Modified:
lldb/trunk/test/tools/lldb-mi/data/TestMiData.py
lldb/trunk/test/tools/lldb-mi/stack/TestMiStack.py
lldb/trunk/tools/lldb-mi/MICmdArgSet.cpp
lldb/trunk/tools/lldb-mi/MICmdArgValConsume.cpp
lldb/trunk/tools/lldb-mi/MICmdArgValFile.cpp
lldb/trunk/tools/lldb-mi/MICmdArgValListOfN.cpp
lldb/trunk/tools/lldb-mi/MICmdArgValNumber.cpp
lldb/trunk/tools/lldb-mi/MICmdArgValOptionLong.cpp
lldb/trunk/tools/lldb-mi/MICmdArgValPrintValues.cpp
lldb/trunk/tools/lldb-mi/MICmdArgValString.cpp
lldb/trunk/tools/lldb-mi/MICmdArgValThreadGrp.cpp
lldb/trunk/tools/lldb-mi/MICmdCmdData.cpp
lldb/trunk/tools/lldb-mi/MICmnResources.cpp
lldb/trunk/tools/lldb-mi/MICmnResources.h
Modified: lldb/trunk/test/tools/lldb-mi/data/TestMiData.py
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/test/tools/lldb-mi/data/TestMiData.py?rev=239297&r1=239296&r2=239297&view=diff
==============================================================================
--- lldb/trunk/test/tools/lldb-mi/data/TestMiData.py (original)
+++ lldb/trunk/test/tools/lldb-mi/data/TestMiData.py Mon Jun 8 06:15:09 2015
@@ -200,10 +200,13 @@ class MiDataTestCase(lldbmi_testcase.MiT
self.runCmd('-data-read-memory-bytes &array')
self.expect(r'\^error')
- # Test that the address argument is required when other options are present
+ # Test that the address and count arguments are required when other options are present
self.runCmd('-data-read-memory-bytes --thread 1')
self.expect(r'\^error')
+ self.runCmd('-data-read-memory-bytes --thread 1 --frame 0')
+ self.expect(r'\^error')
+
# Test that the count argument is required when other options are present
self.runCmd('-data-read-memory-bytes --thread 1 &array')
self.expect(r'\^error')
Modified: lldb/trunk/test/tools/lldb-mi/stack/TestMiStack.py
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/test/tools/lldb-mi/stack/TestMiStack.py?rev=239297&r1=239296&r2=239297&view=diff
==============================================================================
--- lldb/trunk/test/tools/lldb-mi/stack/TestMiStack.py (original)
+++ lldb/trunk/test/tools/lldb-mi/stack/TestMiStack.py Mon Jun 8 06:15:09 2015
@@ -445,7 +445,7 @@ class MiStackTestCase(lldbmi_testcase.Mi
# Test that -stack-select-frame requires 1 mandatory argument
self.runCmd("-stack-select-frame")
- self.expect("\^error,msg=\"Command 'stack-select-frame'\. Command Args\. Missing options, 1 or more required\"")
+ self.expect("\^error,msg=\"Command 'stack-select-frame'\. Command Args\. Validation failed. Mandatory args not found: frame\"")
# Test that -stack-select-frame fails on invalid frame number
self.runCmd("-stack-select-frame 99")
Modified: lldb/trunk/tools/lldb-mi/MICmdArgSet.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/tools/lldb-mi/MICmdArgSet.cpp?rev=239297&r1=239296&r2=239297&view=diff
==============================================================================
--- lldb/trunk/tools/lldb-mi/MICmdArgSet.cpp (original)
+++ lldb/trunk/tools/lldb-mi/MICmdArgSet.cpp Mon Jun 8 06:15:09 2015
@@ -169,28 +169,24 @@ CMICmdArgSet::Validate(const CMIUtilStri
m_cmdArgContext = vwCmdArgsText;
// Iterate all the arguments or options required by a command
- const MIuint nArgs = vwCmdArgsText.GetNumberArgsPresent();
- MIuint nArgsMandatoryCnt = 0;
SetCmdArgs_t::const_iterator it = m_setCmdArgs.begin();
while (it != m_setCmdArgs.end())
{
const CMICmdArgValBase *pArg(*it);
- const CMIUtilString &rArgName(pArg->GetName());
- MIunused(rArgName);
- if (pArg->GetIsMandatory())
- nArgsMandatoryCnt++;
+
if (!const_cast<CMICmdArgValBase *>(pArg)->Validate(vwCmdArgsText))
{
- if (pArg->GetIsMandatory() && !pArg->GetFound())
- m_setCmdArgsThatAreMissing.push_back(const_cast<CMICmdArgValBase *>(pArg));
- else if (pArg->GetFound())
+ if (pArg->GetFound())
{
if (pArg->GetIsMissingOptions())
m_setCmdArgsMissingInfo.push_back(const_cast<CMICmdArgValBase *>(pArg));
else if (!pArg->GetValid())
m_setCmdArgsThatNotValid.push_back(const_cast<CMICmdArgValBase *>(pArg));
}
+ else if (pArg->GetIsMandatory())
+ m_setCmdArgsThatAreMissing.push_back(const_cast<CMICmdArgValBase *>(pArg));
}
+
if (pArg->GetFound() && !pArg->GetIsHandledByCmd())
{
m_bIsArgsPresentButNotHandledByCmd = true;
@@ -201,14 +197,7 @@ CMICmdArgSet::Validate(const CMIUtilStri
++it;
}
- // Check that one or more argument objects have any issues to report...
-
- if (nArgs < nArgsMandatoryCnt)
- {
- SetErrorDescription(CMIUtilString::Format(MIRSRC(IDS_CMD_ARGS_ERR_N_OPTIONS_REQUIRED), nArgsMandatoryCnt));
- return MIstatus::failure;
- }
-
+ // report any issues with arguments/options
if (IsArgsPresentButNotHandledByCmd())
WarningArgsNotHandledbyCmdLogFile(vStrMiCmd);
Modified: lldb/trunk/tools/lldb-mi/MICmdArgValConsume.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/tools/lldb-mi/MICmdArgValConsume.cpp?rev=239297&r1=239296&r2=239297&view=diff
==============================================================================
--- lldb/trunk/tools/lldb-mi/MICmdArgValConsume.cpp (original)
+++ lldb/trunk/tools/lldb-mi/MICmdArgValConsume.cpp Mon Jun 8 06:15:09 2015
@@ -59,7 +59,7 @@ bool
CMICmdArgValConsume::Validate(CMICmdArgContext &vwArgContext)
{
if (vwArgContext.IsEmpty())
- return MIstatus::success;
+ return m_bMandatory ? MIstatus::failure : MIstatus::success;
// Consume the optional file, line, linenum arguments till the mode '--' argument
const CMIUtilString::VecString_t vecOptions(vwArgContext.GetArgs());
@@ -68,15 +68,15 @@ CMICmdArgValConsume::Validate(CMICmdArgC
{
const CMIUtilString & rTxt( *it );
- if ( rTxt.compare( "--" ) == 0 )
+ if ( rTxt.compare( "--" ) == 0 )
{
m_bFound = true;
m_bValid = true;
- return MIstatus::success;
- }
+ return MIstatus::success;
+ }
- if ( !vwArgContext.RemoveArg( rTxt ) )
- return MIstatus::failure;
+ if ( !vwArgContext.RemoveArg( rTxt ) )
+ return MIstatus::failure;
// Next
++it;
Modified: lldb/trunk/tools/lldb-mi/MICmdArgValFile.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/tools/lldb-mi/MICmdArgValFile.cpp?rev=239297&r1=239296&r2=239297&view=diff
==============================================================================
--- lldb/trunk/tools/lldb-mi/MICmdArgValFile.cpp (original)
+++ lldb/trunk/tools/lldb-mi/MICmdArgValFile.cpp Mon Jun 8 06:15:09 2015
@@ -60,7 +60,7 @@ bool
CMICmdArgValFile::Validate(CMICmdArgContext &vwArgContext)
{
if (vwArgContext.IsEmpty())
- return MIstatus::success;
+ return m_bMandatory ? MIstatus::failure : MIstatus::success;
// The GDB/MI spec suggests there is only parameter
Modified: lldb/trunk/tools/lldb-mi/MICmdArgValListOfN.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/tools/lldb-mi/MICmdArgValListOfN.cpp?rev=239297&r1=239296&r2=239297&view=diff
==============================================================================
--- lldb/trunk/tools/lldb-mi/MICmdArgValListOfN.cpp (original)
+++ lldb/trunk/tools/lldb-mi/MICmdArgValListOfN.cpp Mon Jun 8 06:15:09 2015
@@ -74,7 +74,7 @@ CMICmdArgValListOfN::Validate(CMICmdArgC
}
if (vwArgContext.IsEmpty())
- return MIstatus::success;
+ return m_bMandatory ? MIstatus::failure : MIstatus::success;
const CMIUtilString &rArg(vwArgContext.GetArgsLeftToParse());
if (IsListOfN(rArg) && CreateList(rArg))
Modified: lldb/trunk/tools/lldb-mi/MICmdArgValNumber.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/tools/lldb-mi/MICmdArgValNumber.cpp?rev=239297&r1=239296&r2=239297&view=diff
==============================================================================
--- lldb/trunk/tools/lldb-mi/MICmdArgValNumber.cpp (original)
+++ lldb/trunk/tools/lldb-mi/MICmdArgValNumber.cpp Mon Jun 8 06:15:09 2015
@@ -66,7 +66,7 @@ bool
CMICmdArgValNumber::Validate(CMICmdArgContext &vwArgContext)
{
if (vwArgContext.IsEmpty())
- return MIstatus::success;
+ return m_bMandatory ? MIstatus::failure : MIstatus::success;
if (vwArgContext.GetNumberArgsPresent() == 1)
{
Modified: lldb/trunk/tools/lldb-mi/MICmdArgValOptionLong.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/tools/lldb-mi/MICmdArgValOptionLong.cpp?rev=239297&r1=239296&r2=239297&view=diff
==============================================================================
--- lldb/trunk/tools/lldb-mi/MICmdArgValOptionLong.cpp (original)
+++ lldb/trunk/tools/lldb-mi/MICmdArgValOptionLong.cpp Mon Jun 8 06:15:09 2015
@@ -108,7 +108,7 @@ bool
CMICmdArgValOptionLong::Validate(CMICmdArgContext &vwArgContext)
{
if (vwArgContext.IsEmpty())
- return MIstatus::success;
+ return m_bMandatory ? MIstatus::failure : MIstatus::success;
if (vwArgContext.GetNumberArgsPresent() == 1)
{
Modified: lldb/trunk/tools/lldb-mi/MICmdArgValPrintValues.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/tools/lldb-mi/MICmdArgValPrintValues.cpp?rev=239297&r1=239296&r2=239297&view=diff
==============================================================================
--- lldb/trunk/tools/lldb-mi/MICmdArgValPrintValues.cpp (original)
+++ lldb/trunk/tools/lldb-mi/MICmdArgValPrintValues.cpp Mon Jun 8 06:15:09 2015
@@ -62,7 +62,7 @@ bool
CMICmdArgValPrintValues::Validate(CMICmdArgContext &vwArgContext)
{
if (vwArgContext.IsEmpty())
- return MIstatus::success;
+ return m_bMandatory ? MIstatus::failure : MIstatus::success;
const CMIUtilString strArg(vwArgContext.GetArgs()[0]);
if (IsArgPrintValues(strArg) && ExtractPrintValues(strArg))
Modified: lldb/trunk/tools/lldb-mi/MICmdArgValString.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/tools/lldb-mi/MICmdArgValString.cpp?rev=239297&r1=239296&r2=239297&view=diff
==============================================================================
--- lldb/trunk/tools/lldb-mi/MICmdArgValString.cpp (original)
+++ lldb/trunk/tools/lldb-mi/MICmdArgValString.cpp Mon Jun 8 06:15:09 2015
@@ -108,7 +108,7 @@ bool
CMICmdArgValString::Validate(CMICmdArgContext &vrwArgContext)
{
if (vrwArgContext.IsEmpty())
- return MIstatus::success;
+ return m_bMandatory ? MIstatus::failure : MIstatus::success;
if (m_bHandleQuotedString)
return ValidateQuotedText(vrwArgContext);
Modified: lldb/trunk/tools/lldb-mi/MICmdArgValThreadGrp.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/tools/lldb-mi/MICmdArgValThreadGrp.cpp?rev=239297&r1=239296&r2=239297&view=diff
==============================================================================
--- lldb/trunk/tools/lldb-mi/MICmdArgValThreadGrp.cpp (original)
+++ lldb/trunk/tools/lldb-mi/MICmdArgValThreadGrp.cpp Mon Jun 8 06:15:09 2015
@@ -62,7 +62,7 @@ bool
CMICmdArgValThreadGrp::Validate(CMICmdArgContext &vwArgContext)
{
if (vwArgContext.IsEmpty())
- return MIstatus::success;
+ return m_bMandatory ? MIstatus::failure : MIstatus::success;
if (vwArgContext.GetNumberArgsPresent() == 1)
{
Modified: lldb/trunk/tools/lldb-mi/MICmdCmdData.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/tools/lldb-mi/MICmdCmdData.cpp?rev=239297&r1=239296&r2=239297&view=diff
==============================================================================
--- lldb/trunk/tools/lldb-mi/MICmdCmdData.cpp (original)
+++ lldb/trunk/tools/lldb-mi/MICmdCmdData.cpp Mon Jun 8 06:15:09 2015
@@ -617,21 +617,6 @@ CMICmdCmdDataReadMemoryBytes::Execute(vo
return MIstatus::failure;
}
- // FIXME: shouldn't have to ensure mandatory arguments are present, that should've been handled
- // in ParseArgs(), unfortunately that seems kinda sorta broken right now if options are provided
- // but mandatory arguments are missing, so here we go...
- if (!pArgAddrExpr->GetFound())
- {
- SetError(CMIUtilString::Format(MIRSRC(IDS_CMD_ARGS_ERR_VALIDATION_MANDATORY), m_constStrArgAddrExpr.c_str()));
- return MIstatus::failure;
- }
-
- if (!pArgNumBytes->GetFound())
- {
- SetError(CMIUtilString::Format(MIRSRC(IDS_CMD_ARGS_ERR_VALIDATION_MANDATORY), m_constStrArgNumBytes.c_str()));
- return MIstatus::failure;
- }
-
CMICmnLLDBDebugSessionInfo &rSessionInfo(CMICmnLLDBDebugSessionInfo::Instance());
lldb::SBProcess sbProcess = rSessionInfo.GetProcess();
if (!sbProcess.IsValid())
Modified: lldb/trunk/tools/lldb-mi/MICmnResources.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/tools/lldb-mi/MICmnResources.cpp?rev=239297&r1=239296&r2=239297&view=diff
==============================================================================
--- lldb/trunk/tools/lldb-mi/MICmnResources.cpp (original)
+++ lldb/trunk/tools/lldb-mi/MICmnResources.cpp Mon Jun 8 06:15:09 2015
@@ -178,7 +178,6 @@ const CMICmnResources::SRsrcTextData CMI
{IDS_DRIVER_ERR_LOCAL_DEBUG_NOT_IMPL, "Driver. --executable argument given. Local debugging is not implemented."},
{IDS_DRIVER_ERR_LOCAL_DEBUG_INIT, "Driver. --executable argument given. Initialising local debugging failed."},
{IDS_STDERR_ERR_NOT_ALL_DATA_WRITTEN, "Stderr. Not all data was written to stream. The data '%s'"},
- {IDS_CMD_ARGS_ERR_N_OPTIONS_REQUIRED, "Command Args. Missing options, %d or more required"},
{IDS_CMD_ARGS_ERR_OPTION_NOT_FOUND, "Command Args. Option '%s' not found"},
{IDS_CMD_ARGS_ERR_VALIDATION_MANDATORY, "Mandatory args not found: %s"},
{IDS_CMD_ARGS_ERR_VALIDATION_INVALID, "Invalid args: %s"},
Modified: lldb/trunk/tools/lldb-mi/MICmnResources.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/tools/lldb-mi/MICmnResources.h?rev=239297&r1=239296&r2=239297&view=diff
==============================================================================
--- lldb/trunk/tools/lldb-mi/MICmnResources.h (original)
+++ lldb/trunk/tools/lldb-mi/MICmnResources.h Mon Jun 8 06:15:09 2015
@@ -193,7 +193,6 @@ enum
IDS_STDERR_ERR_NOT_ALL_DATA_WRITTEN,
- IDS_CMD_ARGS_ERR_N_OPTIONS_REQUIRED,
IDS_CMD_ARGS_ERR_OPTION_NOT_FOUND,
IDS_CMD_ARGS_ERR_VALIDATION_MANDATORY,
IDS_CMD_ARGS_ERR_VALIDATION_INVALID,
More information about the lldb-commits
mailing list