[SR-2660] Teach the driver to accept multiple swiftmodules as linker inputs, for static linking

[trfiala]

Previous ID	SR-2660
Radar	rdar://problem/34795599
Original Reporter	@trfiala
Type	Bug
Status	Resolved
Resolution	Done

Attachment: Download

Environment

Apple Swift version 3.0 (swiftlang-800.0.46.2 clang-800.0.38)
Target: x86_64-apple-macosx10.9

Additional Detail from JIRA

Votes	0
Component/s	Compiler
Labels	Bug, Driver, StarterBug
Assignee	@JDevlieghere
Priority	Medium

md5: 6b6c547b0589245db749baf930bc2e1b

blocks:

• SR-3280 Package manager should link main swiftmodule in executables

cloned from:

• SR-2637 Issue with debugging programs that contain multiple Swift modules

is duplicated by:

• SR-3863 swift emits a temporary swiftmodule when swiftmodule of main module is passed with -emit-executable

Issue Description:

Suppose we have an Objective-C program that uses two Swift modules. We would like to debug this program with LLDB without having a dSYM for faster builds.

                         ┌───────┐
                      ┌──│ Swift │
┌───────┐   ┌───────┐ │  └───────┘
│  Bin  │◀──│ ObjC  │◀┤           
└───────┘   └───────┘ │  ┌───────┐
                      └──│ Swift │
                         └───────┘

Each Swift module consist of a .swiftmodule, .o, and the .h header.

swiftc -c -g src/foo.swift \
  -module-name Foo \
  -emit-module-path out/Foo.swiftmodule \
  -emit-objc-header-path out/Foo-Swift.h \
  ...

We then link the two Swift objects and Objective-C objects into the final binary. It looks like LLDB needs to have .swiftmodule information for debugging, so we include references as AST entries.

clang src/main.m -fobjc-arc -o out/main out/foo.o out/bar.o \
  -Xlinker -add_ast_path -Xlinker out/Foo.swiftmodule \
  -Xlinker -add_ast_path -Xlinker out/Bar.swiftmodule
  ...

When we run LLDB on this binary and set breakpoints inside of the code for Foo and Bar modules, only the Foo types are picked up by LLDB. It only reads the first AST reference, ignoring others. If we swap the order of add_ast_path options we get symbols from the other module.

The full build is in attached zip file. Unpack and run build.sh. In its output, you will see that frame variable self command only correctly prints the variable for one module, not both.

[trfiala]

@belkadan, this is the JIRA for the front-end work you wanted to consider for better handling command line options for this. I'll let you fill in the blanks with exactly what you wanted. One of the items discussed was adding whatever was needed on the driver side to properly spit out linker options that could be consumed by the appropriate linker.

[belkadan]

Thanks, Todd. I think the first goal is to teach the driver that swiftmodule inputs should only be merged if you pass -emit-module, and that they should be considered linker inputs otherwise. We can treat the "plug me into another linker" part separately.

[belkadan]

Tagging as StarterBug to increase the chances of someone getting to it before me, though for this one I don't think it's necessary to save it for a newcomer. This is a slightly ambitious starter bug, but the steps aren't too hard:

• Look at how the output from the MergeModule build step is being fed into the linker, and just pass inputs that way. This will break the module merging behavior, but that's fine.

• Add a check to not do that if -emit-module is passed, and instead to treat them as MergeModule inputs.

• Test on Linux and Darwin.

I'm sure it's not that easy in practice, but it seems digestible.

[modocache]

I've been trying to learn more about lib/Driver, and this seems like a meaty task that's too good to pass up. I'll start working on this tonight. Of course, others should feel free to grab it from me if they'd like. 🙂

[modocache]

Sorry for the slow progress here – it took me a while to understand the goals of the task.

Look at how the output from the MergeModule build step is being fed into the linker, and just pass inputs that way.

The output from a "merge module" action is used as the input to a link action when producing a library:

