dawn updated this revision to Diff 43152.
dawn marked an inline comment as done.
dawn added a comment.

Updated patch with suggestions from Ilia's review (thanks Ilia!).


Repository:
  rL LLVM

http://reviews.llvm.org/D15593

Files:
  packages/Python/lldbsuite/test/help/TestHelp.py
  packages/Python/lldbsuite/test/tools/lldb-mi/symbol/TestMiSymbol.py
  
packages/Python/lldbsuite/test/tools/lldb-mi/symbol/symbol_list_lines_inline_test.h
  source/Commands/CommandObjectTarget.cpp
  tools/lldb-mi/MICmdCmdSymbol.cpp

Index: tools/lldb-mi/MICmdCmdSymbol.cpp
===================================================================
--- tools/lldb-mi/MICmdCmdSymbol.cpp
+++ tools/lldb-mi/MICmdCmdSymbol.cpp
@@ -82,11 +82,7 @@
     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()));
+    const CMIUtilString strCmd(CMIUtilString::Format("target modules dump lines --compile-unit-only false --explicit-matches-only true --show-raw false \"%s\"", strFilePath.AddSlashes().c_str()));
 
     CMICmnLLDBDebugSessionInfo &rSessionInfo(CMICmnLLDBDebugSessionInfo::Instance());
     const lldb::ReturnStatus rtn = rSessionInfo.GetDebugger().GetCommandInterpreter().HandleCommand(strCmd.c_str(), m_lldbResult);
@@ -110,10 +106,10 @@
 {
     // Match LineEntry using regex.
     static MIUtilParse::CRegexParser g_lineentry_header_regex( 
-        "^ *Line table for (.+) in `(.+)$");
-        //                 ^1=file  ^2=module
+        "^ *Lines for file (.+) in compilation unit (.+) in `(.+)$");
+        //                 ^1=file                  ^2=cu    ^3=module
 
-    MIUtilParse::CRegexParser::Match match(3);
+    MIUtilParse::CRegexParser::Match match(4);
 
     const bool ok = g_lineentry_header_regex.Execute(input, match);
     if (ok)
@@ -146,12 +142,12 @@
 
     // Match LineEntry using regex.
     static MIUtilParse::CRegexParser g_lineentry_nocol_regex( 
-        "^ *(0x[0-9a-fA-F]+): (.+):([0-9]+)$");
+        "^ *\\[(0x[0-9a-fA-F]+)-(0x[0-9a-fA-F]+)\\): (.+):([0-9]+)$");
     static MIUtilParse::CRegexParser g_lineentry_col_regex( 
-        "^ *(0x[0-9a-fA-F]+): (.+):([0-9]+):[0-9]+$");
-        //  ^1=addr           ^2=f ^3=line ^4=:col(opt)
+        "^ *\\[(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)
 
-    MIUtilParse::CRegexParser::Match match(5);
+    MIUtilParse::CRegexParser::Match match(6);
 
     // First try matching the LineEntry with the column,
     // then try without the column.
@@ -160,8 +156,8 @@
     if (ok)
     {
         addr = match.GetMatchAtIndex(1);
-        file = match.GetMatchAtIndex(2);
-        line = match.GetMatchAtIndex(3);
+        file = match.GetMatchAtIndex(3);
+        line = match.GetMatchAtIndex(4);
     }
     return ok;
 }
@@ -222,10 +218,6 @@
             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);
Index: source/Commands/CommandObjectTarget.cpp
===================================================================
--- source/Commands/CommandObjectTarget.cpp
+++ source/Commands/CommandObjectTarget.cpp
@@ -1485,39 +1485,118 @@
 }
 
 static uint32_t
