Implement -debug-prefix-map flag.

[allevato]

Like Clang's -fdebug-prefix-map, this lets the user remap absolute paths in debug info. (The implementation here is based largely on the Clang implementation.) This is necessary for reproducible builds and allows debugging to work correctly on a machine other than the one that built the code because absolute paths to the source may be different.

Addressing search paths/ClangImporter flags in .swiftmodule files will happen separately; I've preserved my original discussion items below.

---

In addition to the path changes in the emitted debug info, this also necessarily remaps the search paths serialized in the INPUT_BLOCK of .swiftmodules so that those paths can also be resolved correctly when LLDB loads the ASTs.

I've done some local testing of this, remapping the debug info paths and search paths of a multi-module executable, and LLDB finds both the source and the dependent modules correctly when started in the appropriate CWD.

Open question: Remapping ClangImporter flags

I looked into remapping the full set of ClangImporter flags in the OPTIONS_BLOCK as well, but this is somewhat challenging:

1. The arguments are just a list of strings, and there's no guarantee that a particular element is a path that should be remapped just because it matches one of the mappings. There's the potential that we could remap something that we shouldn't, unless we used context from the surrounding flags (e.g., remap -I foo, but not -D foo).
2. Similarly, some paths that need to be remapped appear in flags that are joined without intervening spaces. For example, if I compile with -debug-prefix-map foo=bar, then -Ifoo needs to become -Ibar and -isystemfoo needs to become -isystembar. This would require keeping an explicit whitelist of such flags.

Remapping the ClangImporter flags isn't necessary in all cases. They're passed to ClangImporter verbatim as the user typed them in the invocation, and therefore aren't converted to absolute paths like the Swift import search paths are. So in situations where the user is using -debug-prefix-map to remove an absolute path prefix (by remapping it to the empty string or .), they can avoid the need to remap ClangImporter flags by simply passing those relative paths to begin with. But in the case where you need to actually change a path prefix, it may not be sufficient. I'm not sure if that's a blocking issue here.

Known issue: Duplicate search paths

As currently written, this PR causes duplicate search paths to appear in some .swiftmodules. The problem is this:

1. Module A is compiled with -I foo -debug-prefix-map foo=bar. The compiler adds foo to the AST context's import search paths, and it is remapped to bar when the module is serialized.
2. Module B is also compiled with -I foo -debug-prefix-map foo=bar, and it imports A. First, the compiler adds foo from the command line to the AST context's search paths. Then module A is loaded, which adds bar to the context's search paths. When B.swiftmodule is serialized, there's no attempt to uniquify these; foo becomes bar, the second bar stays the same, and we end up with two identical entries in the module.

I can think of a couple ways to address this:

1. Uniquify the paths before serializing.
2. Don't load search paths into the AST context when loading a module for compilation; only do it when LLDB loads it for debugging. This seems like it would be OK because people shouldn't rely on transitive search paths that are only persisted in debugging options, but I'm not sure if there are other issues that would arise here. I filed SR-7845 to try to figure this out, and I have an implementation of it that I can open as another PR (it would also require an LLDB change).

@jrose-apple @adrian-prantl You've both given me some conceptual input on this already; is there anything I'm overlooking?

[adrian-prantl]

Open question: Remapping ClangImporter flags

I looked into remapping the full set of ClangImporter flags in the OPTIONS_BLOCK as well, but this is somewhat challenging:

You analysis that this is difficult is correct, but I'm afraid, we can't ignore it. Debugging Swift code currently depends on LLDB's embedded swift compiler importing Swift modules, and that depends on Swift's embedded Clang importing Clang modules. There are two approaches to apply the remapping:

• do a dumb remapping of all strings and risk that we accidentally remap macro definitions as shown in your example
• implement a command line option parser that supports a few known important flags, such as -I, -isystem, -isysroot, ... and remap those only; with the list of supported flags being something that could be expanded later on. If we are going down this route, it would be great to factor the parsing out into a common helper somewhere, because LLDB currently also implements the reverse operation (using yet another ad-hoc GCC-style command line option parser) in SwiftASTContext.cpp to support path remapping dictionaries from .dSYM bundle plists.

The second variant would be cleaner.

Remapping the ClangImporter flags isn't necessary in all cases.

Maybe the right answer is to realpath them before serialization, too, just like the other paths that go into debug info?

Known issue: Duplicate search paths

As a general guidance: Everything we can do to make compilation and debugging more similar is a win. I would not want to make any changes that would make it easier to have search path issues that only reproduce when in the debugger.

[jrose-apple]

I'm with Adrian on the "only use search paths when debugging" thing. We used to do that, and it led to a bunch of weird cases where you could compile but not debug.

ClangImporter flags are another big mess. I don't want to have to recreate Clang parsing or guess at what's a path. And really, a whole bunch of Clang flags are probably not interesting for debugging. It's just hard to know, and even weirder for Swift to know what they would be. I'd rather take a step back and think about how debugging with Clang flags is supposed to work at all. (Sorry, I think I've said this to both of you multiple times in the past, even though we've never actually set a time for such a rethink/discussion.)

That said, this patch is still useful just to improve -gline-tables-only, even without any of the Serialization stuff. I say let's get that part in.

In the near term, how about something brute force for the swiftmodule side: if some flag is passed, we don't serialize search paths (or -Xcc options) at all, and then it's part of setting up a proper debug environment? I don't think that's a good enough answer to be the default mode for everyone yet, but it might be good enough for what you're doing.

