Fix -data-read-memory-bytes command (MI)

[ki.stfu]

Summary

• Add IsHexadecimalNumber method to CMIUtilString (MI)
• Add number format (dec,hex,auto) to CMICmdArgValNumber (MI)
• Fix -data-read-memory-bytes to pass address in hex format (MI)
• Fix output begin/end/offset fields format in -data-read-memory-bytes
• Fix CMICmdArgValNumber::ExtractNumber to extract 64bit value
• + tests

All tests passed on OS X

[ki.stfu]

Fix MiExecTestCase tests

[ki.stfu]

tools/lldb-mi/MIUtilString.cpp

460–464

set errno to 0 before strtoull()

[ki.stfu]

Fix strtoull() in CMIUtilString::ExtractNumberFromHexadecimal

[ki.stfu]

Minor fixes of comments

[clayborg]

Check your strtoull call and make sure you don't need to check if all characters were consumed otherwise the call to strtoull can succeed with "123qed" or any string that isn't all numbers.

test/tools/lldb-mi/TestMiData.py

46

Newbie MI question: no quotes needed around the path?

tools/lldb-mi/MIUtilString.cpp

461

Has the string been verified to only contain value number characters? If not, then you want to change this to:

char *end = NULL;
const MIuint64 nNum = ::strtoull(this->c_str(), &end, 16);
if (end && *end != '\0')
    return false;

[ki.stfu]

test/tools/lldb-mi/TestMiData.py

46

In this case not, because self.myexe equals to "a.out", which is loaded from current directory.

tools/lldb-mi/MIUtilString.cpp

461

This function is called by CMICmdArgValNumber::ExtractNumber() which verifies that string contains only a number.

In other cases I think we can use it like:

CMIUtilString str("0x500 MyFunctionName");
MIuint64 n;
str.ExtractNumberFromHexadecimal(n);
assert (n == 0x500);

[clayborg]

Thanks for the explantation, looks good.

test/tools/lldb-mi/TestMiData.py

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=\"0x%08x\",func-name=\"main\",offset=\"0\",size=\"[1-9]+\",inst=\".+\"}," % addr)
 
     @lldbmi_test
     @expectedFailureWindows("llvm.org/pr22274: need a pexpect replacement for windows")
+    def test_lldbmi_data_read_memory_bytes(self):
+        """Test that 'lldb-mi --interpreter' works for -data-read-memory-bytes."""
+
+        self.spawnLldbMi(args = None)
+
+        # Load executable
+        self.runCmd("-file-exec-and-symbols %s" % self.myexe)
+        self.expect("\^done")
+
+        # Run to main
+        self.runCmd("-break-insert -f main")
+        self.expect("\^done,bkpt={number=\"1\"")
+        self.runCmd("-exec-run")
+        self.expect("\^running")
+        self.expect("\*stopped,reason=\"breakpoint-hit\"")
+
+        # Get address of s_RawData
+        self.runCmd("-data-evaluate-expression &s_RawData")
+        self.expect("\^done,value=\"0x[0-9a-f]+\"",timeout=1)
+        addr = int(self.child.after.split("\"")[1], 16)
+        size = 5
+
+        # Test -data-read-memory-bytes: try to read data of s_RawData
+        self.runCmd("-data-read-memory-bytes %#x %d" % (addr, size))
+        self.expect("\^done,memory=\[{begin=\"0x0*%x\",offset=\"0x0+\",end=\"0x0*%x\",contents=\"1234567800\"}\]" % (addr, addr + size))
+
+    @lldbmi_test
+    @expectedFailureWindows("llvm.org/pr22274: need a pexpect replacement for windows")
     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")

test/tools/lldb-mi/main.c

 #include <stdio.h>
 extern int a_MyFunction();
 extern int b_MyFunction();
 extern int infloop();
 extern int local_test();
 int doloop, dosegfault;
 int g_MyVar = 3;
 static int s_MyVar = 4;
