<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Wed, Nov 20, 2013 at 7:59 PM, Saleem Abdulrasool <span dir="ltr"><<a href="mailto:compnerd@compnerd.org" target="_blank">compnerd@compnerd.org</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">
<div class="im">On Wed, Nov 20, 2013 at 5:08 PM, Rui Ueyama <span dir="ltr"><<a href="mailto:ruiu@google.com" target="_blank">ruiu@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
Author: ruiu<br>
Date: Wed Nov 20 19:08:53 2013<br>
New Revision: 195295<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=195295&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=195295&view=rev</a><br>
Log:<br>
[PECOFF] Move files with ".lib" extension to the end of the input file list.<br>
<br>
It's allowed to specify library files *before* object files in the command<br>
line. Object files seems to be processed first, and then their undefined<br>
symbols are resolved from the libraries. This patch implements the compatible<br>
behavior.<br></blockquote><div><br></div></div><div>All object files are considered for their contributions. Any unresolved symbols are searched for in the import libraries (which may contain object files, e.g {msv,}crt{,d}.lib provide entry point stubs via this method).</div>
<div><br></div><div>Reordering these concerns me slightly as even though you use a stable sort to preserve the ordering, it seems like you don't account for the fact that someone may directly link against a module rather than an import library. This can be problematic. You may end up resolving functions from the wrong source as a result of the reordering. If the unfortunate mis-resolved function is {Dll,Win}MainCRTStartup, you may end up invoking constructors multiple times for a given module, and none for another. Perhaps Im missing something subtle, but, this change, as is, seems like it can generate binaries which behave incorrectly and are tricky to diagnose what went wrong.</div>
</div></div></div></blockquote><div><br></div><div>As Mikael pointed out in a reply for r195289, this behavior is not very accurate as a Windows linker. It seems that in most cases I believe the LLD will create valid executables, but there could be a chance that it mis-link. We definitely need to revisit this to fix the link order.</div>
<div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">
<div><div class="h5"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
Modified:<br>
lld/trunk/lib/Driver/WinLinkDriver.cpp<br>
lld/trunk/unittests/DriverTests/WinLinkDriverTest.cpp<br>
<br>
Modified: lld/trunk/lib/Driver/WinLinkDriver.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/lld/trunk/lib/Driver/WinLinkDriver.cpp?rev=195295&r1=195294&r2=195295&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/lld/trunk/lib/Driver/WinLinkDriver.cpp?rev=195295&r1=195294&r2=195295&view=diff</a><br>
==============================================================================<br>
--- lld/trunk/lib/Driver/WinLinkDriver.cpp (original)<br>
+++ lld/trunk/lib/Driver/WinLinkDriver.cpp Wed Nov 20 19:08:53 2013<br>
@@ -13,6 +13,7 @@<br>
///<br>
//===----------------------------------------------------------------------===//<br>
<br>
+#include <algorithm><br>
#include <cctype><br>
#include <sstream><br>
#include <map><br>
@@ -600,6 +601,8 @@ WinLinkDriver::parse(int argc, const cha<br>
defaultLibs.push_back((*it)->getValue());<br>
}<br>
<br>
+ std::vector<StringRef> inputFiles;<br>
+<br>
// Process all the arguments and create Input Elements<br>
for (auto inputArg : *parsedArgs) {<br>
switch (inputArg->getOption().getID()) {<br>
@@ -850,8 +853,7 @@ WinLinkDriver::parse(int argc, const cha<br>
break;<br>
<br>
case OPT_INPUT:<br>
- inputElements.push_back(std::unique_ptr<InputElement>(<br>
- new PECOFFFileNode(ctx, ctx.allocate(inputArg->getValue()))));<br>
+ inputFiles.push_back(ctx.allocate(inputArg->getValue()));<br>
break;<br>
<br>
#define DEFINE_BOOLEAN_FLAG(name, setter) \<br>
@@ -877,6 +879,17 @@ WinLinkDriver::parse(int argc, const cha<br>
}<br>
}<br>
<br>
+ // Move files with ".lib" extension at the end of the input file list. Say<br>
+ // foo.obj depends on bar.lib. The linker needs to accept both "bar.lib<br>
+ // foo.obj" and "foo.obj bar.lib".<br>
+ auto compfn = [](StringRef a, StringRef b) {<br>
+ return !a.endswith_lower(".lib") && b.endswith_lower(".lib");<br>
+ };<br>
+ std::stable_sort(inputFiles.begin(), inputFiles.end(), compfn);<br>
+ for (StringRef path : inputFiles)<br>
+ inputElements.push_back(std::unique_ptr<InputElement>(<br>
+ new PECOFFFileNode(ctx, path)));<br>
+<br>
// Use the default entry name if /entry option is not given.<br>
if (ctx.entrySymbolName().empty())<br>
ctx.setEntrySymbolName(getDefaultEntrySymbolName(ctx));<br>
<br>
Modified: lld/trunk/unittests/DriverTests/WinLinkDriverTest.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/lld/trunk/unittests/DriverTests/WinLinkDriverTest.cpp?rev=195295&r1=195294&r2=195295&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/lld/trunk/unittests/DriverTests/WinLinkDriverTest.cpp?rev=195295&r1=195294&r2=195295&view=diff</a><br>
==============================================================================<br>
--- lld/trunk/unittests/DriverTests/WinLinkDriverTest.cpp (original)<br>
+++ lld/trunk/unittests/DriverTests/WinLinkDriverTest.cpp Wed Nov 20 19:08:53 2013<br>
@@ -131,6 +131,21 @@ TEST_F(WinLinkParserTest, Libpath) {<br>
}<br>
<br>
//<br>
+// Tests for input file order<br>
+//<br>
+<br>
+TEST_F(WinLinkParserTest, InputOrder) {<br>
+ EXPECT_TRUE(parse("link.exe", "b.lib", "b.obj", "c.obj", "a.lib", "a.obj",<br>
+ nullptr));<br>
+ EXPECT_EQ(5, inputFileCount());<br>
+ EXPECT_EQ("b.obj", inputFile(0));<br>
+ EXPECT_EQ("c.obj", inputFile(1));<br>
+ EXPECT_EQ("a.obj", inputFile(2));<br>
+ EXPECT_EQ("b.lib", inputFile(3));<br>
+ EXPECT_EQ("a.lib", inputFile(4));<br>
+}<br>
+<br>
+//<br>
// Tests for command line options that take values.<br>
//<br>
<br>
<br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">llvm-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
</blockquote></div></div></div><span class=""><font color="#888888"><br><br clear="all"><div><br></div>-- <br>Saleem Abdulrasool<br>compnerd (at) compnerd (dot) org
</font></span></div></div>
</blockquote></div><br></div></div>