$ swiftc -c -g Foo.swift -emit-library -driver-print-actions
0: input, "Foo.swift", swift
1: compile, {0}, object
2: merge-module, {1}, swiftmodule
3: link, {1, 2}, image
4: generate-dSYM, {3}, dSYM

We can see how the input is passed to the linker by using -driver-print-jobs:

$ swiftc -c -g Foo.swift -emit-library -driver-print-jobs
/Users/bgesiak/GitHub/apple/build/Ninja-DebugAssert/swift-macosx-x86_64/bin/swift -frontend -c -primary-file Foo.swift -target x86_64-apple-macosx10.9 -enable-objc-interop -g -emit-module-doc-path /var/folders/ry/2ryfdsb56b30092626qprw6d3rb3ss/T/Foo-4c5e4c.swiftdoc -color-diagnostics -parse-as-library -module-name Foo -emit-module-path /var/folders/ry/2ryfdsb56b30092626qprw6d3rb3ss/T/Foo-4c5e4c.swiftmodule -o /var/folders/ry/2ryfdsb56b30092626qprw6d3rb3ss/T/Foo-4c5e4c.o
/Users/bgesiak/GitHub/apple/build/Ninja-DebugAssert/swift-macosx-x86_64/bin/swift -frontend -emit-module /var/folders/ry/2ryfdsb56b30092626qprw6d3rb3ss/T/Foo-4c5e4c.swiftmodule -parse-as-library -target x86_64-apple-macosx10.9 -enable-objc-interop -g -emit-module-doc-path /var/folders/ry/2ryfdsb56b30092626qprw6d3rb3ss/T/Foo-acf884.swiftdoc -color-diagnostics -module-name Foo -o /var/folders/ry/2ryfdsb56b30092626qprw6d3rb3ss/T/Foo-acf884.swiftmodule
/usr/bin/ld /var/folders/ry/2ryfdsb56b30092626qprw6d3rb3ss/T/Foo-4c5e4c.o -add_ast_path /var/folders/ry/2ryfdsb56b30092626qprw6d3rb3ss/T/Foo-acf884.swiftmodule -dylib -force_load /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/lib/arc/libarclite_macosx.a -framework CoreFoundation -lobjc -lSystem -arch x86_64 -L /Users/bgesiak/GitHub/apple/build/Ninja-DebugAssert/swift-macosx-x86_64/lib/swift/macosx -rpath /Users/bgesiak/GitHub/apple/build/Ninja-DebugAssert/swift-macosx-x86_64/lib/swift/macosx -macosx_version_min 10.9.0 -no_objc_category_merging -o libFoo.dylib
/usr/bin/dsymutil libFoo.dylib -o libFoo.dylib.dSYM

That's a lot to take in, but here's the relevant part of the link job:

/usr/bin/ld -add_ast_path /var/folders/ry/2ryfdsb56b30092626qprw6d3rb3ss/T/Foo-acf884.swiftmodule ...

So it's my understanding that, for this task, we'd like to allow the driver to do the following:

$ swiftc -c -g Bar.swift Foo.swiftmodule -emit-library -L Foo.dylib

...which would result in a link action that looks like this:

/usr/bin/ld \
    /var/folders/ry/2ryfdsb56b30092626qprw6d3rb3ss/T/Bar-4c5e4c.o \
    -add_ast_path Foo.swiftmodule \
    ...

Namely, .swiftmodule arguments should be passed to the linker with -add_ast_path – is that right?

Add a check to not do that if -emit-module is passed, and instead to treat them as MergeModule inputs.

It's my understanding that when -emit-library is specified, .swiftmodule inputs should be passed to the linker, using -add_ast_path. Otherwise, if the user is using -emit-module, .swiftmodule inputs should be passed in as inputs to a merge module job. I suppose this is useful to end users that want to combine several .swiftmodule files into a single .swiftmodule.

I'm sure it's not that easy in practice, but it seems digestible.

Right now it seems like .swiftmodule files aren't accepted as input by the driver under any circumstances, so I'll start by changing that. I guess they are accepted as input when generating a module, my bad.

