[lldb/Test] Assert that no targets or global modules remain after a test completes.

[JDevlieghere]

Summary

The reproducer intentionally leak every object allocated during replay, which means that modules never get orphaned. If this were to happen for another reason, we might not be testing what we think we are. Assert that there are no targets left at the end of a test and that the global module cache is empty in the non-reproducer scenario.

[vsk]

lldb/bindings/interface/SBModule.i

347

Can we add a %feature("docstring", ...) blurb about this, advising script authors that it's probably not an API they're interested in?

[JDevlieghere]

Address @vsk's feedback.

[labath]

I'm pretty indifferent about this functionality -- I don't think it hurts, but it also doesn't seem like a pressing problem that needs addressing.

Regarding the implementation, be aware that assertion failures during test teardown are reproted pretty weirdly -- IIRC at this point the test has already been declared "successful", and these failures manifest as "CLEANUP ERROR"s somewhere. And I have a feeling they don't actually fail the check-lldb command. I don't know if this is due to something we've done, or if it's just how the python unittest framework works...

[JDevlieghere]

In D81612#2086939, @labath wrote:

I'm pretty indifferent about this functionality -- I don't think it hurts, but it also doesn't seem like a pressing problem that needs addressing.

Yeah. I agree it's not a problem we face right now, but the reproducer scenario showed that we can get into a situation where we're not testing what we thing we are. I took me and Jim a little time to understand why that was happening and I think this is a small price to pay to avoid that in the future and guarantee some sanity.

Regarding the implementation, be aware that assertion failures during test teardown are reproted pretty weirdly -- IIRC at this point the test has already been declared "successful", and these failures manifest as "CLEANUP ERROR"s somewhere. And I have a feeling they don't actually fail the check-lldb command. I don't know if this is due to something we've done, or if it's just how the python unittest framework works...

Okay, I was on the fence between using the unittest2 assert method or the built in Python assertion. The latter might be a better fit.

[vsk]

Lgtm.

lldb/bindings/interface/SBModule.i

public:
     GetSymbolFileSpec() const;
 
     lldb::SBAddress
     GetObjectFileHeaderAddress() const;
 
     lldb::SBAddress
     GetObjectFileEntryPointAddress() const;
 
+    %feature("docstring", "
+    Returns the number of modules in the module cache. This is an
+    implementation detail exposed for testing and should not be relied upon.
+
+    @return
+        The number of modules in the module cache.") GetNumberAllocatedModules;
+    static uint32_t
+    GetNumberAllocatedModules();
+
     STRING_EXTENSION(SBModule)
 
 #ifdef SWIGPYTHON
     %pythoncode %{
         def __len__(self):
             '''Return the number of symbols in a lldb.SBModule object.'''
             return self.GetNumSymbols()
 

lldb/include/lldb/API/SBModule.h

public:
   ///
   /// \return
   ///     A const reference to the file specification object.
   lldb::SBFileSpec GetSymbolFileSpec() const;
 
   lldb::SBAddress GetObjectFileHeaderAddress() const;
   lldb::SBAddress GetObjectFileEntryPointAddress() const;
 
+  /// Get the number of global modules.
+  static uint32_t GetNumberAllocatedModules();
+
 private:
   friend class SBAddress;
   friend class SBFrame;
   friend class SBSection;
   friend class SBSymbolContext;
   friend class SBTarget;
 
   explicit SBModule(const lldb::ModuleSP &module_sp);

lldb/packages/Python/lldbsuite/test/lldbtest.py

def tearDown(self):
         # the initialization overhead.
         targets = []
         for target in self.dbg:
             if target:
                 targets.append(target)
                 process = target.GetProcess()
                 if process:
                     rc = self.invoke(process, "Kill")
-                    self.assertTrue(rc.Success(), PROCESS_KILLED)
+                    assert rc.Success()
         for target in targets:
             self.dbg.DeleteTarget(target)
 
+        # Modules are not orphaned during reproducer replay because they're
+        # leaked on purpose.
+        if not configuration.is_reproducer():
+            # Assert that all targets are deleted.
+            assert self.dbg.GetNumTargets() == 0
+            # Assert that the global module cache is empty.
+            assert lldb.SBModule.GetNumberAllocatedModules() == 0
+
         # Do this last, to make sure it's in reverse order from how we setup.
         Base.tearDown(self)
 
     def switch_to_thread_with_stop_reason(self, stop_reason):
         """
         Run the 'thread list' command, and select the thread with stop reason as
         'stop_reason'.  If no such thread exists, no select action is done.
         """

lldb/source/API/SBModule.cpp

lldb::SBAddress SBModule::GetObjectFileEntryPointAddress() const {
   if (module_sp) {
     ObjectFile *objfile_ptr = module_sp->GetObjectFile();
     if (objfile_ptr)
       sb_addr.ref() = objfile_ptr->GetEntryPointAddress();
   }
   return LLDB_RECORD_RESULT(sb_addr);
 }
 
+uint32_t SBModule::GetNumberAllocatedModules() {
+  LLDB_RECORD_STATIC_METHOD_NO_ARGS(uint32_t, SBModule,
+                                    GetNumberAllocatedModules);
+
+  return Module::GetNumberAllocatedModules();
+}
+
 namespace lldb_private {
 namespace repro {
 
 template <>
 void RegisterMethods<SBModule>(Registry &R) {
   LLDB_REGISTER_CONSTRUCTOR(SBModule, ());
   LLDB_REGISTER_CONSTRUCTOR(SBModule, (const lldb::SBModuleSpec &));
   LLDB_REGISTER_CONSTRUCTOR(SBModule, (const lldb::SBModule &));
void RegisterMethods<SBModule>(Registry &R) {
   LLDB_REGISTER_METHOD(uint32_t, SBModule, GetVersion,
                        (uint32_t *, uint32_t));
   LLDB_REGISTER_METHOD_CONST(lldb::SBFileSpec, SBModule, GetSymbolFileSpec,
                              ());
   LLDB_REGISTER_METHOD_CONST(lldb::SBAddress, SBModule,
                              GetObjectFileHeaderAddress, ());
   LLDB_REGISTER_METHOD_CONST(lldb::SBAddress, SBModule,
                              GetObjectFileEntryPointAddress, ());
+  LLDB_REGISTER_STATIC_METHOD(uint32_t, SBModule, GetNumberAllocatedModules,
+                              ());
 }
 
 }
 }