[lld] r195295 - [PECOFF] Move files with ".lib" extension to the end of the input file list.

[Saleem Abdulrasool]

On Wed, Nov 20, 2013 at 5:08 PM, Rui Ueyama <ruiu at google.com> wrote:

> Author: ruiu
> Date: Wed Nov 20 19:08:53 2013
> New Revision: 195295
>
> URL: http://llvm.org/viewvc/llvm-project?rev=195295&view=rev
> Log:
> [PECOFF] Move files with ".lib" extension to the end of the input file
> list.
>
> It's allowed to specify library files *before* object files in the command
> line. Object files seems to be processed first, and then their undefined
> symbols are resolved from the libraries. This patch implements the
> compatible
> behavior.
>

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).

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.

Modified:
>     lld/trunk/lib/Driver/WinLinkDriver.cpp
>     lld/trunk/unittests/DriverTests/WinLinkDriverTest.cpp
>
> Modified: lld/trunk/lib/Driver/WinLinkDriver.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/lld/trunk/lib/Driver/WinLinkDriver.cpp?rev=195295&r1=195294&r2=195295&view=diff
>
> ==============================================================================
> --- lld/trunk/lib/Driver/WinLinkDriver.cpp (original)
> +++ lld/trunk/lib/Driver/WinLinkDriver.cpp Wed Nov 20 19:08:53 2013
> @@ -13,6 +13,7 @@
>  ///
>
>  //===----------------------------------------------------------------------===//
>
> +#include <algorithm>
>  #include <cctype>
>  #include <sstream>
>  #include <map>
> @@ -600,6 +601,8 @@ WinLinkDriver::parse(int argc, const cha
>      defaultLibs.push_back((*it)->getValue());
>    }
>
> +  std::vector<StringRef> inputFiles;
> +
>    // Process all the arguments and create Input Elements
>    for (auto inputArg : *parsedArgs) {
>      switch (inputArg->getOption().getID()) {
> @@ -850,8 +853,7 @@ WinLinkDriver::parse(int argc, const cha
>        break;
>
>      case OPT_INPUT:
> -      inputElements.push_back(std::unique_ptr<InputElement>(
> -          new PECOFFFileNode(ctx, ctx.allocate(inputArg->getValue()))));
> +      inputFiles.push_back(ctx.allocate(inputArg->getValue()));
>        break;
>
>  #define DEFINE_BOOLEAN_FLAG(name, setter)       \
> @@ -877,6 +879,17 @@ WinLinkDriver::parse(int argc, const cha
>      }
>    }
>
> +  // Move files with ".lib" extension at the end of the input file list.
> Say
> +  // foo.obj depends on bar.lib. The linker needs to accept both "bar.lib
> +  // foo.obj" and "foo.obj bar.lib".
> +  auto compfn = [](StringRef a, StringRef b) {
> +    return !a.endswith_lower(".lib") && b.endswith_lower(".lib");
> +  };
> +  std::stable_sort(inputFiles.begin(), inputFiles.end(), compfn);
> +  for (StringRef path : inputFiles)
> +    inputElements.push_back(std::unique_ptr<InputElement>(
> +        new PECOFFFileNode(ctx, path)));
> +
>    // Use the default entry name if /entry option is not given.
>    if (ctx.entrySymbolName().empty())
>      ctx.setEntrySymbolName(getDefaultEntrySymbolName(ctx));
>
> Modified: lld/trunk/unittests/DriverTests/WinLinkDriverTest.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/lld/trunk/unittests/DriverTests/WinLinkDriverTest.cpp?rev=195295&r1=195294&r2=195295&view=diff
>
> ==============================================================================
> --- lld/trunk/unittests/DriverTests/WinLinkDriverTest.cpp (original)
> +++ lld/trunk/unittests/DriverTests/WinLinkDriverTest.cpp Wed Nov 20
> 19:08:53 2013
> @@ -131,6 +131,21 @@ TEST_F(WinLinkParserTest, Libpath) {
>  }
>
>  //
> +// Tests for input file order
> +//
> +
> +TEST_F(WinLinkParserTest, InputOrder) {
> +  EXPECT_TRUE(parse("link.exe", "b.lib", "b.obj", "c.obj", "a.lib",
> "a.obj",
> +                    nullptr));
> +  EXPECT_EQ(5, inputFileCount());
> +  EXPECT_EQ("b.obj", inputFile(0));
> +  EXPECT_EQ("c.obj", inputFile(1));
> +  EXPECT_EQ("a.obj", inputFile(2));
> +  EXPECT_EQ("b.lib", inputFile(3));
> +  EXPECT_EQ("a.lib", inputFile(4));
> +}
> +
> +//
>  // Tests for command line options that take values.
>  //
>
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>



-- 
Saleem Abdulrasool
compnerd (at) compnerd (dot) org
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20131120/81d59144/attachment.html>