-DumpCompileUnitLineTable (CommandInterpreter &interpreter,
-                          Stream &strm,
-                          Module *module,
-                          const FileSpec &file_spec,
-                          bool load_addresses)
+DumpFileLines (CommandInterpreter &interpreter,
+               Stream &strm,
+               Module *module,
+               const FileSpec &file_spec,
+               bool load_addresses,
+               bool compile_unit_only,
+               bool explicit_matches_only,
+               bool show_raw)
 {
     uint32_t num_matches = 0;
     if (module)
     {
-        SymbolContextList sc_list;
-        num_matches = module->ResolveSymbolContextsForFileSpec (file_spec,
-                                                                0,
-                                                                false,
-                                                                eSymbolContextCompUnit,
-                                                                sc_list);
+        if (compile_unit_only == true && explicit_matches_only == false && show_raw == true)
+        {
+            // Find the compilation units for this file and and dump their line tables.
+            SymbolContextList sc_list;
+            num_matches = module->ResolveSymbolContextsForFileSpec(file_spec,
+                                                                   0,
+                                                                   false,
+                                                                   eSymbolContextCompUnit,
+                                                                   sc_list);
 
-        for (uint32_t i=0; i<num_matches; ++i)
+            for (uint32_t i = 0; i < num_matches; ++i)
+            {
+                SymbolContext sc;
+                if (sc_list.GetContextAtIndex(i, sc))
+                {
+                    if (i > 0)
+                        strm << "\n\n";
+
+                    strm << "Line table for " << *static_cast<FileSpec*>(sc.comp_unit) << " in `"
+                         << module->GetFileSpec().GetFilename() << "\n";
+                    LineTable *line_table = sc.comp_unit->GetLineTable();
+                    if (line_table)
+                        line_table->GetDescription(&strm, 
+                                                   interpreter.GetExecutionContext().GetTargetPtr(), 
+                                                   lldb::eDescriptionLevelBrief);
+                    else
+                        strm << "No line table";
+                }
+            }
+        }
+        else if (compile_unit_only == false && explicit_matches_only == true && show_raw==false)
         {
-            SymbolContext sc;
-            if (sc_list.GetContextAtIndex(i, sc))
+            // Look through all the compilation units (CUs) for ones that contain
+            // lines of code from this source file.
+            assert(file_spec.GetFilename().AsCString());
+            bool has_path = (file_spec.GetDirectory().AsCString() != 0);
+            int ncus = module->GetNumCompileUnits();
+            for (size_t i = 0; i < ncus; i++)
             {
-                if (i > 0)
-                    strm << "\n\n";
-
-                strm << "Line table for " << *static_cast<FileSpec*> (sc.comp_unit) << " in `"
-                << module->GetFileSpec().GetFilename() << "\n";
-                LineTable *line_table = sc.comp_unit->GetLineTable();
-                if (line_table)
-                    line_table->GetDescription (&strm, 
-                                                interpreter.GetExecutionContext().GetTargetPtr(), 
-                                                lldb::eDescriptionLevelBrief);
-                else
-                    strm << "No line table";
+                // Look for a matching source file in this CU.
+                CompUnitSP cu_sp(module->GetCompileUnitAtIndex(i));
+                if (!cu_sp)
+                    continue;
+                CompileUnit *cu = cu_sp.get();
+                const FileSpecList &cu_file_list = cu->GetSupportFiles();
+                size_t file_idx = cu_file_list.FindFileIndex(0, file_spec, has_path);
+                if (file_idx == UINT32_MAX)
+                    // No such file in this CU.
+                    continue;
+
+                // Update the file to how it appears in the CU.
+                const FileSpec &cu_file_spec = cu_file_list.GetFileSpecAtIndex(file_idx);
+
+                // Dump all the line entries for the file in the CU.
+                const ConstString &file_spec_name = file_spec.GetFilename();
+                const ConstString &module_file_name = module->GetFileSpec().GetFilename();
+                uint32_t line = 0;
+                bool cu_header_printed = false;
+                while (true)
+                {
+                    LineEntry line_entry;
+
+                    // Find the lowest index of a line entry with a line equal to
+                    // or higher than 'line'.
+                    uint32_t start_idx = 0;
+                    start_idx = cu->FindLineEntry(start_idx, line, &cu_file_spec, /*exact=*/false, &line_entry);
+                    if (start_idx == UINT32_MAX)
+                        // No more line entries for our file in this CU.
+                        break;
+
+                    // Loop through to find any other entries for this line, dumping each.
+                    line = line_entry.line;
+                    do
+                    {
+                        assert(lldb_private::FileSpec::Equal(cu_file_spec, line_entry.file, has_path));
+                        if (!cu_header_printed)
+                        {
+                            if (num_matches > 0)
+                                strm << "\n\n";
+                            strm << "Lines for file " << file_spec_name
+                                 << " in compilation unit " << cu->GetFilename()
+                                 << " in `" << module_file_name << "\n";
+                            cu_header_printed = true;
+                        }
+                        line_entry.GetDescription(&strm,
+                                                  lldb::eDescriptionLevelBrief,
+                                                  cu,
+                                                  interpreter.GetExecutionContext().GetTargetPtr(),
+                                                  /*show_address_only=*/false);
+                        strm << "\n";
+                        num_matches++;
+
+                        // Anymore after this one?
+                        start_idx++;
+                        start_idx = cu->FindLineEntry(start_idx, line, &cu_file_spec, /*exact=*/true, &line_entry);
+                    } while (start_idx != UINT32_MAX);
+
+                    // Try the next higher line, starting over at start_idx 0.
+                    line++;
+                }
             }
         }
     }
@@ -2542,26 +2621,119 @@
 };
 
 