[jrose-apple]

It wasn't obvious to me that this should be using startswith instead of checking for a path delimiter, but it looks like that's what Clang's mapping does too (albeit without preserving order). Can you leave a comment to note that this was thought about?

[allevato]

Done.

[jrose-apple]

Nitpick: Option help strings should be capitalized.

[allevato]

Done.

[jrose-apple]

Good catch. I feel like I would have missed this. :-)

Note that Clang does not preserve the order of options today; it probably should in the future. At least that's backwards-compatible.

[allevato]

Good point; I updated the comment to mention the (admittedly rare?) situations where the difference matters.

[allevato]

Thanks for the feedback! I've reverted the pieces that remapped paths inside .swiftmodules and left only the DebugInfo bits, so I think this is what you were looking for. I'm happy to work on this incrementally—the paths in .swiftmodules are an issue for us not only because of remote debugging, but for reproducible builds in general because these the files can't be reliably cached.

I understand the desire to keep module loading the same during compilation and debugging, although based on my exploration in SR-7845, it still seems problematic to me that serialized debugging options would modify how modules are found during compilation. Maybe this is a bigger issue for us based on how we use Bazel to build Swift, though? Unlike Xcode where an entire application is typically a single module (excepting dynamic frameworks), we build multiple small modules as static libraries, always with an Objective-C header for interop, and then use Clang, not swiftc, to link the final binary. So we have to manually pass -Xfrontend -serialize-debugging-options to each of those modules to get debugging to work, because the heuristic used in FrontendTool.cpp doesn't always apply to us.

...which means that we can actually explore your suggestion of disabling serialization today by just not passing that flag. Once this change is in, maybe we can configure the rest of the debugging environment enough that it could be made to work.

[jrose-apple]

Yes, this is what I meant for an uncontroversial first step. @adrian-prantl, what do you think?

@swift-ci Please smoke test

[allevato]

@adrian-prantl Friendly bump 🙂

[allevato]

Bump again—we would love to see this make it into Swift 4.2 if possible, since it opens the way for us to debug distributed builds. If there's anything else I need to change before it can be merged, please let me know. Thanks!

[jrose-apple]

I'm sorry, I forgot that Adrian is away on vacation. @dcci, @vedantk, what do you think?

[dcci]

I'll take a look at this today.

[dcci]

The bits in IRGenDebugInfo.cpp are fine, but I can't comment on the Driver changes as I'm not really familiar with them, so you'll need somebody else to approve that part.

[dcci]

I guess private is redundant here, no?

[allevato]

Indeed! Removed.

[jrose-apple]

I'm good with the Driver bits. In the interest of expediency, I'm okay with merging this to master today, but I'm not sure Ted will still want to take it on the 4.2 branch at this point. Doesn't hurt to ask, though (via PR).

[dcci]

@swift-ci please test

[swift-ci]

Build failed
Swift Test OS X Platform
Git Sha - cb56564

[swift-ci]

Build failed
Swift Test Linux Platform
Git Sha - cb56564

[allevato]

Thanks all! Once this merged, I'll float a PR to cherry-pick it and see what the release manager thinks.

There's still a minor behavioral change even when the -debug-prefix-map flag is not used, so there's some potential amount of risk there; Swift previously stored the full absolute path as the file path in the directory/file pair in the DIFile, but now we only store the compilation-dir-relative path and let the debugger join them as it expects. This aligns with how Clang stores those paths, however, and was required for remapping to work correctly. (It worked "accidentally" before because I believe LLDB was "joining" the directory path and the absolute file path in a file-system-correct way, which meant that the second path was just used by virtue of being absolute.)

I'm not familiar enough with LLDB's test suite to know whether passing tests there are a good measure of whether that change worked as intended; I did some local testing to verify it by hand and hoped that I covered everything. @dcci, do you have some additional insight?

[dcci]

I'm not familiar enough with LLDB's test suite to know whether passing tests there are a good measure of whether that change worked as intended; I did some local testing to verify it by hand and hoped that I covered everything. @dcci, do you have some additional insight?

The swift-pr cycle runs the swift lldb tests as part of this so you should be fine.
If you want to be sure and run the full testsuite locally, just do:

swift/utils/build-script --lldb --release --test -- --skip-build-benchmarks --no-swift-stdlib-assertions --lldb-use-system-debugserver --lldb-build-with-cmake --skip-test-swift --skip-test-cmark

[dcci]

If you want to add a testcase to the lldb testsuite where you build a source file with this command and make sure everything works as intended, though, it will be really appreciated as a follow-up.

[allevato]

From glancing over the OS X log, it looks like the failure is unrelated (but I'm not sure why it occurred).

[dcci]

@allevato That assertion has been committed 4 days ago. Have you tried to reproduce this locally on ToT?

[dcci]

(although I agree it doesn't seem immediately related).

[dcci]

@swift-ci please test macos

[allevato]

Unfortunately I'm not in front of that particular development machine right now; if the re-run fails the same way, I'll try to reproduce it later today and report back.

[jrose-apple]

It passed this time, so I'm going to merge to give Tony the opportunity to make a 4.2 PR.

[JianweiWangs]

Hello, @allevato, I am using this feat. but not work in my proj, this is example proj, could you help me for some review?
Code.zip

• Xcode 11.0 beta and Xcode 10.2.1 release
• macOS Mojave 10.14.4
• objc work
• swift not work
• remap to /tmp/Map/#fileName

Please!

Email