Skip to content

Commit f04452d

Browse files
cyndyishidacyndyishida
authored
[InstallAPI] Tie lifetime of FE objects to DylibVerifier (#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.
1 parent eec41d2 commit f04452d

File tree

6 files changed

+40
-37
lines changed

6 files changed

+40
-37
lines changed

‎clang/include/clang/InstallAPI/DylibVerifier.h

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
#define LLVM_CLANG_INSTALLAPI_DYLIBVERIFIER_H
1111

1212
#include "clang/Basic/Diagnostic.h"
13+
#include "clang/Basic/SourceManager.h"
1314
#include "clang/InstallAPI/MachO.h"
1415

1516
namespace clang {
@@ -99,11 +100,7 @@ class DylibVerifier : llvm::MachO::RecordVisitor {
99100
Result getState() const { return Ctx.FrontendState; }
100101

101102
/// Set different source managers to the same diagnostics engine.
102-
void setSourceManager(SourceManager &SourceMgr) const {
103-
if (!Ctx.Diag)
104-
return;
105-
Ctx.Diag->setSourceManager(&SourceMgr);
106-
}
103+
void setSourceManager(IntrusiveRefCntPtr<SourceManager> SourceMgr);
107104

108105
private:
109106
/// Determine whether to compare declaration to symbol in binary.
@@ -190,6 +187,9 @@ class DylibVerifier : llvm::MachO::RecordVisitor {
190187

191188
// Track DWARF provided source location for dylibs.
192189
DWARFContext *DWARFCtx = nullptr;
190+
191+
// Source manager for each unique compiler instance.
192+
llvm::SmallVector<IntrusiveRefCntPtr<SourceManager>, 12> SourceManagers;
193193
};
194194

195195
} // namespace installapi

‎clang/include/clang/InstallAPI/Frontend.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ class InstallAPIAction : public ASTFrontendAction {
3636
std::unique_ptr<ASTConsumer> CreateASTConsumer(CompilerInstance &CI,
3737
StringRef InFile) override {
3838
Ctx.Diags->getClient()->BeginSourceFile(CI.getLangOpts());
39-
Ctx.Verifier->setSourceManager(CI.getSourceManager());
39+
Ctx.Verifier->setSourceManager(CI.getSourceManagerPtr());
4040
return std::make_unique<InstallAPIVisitor>(
4141
CI.getASTContext(), Ctx, CI.getSourceManager(), CI.getPreprocessor());
4242
}

‎clang/include/clang/InstallAPI/FrontendRecords.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ namespace installapi {
2121
struct FrontendAttrs {
2222
const AvailabilityInfo Avail;
2323
const Decl *D;
24+
const SourceLocation Loc;
2425
const HeaderType Access;
2526
};
2627

‎clang/lib/InstallAPI/DylibVerifier.cpp

Lines changed: 23 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -214,16 +214,16 @@ bool DylibVerifier::compareObjCInterfaceSymbols(const Record *R,
214214
StringRef SymName, bool PrintAsWarning = false) {
215215
if (SymLinkage == RecordLinkage::Unknown)
216216
Ctx.emitDiag([&]() {
217-
Ctx.Diag->Report(SymCtx.FA->D->getLocation(),
218-
PrintAsWarning ? diag::warn_library_missing_symbol
219-
: diag::err_library_missing_symbol)
217+
Ctx.Diag->Report(SymCtx.FA->Loc, PrintAsWarning
218+
? diag::warn_library_missing_symbol
219+
: diag::err_library_missing_symbol)
220220
<< SymName;
221221
});
222222
else
223223
Ctx.emitDiag([&]() {
224-
Ctx.Diag->Report(SymCtx.FA->D->getLocation(),
225-
PrintAsWarning ? diag::warn_library_hidden_symbol
226-
: diag::err_library_hidden_symbol)
224+
Ctx.Diag->Report(SymCtx.FA->Loc, PrintAsWarning
225+
? diag::warn_library_hidden_symbol
226+
: diag::err_library_hidden_symbol)
227227
<< SymName;
228228
});
229229
};
@@ -270,16 +270,14 @@ DylibVerifier::Result DylibVerifier::compareVisibility(const Record *R,
270270
if (R->isExported()) {
271271
if (!DR) {
272272
Ctx.emitDiag([&]() {
273-
Ctx.Diag->Report(SymCtx.FA->D->getLocation(),
274-
diag::err_library_missing_symbol)
273+
Ctx.Diag->Report(SymCtx.FA->Loc, diag::err_library_missing_symbol)
275274
<< getAnnotatedName(R, SymCtx);
276275
});
277276
return Result::Invalid;
278277
}
279278
if (DR->isInternal()) {
280279
Ctx.emitDiag([&]() {
281-
Ctx.Diag->Report(SymCtx.FA->D->getLocation(),
282-
diag::err_library_hidden_symbol)
280+
Ctx.Diag->Report(SymCtx.FA->Loc, diag::err_library_hidden_symbol)
283281
<< getAnnotatedName(R, SymCtx);
284282
});
285283
return Result::Invalid;
@@ -306,8 +304,7 @@ DylibVerifier::Result DylibVerifier::compareVisibility(const Record *R,
306304
Outcome = Result::Invalid;
307305
}
308306
Ctx.emitDiag([&]() {
309-
Ctx.Diag->Report(SymCtx.FA->D->getLocation(), ID)
310-
<< getAnnotatedName(R, SymCtx);
307+
Ctx.Diag->Report(SymCtx.FA->Loc, ID) << getAnnotatedName(R, SymCtx);
311308
});
312309
return Outcome;
313310
}
@@ -329,15 +326,13 @@ DylibVerifier::Result DylibVerifier::compareAvailability(const Record *R,
329326
switch (Mode) {
330327
case VerificationMode::ErrorsAndWarnings:
331328
Ctx.emitDiag([&]() {
332-
Ctx.Diag->Report(SymCtx.FA->D->getLocation(),
333-
diag::warn_header_availability_mismatch)
329+
Ctx.Diag->Report(SymCtx.FA->Loc, diag::warn_header_availability_mismatch)
334330
<< getAnnotatedName(R, SymCtx) << IsDeclAvailable << IsDeclAvailable;
335331
});
336332
return Result::Ignore;
337333
case VerificationMode::Pedantic:
338334
Ctx.emitDiag([&]() {
339-
Ctx.Diag->Report(SymCtx.FA->D->getLocation(),
340-
diag::err_header_availability_mismatch)
335+
Ctx.Diag->Report(SymCtx.FA->Loc, diag::err_header_availability_mismatch)
341336
<< getAnnotatedName(R, SymCtx) << IsDeclAvailable << IsDeclAvailable;
342337
});
343338
return Result::Invalid;
@@ -353,33 +348,29 @@ bool DylibVerifier::compareSymbolFlags(const Record *R, SymbolContext &SymCtx,
353348
const Record *DR) {
354349
if (DR->isThreadLocalValue() && !R->isThreadLocalValue()) {
355350
Ctx.emitDiag([&]() {
356-
Ctx.Diag->Report(SymCtx.FA->D->getLocation(),
357-
diag::err_dylib_symbol_flags_mismatch)
351+
Ctx.Diag->Report(SymCtx.FA->Loc, diag::err_dylib_symbol_flags_mismatch)
358352
<< getAnnotatedName(DR, SymCtx) << DR->isThreadLocalValue();
359353
});
360354
return false;
361355
}
362356
if (!DR->isThreadLocalValue() && R->isThreadLocalValue()) {
363357
Ctx.emitDiag([&]() {
364-
Ctx.Diag->Report(SymCtx.FA->D->getLocation(),
365-
diag::err_header_symbol_flags_mismatch)
358+
Ctx.Diag->Report(SymCtx.FA->Loc, diag::err_header_symbol_flags_mismatch)
366359
<< getAnnotatedName(R, SymCtx) << R->isThreadLocalValue();
367360
});
368361
return false;
369362
}
370363

371364
if (DR->isWeakDefined() && !R->isWeakDefined()) {
372365
Ctx.emitDiag([&]() {
373-
Ctx.Diag->Report(SymCtx.FA->D->getLocation(),
374-
diag::err_dylib_symbol_flags_mismatch)
366+
Ctx.Diag->Report(SymCtx.FA->Loc, diag::err_dylib_symbol_flags_mismatch)
375367
<< getAnnotatedName(DR, SymCtx) << R->isWeakDefined();
376368
});
377369
return false;
378370
}
379371
if (!DR->isWeakDefined() && R->isWeakDefined()) {
380372
Ctx.emitDiag([&]() {
381-
Ctx.Diag->Report(SymCtx.FA->D->getLocation(),
382-
diag::err_header_symbol_flags_mismatch)
373+
Ctx.Diag->Report(SymCtx.FA->Loc, diag::err_header_symbol_flags_mismatch)
383374
<< getAnnotatedName(R, SymCtx) << R->isWeakDefined();
384375
});
385376
return false;
@@ -487,6 +478,14 @@ void DylibVerifier::setTarget(const Target &T) {
487478
assignSlice(T);
488479
}
489480

481+
void DylibVerifier::setSourceManager(
482+
IntrusiveRefCntPtr<SourceManager> SourceMgr) {
483+
if (!Ctx.Diag)
484+
return;
485+
SourceManagers.push_back(std::move(SourceMgr));
486+
Ctx.Diag->setSourceManager(SourceManagers.back().get());
487+
}
488+
490489
DylibVerifier::Result DylibVerifier::verify(ObjCIVarRecord *R,
491490
const FrontendAttrs *FA,
492491
const StringRef SuperClass) {

‎clang/lib/InstallAPI/Frontend.cpp

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,8 @@ std::pair<GlobalRecord *, FrontendAttrs *> FrontendRecordsSlice::addGlobal(
2323

2424
GlobalRecord *GR =
2525
llvm::MachO::RecordsSlice::addGlobal(Name, Linkage, GV, Flags, Inlined);
26-
auto Result = FrontendRecords.insert({GR, FrontendAttrs{Avail, D, Access}});
26+
auto Result = FrontendRecords.insert(
27+
{GR, FrontendAttrs{Avail, D, D->getLocation(), Access}});
2728
return {GR, &(Result.first->second)};
2829
}
2930

@@ -39,8 +40,8 @@ FrontendRecordsSlice::addObjCInterface(StringRef Name, RecordLinkage Linkage,
3940

4041
ObjCInterfaceRecord *ObjCR =
4142
llvm::MachO::RecordsSlice::addObjCInterface(Name, Linkage, SymType);
42-
auto Result =
43-
FrontendRecords.insert({ObjCR, FrontendAttrs{Avail, D, Access}});
43+
auto Result = FrontendRecords.insert(
44+
{ObjCR, FrontendAttrs{Avail, D, D->getLocation(), Access}});
4445
return {ObjCR, &(Result.first->second)};
4546
}
4647

@@ -51,8 +52,8 @@ FrontendRecordsSlice::addObjCCategory(StringRef ClassToExtend,
5152
const Decl *D, HeaderType Access) {
5253
ObjCCategoryRecord *ObjCR =
5354
llvm::MachO::RecordsSlice::addObjCCategory(ClassToExtend, CategoryName);
54-
auto Result =
55-
FrontendRecords.insert({ObjCR, FrontendAttrs{Avail, D, Access}});
55+
auto Result = FrontendRecords.insert(
56+
{ObjCR, FrontendAttrs{Avail, D, D->getLocation(), Access}});
5657
return {ObjCR, &(Result.first->second)};
5758
}
5859

@@ -67,8 +68,8 @@ std::pair<ObjCIVarRecord *, FrontendAttrs *> FrontendRecordsSlice::addObjCIVar(
6768
Linkage = RecordLinkage::Internal;
6869
ObjCIVarRecord *ObjCR =
6970
llvm::MachO::RecordsSlice::addObjCIVar(Container, IvarName, Linkage);
70-
auto Result =
71-
FrontendRecords.insert({ObjCR, FrontendAttrs{Avail, D, Access}});
71+
auto Result = FrontendRecords.insert(
72+
{ObjCR, FrontendAttrs{Avail, D, D->getLocation(), Access}});
7273

7374
return {ObjCR, &(Result.first->second)};
7475
}

‎clang/tools/clang-installapi/ClangInstallAPI.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,7 @@ static bool run(ArrayRef<const char *> Args, const char *ProgName) {
121121
// Execute, verify and gather AST results.
122122
// An invocation is ran for each unique target triple and for each header
123123
// access level.
124+
Records FrontendRecords;
124125
for (const auto &[Targ, Trip] : Opts.DriverOpts.Targets) {
125126
Ctx.Verifier->setTarget(Targ);
126127
Ctx.Slice = std::make_shared<FrontendRecordsSlice>(Trip);
@@ -131,6 +132,7 @@ static bool run(ArrayRef<const char *> Args, const char *ProgName) {
131132
InMemoryFileSystem.get(), Opts.getClangFrontendArgs()))
132133
return EXIT_FAILURE;
133134
}
135+
FrontendRecords.emplace_back(std::move(Ctx.Slice));
134136
}
135137

136138
if (Ctx.Verifier->verifyRemainingSymbols() == DylibVerifier::Result::Invalid)

0 commit comments

Comments
 (0)