Add '-emit-archive' option to mark modules as being part of static libraries #25088

troughton · 2019-05-28T10:34:35Z

Following discussion here, this is a draft PR to add a -emit-archive option to Swift that controls the linkage of symbols within Swift modules (and potentially more in the future).

The core problem is that Windows needs different linkage – DLL import vs. default – for shared vs. static libraries. For Swift, cache whether a module is built as a static or shared library on a per-module basis, and use that to determine what linkage should be applied.

cc @compnerd

gottesmm · 2019-05-28T17:38:42Z

@jrose-apple

compnerd · 2019-05-28T20:15:04Z

This isn't even really ready to look at - this cannot generate static libraries yet as clang does not know how to generate static libraries.

harlanhaskins · 2019-05-28T20:40:20Z

include/swift/Option/Options.td

@@ -368,6 +368,10 @@ def emit_objc_header_path : Separate<["-"], "emit-objc-header-path">,
         ArgumentIsPath]>,
  MetaVarName<"<path>">, HelpText<"Emit an Objective-C header file to <path>">;

+def emit_archive : Flag<["-"], "emit-archive">,
+  Flags<[FrontendOption, NoInteractiveOption, DoesNotAffectIncrementalBuild]>,


I'll leave it to @jrose-apple to decide if this is how this should happen, but if this is going to affect the module, then it should go in FrontendOptions and have the ModuleInterfaceOption flag added, so it is included in the set of command-line flags that go into a .swiftinterface file.

It's a driver flag as well as a frontend flag, so this is the correct file, but Harlan is correct that it is a ModuleInterfaceOption, and it certainly does affect the incremental build.

If ModuleInterfaceOption ensures it's automatically serialised, does that mean the IsStaticLibrary flag I've added elsewhere is redundant?

More specifically: I see that CompilerInstance::getMainModule copies the frontend arguments to the main module; does that also apply to deserialised modules or are the options_block::IS_STATIC_LIBRARY and serialization::ExtendedValidationInfo changes necessary?

troughton · 2019-05-28T22:49:30Z

This isn't even really ready to look at - this cannot generate static libraries yet as clang does not know how to generate static libraries.

The main reasons I put this up were to make sure I was adding the flag in the right place, have the serialisation logic checked, and to have the DLL storage changes looked over.

In terms of generating static libraries:

Would the invocation ar crs outputLibraryName objectFiles... work on every platform?
Do you think it'd be worth searching for llvm-ar and prioritising that, or just assuming the user has a sufficiently-modern ar tool installed?
How does the ar invocation interact with the linker invocation? Can we still use clang to call the linker and then call ar on the result? If so, how do you invoke clang in such a way that it doesn't try to build an executable?

compnerd · 2019-05-29T00:43:31Z

Ish - that needs a bit of a tweak to be truly portable. BSD uses a BSD style archive and you need to indicate that (as llvm-ar --format=bsd) and most others can do --format=gnu. I believe that Darwin can get away with the BSD archive, though llvm-ar does have a special darwin format as well. That said, if we do not find llvm-ar, we should fall back to ar which should be fine except when cross-compiling where the host is BSD (or a derivative cough Darwin cough) and the target is a non-BSD target or the inverse. I think that searching explicitly for llvm-ar makes sense and should be relatively cheap.

troughton · 2019-05-29T03:56:50Z

I've updated the PR with a minimum-viable invocation of ar. It still needs tests, but I've tried locally and am able to successfully produce a static library.

If the implementation looks good, I'll make a start on tests – just want to make sure I'm not testing incorrect behaviour.

jrose-apple

This looks pretty reasonable to me, but I suggest breaking up the Driver and Frontend parts into two PRs. We can get static linking working on Linux and macOS first (or even just Linux for now), then move on to Windows.

jrose-apple · 2019-05-30T15:38:28Z

include/swift/Option/Options.td

@@ -368,6 +368,10 @@ def emit_objc_header_path : Separate<["-"], "emit-objc-header-path">,
         ArgumentIsPath]>,
  MetaVarName<"<path>">, HelpText<"Emit an Objective-C header file to <path>">;

+def static_library : Flag<["-"], "static">,


