Skip to content
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

[InstallAPI] Verify that declarations in headers map to exports found in dylib #85348

Merged
merged 5 commits into from
Mar 20, 2024

Conversation

cyndyishida
Copy link
Member

  • 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.

@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Mar 15, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented Mar 15, 2024

@llvm/pr-subscribers-clang

Author: Cyndy Ishida (cyndyishida)

Changes
  • 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 --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]

@cyndyishida cyndyishida force-pushed the users/cyndyishida/installapiverifier branch from 192f306 to 3f0def7 Compare March 16, 2024 16:24
@cyndyishida cyndyishida changed the base branch from users/cyndyishida/installapiverifier to main March 16, 2024 17:36
dylib

* This patch completes support for verifying every declaration found in a
header is discovered in the dylib. Diagnostics are reported for each
class for differences that is representable in TBD files.

* This patch also now captures unavailable attributes that depend on target triples. This is needed for proper tbd file generation.
@cyndyishida cyndyishida force-pushed the users/cyndyishida/installapiveriferp2 branch from 56efc49 to d45081b Compare March 16, 2024 17:40
@cyndyishida cyndyishida force-pushed the users/cyndyishida/installapiveriferp2 branch from ac25849 to 4a44bf9 Compare March 18, 2024 15:50
Copy link

github-actions bot commented Mar 18, 2024

⚠️ C/C++ code formatter, clang-format found issues in your code. ⚠️

You can test this locally with the following command:
git-clang-format --diff f2794ccede6d32a6b5ef7a376ced420331e2be27 c0d0fb3f6da1258f6247c15eab0f0deaaf86f05e -- clang/include/clang/AST/Availability.h clang/include/clang/InstallAPI/DylibVerifier.h clang/include/clang/InstallAPI/Frontend.h clang/include/clang/InstallAPI/MachO.h clang/lib/AST/Availability.cpp clang/lib/InstallAPI/DylibVerifier.cpp clang/lib/InstallAPI/Visitor.cpp clang/tools/clang-installapi/ClangInstallAPI.cpp clang/tools/clang-installapi/Options.cpp clang/tools/diagtool/DiagnosticNames.cpp
View the diff from clang-format here.
diff --git a/clang/tools/diagtool/DiagnosticNames.cpp b/clang/tools/diagtool/DiagnosticNames.cpp
index eb90f08243..6b8149b0a9 100644
--- a/clang/tools/diagtool/DiagnosticNames.cpp
+++ b/clang/tools/diagtool/DiagnosticNames.cpp
@@ -30,19 +30,19 @@ static const DiagnosticRecord BuiltinDiagnosticsByID[] = {
 #define DIAG(ENUM, CLASS, DEFAULT_MAPPING, DESC, GROUP, SFINAE, NOWERROR,      \
              SHOWINSYSHEADER, SHOWINSYSMACRO, DEFER, CATEGORY)                 \
   {#ENUM, diag::ENUM, STR_SIZE(#ENUM, uint8_t)},
+#include "clang/Basic/DiagnosticASTKinds.inc"
+#include "clang/Basic/DiagnosticAnalysisKinds.inc"
+#include "clang/Basic/DiagnosticCommentKinds.inc"
 #include "clang/Basic/DiagnosticCommonKinds.inc"
 #include "clang/Basic/DiagnosticCrossTUKinds.inc"
 #include "clang/Basic/DiagnosticDriverKinds.inc"
 #include "clang/Basic/DiagnosticFrontendKinds.inc"
-#include "clang/Basic/DiagnosticSerializationKinds.inc"
+#include "clang/Basic/DiagnosticInstallAPIKinds.inc"
 #include "clang/Basic/DiagnosticLexKinds.inc"
 #include "clang/Basic/DiagnosticParseKinds.inc"
-#include "clang/Basic/DiagnosticASTKinds.inc"
-#include "clang/Basic/DiagnosticCommentKinds.inc"
-#include "clang/Basic/DiagnosticSemaKinds.inc"
-#include "clang/Basic/DiagnosticAnalysisKinds.inc"
 #include "clang/Basic/DiagnosticRefactoringKinds.inc"
-#include "clang/Basic/DiagnosticInstallAPIKinds.inc"
+#include "clang/Basic/DiagnosticSemaKinds.inc"
+#include "clang/Basic/DiagnosticSerializationKinds.inc"
 #undef DIAG
 };
 

Copy link
Collaborator

@ributzka ributzka left a comment

Choose a reason for hiding this comment

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

LGTM

@cyndyishida cyndyishida force-pushed the users/cyndyishida/installapiveriferp2 branch from f14ffb0 to a752315 Compare March 19, 2024 21:54
@cyndyishida cyndyishida merged commit 936519f into main Mar 20, 2024
2 of 4 checks passed
@cyndyishida cyndyishida deleted the users/cyndyishida/installapiveriferp2 branch March 20, 2024 01:36
chencha3 pushed a commit to chencha3/llvm-project that referenced this pull request Mar 23, 2024
… in dylib (llvm#85348)

* 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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants