Skip to content

[InstallAPI] Introduce Basic Verifier #85106

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

Merged
merged 2 commits into from
Mar 16, 2024

Conversation

cyndyishida
Copy link
Member

NOTE: This PR is part of a stack, please review #85100 first.

This adds basic support for calling the verifier on global declarations that are expected to represent symbol exports. The driver now exclusively uses this for knowing what symbols make up a TBD file. Future patches will check against the dylib's symbol table.

@cyndyishida cyndyishida requested review from zixu-w and ributzka March 13, 2024 16:18
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Mar 13, 2024
@llvmbot
Copy link
Member

llvmbot commented Mar 13, 2024

@llvm/pr-subscribers-clang

Author: Cyndy Ishida (cyndyishida)

Changes

NOTE: This PR is part of a stack, please review #85100 first.

This adds basic support for calling the verifier on global declarations that are expected to represent symbol exports. The driver now exclusively uses this for knowing what symbols make up a TBD file. Future patches will check against the dylib's symbol table.


Patch is 38.21 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/85106.diff

14 Files Affected:

  • (modified) clang/include/clang/AST/Availability.h (+3)
  • (modified) clang/include/clang/InstallAPI/Context.h (+4)
  • (modified) clang/include/clang/InstallAPI/DylibVerifier.h (+78-1)
  • (modified) clang/include/clang/InstallAPI/Frontend.h (-1)
  • (modified) clang/include/clang/InstallAPI/FrontendRecords.h (+27-23)
  • (modified) clang/include/clang/InstallAPI/MachO.h (+3)
  • (modified) clang/lib/InstallAPI/CMakeLists.txt (+2)
  • (added) clang/lib/InstallAPI/DylibVerifier.cpp (+212)
  • (modified) clang/lib/InstallAPI/Frontend.cpp (+28-21)
  • (modified) clang/lib/InstallAPI/Visitor.cpp (+61-40)
  • (added) clang/test/InstallAPI/asm.test (+90)
  • (modified) clang/tools/clang-installapi/ClangInstallAPI.cpp (+5-7)
  • (modified) clang/tools/clang-installapi/Options.cpp (+6-3)
  • (modified) llvm/include/llvm/TextAPI/Record.h (+6-1)
