From 8f8935139adf44584634d4bacd35e3eee3f648a5 Mon Sep 17 00:00:00 2001 From: Greg Clayton Date: Thu, 22 Sep 2022 17:54:06 -0700 Subject: [PATCH] Track which modules have debug info variable errors. Now that we display an error when users try to get variables, but something in the debug info is preventing variables from showing up, track this with a new bool in each module's statistic information named "debugInfoHadVariableErrors". This patch modifies the code to track when we have variable errors in a module and adds accessors to get/set this value. This value is used in the module statistics and we added a test to verify this value gets set correctly. Differential Revision: https://reviews.llvm.org/D134508 --- lldb/include/lldb/Symbol/SymbolFile.h | 30 ++++++++++++- lldb/include/lldb/Symbol/SymbolFileOnDemand.h | 6 +++ lldb/include/lldb/Target/Statistics.h | 1 + .../Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp | 2 +- .../Plugins/SymbolFile/DWARF/SymbolFileDWARF.h | 2 +- .../SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp | 2 +- .../SymbolFile/DWARF/SymbolFileDWARFDebugMap.h | 2 +- lldb/source/Target/Statistics.cpp | 4 ++ .../API/commands/statistics/basic/TestStats.py | 49 ++++++++++++++++++++++ 9 files changed, 93 insertions(+), 5 deletions(-) diff --git a/lldb/include/lldb/Symbol/SymbolFile.h b/lldb/include/lldb/Symbol/SymbolFile.h index 62c2207..74ce1f7 100644 --- a/lldb/include/lldb/Symbol/SymbolFile.h +++ b/lldb/include/lldb/Symbol/SymbolFile.h @@ -251,7 +251,19 @@ public: /// \returns /// An error specifying why there should have been debug info with variable /// information but the variables were not able to be resolved. - virtual Status GetFrameVariableError(StackFrame &frame) { + Status GetFrameVariableError(StackFrame &frame) { + Status err = CalculateFrameVariableError(frame); + if (err.Fail()) + SetDebugInfoHadFrameVariableErrors(); + return err; + } + + /// Subclasses will override this function to for GetFrameVariableError(). + /// + /// This allows GetFrameVariableError() to set the member variable + /// m_debug_info_had_variable_errors correctly without users having to do it + /// manually which is error prone. + virtual Status CalculateFrameVariableError(StackFrame &frame) { return Status(); } virtual uint32_t @@ -393,6 +405,12 @@ public: virtual void SetDebugInfoIndexWasSavedToCache() = 0; /// \} + /// Accessors for the bool that indicates if there was debug info, but errors + /// stopped variables from being able to be displayed correctly. See + /// GetFrameVariableError() for details on what are considered errors. + virtual bool GetDebugInfoHadFrameVariableErrors() const = 0; + virtual void SetDebugInfoHadFrameVariableErrors() = 0; + protected: void AssertModuleLock(); @@ -466,6 +484,12 @@ public: void SetDebugInfoIndexWasSavedToCache() override { m_index_was_saved_to_cache = true; } + bool GetDebugInfoHadFrameVariableErrors() const override { + return m_debug_info_had_variable_errors; + } + void SetDebugInfoHadFrameVariableErrors() override { + m_debug_info_had_variable_errors = true; + } protected: virtual uint32_t CalculateNumCompileUnits() = 0; @@ -484,6 +508,10 @@ protected: bool m_calculated_abilities = false; bool m_index_was_loaded_from_cache = false; bool m_index_was_saved_to_cache = false; + /// Set to true if any variable feteching errors have been found when calling + /// GetFrameVariableError(). This will be emitted in the "statistics dump" + /// information for a module. + bool m_debug_info_had_variable_errors = false; private: SymbolFileCommon(const SymbolFileCommon &) = delete; diff --git a/lldb/include/lldb/Symbol/SymbolFileOnDemand.h b/lldb/include/lldb/Symbol/SymbolFileOnDemand.h index 653a004..0570839 100644 --- a/lldb/include/lldb/Symbol/SymbolFileOnDemand.h +++ b/lldb/include/lldb/Symbol/SymbolFileOnDemand.h @@ -216,6 +216,12 @@ public: void SetDebugInfoIndexWasSavedToCache() override { m_sym_file_impl->SetDebugInfoIndexWasSavedToCache(); } + bool GetDebugInfoHadFrameVariableErrors() const override { + return m_sym_file_impl->GetDebugInfoHadFrameVariableErrors(); + } + void SetDebugInfoHadFrameVariableErrors() override { + return m_sym_file_impl->SetDebugInfoHadFrameVariableErrors(); + } private: Log *GetLog() const { return ::lldb_private::GetLog(LLDBLog::OnDemand); } diff --git a/lldb/include/lldb/Target/Statistics.h b/lldb/include/lldb/Target/Statistics.h index 2a8fc5b..db6494c 100644 --- a/lldb/include/lldb/Target/Statistics.h +++ b/lldb/include/lldb/Target/Statistics.h @@ -118,6 +118,7 @@ struct ModuleStats { bool debug_info_index_saved_to_cache = false; bool debug_info_enabled = true; bool symtab_stripped = false; + bool debug_info_had_variable_errors = false; }; struct ConstStringStats { diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp index 68d86f9..a9d6974 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp +++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp @@ -4141,7 +4141,7 @@ StatsDuration::Duration SymbolFileDWARF::GetDebugInfoIndexTime() { return {}; } -Status SymbolFileDWARF::GetFrameVariableError(StackFrame &frame) { +Status SymbolFileDWARF::CalculateFrameVariableError(StackFrame &frame) { std::lock_guard guard(GetModuleMutex()); CompileUnit *cu = frame.GetSymbolContext(eSymbolContextCompUnit).comp_unit; if (!cu) diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h index 7bab17c..7a939a0 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h +++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h @@ -164,7 +164,7 @@ public: lldb_private::SymbolContext &sc) override; lldb_private::Status - GetFrameVariableError(lldb_private::StackFrame &frame) override; + CalculateFrameVariableError(lldb_private::StackFrame &frame) override; uint32_t ResolveSymbolContext( const lldb_private::SourceLocationSpec &src_location_spec, diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp index 71f3d3e..9714070 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp +++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp @@ -1453,7 +1453,7 @@ ModuleList SymbolFileDWARFDebugMap::GetDebugInfoModules() { return oso_modules; } -Status SymbolFileDWARFDebugMap::GetFrameVariableError(StackFrame &frame) { +Status SymbolFileDWARFDebugMap::CalculateFrameVariableError(StackFrame &frame) { std::lock_guard guard(GetModuleMutex()); // We need to make sure that our PC value from the frame matches the module diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h index 56d3d0d..d743954 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h +++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h @@ -103,7 +103,7 @@ public: lldb_private::SymbolContextList &sc_list) override; lldb_private::Status - GetFrameVariableError(lldb_private::StackFrame &frame) override; + CalculateFrameVariableError(lldb_private::StackFrame &frame) override; void FindGlobalVariables(lldb_private::ConstString name, diff --git a/lldb/source/Target/Statistics.cpp b/lldb/source/Target/Statistics.cpp index 894552c..0ea09743 100644 --- a/lldb/source/Target/Statistics.cpp +++ b/lldb/source/Target/Statistics.cpp @@ -63,6 +63,8 @@ json::Value ModuleStats::ToJSON() const { module.try_emplace("debugInfoIndexSavedToCache", debug_info_index_saved_to_cache); module.try_emplace("debugInfoEnabled", debug_info_enabled); + module.try_emplace("debugInfoHadVariableErrors", + debug_info_had_variable_errors); module.try_emplace("symbolTableStripped", symtab_stripped); if (!symfile_path.empty()) module.try_emplace("symbolFilePath", symfile_path); @@ -242,6 +244,8 @@ llvm::json::Value DebuggerStats::ReportStatistics(Debugger &debugger, ++num_stripped_modules; module_stat.debug_info_enabled = sym_file->GetLoadDebugInfoEnabled() && module_stat.debug_info_size > 0; + module_stat.debug_info_had_variable_errors = + sym_file->GetDebugInfoHadFrameVariableErrors(); if (module_stat.debug_info_enabled) ++num_debug_info_enabled_modules; if (module_stat.debug_info_size > 0) diff --git a/lldb/test/API/commands/statistics/basic/TestStats.py b/lldb/test/API/commands/statistics/basic/TestStats.py index 471872a..92c2016 100644 --- a/lldb/test/API/commands/statistics/basic/TestStats.py +++ b/lldb/test/API/commands/statistics/basic/TestStats.py @@ -550,3 +550,52 @@ class TestCase(TestBase): self.assertNotEqual(o_module, None) # Make sure each .o file has some debug info bytes. self.assertGreater(o_module['debugInfoByteSize'], 0) + + @skipUnlessDarwin + @no_debug_info_test + def test_had_frame_variable_errors(self): + """ + Test that if we have frame variable errors that we see this in the + statistics for the module that had issues. + """ + self.build(debug_info="dwarf") + exe_name = 'a.out' + exe = self.getBuildArtifact(exe_name) + dsym = self.getBuildArtifact(exe_name + ".dSYM") + main_obj = self.getBuildArtifact('main.o') + print("carp: dsym = '%s'" % (dsym)) + # Make sure the executable file exists after building. + self.assertEqual(os.path.exists(exe), True) + # Make sure the dSYM file doesn't exist after building. + self.assertEqual(os.path.isdir(dsym), False) + # Make sure the main.o object file exists after building. + self.assertEqual(os.path.exists(main_obj), True) + + # Delete the main.o file that contains the debug info so we force an + # error when we run to main and try to get variables + os.unlink(main_obj) + + (target, process, thread, bkpt) = lldbutil.run_to_name_breakpoint(self, 'main') + + # Get stats and verify we had errors. + exe_stats = self.find_module_in_metrics(exe, self.get_stats()) + self.assertTrue(exe_stats is not None) + + # Make sure we have "debugInfoHadVariableErrors" variable that is set to + # false before failing to get local variables due to missing .o file. + self.assertEqual(exe_stats['debugInfoHadVariableErrors'], False) + + # Try and fail to get variables + vars = thread.GetFrameAtIndex(0).GetVariables(True, True, False, True) + + # Make sure we got an error back that indicates that variables were not + # available + self.assertTrue(vars.GetError().Fail()) + + # Get stats and verify we had errors. + exe_stats = self.find_module_in_metrics(exe, self.get_stats()) + self.assertTrue(exe_stats is not None) + + # Make sure we have "hadFrameVariableErrors" variable that is set to + # true after failing to get local variables due to missing .o file. + self.assertEqual(exe_stats['debugInfoHadVariableErrors'], True) -- 2.7.4