[PATCH] D70378: [LLD][COFF] Fix missing cache cleanup in COFF::link()

[Erik McClure via Phabricator via llvm-commits]

blackhole12 created this revision.
blackhole12 added a reviewer: lld.
blackhole12 added projects: LLVM, lld.

After I provided this fix <https://reviews.llvm.org/D63042> for missing global variable cleanup, two new global caches were introduced that do not get correctly cleaned up. This adds cleanup functions and calls them from COFF::link().

I am increasingly concerned about these kinds of bugs being repeatedly introduced into LLD. If programmers cannot reliably remember to clean up global caches, and there is no test that looks for these bugs, maybe LLD should stop relying on error-prone global variables and instead create a single "global cache" object that must contain all global variables?


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D70378

Files:
  lld/COFF/DebugTypes.cpp
  lld/COFF/DebugTypes.h
  lld/COFF/Driver.cpp
  lld/COFF/Writer.cpp
  lld/COFF/Writer.h


Index: lld/COFF/Writer.h
===================================================================
--- lld/COFF/Writer.h
+++ lld/COFF/Writer.h
@@ -75,6 +75,8 @@
 
   std::vector<PartialSection *> contribSections;
 
+  static void clearOutputSections();
+
 private:
   uint32_t stringTableOff = 0;
 };
Index: lld/COFF/Writer.cpp
===================================================================
--- lld/COFF/Writer.cpp
+++ lld/COFF/Writer.cpp
@@ -82,6 +82,8 @@
 // this can be indexed by Chunk::getOutputSection.
 static std::vector<OutputSection *> outputSections;
 
+void OutputSection::clearOutputSections() { outputSections.clear(); }
+
 OutputSection *Chunk::getOutputSection() const {
   return osidx == 0 ? nullptr : outputSections[osidx - 1];
 }
Index: lld/COFF/Driver.cpp
===================================================================
--- lld/COFF/Driver.cpp
+++ lld/COFF/Driver.cpp
@@ -84,6 +84,8 @@
   ImportFile::instances.clear();
   BitcodeFile::instances.clear();
   memset(MergeChunk::instances, 0, sizeof(MergeChunk::instances));
+  OutputSection::clearOutputSections();
+  clearTypeServerSourceInstances();
   return !errorCount();
 }
 
Index: lld/COFF/DebugTypes.h
===================================================================
--- lld/COFF/DebugTypes.h
+++ lld/COFF/DebugTypes.h
@@ -46,6 +46,7 @@
                                 const llvm::codeview::PrecompRecord *precomp);
 
 void loadTypeServerSource(llvm::MemoryBufferRef m);
+void clearTypeServerSourceInstances();
 
 // Temporary interface to get the dependency
 template <typename T> const T &retrieveDependencyInfo(const TpiSource *source);
Index: lld/COFF/DebugTypes.cpp
===================================================================
--- lld/COFF/DebugTypes.cpp
+++ lld/COFF/DebugTypes.cpp
@@ -90,6 +90,8 @@
 };
 } // namespace
 
+void lld::coff::clearTypeServerSourceInstances() { TypeServerSource::instances.clear(); }
+
 static std::vector<std::unique_ptr<TpiSource>> GC;
 
 TpiSource::TpiSource(TpiKind k, const ObjFile *f) : kind(k), file(f) {


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D70378.229762.patch
Type: text/x-patch
Size: 2040 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20191118/6c0f1064/attachment.bin>