[PECOFF] Allow multiple directives in one module-definition file.

I'm a bit surprised that I have not implemented this yet. This is
definitely needed to handle real-world module definition files.
This patch contains a unit test for r207294.

llvm-svn: 207297

lld/include/lld/Driver/WinLinkModuleDef.h

@@ -20,6 +20,8 @@
 #include "llvm/ADT/Optional.h"
 #include "llvm/Support/Allocator.h"
 
+#include <vector>
+
 namespace lld {
 namespace moduledef {
 
@@ -171,7 +173,7 @@ public:
   Parser(Lexer &lex, llvm::BumpPtrAllocator &alloc)
       : _lex(lex), _alloc(alloc) {}
 
-  llvm::Optional<Directive *> parse();
+  bool parse(std::vector<Directive *> &ret);
 
 private:
   void consumeToken();
@@ -181,6 +183,7 @@ private:
   void ungetToken();
   void error(const Token &tok, Twine msg);
 
+  bool parseOne(Directive *&dir);
   bool parseExport(PECOFFLinkingContext::ExportDesc &result);
   bool parseMemorySize(uint64_t &reserve, uint64_t &commit);
   bool parseName(std::string &outfile, uint64_t &baseaddr);

lld/lib/Driver/WinLinkDriver.cpp

@@ -351,14 +351,14 @@ static bool parseExport(StringRef option,
 }
 
 // Read module-definition file.
-static llvm::Optional<moduledef::Directive *>
+static bool parseDef(StringRef option, llvm::BumpPtrAllocator &alloc,
-parseDef(StringRef option, llvm::BumpPtrAllocator &alloc) {
+                     std::vector<moduledef::Directive *> &result) {
   std::unique_ptr<MemoryBuffer> buf;
   if (MemoryBuffer::getFile(option, buf))
     return llvm::None;
   moduledef::Lexer lexer(std::move(buf));
   moduledef::Parser parser(lexer, alloc);
-  return parser.parse();
+  return parser.parse(result);
 }
 
 static StringRef replaceExtension(PECOFFLinkingContext &ctx, StringRef path,
@@ -1050,37 +1050,37 @@ bool WinLinkDriver::parse(int argc, const char *argv[],
 
     case OPT_deffile: {
       llvm::BumpPtrAllocator alloc;
-      llvm::Optional<moduledef::Directive *> dir =
+      std::vector<moduledef::Directive *> dirs;
-          parseDef(inputArg->getValue(), alloc);
+      if (!parseDef(inputArg->getValue(), alloc, dirs)) {
-      if (!dir.hasValue()) {
         diag << "Error: invalid module-definition file\n";
         return false;
       }
-
+      for (moduledef::Directive *dir : dirs) {
-      if (auto *exp = dyn_cast<moduledef::Exports>(dir.getValue())) {
+        if (auto *exp = dyn_cast<moduledef::Exports>(dir)) {
-        for (PECOFFLinkingContext::ExportDesc desc : exp->getExports()) {
+          for (PECOFFLinkingContext::ExportDesc desc : exp->getExports()) {
-          desc.name = ctx.decorateSymbol(desc.name);
+            desc.name = ctx.decorateSymbol(desc.name);
-          ctx.addDllExport(desc);
+            ctx.addDllExport(desc);
+          }
+        } else if (auto *hs = dyn_cast<moduledef::Heapsize>(dir)) {
+          ctx.setHeapReserve(hs->getReserve());
+          ctx.setHeapCommit(hs->getCommit());
+        } else if (auto *lib = dyn_cast<moduledef::Library>(dir)) {
+          ctx.setIsDll(true);
+          ctx.setOutputPath(ctx.allocate(lib->getName()));
+          if (lib->getBaseAddress() && !ctx.getBaseAddress())
+            ctx.setBaseAddress(lib->getBaseAddress());
+        } else if (auto *name = dyn_cast<moduledef::Name>(dir)) {
+          if (!name->getOutputPath().empty() && ctx.outputPath().empty())
+            ctx.setOutputPath(ctx.allocate(name->getOutputPath()));
+          if (name->getBaseAddress() && ctx.getBaseAddress())
+            ctx.setBaseAddress(name->getBaseAddress());
+        } else if (auto *ver = dyn_cast<moduledef::Version>(dir)) {
+          ctx.setImageVersion(PECOFFLinkingContext::Version(
+              ver->getMajorVersion(), ver->getMinorVersion()));
+        } else {
+          llvm::dbgs() << static_cast<int>(dir->getKind()) << "\n";
+          llvm_unreachable("Unknown module-definition directive.\n");
         }
-      } else if (auto *hs = dyn_cast<moduledef::Heapsize>(dir.getValue())) {
-        ctx.setHeapReserve(hs->getReserve());
-        ctx.setHeapCommit(hs->getCommit());
-      } else if (auto *lib = dyn_cast<moduledef::Library>(dir.getValue())) {
-        ctx.setIsDll(true);
-        ctx.setOutputPath(ctx.allocate(lib->getName()));
-        if (lib->getBaseAddress() && !ctx.getBaseAddress())
-          ctx.setBaseAddress(lib->getBaseAddress());
-      } else if (auto *name = dyn_cast<moduledef::Name>(dir.getValue())) {
-        if (!name->getOutputPath().empty() && ctx.outputPath().empty())
-          ctx.setOutputPath(ctx.allocate(name->getOutputPath()));
-        if (name->getBaseAddress() && ctx.getBaseAddress())
-          ctx.setBaseAddress(name->getBaseAddress());
-      } else if (auto *ver = dyn_cast<moduledef::Version>(dir.getValue())) {
-        ctx.setImageVersion(PECOFFLinkingContext::Version(
-            ver->getMajorVersion(), ver->getMinorVersion()));
-      } else {
-        llvm::dbgs() << static_cast<int>(dir.getValue()->getKind()) << "\n";
-        llvm_unreachable("Unknown module-definition directive.\n");
       }
     }
 

lld/lib/Driver/WinLinkModuleDef.cpp

@@ -111,9 +111,22 @@ void Parser::error(const Token &tok, Twine msg) {
       msg);
 }
 
-llvm::Optional<Directive *> Parser::parse() {
+bool Parser::parse(std::vector<Directive *> &ret) {
+  for (;;) {
+    Directive *dir = nullptr;
+    if (!parseOne(dir))
+      return false;
+    if (!dir)
+      return true;
+    ret.push_back(dir);
+  }
+}
+
+bool Parser::parseOne(Directive *&ret) {
   consumeToken();
   switch (_tok._kind) {
+  case Kind::eof:
+    return true;
   case Kind::kw_exports: {
     // EXPORTS
     std::vector<PECOFFLinkingContext::ExportDesc> exports;
@@ -123,48 +136,54 @@ llvm::Optional<Directive *> Parser::parse() {
         break;
       exports.push_back(desc);
     }
-    return new (_alloc) Exports(exports);
+    ret = new (_alloc) Exports(exports);
+    return true;
   }
   case Kind::kw_heapsize: {
     // HEAPSIZE
     uint64_t reserve, commit;
     if (!parseMemorySize(reserve, commit))
-      return llvm::None;
+      return false;
-    return new (_alloc) Heapsize(reserve, commit);
+    ret = new (_alloc) Heapsize(reserve, commit);
+    return true;
   }
   case Kind::kw_library: {
     // LIBRARY
     std::string name;
     uint64_t baseaddr;
     if (!parseName(name, baseaddr))
-      return llvm::None;
+      return false;
-    return new (_alloc) Library(name, baseaddr);
+    ret = new (_alloc) Library(name, baseaddr);
+    return true;
   }
   case Kind::kw_stacksize: {
     // STACKSIZE
     uint64_t reserve, commit;
     if (!parseMemorySize(reserve, commit))
-      return llvm::None;
+      return false;
-    return new (_alloc) Stacksize(reserve, commit);
+    ret = new (_alloc) Stacksize(reserve, commit);
+    return true;
   }
   case Kind::kw_name: {
     // NAME
     std::string outputPath;
     uint64_t baseaddr;
     if (!parseName(outputPath, baseaddr))
-      return llvm::None;
+      return false;
-    return new (_alloc) Name(outputPath, baseaddr);
+    ret = new (_alloc) Name(outputPath, baseaddr);
+    return true;
   }
   case Kind::kw_version: {
     // VERSION
     int major, minor;
     if (!parseVersion(major, minor))
-      return llvm::None;
+      return false;
-    return new (_alloc) Version(major, minor);
+    ret = new (_alloc) Version(major, minor);
+    return true;
   }
   default:
     error(_tok, Twine("Unknown directive: ") + _tok._range);
-    return llvm::None;
+    return false;
   }
 }
 
@@ -219,16 +238,19 @@ bool Parser::parseName(std::string &outputPath, uint64_t &baseaddr) {
   consumeToken();
   if (_tok._kind == Kind::identifier) {
     outputPath = _tok._range;
-    consumeToken();
   } else {
     outputPath = "";
+    ungetToken();
+    return true;
   }
+  consumeToken();
   if (_tok._kind == Kind::kw_base) {
     if (!expectAndConsume(Kind::equal, "'=' expected"))
       return false;
     if (!consumeTokenAsInt(baseaddr))
       return false;
   } else {
+    ungetToken();
     baseaddr = 0;
   }
   return true;

lld/unittests/DriverTests/WinLinkModuleDefTest.cpp

@@ -16,26 +16,19 @@
 using namespace llvm;
 using namespace lld;
 
-template <typename T> class ParserTest : public testing::Test {
+class ParserTest : public testing::Test {
 protected:
-  T *parse(const char *contents) {
+  std::vector<moduledef::Directive *> _dirs;
+
+  void parse(const char *contents) {
     auto membuf =
         std::unique_ptr<MemoryBuffer>(MemoryBuffer::getMemBuffer(contents));
     moduledef::Lexer lexer(std::move(membuf));
     moduledef::Parser parser(lexer, _alloc);
-    llvm::Optional<moduledef::Directive *> dir = parser.parse();
+    EXPECT_TRUE(parser.parse(_dirs));
-    EXPECT_TRUE(dir.hasValue());
+    EXPECT_TRUE(!_dirs.empty());
-    T *ret = dyn_cast<T>(dir.getValue());
-    EXPECT_TRUE(ret != nullptr);
-    return ret;
   }
 
-private:
-  llvm::BumpPtrAllocator _alloc;
-};
-
-class ExportsTest : public ParserTest<moduledef::Exports> {
-public:
   void verifyExportDesc(const PECOFFLinkingContext::ExportDesc &exp,
                         StringRef sym, int ordinal, bool noname, bool isData) {
     EXPECT_EQ(sym, exp.name);
@@ -43,22 +36,21 @@ public:
     EXPECT_EQ(noname, exp.noname);
     EXPECT_EQ(isData, exp.isData);
   }
+
+private:
+  llvm::BumpPtrAllocator _alloc;
 };
 
-class HeapsizeTest : public ParserTest<moduledef::Heapsize> {};
+TEST_F(ParserTest, Exports) {
-class StacksizeTest : public ParserTest<moduledef::Stacksize> {};
+  parse("EXPORTS\n"
-class NameTest : public ParserTest<moduledef::Name> {};
+        "  sym1\n"
-class VersionTest : public ParserTest<moduledef::Version> {};
+        "  sym2 @5\n"
-
+        "  sym3 @8 NONAME\n"
-TEST_F(ExportsTest, Basic) {
+        "  sym4 DATA\n"
-  moduledef::Exports *dir = parse("EXPORTS\n"
+        "  sym5 @10 NONAME DATA\n");
-                                  "  sym1\n"
+  EXPECT_EQ(1U, _dirs.size());
-                                  "  sym2 @5\n"
-                                  "  sym3 @8 NONAME\n"
-                                  "  sym4 DATA\n"
-                                  "  sym5 @10 NONAME DATA\n");
   const std::vector<PECOFFLinkingContext::ExportDesc> &exports =
-      dir->getExports();
+      cast<moduledef::Exports>(_dirs[0])->getExports();
   EXPECT_EQ(5U, exports.size());
   verifyExportDesc(exports[0], "sym1", -1, false, false);
   verifyExportDesc(exports[1], "sym2", 5, false, false);
@@ -67,56 +59,98 @@ TEST_F(ExportsTest, Basic) {
   verifyExportDesc(exports[4], "sym5", 10, true, true);
 }
 
-TEST_F(HeapsizeTest, Basic) {
+TEST_F(ParserTest, Heapsize) {
-  moduledef::Heapsize *heapsize = parse("HEAPSIZE 65536");
+  parse("HEAPSIZE 65536");
+  EXPECT_EQ(1U, _dirs.size());
+  auto *heapsize = cast<moduledef::Heapsize>(_dirs[0]);
   EXPECT_EQ(65536U, heapsize->getReserve());
   EXPECT_EQ(0U, heapsize->getCommit());
 }
 
-TEST_F(HeapsizeTest, WithCommit) {
+TEST_F(ParserTest, HeapsizeWithCommit) {
-  moduledef::Heapsize *heapsize = parse("HEAPSIZE 65536, 8192");
+  parse("HEAPSIZE 65536, 8192");
+  EXPECT_EQ(1U, _dirs.size());
+  auto *heapsize = cast<moduledef::Heapsize>(_dirs[0]);
   EXPECT_EQ(65536U, heapsize->getReserve());
   EXPECT_EQ(8192U, heapsize->getCommit());
 }
 
-TEST_F(StacksizeTest, Basic) {
+TEST_F(ParserTest, StacksizeBasic) {
-  moduledef::Stacksize *stacksize = parse("STACKSIZE 65536");
+  parse("STACKSIZE 65536");
+  EXPECT_EQ(1U, _dirs.size());
+  auto *stacksize = cast<moduledef::Stacksize>(_dirs[0]);
   EXPECT_EQ(65536U, stacksize->getReserve());
   EXPECT_EQ(0U, stacksize->getCommit());
 }
 
-TEST_F(StacksizeTest, WithCommit) {
+TEST_F(ParserTest, StacksizeWithCommit) {
-  moduledef::Stacksize *stacksize = parse("STACKSIZE 65536, 8192");
+  parse("STACKSIZE 65536, 8192");
+  EXPECT_EQ(1U, _dirs.size());
+  auto *stacksize = cast<moduledef::Stacksize>(_dirs[0]);
   EXPECT_EQ(65536U, stacksize->getReserve());
   EXPECT_EQ(8192U, stacksize->getCommit());
 }
 
-TEST_F(NameTest, Basic) {
+TEST_F(ParserTest, Library) {
-  moduledef::Name *name = parse("NAME foo.exe");
+  parse("LIBRARY foo.dll");
+  EXPECT_EQ(1U, _dirs.size());
+  auto *lib = cast<moduledef::Library>(_dirs[0]);
+  EXPECT_EQ("foo.dll", lib->getName());
+}
+
+TEST_F(ParserTest, NameBasic) {
+  parse("NAME foo.exe");
+  EXPECT_EQ(1U, _dirs.size());
+  auto *name = cast<moduledef::Name>(_dirs[0]);
   EXPECT_EQ("foo.exe", name->getOutputPath());
   EXPECT_EQ(0U, name->getBaseAddress());
 }
 
-TEST_F(NameTest, WithBase) {
+TEST_F(ParserTest, NameWithBase) {
-  moduledef::Name *name = parse("NAME foo.exe BASE=4096");
+  parse("NAME foo.exe BASE=4096");
+  EXPECT_EQ(1U, _dirs.size());
+  auto *name = cast<moduledef::Name>(_dirs[0]);
   EXPECT_EQ("foo.exe", name->getOutputPath());
   EXPECT_EQ(4096U, name->getBaseAddress());
 }
 
-TEST_F(NameTest, LongFileName) {
+TEST_F(ParserTest, NameLongFileName) {
-  moduledef::Name *name = parse("NAME \"a long file name.exe\"");
+  parse("NAME \"a long file name.exe\"");
+  EXPECT_EQ(1U, _dirs.size());
+  auto *name = cast<moduledef::Name>(_dirs[0]);
   EXPECT_EQ("a long file name.exe", name->getOutputPath());
   EXPECT_EQ(0U, name->getBaseAddress());
 }
 
-TEST_F(VersionTest, Major) {
+TEST_F(ParserTest, VersionMajor) {
-  moduledef::Version *ver = parse("VERSION 12");
+  parse("VERSION 12");
+  EXPECT_EQ(1U, _dirs.size());
+  auto *ver = cast<moduledef::Version>(_dirs[0]);
   EXPECT_EQ(12, ver->getMajorVersion());
   EXPECT_EQ(0, ver->getMinorVersion());
 }
 
-TEST_F(VersionTest, MajorMinor) {
+TEST_F(ParserTest, VersionMajorMinor) {
-  moduledef::Version *ver = parse("VERSION 12.34");
+  parse("VERSION 12.34");
+  EXPECT_EQ(1U, _dirs.size());
+  auto *ver = cast<moduledef::Version>(_dirs[0]);
   EXPECT_EQ(12, ver->getMajorVersion());
   EXPECT_EQ(34, ver->getMinorVersion());
 }
+
+TEST_F(ParserTest, Multiple) {
+  parse("LIBRARY foo\n"
+        "EXPORTS sym\n"
+        "VERSION 12");
+  EXPECT_EQ(3U, _dirs.size());
+  auto *lib = cast<moduledef::Library>(_dirs[0]);
+  EXPECT_EQ("foo", lib->getName());
+
+  const std::vector<PECOFFLinkingContext::ExportDesc> &exports =
+      cast<moduledef::Exports>(_dirs[1])->getExports();
+  EXPECT_EQ(1U, exports.size());
+  verifyExportDesc(exports[0], "sym", -1, false, false);
+
+  auto *ver = cast<moduledef::Version>(_dirs[2]);
+  EXPECT_EQ(12, ver->getMajorVersion());
+}