Index: test/tools/lldb-mi/data/TestMiData.py =================================================================== --- test/tools/lldb-mi/data/TestMiData.py +++ test/tools/lldb-mi/data/TestMiData.py @@ -41,9 +41,9 @@ @lldbmi_test @expectedFailureWindows("llvm.org/pr22274: need a pexpect replacement for windows") @skipIfFreeBSD # llvm.org/pr22411: Failure presumably due to known thread races - @unittest2.skip("-data-evaluate-expression doesn't work") #FIXME: the global case worked before refactoring - def test_lldbmi_data_read_memory_bytes(self): - """Test that 'lldb-mi --interpreter' works for -data-read-memory-bytes.""" + @unittest2.skip("-data-evaluate-expression doesn't work on globals") #FIXME: the global case worked before refactoring + def test_lldbmi_data_read_memory_bytes_global(self): + """Test that -data-read-memory-bytes can access global buffers.""" self.spawnLldbMi(args = None) @@ -81,6 +81,114 @@ @lldbmi_test @expectedFailureWindows("llvm.org/pr22274: need a pexpect replacement for windows") @skipIfFreeBSD # llvm.org/pr22411: Failure presumably due to known thread races + def test_lldbmi_data_read_memory_bytes_local(self): + """Test that -data-read-memory-bytes can access local buffers.""" + + self.spawnLldbMi(args = None) + self.expect(self.child_prompt, exactly = True) + + # Load executable + self.runCmd('-file-exec-and-symbols %s' % self.myexe) + self.expect(r'\^done') + + # Run to BP_local_array_test_inner + line = line_number('main.cpp', '// BP_local_array_test_inner') + self.runCmd('-break-insert main.cpp:%d' % line) + self.expect(r'\^done,bkpt=\{number="1"') + self.runCmd('-exec-run') + self.expect(r'\^running') + self.expect(r'\*stopped,reason="breakpoint-hit"') + + # Get address of local char[] + self.runCmd('-data-evaluate-expression &array') + self.expect(r'\^done,value="0x[0-9a-f]+"') + addr = int(self.child.after.split('"')[1], 16) + size = 4 + + # Test that an unquoted hex literal address works + self.runCmd('-data-read-memory-bytes %#x %d' % (addr, size)) + self.expect(r'\^done,memory=\[\{begin="0x0*%x",offset="0x0+",end="0x0*%x",contents="01020304"\}\]' % (addr, addr + size)) + + # Test that a double-quoted hex literal address works + self.runCmd('-data-read-memory-bytes "%#x" %d' % (addr, size)) + self.expect(r'\^done,memory=\[\{begin="0x0*%x",offset="0x0+",end="0x0*%x",contents="01020304"\}\]' % (addr, addr + size)) + + # Test that unquoted expressions work + self.runCmd('-data-read-memory-bytes &array %d' % size) + self.expect(r'\^done,memory=\[\{begin="0x0*%x",offset="0x0+",end="0x0*%x",contents="01020304"\}\]' % (addr, addr + size)) + + # This doesn't work, and perhaps that makes sense, but it does work on GDB + self.runCmd('-data-read-memory-bytes array 4') + self.expect(r'\^error') + #self.expect(r'\^done,memory=\[\{begin="0x0*%x",offset="0x0+",end="0x0*%x",contents="01020304"\}\]' % (addr, addr + size)) + + self.runCmd('-data-read-memory-bytes &array[2] 2') + self.expect(r'\^done,memory=\[\{begin="0x0*%x",offset="0x0+",end="0x0*%x",contents="0304"\}\]' % (addr + 2, addr + size)) + + self.runCmd('-data-read-memory-bytes first_element_ptr %d' % size) + self.expect(r'\^done,memory=\[\{begin="0x0*%x",offset="0x0+",end="0x0*%x",contents="01020304"\}\]' % (addr, addr + size)) + + # Test that double-quoted expressions work + self.runCmd('-data-read-memory-bytes "&array" %d' % size) + self.expect(r'\^done,memory=\[\{begin="0x0*%x",offset="0x0+",end="0x0*%x",contents="01020304"\}\]' % (addr, addr + size)) + + self.runCmd('-data-read-memory-bytes "&array[0] + 1" 3') + self.expect(r'\^done,memory=\[\{begin="0x0*%x",offset="0x0+",end="0x0*%x",contents="020304"\}\]' % (addr + 1, addr + size)) + + self.runCmd('-data-read-memory-bytes "first_element_ptr + 1" 3') + self.expect(r'\^done,memory=\[\{begin="0x0*%x",offset="0x0+",end="0x0*%x",contents="020304"\}\]' % (addr + 1, addr + size)) + + # Test the -o (offset) option + self.runCmd('-data-read-memory-bytes -o 1 &array 3') + self.expect(r'\^done,memory=\[\{begin="0x0*%x",offset="0x0+",end="0x0*%x",contents="020304"\}\]' % (addr + 1, addr + size)) + + # Test the --thread option + self.runCmd('-data-read-memory-bytes --thread 1 &array 4') + self.expect(r'\^done,memory=\[\{begin="0x0*%x",offset="0x0+",end="0x0*%x",contents="01020304"\}\]' % (addr, addr + size)) + + # Test the --thread option with an invalid value + self.runCmd('-data-read-memory-bytes --thread 999 &array 4') + self.expect(r'\^error') + + # Test the --frame option (current frame) + self.runCmd('-data-read-memory-bytes --frame 0 &array 4') + self.expect(r'\^done,memory=\[\{begin="0x0*%x",offset="0x0+",end="0x0*%x",contents="01020304"\}\]' % (addr, addr + size)) + + # Test the --frame option (outer frame) + self.runCmd('-data-read-memory-bytes --frame 1 &array 4') + self.expect(r'\^done,memory=\[\{begin="0x[0-9a-f]+",offset="0x0+",end="0x[0-9a-f]+",contents="05060708"\}\]') + + # Test the --frame option with an invalid value + self.runCmd('-data-read-memory-bytes --frame 999 &array 4') + self.expect(r'\^error') + + # Test all the options at once + self.runCmd('-data-read-memory-bytes --thread 1 --frame 1 -o 2 &array 2') + self.expect(r'\^done,memory=\[\{begin="0x[0-9a-f]+",offset="0x0+",end="0x[0-9a-f]+",contents="0708"\}\]') + + # Test that an expression that references undeclared variables doesn't work + self.runCmd('-data-read-memory-bytes "&undeclared_array1 + undeclared_array2[1]" 2') + self.expect(r'\^error') + + # Test that the address argument is required + self.runCmd('-data-read-memory-bytes') + self.expect(r'\^error') + + # Test that the count argument is required + self.runCmd('-data-read-memory-bytes &array') + self.expect(r'\^error') + + # Test that the address argument is required when other options are present + self.runCmd('-data-read-memory-bytes --thread 1') + 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') + + @lldbmi_test + @expectedFailureWindows("llvm.org/pr22274: need a pexpect replacement for windows") + @skipIfFreeBSD # llvm.org/pr22411: Failure presumably due to known thread races def test_lldbmi_data_list_register_names(self): """Test that 'lldb-mi --interpreter' works for -data-list-register-names.""" Index: test/tools/lldb-mi/data/main.cpp =================================================================== --- test/tools/lldb-mi/data/main.cpp +++ test/tools/lldb-mi/data/main.cpp @@ -10,8 +10,27 @@ const char g_CharArray[] = "\x10\x11\x12\x13"; static const char s_CharArray[] = "\x20\x21\x22\x23"; +void +local_array_test_inner() +{ + char array[] = { 0x01, 0x02, 0x03, 0x04 }; + char *first_element_ptr = &array[0]; + // BP_local_array_test_inner + return; +} + +void +local_array_test() +{ + char array[] = { 0x05, 0x06, 0x07, 0x08 }; + // BP_local_array_test + local_array_test_inner(); + return; +} + int main(int argc, char const *argv[]) { // FUNC_main + local_array_test(); return 0; } Index: tools/lldb-mi/MICmdCmdData.h =================================================================== --- tools/lldb-mi/MICmdCmdData.h +++ tools/lldb-mi/MICmdCmdData.h @@ -150,14 +150,14 @@ // Attributes: private: - const CMIUtilString m_constStrArgThread; // Not specified in MI spec but Eclipse gives this option. Not handled by command. + const CMIUtilString m_constStrArgThread; // Not in the MI spec but implemented by GDB. + const CMIUtilString m_constStrArgFrame; // Not in the MI spec but implemented by GDB. const CMIUtilString m_constStrArgByteOffset; - const CMIUtilString m_constStrArgAddrStart; + const CMIUtilString m_constStrArgAddrExpr; const CMIUtilString m_constStrArgNumBytes; MIuchar *m_pBufferMemory; MIuint64 m_nAddrStart; MIuint64 m_nAddrNumBytesToRead; - MIuint64 m_nAddrOffset; }; //++ ============================================================================ Index: tools/lldb-mi/MICmdCmdData.cpp =================================================================== --- tools/lldb-mi/MICmdCmdData.cpp +++ tools/lldb-mi/MICmdCmdData.cpp @@ -517,13 +517,13 @@ //-- CMICmdCmdDataReadMemoryBytes::CMICmdCmdDataReadMemoryBytes(void) : m_constStrArgThread("thread") + , m_constStrArgFrame("frame") , m_constStrArgByteOffset("o") - , m_constStrArgAddrStart("address") + , m_constStrArgAddrExpr("address") , m_constStrArgNumBytes("count") , m_pBufferMemory(nullptr) , m_nAddrStart(0) , m_nAddrNumBytesToRead(0) - , m_nAddrOffset(0) { // Command factory matches this name with that received from the stdin stream m_strMiCmd = "data-read-memory-bytes"; @@ -561,11 +561,13 @@ CMICmdCmdDataReadMemoryBytes::ParseArgs(void) { bool bOk = - m_setCmdArgs.Add(*(new CMICmdArgValOptionLong(m_constStrArgThread, false, false, CMICmdArgValListBase::eArgValType_Number, 1))); + m_setCmdArgs.Add(*(new CMICmdArgValOptionLong(m_constStrArgThread, false, true, CMICmdArgValListBase::eArgValType_Number, 1))); + bOk = bOk && + m_setCmdArgs.Add(*(new CMICmdArgValOptionLong(m_constStrArgFrame, false, true, 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, CMICmdArgValNumber::eArgValNumberFormat_Auto))); + bOk = bOk && m_setCmdArgs.Add(*(new CMICmdArgValString(m_constStrArgAddrExpr, true, true, true, true))); bOk = bOk && m_setCmdArgs.Add(*(new CMICmdArgValNumber(m_constStrArgNumBytes, true, true))); return (bOk && ParseValidateCmdOptions()); } @@ -575,22 +577,109 @@ // The command is likely to communicate with the LLDB SBDebugger in here. // Type: Overridden. // Args: None. -// Return: MIstatus::success - Functional succeeded. -// MIstatus::failure - Functional failed. +// Return: MIstatus::success - Function succeeded. +// MIstatus::failure - Function failed. // Throws: None. //-- bool CMICmdCmdDataReadMemoryBytes::Execute(void) { - CMICMDBASE_GETOPTION(pArgAddrStart, Number, m_constStrArgAddrStart); - CMICMDBASE_GETOPTION(pArgAddrOffset, Number, m_constStrArgByteOffset); + CMICMDBASE_GETOPTION(pArgThread, OptionLong, m_constStrArgThread); + CMICMDBASE_GETOPTION(pArgFrame, OptionLong, m_constStrArgFrame); + CMICMDBASE_GETOPTION(pArgAddrOffset, OptionShort, m_constStrArgByteOffset); + CMICMDBASE_GETOPTION(pArgAddrExpr, String, m_constStrArgAddrExpr); CMICMDBASE_GETOPTION(pArgNumBytes, Number, m_constStrArgNumBytes); + + // get the --thread option value + MIuint64 nThreadId = UINT64_MAX; + if (pArgThread->GetFound() && !pArgThread->GetExpectedOption(nThreadId)) + { + SetError(CMIUtilString::Format(MIRSRC(IDS_CMD_ERR_OPTION_NOT_FOUND), + m_cmdData.strMiCmd.c_str(), m_constStrArgThread.c_str())); + return MIstatus::failure; + } - const MIuint64 nAddrStart = pArgAddrStart->GetValue(); - const MIuint64 nAddrNumBytes = pArgNumBytes->GetValue(); - if (pArgAddrOffset->GetFound()) - m_nAddrOffset = pArgAddrOffset->GetValue(); + // get the --frame option value + MIuint64 nFrame = UINT64_MAX; + if (pArgFrame->GetFound() && !pArgFrame->GetExpectedOption(nFrame)) + { + SetError(CMIUtilString::Format(MIRSRC(IDS_CMD_ERR_OPTION_NOT_FOUND), + m_cmdData.strMiCmd.c_str(), m_constStrArgFrame.c_str())); + return MIstatus::failure; + } + + // get the -o option value + MIuint64 nAddrOffset = 0; + if (pArgAddrOffset->GetFound() && !pArgAddrOffset->GetExpectedOption(nAddrOffset)) + { + SetError(CMIUtilString::Format(MIRSRC(IDS_CMD_ERR_OPTION_NOT_FOUND), + m_cmdData.strMiCmd.c_str(), m_constStrArgByteOffset.c_str())); + 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()) + { + SetError(CMIUtilString::Format(MIRSRC(IDS_CMD_ERR_INVALID_PROCESS), m_cmdData.strMiCmd.c_str())); + return MIstatus::failure; + } + + lldb::SBThread thread = (nThreadId != UINT64_MAX) ? + sbProcess.GetThreadByIndexID(nThreadId) : sbProcess.GetSelectedThread(); + if (!thread.IsValid()) + { + SetError(CMIUtilString::Format(MIRSRC(IDS_CMD_ERR_THREAD_INVALID), m_cmdData.strMiCmd.c_str())); + return MIstatus::failure; + } + + lldb::SBFrame frame = (nFrame != UINT64_MAX) ? + thread.GetFrameAtIndex(nFrame) : thread.GetSelectedFrame(); + if (!frame.IsValid()) + { + SetError(CMIUtilString::Format(MIRSRC(IDS_CMD_ERR_FRAME_INVALID), m_cmdData.strMiCmd.c_str())); + return MIstatus::failure; + } + + const CMIUtilString &rAddrExpr = pArgAddrExpr->GetValue(); + lldb::SBValue addrExprValue = frame.EvaluateExpression(rAddrExpr.c_str()); + lldb::SBError error = addrExprValue.GetError(); + if (error.Fail()) + { + SetError(error.GetCString()); + return MIstatus::failure; + } + else if (!addrExprValue.IsValid()) + { + SetError(CMIUtilString::Format(MIRSRC(IDS_CMD_ERR_EXPR_INVALID), rAddrExpr.c_str())); + return MIstatus::failure; + } + MIuint64 nAddrStart = 0; + if (!CMICmnLLDBProxySBValue::GetValueAsUnsigned(addrExprValue, nAddrStart)) + { + SetError(CMIUtilString::Format(MIRSRC(IDS_CMD_ERR_EXPR_INVALID), rAddrExpr.c_str())); + return MIstatus::failure; + } + + nAddrStart += nAddrOffset; + const MIuint64 nAddrNumBytes = pArgNumBytes->GetValue(); + m_pBufferMemory = new MIuchar[nAddrNumBytes]; if (m_pBufferMemory == nullptr) { @@ -598,9 +687,7 @@ return MIstatus::failure; } - CMICmnLLDBDebugSessionInfo &rSessionInfo(CMICmnLLDBDebugSessionInfo::Instance()); - lldb::SBProcess sbProcess = rSessionInfo.GetProcess(); - lldb::SBError error; + error.Clear(); const MIuint64 nReadBytes = sbProcess.ReadMemory(static_cast(nAddrStart), (void *)m_pBufferMemory, nAddrNumBytes, error); if (nReadBytes != nAddrNumBytes) { @@ -640,7 +727,8 @@ const CMICmnMIValueConst miValueConst(CMIUtilString::Format("0x%016" PRIx64, m_nAddrStart)); const CMICmnMIValueResult miValueResult("begin", miValueConst); CMICmnMIValueTuple miValueTuple(miValueResult); - const CMICmnMIValueConst miValueConst2(CMIUtilString::Format("0x%016" PRIx64, m_nAddrOffset)); + const MIuint64 nAddrOffset = 0; + const CMICmnMIValueConst miValueConst2(CMIUtilString::Format("0x%016" PRIx64, nAddrOffset)); const CMICmnMIValueResult miValueResult2("offset", miValueConst2); miValueTuple.Add(miValueResult2); const CMICmnMIValueConst miValueConst3(CMIUtilString::Format("0x%016" PRIx64, m_nAddrStart + m_nAddrNumBytesToRead)); Index: tools/lldb-mi/MICmnResources.h =================================================================== --- tools/lldb-mi/MICmnResources.h +++ tools/lldb-mi/MICmnResources.h @@ -267,7 +267,8 @@ IDS_CMD_ERR_GDBSET_OPT_PRINT_BAD_ARGS, IDS_CMD_ERR_GDBSET_OPT_PRINT_UNKNOWN_OPTION, IDS_CMD_ERR_GDBSHOW_OPT_PRINT_BAD_ARGS, - IDS_CMD_ERR_GDBSHOW_OPT_PRINT_UNKNOWN_OPTION + IDS_CMD_ERR_GDBSHOW_OPT_PRINT_UNKNOWN_OPTION, + IDS_CMD_ERR_EXPR_INVALID }; //++ ============================================================================ Index: tools/lldb-mi/MICmnResources.cpp =================================================================== --- tools/lldb-mi/MICmnResources.cpp +++ tools/lldb-mi/MICmnResources.cpp @@ -249,7 +249,8 @@ {IDS_CMD_ERR_GDBSET_OPT_PRINT_BAD_ARGS, "'print' expects option-name and \"on\" or \"off\""}, {IDS_CMD_ERR_GDBSET_OPT_PRINT_UNKNOWN_OPTION, "'print' error. The option '%s' not found"}, {IDS_CMD_ERR_GDBSHOW_OPT_PRINT_BAD_ARGS, "'print' expects option-name and \"on\" or \"off\""}, - {IDS_CMD_ERR_GDBSHOW_OPT_PRINT_UNKNOWN_OPTION, "'print' error. The option '%s' not found"}}; + {IDS_CMD_ERR_GDBSHOW_OPT_PRINT_UNKNOWN_OPTION, "'print' error. The option '%s' not found"}, + {IDS_CMD_ERR_EXPR_INVALID, "Failed to evaluate expression: %s"}}; //++ ------------------------------------------------------------------------------------ // Details: CMICmnResources constructor.