[ELF/Driver] Avoid a reference to the ELFOptTable after it has gone out of scope

[filcab]

Summary

This solves PR26908. ASan detects it, but not by default.
It needs ASAN_OPTIONS=detect_stack_use_after_return=true

[ruiu]

Ah, thank you for finding this one! I quickly took a look when I updated the bug but couldn't figure out what was wrong.

I'd like to keep the parser function in DriverUtils. I'd define a new class to own ELFOptTable in DriverUtils.

[ruiu]

Does http://reviews.llvm.org/D18169 solves PR26908?

[filcab]

It looks like it should, but I can't try it out now.
I can try it tonight or tomorrow morning at most. If you want, you can
compile llvm+lld with ASan and turn on the Asan option (double-check that
you catch the bug first ;-) ).

CCing Kostya:
Should we add ASAN_OPTIONS=detect_stack_use_after_free=true to the
sanitizer bot?
It would have caught this bug a long time ago.
We might also need to do some cleanup before we can do it.

Thank you,

Filipe

ELF/Driver.h

private:
   template <class ELFT> void link(llvm::opt::InputArgList &Args);
 
   llvm::BumpPtrAllocator Alloc;
   bool WholeArchive = false;
   std::vector<std::unique_ptr<InputFile>> Files;
   std::vector<std::unique_ptr<MemoryBuffer>> OwningMBs;
 };
 
-// Parses command line options.
-llvm::opt::InputArgList parseArgs(llvm::BumpPtrAllocator *A,
-                                  ArrayRef<const char *> Args);
-
 // Create enum with OPT_xxx values for each option in Options.td
 enum {
   OPT_INVALID = 0,
 #define OPTION(_1, _2, ID, _4, _5, _6, _7, _8, _9, _10, _11) OPT_##ID,
 #include "Options.inc"
 #undef OPTION
 };
 
-void printHelp(const char *Argv0);
 void printVersion();
 
 std::string findFromSearchPaths(StringRef Path);
 std::string searchLibrary(StringRef Path);
 std::string buildSysrootedPath(llvm::StringRef Dir, llvm::StringRef File);
 
 } // namespace elf
 } // namespace lld
 
 #endif

ELF/Driver.cpp

 #include "ICF.h"
 #include "InputFiles.h"
 #include "LinkerScript.h"
 #include "SymbolTable.h"
 #include "Target.h"
 #include "Writer.h"
 #include "lld/Driver/Driver.h"
 #include "llvm/ADT/StringExtras.h"
+#include "llvm/Support/CommandLine.h"
 #include "llvm/Support/TargetSelect.h"
 #include "llvm/Support/raw_ostream.h"
 #include <utility>
 
 using namespace llvm;
 using namespace llvm::ELF;
 using namespace llvm::object;
 
 using namespace lld;
 using namespace lld::elf;
 
 Configuration *elf::Config;
 LinkerDriver *elf::Driver;
 
