Fix -data-list-register-names/-data-disassemble, minor fix in -data-list-register-values (MI)

[ki.stfu]

Summary

• Fix -data-list-register-names command: previously it ignored regno arguments and always showed all registers
• Add 'size' field to -data-disassemble command: now we are able to get an instruction's size
• Minor fix in -data-list-register-value: fix comments/code style
• Enable all tests in MiDataTestCase
• Fix the GetRegister function that gets an register by its index

These changes were tested on OS X; all MiDataTestCase tests were passed.

[ki.stfu]

add full-contex patch

[clayborg]

Looks good.

[abidh]

Looks good. Thanks for taking care of multiple register sets in GetRegister() and changing the index correctly. We still need to handle the "--skip-unavailable" option but that can be done separately.

test/tools/lldb-mi/TestMiData.py

 """
 Test that the lldb-mi driver works with -data-xxx commands
 """
 
 import lldbmi_testcase
 from lldbtest import *
 import unittest2
 
 class MiDataTestCase(lldbmi_testcase.MiTestCaseBase):
 
     @lldbmi_test
     @expectedFailureWindows("llvm.org/pr22274: need a pexpect replacement for windows")
-    @unittest2.skip("-data-disassemble doesn't show 'size' field")
     def test_lldbmi_data_disassemble(self):
         """Test that 'lldb-mi --interpreter' works for -data-disassemble."""
 
         self.spawnLldbMi(args = None)
 
         # Load executable
         self.runCmd("-file-exec-and-symbols %s" % self.myexe)
         self.expect("\^done")
def test_lldbmi_data_disassemble(self):
         addr = int(self.child.after.split("\"")[1], 16)
 
         # Test -data-disassemble: try to disassemble some address
         self.runCmd("-data-disassemble -s %#x -e %#x -- 0" % (addr, addr + 0x10))
         self.expect("\^done,asm_insns=\[{address=\"%#x\",func-name=\"main\",offset=\"0x0\",size=\"[1-9]\",inst=\".+\"}," % addr)
 
     @lldbmi_test
     @expectedFailureWindows("llvm.org/pr22274: need a pexpect replacement for windows")
-    @unittest2.skip("-data-list-register-names doesn't work properly")
     def test_lldbmi_data_list_register_names(self):
         """Test that 'lldb-mi --interpreter' works for -data-list-register-names."""
 
         self.spawnLldbMi(args = None)
 
         # Load executable
         self.runCmd("-file-exec-and-symbols %s" % self.myexe)
         self.expect("\^done")
def test_lldbmi_data_list_register_names(self):
         self.expect("\^done,register-names=\[\".+\",")
 
         # Test -data-list-register-names: try to get specified registers
         self.runCmd("-data-list-register-names 0")
         self.expect("\^done,register-names=\[\".+\"\]")
 
     @lldbmi_test
     @expectedFailureWindows("llvm.org/pr22274: need a pexpect replacement for windows")
-    @unittest2.skip("-data-list-register-values doesn't work properly")
     def test_lldbmi_data_list_register_values(self):
         """Test that 'lldb-mi --interpreter' works for -data-list-register-values."""
 
         self.spawnLldbMi(args = None)
 
         # Load executable
         self.runCmd("-file-exec-and-symbols %s" % self.myexe)
         self.expect("\^done")

tools/lldb-mi/MICmdCmdData.h

class CMICmdCmdDataListRegisterNames : public CMICmdBase
   public:
     // From CMICmdInvoker::ICmd
     virtual bool Execute(void);
     virtual bool Acknowledge(void);
     virtual bool ParseArgs(void);
     // From CMICmnBase
     /* dtor */ virtual ~CMICmdCmdDataListRegisterNames(void);
 
