[Lldb-commits] [lldb] 8f89351 - Track which modules have debug info variable errors.

Greg Clayton via lldb-commits lldb-commits at lists.llvm.org
Wed Sep 28 15:40:06 PDT 2022


Author: Greg Clayton
Date: 2022-09-28T15:39:54-07:00
New Revision: 8f8935139adf44584634d4bacd35e3eee3f648a5

URL: https://github.com/llvm/llvm-project/commit/8f8935139adf44584634d4bacd35e3eee3f648a5
DIFF: https://github.com/llvm/llvm-project/commit/8f8935139adf44584634d4bacd35e3eee3f648a5.diff

LOG: 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

Added: 
    

Modified: 
    lldb/include/lldb/Symbol/SymbolFile.h
    lldb/include/lldb/Symbol/SymbolFileOnDemand.h
    lldb/include/lldb/Target/Statistics.h
    lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
    lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
    lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
    lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h
    lldb/source/Target/Statistics.cpp
    lldb/test/API/commands/statistics/basic/TestStats.py

Removed: 
    


################################################################################
diff  --git a/lldb/include/lldb/Symbol/SymbolFile.h b/lldb/include/lldb/Symbol/SymbolFile.h
index 62c22076a772b..74ce1f7f82a04 100644
--- a/lldb/include/lldb/Symbol/SymbolFile.h
+++ b/lldb/include/lldb/Symbol/SymbolFile.h
@@ -251,7 +251,19 @@ class SymbolFile : public PluginInterface {
   /// \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 @@ class SymbolFile : public PluginInterface {
   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 @@ class SymbolFileCommon : public SymbolFile {
   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 @@ class SymbolFileCommon : public SymbolFile {
   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 653a004f77894..05708395687f2 100644
--- a/lldb/include/lldb/Symbol/SymbolFileOnDemand.h
+++ b/lldb/include/lldb/Symbol/SymbolFileOnDemand.h
@@ -216,6 +216,12 @@ class SymbolFileOnDemand : public lldb_private::SymbolFile {
   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 2a8fc5be0e05f..db6494ce7899e 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 68d86f978da03..a9d697480deef 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<std::recursive_mutex> 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 7bab17c783d5f..7a939a0b58de4 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
@@ -164,7 +164,7 @@ class SymbolFileDWARF : public lldb_private::SymbolFileCommon,
                                 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 71f3d3e4a7607..97140704ad33e 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<std::recursive_mutex> 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 56d3d0dba31ee..d743954a9bafb 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h
@@ -103,7 +103,7 @@ class SymbolFileDWARFDebugMap : public lldb_private::SymbolFileCommon {
       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 894552cecc516..0ea09743d1300 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 471872a13869e..92c20162ed3d8 100644
--- a/lldb/test/API/commands/statistics/basic/TestStats.py
+++ b/lldb/test/API/commands/statistics/basic/TestStats.py
@@ -550,3 +550,52 @@ def test_no_dsym_binary_has_symfile_identifiers_in_stats(self):
             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)


        


More information about the lldb-commits mailing list