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

[Rui Ueyama]

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


On Mon, Aug 12, 2013 at 2:58 PM, Rui Ueyama <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
>
>
>
> On Mon, Aug 12, 2013 at 12:49 PM, Jesús Serrano <dagonoweda at gmail.com>wrote:
>
>>  On 31/07/2013 2:13, Reid Kleckner wrote:
>>
>> On Tue, Jul 30, 2013 at 4:12 PM, Chandler Carruth <chandlerc at google.com>wrote:
>>
>>>
>>>  On Tue, Jul 30, 2013 at 3:46 PM, Jesús Serrano García <
>>> dagonoweda at gmail.com> wrote:
>>>
>>>>  Looks like the entry function name should be _mainCRTStartup if
>>>> subsystem is "console",
>>>> _WinMainCRTStartup if subsystem is "windows", and "__DllMainCRTStartup"
>>>> for DLL.
>>>> See http://msdn.microsoft.com/en-us/library/f9t8842e(v=vs.80).aspx.
>>>>
>>>>  I think this is rather a Visual Studio convention for its C runtime
>>>> library, and the PECOFF spec says nothing about it, so I though that
>>>> "_main" is a good default entry point name, taking into account that
>>>> there is currently no CRT guidelines (or at least I'm unaware of them) in
>>>> the lld project.
>>>
>>>
>>>  Sorry for the ambiguity, but the goal of all the PECOFF work in LLD is
>>> to support linking in a compatible way for Windows. If you have another
>>> usecase for linking PECOFF, you should raise that on the discussion list
>>> (not the commit list) as it'll need a separate design from what we're
>>> currently working on.
>>>
>>>  For now, as Rui has said, we should follow the MSDN spec he cited.
>>>
>>
>>  Maybe the driver should call setEntrySymbolName() with a default if the
>> user didn't specify anything.  Then the link.exe driver could use the
>> Microsoft name and the GNU ld driver can use main / _main.
>>
>> I've attached a new patch with this changes. Now the windows driver sets
>> the entry symbol name according to the corresponding options. However, this
>> won't work in all cases. Depending on whether the crt is compiled as
>> multibyte or unicode, the entry point symbol name changes from
>> "mainCRTStartup" to "wmainCRTStartup", and the same for the windows
>> subsystem. The problem with this is that the use of unicode is unknown at
>> link time (or at least I don't know how to retrieve this option from object
>> files). One possible solution would be to have two entry point names in
>> the linking context, and find a symbol mathing any of them, but I don't
>> like it so much. Any ideas on this?
>>
>> Also, I'd like to add some tests for this feature, but I'm still learning
>> how the test system works.
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130812/7a5024f4/attachment.html>