Does this all sound right, @belkadan?

[modocache]

I spent a little more time reading the original task https://bugs.swift.org/browse/SR-2637, and I think I have a better understanding now.

$ swiftc Foo.swift -emit-object -parse-as-library
$ swiftc Foo.swift -emit-module -emit-module-path `pwd`/Foo.swiftmodule
$ swiftc main.swift -emit-object
$ swiftc main.o Foo.o Foo.swiftmodule -g
<unknown>:0: error: cannot load module 'Foo' as 'main'
<unknown>:0: error: merge-module command failed with exit code 1 (use -v to see invocation)

It's my understanding that the goal of this task is to prevent the above error, which occurs because the merge module step attempts to merge Foo.swiftmodule into main.swiftmodule:

$ swiftc main.o Foo.o Foo.swiftmodule -g -driver-print-actions
0: input, "main.o", object
1: input, "Foo.o", object
2: input, "Foo.swiftmodule", swiftmodule
3: merge-module, {2}, swiftmodule
4: link, {0, 1, 3}, image
5: generate-dSYM, {4}, dSYM

$ swiftc main.o Foo.o Foo.swiftmodule -g -driver-print-jobs
swift -frontend -emit-module Foo.swiftmodule -parse-as-library -g -module-name main -o /tmp/main.swiftmodule
/usr/bin/ld main.o Foo.o -add_ast_path /tmp/main.swiftmodule -o main
/usr/bin/dsymutil main -o main.dSYM

Instead, I understand this task to mean that Foo.swiftmodule should be treated as an input to the linker – so I guess something like this:

$ swiftc main.o Foo.o Foo.swiftmodule -g -driver-print-actions
0: input, "main.o", object
1: input, "Foo.o", object
2: input, "Foo.swiftmodule", swiftmodule
3: link, {0, 1, 2}, image
4: generate-dSYM, {3}, dSYM

$ swiftc main.o Foo.o Foo.swiftmodule -g -driver-print-jobs
/usr/bin/ld main.o Foo.o -add_ast_path Foo.swiftmodule -o main
/usr/bin/dsymutil main -o main.dSYM

Does that sound right?

By the way, as noted in the other task, without -g, .swiftmodule inputs aren't accepted by the driver. I think this is due to logic in lib/Driver/Driver.cpp that rejects any .swiftmodule input unless a module is being generated: https://github.com/apple/swift/blob/6500ff9ff75024adc196e2718ea0f9b043c316a4/lib/Driver/Driver.cpp#L1182-L1190.

$ swiftc main.o Foo.o Foo.swiftmodule
<unknown>:0: error: unexpected input file: out/Foo.swiftmodule

[belkadan]

That's mostly it. -emit-module[-path] is actually a modifier on other build kinds, so the more common case is

swiftc -c -emit-module-path Foo.swiftmodule -parse-as-library foo.swift -module-name Foo

(or something with multiple .swift files and an output file map).

I'm happy to disconnect -g from whether or not we do anything with swiftmodules. There'll be a bunch of assertions to remove, though—in the first implementation we wanted to be very sure we didn't accidentally try to stick partial modules in the final build product!

[modocache]

I'm taking a break from this for now, so I'll free up this task in case anyone else would like to try it. 🙂

[trfiala]

Thanks for taking a whack at it, Brian!

[adrian-prantl]

@swift-ci create

[swift-ci]

Comment by Steven Hepting (JIRA)

matthew carroll (JIRA User) We are looking forward to any progress updates you might have on this one!

Relevant issue in Buck: facebook/buck#423

[modocache]

I took another look at this. Thanks to Todd for attaching such a great sample project. It didn't work with Swift ToT, so I updated it: https://github.com/modocache/SR-2660

Here's the thing, though: the updated sample project works for me when using Swift 98ba6a4 (Oct 18 ToT) and swift-lldb 7fc171f (Oct 13 ToT). The linker is invoked with -add_ast_path Foo.swiftmodule -add_ast_path Bar.swiftmodule and lldb is able to print out the type information correctly.