Nitpick: the name of the option should match the name of the flag. (It'll get concatenated to OPT_ so it shouldn't collide. Unless you already tried it and I'm wrong?)

jrose-apple · 2019-05-30T15:39:30Z

lib/Driver/Driver.cpp

@@ -1371,10 +1373,15 @@ void Driver::buildOutputInfo(const ToolChain &TC, const DerivedArgList &Args,
      break;

    case options::OPT_emit_library:
-      OI.LinkAction = LinkKind::DynamicLibrary;
+      OI.LinkAction = Args.hasArg(options::OPT_static_library) ?


It's probably worth complaining when using -static with -emit-executable.

jrose-apple · 2019-05-30T15:40:20Z

include/swift/Driver/Action.h

+    assert(K == LinkKind::StaticLibrary);
+  }
+
+  LinkKind getKind() const { return LinkKind::StaticLibrary; }


You probably don't need this. I appreciate it being passed in to the constructor, though.

lib/Driver/Driver.cpp

jrose-apple · 2019-05-30T15:45:44Z

lib/Driver/ToolChains.cpp

+
+  }
+  if (AR == nullptr) {
+    if (auto pathAR = llvm::sys::findProgramByName("llvm-ar", None)) {


I'm not sure we actually want to prefer llvm-ar like this, and I'm not sure whether this belongs in the default ToolChain either (rather than GenericUnix). On Apple platforms, libtool -static is what Xcode uses to create static libraries, not ar.

@cooperp, what's the current recommendation on this? ar or libtool for static libraries on Apple platforms?

I believe it's libtool. Otherwise you tend to have to call ranlib after ar, while the man page for libtool says it can handle both for you: "Libtool with -static is intended to replace ar(5) and ranlib."

@compnerd, is what’s here what we want on both GenericUnix and Windows, or should the behaviour differ? If llvm-ar is unavailable on Windows should we fall back to LIB.exe?

@cooperp - thats the trick wrt ar crs - it creates the index as part of the invocation. With llvm-ar that also means that we can invoke it as ar --format bsd crs ... which will create the BSD style archive with the index even on a non-BSD host.

Xcode doesn't ship with llvm-ar. Please stick to libtool on Darwin. (In general, please stick to "what Xcode does when you make a static library from a template".)

Sure, but it does ship with ar from cctools, which also supports the crs mode. The problem is that you lose compatibility with AT&T Unix, which I believe didn't support the s option.

Hm. I'll let @cooperp decide what's right here. We already aren't getting code sharing because the Darwin toolchain deliberately isn't a GenericUnix toolchain, on account of Apple ~~doing weird things~~ having more control over the tools it ships.

@cooperp, what do you think is best here?

jrose-apple · 2019-05-30T15:46:00Z

lib/Driver/ToolChains.cpp

+
+  if (isLLVMAR) {
+    switch (getTriple().getOS()) {
+      case llvm::Triple::Darwin:


Formatting nitpick: no indentation for switch cases.

jrose-apple · 2019-05-31T00:11:51Z

lib/Driver/Driver.cpp

@@ -1368,18 +1368,20 @@ void Driver::buildOutputInfo(const ToolChain &TC, const DerivedArgList &Args,

    switch (OutputModeArg->getOption().getID()) {
    case options::OPT_emit_executable:
+      assert(!Args.hasArg(options::OPT_static) &&
+             "-static may not be used with -emit-executable.");


This shouldn't be an assertion; it should be a proper diagnostic somewhere for a user who doesn't know.

troughton · 2019-05-31T23:37:12Z

I think this is almost ready for tests (and splitting out the frontend parts into a separate PR). However, I'm still not sure about what we want to do for llvm-ar vs. ar vs. libtoolvs. lib.exe. The approach I'm thinking makes most sense is to just use each platform's default (ar for GenericUnix, libtool for Darwin, and lib.exe for Windows).

That has some issues for Linux/Windows <-> Darwin cross-compilation; however, since the Linux and Windows archive formats are apparently the same Linux <-> Windows should just work with the default linker. Not sure what to do about the Darwin case – we could maybe use llvm-ar as a fallback?

jrose-apple · 2019-06-01T01:22:03Z

Let's worry about cross-compilation later. Using swiftc to link when cross-compiling for Darwin also assumes you have dsymutil if you're using debug info, not to mention an ld that supports Mach-O and -add_ast_path.

troughton · 2019-06-02T06:52:47Z

The driver portion of this PR has been extracted into #25202

compnerd · 2019-06-03T00:37:40Z

@jrose-apple - dsymutil is now llvm-dsymutil which is present :-)

jrose-apple · 2019-06-12T16:35:28Z

@jrose-apple - dsymutil is now llvm-dsymutil which is present :-)

It might be present. It's not installed by default, and I don't think we want it to be. Nor do I think we should automatically look for LLVM variants of tools; it'd be better to just allow overriding them in general.

troughton · 2019-06-13T00:13:47Z

Do we currently have a general way of overriding the tools? I'm only aware of -use-ld, which isn't exactly the same situation – for static libraries we need to find the archive tool ourselves, whereas -use-ld just passes its argument through to Clang.

I'm also perfectly happy (and would probably prefer) to just drop support from anything other than the system default tools in the PR rather than trying to scope-creep a more general solution.

jrose-apple · 2019-06-13T00:38:32Z

Yeah, I was more talking about future expansion. I only care about the system tools. @compnerd, however, has been cultivating cross-compilation stacks for a while now, so I see the tendency to not box ourselves into a situation where we can't support cross-compilation.

keith · 2019-06-13T00:45:27Z

FWIW swiftpm (llbuild) uses ar to create libraries. Should we change that to use libtool on macOS as mentioned above, or is ar an acceptable choice for here too? (Since this was discussed above)

CodaFi · 2020-04-15T00:02:57Z

@troughton Are you still pursuing this?

troughton · 2020-04-15T00:07:45Z

@CodaFi This is still necessary if we want to support static linking on Windows, but that's not something I've been actively pursuing recently, and the remainder of the patch (since #25202 was merged) is fairly minimal. I'm fine with this PR being closed if that's what you're checking.

CodaFi · 2020-04-15T00:33:00Z

Thanks. I can file an SR for somebody to finish this up of you’d like.

troughton · 2020-04-15T00:36:25Z

There's already https://bugs.swift.org/browse/SR-10797 filed which was the main blocking piece of work. Once that's fixed (assuming it hasn't been already) I can revisit this patch, but I'll close it for now.

harlanhaskins reviewed May 28, 2019

View reviewed changes

troughton force-pushed the static-libraries branch 2 times, most recently from 4b7e6c1 to 9fd991d Compare May 29, 2019 03:36

troughton added 7 commits May 30, 2019 11:59

Add support for building static libraries.

0b7369c

Add driver support for LinkKind::StaticLibrary

ff3498c

emit_archive -> static_library

d0eb649

Add ArchiveJobAction for producing static libraries.

2af0bae

Move a comment for clarity

bdcac7f

Fix issues with the ar invocation

2421f37

Add FreeBSD as an AR format

d82e47a

troughton force-pushed the static-libraries branch from b86c603 to d82e47a Compare May 29, 2019 23:59

jrose-apple reviewed May 30, 2019

View reviewed changes

Adjust for review comments.

3463e5d

jrose-apple reviewed May 31, 2019

View reviewed changes

Use a proper diagnostic for -emit-executable -static

248b0f7

troughton mentioned this pull request Jun 2, 2019

[Driver] Add -static flag for generating static archives #25202

Merged

troughton closed this Apr 15, 2020

Add '-emit-archive' option to mark modules as being part of static libraries #25088

Add '-emit-archive' option to mark modules as being part of static libraries #25088

Uh oh!

Conversation

troughton commented May 28, 2019

Uh oh!

gottesmm commented May 28, 2019

Uh oh!

compnerd commented May 28, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

troughton commented May 28, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

compnerd commented May 29, 2019

Uh oh!

troughton commented May 29, 2019

Uh oh!

jrose-apple left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

troughton May 30, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jrose-apple Jun 3, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

troughton commented May 31, 2019

Uh oh!

jrose-apple commented Jun 1, 2019

Uh oh!

troughton commented Jun 2, 2019

Uh oh!

compnerd commented Jun 3, 2019

Uh oh!

jrose-apple commented Jun 12, 2019

Uh oh!

troughton commented Jun 13, 2019

Uh oh!

jrose-apple commented Jun 13, 2019

Uh oh!

keith commented Jun 13, 2019

Uh oh!

CodaFi commented Apr 15, 2020

Uh oh!

troughton commented Apr 15, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

troughton commented May 28, 2019 •

edited

Loading

jrose-apple left a comment •

edited

Loading

troughton May 30, 2019 •

edited

Loading

jrose-apple Jun 3, 2019 •

edited

Loading

troughton commented Apr 15, 2020 •

edited

Loading

troughton commented Apr 15, 2020 •

edited

Loading