[PATCH] D128167: CommandLine: Unregister options in the destructor

[Nicolai Hähnle via Phabricator via llvm-commits]

nhaehnle created this revision.
Herald added a subscriber: hiraditya.
Herald added a project: All.
nhaehnle requested review of this revision.
Herald added a project: LLVM.

Fix a corruption issue when LLVM is used as a shared library in a plugin
setting.

Consider the case where A depends on B depends on LLVM, where B is a
shared compiler library built on LLVM that defines command-line options
(cl::opt) because it can also be used in a non-plugin setting.

If A is loaded and unloaded multiple times, causing B to be loaded and
unloaded multiple times while LLVM *isn't* unloaded, then corruption
results without this change: as B is unloaded, its cl::opt globals
disappear but the GlobalParser still holds a (dangling) reference to
them. Furthermore, if A and B are loaded a second time, the cl::opt
globals are constructed again which fails because options of the same
name are still registered.

The straightforward fix is to remove cl::opts from the GlobalParser in
the destructor.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D128167

Files:
  llvm/include/llvm/Support/CommandLine.h
  llvm/lib/Support/CommandLine.cpp


Index: llvm/lib/Support/CommandLine.cpp
===================================================================
--- llvm/lib/Support/CommandLine.cpp
+++ llvm/lib/Support/CommandLine.cpp
@@ -448,7 +448,12 @@
   FullyInitialized = true;
 }
 
-void Option::removeArgument() { GlobalParser->removeOption(this); }
+void Option::removeArgument() {
+  // This method may be called from global destructors. Do nothing if the global
+  // parser has already been destroyed.
+  if (auto *Parser = GlobalParser.peek())
+    Parser->removeOption(this);
+}
 
 void Option::setArgStr(StringRef S) {
   if (FullyInitialized)
Index: llvm/include/llvm/Support/CommandLine.h
===================================================================
--- llvm/include/llvm/Support/CommandLine.h
+++ llvm/include/llvm/Support/CommandLine.h
@@ -339,7 +339,10 @@
   inline void setNumAdditionalVals(unsigned n) { AdditionalVals = n; }
 
 public:
-  virtual ~Option() = default;
+  virtual ~Option() {
+    if (FullyInitialized)
+      removeArgument();
+  }
 
   // Register this argument with the commandline system.
   //
@@ -347,8 +350,6 @@
 
   /// Unregisters this option from the CommandLine system.
   ///
-  /// This option must have been the last option registered.
-  /// For testing purposes only.
   void removeArgument();
 
   // Return the width of the option tag for printing...


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D128167.438307.patch
Type: text/x-patch
Size: 1361 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20220620/7286b08a/attachment.bin>