<div dir="ltr"><div>Code mostly looks good, some nits.</div><div><br></div><div>> +// Sets a default entry point symbol name depending on context image type and</div><div>> +// subsystem. These default names are MS CRT compliant.</div>
<div>> +void setDefaultEntrySymbolName(PECOFFLinkingContext &context)</div><div>> +{</div><div>> + if (context.getImageType() == PECOFFLinkingContext::ImageType::IMAGE_DLL)</div><div>> + context.setEntrySymbolName("_DllMainCRTStartup");</div>
<div>> + else {</div><div><br></div><div>nit: we usually use {} if "else" has takes a block.</div><div><br></div><div>> + llvm::COFF::WindowsSubsystem subsystem = context.getSubsystem();</div><div>> + if (subsystem == llvm::COFF::WindowsSubsystem::IMAGE_SUBSYSTEM_WINDOWS_GUI)</div>
<div>> + context.setEntrySymbolName("WinMainCRTStartup");</div><div>> + else if (subsystem ==</div><div>> + llvm::COFF::WindowsSubsystem::IMAGE_SUBSYSTEM_WINDOWS_CUI)</div><div>> + context.setEntrySymbolName("mainCRTStartup");</div>
<div>> + }</div><div>> +}</div><div><br></div><div>You may want to add an underscore at the beginning of the symbols. That symbol</div><div>mangling is needed only on Windows/x86, so we will have to add "_" only when on</div>
<div>x86, but currently we are hard-coding the underscore.</div><div><br></div><div>> // Parses the given command line options and returns the result. Returns NULL if</div><div>> // there's an error in the options.</div>
<div>> std::unique_ptr<llvm::opt::InputArgList> parseArgs(int argc, const char *argv[],</div><div>> @@ -356,6 +372,8 @@ bool WinLinkDriver::parse(int argc, const char *argv[],</div><div>> // handle /entry</div>
<div>> if (llvm::opt::Arg *arg = parsedArgs->getLastArg(OPT_entry))</div><div>> context.setEntrySymbolName(arg->getValue());</div><div>> + else</div><div>> + setDefaultEntrySymbolName(context);</div>
<div><br></div><div>nit: brace</div><div><br></div><div>> + void setAddressOfEntryPoint(TextSectionChunk *text, PEHeaderChunk *peHeader) {</div><div>> + // Find the virtual address of the entry point symbol if any.</div>
<div>> + // PECOFF spec says that entry point for dll images is optional, in which</div><div>> + // case it must be set to 0.</div><div>> + if (_PECOFFLinkingContext.entrySymbolName().empty() &&</div>
<div>> + _PECOFFLinkingContext.getImageType() == PECOFFLinkingContext::IMAGE_DLL)</div><div>> + peHeader->setAddressOfEntryPoint(0);</div><div>> + else {</div><div><br></div><div>nit: brace</div>
<div><br></div></div><div class="gmail_extra"><br><br><div class="gmail_quote">On Mon, Aug 12, 2013 at 12:49 PM, Jesús Serrano <span dir="ltr"><<a href="mailto:dagonoweda@gmail.com" target="_blank">dagonoweda@gmail.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div text="#000000" bgcolor="#FFFFFF"><div><div class="h5">
<div>On 31/07/2013 2:13, Reid Kleckner
wrote:<br>
</div>
<blockquote type="cite">
<div dir="ltr">On Tue, Jul 30, 2013 at 4:12 PM, Chandler Carruth <span dir="ltr"><<a href="mailto:chandlerc@google.com" target="_blank">chandlerc@google.com</a>></span>
wrote:<br>
<div class="gmail_extra">
<div class="gmail_quote">
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div dir="ltr">
<div class="gmail_extra"><br>
<div class="gmail_quote">
<div>On Tue, Jul 30, 2013 at 3:46 PM,
Jesús Serrano García <span dir="ltr"><<a href="mailto:dagonoweda@gmail.com" target="_blank">dagonoweda@gmail.com</a>></span>
wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div>
<blockquote type="cite">
<div>Looks like the <span>entry</span>
function name should be _mainCRTStartup if
subsystem is "console",</div>
<div>_WinMainCRTStartup if subsystem is
"windows", and "__DllMainCRTStartup" for
DLL.</div>
<div>See <a href="http://msdn.microsoft.com/en-us/library/f9t8842e%28v=vs.80%29.aspx" target="_blank">http://msdn.microsoft.com/en-us/library/f9t8842e(v=vs.80).aspx</a>.</div>
</blockquote>
</div>
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 <span>entry</span>
<span>point</span> name, taking into account
that there is currently no CRT guidelines (or at
least I'm unaware of them) in the lld project.</blockquote>
<div><br>
</div>
</div>
<div>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.</div>
<div><br>
</div>
<div>For now, as Rui has said, we should follow the
MSDN spec he cited.</div>
</div>
</div>
</div>
</blockquote>
</div>
<br>
</div>
<div class="gmail_extra">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.</div>
</div>
</blockquote></div></div>
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<span lang="en"><span> whether</span></span>
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 <span lang="en"><span>possible
solution</span> <span>would be to have</span> two
entry point <span>names in the linking context</span></span>,
and find a symbol mathing any of them, but I don't like it so much.
Any ideas on this?<br>
<br>
Also, I'd like to add some tests for this feature, but I'm still
learning how the test system works.<br>
<br>
</div>
</blockquote></div><br></div>