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

@@ -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;

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); }

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 {

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)

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,

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

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,

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)

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)