Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SR-2637] Issue with debugging programs that contain multiple Swift modules #4588

Closed
swift-ci opened this issue Sep 14, 2016 · 25 comments
Closed
Assignees
Labels
bug Something isn't working LLDB for Swift

Comments

@swift-ci
Copy link

Previous ID SR-2637
Radar None
Original Reporter dmishe (JIRA User)
Type Bug
Status Closed
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 LLDB for Swift
Labels Bug
Assignee @trfiala
Priority Medium

md5: b76ab4f38b9543ef86bb7f211e8dd45d

cloned to:

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

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
Copy link
Mannequin

trfiala mannequin commented Sep 14, 2016

Thanks for giving us more detail here, Dmitry!

I'll track down somebody to expand on options here.

@trfiala
Copy link
Mannequin

trfiala mannequin commented Sep 14, 2016

HI Dmitry,

I've poked around with a few people over here. The first question is around manually calling clang to do the linking:

Q. Is the only reason that you're calling clang directly to link so that you can avoid getting a dsym built? If so, we have a few options here, which I discussed with @belkadan and @DougGregor:

1. Modify swiftc linking to avoid calling dsymutil if linking without the -g flag when on Apple OSes. This would then mirror the way clang linkage currently works.

2. Add a swiftc flag to explicitly disable dsymutil from being called. This would not match clang, but isn't necessarily a bad option.

3. Add a swiftc mode that just gathers linker flags, so that another linker (such as clang or ld) can link a swift binary properly.

Jordan is willing to make changes to support one or more of the above.

One avenue to investigate, and which the above might correct if this is the issue, is whether the -add_ast_path settings are correct. You can run swiftc in a mode where it dumps out the command lines it generates when calling other tools under the covers. To get that behavior, add '-###' (dash pound pound pound) to the swiftc command line. If you do that, you could validate what flags swiftc would have used to link, and ensure yours are matching.

Based on what you are seeing, I suspect we may also have an issue that could be addressed on the LLDB side, and quite possibly because you're generating binaries in a way that exposes a latent issue that we don't normally hit with standard swift linkage (and a call to dsymutil).

If you can try out the swiftc -### and figure out what it would have done for flags, and verify the ones you are using on the clang linkage line match, that would be a good first step to diagnosing.

Thanks!

@belkadan
Copy link

To be clear, we already do (1); it's just that it would require two invocations of swiftc. But if you're using Clang to link you're effectively doing that anyway.

@trfiala
Copy link
Mannequin

trfiala mannequin commented Sep 14, 2016

(Oops, thanks for correcting, Jordan!)

@belkadan
Copy link

Whether or not you can have multiple -add_ast_paths in one file is an LLDB-side issue. Static linking isn't really supported, so it hasn't come up.

@trfiala
Copy link
Mannequin

trfiala mannequin commented Sep 14, 2016

Greg Clayton, @belkadan and I discussed this a bit. We know we're likely to support static linking anyway, so we're going to take a crack at handling this. We see that the linked file has the multiple ast paths in it. We need to loop over them and feed them to the ast context that we build up internally.

I'm going to have a look and see if I can get that working in the GitHub master-next branches. (That would be swift/master-next, swift-lldb/master-next, swift-llvm/stable-next and swift-clang/stable-next branches).

@trfiala
Copy link
Mannequin

trfiala mannequin commented Sep 14, 2016

(Dmitry, if this gets quite involved, I may need to pass this off to you to try to implement).

@swift-ci
Copy link
Author

Comment by Dmitry Shevchenko (JIRA)

That sounds great!

Could you clarify the state of the static linking support? It seems possible to link with the stdlib statically, and SPM links packages statically (not the runtime though), so what is really missing?

Using swiftc to link, with -g, it tries to merge the modules provided into "main" module, and fails:

$ swiftc out/foo.o out/bar.o  out/main.o  out/Foo.swiftmodule out/Bar.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)

@swift-ci
Copy link
Author

Comment by Dmitry Shevchenko (JIRA)

Todd, and just to clarify, there's no work around we can try in the meantime, we should look into whether this works with dSYM?

@belkadan
Copy link

Static linking works in limited circumstances, including building a single command-line tool with no non-system dependencies. In other cases (including the package manager), I think we're squeaking by with regards to debugging, or possibly just leaving things on the floor. "It compiles but debugging is utterly broken" doesn't really count as supported. (cc @ddunbar)

I'm sorry, I forgot that swiftc will try to merge modules given to it. That could still be useful if -c didn't also merge modules, so I don't know if it makes sense to change that behavior. (The suggestion was to link without -g, though.)

@swift-ci
Copy link
Author

Comment by Dmitry Shevchenko (JIRA)

Oh I misunderstood, linking without -g does not allow me to even specify a swiftmodule input at all. It does not record anything in AST and predictably debugger fails to figure out types.

