Merge pull request swiftlang#33888 from eeckstein/fix-leaks

SIL: fix some memory leaks and add verification for leaked instructions

Author: eeckstein

include/swift/SIL/SILBasicBlock.h

@@ -76,6 +76,9 @@ public llvm::ilist_node<SILBasicBlock>, public SILAllocated<SILBasicBlock> {
   /// This method unlinks 'self' from the containing SILFunction and deletes it.
   void eraseFromParent();
 
+  /// Remove all instructions of a SILGlobalVariable's static initializer block.
+  void clearStaticInitializerBlock(SILModule &module);
+
   //===--------------------------------------------------------------------===//
   // SILInstruction List Inspection and Manipulation
   //===--------------------------------------------------------------------===//

include/swift/SIL/SILFunction.h

@@ -141,11 +141,11 @@ class SILFunction
   CanSILFunctionType LoweredType;
 
   /// The context archetypes of the function.
-  GenericEnvironment *GenericEnv;
+  GenericEnvironment *GenericEnv = nullptr;
 
   /// The information about specialization.
   /// Only set if this function is a specialization of another function.
-  const GenericSpecializationInformation *SpecializationInfo;
+  const GenericSpecializationInformation *SpecializationInfo = nullptr;
 
   /// The forwarding substitution map, lazily computed.
   SubstitutionMap ForwardingSubMap;
@@ -158,10 +158,10 @@ class SILFunction
   ValueDecl *ClangNodeOwner = nullptr;
 
   /// The source location and scope of the function.
-  const SILDebugScope *DebugScope;
+  const SILDebugScope *DebugScope = nullptr;
 
   /// The AST decl context of the function.
-  DeclContext *DeclCtxt;
+  DeclContext *DeclCtxt = nullptr;
 
   /// The profiler for instrumentation based profiling, or null if profiling is
   /// disabled.
@@ -315,8 +315,7 @@ class SILFunction
               IsTransparent_t isTrans, IsSerialized_t isSerialized,
               ProfileCounter entryCount, IsThunk_t isThunk,
               SubclassScope classSubclassScope, Inline_t inlineStrategy,
-              EffectsKind E, SILFunction *insertBefore,
-              const SILDebugScope *debugScope,
+              EffectsKind E, const SILDebugScope *debugScope,
               IsDynamicallyReplaceable_t isDynamic,
               IsExactSelfClass_t isExactSelfClass);
 
@@ -334,6 +333,18 @@ class SILFunction
          SILFunction *InsertBefore = nullptr,
          const SILDebugScope *DebugScope = nullptr);
 
+  void init(SILLinkage Linkage, StringRef Name,
+                         CanSILFunctionType LoweredType,
+                         GenericEnvironment *genericEnv,
+                         Optional<SILLocation> Loc, IsBare_t isBareSILFunction,
+                         IsTransparent_t isTrans, IsSerialized_t isSerialized,
+                         ProfileCounter entryCount, IsThunk_t isThunk,
+                         SubclassScope classSubclassScope,
+                         Inline_t inlineStrategy, EffectsKind E,
+                         const SILDebugScope *DebugScope,
+                         IsDynamicallyReplaceable_t isDynamic,
+                         IsExactSelfClass_t isExactSelfClass);
+
   /// Set has ownership to the given value. True means that the function has
   /// ownership, false means it does not.
   ///

include/swift/SIL/SILGlobalVariable.h

