[PATCH][lld][PECOFF] Add Support for entry point symbol name

Jesús Serrano dagonoweda at gmail.com
Tue Aug 13 10:56:10 PDT 2013


On 13/08/2013 0:02, Rui Ueyama wrote:
> We have unit tests for the driver in the following file. In the file, 
> we have bunch of tests for all the command line options. You could add 
> two new tests for the cases (1) if /entry is given or (2) if /entry is 
> not given.
>
> lld/unittests/DriverTests/WinLinkDriverTest.cpp
>
I've added 2 tests, both to check the default entry symbol name, one for 
console subsystem and another for windows subsystem. I've not added a 
test for the case the /entry is given since the Basic test already 
checks it.
Besides the driver tests, I'd like to add tests to check if the 
AddressOfEntryPoint is correctly setted in the PE Header of the 
generated image. I understand that such tests must be implemented in Lit 
test system.
>
> On Mon, Aug 12, 2013 at 2:58 PM, Rui Ueyama <ruiu at google.com 
> <mailto:ruiu at google.com>> wrote:
>
>     Code mostly looks good, some nits.
>
>     > +// Sets a default entry point symbol name depending on context
>     image type and
>     > +// subsystem. These default names are MS CRT compliant.
>     > +void setDefaultEntrySymbolName(PECOFFLinkingContext &context)
>     > +{
>     > +  if (context.getImageType() ==
>     PECOFFLinkingContext::ImageType::IMAGE_DLL)
>     > +  context.setEntrySymbolName("_DllMainCRTStartup");
>     > +  else {
>
>     nit: we usually use {} if "else" has takes a block.
>
>     > +    llvm::COFF::WindowsSubsystem subsystem =
>     context.getSubsystem();
>     > +    if (subsystem ==
>     llvm::COFF::WindowsSubsystem::IMAGE_SUBSYSTEM_WINDOWS_GUI)
>     > +  context.setEntrySymbolName("WinMainCRTStartup");
>     > +    else if (subsystem ==
>     > +  llvm::COFF::WindowsSubsystem::IMAGE_SUBSYSTEM_WINDOWS_CUI)
>     > +  context.setEntrySymbolName("mainCRTStartup");
>     > +  }
>     > +}
>
>     You may want to add an underscore at the beginning of the symbols.
>     That symbol
>     mangling is needed only on Windows/x86, so we will have to add "_"
>     only when on
>     x86, but currently we are hard-coding the underscore.
>
>     >  // Parses the given command line options and returns the
>     result. Returns NULL if
>     >  // there's an error in the options.
>     >  std::unique_ptr<llvm::opt::InputArgList> parseArgs(int argc,
>     const char *argv[],
>     > @@ -356,6 +372,8 @@ bool WinLinkDriver::parse(int argc, const
>     char *argv[],
>     >    // handle /entry
>     >    if (llvm::opt::Arg *arg = parsedArgs->getLastArg(OPT_entry))
>     >  context.setEntrySymbolName(arg->getValue());
>     > +  else
>     > +    setDefaultEntrySymbolName(context);
>
>     nit: brace
>
>     > +  void setAddressOfEntryPoint(TextSectionChunk *text,
>     PEHeaderChunk *peHeader) {
>     > +    // Find the virtual address of the entry point symbol if any.
>     > +    // PECOFF spec says that entry point for dll images is
>     optional, in which
>     > +    // case it must be set to 0.
>     > +    if (_PECOFFLinkingContext.entrySymbolName().empty() &&
>     > +        _PECOFFLinkingContext.getImageType() ==
>     PECOFFLinkingContext::IMAGE_DLL)
>     > +      peHeader->setAddressOfEntryPoint(0);
>     > +    else {
>
>     nit: brace
>

Corrections made in the attached patch.

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130813/cf9a75a0/attachment.html>
-------------- next part --------------
diff --git include/lld/ReaderWriter/PECOFFLinkingContext.h include/lld/ReaderWriter/PECOFFLinkingContext.h
index 7112126..a6ea859 100644
--- include/lld/ReaderWriter/PECOFFLinkingContext.h
+++ include/lld/ReaderWriter/PECOFFLinkingContext.h
@@ -29,7 +29,8 @@ public:
         _heapReserve(1024 * 1024), _heapCommit(4096),
         _subsystem(llvm::COFF::IMAGE_SUBSYSTEM_UNKNOWN), _minOSVersion(6, 0),
         _nxCompat(true), _largeAddressAware(false),
-        _baseRelocationEnabled(true), _terminalServerAware(true) {}
+        _baseRelocationEnabled(true), _terminalServerAware(true),
+        _imageType(ImageType::IMAGE_EXE) {}
 
   struct OSVersion {
     OSVersion(int v1, int v2) : majorVersion(v1), minorVersion(v2) {}
@@ -37,6 +38,11 @@ public:
     int minorVersion;
   };
 
+  enum ImageType {
+    IMAGE_EXE,
+    IMAGE_DLL
+  };
+
   virtual error_code
   parseFile(std::unique_ptr<MemoryBuffer> &mb,
             std::vector<std::unique_ptr<File> > &result) const;
@@ -93,6 +99,9 @@ public:
   virtual ErrorOr<Reference::Kind> relocKindFromString(StringRef str) const;
   virtual ErrorOr<std::string> stringFromRelocKind(Reference::Kind kind) const;
 
+  void setImageType(ImageType type) { _imageType = type; }
+  ImageType getImageType() const { return _imageType; }
+
   StringRef allocateString(const StringRef &ref) {
     char *x = _alloc.Allocate<char>(ref.size() + 1);
     memcpy(x, ref.data(), ref.size());
@@ -115,6 +124,7 @@ private:
   bool _largeAddressAware;
   bool _baseRelocationEnabled;
   bool _terminalServerAware;
+  ImageType _imageType;
 
   std::vector<StringRef> _inputSearchPaths;
   mutable std::unique_ptr<Reader> _reader;
diff --git lib/Driver/WinLinkDriver.cpp lib/Driver/WinLinkDriver.cpp
index d7ee924..7d8edde 100644
--- lib/Driver/WinLinkDriver.cpp
+++ lib/Driver/WinLinkDriver.cpp
@@ -270,6 +270,22 @@ void processLibEnv(PECOFFLinkingContext &context) {
       context.appendInputSearchPath(context.allocateString(path));
 }
 
+// Sets a default entry point symbol name depending on context image type and
+// subsystem. These default names are MS CRT compliant.
+void setDefaultEntrySymbolName(PECOFFLinkingContext &context)
+{
+  if (context.getImageType() == PECOFFLinkingContext::ImageType::IMAGE_DLL) {
+    context.setEntrySymbolName("__DllMainCRTStartup");
+  } else {
+    llvm::COFF::WindowsSubsystem subsystem = context.getSubsystem();
+    if (subsystem == llvm::COFF::WindowsSubsystem::IMAGE_SUBSYSTEM_WINDOWS_GUI)
+      context.setEntrySymbolName("_WinMainCRTStartup");
+    else if (subsystem == 
+      llvm::COFF::WindowsSubsystem::IMAGE_SUBSYSTEM_WINDOWS_CUI)
+      context.setEntrySymbolName("_mainCRTStartup");
+  }
+}
+
 // Parses the given command line options and returns the result. Returns NULL if
 // there's an error in the options.
 std::unique_ptr<llvm::opt::InputArgList> parseArgs(int argc, const char *argv[],
@@ -354,8 +370,11 @@ bool WinLinkDriver::parse(int argc, const char *argv[],
       return true;
 
   // handle /entry
-  if (llvm::opt::Arg *arg = parsedArgs->getLastArg(OPT_entry))
+  if (llvm::opt::Arg *arg = parsedArgs->getLastArg(OPT_entry)) {
     context.setEntrySymbolName(arg->getValue());
+  } else {
+    setDefaultEntrySymbolName(context);
+  }
 
   // handle /libpath
   for (llvm::opt::arg_iterator it = parsedArgs->filtered_begin(OPT_libpath),
diff --git lib/ReaderWriter/PECOFF/WriterPECOFF.cpp lib/ReaderWriter/PECOFF/WriterPECOFF.cpp
index f9402e2..31c1ca2 100644
--- lib/ReaderWriter/PECOFF/WriterPECOFF.cpp
+++ lib/ReaderWriter/PECOFF/WriterPECOFF.cpp
@@ -248,6 +248,10 @@ public:
 
   virtual void setSizeOfImage(uint32_t size) { _peHeader.SizeOfImage = size; }
 
+  virtual void setAddressOfEntryPoint(uint32_t address) { 
+    _peHeader.AddressOfEntryPoint = address; 
+  }
+
 private:
   llvm::object::coff_file_header _coffHeader;
   llvm::object::pe32_header _peHeader;
@@ -356,6 +360,13 @@ public:
       layout->_virtualAddr += rva;
   }
 
+  uint64_t getAtomVirtualAddress(StringRef name) {
+    for (auto atomLayout : _atomLayouts)
+      if (atomLayout->_atom->name() == name)
+        return atomLayout->_virtualAddr;
+    return 0;
+  }
+
   static bool classof(const Chunk *c) {
     Kind kind = c->getKind();
     return kind == kindSection || kind == kindDataDirectory;
@@ -794,6 +805,8 @@ public:
     peHeader->setSizeOfUninitializedData(bss->size());
     peHeader->setNumberOfSections(_numSections);
     peHeader->setSizeOfImage(_imageSizeInMemory);
+
+    setAddressOfEntryPoint(text, peHeader);
   }
 
   virtual error_code writeFile(const File &linkedFile, StringRef path) {
@@ -853,6 +866,22 @@ private:
     }
   }
 
+  void setAddressOfEntryPoint(TextSectionChunk *text, PEHeaderChunk *peHeader) {
+    // Find the virtual address of the entry point symbol if any.
+    // PECOFF spec says that entry point for dll images is optional, in which
+    // case it must be set to 0.
+    if (_PECOFFLinkingContext.entrySymbolName().empty() && 
+        _PECOFFLinkingContext.getImageType() 
+                              == PECOFFLinkingContext::IMAGE_DLL) {
+      peHeader->setAddressOfEntryPoint(0);
+    } else {
+      uint64_t entryPointAddress = text->getAtomVirtualAddress(
+        _PECOFFLinkingContext.entrySymbolName()); 
+      if (entryPointAddress != 0)
+        peHeader->setAddressOfEntryPoint(entryPointAddress);
+    }
+  }
+
   std::vector<std::unique_ptr<Chunk>> _chunks;
   const PECOFFLinkingContext &_PECOFFLinkingContext;
   uint32_t _numSections;
diff --git unittests/DriverTests/WinLinkDriverTest.cpp unittests/DriverTests/WinLinkDriverTest.cpp
index d048c89..42a9083 100644
--- unittests/DriverTests/WinLinkDriverTest.cpp
+++ unittests/DriverTests/WinLinkDriverTest.cpp
@@ -221,4 +221,14 @@ TEST_F(WinLinkParserTest, Nologo) {
   EXPECT_EQ("a.obj", inputFile(0));
 }
 
+TEST_F(WinLinkParserTest, DefEntryNameConsole) {
+  EXPECT_FALSE(parse("link.exe", "/subsystem:console", "a.obj", nullptr));
+  EXPECT_EQ("_mainCRTStartup", _context.entrySymbolName());
+}
+
+TEST_F(WinLinkParserTest, DefEntryNameWindows) {
+  EXPECT_FALSE(parse("link.exe", "/subsystem:windows", "a.obj", nullptr));
+  EXPECT_EQ("_WinMainCRTStartup", _context.entrySymbolName());
+}
+
 } // end anonymous namespace


More information about the llvm-commits mailing list