After more experimentation, I found out that to make this work well I need both dSYM generation and both AST records. After linking, the dSYM product contains a __swift_ast section of size equal to two .swiftmodules. LLDB then is able to parse both through the magic of Swift's ASTSectionImporter, neat 🙂 I suppose that means that there's no reason LLDB can't read multiple AST refs in the first place.

@trfiala
Copy link
Mannequin

trfiala mannequin commented Sep 15, 2016

> Oh I misunderstood, linking without -g does not allow me to even specify a swiftmodule input at all.

Hmm, @belkadan, I thought we saw differently when we looked at the code this morning?

> After more experimentation, I found out that to make this work well I need both dSYM generation and both AST records.

dmishe (JIRA User), I would expect that you would need both AST records. If you ran swiftc with the -### option and looked at the link step, I think that would show up there.

The piece I'm looking at is to see if we can find and feed all the ASTs we find into our AST Context. There's a chance we'll wire this up on the LLDB side and still need a bit of help from Jordan, et al. But I should know soon enough. (I got stuck on a couple hot potatoes today so I'm going to have to tackle this in the morning). I'm looking to change our SymbolFile AST retrieval method to return a vector of AST data blobs instead of just one, and then loop around and feed them all into the ingestion process in SwiftASTContext. I'll then see what happens from there.

If this does work, I need to really do this via a change to LLVM.org LLDB, pull that through, and make the Swift LLDB change on top of that.

I'll keep you posted!

@swift-ci
Copy link
Author

Comment by Dmitry Shevchenko (JIRA)

When I have one module, yes, -add_ast_path shows up in swiftc link step when invoked with -g. But I can't really figure out how to make it link 2 modules into a binary without merging them. With -g it tries to merge swiftmodule inputs as I mentioned, and without -g it does not care about modules at all.

I can manually construct the link step using clang -g -add_ast_path Foo.swiftmodule -add_ast_path Bar.swiftmodule and then both modules will be written into dSYM DWARF segment.

@trfiala
Copy link
Mannequin

trfiala mannequin commented Sep 15, 2016

With the binaries you attached, Greg Clayton and I were able to see all the ast path info we needed I thought. So your initial usage scenario is what I'm hoping to get working on the LLDB side (with the caveat that it might incur a required change on the Swift side, we'll know soonish).

@swift-ci
Copy link
Author

Comment by Dmitry Shevchenko (JIRA)

Just to clarify, this is what I mean by saying that swiftc won't accept swiftmodules without -g:

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

@trfiala
Copy link
Mannequin

trfiala mannequin commented Sep 15, 2016

dmishe (JIRA User), side comment: nice job putting together the repro zip file! I've put together a potential implementation to try tonight, and getting the setup for the failure is very simple with your scripts.

@trfiala
Copy link
Mannequin

trfiala mannequin commented Sep 15, 2016

I'm working on the trial solution here:
https://github.com/tfiala/swift-lldb/tree/SR-2637

I've got at least one TODO left, and the code currently asserts in what may be fallout from the bit I glossed over. I'll get back to this in the morning, and I plan to enable a fair amount of additional logging.

@trfiala
Copy link
Mannequin

trfiala mannequin commented Sep 15, 2016

I've attached build.sh, which I modified a bit, so that it explicitly calls out the just-built LLDB and Swift, and sets the SDK path properly. It would need to be tweaked for a different developer's build dir. With LLDB and Swift, it is essential that they both match (i.e. are built from the same point in time in the source trees), and since I'm testing changes to LLDB, I modified the script to spell out exactly where I need them to pick up the Swift compiler and LLDB I just built.

@trfiala
Copy link
Mannequin

trfiala mannequin commented Sep 15, 2016

I have a fix that seems to work with the original reported bug.

I need to run the rest of the test suite, address any issues there, and bundle up this example as a test. Then I'll put up a PR with the fix.

@trfiala
Copy link
Mannequin

trfiala mannequin commented Sep 15, 2016

Jordan Rose, Greg Clayton and I talked about this some more. Based on that, I cloned this ticket to represent some enhancements Jordan suggested for the driver to better support this scenario. Those changes are not necessary to get this issue here to work, though.

The changes I made in my branch are not quite what we want yet. I need to call ```CompilerInvocation.loadFromSerializedAST(...)``` with only the "main" serialized .swiftmodule content. This is because this method does some one-time setup (like specifying the SDK path) that should not and cannot change from one AST to another. Here, "main" means whatever is closest to the primary module for the executable, and contains the most restrictive triple-related information in it since this information is going to be used for all internal Swift compiler-related operations performed by LLDB (e.g. expression evaluation, data formatting, etc.)