+    // Methods:
+  private:
+    lldb::SBValue GetRegister(const MIuint vRegisterIndex) const;
+
     // Attributes:
   private:
     const CMIUtilString m_constStrArgThreadGroup; // Not specified in MI spec but Eclipse gives this option
     const CMIUtilString m_constStrArgRegNo;       // Not handled by *this command
     CMICmnMIValueList m_miValueList;
 };
 
 //++ ============================================================================
public:
     virtual bool Acknowledge(void);
     virtual bool ParseArgs(void);
     // From CMICmnBase
     /* dtor */ virtual ~CMICmdCmdDataListRegisterValues(void);
 
     // Methods:
   private:
     lldb::SBValue GetRegister(const MIuint vRegisterIndex) const;
-    void AddToOutput (const lldb::SBValue& value, MIuint index, CMICmnLLDBDebugSessionInfoVarObj::varFormat_e eFormat);
+    bool AddToOutput(const MIuint vnIndex, const lldb::SBValue &vrValue, CMICmnLLDBDebugSessionInfoVarObj::varFormat_e veVarFormat);
 
     // Attributes:
   private:
     const CMIUtilString m_constStrArgThread; // Not specified in MI spec but Eclipse gives this option
     const CMIUtilString m_constStrArgSkip;   // Not handled by *this command
     const CMIUtilString m_constStrArgFormat;
     const CMIUtilString m_constStrArgRegNo;
     CMICmnMIValueList m_miValueList;

tools/lldb-mi/MICmdCmdData.cpp

for (size_t i = 0; i < nInstructions; i++)
         pStrMnemonic = (pStrMnemonic != nullptr) ? pStrMnemonic : pUnknown;
         lldb::SBAddress address = instrt.GetAddress();
         lldb::addr_t addr = address.GetLoadAddress(sbTarget);
         const MIchar *pFnName = address.GetFunction().GetName();
         pFnName = (pFnName != nullptr) ? pFnName : pUnknown;
         lldb::addr_t addrOffSet = address.GetOffset();
         const MIchar *pStrOperands = instrt.GetOperands(sbTarget);
         pStrOperands = (pStrOperands != nullptr) ? pStrOperands : pUnknown;
+        const size_t instrtSize = instrt.GetByteSize();
 
         // MI "{address=\"0x%08llx\",func-name=\"%s\",offset=\"%lld\",inst=\"%s %s\"}"
         const CMICmnMIValueConst miValueConst(CMIUtilString::Format("0x%08llx", addr));
         const CMICmnMIValueResult miValueResult("address", miValueConst);
         CMICmnMIValueTuple miValueTuple(miValueResult);
         const CMICmnMIValueConst miValueConst2(pFnName);
         const CMICmnMIValueResult miValueResult2("func-name", miValueConst2);
         miValueTuple.Add(miValueResult2);
         const CMICmnMIValueConst miValueConst3(CMIUtilString::Format("0x%lld", addrOffSet));
         const CMICmnMIValueResult miValueResult3("offset", miValueConst3);
         miValueTuple.Add(miValueResult3);
-        const CMICmnMIValueConst miValueConst4(CMIUtilString::Format("%s %s", pStrMnemonic, pStrOperands));
-        const CMICmnMIValueResult miValueResult4("inst", miValueConst4);
+        const CMICmnMIValueConst miValueConst4(CMIUtilString::Format("%d", instrtSize));
+        const CMICmnMIValueResult miValueResult4("size", miValueConst4);
         miValueTuple.Add(miValueResult4);
