This revision was automatically updated to reflect the committed changes.
Closed by commit rG8f8935139adf: Track which modules have debug info variable 
errors. (authored by clayborg).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D134508/new/

https://reviews.llvm.org/D134508

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

Index: lldb/test/API/commands/statistics/basic/TestStats.py
===================================================================
--- lldb/test/API/commands/statistics/basic/TestStats.py
+++ lldb/test/API/commands/statistics/basic/TestStats.py
@@ -550,3 +550,52 @@
             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)
Index: lldb/source/Target/Statistics.cpp
===================================================================
--- lldb/source/Target/Statistics.cpp
+++ lldb/source/Target/Statistics.cpp
@@ -63,6 +63,8 @@
   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 @@
         ++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)
Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h
===================================================================
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h
@@ -103,7 +103,7 @@
       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,
Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
===================================================================
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
@@ -1453,7 +1453,7 @@
   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
Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
===================================================================
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
@@ -164,7 +164,7 @@
                                 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,
Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===================================================================
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -4141,7 +4141,7 @@
   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)
Index: lldb/include/lldb/Target/Statistics.h
===================================================================
--- lldb/include/lldb/Target/Statistics.h
+++ lldb/include/lldb/Target/Statistics.h
@@ -118,6 +118,7 @@
   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 {
Index: lldb/include/lldb/Symbol/SymbolFileOnDemand.h
===================================================================
--- lldb/include/lldb/Symbol/SymbolFileOnDemand.h
+++ lldb/include/lldb/Symbol/SymbolFileOnDemand.h
@@ -216,6 +216,12 @@
   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); }
Index: lldb/include/lldb/Symbol/SymbolFile.h
===================================================================
--- lldb/include/lldb/Symbol/SymbolFile.h
+++ lldb/include/lldb/Symbol/SymbolFile.h
@@ -251,7 +251,19 @@
   /// \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 @@
   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 @@
   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 @@
   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;
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to