I'm looking into seeing if the -add_ast_path is stable, and if so, figure out whether we need to specify the "main" .swiftmodule file first or last. (We want it to be the first .swiftmodule file name listed in the linked executable). In the future, this type of requirement will hopefully be codified by the driver when it is able to spit out the required linker flags for a given configuration of .o/.swiftmodule files, so that those who want to link with another linker know the proper flags and ordering to get this right.

@trfiala
Copy link
Mannequin

trfiala mannequin commented Sep 16, 2016

> I'm looking into seeing if the -add_ast_path is stable

The -add_ast_path command-line arguments seem to be stable on the Xcode 8 linker and look to appear in the order specified on the command line. Given this, you'd want to have the most-representative .swiftmodule (i.e. the module that best represents the executable binary) as the first .swiftmodule on the command line. Essentially on macOS targets, you're looking for (1) the proper SDK path in the .swiftmodule. That's probably not likely to change across multiple Swift modules that are linked together, unless they are built with different SDKs per node or saved for some period of time. And (2) the target triple, which can be more restrictive for some modules vs. others, in which case you want the most restrictive triple to be in the "main" .swiftmodule specified first with -add_ast_path. (If they're all the same, it doesn't really matter which one you put first).

I'm looking to make a test in the LLDB test suite to test this. Then this is ready to go.

@swift-ci
Copy link
Author

Comment by Dmitry Shevchenko (JIRA)

Thanks for your work!

It's great that this functionality will find its way into the driver. I am happy to help be it testing or providing more details about what our builds look like.

Luckily, our swiftmodules are all compiled with exactly the same flags, against the same SDK, so we'll just reference them all.

That said, while playing with this setup in a program written solely in Swift (think Main imports Foo, imports Bar), I found that referencing just the main module in AST is enough for LLDB to discover other imported modules. How does that work? How are the imports of other modules encoded exactly, just names or full paths to swiftmodule files?

@trfiala
Copy link
Mannequin

trfiala mannequin commented Sep 16, 2016

I've got the code in a PR here:
apple/swift-lldb#53

> Luckily, our swiftmodules are all compiled with exactly the same flags, against the same SDK, so we'll just reference them all.

Yes, regardless you do need to specify all the .swiftmodule files. While only the first one is used to initialize some critical one-time information, all of them are used to scan for other parts (LLVM modules) that are needed by the expression engine. The recommendation earlier of the ordering of --add_ast_path entries is solely about determining which one to put first. (Which, for your case, doesn't matter, but might for others who may end up looking at this sometime in the future).

> That said, while playing with this setup in a program written solely in Swift (think Main imports Foo, imports Bar), I found that referencing just the main module in AST is enough for LLDB to discover other imported modules. How does that work?

Was it compiled with multiple .swift files provided to swiftc in a single invocation? If so, then swiftc ends up merging all the partial .swiftmodule entries into a single one, and there is only one that appears in the final output. (In fact, this was the root of the issue with the scenario you initially supplied above: we haven't yet prioritized having a static linking story for Swift, and we hadn't yet ever had to deal with more than one .swiftmodule being specified before. The SwiftPM folks that we talked to today have tried doing some static linking, so it appears, but have not been supplying the .swiftmodule files to the linker, so those just wouldn't be debuggable for similar reasons to why your second module's types couldn't be located. Their situation is worse, though, as the debugger wouldn't be able to evaluate anything that needed to know about any types in any of those modules IIUC. They are looking into what it would take to address that.)

> How are the imports of other modules encoded exactly, just names or full paths to swiftmodule files?

I believe this happens slightly differently on Linux with some kind of wrapper, so I would have to look there before commenting further on the Linux side. On the macOS side, the Apple Linker had the -add_ast_path command-line argument added. All of those are embedded by name into a section in the final-linked binary, and put under a known symbol/symbol type. (I think they are one .swiftmodule file name per symbol). The setup you are using, with the loose .o files, provides the .swiftmodule file names as relative paths that are of course not going to be valid if the .o files are moved around relative to the working directory at the time they were fed to the linker. And if they are moved around such that the relative path does not evaluate to anything that we can resolve in the debugger, then the debugger expression engine's swift compiler will not know anything about the types described in that .swiftmodule file.

@trfiala
Copy link
Mannequin

trfiala mannequin commented Sep 16, 2016

I've implemented the tests for macOS for this. I'm running them through CI via the PR now.

@trfiala
Copy link
Mannequin

trfiala mannequin commented Sep 17, 2016

The support went into swift-lldb here:
apple/swift-lldb@88e778c

The original tests hit an issue with file paths on the CI. I had to revert out the tests, then put them back in here:
apple/swift-lldb@5d6a3ea

@swift-ci swift-ci transferred this issue from apple/swift-issues Apr 25, 2022
@shahmishal shahmishal transferred this issue from swiftlang/swift May 7, 2022
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working LLDB for Swift
Projects
None yet
Development

No branches or pull requests

2 participants