+        const CMICmnMIValueConst miValueConst5(CMIUtilString::Format("%s %s", pStrMnemonic, pStrOperands));
+        const CMICmnMIValueResult miValueResult5("inst", miValueConst5);
+        miValueTuple.Add(miValueResult5);
 
         if (nDisasmMode == 1)
         {
             lldb::SBLineEntry lineEntry = address.GetLineEntry();
             const MIuint nLine = lineEntry.GetLine();
             const MIchar *pFileName = lineEntry.GetFileSpec().GetFilename();
             pFileName = (pFileName != nullptr) ? pFileName : pUnknown;
 

 // Args:    None.
 // Return:  MIstatus::success - Functional succeeded.
 //          MIstatus::failure - Functional failed.
 // Throws:  None.
 //--
 bool
 CMICmdCmdDataListRegisterNames::Execute(void)
 {
+    CMICMDBASE_GETOPTION(pArgRegNo, ListOfN, m_constStrArgRegNo);
+
     CMICmnLLDBDebugSessionInfo &rSessionInfo(CMICmnLLDBDebugSessionInfo::Instance());
     lldb::SBProcess sbProcess = rSessionInfo.GetProcess();
     if (!sbProcess.IsValid())
     {
         SetError(CMIUtilString::Format(MIRSRC(IDS_CMD_ERR_INVALID_PROCESS), m_cmdData.strMiCmd.c_str()));
         return MIstatus::failure;
     }
 
+    const CMICmdArgValListBase::VecArgObjPtr_t &rVecRegNo(pArgRegNo->GetExpectedOptions());
+    if (!rVecRegNo.empty())
+    {
+        // List of required registers
+        CMICmdArgValListBase::VecArgObjPtr_t::const_iterator it = rVecRegNo.begin();
+        while (it != rVecRegNo.end())
+        {
+            const CMICmdArgValNumber *pRegNo = static_cast<CMICmdArgValNumber *>(*it);
+            const MIuint nRegIndex = pRegNo->GetValue();
+            lldb::SBValue regValue = GetRegister(nRegIndex);
+            if (regValue.IsValid())
+            {
+                const CMICmnMIValueConst miValueConst(CMICmnLLDBUtilSBValue(regValue).GetName());
+                m_miValueList.Add(miValueConst);
+            }
+
+            // Next
+            ++it;
+        }
+    }
+    else
+    {
+        // List of all registers
         lldb::SBThread thread = sbProcess.GetSelectedThread();
         lldb::SBFrame frame = thread.GetSelectedFrame();
         lldb::SBValueList registers = frame.GetRegisters();
         const MIuint nRegisters = registers.GetSize();
         for (MIuint i = 0; i < nRegisters; i++)
         {
             lldb::SBValue value = registers.GetValueAtIndex(i);
             const MIuint nRegChildren = value.GetNumChildren();
             for (MIuint j = 0; j < nRegChildren; j++)
             {
-            lldb::SBValue value2 = value.GetChildAtIndex(j);
-            if (value2.IsValid())
+                lldb::SBValue regValue = value.GetChildAtIndex(j);
+                if (regValue.IsValid())
                 {
-                const CMICmnMIValueConst miValueConst(CMICmnLLDBUtilSBValue(value2).GetName());
-                m_miValueList.Add(miValueConst);
+                    const CMICmnMIValueConst miValueConst(CMICmnLLDBUtilSBValue(regValue).GetName());
+                    const bool bOk = m_miValueList.Add(miValueConst);
+                    if (!bOk)
+                        return MIstatus::failure;
+                }
             }
         }
     }
 
     return MIstatus::success;
 }
 
 //++ ------------------------------------------------------------------------------------

 // Throws:  None.
 //--
 CMICmdBase *
 CMICmdCmdDataListRegisterNames::CreateSelf(void)
 {
     return new CMICmdCmdDataListRegisterNames();
 }
 
+//++ ------------------------------------------------------------------------------------
+// Details: Required by the CMICmdFactory when registering *this command. The factory
+//          calls this function to create an instance of *this command.
+// Type:    Method.
+// Args:    None.
+// Return:  lldb::SBValue - LLDB SBValue object.
+// Throws:  None.
+//--
+lldb::SBValue
+CMICmdCmdDataListRegisterNames::GetRegister(const MIuint vRegisterIndex) const
+{
+    lldb::SBThread thread = CMICmnLLDBDebugSessionInfo::Instance().GetProcess().GetSelectedThread();
+    lldb::SBFrame frame = thread.GetSelectedFrame();
+    lldb::SBValueList registers = frame.GetRegisters();
+    const MIuint nRegisters = registers.GetSize();
+    MIuint nRegisterIndex(vRegisterIndex);
+    for (MIuint i = 0; i < nRegisters; i++)
+    {
+        lldb::SBValue value = registers.GetValueAtIndex(i);
+        const MIuint nRegChildren = value.GetNumChildren();
+        if (nRegisterIndex >= nRegChildren)
+        {
+            nRegisterIndex -= nRegChildren;
+            continue;
+        }
+
+        lldb::SBValue value2 = value.GetChildAtIndex(nRegisterIndex);
+        if (value2.IsValid())
+        {
+            return value2;
+        }
+    }
+
+    return lldb::SBValue();
+}
+
 //---------------------------------------------------------------------------------------
 //---------------------------------------------------------------------------------------
 //---------------------------------------------------------------------------------------
 
 //++ ------------------------------------------------------------------------------------
 // Details: CMICmdCmdDataListRegisterValues constructor.
 // Type:    Method.
 // Args:    None.
CMICmdCmdDataListRegisterValues::Execute(void)
     lldb::SBProcess sbProcess = rSessionInfo.GetProcess();
     if (!sbProcess.IsValid())
     {
         SetError(CMIUtilString::Format(MIRSRC(IDS_CMD_ERR_INVALID_PROCESS), m_cmdData.strMiCmd.c_str()));
         return MIstatus::failure;
     }
 
     const CMICmdArgValListBase::VecArgObjPtr_t &rVecRegNo(pArgRegNo->GetExpectedOptions());
-    if (!rVecRegNo.empty ())
+    if (!rVecRegNo.empty())
     {
+        // List of required registers
         CMICmdArgValListBase::VecArgObjPtr_t::const_iterator it = rVecRegNo.begin();
-        while (it != rVecRegNo.end ())
+        while (it != rVecRegNo.end())
         {
             const CMICmdArgValNumber *pRegNo = static_cast<CMICmdArgValNumber *>(*it);
-            const MIuint nReg = pRegNo->GetValue ();
-            lldb::SBValue regValue = GetRegister (nReg);
-            AddToOutput (regValue, nReg, eFormat);
+            const MIuint nRegIndex = pRegNo->GetValue();
+            lldb::SBValue regValue = GetRegister(nRegIndex);
+            if (regValue.IsValid())
+            {
+                const bool bOk = AddToOutput(nRegIndex, regValue, eFormat);
+                if (!bOk)
+                    return MIstatus::failure;
+            }
+
             // Next
             ++it;
         }
     }
     else
     {
         // No register numbers are provided. Output all registers.
-        lldb::SBThread thread = sbProcess.GetSelectedThread ();
-        lldb::SBFrame frame = thread.GetSelectedFrame ();
-        lldb::SBValueList registers = frame.GetRegisters ();
-        const MIuint nRegisters = registers.GetSize ();
-        MIuint index = 0;
+        lldb::SBThread thread = sbProcess.GetSelectedThread();
+        lldb::SBFrame frame = thread.GetSelectedFrame();
+        lldb::SBValueList registers = frame.GetRegisters();
+        const MIuint nRegisters = registers.GetSize();
+        MIuint nRegIndex = 0;
         for (MIuint i = 0; i < nRegisters; i++)
         {
-            lldb::SBValue value = registers.GetValueAtIndex (i);
-            const MIuint nRegChildren = value.GetNumChildren ();
+            lldb::SBValue value = registers.GetValueAtIndex(i);
+            const MIuint nRegChildren = value.GetNumChildren();
             for (MIuint j = 0; j < nRegChildren; j++)
             {
-                lldb::SBValue reg_value = value.GetChildAtIndex (j);
-                if (reg_value.IsValid ())
+                lldb::SBValue regValue = value.GetChildAtIndex(j);
+                if (regValue.IsValid())
                 {
-                    AddToOutput (reg_value, index, eFormat);
-                    index++;
+                    const bool bOk = AddToOutput(nRegIndex, regValue, eFormat);
+                    if (!bOk)
+                        return MIstatus::failure;
                 }
+
+                // Next
+                ++nRegIndex;
             }
         }
     }
 
     return MIstatus::success;
 }
 
 //++ ------------------------------------------------------------------------------------

 //--
 lldb::SBValue
 CMICmdCmdDataListRegisterValues::GetRegister(const MIuint vRegisterIndex) const
 {
     lldb::SBThread thread = CMICmnLLDBDebugSessionInfo::Instance().GetProcess().GetSelectedThread();
     lldb::SBFrame frame = thread.GetSelectedFrame();
     lldb::SBValueList registers = frame.GetRegisters();
     const MIuint nRegisters = registers.GetSize();
+    MIuint nRegisterIndex(vRegisterIndex);
     for (MIuint i = 0; i < nRegisters; i++)
     {
         lldb::SBValue value = registers.GetValueAtIndex(i);
         const MIuint nRegChildren = value.GetNumChildren();
-        if (nRegChildren > 0)
+        if (nRegisterIndex >= nRegChildren)
         {
-            lldb::SBValue value2 = value.GetChildAtIndex(vRegisterIndex);
-            if (value2.IsValid())
-            {
-                return value2;
-            }
-        }
-    }
+            nRegisterIndex -= nRegChildren;
+            continue;
+        }
+
+        lldb::SBValue value2 = value.GetChildAtIndex(nRegisterIndex);
 
     return lldb::SBValue();
 }
 
 //++ ------------------------------------------------------------------------------------
 // Details: Adds the register value to the output list.
 // Type:    Method.
 // Args:    Value of the register, its index and output format.
 // Return:  None
 // Throws:  None.
 //--
