[llvm-branch-commits] [clang] [InstallAPI] Verify that declarations in headers map to exports found in dylib (PR #85348)
via llvm-branch-commits
llvm-branch-commits at lists.llvm.org
Thu Mar 14 18:52:14 PDT 2024
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang
Author: Cyndy Ishida (cyndyishida)
<details>
<summary>Changes</summary>
* This completes support for verifying every declaration found in a header is discovered in the dylib. Diagnostics are reported for each class for differences that are representable in TBD files.
* This patch also now captures unavailable attributes that depend on target triples. This is needed for proper tbd file generation.
---
Patch is 85.46 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/85348.diff
13 Files Affected:
- (modified) clang/include/clang/AST/Availability.h (+9-4)
- (modified) clang/include/clang/Basic/DiagnosticInstallAPIKinds.td (+23)
- (modified) clang/include/clang/InstallAPI/DylibVerifier.h (+51-6)
- (modified) clang/include/clang/InstallAPI/Frontend.h (+3)
- (modified) clang/include/clang/InstallAPI/MachO.h (+1)
- (modified) clang/lib/AST/Availability.cpp (+3-3)
- (modified) clang/lib/InstallAPI/DylibVerifier.cpp (+321-5)
- (modified) clang/lib/InstallAPI/Visitor.cpp (+1-1)
- (added) clang/test/InstallAPI/availability.test (+626)
- (added) clang/test/InstallAPI/diagnostics-cpp.test (+461)
- (added) clang/test/InstallAPI/hiddens.test (+262)
- (modified) clang/tools/clang-installapi/ClangInstallAPI.cpp (+2-4)
- (modified) clang/tools/clang-installapi/Options.cpp (+3-1)
``````````diff
diff --git a/clang/include/clang/AST/Availability.h b/clang/include/clang/AST/Availability.h
index 5cfbaf0cdfbd21..2ccc607d4b63dc 100644
--- a/clang/include/clang/AST/Availability.h
+++ b/clang/include/clang/AST/Availability.h
@@ -67,6 +67,7 @@ struct AvailabilityInfo {
VersionTuple Introduced;
VersionTuple Deprecated;
VersionTuple Obsoleted;
+ bool Unavailable = false;
bool UnconditionallyDeprecated = false;
bool UnconditionallyUnavailable = false;
@@ -78,6 +79,9 @@ struct AvailabilityInfo {
/// Check if the symbol has been obsoleted.
bool isObsoleted() const { return !Obsoleted.empty(); }
+ /// Check if the symbol is unavailable for the active platform and os version.
+ bool isUnavailable() const { return Unavailable; }
+
/// Check if the symbol is unconditionally deprecated.
///
/// i.e. \code __attribute__((deprecated)) \endcode
@@ -91,9 +95,10 @@ struct AvailabilityInfo {
}
AvailabilityInfo(StringRef Domain, VersionTuple I, VersionTuple D,
- VersionTuple O, bool UD, bool UU)
+ VersionTuple O, bool U, bool UD, bool UU)
: Domain(Domain), Introduced(I), Deprecated(D), Obsoleted(O),
- UnconditionallyDeprecated(UD), UnconditionallyUnavailable(UU) {}
+ Unavailable(U), UnconditionallyDeprecated(UD),
+ UnconditionallyUnavailable(UU) {}
friend bool operator==(const AvailabilityInfo &Lhs,
const AvailabilityInfo &Rhs);
@@ -105,10 +110,10 @@ struct AvailabilityInfo {
inline bool operator==(const AvailabilityInfo &Lhs,
const AvailabilityInfo &Rhs) {
return std::tie(Lhs.Introduced, Lhs.Deprecated, Lhs.Obsoleted,
- Lhs.UnconditionallyDeprecated,
+ Lhs.Unavailable, Lhs.UnconditionallyDeprecated,
Lhs.UnconditionallyUnavailable) ==
std::tie(Rhs.Introduced, Rhs.Deprecated, Rhs.Obsoleted,
- Rhs.UnconditionallyDeprecated,
+ Rhs.Unavailable, Rhs.UnconditionallyDeprecated,
Rhs.UnconditionallyUnavailable);
}
diff --git a/clang/include/clang/Basic/DiagnosticInstallAPIKinds.td b/clang/include/clang/Basic/DiagnosticInstallAPIKinds.td
index 31be4f09cf3a1c..5ed2e23425dc5f 100644
--- a/clang/include/clang/Basic/DiagnosticInstallAPIKinds.td
+++ b/clang/include/clang/Basic/DiagnosticInstallAPIKinds.td
@@ -17,4 +17,27 @@ def err_no_install_name : Error<"no install name specified: add -install_name <p
def err_no_output_file: Error<"no output file specified">;
} // end of command line category.
+let CategoryName = "Verification" in {
+def warn_target: Warning<"violations found for %0">;
+def err_library_missing_symbol : Error<"declaration has external linkage, but dynamic library doesn't have symbol '%0'">;
+def warn_library_missing_symbol : Warning<"declaration has external linkage, but dynamic library doesn't have symbol '%0'">;
+def err_library_hidden_symbol : Error<"declaration has external linkage, but symbol has internal linkage in dynamic library '%0'">;
+def warn_library_hidden_symbol : Warning<"declaration has external linkage, but symbol has internal linkage in dynamic library '%0'">;
+def warn_header_hidden_symbol : Warning<"symbol exported in dynamic library, but marked hidden in declaration '%0'">;
+def err_header_hidden_symbol : Error<"symbol exported in dynamic library, but marked hidden in declaration '%0'">;
+def err_header_symbol_missing : Error<"no declaration found for exported symbol '%0' in dynamic library">;
+def warn_header_availability_mismatch : Warning<"declaration '%0' is marked %select{available|unavailable}1,"
+ " but symbol is %select{not |}2exported in dynamic library">;
+def err_header_availability_mismatch : Error<"declaration '%0' is marked %select{available|unavailable}1,"
+ " but symbol is %select{not |}2exported in dynamic library">;
+def warn_dylib_symbol_flags_mismatch : Warning<"dynamic library symbol '%0' is "
+ "%select{weak defined|thread local}1, but its declaration is not">;
+def warn_header_symbol_flags_mismatch : Warning<"declaration '%0' is "
+ "%select{weak defined|thread local}1, but symbol is not in dynamic library">;
+def err_dylib_symbol_flags_mismatch : Error<"dynamic library symbol '%0' is "
+ "%select{weak defined|thread local}1, but its declaration is not">;
+def err_header_symbol_flags_mismatch : Error<"declaration '%0' is "
+ "%select{weak defined|thread local}1, but symbol is not in dynamic library">;
+} // end of Verification category.
+
} // end of InstallAPI component
diff --git a/clang/include/clang/InstallAPI/DylibVerifier.h b/clang/include/clang/InstallAPI/DylibVerifier.h
index 72c4743fdf65e0..2094548ced8d5f 100644
--- a/clang/include/clang/InstallAPI/DylibVerifier.h
+++ b/clang/include/clang/InstallAPI/DylibVerifier.h
@@ -38,19 +38,34 @@ class DylibVerifier {
// Current target being verified against the AST.
llvm::MachO::Target Target;
+ // Target specific API from binary.
+ RecordsSlice *DylibSlice;
+
// Query state of verification after AST has been traversed.
Result FrontendState;
// First error for AST traversal, which is tied to the target triple.
bool DiscoveredFirstError;
+
+ // Determines what kind of banner to print a violation for.
+ bool PrintArch = false;
+
+ // Engine for reporting violations.
+ DiagnosticsEngine *Diag = nullptr;
+
+ // Handle diagnostics reporting for target level violations.
+ void emitDiag(llvm::function_ref<void()> Report);
+
+ VerifierContext() = default;
+ VerifierContext(DiagnosticsEngine *Diag) : Diag(Diag) {}
};
DylibVerifier() = default;
DylibVerifier(llvm::MachO::Records &&Dylib, DiagnosticsEngine *Diag,
VerificationMode Mode, bool Demangle)
- : Dylib(std::move(Dylib)), Diag(Diag), Mode(Mode), Demangle(Demangle),
- Exports(std::make_unique<SymbolSet>()) {}
+ : Dylib(std::move(Dylib)), Mode(Mode), Demangle(Demangle),
+ Exports(std::make_unique<SymbolSet>()), Ctx(VerifierContext{Diag}) {}
Result verify(GlobalRecord *R, const FrontendAttrs *FA);
Result verify(ObjCInterfaceRecord *R, const FrontendAttrs *FA);
@@ -66,6 +81,13 @@ class DylibVerifier {
/// Get result of verification.
Result getState() const { return Ctx.FrontendState; }
+ /// Set different source managers to the same diagnostics engine.
+ void setSourceManager(SourceManager &SourceMgr) const {
+ if (!Ctx.Diag)
+ return;
+ Ctx.Diag->setSourceManager(&SourceMgr);
+ }
+
private:
/// Determine whether to compare declaration to symbol in binary.
bool canVerify();
@@ -73,6 +95,29 @@ class DylibVerifier {
/// Shared implementation for verifying exported symbols.
Result verifyImpl(Record *R, SymbolContext &SymCtx);
+ /// Check if declaration is marked as obsolete, they are
+ // expected to result in a symbol mismatch.
+ bool shouldIgnoreObsolete(const Record *R, SymbolContext &SymCtx,
+ const Record *DR);
+
+ /// Compare the visibility declarations to the linkage of symbol found in
+ /// dylib.
+ Result compareVisibility(const Record *R, SymbolContext &SymCtx,
+ const Record *DR);
+
+ /// An ObjCInterfaceRecord can represent up to three symbols. When verifying,
+ // account for this granularity.
+ bool compareObjCInterfaceSymbols(const Record *R, SymbolContext &SymCtx,
+ const ObjCInterfaceRecord *DR);
+
+ /// Validate availability annotations against dylib.
+ Result compareAvailability(const Record *R, SymbolContext &SymCtx,
+ const Record *DR);
+
+ /// Compare and validate matching symbol flags.
+ bool compareSymbolFlags(const Record *R, SymbolContext &SymCtx,
+ const Record *DR);
+
/// Update result state on each call to `verify`.
void updateState(Result State);
@@ -80,14 +125,14 @@ class DylibVerifier {
void addSymbol(const Record *R, SymbolContext &SymCtx,
TargetList &&Targets = {});
+ /// Find matching dylib slice for target triple that is being parsed.
+ void assignSlice(const Target &T);
+
// Symbols in dylib.
llvm::MachO::Records Dylib;
- // Engine for reporting violations.
- [[maybe_unused]] DiagnosticsEngine *Diag = nullptr;
-
// Controls what class of violations to report.
- [[maybe_unused]] VerificationMode Mode = VerificationMode::Invalid;
+ VerificationMode Mode = VerificationMode::Invalid;
// Attempt to demangle when reporting violations.
bool Demangle = false;
diff --git a/clang/include/clang/InstallAPI/Frontend.h b/clang/include/clang/InstallAPI/Frontend.h
index 660fc8cd69a59d..5cccd891c58093 100644
--- a/clang/include/clang/InstallAPI/Frontend.h
+++ b/clang/include/clang/InstallAPI/Frontend.h
@@ -17,6 +17,7 @@
#include "clang/Frontend/CompilerInstance.h"
#include "clang/Frontend/FrontendActions.h"
#include "clang/InstallAPI/Context.h"
+#include "clang/InstallAPI/DylibVerifier.h"
#include "clang/InstallAPI/Visitor.h"
#include "llvm/ADT/Twine.h"
#include "llvm/Support/MemoryBuffer.h"
@@ -34,6 +35,8 @@ class InstallAPIAction : public ASTFrontendAction {
std::unique_ptr<ASTConsumer> CreateASTConsumer(CompilerInstance &CI,
StringRef InFile) override {
+ Ctx.Diags->getClient()->BeginSourceFile(CI.getLangOpts());
+ Ctx.Verifier->setSourceManager(CI.getSourceManager());
return std::make_unique<InstallAPIVisitor>(
CI.getASTContext(), Ctx, CI.getSourceManager(), CI.getPreprocessor());
}
diff --git a/clang/include/clang/InstallAPI/MachO.h b/clang/include/clang/InstallAPI/MachO.h
index 6dee6f22420381..a77766511fa3e5 100644
--- a/clang/include/clang/InstallAPI/MachO.h
+++ b/clang/include/clang/InstallAPI/MachO.h
@@ -32,6 +32,7 @@ using ObjCInterfaceRecord = llvm::MachO::ObjCInterfaceRecord;
using ObjCCategoryRecord = llvm::MachO::ObjCCategoryRecord;
using ObjCIVarRecord = llvm::MachO::ObjCIVarRecord;
using Records = llvm::MachO::Records;
+using RecordsSlice = llvm::MachO::RecordsSlice;
using BinaryAttrs = llvm::MachO::RecordsSlice::BinaryAttrs;
using SymbolSet = llvm::MachO::SymbolSet;
using SimpleSymbol = llvm::MachO::SimpleSymbol;
diff --git a/clang/lib/AST/Availability.cpp b/clang/lib/AST/Availability.cpp
index d0054e37e4dcd2..238359a2dedfcf 100644
--- a/clang/lib/AST/Availability.cpp
+++ b/clang/lib/AST/Availability.cpp
@@ -28,9 +28,9 @@ AvailabilityInfo AvailabilityInfo::createFromDecl(const Decl *Decl) {
for (const auto *A : RD->specific_attrs<AvailabilityAttr>()) {
if (A->getPlatform()->getName() != PlatformName)
continue;
- Availability =
- AvailabilityInfo(A->getPlatform()->getName(), A->getIntroduced(),
- A->getDeprecated(), A->getObsoleted(), false, false);
+ Availability = AvailabilityInfo(
+ A->getPlatform()->getName(), A->getIntroduced(), A->getDeprecated(),
+ A->getObsoleted(), A->getUnavailable(), false, false);
break;
}
diff --git a/clang/lib/InstallAPI/DylibVerifier.cpp b/clang/lib/InstallAPI/DylibVerifier.cpp
index b7dd85d63fa14f..700763b3fee0db 100644
--- a/clang/lib/InstallAPI/DylibVerifier.cpp
+++ b/clang/lib/InstallAPI/DylibVerifier.cpp
@@ -1,5 +1,6 @@
#include "clang/InstallAPI/DylibVerifier.h"
#include "clang/InstallAPI/FrontendRecords.h"
+#include "clang/InstallAPI/InstallAPIDiagnostic.h"
#include "llvm/Demangle/Demangle.h"
using namespace llvm::MachO;
@@ -24,6 +25,9 @@ struct DylibVerifier::SymbolContext {
// The ObjCInterface symbol type, if applicable.
ObjCIFSymbolKind ObjCIFKind = ObjCIFSymbolKind::None;
+
+ // Whether Decl is inlined.
+ bool Inlined = false;
};
static std::string
@@ -80,10 +84,11 @@ getAnnotatedName(const Record *R, EncodeKind Kind, StringRef Name,
}
static std::string demangle(StringRef Name) {
- // Itanium encoding requires 1 or 3 leading underscores, followed by 'Z'.
- if (!(Name.starts_with("_Z") || Name.starts_with("___Z")))
+ // InstallAPI currently only supports itanium manglings.
+ if (!(Name.starts_with("_Z") || Name.starts_with("__Z") ||
+ Name.starts_with("___Z")))
return Name.str();
- char *Result = llvm::itaniumDemangle(Name.data());
+ char *Result = llvm::itaniumDemangle(Name);
if (!Result)
return Name.str();
@@ -109,6 +114,30 @@ static DylibVerifier::Result updateResult(const DylibVerifier::Result Prev,
return Curr;
}
+// __private_extern__ is a deprecated specifier that clang does not
+// respect in all contexts, it should just be considered hidden for InstallAPI.
+static bool shouldIgnorePrivateExternAttr(const Decl *D) {
+ if (const FunctionDecl *FD = cast<FunctionDecl>(D))
+ return FD->getStorageClass() == StorageClass::SC_PrivateExtern;
+ if (const VarDecl *VD = cast<VarDecl>(D))
+ return VD->getStorageClass() == StorageClass::SC_PrivateExtern;
+
+ return false;
+}
+
+Record *findRecordFromSlice(const RecordsSlice *Slice, StringRef Name,
+ EncodeKind Kind) {
+ switch (Kind) {
+ case EncodeKind::GlobalSymbol:
+ return Slice->findGlobal(Name);
+ case EncodeKind::ObjectiveCInstanceVariable:
+ return Slice->findObjCIVar(Name.contains('.'), Name);
+ case EncodeKind::ObjectiveCClass:
+ case EncodeKind::ObjectiveCClassEHType:
+ return Slice->findObjCInterface(Name);
+ }
+ llvm_unreachable("unexpected end when finding record");
+}
void DylibVerifier::updateState(Result State) {
Ctx.FrontendState = updateResult(Ctx.FrontendState, State);
@@ -122,17 +151,272 @@ void DylibVerifier::addSymbol(const Record *R, SymbolContext &SymCtx,
Exports->addGlobal(SymCtx.Kind, SymCtx.SymbolName, R->getFlags(), Targets);
}
+bool DylibVerifier::shouldIgnoreObsolete(const Record *R, SymbolContext &SymCtx,
+ const Record *DR) {
+ return SymCtx.FA->Avail.isObsoleted();
+}
+
+bool DylibVerifier::compareObjCInterfaceSymbols(const Record *R,
+ SymbolContext &SymCtx,
+ const ObjCInterfaceRecord *DR) {
+ const bool IsDeclVersionComplete =
+ ((SymCtx.ObjCIFKind & ObjCIFSymbolKind::Class) ==
+ ObjCIFSymbolKind::Class) &&
+ ((SymCtx.ObjCIFKind & ObjCIFSymbolKind::MetaClass) ==
+ ObjCIFSymbolKind::MetaClass);
+
+ const bool IsDylibVersionComplete = DR->isCompleteInterface();
+
+ // The common case, a complete ObjCInterface.
+ if (IsDeclVersionComplete && IsDylibVersionComplete)
+ return true;
+
+ auto PrintDiagnostic = [&](auto SymLinkage, const Record *Record,
+ StringRef SymName, bool PrintAsWarning = false) {
+ if (SymLinkage == RecordLinkage::Unknown)
+ Ctx.emitDiag([&]() {
+ Ctx.Diag->Report(SymCtx.FA->D->getLocation(),
+ PrintAsWarning ? diag::warn_library_missing_symbol
+ : diag::err_library_missing_symbol)
+ << SymName;
+ });
+ else
+ Ctx.emitDiag([&]() {
+ Ctx.Diag->Report(SymCtx.FA->D->getLocation(),
+ PrintAsWarning ? diag::warn_library_hidden_symbol
+ : diag::err_library_hidden_symbol)
+ << SymName;
+ });
+ };
+
+ if (IsDeclVersionComplete) {
+ // The decl represents a complete ObjCInterface, but the symbols in the
+ // dylib do not. Determine which symbol is missing. To keep older projects
+ // building, treat this as a warning.
+ if (!DR->isExportedSymbol(ObjCIFSymbolKind::Class))
+ PrintDiagnostic(DR->getLinkageForSymbol(ObjCIFSymbolKind::Class), R,
+ getAnnotatedName(R, SymCtx.Kind, SymCtx.PrettyPrintName,
+ /*ValidSourceLoc=*/true,
+ ObjCIFSymbolKind::Class),
+ /*PrintAsWarning=*/true);
+
+ if (!DR->isExportedSymbol(ObjCIFSymbolKind::MetaClass))
+ PrintDiagnostic(DR->getLinkageForSymbol(ObjCIFSymbolKind::MetaClass), R,
+ getAnnotatedName(R, SymCtx.Kind, SymCtx.PrettyPrintName,
+ /*ValidSourceLoc=*/true,
+ ObjCIFSymbolKind::MetaClass),
+ /*PrintAsWarning=*/true);
+ return true;
+ }
+
+ if (DR->isExportedSymbol(SymCtx.ObjCIFKind)) {
+ if (!IsDylibVersionComplete) {
+ // Both the declaration and dylib have a non-complete interface.
+ SymCtx.Kind = EncodeKind::GlobalSymbol;
+ SymCtx.SymbolName = R->getName();
+ }
+ return true;
+ }
+
+ // At this point that means there was not a matching class symbol
+ // to represent the one discovered as a declaration.
+ PrintDiagnostic(DR->getLinkageForSymbol(SymCtx.ObjCIFKind), R,
+ SymCtx.PrettyPrintName);
+ return false;
+}
+
+DylibVerifier::Result DylibVerifier::compareVisibility(const Record *R,
+ SymbolContext &SymCtx,
+ const Record *DR) {
+
+ if (R->isExported()) {
+ if (!DR) {
+ Ctx.emitDiag([&]() {
+ Ctx.Diag->Report(SymCtx.FA->D->getLocation(),
+ diag::err_library_missing_symbol)
+ << SymCtx.PrettyPrintName;
+ });
+ return Result::Invalid;
+ }
+ if (DR->isInternal()) {
+ Ctx.emitDiag([&]() {
+ Ctx.Diag->Report(SymCtx.FA->D->getLocation(),
+ diag::err_library_hidden_symbol)
+ << SymCtx.PrettyPrintName;
+ });
+ return Result::Invalid;
+ }
+ }
+
+ // Emit a diagnostic for hidden declarations with external symbols, except
+ // when theres an inlined attribute.
+ if ((R->isInternal() && !SymCtx.Inlined) && DR && DR->isExported()) {
+
+ if (Mode == VerificationMode::ErrorsOnly)
+ return Result::Ignore;
+
+ if (shouldIgnorePrivateExternAttr(SymCtx.FA->D))
+ return Result::Ignore;
+
+ unsigned ID;
+ Result Outcome;
+ if (Mode == VerificationMode::ErrorsAndWarnings) {
+ ID = diag::warn_header_hidden_symbol;
+ Outcome = Result::Ignore;
+ } else {
+ ID = diag::err_header_hidden_symbol;
+ Outcome = Result::Invalid;
+ }
+ Ctx.emitDiag([&]() {
+ Ctx.Diag->Report(SymCtx.FA->D->getLocation(), ID)
+ << SymCtx.PrettyPrintName;
+ });
+ return Outcome;
+ }
+
+ if (R->isInternal())
+ return Result::Ignore;
+
+ return Result::Valid;
+}
+
+DylibVerifier::Result DylibVerifier::compareAvailability(const Record *R,
+ SymbolContext &SymCtx,
+ const Record *DR) {
+ if (!SymCtx.FA->Avail.isUnavailable())
+ return Result::Valid;
+
+ const bool IsDeclAvailable = SymCtx.FA->Avail.isUnavailable();
+
+ switch (Mode) {
+ case VerificationMode::ErrorsAndWarnings:
+ Ctx.emitDiag([&]() {
+ Ctx.Diag->Report(SymCtx.FA->D->getLocation(),
+ diag::warn_header_availability_mismatch)
+ << SymCtx.PrettyPrintName << IsDeclAvailable << IsDeclAvailable;
+ });
+ return Result::Ignore;
+ case VerificationMode::Pedantic:
+ Ctx.emitDiag([&]() {
+ Ctx.Diag->Report(SymCtx.FA->D->getLocation(),
+ diag::err_header_availability_mismatch)
+ << SymCtx.PrettyPrintName << IsDeclAvailable << IsDeclAvailable;
+ });
+ return Result::Invalid;
+ case VerificationMode::ErrorsOnly:
+ return Result::Ignore;
+ case VerificationMode::Invalid:
+ llvm_unreachable("Unexpected verification mode symbol verification");
+ }
+ llvm_unreachable("Unexpected verification mode symbol verification");
+}
+
+bool DylibVerifier::compareSymbolFlags(const Record *R, SymbolContext &SymCtx,
+ const Record *DR) {
+ std::string DisplayName =
+ Demangle ? demangle(DR->getName()) : DR->getName().str();
+
+ if (DR->isThreadLocalValue() && !R->isThreadLocalValue()) {
+ Ctx.emitDiag([&]() {
+ Ctx.Diag->Report(SymCtx.FA->D->getLocation(),
+ diag::err_dylib_symbol_flags_mismatch)
+ << getAnnotatedName(DR, SymCtx.Kind, DisplayName)
+ << DR->isThreadLocalValue();
+ });
+ return false;
+ }
+ i...
[truncated]
``````````
</details>
https://github.com/llvm/llvm-project/pull/85348
More information about the llvm-branch-commits
mailing list