dawn retitled this revision from "[LLDB-MI] Fix -data-info-line when Windows
filenames are used." to "[LLDB-MI] Fix -data-info-line and -symbol-list-lines
when Windows filenames are used.".
dawn updated the summary for this revision.
dawn updated this revision to Diff 34255.
dawn added a comment.
This changes the code to use regex to handle the parsing. It is cleaner, more
accurate, and faster than the previous code, while not requiring special
support to check for Windows paths.
In reworking this patch, I discovered a major bug in -symbol-list-lines where
it didn't check the filename so would report lines in header files as belonging
to the compilation unit. That is fixed in this patch and a test added, but to
fix -symbol-list-lines for header files requires a rewrite of
-symbol-list-lines which I didn't have time for. I've added a FIXME for now.
Repository:
rL LLVM
http://reviews.llvm.org/D12115
Files:
test/tools/lldb-mi/symbol/Makefile
test/tools/lldb-mi/symbol/TestMiSymbol.py
test/tools/lldb-mi/symbol/main.cpp
test/tools/lldb-mi/symbol/x.cpp
test/tools/lldb-mi/symbol/x.h
tools/lldb-mi/MICmdCmdData.cpp
tools/lldb-mi/MICmdCmdSymbol.cpp
tools/lldb-mi/MIUtilString.cpp
tools/lldb-mi/MIUtilString.h
Index: tools/lldb-mi/MIUtilString.h
===================================================================
--- tools/lldb-mi/MIUtilString.h
+++ tools/lldb-mi/MIUtilString.h
@@ -43,6 +43,7 @@
/* ctor */ CMIUtilString();
/* ctor */ CMIUtilString(const char *vpData);
/* ctor */ CMIUtilString(const char *const *vpData);
+ /* ctor */ CMIUtilString(const char *vpData, size_t nLen);
//
bool ExtractNumber(MIint64 &vwrNumber) const;
CMIUtilString FindAndReplace(const CMIUtilString &vFind, const CMIUtilString &vReplaceWith) const;
Index: tools/lldb-mi/MIUtilString.cpp
===================================================================
--- tools/lldb-mi/MIUtilString.cpp
+++ tools/lldb-mi/MIUtilString.cpp
@@ -55,6 +55,20 @@
}
//++ ------------------------------------------------------------------------------------
+// Details: CMIUtilString constructor.
+// Type: Method.
+// Args: vpData - Pointer to UTF8 text data.
+// nLen - Length of string.
+// Return: None.
+// Throws: None.
+//--
+CMIUtilString::CMIUtilString(const char *vpData, size_t nLen)
+ : std::string(vpData, nLen)
+{
+}
+
+
+//++ ------------------------------------------------------------------------------------
// Details: CMIUtilString assignment operator.
// Type: Method.
// Args: vpRhs - Pointer to UTF8 text data.
Index: tools/lldb-mi/MICmdCmdSymbol.cpp
===================================================================
--- tools/lldb-mi/MICmdCmdSymbol.cpp
+++ tools/lldb-mi/MICmdCmdSymbol.cpp
@@ -11,6 +11,9 @@
// Third Party Headers:
#include "lldb/API/SBCommandInterpreter.h"
+#include "lldb/Host/FileSpec.h"
+#include "lldb/Core/RegularExpression.h"
+#include "llvm/ADT/StringRef.h"
// In-house headers:
#include "MICmdArgValFile.h"
@@ -20,6 +23,8 @@
#include "MICmnMIValueList.h"
#include "MICmnMIValueTuple.h"
+using namespace lldb_private;
+
//++ ------------------------------------------------------------------------------------
// Details: CMICmdCmdSymbolListLines constructor.
// Type: Method.
@@ -81,6 +86,10 @@
CMICMDBASE_GETOPTION(pArgFile, File, m_constStrArgNameFile);
const CMIUtilString &strFilePath(pArgFile->GetValue());
+ // FIXME: this won't work for header files! To try and use existing
+ // commands to get this to work for header files would be too slow.
+ // Instead, this code should be rewritten to use APIs and/or support
+ // should be added to lldb which would work for header files.
const CMIUtilString strCmd(CMIUtilString::Format("target modules dump line-table \"%s\"", strFilePath.AddSlashes().c_str()));
CMICmnLLDBDebugSessionInfo &rSessionInfo(CMICmnLLDBDebugSessionInfo::Instance());
@@ -91,6 +100,100 @@
}
//++ ------------------------------------------------------------------------------------
+// Details: Helper function for parsing the header returned from lldb for the command:
+// target modules dump line-table <file>
+// where the header is of the format:
+// Line table for /path/to/file in `/path/to/module
+// Args: input - (R) Input string to parse.
+// file - (W) String representing the file.
+// Return: bool - True = input was parsed successfully, false = input could not be parsed.
+// Throws: None.
+//--
+static bool
+ParseLLDBLineAddressHeader (const char *input, CMIUtilString &file)
+{
+ // Match LineEntry using regex.
+ static RegularExpression g_lineentry_header_regex(
+ "^ *Line table for (.+) in `(.+)$");
+ // ^1=file
+
+ llvm::StringRef fileRef;
+ RegularExpression::Match match(3);
+
+ if (g_lineentry_header_regex.Execute (input, &match))
+ {
+ match.GetMatchAtIndex(input, 1, fileRef);
+ }
+ else
+ {
+ // No match.
+ return false;
+ }
+ file = CMIUtilString(fileRef.data(), fileRef.size());
+ return true;
+}
+
+//++ ------------------------------------------------------------------------------------
+// Details: Helper function for parsing a line entry returned from lldb for the command:
+// target modules dump line-table <file>
+// where the line entry is of the format:
+// 0x0000000100000e70: /path/to/file:3002[:4]
+// addr file line column(opt)
+// Args: input - (R) Input string to parse.
+// addr - (W) String representing the pc address.
+// file - (W) String representing the file.
+// line - (W) String representing the line.
+// Return: bool - True = input was parsed successfully, false = input could not be parsed.
+// Throws: None.
+//--
+static bool
+ParseLLDBLineAddressEntry (const char *input, CMIUtilString &addr,
+ CMIUtilString &file, CMIUtilString &line)
+{
+ // Note: Ambiguities arise because the column is optional, and
+ // because : can appear in filenames or as a byte in a multibyte
+ // UTF8 character. We keep those cases to a minimum by using regex
+ // to work on the string from both the left and right, so that what
+ // is remains is assumed to be the filename.
+
+ // Match LineEntry using regex.
+ static RegularExpression g_lineentry_nocol_regex(
+ "^ *(0x[0-9a-fA-F]+): (.+):([0-9]+)$");
+ static RegularExpression g_lineentry_col_regex(
+ "^ *(0x[0-9a-fA-F]+): (.+):([0-9]+):[0-9]+$");
+ // ^1=addr ^2=f ^3=line ^4=:col(opt)
+
+ llvm::StringRef addrRef;
+ llvm::StringRef fileRef;
+ llvm::StringRef lineRef;
+ RegularExpression::Match match(4);
+
+ // First try matching the LineEntry with the column.
+ if (g_lineentry_col_regex.Execute (input, &match))
+ {
+ match.GetMatchAtIndex(input, 1, addrRef);
+ match.GetMatchAtIndex(input, 2, fileRef);
+ match.GetMatchAtIndex(input, 3, lineRef);
+ }
+ // Then try without the column.
+ else if (g_lineentry_nocol_regex.Execute (input, &match))
+ {
+ match.GetMatchAtIndex(input, 1, addrRef);
+ match.GetMatchAtIndex(input, 2, fileRef);
+ match.GetMatchAtIndex(input, 3, lineRef);
+ }
+ else
+ {
+ // No match.
+ return false;
+ }
+ addr = CMIUtilString(addrRef.data(), addrRef.size());
+ file = CMIUtilString(fileRef.data(), fileRef.size());
+ line = CMIUtilString(lineRef.data(), lineRef.size());
+ return true;
+}
+
+//++ ------------------------------------------------------------------------------------
// Details: The invoker requires this function. The command prepares a MI Record Result
// for the work carried out in the Execute().
// Type: Overridden.
@@ -117,6 +220,14 @@
const CMIUtilString strLldbMsg(m_lldbResult.GetOutput());
const MIuint nLines(strLldbMsg.SplitLines(vecLines));
+ // Parse the file from the header.
+ const CMIUtilString &rWantFile(vecLines[0]);
+ CMIUtilString strWantFile;
+ if (!ParseLLDBLineAddressHeader (rWantFile.c_str(), strWantFile))
+ // No lines for requested file.
+ return MIstatus::failure;
+
+ // Parse the line address entries.
CMICmnMIValueList miValueList(true);
for (MIuint i = 1; i < nLines; ++i)
{
@@ -123,23 +234,21 @@
// String looks like:
// 0x0000000100000e70: /path/to/file:3[:4]
const CMIUtilString &rLine(vecLines[i]);
+ CMIUtilString strAddr;
+ CMIUtilString strFile;
+ CMIUtilString strLine;
- // 0x0000000100000e70: /path/to/file:3[:4]
- // ^^^^^^^^^^^^^^^^^^ -- pc
- const size_t nAddrEndPos = rLine.find(':');
- const CMIUtilString strAddr(rLine.substr(0, nAddrEndPos).c_str());
+ if (!ParseLLDBLineAddressEntry(rLine.c_str(), strAddr, strFile, strLine))
+ continue;
+
+ // Skip entries which don't match the desired source.
+ if (strWantFile != strFile)
+ continue;
+
const CMICmnMIValueConst miValueConst(strAddr);
const CMICmnMIValueResult miValueResult("pc", miValueConst);
CMICmnMIValueTuple miValueTuple(miValueResult);
- // 0x0000000100000e70: /path/to/file:3[:4]
- // ^ -- line
- const size_t nLineOrColumnStartPos = rLine.rfind(':');
- const CMIUtilString strLineOrColumn(rLine.substr(nLineOrColumnStartPos + 1).c_str());
- const size_t nPathOrLineStartPos = rLine.rfind(':', nLineOrColumnStartPos - 1);
- const size_t nPathOrLineLen = nLineOrColumnStartPos - nPathOrLineStartPos - 1;
- const CMIUtilString strPathOrLine(rLine.substr(nPathOrLineStartPos + 1, nPathOrLineLen).c_str());
- const CMIUtilString strLine(strPathOrLine.IsNumber() ? strPathOrLine : strLineOrColumn);
const CMICmnMIValueConst miValueConst2(strLine);
const CMICmnMIValueResult miValueResult2("line", miValueConst2);
miValueTuple.Add(miValueResult2);
Index: tools/lldb-mi/MICmdCmdData.cpp
===================================================================
--- tools/lldb-mi/MICmdCmdData.cpp
+++ tools/lldb-mi/MICmdCmdData.cpp
@@ -25,6 +25,8 @@
#include "lldb/API/SBInstruction.h"
#include "lldb/API/SBInstructionList.h"
#include "lldb/API/SBStream.h"
+#include "lldb/Core/RegularExpression.h"
+#include "llvm/ADT/StringRef.h"
// In-house headers:
#include "MICmdCmdData.h"
@@ -43,6 +45,8 @@
#include "MICmnLLDBDebugSessionInfoVarObj.h"
#include "MICmnLLDBUtilSBValue.h"
+using namespace lldb_private;
+
//++ ------------------------------------------------------------------------------------
// Details: CMICmdCmdDataEvaluateExpression constructor.
// Type: Method.
@@ -1686,6 +1690,71 @@
}
//++ ------------------------------------------------------------------------------------
+// Details: Helper function for parsing a line entry returned from lldb for the command:
+// target modules lookup -v <location>
+// where the line entry is of the format:
+// LineEntry: \[0x0000000100000f37-0x0000000100000f45\): /path/file:3[:1]
+// start end file line column(opt)
+// Args: input - (R) Input string to parse.
+// start - (W) String representing the start address.
+// end - (W) String representing the end address.
+// file - (W) String representing the file.
+// line - (W) String representing the line.
+// Return: bool - True = input was parsed successfully, false = input could not be parsed.
+// Throws: None.
+//--
+static bool
+ParseLLDBLineEntry (const char *input, CMIUtilString &start, CMIUtilString &end,
+ CMIUtilString &file, CMIUtilString &line)
+{
+ // Note: Ambiguities arise because the column is optional, and
+ // because : can appear in filenames or as a byte in a multibyte
+ // UTF8 character. We keep those cases to a minimum by using regex
+ // to work on the string from both the left and right, so that what
+ // is remains is assumed to be the filename.
+
+ // Match LineEntry using regex.
+ static RegularExpression g_lineentry_nocol_regex(
+ "^ *LineEntry: \\[(0x[0-9a-fA-F]+)-(0x[0-9a-fA-F]+)\\): (.+):([0-9]+)$");
+ static RegularExpression g_lineentry_col_regex(
+ "^ *LineEntry: \\[(0x[0-9a-fA-F]+)-(0x[0-9a-fA-F]+)\\): (.+):([0-9]+):[0-9]+$");
+ // ^1=start ^2=end ^3=f ^4=line ^5=:col(opt)
+
+ llvm::StringRef startRef;
+ llvm::StringRef endRef;
+ llvm::StringRef fileRef;
+ llvm::StringRef lineRef;
+ RegularExpression::Match match(5);
+
+ // First try matching the LineEntry with the column.
+ if (g_lineentry_col_regex.Execute (input, &match))
+ {
+ match.GetMatchAtIndex(input, 1, startRef);
+ match.GetMatchAtIndex(input, 2, endRef);
+ match.GetMatchAtIndex(input, 3, fileRef);
+ match.GetMatchAtIndex(input, 4, lineRef);
+ }
+ // Then try without the column.
+ else if (g_lineentry_nocol_regex.Execute (input, &match))
+ {
+ match.GetMatchAtIndex(input, 1, startRef);
+ match.GetMatchAtIndex(input, 2, endRef);
+ match.GetMatchAtIndex(input, 3, fileRef);
+ match.GetMatchAtIndex(input, 4, lineRef);
+ }
+ else
+ {
+ // No match.
+ return false;
+ }
+ start = CMIUtilString(startRef.data(), startRef.size());
+ end = CMIUtilString(endRef.data(), endRef.size());
+ file = CMIUtilString(fileRef.data(), fileRef.size());
+ line = CMIUtilString(lineRef.data(), lineRef.size());
+ return true;
+}
+
+//++ ------------------------------------------------------------------------------------
// Details: The invoker requires this function. The command prepares a MI Record Result
// for the work carried out in the Execute().
// Type: Overridden.
@@ -1716,58 +1785,25 @@
// String looks like:
// LineEntry: \[0x0000000100000f37-0x0000000100000f45\): /path/to/file:3[:1]
const CMIUtilString &rLine(vecLines[i]);
+ CMIUtilString strStart;
+ CMIUtilString strEnd;
+ CMIUtilString strFile;
+ CMIUtilString strLine;
- // LineEntry: \[0x0000000100000f37-0x0000000100000f45\): /path/to/file:3[:1]
- // ^^^^^^^^^ -- property
- const size_t nPropertyStartPos = rLine.find_first_not_of(' ');
- const size_t nPropertyEndPos = rLine.find(':');
- const size_t nPropertyLen = nPropertyEndPos - nPropertyStartPos;
- const CMIUtilString strProperty(rLine.substr(nPropertyStartPos, nPropertyLen).c_str());
-
- // Skip all except LineEntry
- if (!CMIUtilString::Compare(strProperty, "LineEntry"))
+ if (!ParseLLDBLineEntry(rLine.c_str(), strStart, strEnd, strFile, strLine))
continue;
- // LineEntry: \[0x0000000100000f37-0x0000000100000f45\): /path/to/file:3[:1]
- // ^^^^^^^^^^^^^^^^^^ -- start address
- const size_t nStartAddressStartPos = rLine.find('[');
- const size_t nStartAddressEndPos = rLine.find('-');
- const size_t nStartAddressLen = nStartAddressEndPos - nStartAddressStartPos - 1;
- const CMIUtilString strStartAddress(rLine.substr(nStartAddressStartPos + 1, nStartAddressLen).c_str());
- const CMICmnMIValueConst miValueConst(strStartAddress);
+ const CMICmnMIValueConst miValueConst(strStart);
const CMICmnMIValueResult miValueResult("start", miValueConst);
- CMICmnMIResultRecord miRecordResult(m_cmdData.strMiCmdToken, CMICmnMIResultRecord::eResultClass_Done, miValueResult);
-
- // LineEntry: \[0x0000000100000f37-0x0000000100000f45\): /path/to/file:3[:1]
- // ^^^^^^^^^^^^^^^^^^ -- end address
- const size_t nEndAddressEndPos = rLine.find(')');
- const size_t nEndAddressLen = nEndAddressEndPos - nStartAddressEndPos - 1;
- const CMIUtilString strEndAddress(rLine.substr(nStartAddressEndPos + 1, nEndAddressLen).c_str());
- const CMICmnMIValueConst miValueConst2(strEndAddress);
+ CMICmnMIResultRecord miRecordResult(m_cmdData.strMiCmdToken,
+ CMICmnMIResultRecord::eResultClass_Done,
+ miValueResult);
+ const CMICmnMIValueConst miValueConst2(strEnd);
const CMICmnMIValueResult miValueResult2("end", miValueConst2);
miRecordResult.Add(miValueResult2);
-
- // LineEntry: \[0x0000000100000f37-0x0000000100000f45\): /path/to/file:3[:1]
- // ^^^^^^^^^^^^^ -- file
- // ^ -- line
- // ^ -- column (optional)
- const size_t nFileStartPos = rLine.find_first_not_of(' ', nEndAddressEndPos + 2);
- const size_t nFileOrLineEndPos = rLine.rfind(':');
- const size_t nFileOrLineStartPos = rLine.rfind(':', nFileOrLineEndPos - 1);
- const size_t nFileEndPos = nFileStartPos < nFileOrLineStartPos ? nFileOrLineStartPos : nFileOrLineEndPos;
- const size_t nFileLen = nFileEndPos - nFileStartPos;
- const CMIUtilString strFile(rLine.substr(nFileStartPos, nFileLen).c_str());
const CMICmnMIValueConst miValueConst3(strFile);
const CMICmnMIValueResult miValueResult3("file", miValueConst3);
miRecordResult.Add(miValueResult3);
-
- // LineEntry: \[0x0000000100000f37-0x0000000100000f45\): /path/to/file:3[:1]
- // ^ -- line
- const size_t nLineStartPos = nFileEndPos + 1;
- const size_t nLineEndPos = rLine.find(':', nLineStartPos);
- const size_t nLineLen = nLineEndPos != std::string::npos ? nLineEndPos - nLineStartPos
- : std::string::npos;
- const CMIUtilString strLine(rLine.substr(nLineStartPos, nLineLen).c_str());
const CMICmnMIValueConst miValueConst4(strLine);
const CMICmnMIValueResult miValueResult4("line", miValueConst4);
miRecordResult.Add(miValueResult4);
Index: test/tools/lldb-mi/symbol/x.h
===================================================================
--- test/tools/lldb-mi/symbol/x.h
+++ test/tools/lldb-mi/symbol/x.h
@@ -0,0 +1,14 @@
+namespace ns {
+ inline int ifunc(int i) {
+ return i;
+ }
+ struct S {
+ int a;
+ int b;
+ S() : a(3), b(4) {}
+ int mfunc() {
+ return a + b;
+ }
+ };
+ extern S s;
+}
Index: test/tools/lldb-mi/symbol/x.cpp
===================================================================
--- test/tools/lldb-mi/symbol/x.cpp
+++ test/tools/lldb-mi/symbol/x.cpp
@@ -0,0 +1,27 @@
+// Skip lines so we can make sure we're not seeing any lines from x.h
+// included in -symbol-list-lines x.cpp, by checking that all the lines
+// are between 20 and 29.
+// line 4
+// line 5
+// line 6
+// line 7
+// line 8
+// line 9
+// line 10
+// line 11
+// line 12
+// line 13
+// line 14
+// line 15
+// line 16
+// line 17
+// line 18
+// line 19
+#include "x.h"
+int j = 2;
+int gfunc(int i) { // FUNC_gfunc
+ return ++i;
+}
+namespace ns {
+ S s; // STRUCT_s
+}
Index: test/tools/lldb-mi/symbol/main.cpp
===================================================================
--- test/tools/lldb-mi/symbol/main.cpp
+++ test/tools/lldb-mi/symbol/main.cpp
@@ -7,8 +7,23 @@
//
//===----------------------------------------------------------------------===//
-int
-main(int argc, char const *argv[])
-{ // FUNC_main
- return 0;
+// Skip lines so we can make sure we're not seeing any lines from x.h
+// included in -symbol-list-lines main.cpp, by checking that all the lines
+// are between 20 and 29.
+// line 13
+// line 14
+// line 15
+// line 16
+// line 17
+// line 18
+// line 19
+#include "x.h"
+extern int j;
+extern int gfunc(int i);
+int i;
+int main() { // FUNC_main
+ i = gfunc(j);
+ i += ns::s.mfunc();
+ i += ns::ifunc(i);
+ return i == 0; // END_main
}
Index: test/tools/lldb-mi/symbol/TestMiSymbol.py
===================================================================
--- test/tools/lldb-mi/symbol/TestMiSymbol.py
+++ test/tools/lldb-mi/symbol/TestMiSymbol.py
@@ -39,6 +39,19 @@
self.runCmd("-symbol-list-lines main.cpp")
self.expect("\^done,lines=\[\{pc=\"0x0*%x\",line=\"%d\"\}(,\{pc=\"0x[0-9a-f]+\",line=\"\d+\"\})+\]" % (addr, line))
+ # Test that -symbol-list-lines doesn't include lines from other sources
+ # by checking the first and last line, and making sure the other lines
+ # are between 20 and 29.
+ sline = line_number('main.cpp', '// FUNC_main')
+ eline = line_number('main.cpp', '// END_main')
+ self.runCmd("-symbol-list-lines main.cpp")
+ self.expect("\^done,lines=\[\{pc=\"0x[0-9a-f]+\",line=\"%d\"\}(,\{pc=\"0x[0-9a-f]+\",line=\"2\d\"\})*,\{pc=\"0x[0-9a-f]+\",line=\"%d\"\}\]" % (sline, eline))
+ ##FIXME: This doesn't work for x.cpp due to clang bug llvm.org/pr24716
+ ##sline = line_number('x.cpp', '// FUNC_gfunc')
+ ##eline = line_number('x.cpp', '// STRUCT_s')
+ ##self.runCmd("-symbol-list-lines x.cpp")
+ ##self.expect("\^done,lines=\[\{pc=\"0x[0-9a-f]+\",line=\"%d\"\}(,\{pc=\"0x[0-9a-f]+\",line=\"2\d\"\})*,\{pc=\"0x[0-9a-f]+\",line=\"%d\"\}\]" % (sline, eline))
+
# Test that -symbol-list-lines fails when file doesn't exist
self.runCmd("-symbol-list-lines unknown_file")
self.expect("\^error,message=\"warning: No source filenames matched 'unknown_file'\. error: no source filenames matched any command arguments \"")
Index: test/tools/lldb-mi/symbol/Makefile
===================================================================
--- test/tools/lldb-mi/symbol/Makefile
+++ test/tools/lldb-mi/symbol/Makefile
@@ -1,5 +1,5 @@
LEVEL = ../../../make
-CXX_SOURCES := main.cpp
+CXX_SOURCES := main.cpp x.cpp
include $(LEVEL)/Makefile.rules
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