So, based on the evidence I have, I think this must have been fixed, perhaps accidentally, at some point. My next steps are:

• "Bisect" the problem in order to determine when exactly the issue reported by this task was fixed. (I'd like to know why it now works!)

• Update the driver so that users don't need to manually invoke clang to link the two .swiftmodule.

• Test this out with Buck.

[modocache]

Oh I see, Todd fixed this in apple/swift-lldb@88e778c6abf65dba49d0a028f2f9f794407d6d1a, and he left a comment on the original issue. Great! So I just need to modify the driver. Once I do that, I'd like to test whether Buck's issue is fixed. Is there an easy way to do that, I wonder?

[modocache]

I sent up #12507 as a first attempt at addressing one of the two driver improvements Jordan mentioned in an earlier comment.

[adrian-prantl]

Brian, with pull request #12507 merged, what is missing here now?

[modocache]

Thanks for the follow-up, @adrian-prantl! There are two pieces of work left, I think:

• One is, as mentioned by @belkadan in an earlier comment on this issue, to decouple -g from the driver's treatment of .swiftmodule inputs. I made an attempt at this, but have not submitted it as a pull request because I'm fairly certain it's not what Jordan had in mind: modocache@e4a7bb1 That commit skips the merge-module action altogether, but the best solution would probably be to still perform merge-module, but pass top-level .swiftmodule inputs directly to the linker. I'd like to attempt this next week, but you're welcome to work on it if you'd like!

• More importantly, in the context of Swift support in Buck: I'm able to confirm, using a small Buck project that uses Swift and Objective-C, that debugging does not work in some cases. In that sample project, passing -add_ast_path Bar.swiftmodule -add_ast_path Foo.swiftmodule provides debugging symbols for Foo, but passing -add_ast_path Foo.swiftmodule -add_ast_path Bar.swiftmodule does not – even when using a recent release of Xcode that includes Todd's lldb fixes. This sample project exists within a private codebase. Give me 2-3 hours and I'll package it up so that even someone not familiar with Buck can reproduce the issue, then attach it here.

[belkadan]

[…] the best solution would probably be to still perform merge-module, but pass top-level .swiftmodule inputs directly to the linker.

Yep, this is where I think we should go.

[JDevlieghere]

Hi Brian, did you manage to package up the reproducer? I'm verifying that dsymutil works with multiple AST nodes and I'd like to ensure it works with your test case too, rather than with just something artificial.

[modocache]

Hi! I apologize for the initial estimate, it turns out the project in question uses a lot of internal Buck functions and so it's not as easy as I thought to split out and attach here. I'll try to do so this week.

As I mentioned above, I wasn't able to reproduce the issue until I generated an Xcode project via Buck and attached a debugger, so I think it'll be a valuable bug once we can narrow down what's going wrong.

[adrian-prantl]

Brian, do you think it would be possible for you to just post a simplified build log of your testcase? We would like to help recreating your testcase but it isn't clear to me what the ingredients are. Let me lay out what I understand so far and perhaps you can tell me where my assumptions and your testcase are different:

1. The main program is Objective-C only.

2. Library A is a Swift only library, compiled by swiftc (which flags exactly?) and consists of exactly one logical Swift module that is distributed over multiple .swift source files. The resulting .o files are packaged with ar into a static archive. Is the .swiftmodule part of the .a file too?

3. Library B is like library A

4. The main program and library A and B are linked using ld64 directly (or are you using the clang or swift driver).

5. Depending on the order of the -add_ast_path options the debugger can only find the debug info for one of the Swift modules.

6. Are you running dsymutil?

7. Can you confirm that both N_AST symbols make it into the symbol table (dump it with dsymutil -s)?

Thanks!

[modocache]

(Oops, sorry, I must have accidentally hit a JIRA keyboard shortcut that briefly assigned this task to me.)

Sure @adrian-prantl, that's a good idea. I'll try to answer what I can.

1. Correct, the main program is an entirely Objective-C macOS app that defines an NSApplicationDelegate and runs NSApplicationMain.

2. Library A, in this case Foo, is a mixed Objective-C and Swift library. The resulting .o files are packaged into a static archive libFoo.a using libtool -static.

3. Library B, in this case Bar, is also a mixed Objective-C and Swift library. It has a dependency upon A (Foo), and imports both an Objective-C header and a bridging header from A (Foo).

4. Yes, the main program and libraries A (Foo) and B (Bar) are linked using the clang driver.

5. Yes, the order of the -add_ast_path options appears to matter.

6. No, as far as I can tell, dsymutil is not being run.

7. Yes, I confirmed both N_AST symbols are present.

Thanks for your help, and sorry I don't have a clean project I can share with you yet. For now, I'll attach the build logs you requested, as well as the source code (but none of the .xcodeproj or build files) of the example project. Hope this helps for now! I'll also try to upload a clear reproduction case as soon as I can.

[modocache]

Thanks for your patience on this one! The original reporter of the Buck+Swift bug is on vacation until November 6th. In the meantime, I'm trying to produce a small sample project. I also sent up #12708 hopefully this is close to what @belkadan had in mind for handling top-level .swiftmodule inputs with -g.

[swift-ci]

Comment by Milen Dzhumerov (JIRA)

Apologies for not being able to attach a repro case earlier, I'm currently busy with another work stream that demands all my attention. I will try my best to setup a repro Xcode project as soon as I can.

[swift-ci]

Comment by Milen Dzhumerov (JIRA)

@adrian-prantl @modocache I've created a repro Xcode project from scratch that mimics how Buck would generate it (but without using Buck at all).

TestApp.zip

[modocache]

Thanks, milen (JIRA User)! I can confirm that the problem reproduces for me – placing a breakpoint at Bar.swift:19 and attempting to po self results in an error.

Running dsymutil -s on the app executable, I don't see any N_AST entries.

I'm able to work around the error by adding the following to the Xcode project's TestApp target's OTHER_LDFLAGS:

OTHER_LDFLAGS = (
        "-Xlinker",
        "-add_ast_path",
        "-Xlinker",
        "$(BUILT_PRODUCTS_DIR)/Bar.swiftmodule/x86_64.swiftmodule",
        "-Xlinker",
        "-add_ast_path",
        "-Xlinker",
        "$(BUILT_PRODUCTS_DIR)/Foo.swiftmodule/x86_64.swiftmodule",
);

This results in N_AST entries being added and in debugging info working within Xcode. I'm still trying to figure out how Xcode's behavior differs in this case when compared to an app project with dynamic framework dependencies.

[JDevlieghere]

Hi Brian, Milen,

Thanks a lot for attaching the reproducer. I had another look at this using Xcode 9.3 beta. It make sense that without the ASTs being added by the linker they don't end up in the debug map. If you want to verify this yourself, you can run:

dsymutil -dump-debug-map <binary>

With the linker flags added manually, I'm able to break in both static libs. I put a breakpoint at Foo.swift:14 and Bar.swift:19 as you suggested.

(lldb) po self
Bar.BarObject
(lldb) po self
FooStruct
  - version : 5

Can you confirm that this scenario also works for you using the latest Xcode?

The difference with dynamic libraries is that Xcode calls libtool instead of the linker to create the static archive. The latter doesn't know what to do with this add_ast_path flag (or the AST for that matter). This explains why you have to pass the ASTs explicitly when linking the final binary.

[swift-ci]

Comment by Steven Hepting (JIRA)

Are there any more updates on progress for this bug?

[CodaFi]

@JDevlieghere The radar for this is resolved, is the bug as well?

[JDevlieghere]

I believe so. Brian or Milen, can you please verify this works for your use case?

[swift-ci]

Comment by Milen Dzhumerov (JIRA)

I have now verified that this works as expected.