-void
-CMICmdCmdDataListRegisterValues::AddToOutput (const lldb::SBValue& value, MIuint index,
-      CMICmnLLDBDebugSessionInfoVarObj::varFormat_e eFormat)
+bool
+CMICmdCmdDataListRegisterValues::AddToOutput(const MIuint vnIndex, const lldb::SBValue &vrValue,
+      CMICmnLLDBDebugSessionInfoVarObj::varFormat_e veVarFormat)
 {
-    const CMICmnMIValueConst miValueConst (CMIUtilString::Format ("%u", index));
-    const CMICmnMIValueResult miValueResult ("number", miValueConst);
-    const CMIUtilString strRegValue (CMICmnLLDBDebugSessionInfoVarObj::GetValueStringFormatted (value, eFormat));
-    const CMICmnMIValueConst miValueConst2 (strRegValue);
-    const CMICmnMIValueResult miValueResult2 ("value", miValueConst2);
-
-    CMICmnMIValueTuple miValueTuple (miValueResult);
-    miValueTuple.Add (miValueResult2);
-    m_miValueList.Add (miValueTuple);
+    const CMICmnMIValueConst miValueConst(CMIUtilString::Format("%u", vnIndex));
+    const CMICmnMIValueResult miValueResult("number", miValueConst);
+    CMICmnMIValueTuple miValueTuple(miValueResult);
+    const CMIUtilString strRegValue(CMICmnLLDBDebugSessionInfoVarObj::GetValueStringFormatted(vrValue, veVarFormat));
+    const CMICmnMIValueConst miValueConst2(strRegValue);
+    const CMICmnMIValueResult miValueResult2("value", miValueConst2);
+    bool bOk = miValueTuple.Add(miValueResult2);
+    return bOk && m_miValueList.Add(miValueTuple);
 }
 
 //---------------------------------------------------------------------------------------
 //---------------------------------------------------------------------------------------
 //---------------------------------------------------------------------------------------
 
 //++ ------------------------------------------------------------------------------------
 // Details: CMICmdCmdDataListRegisterChanged constructor.