diff --git a/clang/include/clang/AST/Availability.h b/clang/include/clang/AST/Availability.h
index ae3acbeffe7f18..5cfbaf0cdfbd21 100644
--- a/clang/include/clang/AST/Availability.h
+++ b/clang/include/clang/AST/Availability.h
@@ -75,6 +75,9 @@ struct AvailabilityInfo {
   /// Determine if this AvailabilityInfo represents the default availability.
   bool isDefault() const { return *this == AvailabilityInfo(); }
 
+  /// Check if the symbol has been obsoleted.
+  bool isObsoleted() const { return !Obsoleted.empty(); }
+
   /// Check if the symbol is unconditionally deprecated.
   ///
   /// i.e. \code __attribute__((deprecated)) \endcode
diff --git a/clang/include/clang/InstallAPI/Context.h b/clang/include/clang/InstallAPI/Context.h
index bdb576d7d85fb6..074ff6f969773c 100644
--- a/clang/include/clang/InstallAPI/Context.h
+++ b/clang/include/clang/InstallAPI/Context.h
@@ -18,6 +18,7 @@
 namespace clang {
 namespace installapi {
 class FrontendRecordsSlice;
+class DylibVerifier;
 
 /// Struct used for generating validating InstallAPI.
 /// The attributes captured represent all necessary information
@@ -45,6 +46,9 @@ struct InstallAPIContext {
   /// DiagnosticsEngine for all error reporting.
   DiagnosticsEngine *Diags = nullptr;
 
+  /// Verifier when binary dylib is passed as input.
+  std::unique_ptr<DylibVerifier> Verifier = nullptr;
+
   /// File Path of output location.
   llvm::StringRef OutputLoc{};
 
diff --git a/clang/include/clang/InstallAPI/DylibVerifier.h b/clang/include/clang/InstallAPI/DylibVerifier.h
index 1a6121b3a258b5..72c4743fdf65e0 100644
--- a/clang/include/clang/InstallAPI/DylibVerifier.h
+++ b/clang/include/clang/InstallAPI/DylibVerifier.h
@@ -9,10 +9,12 @@
 #ifndef LLVM_CLANG_INSTALLAPI_DYLIBVERIFIER_H
 #define LLVM_CLANG_INSTALLAPI_DYLIBVERIFIER_H
 
-#include "llvm/TextAPI/Target.h"
+#include "clang/Basic/Diagnostic.h"
+#include "clang/InstallAPI/MachO.h"
 
 namespace clang {
 namespace installapi {
+struct FrontendAttrs;
 
 /// A list of InstallAPI verification modes.
 enum class VerificationMode {
@@ -22,6 +24,81 @@ enum class VerificationMode {
   Pedantic,
 };
 
+/// Service responsible to tracking state of verification across the
+/// lifetime of InstallAPI.
+/// As declarations are collected during AST traversal, they are
+/// compared as symbols against what is available in the binary dylib.
+class DylibVerifier {
+private:
+  struct SymbolContext;
+
+public:
+  enum class Result { NoVerify, Ignore, Valid, Invalid };
+  struct VerifierContext {
+    // Current target being verified against the AST.
+    llvm::MachO::Target Target;
+
+    // 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;
+  };
+
+  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>()) {}
+
+  Result verify(GlobalRecord *R, const FrontendAttrs *FA);
+  Result verify(ObjCInterfaceRecord *R, const FrontendAttrs *FA);
+  Result verify(ObjCIVarRecord *R, const FrontendAttrs *FA,
+                const StringRef SuperClass);
+
+  /// Initialize target for verification.
+  void setTarget(const Target &T);
+
+  /// Release ownership over exports.
+  std::unique_ptr<SymbolSet> getExports() { return std::move(Exports); }
+
+  /// Get result of verification.
+  Result getState() const { return Ctx.FrontendState; }
+
+private:
+  /// Determine whether to compare declaration to symbol in binary.
+  bool canVerify();
+
+  /// Shared implementation for verifying exported symbols.
+  Result verifyImpl(Record *R, SymbolContext &SymCtx);
+
+  /// Update result state on each call to `verify`.
+  void updateState(Result State);
+
+  /// Add verified exported symbol.
+  void addSymbol(const Record *R, SymbolContext &SymCtx,
+                 TargetList &&Targets = {});
+
+  // 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;
+
+  // Attempt to demangle when reporting violations.
+  bool Demangle = false;
+
+  // Valid symbols in final text file.
+  std::unique_ptr<SymbolSet> Exports = std::make_unique<SymbolSet>();
+
+  // Track current state of verification while traversing AST.
+  VerifierContext Ctx;
+};
+
 } // namespace installapi
 } // namespace clang
 #endif // LLVM_CLANG_INSTALLAPI_DYLIBVERIFIER_H
diff --git a/clang/include/clang/InstallAPI/Frontend.h b/clang/include/clang/InstallAPI/Frontend.h
index 873cb50d60a542..660fc8cd69a59d 100644
--- a/clang/include/clang/InstallAPI/Frontend.h
+++ b/clang/include/clang/InstallAPI/Frontend.h
@@ -14,7 +14,6 @@
 #define LLVM_CLANG_INSTALLAPI_FRONTEND_H
 
 #include "clang/AST/ASTConsumer.h"
-#include "clang/AST/Availability.h"
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Frontend/FrontendActions.h"
 #include "clang/InstallAPI/Context.h"
diff --git a/clang/include/clang/InstallAPI/FrontendRecords.h b/clang/include/clang/InstallAPI/FrontendRecords.h
index 333015b6a11365..59271e81e230c2 100644
--- a/clang/include/clang/InstallAPI/FrontendRecords.h
+++ b/clang/include/clang/InstallAPI/FrontendRecords.h
@@ -11,6 +11,7 @@
 
 #include "clang/AST/Availability.h"
 #include "clang/AST/DeclObjC.h"
+#include "clang/InstallAPI/HeaderFile.h"
 #include "clang/InstallAPI/MachO.h"
 
 namespace clang {
@@ -42,13 +43,13 @@ class FrontendRecordsSlice : public llvm::MachO::RecordsSlice {
   /// \param Flags The flags that describe attributes of the symbol.
   /// \param Inlined Whether declaration is inlined, only applicable to
   /// functions.
-  /// \return The non-owning pointer to added record in slice.
-  GlobalRecord *addGlobal(StringRef Name, RecordLinkage Linkage,
-                          GlobalRecord::Kind GV,
-                          const clang::AvailabilityInfo Avail, const Decl *D,
-                          const HeaderType Access,
-                          SymbolFlags Flags = SymbolFlags::None,
-                          bool Inlined = false);
+  /// \return The non-owning pointer to added record in slice with it's frontend
+  /// attributes.
+  std::pair<GlobalRecord *, FrontendAttrs *>
+  addGlobal(StringRef Name, RecordLinkage Linkage, GlobalRecord::Kind GV,
+            const clang::AvailabilityInfo Avail, const Decl *D,
+            const HeaderType Access, SymbolFlags Flags = SymbolFlags::None,
+            bool Inlined = false);
 
   /// Add ObjC Class record with attributes from AST.
   ///
@@ -59,11 +60,12 @@ class FrontendRecordsSlice : public llvm::MachO::RecordsSlice {
   /// \param D The pointer to the declaration from traversing AST.
   /// \param Access The intended access level of symbol.
   /// \param IsEHType Whether declaration has an exception attribute.
-  /// \return The non-owning pointer to added record in slice.
-  ObjCInterfaceRecord *addObjCInterface(StringRef Name, RecordLinkage Linkage,
-                                        const clang::AvailabilityInfo Avail,
-                                        const Decl *D, HeaderType Access,
-                                        bool IsEHType);
+  /// \return The non-owning pointer to added record in slice with it's frontend
+  /// attributes.
+  std::pair<ObjCInterfaceRecord *, FrontendAttrs *>
+  addObjCInterface(StringRef Name, RecordLinkage Linkage,
+                   const clang::AvailabilityInfo Avail, const Decl *D,
+                   HeaderType Access, bool IsEHType);
 
   /// Add ObjC Category record with attributes from AST.
   ///
@@ -74,11 +76,12 @@ class FrontendRecordsSlice : public llvm::MachO::RecordsSlice {
   /// to the active target triple.
   /// \param D The pointer to the declaration from traversing AST.
   /// \param Access The intended access level of symbol.
-  /// \return The non-owning pointer to added record in slice.
-  ObjCCategoryRecord *addObjCCategory(StringRef ClassToExtend,
-                                      StringRef CategoryName,
-                                      const clang::AvailabilityInfo Avail,
-                                      const Decl *D, HeaderType Access);
+  /// \return The non-owning pointer to added record in slice with it's frontend
+  /// attributes.
+  std::pair<ObjCCategoryRecord *, FrontendAttrs *>
+  addObjCCategory(StringRef ClassToExtend, StringRef CategoryName,
+                  const clang::AvailabilityInfo Avail, const Decl *D,
+                  HeaderType Access);
 
   /// Add ObjC IVar record with attributes from AST.
   ///
@@ -90,12 +93,13 @@ class FrontendRecordsSlice : public llvm::MachO::RecordsSlice {
   /// \param D The pointer to the declaration from traversing AST.
   /// \param Access The intended access level of symbol.
   /// \param AC The access control tied to the ivar declaration.
-  /// \return The non-owning pointer to added record in slice.
-  ObjCIVarRecord *addObjCIVar(ObjCContainerRecord *Container,
-                              StringRef IvarName, RecordLinkage Linkage,
-                              const clang::AvailabilityInfo Avail,
-                              const Decl *D, HeaderType Access,
-                              const clang::ObjCIvarDecl::AccessControl AC);
+  /// \return The non-owning pointer to added record in slice with it's frontend
+  /// attributes.
+  std::pair<ObjCIVarRecord *, FrontendAttrs *>
+  addObjCIVar(ObjCContainerRecord *Container, StringRef IvarName,
+              RecordLinkage Linkage, const clang::AvailabilityInfo Avail,
+              const Decl *D, HeaderType Access,
+              const clang::ObjCIvarDecl::AccessControl AC);
 
 private:
   /// Mapping of records stored in slice to their frontend attributes.
diff --git a/clang/include/clang/InstallAPI/MachO.h b/clang/include/clang/InstallAPI/MachO.h
index 55e5591389ce1f..6dee6f22420381 100644
--- a/clang/include/clang/InstallAPI/MachO.h
+++ b/clang/include/clang/InstallAPI/MachO.h
@@ -18,6 +18,7 @@
 #include "llvm/TextAPI/PackedVersion.h"
 #include "llvm/TextAPI/Platform.h"
 #include "llvm/TextAPI/RecordVisitor.h"
+#include "llvm/TextAPI/Symbol.h"
 #include "llvm/TextAPI/Target.h"
 #include "llvm/TextAPI/TextAPIWriter.h"
 #include "llvm/TextAPI/Utils.h"
@@ -33,8 +34,10 @@ using ObjCIVarRecord = llvm::MachO::ObjCIVarRecord;
 using Records = llvm::MachO::Records;
 using BinaryAttrs = llvm::MachO::RecordsSlice::BinaryAttrs;
 using SymbolSet = llvm::MachO::SymbolSet;
+using SimpleSymbol = llvm::MachO::SimpleSymbol;
 using FileType = llvm::MachO::FileType;
 using PackedVersion = llvm::MachO::PackedVersion;
 using Target = llvm::MachO::Target;
+using TargetList = llvm::MachO::TargetList;
 
 #endif // LLVM_CLANG_INSTALLAPI_MACHO_H
diff --git a/clang/lib/InstallAPI/CMakeLists.txt b/clang/lib/InstallAPI/CMakeLists.txt
index dc90d6370de418..cba1c3fcafdf20 100644
--- a/clang/lib/InstallAPI/CMakeLists.txt
+++ b/clang/lib/InstallAPI/CMakeLists.txt
@@ -1,10 +1,12 @@
 set(LLVM_LINK_COMPONENTS
   Support
   TextAPI
+  Demangle 
   Core
   )
 
 add_clang_library(clangInstallAPI
+  DylibVerifier.cpp
   FileList.cpp
   Frontend.cpp
   HeaderFile.cpp
diff --git a/clang/lib/InstallAPI/DylibVerifier.cpp b/clang/lib/InstallAPI/DylibVerifier.cpp
new file mode 100644
index 00000000000000..b7dd85d63fa14f
--- /dev/null
+++ b/clang/lib/InstallAPI/DylibVerifier.cpp
@@ -0,0 +1,212 @@
+#include "clang/InstallAPI/DylibVerifier.h"
+#include "clang/InstallAPI/FrontendRecords.h"
+#include "llvm/Demangle/Demangle.h"
+
+using namespace llvm::MachO;
+
+namespace clang {
+namespace installapi {
+
+/// Metadata stored about a mapping of a declaration to a symbol.
+struct DylibVerifier::SymbolContext {
+  // Name to use for printing in diagnostics.
+  std::string PrettyPrintName{""};
+
+  // Name to use for all querying and verification
+  // purposes.
+  std::string SymbolName{""};
+
+  // Kind to map symbol type against record.
+  EncodeKind Kind = EncodeKind::GlobalSymbol;
+
+  // Frontend Attributes tied to the AST.
+  const FrontendAttrs *FA = nullptr;
+
+  // The ObjCInterface symbol type, if applicable.
+  ObjCIFSymbolKind ObjCIFKind = ObjCIFSymbolKind::None;
+};
+
+static std::string
+getAnnotatedName(const Record *R, EncodeKind Kind, StringRef Name,
+                 bool ValidSourceLoc = true,
+                 ObjCIFSymbolKind ObjCIF = ObjCIFSymbolKind::None) {
+  assert(!Name.empty() && "Need symbol name for printing");
+
+  std::string Annotation;
+  if (R->isWeakDefined())
+    Annotation += "(weak-def) ";
+  if (R->isWeakReferenced())
+    Annotation += "(weak-ref) ";
+  if (R->isThreadLocalValue())
+    Annotation += "(tlv) ";
+
+  // Check if symbol represents only part of a @interface declaration.
+  const bool IsAnnotatedObjCClass = ((ObjCIF != ObjCIFSymbolKind::None) &&
+                                     (ObjCIF <= ObjCIFSymbolKind::EHType));
+
+  if (IsAnnotatedObjCClass) {
+    if (ObjCIF == ObjCIFSymbolKind::EHType)
+      Annotation += "Exception Type of ";
+    if (ObjCIF == ObjCIFSymbolKind::MetaClass)
+      Annotation += "Metaclass of ";
+    if (ObjCIF == ObjCIFSymbolKind::Class)
+      Annotation += "Class of ";
+  }
+
+  // Only print symbol type prefix or leading "_" if there is no source location
+  // tied to it. This can only ever happen when the location has to come from
+  // debug info.
+  if (ValidSourceLoc) {
+    if ((Kind == EncodeKind::GlobalSymbol) && Name.starts_with("_"))
+      return Annotation + Name.drop_front(1).str();
+    return Annotation + Name.str();
+  }
+
+  if (IsAnnotatedObjCClass)
+    return Annotation + Name.str();
+
+  switch (Kind) {
+  case EncodeKind::GlobalSymbol:
+    return Annotation + Name.str();
+  case EncodeKind::ObjectiveCInstanceVariable:
+    return Annotation + "(ObjC IVar) " + Name.str();
+  case EncodeKind::ObjectiveCClass:
+    return Annotation + "(ObjC Class) " + Name.str();
+  case EncodeKind::ObjectiveCClassEHType:
+    return Annotation + "(ObjC Class EH) " + Name.str();
+  }
+
+  llvm_unreachable("unexpected case for EncodeKind");
+}
+
+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")))
+    return Name.str();
+  char *Result = llvm::itaniumDemangle(Name.data());
+  if (!Result)
+    return Name.str();
+
+  std::string Demangled(Result);
+  free(Result);
+  return Demangled;
+}
+
+static DylibVerifier::Result updateResult(const DylibVerifier::Result Prev,
+                                          const DylibVerifier::Result Curr) {
+  if (Prev == Curr)
+    return Prev;
+
+  // Never update from invalid or noverify state.
+  if ((Prev == DylibVerifier::Result::Invalid) ||
+      (Prev == DylibVerifier::Result::NoVerify))
+    return Prev;
+
+  // Don't let an ignored verification remove a valid one.
+  if (Prev == DylibVerifier::Result::Valid &&
+      Curr == DylibVerifier::Result::Ignore)
+    return Prev;
+
+  return Curr;
+}
+
+void DylibVerifier::updateState(Result State) {
+  Ctx.FrontendState = updateResult(Ctx.FrontendState, State);
+}
+
+void DylibVerifier::addSymbol(const Record *R, SymbolContext &SymCtx,
+                              TargetList &&Targets) {
+  if (Targets.empty())
+    Targets = {Ctx.Target};
+
+  Exports->addGlobal(SymCtx.Kind, SymCtx.SymbolName, R->getFlags(), Targets);
+}
+
+DylibVerifier::Result DylibVerifier::verifyImpl(Record *R,
+                                                SymbolContext &SymCtx) {
+  R->setVerify();
+  if (!canVerify()) {
+    // Accumulate symbols when not in verifying against dylib.
+    if (R->isExported() && !SymCtx.FA->Avail.isUnconditionallyUnavailable() &&
+        !SymCtx.FA->Avail.isObsoleted()) {
+      addSymbol(R, SymCtx);
+    }
+    return Ctx.FrontendState;
+  }
+  return Ctx.FrontendState;
+}
+
+bool DylibVerifier::canVerify() {
+  return Ctx.FrontendState != Result::NoVerify;
+}
+
+void DylibVerifier::setTarget(const Target &T) {
+  Ctx.Target = T;
+  Ctx.DiscoveredFirstError = false;
+  updateState(Dylib.empty() ? Result::NoVerify : Result::Ignore);
+}
+
+DylibVerifier::Result DylibVerifier::verify(ObjCIVarRecord *R,
+                                            const FrontendAttrs *FA,
+                                            const StringRef SuperClass) {
+  if (R->isVerified())
+    return getState();
+
+  std::string FullName =
+      ObjCIVarRecord::createScopedName(SuperClass, R->getName());
+  SymbolContext SymCtx{
+      getAnnotatedName(R, EncodeKind::ObjectiveCInstanceVariable,
+                       Demangle ? demangle(FullName) : FullName),
+      FullName, EncodeKind::ObjectiveCInstanceVariable, FA};
+  return verifyImpl(R, SymCtx);
+}
+
+static ObjCIFSymbolKind assignObjCIFSymbolKind(const ObjCInterfaceRecord *R) {
+  ObjCIFSymbolKind Result = ObjCIFSymbolKind::None;
+  if (R->getLinkageForSymbol(ObjCIFSymbolKind::Class) != RecordLinkage::Unknown)
+    Result |= ObjCIFSymbolKind::Class;
+  if (R->getLinkageForSymbol(ObjCIFSymbolKind::MetaClass) !=
+      RecordLinkage::Unknown)
+    Result |= ObjCIFSymbolKind::MetaClass;
+  if (R->getLinkageForSymbol(ObjCIFSymbolKind::EHType) !=
+      RecordLinkage::Unknown)
+    Result |= ObjCIFSymbolKind::EHType;
+  return Result;
+}
+
+DylibVerifier::Result DylibVerifier::verify(ObjCInterfaceRecord *R,
+                                            const FrontendAttrs *FA) {
+  if (R->isVerified())
+    return getState();
+  SymbolContext SymCtx;
+  SymCtx.SymbolName = R->getName();
+  SymCtx.ObjCIFKind = assignObjCIFSymbolKind(R);
+
+  std::string DisplayName =
+      Demangle ? demangle(SymCtx.SymbolName) : SymCtx.SymbolName;
+  SymCtx.Kind = R->hasExceptionAttribute() ? EncodeKind::ObjectiveCClassEHType
+                                           : EncodeKind::ObjectiveCClass;
+  SymCtx.PrettyPrintName = getAnnotatedName(R, SymCtx.Kind, DisplayName);
+  SymCtx.FA = FA;
+
+  return verifyImpl(R, SymCtx);
+}
+
+DylibVerifier::Result DylibVerifier::verify(GlobalRecord *R,
+                                            const FrontendAttrs *FA) {
+  if (R->isVerified())
+    return getState();
+
+  // Global classifications could be obfusciated with `asm`.
+  SimpleSymbol Sym = parseSymbol(R->getName());
+  SymbolContext SymCtx;
+  SymCtx.SymbolName = Sym.Name;
+  SymCtx.PrettyPrintName =
+      getAnnotatedName(R, Sym.Kind, Demangle ? demangle(Sym.Name) : Sym.Name);
+  SymCtx.Kind = Sym.Kind;
+  SymCtx.FA = FA;
+  return verifyImpl(R, SymCtx);
+}
+
+} // namespace installapi
+} // namespace clang
diff --git a/clang/lib/InstallAPI/Frontend.cpp b/clang/lib/InstallAPI/Frontend.cpp
index 707aeb17dc8906..12cd5fcbc22bf7 100644
--- a/clang/lib/InstallAPI/Frontend.cpp
+++ b/clang/lib/InstallAPI/Frontend.cpp
@@ -16,41 +16,47 @@ using namespace llvm;
 using namespace llvm::MachO;
 
 namespace clang::installapi {
-
-GlobalRecord *FrontendRecordsSlice::addGlobal(
+std::pair<GlobalRecord *, FrontendAttrs *> FrontendRecordsSlice::addGlobal(
     StringRef Name, RecordLinkage Linkage, GlobalRecord::Kind GV,
     const clang::AvailabilityInfo Avail, const Decl *D, const HeaderType Access,
     SymbolFlags Flags, bool Inlined) {
 
-  auto *GR =
+  GlobalRecord *GR =
       llvm::MachO::RecordsSlice::addGlobal(Name, Linkage, GV, Flags, Inlined);
-  FrontendRecords.insert({GR, FrontendAttrs{Avail, D, Access}});
-  return GR;
+  auto Result = FrontendRecords.insert({GR, FrontendAttrs{Avail, D, Access}});
+  return {GR, &(Result.first->second)};
 }
 
-ObjCInterfaceRecord *FrontendRecordsSlice::addObjCInterface(
-    StringRef Name, RecordLinkage Linkage, const clang::AvailabilityInfo Avail,
-    const Decl *D, HeaderType Access, bool IsEHType) {
+std::pair<ObjCInterfaceRecord *, FrontendAttrs *>
+FrontendRecordsSlice::addObjCInterface(StringRef Name, RecordLinkage Linkage,
+                                       const clang::AvailabilityInfo Avail,
+                                       const Decl *D, HeaderType Access,
+                                       bool IsEHType) {
   ObjCIFSymbolKind SymType =
       ObjCIFSymbolKind::Class | ObjCIFSymbolKind::MetaClass;
   if (IsEHType)
     SymType |= ObjCI...
[truncated]

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/installapiopts branch from 95a2bfe to 1281dfa Compare March 14, 2024 19:08
@cyndyishida cyndyishida force-pushed the users/cyndyishida/installapiverifier branch 2 times, most recently from 91fe39d to 192f306 Compare March 14, 2024 19:11
@cyndyishida cyndyishida changed the base branch from users/cyndyishida/installapiopts to main March 16, 2024 16:02
This adds basic support for calling the verifier on global declarations
that are expected to represent symbol exports. The driver now
exclusively uses this for knowing what symbols make up a TBD file.
Future patches will actually check against the dylib's symbol table.
@cyndyishida cyndyishida force-pushed the users/cyndyishida/installapiverifier branch from 192f306 to 3f0def7 Compare March 16, 2024 16:24
@cyndyishida cyndyishida merged commit f2794cc into main Mar 16, 2024
@cyndyishida cyndyishida deleted the users/cyndyishida/installapiverifier branch March 16, 2024 17:37
delcypher pushed a commit to swiftlang/llvm-project that referenced this pull request Mar 19, 2024
…llAPI] Introduce Basic Verifier (llvm#85106)))

Merge commit 'f2794ccede6d' from llvm.org/main into next

Conflicts:
	clang/tools/clang-installapi/ClangInstallAPI.cpp

rdar://124977504
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.

3 participants