-#pragma mark CommandObjectTargetModulesDumpLineTable
+#pragma mark CommandObjectTargetModulesDumpLines
 
 //----------------------------------------------------------------------
-// Image debug line table dumping command
+// Image debug lines dumping command
 //----------------------------------------------------------------------
 
-class CommandObjectTargetModulesDumpLineTable : public CommandObjectTargetModulesSourceFileAutoComplete
+class CommandObjectTargetModulesDumpLines : public CommandObjectTargetModulesSourceFileAutoComplete
 {
 public:
-    CommandObjectTargetModulesDumpLineTable (CommandInterpreter &interpreter) :
-    CommandObjectTargetModulesSourceFileAutoComplete (interpreter,
-                                                      "target modules dump line-table",
-                                                      "Dump the line table for one or more compilation units.",
-                                                      NULL,
-                                                      eCommandRequiresTarget)
+
+    class CommandOptions : public Options
+    {
+    public:
+
+        CommandOptions (CommandInterpreter &interpreter) :
+            Options(interpreter),
+            m_compile_unit_only(true),
+            m_explicit_matches_only(false),
+            m_show_raw(true)
+        {
+        }
+
+        ~CommandOptions () override
+        {
+        }
+
+        Error
+        SetOptionValue (uint32_t option_idx, const char *option_arg) override
+        {
+            Error error;
+
+            const int short_option = m_getopt_table[option_idx].val;
+
+            switch (short_option)
+            {
+                case 'u':
+                {
+                    bool success;
+                    m_compile_unit_only = Args::StringToBoolean(option_arg, true, &success);
+                    if (!success)
+                        error.SetErrorStringWithFormat("Invalid boolean value for compile-unit-only option: '%s'", option_arg);
+                    break;
+                }
+
+                case 'e':
+                {
+                    bool success;
+                    m_explicit_matches_only = Args::StringToBoolean(option_arg, true, &success);
+                    if (!success)
+                        error.SetErrorStringWithFormat("Invalid boolean value for explicit-matches-only option: '%s'", option_arg);
+                    break;
+                }
+
+                case 'r':
+                {
+                    bool success;
+                    m_show_raw = Args::StringToBoolean(option_arg, true, &success);
+                    if (!success)
+                        error.SetErrorStringWithFormat("Invalid boolean value for show-raw option: '%s'", option_arg);
+                    break;
+                }
+
+                default:
+                    error.SetErrorStringWithFormat("unrecognized option '%c'.", short_option);
+                    break;
+            }
+
+            return error;
+        }
+
+        void
+        OptionParsingStarting () override
+        {
+            m_compile_unit_only = true;
+            m_explicit_matches_only = false;
+            m_show_raw = true;
+        }
+
+        const OptionDefinition*
+        GetDefinitions () override
+        {
+            return g_option_table;
+        }
+
+        // Options table: Required for subclasses of Options.
+
+        static OptionDefinition g_option_table[];
+
+        // Instance variables to hold the values for command options.
+
+        bool m_compile_unit_only;
+        bool m_explicit_matches_only;
+        bool m_show_raw;
+    };
+
+    CommandObjectTargetModulesDumpLines (CommandInterpreter &interpreter) :
+        CommandObjectTargetModulesSourceFileAutoComplete(interpreter,
+                                                         "target modules dump lines",
+                                                         "Dump the lines for one or more files.",
+                                                         NULL,
+                                                         eCommandRequiresTarget),
+        m_options(interpreter)
+    {
+    }
+
+    ~CommandObjectTargetModulesDumpLines () override
     {
     }
 
-    ~CommandObjectTargetModulesDumpLineTable () override
+    Options *
+    GetOptions () override
     {
+        return &m_options;
     }
 
 protected:
@@ -2577,8 +2749,23 @@
 
         if (command.GetArgumentCount() == 0)
         {
-            result.AppendErrorWithFormat ("\nSyntax: %s\n", m_cmd_syntax.c_str());
-            result.SetStatus (eReturnStatusFailed);
+            result.AppendError("file option must be specified.");
+            result.SetStatus(eReturnStatusFailed);
+            return result.Succeeded();
+        }
+        else if (!(m_options.m_compile_unit_only == true &&
+                   m_options.m_explicit_matches_only == false &&
+                   m_options.m_show_raw == true) &&
+                 !(m_options.m_compile_unit_only == false &&
+                   m_options.m_explicit_matches_only == true &&
+                   m_options.m_show_raw==false))
+        {
+            result.AppendError ("the only two combinations of flags currently supported are:\n"
+                                "    --compile-unit-only true --explicit-matches-only false --show-raw true\n"
+                                "and\n"
+                                "    --compile-unit-only false --explicit-matches-only true --show-raw false");
+            result.SetStatus(eReturnStatusFailed);
+            return result.Succeeded();
         }
         else
         {
@@ -2594,17 +2781,20 @@
                 if (num_modules > 0)
                 {
                     uint32_t num_dumped = 0;
-                    for (uint32_t i = 0; i<num_modules; ++i)
+                    for (uint32_t i = 0; i < num_modules; ++i)
                     {
-                        if (DumpCompileUnitLineTable (m_interpreter,
-                                                      result.GetOutputStream(),
-                                                      target_modules.GetModulePointerAtIndexUnlocked(i),
-                                                      file_spec,
-                                                      m_exe_ctx.GetProcessPtr() && m_exe_ctx.GetProcessRef().IsAlive()))
+                        if (DumpFileLines(m_interpreter,
+                                          result.GetOutputStream(),
+                                          target_modules.GetModulePointerAtIndexUnlocked(i),
+                                          file_spec,
+                                          m_exe_ctx.GetProcessPtr() && m_exe_ctx.GetProcessRef().IsAlive(),
+                                          m_options.m_compile_unit_only,
+                                          m_options.m_explicit_matches_only,
+                                          m_options.m_show_raw))
                             num_dumped++;
                     }
                     if (num_dumped == 0)
-                        result.AppendWarningWithFormat ("No source filenames matched '%s'.\n", arg_cstr);
+                        result.AppendWarningWithFormat("No source filenames matched '%s'.\n", arg_cstr);
                     else
                         total_num_dumped += num_dumped;
                 }
@@ -2612,14 +2802,28 @@
         }
 
         if (total_num_dumped > 0)
-            result.SetStatus (eReturnStatusSuccessFinishResult);
+            result.SetStatus(eReturnStatusSuccessFinishResult);
         else
         {
-            result.AppendError ("no source filenames matched any command arguments");
-            result.SetStatus (eReturnStatusFailed);
+            result.AppendError("no source filenames matched any command arguments");
+            result.SetStatus(eReturnStatusFailed);
         }
         return result.Succeeded();
     }
