Skip to content

Implement a warning that detects Swift compile units that were compiled with #3361

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

Merged
merged 5 commits into from
Oct 14, 2021

Conversation

adrian-prantl
Copy link

Implement a warning that detects Swift compile units that were compiled with a different Swift compiler than the one that is embedded in LLDB.

rdar://82982242

# RUN: %target-swiftc -g \
# RUN: -module-cache-path %t/cache %S/Inputs/main.swift \
# RUN: -module-name main -o a.ll -emit-ir
# RUN: sed -i -e 's/producer: "Swift [^"]*"/producer: "Swift (swiftlang-9999.8.7.6)"/g' a.ll

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cool

@@ -241,6 +241,9 @@ let Definition = "process" in {
def WarningUnsupportedLanguage: Property<"unsupported-language-warnings", "Boolean">,
DefaultTrue,
Desc<"If true, warn when stopped in code that is written in a source language that LLDB does not support.">;
def WarningToolchainMismatch: Property<"toolchain-mismatch-warnings", "Boolean">,

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Which scenarios would people want this off?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any warning that potentially prints on every frame should be switchable. More specifically, I can see how this warning gets annoying when you use LLDB to debug or symbolicate crashes of previously built production software where you just don't have access to a historical matching LLDB.

@adrian-prantl
Copy link
Author

Also added a sanity check to the test.

@adrian-prantl
Copy link
Author

@JDevlieghere Do you think it's reasonable to expect the swiftlang tag to match or should we just match the three most significant components of the version number?

@kastiglione
Copy link

One thing to add: this change may be complimentary, or redundant, to a change in swiftc where it will begin to embed the sdk into swiftmodules. When that happens, deserializing such swiftmodules will return a status of SDKMismatch. See swiftlang/swift-driver#838 and swiftlang/swift#37768.

This has the nice side-effect that it can actually store the quadruple version numbers that Apple's tools are using nowadays.

rdar://82982162

Differential Revision: https://reviews.llvm.org/D111200

(cherry picked from commit 14aa3f3)

 Conflicts:
	lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
(cherry picked from commit 2edb905)
This patch adds support for Swift compiler producer strings to DWARFUnit.

Differential Revision: https://reviews.llvm.org/D111278

(cherry picked from commit aab7afeec1fb771c8ae044257d44f6362d72c732)
@adrian-prantl adrian-prantl force-pushed the 82982242 branch 2 times, most recently from 4aa3e49 to 58c5b07 Compare October 9, 2021 00:54
@adrian-prantl
Copy link
Author

After discussing this with @JDevlieghere I changed the check to leave off the build number, to reduce the risk of benign mismatches triggering false positives. Due to procedural reasons the compiler and debugger may be legitimately a few numbers apart.

@adrian-prantl
Copy link
Author

@swift-ci test

@adrian-prantl
Copy link
Author

@swift-ci test

@adrian-prantl
Copy link
Author

@swift-ci test

3 similar comments
@adrian-prantl
Copy link
Author

@swift-ci test

@adrian-prantl
Copy link
Author

@swift-ci test

@shahmishal
Copy link
Member

@swift-ci test

@@ -93,6 +93,9 @@ class ProcessProperties : public Properties {
void SetDetachKeepsStopped(bool keep_stopped);
bool GetWarningsOptimization() const;
bool GetWarningsUnsupportedLanguage() const;
#ifdef LLDB_ENABLE_SWIFT
bool GetWarningsToolchainMismatch() const;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As a follow up patch, can we get rid of this function and the PrintWarning.* ones below by moving them into the language plugin?

Comment on lines +612 to +615
if (GetProducer() == eProducerClang)
return GetProducerVersion() >= llvm::VersionTuple(425, 0, 13);
// Assume all other compilers didn't have incorrect ObjC bitfield info.
return true;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why can't this live upstream?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a cherry-pick.

@adrian-prantl
Copy link
Author

@swift-ci test

1 similar comment
@adrian-prantl
Copy link
Author

@swift-ci test

@adrian-prantl
Copy link
Author

@swift-ci test

@adrian-prantl
Copy link
Author

@swift-ci test

…ed with

a different Swift compiler than the one that is embedded in LLDB.

rdar://82982242
@adrian-prantl
Copy link
Author

@swift-ci test

1 similar comment
@adrian-prantl
Copy link
Author

@swift-ci test

@adrian-prantl adrian-prantl merged commit 193a81e into swiftlang:stable/20210726 Oct 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants