swiftlang / llvm-project

The LLVM Project is a collection of modular and reusable compiler and toolchain technologies. This fork is used to manage Swift’s stable releases of Clang as well as support the Swift project.
https://llvm.org
Other
1.12k stars 330 forks source link

[SR-14048] LLDB crashes when parsing certain .h files #4325

Open robinkunde opened 3 years ago

robinkunde commented 3 years ago
Previous ID SR-14048
Radar rdar://73385622
Original Reporter @robinkunde
Type Bug

Attachment: Download

Additional Detail from JIRA | | | |------------------|-----------------| |Votes | 0 | |Component/s | LLDB for Swift | |Labels | Bug | |Assignee | @kastiglione | |Priority | Medium | md5: 62b49bc804e7b2cbe014d897d44e69b5

Issue Description:

The symptom I'm seeing is that when execution stops at a breakpoint, the variable view in Xcode will stay completely blank, and when I try to `po` a local variable, LLDB will spit out the following (private details omitted) and then crash.

(lldb) po application
warning: Swift error in scratch context: <module-includes>:1:9: note: in file included from <module-includes>:1:
#import "Framework-Header.h"
        ^

{similar errors omitted}

.
Shared Swift state for PointOfSaleDev has developed fatal errors and is being discarded.
REPL definitions and persistent names/types will be lost.

This started happening after I updated a vendor's SDK. After a lot of trial and error, I narrowed it down to these lines in one of the framework headers.

#if __IPHONE_OS_VERSION_MIN_REQUIRED < __IPHONE_10_0
#error This SDK requires iOS 10.
#endif

If I change "_IPHONE_10_0" to "_IPHONE_9_0" the problem goes away. Same if I remove "#error" or change it to "#warning".

Seems like there are two separate issues here:

  1. LLDB is getting the wrong value for __IPHONE_OS_VERSION_MIN_REQUIRED from somewhere

  2. LLDB chokes when the preprocessor❓ generates an error while parsing the header files to build the debug context

I doublechecked all the places where IPHONEOS_DEPLOYMENT_TARGET is set in the project and it's all 13.0. The only exceptions are SwiftPM packages which I have no control over. I should also mention that all the build settings are kept in xcconfig files instead of the pbxproj.

Assuming my assumptions above are correct, where does LLDB get the value for __IPHONE_OS_VERSION_MIN_REQUIRED when building the scratch context? Once I can track that down, I should be able to build a repro case more easily.

kastiglione commented 3 years ago

Robin, can you add the following to your ~/.lldbinit file, and then launch your app/target. You can use any log path, the tmp path is one example.

log enable -f /tmp/lldb.types.log lldb types

Also, can you share your linker invocation from your build log?

robinkunde commented 3 years ago

I've added my lldb.types.log. I can see that it's finding an iOS 9.0 triple. The build log mentions 9.0 in one of the env variables:

export DEPLOYMENT_TARGET_SUGGESTED_VALUES\=9.0\ 9.1\ 9.2\ 9.3\ 10.0\ 10.1\ 10.2\ 10.3\ 11.0\ 11.1\ 11.2\ 11.3\ 11.4\ 12.0\ 12.1\ 12.2\ 12.3\ 12.4\ 13.0\ 13.1\ 13.2\ 13.3\ 13.4\ 13.5\ 13.6\ 14.0\ 14.1\ 14.2

And also in compiler and linker invocation for some of my SwiftPM dependencies. I've attached one of those. I checked the package.swift file for those dependencies and it lists iOS 9.0 as the platform (https://github.com/devicekit/DeviceKit/blob/dbddb0296e3ee4f83d52f9681e42705020a533e4/Package.swift#L18). The app binary itself is linked and compiled against iOS 13.1 though.

kastiglione commented 3 years ago

From the log:

SwiftASTContext("AppName")::CreateInstance() – Found serialized triple arm64-apple-ios9.0.
SwiftASTContext("AppName")::SetTriple("arm64-apple-ios9.0") setting to "arm64-apple-ios9.0"

What is the value of the -target flag used to compile "AppName"? From these logs, it seems the swiftmodule was compiled with -target arm64-apple-ios9.0.

robinkunde commented 3 years ago

It's "arm64-apple-ios13.1" for AppName.

robinkunde commented 3 years ago

I could email you the whole build log. I just don't want to upload it here where it's public.

typesanitizer commented 3 years ago

If you would like to share a file privately, please attach it to a bug report submitted through feedbackassistant.apple.com. You can mention the Jira number in the Feedback to avoid duplicating all the information.

kastiglione commented 3 years ago

Thanks Varun, yes please file a feedback, a link to this and the build log works for me. When you do, please comment here with the radar number for fastest routing to me.

robinkunde commented 3 years ago