+// Create OptTable
+// Create prefix string literals used in Options.td
+#define PREFIX(NAME, VALUE) const char *const NAME[] = VALUE;
+#include "Options.inc"
+#undef PREFIX
+
+// Create table mapping all options defined in Options.td
+static const opt::OptTable::Info infoTable[] = {
+#define OPTION(X1, X2, ID, KIND, GROUP, ALIAS, X6, X7, X8, X9, X10)            \
+  {                                                                            \
+    X1, X2, X9, X10, OPT_##ID, opt::Option::KIND##Class, X8, X7, OPT_##GROUP,  \
+        OPT_##ALIAS, X6                                                        \
+  }                                                                            \
+  ,
+#include "Options.inc"
+#undef OPTION
+};
+
+class ELFOptTable : public opt::OptTable {
+public:
+  ELFOptTable() : OptTable(infoTable) {}
+};
+
+// Parses a given list of options.
+static opt::InputArgList parseArgs(llvm::BumpPtrAllocator *A,
+                                   ArrayRef<const char *> Argv,
+                                   ELFOptTable &Table) {
+  // Make InputArgList from string vectors.
+  unsigned MissingIndex;
+  unsigned MissingCount;
+
+  // Expand response files. '@<filename>' is replaced by the file's contents.
+  SmallVector<const char *, 256> Vec(Argv.data(), Argv.data() + Argv.size());
+  StringSaver Saver(*A);
+  llvm::cl::ExpandResponseFiles(Saver, llvm::cl::TokenizeGNUCommandLine, Vec);
+
+  // Parse options and then do error checking.
+  opt::InputArgList Args = Table.ParseArgs(Vec, MissingIndex, MissingCount);
+  if (MissingCount)
+    error(Twine("missing arg value for \"") + Args.getArgString(MissingIndex) +
+          "\", expected " + Twine(MissingCount) +
+          (MissingCount == 1 ? " argument.\n" : " arguments"));
+
+  iterator_range<opt::arg_iterator> Unknowns = Args.filtered(OPT_UNKNOWN);
+  for (auto *Arg : Unknowns)
+    warning("warning: unknown argument: " + Arg->getSpelling());
+  if (Unknowns.begin() != Unknowns.end())
+    error("unknown argument(s) found");
+  return Args;
+}
+
+static void printHelp(const char *Argv0) {
+  ELFOptTable Table;
+  Table.PrintHelp(outs(), Argv0, "lld", false);
+}
+
 bool elf::link(ArrayRef<const char *> Args, raw_ostream &Error) {
   HasError = false;
   ErrorOS = &Error;
   Configuration C;
   LinkerDriver D;
   LinkerScript LS;
   Config = &C;
   Driver = &D;

 static bool hasZOption(opt::InputArgList &Args, StringRef Key) {
   for (auto *Arg : Args.filtered(OPT_z))
     if (Key == Arg->getValue())
       return true;
   return false;
 }
 
 void LinkerDriver::main(ArrayRef<const char *> ArgsArr) {
-  opt::InputArgList Args = parseArgs(&Alloc, ArgsArr.slice(1));
+  ELFOptTable Table;
+  opt::InputArgList Args = parseArgs(&Alloc, ArgsArr.slice(1), Table);
   if (Args.hasArg(OPT_help)) {
     printHelp(ArgsArr[0]);
     return;
   }
   if (Args.hasArg(OPT_version)) {
     printVersion();
     return;
   }

ELF/DriverUtils.cpp

 // Driver.cpp less cluttered.
 //
 //===----------------------------------------------------------------------===//
 
 #include "Driver.h"
 #include "Error.h"
 #include "lld/Config/Version.h"
 #include "llvm/ADT/STLExtras.h"
-#include "llvm/Support/CommandLine.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/StringSaver.h"
 
 using namespace llvm;
 
 using namespace lld;
 using namespace lld::elf;
 
-// Create OptTable
-
-// Create prefix string literals used in Options.td
-#define PREFIX(NAME, VALUE) const char *const NAME[] = VALUE;
-#include "Options.inc"
-#undef PREFIX
-
-// Create table mapping all options defined in Options.td
-static const opt::OptTable::Info infoTable[] = {
-#define OPTION(X1, X2, ID, KIND, GROUP, ALIAS, X6, X7, X8, X9, X10)            \
-  {                                                                            \
-    X1, X2, X9, X10, OPT_##ID, opt::Option::KIND##Class, X8, X7, OPT_##GROUP,  \
-        OPT_##ALIAS, X6                                                        \
-  }                                                                            \
-  ,
-#include "Options.inc"
-#undef OPTION
-};
-
-class ELFOptTable : public opt::OptTable {
-public:
-  ELFOptTable() : OptTable(infoTable) {}
-};
-
-// Parses a given list of options.
-opt::InputArgList elf::parseArgs(llvm::BumpPtrAllocator *A,
-                                 ArrayRef<const char *> Argv) {
-  // Make InputArgList from string vectors.
-  ELFOptTable Table;
-  unsigned MissingIndex;
-  unsigned MissingCount;
-
-  // Expand response files. '@<filename>' is replaced by the file's contents.
-  SmallVector<const char *, 256> Vec(Argv.data(), Argv.data() + Argv.size());
-  StringSaver Saver(*A);
-  llvm::cl::ExpandResponseFiles(Saver, llvm::cl::TokenizeGNUCommandLine, Vec);
-
-  // Parse options and then do error checking.
-  opt::InputArgList Args = Table.ParseArgs(Vec, MissingIndex, MissingCount);
-  if (MissingCount)
-    error(Twine("missing arg value for \"") + Args.getArgString(MissingIndex) +
-          "\", expected " + Twine(MissingCount) +
-          (MissingCount == 1 ? " argument.\n" : " arguments"));
-
-  iterator_range<opt::arg_iterator> Unknowns = Args.filtered(OPT_UNKNOWN);
-  for (auto *Arg : Unknowns)
-    warning("warning: unknown argument: " + Arg->getSpelling());
-  if (Unknowns.begin() != Unknowns.end())
-    error("unknown argument(s) found");
-  return Args;
-}
-
-void elf::printHelp(const char *Argv0) {
-  ELFOptTable Table;
-  Table.PrintHelp(outs(), Argv0, "lld", false);
-}
-
 void elf::printVersion() {
   outs() << "LLD " << getLLDVersion();
   std::string S = getLLDRepositoryVersion();
   if (!S.empty())
     outs() << " " << S;
   outs() << "\n";
 }