[clang] f04452d - [InstallAPI] Tie lifetime of FE objects to DylibVerifier (#88189)

via cfe-commits cfe-commits at lists.llvm.org
Tue Apr 9 15:13:58 PDT 2024


Author: Cyndy Ishida
Date: 2024-04-09T15:13:55-07:00
New Revision: f04452de1986e4e01296a80231efb212d6c84c42

URL: https://github.com/llvm/llvm-project/commit/f04452de1986e4e01296a80231efb212d6c84c42
DIFF: https://github.com/llvm/llvm-project/commit/f04452de1986e4e01296a80231efb212d6c84c42.diff

LOG: [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.

Added: 
    

Modified: 
    clang/include/clang/InstallAPI/DylibVerifier.h
    clang/include/clang/InstallAPI/Frontend.h
    clang/include/clang/InstallAPI/FrontendRecords.h
    clang/lib/InstallAPI/DylibVerifier.cpp
    clang/lib/InstallAPI/Frontend.cpp
    clang/tools/clang-installapi/ClangInstallAPI.cpp

Removed: 
    


################################################################################
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 
diff erent 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