+//FIXME -data-evaluate-expression/print can't evaluate value of type "static char[]"
+const char s_RawData[] = "\x12\x34\x56\x78"; //FIXME static const char s_RawData[] = "\x12\x34\x56\x78";
 
 int main (int argc, char const *argv[])
 {
     int a, b;
     printf("argc=%d\n", argc);   //BP_printf_call
     //BP_argctest
     a = a_MyFunction();          //BP_a_MyFunction_call
     b = b_MyFunction();          //BP_b_MyFunction_call

tools/lldb-mi/MICmdArgValNumber.h

 //          argument and so extract a value from it .
 //          Based on the Interpreter pattern.
 // Gotchas: None.
 // Authors: Illya Rudkin 14/04/2014.
 // Changes: None.
 //--
 class CMICmdArgValNumber : public CMICmdArgValBaseTemplate<MIint64>
 {
+    // Enums:
+  public:
+    //++ ---------------------------------------------------------------------------------
+    // Details: CMICmdArgValNumber needs to know what format of argument to look for in
+    //          the command options text.
+    //--
+    enum ArgValNumberFormat_e
+    {
+        eArgValNumberFormat_Decimal     = (1u << 0),
+        eArgValNumberFormat_Hexadecimal = (1u << 1),
+        eArgValNumberFormat_Auto        = ((eArgValNumberFormat_Hexadecimal << 1) - 1u)  ///< Indicates to try and lookup everything up during a query.
+    };
+
     // Methods:
   public:
     /* ctor */ CMICmdArgValNumber(void);
-    /* ctor */ CMICmdArgValNumber(const CMIUtilString &vrArgName, const bool vbMandatory, const bool vbHandleByCmd);
+    /* ctor */ CMICmdArgValNumber(const CMIUtilString &vrArgName, const bool vbMandatory, const bool vbHandleByCmd,
+                                  const MIuint vnNumberFormatMask = eArgValNumberFormat_Decimal);
     //
     bool IsArgNumber(const CMIUtilString &vrTxt) const;
 
     // Overridden:
   public:
     // From CMICmdArgValBase
     /* dtor */ virtual ~CMICmdArgValNumber(void);
     // From CMICmdArgSet::IArg
     virtual bool Validate(CMICmdArgContext &vwArgContext);
 
     // Methods:
   private:
     bool ExtractNumber(const CMIUtilString &vrTxt);
     MIint64 GetNumber(void) const;
 
     // Attributes:
   private:
+    MIuint m_nNumberFormatMask;
     MIint64 m_nNumber;
 };

tools/lldb-mi/MICmdArgValNumber.cpp

 //++ ------------------------------------------------------------------------------------
 // Details: CMICmdArgValNumber constructor.
 // Type:    Method.
 // Args:    None.
 // Return:  None.
 // Throws:  None.
 //--
 CMICmdArgValNumber::CMICmdArgValNumber(void)
-    : m_nNumber(0)
+    : m_nNumberFormatMask(CMICmdArgValNumber::eArgValNumberFormat_Decimal)
+    , m_nNumber(0)
 {
 }
 
 //++ ------------------------------------------------------------------------------------
 // Details: CMICmdArgValNumber constructor.
 // Type:    Method.
-// Args:    vrArgName       - (R) Argument's name to search by.
-//          vbMandatory     - (R) True = Yes must be present, false = optional argument.
-//          vbHandleByCmd   - (R) True = Command processes *this option, false = not handled.
+// Args:    vrArgName          - (R) Argument's name to search by.
+//          vbMandatory        - (R) True = Yes must be present, false = optional argument.
+//          vbHandleByCmd      - (R) True = Command processes *this option, false = not handled.
+//          vnNumberFormatMask - (R) Mask of the number formats. (Dflt = CMICmdArgValNumber::eArgValNumberFormat_Decimal)
 // Return:  None.
 // Throws:  None.
 //--
-CMICmdArgValNumber::CMICmdArgValNumber(const CMIUtilString &vrArgName, const bool vbMandatory, const bool vbHandleByCmd)
+CMICmdArgValNumber::CMICmdArgValNumber(const CMIUtilString &vrArgName, const bool vbMandatory, const bool vbHandleByCmd,
+                                       const MIuint vnNumberFormatMask /* = CMICmdArgValNumber::eArgValNumberFormat_Decimal*/)
     : CMICmdArgValBaseTemplate(vrArgName, vbMandatory, vbHandleByCmd)
+    , m_nNumberFormatMask(vnNumberFormatMask)
     , m_nNumber(0)
 {
 }
 
 //++ ------------------------------------------------------------------------------------
 // Details: CMICmdArgValNumber destructor.
 // Type:    Overridden.
 // Args:    None.

 // Type:    Method.
 // Args:    vrTxt   - (R) Some text.
 // Return:  bool    - True = yes valid arg, false = no.
 // Throws:  None.
 //--
 bool
 CMICmdArgValNumber::IsArgNumber(const CMIUtilString &vrTxt) const
 {
+    const bool bFormatDecimal(m_nNumberFormatMask & CMICmdArgValNumber::eArgValNumberFormat_Decimal);
+    const bool bFormatHexadecimal(m_nNumberFormatMask & CMICmdArgValNumber::eArgValNumberFormat_Hexadecimal);
+
     // Look for --someLongOption
     if (std::string::npos != vrTxt.find("--"))
         return false;
 
-    return vrTxt.IsNumber();
+    if (bFormatDecimal && vrTxt.IsNumber())
+        return true;
+
+    if (bFormatHexadecimal && vrTxt.IsHexadecimalNumber())
+        return true;
+
+    return false;
 }
 
 //++ ------------------------------------------------------------------------------------
 // Details: Extract the thread group number from the thread group argument.
 // Type:    Method.
 // Args:    vrTxt   - (R) Some text.
 // Return:  MIstatus::success - Functional succeeded.
 //          MIstatus::failure - Functional failed.
 // Throws:  None.
 //--
 bool
 CMICmdArgValNumber::ExtractNumber(const CMIUtilString &vrTxt)
 {
     MIint64 nNumber = 0;
     bool bOk = vrTxt.ExtractNumber(nNumber);
     if (bOk)
     {
-        m_nNumber = static_cast<MIint>(nNumber);
+        m_nNumber = static_cast<MIint64>(nNumber);
     }
 
     return bOk;
 }
 
 //++ ------------------------------------------------------------------------------------
 // Details: Retrieve the thread group ID found in the argument.
 // Type:    Method.

tools/lldb-mi/MICmdCmdData.cpp

 bool
 CMICmdCmdDataReadMemoryBytes::ParseArgs(void)
 {
     bool bOk =
         m_setCmdArgs.Add(*(new CMICmdArgValOptionLong(m_constStrArgThread, false, false, CMICmdArgValListBase::eArgValType_Number, 1)));
     bOk =
         bOk &&
         m_setCmdArgs.Add(*(new CMICmdArgValOptionShort(m_constStrArgByteOffset, false, true, CMICmdArgValListBase::eArgValType_Number, 1)));
-    bOk = bOk && m_setCmdArgs.Add(*(new CMICmdArgValNumber(m_constStrArgAddrStart, true, true)));
+    bOk = bOk && m_setCmdArgs.Add(*(new CMICmdArgValNumber(m_constStrArgAddrStart, true, true, CMICmdArgValNumber::eArgValNumberFormat_Auto)));
     bOk = bOk && m_setCmdArgs.Add(*(new CMICmdArgValNumber(m_constStrArgNumBytes, true, true)));
     return (bOk && ParseValidateCmdOptions());
 }
 
 //++ ------------------------------------------------------------------------------------
 // Details: The invoker requires this function. The command does work in this function.
 //          The command is likely to communicate with the LLDB SBDebugger in here.
 // Type:    Overridden.

 // Return:  MIstatus::success - Functional succeeded.
 //          MIstatus::failure - Functional failed.
 // Throws:  None.
 //--
 bool
 CMICmdCmdDataReadMemoryBytes::Acknowledge(void)
 {
     // MI: memory=[{begin=\"0x%08x\",offset=\"0x%08x\",end=\"0x%08x\",contents=\" \" }]"
-    const CMICmnMIValueConst miValueConst(CMIUtilString::Format("0x%08x", m_nAddrStart));
+    const CMICmnMIValueConst miValueConst(CMIUtilString::Format("0x%08llx", m_nAddrStart));
     const CMICmnMIValueResult miValueResult("begin", miValueConst);
     CMICmnMIValueTuple miValueTuple(miValueResult);
-    const CMICmnMIValueConst miValueConst2(CMIUtilString::Format("0x%08x", m_nAddrOffset));
+    const CMICmnMIValueConst miValueConst2(CMIUtilString::Format("0x%08llx", m_nAddrOffset));
     const CMICmnMIValueResult miValueResult2("offset", miValueConst2);
     miValueTuple.Add(miValueResult2);
-    const CMICmnMIValueConst miValueConst3(CMIUtilString::Format("0x%08x", m_nAddrStart + m_nAddrNumBytesToRead));
+    const CMICmnMIValueConst miValueConst3(CMIUtilString::Format("0x%08llx", m_nAddrStart + m_nAddrNumBytesToRead));
     const CMICmnMIValueResult miValueResult3("end", miValueConst3);
     miValueTuple.Add(miValueResult3);
 
     // MI: contents=\" \"
     CMIUtilString strContent;
     strContent.reserve((m_nAddrNumBytesToRead << 1) + 1);
     for (MIuint64 i = 0; i < m_nAddrNumBytesToRead; i++)
     {
-        strContent += CMIUtilString::Format("%02x", m_pBufferMemory[i]);
+        strContent += CMIUtilString::Format("%02hhx", m_pBufferMemory[i]);
     }
     const CMICmnMIValueConst miValueConst4(strContent);
     const CMICmnMIValueResult miValueResult4("contents", miValueConst4);
     miValueTuple.Add(miValueResult4);
     const CMICmnMIValueList miValueList(miValueTuple);
     const CMICmnMIValueResult miValueResult5("memory", miValueList);
 
     const CMICmnMIResultRecord miRecordResult(m_cmdData.strMiCmdToken, CMICmnMIResultRecord::eResultClass_Done, miValueResult5);

tools/lldb-mi/MIUtilString.h

class CMIUtilString : public std::string
   public:
     /* ctor */ CMIUtilString(void);
     /* ctor */ CMIUtilString(const MIchar *vpData);
     /* ctor */ CMIUtilString(const MIchar *const *vpData);
     //
     bool ExtractNumber(MIint64 &vwrNumber) const;
     CMIUtilString FindAndReplace(const CMIUtilString &vFind, const CMIUtilString &vReplaceWith) const;
     bool IsNumber(void) const;
+    bool IsHexadecimalNumber(void) const;
     bool IsQuoted(void) const;
     CMIUtilString RemoveRepeatedCharacters(const MIchar vChar);
     MIuint Split(const CMIUtilString &vDelimiter, VecString_t &vwVecSplits) const;
     MIuint SplitConsiderQuotes(const CMIUtilString &vDelimiter, VecString_t &vwVecSplits) const;
     CMIUtilString StripCREndOfLine(void) const;
     CMIUtilString StripCRAll(void) const;
     CMIUtilString Trim(void) const;
     CMIUtilString Trim(const MIchar vChar) const;

tools/lldb-mi/MIUtilString.cpp

 //
 // Copyright:   None.
 //--
 
 // Third party headers
 #include <memory>   // std::unique_ptr
 #include <stdarg.h> // va_list, va_start, var_end
 #include <sstream>  // std::stringstream
-#include <string.h> // for strcpy
+#include <string.h> // for strncmp
 #include <limits.h> // for ULONG_MAX
 
 // In-house headers:
 #include "MIUtilString.h"
 
 //++ ------------------------------------------------------------------------------------
 // Details: CMIUtilString constructor.
 // Type:    Method.
CMIUtilString::IsNumber(void) const
     const MIint nPos = find_first_not_of("-.0123456789");
     if (nPos != (MIint)std::string::npos)
         return false;
 
     return true;
 }
 
 //++ ------------------------------------------------------------------------------------
+// Details: Check if *this string is a hexadecimal number.
+// Type:    Method.
+// Args:    None.
+// Return:  bool - True = yes number, false not a number.
+// Throws:  None.
+//--
+bool
+CMIUtilString::IsHexadecimalNumber(void) const
+{
+    if ((strncmp(c_str(), "0x", 2) != 0) && (strncmp(c_str(), "0X", 2) != 0)) // Compare '0x..' prefix
+        return false;
+
+    const MIint nPos = find_first_not_of("01234567890ABCDEFabcedf", 2); // Skip '0x..' prefix
+    if (nPos != (MIint)std::string::npos)
+        return false;
+
+    return true;
+}
+
+//++ ------------------------------------------------------------------------------------
 // Details: Extract the number from the string. The number can be either a hexadecimal or
 //          natural number. It cannot contain other non-numeric characters.
 // Type:    Method.
 // Args:    vwrNumber   - (W) Number exracted from the string.
 // Return:  bool - True = yes number, false not a number.
 // Throws:  None.
 //--
 bool

 // Return:  bool - True = yes number, false not a number.
 // Throws:  None.
 //--
 bool
 CMIUtilString::ExtractNumberFromHexadecimal(MIint64 &vwrNumber) const
 {
     vwrNumber = 0;
 
-    const MIint nPos = find_first_not_of("x01234567890ABCDEFabcedf");
+    const MIint nPos = find_first_not_of("xX01234567890ABCDEFabcedf");
     if (nPos != (MIint)std::string::npos)
         return false;
 
-    const MIint64 nNum = ::strtoul(this->c_str(), nullptr, 16);
-    if (nNum != LONG_MAX)
-    {
-        vwrNumber = nNum;
-        return true;
-    }
+    const MIuint64 nNum = ::strtoull(this->c_str(), nullptr, 16);
+    if (nNum == ULLONG_MAX && errno == ERANGE)
+        return false;
+
+    vwrNumber = static_cast<MIint64>(nNum);
 
     return true;
 }
 
 //++ ------------------------------------------------------------------------------------
 // Details: Determine if the text is all valid alpha numeric characters. Letters can be
 //          either upper or lower case.
 // Type:    Static method.