FB8975895

kastiglione commented 3 years ago

A reply was sent via the FB.

robinkunde commented 3 years ago

Thank you, I'll try to get to it tonight. I wanted to see if I can finally put together a minimal repro case with the information you provided.

robinkunde commented 3 years ago

Looks like I was (mostly) successful. I created an empty iOS app project, set minimum iOS version to 13.0, and added two SwiftPM dependencies.

  1. Intercom v9.1.0 has the offending #if statement in its Intercom.h header (https://github.com/intercom/intercom-ios/blob/9.1.0/Intercom.xcframework/ios-arm64_armv7/Intercom.framework/Headers/Intercom.h) but doesn't define a minimum iOS version in its Package.swift file (https://github.com/intercom/intercom-ios/blob/9.1.0/Package.swift)

  2. Alamofire v4.9.1 also does not define a minimum iOS version in its Package.swift file (https://github.com/Alamofire/Alamofire/blob/4.9.1/Package.swift)

Somehow, the combination of these two packages triggers the issue (although with slightly different symptoms). The new lldb.types file I've attached mentions that it "Found serialized triple arm64-apple-ios9.0". As you can see in the screenshot, when it stops at the breakpoint, the variable view in Xcode is blank, but instead of crashing then I try `po application` it says that it "cannot find 'application' in scope".

I hope this helps!

kastiglione commented 3 years ago

Thanks for the updated examples. Here's what I know now.

For lldb evaluate to evaluate expressions, such as `po application`, it creates an in memory Swift compiler. And just like when compiles apps, this swift compiler needs a target.

Each swiftmodule can contain compiler flags, including a target triple aka deployment target. Alamofire doesn't specify a deployment target, and when this Xcode project builds it, a default of iOS9 is being used. I'm surprised Alamofire doesn't inherit the deployment target of the project, so I've asked this.

Lldb loops over the swiftmodules, and sets its target triple to whatever the current swiftmodule says, with the result being that the last one is used. In the example project, Alamofire is the last one, and so lldb uses its iOS9 (which again is just a default). Lldb should probably use the minimum target it finds, not last one, but in this case the result would be the same either way.

Next, lldb goes to load Intercom, and when lldb's internal swift compiler parses Intercom's headers, it reaches the `#error` line in the header that indicates iOS10 or greater is required.

A possible fix is to use a custom Alamofire Package.swift that specifies `platforms: [.iOS(.v13)]`.

I'll look into making Lldb resilient in the face of such errors. Running `po application` doesn't need any of Intercom, so in theory that should still work.

Let me know if the suggested workaround works for you.

kastiglione commented 3 years ago

I just noticed four Alamofire warnings when building the example project, and they look like this:

Alamofire/Source/ResponseSerialization.swift:125:30: warning: conformance of 'DefaultDataResponse' to 'Response' is only available in iOS 10.0 or newer

So it looks like Alamofire should be specifying iOS 10 as its deployment target.

robinkunde commented 3 years ago

Thank you for the explanation.

I don't get any warnings when I compile the project with Xcode 12.4 or 12.3. Which version are you using? The way this conformance was implemented looks fine to me though (all codepaths behind `available` checks, and the property is a type-erased optional).

I'll be taking some steps to protect myself against these issues going forward, including auditing all my dependencies for platform specifications, asking the maintainers to add it, and forking if necessary. Intercom already removed the check in the header though, so it's not causing me any obvious issues right now.

Thank you for making the effort to improve resillience in LLDB.

I agree that the behavior of picking the minimum possible deployment target (iOS 9 is the minimum starting with Xcode 12.x, so for Xcode 11 it might pick iOS 8, but I can't confirm that at the moment) is a bit counter-intuitive. I haven't followed the evolution of SwiftPM much at all, but my guess is that it doesn't inherit the minimum from the project to prevent issues with API deprecation/removal. It seems like allowing Swift packages to be underspecified in this way could is this issue. I'll bring it up in the forum.

kastiglione commented 3 years ago

The reasoning for not inheriting the project's deployment target is: The deployment target is really a property of how the code in the package is written – compiling it with a newer deployment target would for example cause warnings and possibly errors depending on the availability annotations.

Requesting accurate deployment targets in swift packages sounds like a time saving contribution.

robinkunde commented 3 years ago

Got it. One more thing: Can you tell why Intercom by itself does not trigger this issue? Does it have to do with LLDB picking the last target and removing Alamofire jumbles the order in such a way that the last one isn't Intercom either?

kastiglione commented 3 years ago

Right, without Alamofire and its ios9 deployment target, lldb uses the higher deployment target of the main app.

robinkunde commented 3 years ago

What would you like me to do with the radar?

kastiglione commented 3 years ago

I think we can close it as is.