@@ -233,7 +233,7 @@ public ilist_node_traits<::swift::SILGlobalVariable> {
   using SILGlobalVariable = ::swift::SILGlobalVariable;
 
 public:
-  static void deleteNode(SILGlobalVariable *V) {}
+  static void deleteNode(SILGlobalVariable *V) { V->~SILGlobalVariable(); }
 
 private:
   void createNode(const SILGlobalVariable &);

include/swift/SIL/SILModule.h

@@ -171,9 +171,6 @@ class SILModule {
   /// but kept alive for debug info generation.
   FunctionListType zombieFunctions;
 
-  /// Stores the names of zombie functions.
-  llvm::BumpPtrAllocator zombieFunctionNames;
-
   /// Lookup table for SIL vtables from class decls.
   llvm::DenseMap<const ClassDecl *, SILVTable *> VTableMap;
 
@@ -340,7 +337,7 @@ class SILModule {
   /// Specialization can cause a function that was erased before by dead function
   /// elimination to become alive again. If this happens we need to remove it
   /// from the list of zombies.
-  void removeFromZombieList(StringRef Name);
+  SILFunction *removeFromZombieList(StringRef Name);
 
   /// Erase a global SIL variable from the module.
   void eraseGlobalVariable(SILGlobalVariable *G);
@@ -653,6 +650,19 @@ class SILModule {
   /// invariants.
   void verify() const;
 
+  /// Check if there are any leaking instructions.
+  ///
+  /// Aborts with an error if more instructions are allocated than contained in
+  /// the module.
+  void checkForLeaks() const;
+
+  /// Check if there are any leaking instructions after the SILModule is
+  /// destructed.
+  ///
+  /// The SILModule destructor already calls checkForLeaks(). This function is
+  /// useful to check if the destructor itself destroys all data structures.
+  static void checkForLeaksAfterDestruction();
+
   /// Pretty-print the module.
   void dump(bool Verbose = false) const;
 

lib/IRGen/IRGen.cpp

@@ -1009,7 +1009,10 @@ GeneratedModule IRGenRequest::evaluate(Evaluator &evaluator,
 
   // Free the memory occupied by the SILModule.
   // Execute this task in parallel to the embedding of bitcode.
-  auto SILModuleRelease = [&SILMod]() { SILMod.reset(nullptr); };
+  auto SILModuleRelease = [&SILMod]() {
+    SILMod.reset(nullptr);
+    SILModule::checkForLeaksAfterDestruction();
+  };
   auto Thread = std::thread(SILModuleRelease);
   // Wait for the thread to terminate.
   SWIFT_DEFER { Thread.join(); };
@@ -1307,7 +1310,10 @@ static void performParallelIRGeneration(IRGenDescriptor desc) {
 
   // Free the memory occupied by the SILModule.
   // Execute this task in parallel to the LLVM compilation.
-  auto SILModuleRelease = [&SILMod]() { SILMod.reset(nullptr); };
+  auto SILModuleRelease = [&SILMod]() {
+    SILMod.reset(nullptr);
+    SILModule::checkForLeaksAfterDestruction();
+  };
   auto releaseModuleThread = std::thread(SILModuleRelease);
 
   codeGenThreads.runMainThread();

lib/SIL/IR/SILBasicBlock.cpp

@@ -43,30 +43,43 @@ SILBasicBlock::SILBasicBlock(SILFunction *parent, SILBasicBlock *relativeToBB,
   }
 }
 SILBasicBlock::~SILBasicBlock() {
+  if (!getParent()) {
+    assert(ArgumentList.empty() &&
+           "a static initializer block must not have arguments");
+    assert(InstList.empty() &&
+           "a static initializer block must be cleared before the destructor");
+    return;
+  }
+    
+  SILModule &M = getModule();
+
   // Invalidate all of the basic block arguments.
   for (auto *Arg : ArgumentList) {
-    getModule().notifyDeleteHandlers(Arg);
+    M.notifyDeleteHandlers(Arg);
   }
 
   dropAllReferences();
 
-  SILModule *M = nullptr;
-  if (getParent())
-    M = &getParent()->getModule();
-
   for (auto I = begin(), E = end(); I != E;) {
     auto Inst = &*I;
     ++I;
-    if (M) {
-      // Notify the delete handlers that the instructions in this block are
-      // being deleted.
-      M->notifyDeleteHandlers(Inst);
-    }
     erase(Inst);
   }
+  assert(InstList.empty());
+}
+
+void SILBasicBlock::clearStaticInitializerBlock(SILModule &module) {
+  assert(!getParent() && "not a global variable's static initializer block");
+  assert(ArgumentList.empty() &&
+         "a static initializer block must not have arguments");
 
-  // iplist's destructor is going to destroy the InstList.
-  InstList.clearAndLeakNodesUnsafely();
+  for (auto I = begin(), E = end(); I != E;) {
+    SILInstruction *Inst = &*I;
+    ++I;
+    InstList.erase(Inst);
+    module.deallocateInst(Inst);
+  }
+  assert(InstList.empty());
 }
 
 int SILBasicBlock::getDebugID() const {
@@ -112,10 +125,10 @@ void SILBasicBlock::eraseInstructions() {
 /// Returns the iterator following the erased instruction.
 SILBasicBlock::iterator SILBasicBlock::erase(SILInstruction *I) {
   // Notify the delete handlers that this instruction is going away.
-  getModule().notifyDeleteHandlers(&*I);
-  auto *F = getParent();
+  SILModule &module = getModule();
+  module.notifyDeleteHandlers(&*I);
   auto nextIter = InstList.erase(I);
-  F->getModule().deallocateInst(I);
+  module.deallocateInst(I);
   return nextIter;
 }
 

lib/SIL/IR/SILFunction.cpp

@@ -72,13 +72,29 @@ SILFunction::create(SILModule &M, SILLinkage linkage, StringRef name,
     name = entry->getKey();
   }
 
-  auto fn = new (M) SILFunction(M, linkage, name, loweredType, genericEnv, loc,
+  SILFunction *fn = M.removeFromZombieList(name);
+  if (fn) {
+    // Resurrect a zombie function.
+    // This happens for example if a specialized function gets dead and gets
+    // deleted. And afterwards the same specialization is created again.
+    fn->init(linkage, name, loweredType, genericEnv, loc, isBareSILFunction,
+             isTrans, isSerialized, entryCount, isThunk, classSubclassScope,
+             inlineStrategy, E, debugScope, isDynamic, isExactSelfClass);
+    assert(fn->empty());
+  } else {
+    fn = new (M) SILFunction(M, linkage, name, loweredType, genericEnv, loc,
                                 isBareSILFunction, isTrans, isSerialized,
                                 entryCount, isThunk, classSubclassScope,
-                                inlineStrategy, E, insertBefore, debugScope,
+                                inlineStrategy, E, debugScope,
                                 isDynamic, isExactSelfClass);
-
+  }
   if (entry) entry->setValue(fn);
+
+  if (insertBefore)
+    M.functions.insert(SILModule::iterator(insertBefore), fn);
+  else
+    M.functions.push_back(fn);
+
   return fn;
 }
 
@@ -90,41 +106,60 @@ SILFunction::SILFunction(SILModule &Module, SILLinkage Linkage, StringRef Name,
                          ProfileCounter entryCount, IsThunk_t isThunk,
                          SubclassScope classSubclassScope,
                          Inline_t inlineStrategy, EffectsKind E,
-                         SILFunction *InsertBefore,
                          const SILDebugScope *DebugScope,
                          IsDynamicallyReplaceable_t isDynamic,
                          IsExactSelfClass_t isExactSelfClass)
-    : Module(Module), Name(Name), LoweredType(LoweredType),
-      GenericEnv(genericEnv), SpecializationInfo(nullptr),
-      EntryCount(entryCount),
-      Availability(AvailabilityContext::alwaysAvailable()),
-      Bare(isBareSILFunction), Transparent(isTrans),
-      Serialized(isSerialized), Thunk(isThunk),
-      ClassSubclassScope(unsigned(classSubclassScope)), GlobalInitFlag(false),
-      InlineStrategy(inlineStrategy), Linkage(unsigned(Linkage)),
-      HasCReferences(false), IsWeakImported(false),
-      IsDynamicReplaceable(isDynamic),
-      ExactSelfClass(isExactSelfClass),
-      Inlined(false), Zombie(false), HasOwnership(true),
-      WasDeserializedCanonical(false), IsWithoutActuallyEscapingThunk(false),
-      OptMode(unsigned(OptimizationMode::NotSet)),
-      EffectsKindAttr(unsigned(E)) {
-  assert(!Transparent || !IsDynamicReplaceable);
-  validateSubclassScope(classSubclassScope, isThunk, nullptr);
-  setDebugScope(DebugScope);
-
-  if (InsertBefore)
-    Module.functions.insert(SILModule::iterator(InsertBefore), this);
-  else
-    Module.functions.push_back(this);
-
-  Module.removeFromZombieList(Name);
-
+    : Module(Module), Availability(AvailabilityContext::alwaysAvailable())  {
+  init(Linkage, Name, LoweredType, genericEnv, Loc, isBareSILFunction, isTrans,
+       isSerialized, entryCount, isThunk, classSubclassScope, inlineStrategy,
+       E, DebugScope, isDynamic, isExactSelfClass);
+  
   // Set our BB list to have this function as its parent. This enables us to
   // splice efficiently basic blocks in between functions.
   BlockList.Parent = this;
 }
 
+void SILFunction::init(SILLinkage Linkage, StringRef Name,
+                         CanSILFunctionType LoweredType,
+                         GenericEnvironment *genericEnv,
+                         Optional<SILLocation> Loc, IsBare_t isBareSILFunction,
+                         IsTransparent_t isTrans, IsSerialized_t isSerialized,
+                         ProfileCounter entryCount, IsThunk_t isThunk,
+                         SubclassScope classSubclassScope,
+                         Inline_t inlineStrategy, EffectsKind E,
+                         const SILDebugScope *DebugScope,
+                         IsDynamicallyReplaceable_t isDynamic,
+                         IsExactSelfClass_t isExactSelfClass) {
+  this->Name = Name;
+  this->LoweredType = LoweredType;
+  this->GenericEnv = genericEnv;
+  this->SpecializationInfo = nullptr;
+  this->EntryCount = entryCount;
+  this->Availability = AvailabilityContext::alwaysAvailable();
+  this->Bare = isBareSILFunction;
+  this->Transparent = isTrans;
+  this->Serialized = isSerialized;
+  this->Thunk = isThunk;
+  this->ClassSubclassScope = unsigned(classSubclassScope);
+  this->GlobalInitFlag = false;
+  this->InlineStrategy = inlineStrategy;
+  this->Linkage = unsigned(Linkage);
+  this->HasCReferences = false;
+  this->IsWeakImported = false;
+  this->IsDynamicReplaceable = isDynamic;
+  this->ExactSelfClass = isExactSelfClass;
+  this->Inlined = false;
+  this->Zombie = false;
+  this->HasOwnership = true,
+  this->WasDeserializedCanonical = false;
+  this->IsWithoutActuallyEscapingThunk = false;
+  this->OptMode = unsigned(OptimizationMode::NotSet);
+  this->EffectsKindAttr = unsigned(E);
+  assert(!Transparent || !IsDynamicReplaceable);
+  validateSubclassScope(classSubclassScope, isThunk, nullptr);
+  setDebugScope(DebugScope);
+}
+
 SILFunction::~SILFunction() {
   // If the function is recursive, a function_ref inst inside of the function
   // will give the function a non-zero ref count triggering the assertion. Thus

lib/SIL/IR/SILGlobalVariable.cpp

@@ -58,7 +58,8 @@ SILGlobalVariable::SILGlobalVariable(SILModule &Module, SILLinkage Linkage,
 }
 
 SILGlobalVariable::~SILGlobalVariable() {
-  getModule().GlobalVariableMap.erase(Name);
+  StaticInitializerBlock.dropAllReferences();
+  StaticInitializerBlock.clearStaticInitializerBlock(Module);
 }
 
 /// Get this global variable's fragile attribute.