+
+    CommandOptions m_options;
+};
+
+OptionDefinition
+CommandObjectTargetModulesDumpLines::CommandOptions::g_option_table[] =
+{
+    { LLDB_OPT_SET_ALL, false, "compile-unit-only",     'u', OptionParser::eRequiredArgument, NULL, NULL, 0, eArgTypeBoolean,
+        "If set, don't search inside the line tables looking for matches (requires that the the source file be a compile unit) (default: true)."},
+    { LLDB_OPT_SET_ALL, false, "explicit-matches-only", 'e', OptionParser::eRequiredArgument, NULL, NULL, 0, eArgTypeBoolean,
+        "If set, dump only the lines which match the file, otherwise dump all lines in the file's compilation unit(s) (default: false)."},
+    { LLDB_OPT_SET_ALL, false, "show-raw",              'r', OptionParser::eRequiredArgument, NULL, NULL, 0, eArgTypeBoolean,
+        "If set, dump the raw line entries from the debug info, otherwise dump the lines as stored in lldb (default: true)."},
+    { 0,                false, NULL,           0, 0,                 NULL, NULL, 0, eArgTypeNone, NULL }
 };
 
 
@@ -2639,12 +2843,12 @@
     CommandObjectMultiword (interpreter, 
                             "target modules dump",
                             "A set of commands for dumping information about one or more target modules.",
-                            "target modules dump [symtab|sections|symfile|line-table] [<file1> <file2> ...]")
+                            "target modules dump [symtab|sections|symfile|lines] [<file1> <file2> ...]")
     {
-        LoadSubCommand ("symtab",      CommandObjectSP (new CommandObjectTargetModulesDumpSymtab (interpreter)));
-        LoadSubCommand ("sections",    CommandObjectSP (new CommandObjectTargetModulesDumpSections (interpreter)));
-        LoadSubCommand ("symfile",     CommandObjectSP (new CommandObjectTargetModulesDumpSymfile (interpreter)));
-        LoadSubCommand ("line-table",  CommandObjectSP (new CommandObjectTargetModulesDumpLineTable (interpreter)));
+        LoadSubCommand ("symtab",   CommandObjectSP (new CommandObjectTargetModulesDumpSymtab (interpreter)));
+        LoadSubCommand ("sections", CommandObjectSP (new CommandObjectTargetModulesDumpSections (interpreter)));
+        LoadSubCommand ("symfile",  CommandObjectSP (new CommandObjectTargetModulesDumpSymfile (interpreter)));
+        LoadSubCommand ("lines",    CommandObjectSP (new CommandObjectTargetModulesDumpLines (interpreter)));
     }
 
     ~CommandObjectTargetModulesDump() override
