[clang] [InstallAPI] Tie lifetime of FE objects to DylibVerifier (PR #88189)
Cyndy Ishida via cfe-commits
cfe-commits at lists.llvm.org
Tue Apr 9 13:35:10 PDT 2024
https://github.com/cyndyishida created https://github.com/llvm/llvm-project/pull/88189
A few verification checks need to happen until all AST's have been traversed, specifically for zippered framework checking. To keep source location until that time valid, hold onto to references of FrontendRecords + SourceManager.
>From d74c47dcadaa83422637de6ab6b1d0852b07817c Mon Sep 17 00:00:00 2001
From: Cyndy Ishida <cyndy_ishida at apple.com>
Date: Tue, 9 Apr 2024 13:04:18 -0700
Subject: [PATCH] [InstallAPI] Tie lifetime of FE objects to DylibVerifier
A few verification checks need to happen until all AST's have been
traversed, specifically for zippered framework checking.
To keep source location until that time valid, hold onto to
references of FrontendRecords + SourceManager.
---
.../include/clang/InstallAPI/DylibVerifier.h | 10 ++--
clang/include/clang/InstallAPI/Frontend.h | 2 +-
.../clang/InstallAPI/FrontendRecords.h | 1 +
clang/lib/InstallAPI/DylibVerifier.cpp | 47 +++++++++----------
clang/lib/InstallAPI/Frontend.cpp | 15 +++---
.../clang-installapi/ClangInstallAPI.cpp | 2 +
6 files changed, 40 insertions(+), 37 deletions(-)
diff --git a/clang/include/clang/InstallAPI/DylibVerifier.h b/clang/include/clang/InstallAPI/DylibVerifier.h
index 07dbd3bf5f2b10..a3df25f10de4b1 100644
--- a/clang/include/clang/InstallAPI/DylibVerifier.h
+++ b/clang/include/clang/InstallAPI/DylibVerifier.h
@@ -10,6 +10,7 @@
#define LLVM_CLANG_INSTALLAPI_DYLIBVERIFIER_H
#include "clang/Basic/Diagnostic.h"
+#include "clang/Basic/SourceManager.h"
#include "clang/InstallAPI/MachO.h"
namespace clang {
@@ -99,11 +100,7 @@ class DylibVerifier : llvm::MachO::RecordVisitor {
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);
- }
+ void setSourceManager(IntrusiveRefCntPtr<SourceManager> SourceMgr);
private:
/// Determine whether to compare declaration to symbol in binary.
@@ -190,6 +187,9 @@ class DylibVerifier : llvm::MachO::RecordVisitor {
// Track DWARF provided source location for dylibs.
DWARFContext *DWARFCtx = nullptr;
+
+ // Source manager for each unique compiler instance.
+ llvm::SmallVector<IntrusiveRefCntPtr<SourceManager>, 12> SourceManagers;
};
} // namespace installapi
diff --git a/clang/include/clang/InstallAPI/Frontend.h b/clang/include/clang/InstallAPI/Frontend.h
index 5cccd891c58093..bc4e77de2b7256 100644
--- a/clang/include/clang/InstallAPI/Frontend.h
+++ b/clang/include/clang/InstallAPI/Frontend.h
@@ -36,7 +36,7 @@ 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());
+ Ctx.Verifier->setSourceManager(CI.getSourceManagerPtr());
return std::make_unique<InstallAPIVisitor>(
CI.getASTContext(), Ctx, CI.getSourceManager(), CI.getPreprocessor());
}
diff --git a/clang/include/clang/InstallAPI/FrontendRecords.h b/clang/include/clang/InstallAPI/FrontendRecords.h
index 59271e81e230c2..ef82398addd7ac 100644
--- a/clang/include/clang/InstallAPI/FrontendRecords.h
+++ b/clang/include/clang/InstallAPI/FrontendRecords.h
@@ -21,6 +21,7 @@ namespace installapi {
struct FrontendAttrs {
const AvailabilityInfo Avail;
const Decl *D;
+ const SourceLocation Loc;
const HeaderType Access;
};
diff --git a/clang/lib/InstallAPI/DylibVerifier.cpp b/clang/lib/InstallAPI/DylibVerifier.cpp
index 2387ee0e78ad56..4fa2d4e9292c72 100644
--- a/clang/lib/InstallAPI/DylibVerifier.cpp
+++ b/clang/lib/InstallAPI/DylibVerifier.cpp
@@ -214,16 +214,16 @@ bool DylibVerifier::compareObjCInterfaceSymbols(const Record *R,
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)
+ Ctx.Diag->Report(SymCtx.FA->Loc, 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)
+ Ctx.Diag->Report(SymCtx.FA->Loc, PrintAsWarning
+ ? diag::warn_library_hidden_symbol
+ : diag::err_library_hidden_symbol)
<< SymName;
});
};
@@ -270,16 +270,14 @@ DylibVerifier::Result DylibVerifier::compareVisibility(const Record *R,
if (R->isExported()) {
if (!DR) {
Ctx.emitDiag([&]() {
- Ctx.Diag->Report(SymCtx.FA->D->getLocation(),
- diag::err_library_missing_symbol)
+ Ctx.Diag->Report(SymCtx.FA->Loc, diag::err_library_missing_symbol)
<< getAnnotatedName(R, SymCtx);
});
return Result::Invalid;
}
if (DR->isInternal()) {
Ctx.emitDiag([&]() {
- Ctx.Diag->Report(SymCtx.FA->D->getLocation(),
- diag::err_library_hidden_symbol)
+ Ctx.Diag->Report(SymCtx.FA->Loc, diag::err_library_hidden_symbol)
<< getAnnotatedName(R, SymCtx);
});
return Result::Invalid;
@@ -306,8 +304,7 @@ DylibVerifier::Result DylibVerifier::compareVisibility(const Record *R,
Outcome = Result::Invalid;
}
Ctx.emitDiag([&]() {
- Ctx.Diag->Report(SymCtx.FA->D->getLocation(), ID)
- << getAnnotatedName(R, SymCtx);
+ Ctx.Diag->Report(SymCtx.FA->Loc, ID) << getAnnotatedName(R, SymCtx);
});
return Outcome;
}
@@ -329,15 +326,13 @@ DylibVerifier::Result DylibVerifier::compareAvailability(const Record *R,
switch (Mode) {
case VerificationMode::ErrorsAndWarnings:
Ctx.emitDiag([&]() {
- Ctx.Diag->Report(SymCtx.FA->D->getLocation(),
- diag::warn_header_availability_mismatch)
+ Ctx.Diag->Report(SymCtx.FA->Loc, diag::warn_header_availability_mismatch)
<< getAnnotatedName(R, SymCtx) << IsDeclAvailable << IsDeclAvailable;
});
return Result::Ignore;
case VerificationMode::Pedantic:
Ctx.emitDiag([&]() {
- Ctx.Diag->Report(SymCtx.FA->D->getLocation(),
- diag::err_header_availability_mismatch)
+ Ctx.Diag->Report(SymCtx.FA->Loc, diag::err_header_availability_mismatch)
<< getAnnotatedName(R, SymCtx) << IsDeclAvailable << IsDeclAvailable;
});
return Result::Invalid;
@@ -353,16 +348,14 @@ bool DylibVerifier::compareSymbolFlags(const Record *R, SymbolContext &SymCtx,
const Record *DR) {
if (DR->isThreadLocalValue() && !R->isThreadLocalValue()) {
Ctx.emitDiag([&]() {
- Ctx.Diag->Report(SymCtx.FA->D->getLocation(),
- diag::err_dylib_symbol_flags_mismatch)
+ Ctx.Diag->Report(SymCtx.FA->Loc, diag::err_dylib_symbol_flags_mismatch)
<< getAnnotatedName(DR, SymCtx) << DR->isThreadLocalValue();
});
return false;
}
if (!DR->isThreadLocalValue() && R->isThreadLocalValue()) {
Ctx.emitDiag([&]() {
- Ctx.Diag->Report(SymCtx.FA->D->getLocation(),
- diag::err_header_symbol_flags_mismatch)
+ Ctx.Diag->Report(SymCtx.FA->Loc, diag::err_header_symbol_flags_mismatch)
<< getAnnotatedName(R, SymCtx) << R->isThreadLocalValue();
});
return false;
@@ -370,16 +363,14 @@ bool DylibVerifier::compareSymbolFlags(const Record *R, SymbolContext &SymCtx,
if (DR->isWeakDefined() && !R->isWeakDefined()) {
Ctx.emitDiag([&]() {
- Ctx.Diag->Report(SymCtx.FA->D->getLocation(),
- diag::err_dylib_symbol_flags_mismatch)
+ Ctx.Diag->Report(SymCtx.FA->Loc, diag::err_dylib_symbol_flags_mismatch)
<< getAnnotatedName(DR, SymCtx) << R->isWeakDefined();
});
return false;
}
if (!DR->isWeakDefined() && R->isWeakDefined()) {
Ctx.emitDiag([&]() {
- Ctx.Diag->Report(SymCtx.FA->D->getLocation(),
- diag::err_header_symbol_flags_mismatch)
+ Ctx.Diag->Report(SymCtx.FA->Loc, diag::err_header_symbol_flags_mismatch)
<< getAnnotatedName(R, SymCtx) << R->isWeakDefined();
});
return false;
@@ -487,6 +478,14 @@ void DylibVerifier::setTarget(const Target &T) {
assignSlice(T);
}
+void DylibVerifier::setSourceManager(
+ IntrusiveRefCntPtr<SourceManager> SourceMgr) {
+ if (!Ctx.Diag)
+ return;
+ SourceManagers.push_back(std::move(SourceMgr));
+ Ctx.Diag->setSourceManager(SourceManagers.back().get());
+}
+
DylibVerifier::Result DylibVerifier::verify(ObjCIVarRecord *R,
const FrontendAttrs *FA,
const StringRef SuperClass) {
diff --git a/clang/lib/InstallAPI/Frontend.cpp b/clang/lib/InstallAPI/Frontend.cpp
index bd7589c13e043f..04d06f46d26520 100644
--- a/clang/lib/InstallAPI/Frontend.cpp
+++ b/clang/lib/InstallAPI/Frontend.cpp
@@ -23,7 +23,8 @@ std::pair<GlobalRecord *, FrontendAttrs *> FrontendRecordsSlice::addGlobal(
GlobalRecord *GR =
llvm::MachO::RecordsSlice::addGlobal(Name, Linkage, GV, Flags, Inlined);
- auto Result = FrontendRecords.insert({GR, FrontendAttrs{Avail, D, Access}});
+ auto Result = FrontendRecords.insert(
+ {GR, FrontendAttrs{Avail, D, D->getLocation(), Access}});
return {GR, &(Result.first->second)};
}
@@ -39,8 +40,8 @@ FrontendRecordsSlice::addObjCInterface(StringRef Name, RecordLinkage Linkage,
ObjCInterfaceRecord *ObjCR =
llvm::MachO::RecordsSlice::addObjCInterface(Name, Linkage, SymType);
- auto Result =
- FrontendRecords.insert({ObjCR, FrontendAttrs{Avail, D, Access}});
+ auto Result = FrontendRecords.insert(
+ {ObjCR, FrontendAttrs{Avail, D, D->getLocation(), Access}});
return {ObjCR, &(Result.first->second)};
}
@@ -51,8 +52,8 @@ FrontendRecordsSlice::addObjCCategory(StringRef ClassToExtend,
const Decl *D, HeaderType Access) {
ObjCCategoryRecord *ObjCR =
llvm::MachO::RecordsSlice::addObjCCategory(ClassToExtend, CategoryName);
- auto Result =
- FrontendRecords.insert({ObjCR, FrontendAttrs{Avail, D, Access}});
+ auto Result = FrontendRecords.insert(
+ {ObjCR, FrontendAttrs{Avail, D, D->getLocation(), Access}});
return {ObjCR, &(Result.first->second)};
}
@@ -67,8 +68,8 @@ std::pair<ObjCIVarRecord *, FrontendAttrs *> FrontendRecordsSlice::addObjCIVar(
Linkage = RecordLinkage::Internal;
ObjCIVarRecord *ObjCR =
llvm::MachO::RecordsSlice::addObjCIVar(Container, IvarName, Linkage);
- auto Result =
- FrontendRecords.insert({ObjCR, FrontendAttrs{Avail, D, Access}});
+ auto Result = FrontendRecords.insert(
+ {ObjCR, FrontendAttrs{Avail, D, D->getLocation(), Access}});
return {ObjCR, &(Result.first->second)};
}
diff --git a/clang/tools/clang-installapi/ClangInstallAPI.cpp b/clang/tools/clang-installapi/ClangInstallAPI.cpp
index 0c1980ba5d6280..fd71aaec59435b 100644
--- a/clang/tools/clang-installapi/ClangInstallAPI.cpp
+++ b/clang/tools/clang-installapi/ClangInstallAPI.cpp
@@ -121,6 +121,7 @@ static bool run(ArrayRef<const char *> Args, const char *ProgName) {
// Execute, verify and gather AST results.
// An invocation is ran for each unique target triple and for each header
// access level.
+ Records FrontendRecords;
for (const auto &[Targ, Trip] : Opts.DriverOpts.Targets) {
Ctx.Verifier->setTarget(Targ);
Ctx.Slice = std::make_shared<FrontendRecordsSlice>(Trip);
@@ -131,6 +132,7 @@ static bool run(ArrayRef<const char *> Args, const char *ProgName) {
InMemoryFileSystem.get(), Opts.getClangFrontendArgs()))
return EXIT_FAILURE;
}
+ FrontendRecords.emplace_back(std::move(Ctx.Slice));
}
if (Ctx.Verifier->verifyRemainingSymbols() == DylibVerifier::Result::Invalid)
More information about the cfe-commits
mailing list