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

[Rui Ueyama]

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/12991759/attachment.html>