Index: packages/Python/lldbsuite/test/tools/lldb-mi/symbol/symbol_list_lines_inline_test.h
===================================================================
--- packages/Python/lldbsuite/test/tools/lldb-mi/symbol/symbol_list_lines_inline_test.h
+++ packages/Python/lldbsuite/test/tools/lldb-mi/symbol/symbol_list_lines_inline_test.h
@@ -2,7 +2,7 @@
 {
 inline int
 ifunc(int i)
-{
+{ // FUNC_ifunc
     return i;
 }
 struct S
@@ -16,7 +16,7 @@
     }
     int
     mfunc()
-    {
+    { // FUNC_mfunc
         return a + b;
     }
 };
Index: packages/Python/lldbsuite/test/tools/lldb-mi/symbol/TestMiSymbol.py
===================================================================
--- packages/Python/lldbsuite/test/tools/lldb-mi/symbol/TestMiSymbol.py
+++ packages/Python/lldbsuite/test/tools/lldb-mi/symbol/TestMiSymbol.py
@@ -48,12 +48,19 @@
         eline = line_number('symbol_list_lines_inline_test2.cpp', '// END_gfunc2')
         self.runCmd("-symbol-list-lines symbol_list_lines_inline_test2.cpp")
         self.expect("\^done,lines=\[\{pc=\"0x[0-9a-f]+\",line=\"%d\"\}(,\{pc=\"0x[0-9a-f]+\",line=\"3\d\"\})*,\{pc=\"0x[0-9a-f]+\",line=\"%d\"\}(,\{pc=\"0x[0-9a-f]+\",line=\"3\d\"\})*\]" % (sline, eline))
-        ##FIXME: This doesn't work for symbol_list_lines_inline_test.cpp due to clang bug llvm.org/pr24716
+        ##FIXME: This doesn't work for symbol_list_lines_inline_test.cpp due to clang bug llvm.org/pr24716 (fixed in newer versions of clang)
         ##sline = line_number('symbol_list_lines_inline_test.cpp', '// FUNC_gfunc')
         ##eline = line_number('symbol_list_lines_inline_test.cpp', '// STRUCT_s')
         ##self.runCmd("-symbol-list-lines symbol_list_lines_inline_test.cpp")
         ##self.expect("\^done,lines=\[\{pc=\"0x[0-9a-f]+\",line=\"%d\"\}(,\{pc=\"0x[0-9a-f]+\",line=\"3\d\"\})*,\{pc=\"0x[0-9a-f]+\",line=\"%d\"\}\]" % (sline, eline))
 
+        # Test that -symbol-list-lines works on header files by checking the first
+        # and last line, and making sure the other lines are under 29.
+        sline = line_number('symbol_list_lines_inline_test.h', '// FUNC_ifunc')
+        eline = line_number('symbol_list_lines_inline_test.h', '// FUNC_mfunc')
+        self.runCmd("-symbol-list-lines symbol_list_lines_inline_test.h")
+        self.expect("\^done,lines=\[\{pc=\"0x[0-9a-f]+\",line=\"%d\"\}(,\{pc=\"0x[0-9a-f]+\",line=\"\d\"\})*(,\{pc=\"0x[0-9a-f]+\",line=\"1\d\"\})*,\{pc=\"0x[0-9a-f]+\",line=\"%d\"\}(,\{pc=\"0x[0-9a-f]+\",line=\"2\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: packages/Python/lldbsuite/test/help/TestHelp.py
===================================================================
--- packages/Python/lldbsuite/test/help/TestHelp.py
+++ packages/Python/lldbsuite/test/help/TestHelp.py
@@ -137,7 +137,7 @@
         """Command 'help image du line' is not ambiguous and should work."""
         # 'image' is an alias for 'target modules'.
         self.expect("help image du line",
-            substrs = ['Dump the line table for one or more compilation units'])
+            substrs = ['Dump the lines for one or more files'])
 
     @no_debug_info_test
     def test_help_target_variable_syntax(self):
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to