[InstallAPI] Handle zippered frameworks (#88205)

A zippered framework is a single framework that can be loaded in both macOS and macatalyst processes. Broadly to InstallAPI, it means the same interface can represent two separate platforms. A dylib's symbol table does not distinguish between macOS/macCatalyst. `InstallAPI` provides the ability for the tbd file to distinct symbols between them. The verifier handles this special logic by tracking all unavailable and obsoleted APIs in this context and checking against those when determining dylib symbols with no matching declaration. * If there exists an available decl for either platform, do not warn. * If there is no available decl, emit a diagnostic and print the source location for both decls.
llvm · Apr 12, 2024 · c24efff · c24efff
1 parent 334e07f
commit c24efff
Show file tree

Hide file tree

Showing 8 changed files with 964 additions and 16 deletions.
diff --git a/clang/include/clang/Basic/DiagnosticInstallAPIKinds.td b/clang/include/clang/Basic/DiagnosticInstallAPIKinds.td
@@ -20,6 +20,10 @@ def warn_no_such_excluded_header_file : Warning<"no such excluded %select{public
 def warn_glob_did_not_match: Warning<"glob '%0' did not match any header file">, InGroup<InstallAPIViolation>;
 def err_no_such_umbrella_header_file : Error<"%select{public|private|project}1 umbrella header file not found in input: '%0'">;
 def err_cannot_find_reexport : Error<"cannot find re-exported %select{framework|library}0: '%1'">;
+def err_no_matching_target : Error<"no matching target found for target variant '%0'">;
+def err_unsupported_vendor : Error<"vendor '%0' is not supported: '%1'">;
+def err_unsupported_environment : Error<"environment '%0' is not supported: '%1'">;
+def err_unsupported_os : Error<"os '%0' is not supported: '%1'">;
 } // end of command line category.
 
 let CategoryName = "Verification" in {

diff --git a/clang/include/clang/InstallAPI/DylibVerifier.h b/clang/include/clang/InstallAPI/DylibVerifier.h
@@ -28,6 +28,16 @@ enum class VerificationMode {
 using LibAttrs = llvm::StringMap<ArchitectureSet>;
 using ReexportedInterfaces = llvm::SmallVector<llvm::MachO::InterfaceFile, 8>;
 
+// Pointers to information about a zippered declaration used for
+// querying and reporting violations against different
+// declarations that all map to the same symbol.
+struct ZipperedDeclSource {
+  const FrontendAttrs *FA;
+  clang::SourceManager *SrcMgr;
+  Target T;
+};
+using ZipperedDeclSources = std::vector<ZipperedDeclSource>;
+
 /// Service responsible to tracking state of verification across the
 /// lifetime of InstallAPI.
 /// As declarations are collected during AST traversal, they are
@@ -68,10 +78,10 @@ class DylibVerifier : llvm::MachO::RecordVisitor {
   DylibVerifier() = default;
 
   DylibVerifier(llvm::MachO::Records &&Dylib, ReexportedInterfaces &&Reexports,
-                DiagnosticsEngine *Diag, VerificationMode Mode, bool Demangle,
-                StringRef DSYMPath)
+                DiagnosticsEngine *Diag, VerificationMode Mode, bool Zippered,
+                bool Demangle, StringRef DSYMPath)
       : Dylib(std::move(Dylib)), Reexports(std::move(Reexports)), Mode(Mode),
-        Demangle(Demangle), DSYMPath(DSYMPath),
+        Zippered(Zippered), Demangle(Demangle), DSYMPath(DSYMPath),
         Exports(std::make_unique<SymbolSet>()), Ctx(VerifierContext{Diag}) {}
 
   Result verify(GlobalRecord *R, const FrontendAttrs *FA);
@@ -118,6 +128,15 @@ class DylibVerifier : llvm::MachO::RecordVisitor {
   /// symbols should be omitted from the text-api file.
   bool shouldIgnoreReexport(const Record *R, SymbolContext &SymCtx) const;
 
+  // Ignore and omit unavailable symbols in zippered libraries.
+  bool shouldIgnoreZipperedAvailability(const Record *R, SymbolContext &SymCtx);
+
+  // Check if an internal declaration in zippered library has an
+  // external declaration for a different platform. This results
+  // in the symbol being in a "seperate" platform slice.
+  bool shouldIgnoreInternalZipperedSymbol(const Record *R,
+                                          const SymbolContext &SymCtx) const;
+
   /// Compare the visibility declarations to the linkage of symbol found in
   /// dylib.
   Result compareVisibility(const Record *R, SymbolContext &SymCtx,
@@ -173,6 +192,9 @@ class DylibVerifier : llvm::MachO::RecordVisitor {
   // Controls what class of violations to report.
   VerificationMode Mode = VerificationMode::Invalid;
 
+  // Library is zippered.
+  bool Zippered = false;
+
   // Attempt to demangle when reporting violations.
   bool Demangle = false;
 
@@ -182,6 +204,10 @@ class DylibVerifier : llvm::MachO::RecordVisitor {
   // Valid symbols in final text file.
   std::unique_ptr<SymbolSet> Exports = std::make_unique<SymbolSet>();
 
+  // Unavailable or obsoleted declarations for a zippered library.
+  // These are cross referenced against symbols in the dylib.
+  llvm::StringMap<ZipperedDeclSources> DeferredZipperedSymbols;
+
   // Track current state of verification while traversing AST.
   VerifierContext Ctx;
 

diff --git a/clang/lib/InstallAPI/DylibVerifier.cpp b/clang/lib/InstallAPI/DylibVerifier.cpp
@@ -176,7 +176,13 @@ void DylibVerifier::addSymbol(const Record *R, SymbolContext &SymCtx,
 
 bool DylibVerifier::shouldIgnoreObsolete(const Record *R, SymbolContext &SymCtx,
                                          const Record *DR) {
-  return SymCtx.FA->Avail.isObsoleted();
+  if (!SymCtx.FA->Avail.isObsoleted())
+    return false;
+
+  if (Zippered)
+    DeferredZipperedSymbols[SymCtx.SymbolName].emplace_back(ZipperedDeclSource{
+        SymCtx.FA, &Ctx.Diag->getSourceManager(), Ctx.Target});
+  return true;
 }
 
 bool DylibVerifier::shouldIgnoreReexport(const Record *R,
@@ -195,6 +201,28 @@ bool DylibVerifier::shouldIgnoreReexport(const Record *R,
   return false;
 }
 
+bool DylibVerifier::shouldIgnoreInternalZipperedSymbol(
+    const Record *R, const SymbolContext &SymCtx) const {
+  if (!Zippered)
+    return false;
+
+  return Exports->findSymbol(SymCtx.Kind, SymCtx.SymbolName,
+                             SymCtx.ObjCIFKind) != nullptr;
+}
+
+bool DylibVerifier::shouldIgnoreZipperedAvailability(const Record *R,
+                                                     SymbolContext &SymCtx) {
+  if (!(Zippered && SymCtx.FA->Avail.isUnavailable()))
+    return false;
+
+  // Collect source location incase there is an exported symbol to diagnose
+  // during `verifyRemainingSymbols`.
+  DeferredZipperedSymbols[SymCtx.SymbolName].emplace_back(
+      ZipperedDeclSource{SymCtx.FA, SourceManagers.back().get(), Ctx.Target});
+
+  return true;
+}
+
 bool DylibVerifier::compareObjCInterfaceSymbols(const Record *R,
                                                 SymbolContext &SymCtx,
                                                 const ObjCInterfaceRecord *DR) {
@@ -294,6 +322,9 @@ DylibVerifier::Result DylibVerifier::compareVisibility(const Record *R,
     if (shouldIgnorePrivateExternAttr(SymCtx.FA->D))
       return Result::Ignore;
 
+    if (shouldIgnoreInternalZipperedSymbol(R, SymCtx))
+      return Result::Ignore;
+
     unsigned ID;
     Result Outcome;
     if (Mode == VerificationMode::ErrorsAndWarnings) {
@@ -321,6 +352,9 @@ DylibVerifier::Result DylibVerifier::compareAvailability(const Record *R,
   if (!SymCtx.FA->Avail.isUnavailable())
     return Result::Valid;
 
+  if (shouldIgnoreZipperedAvailability(R, SymCtx))
+    return Result::Ignore;
+
   const bool IsDeclAvailable = SymCtx.FA->Avail.isUnavailable();
 
   switch (Mode) {
@@ -588,13 +622,58 @@ void DylibVerifier::visitSymbolInDylib(const Record &R, SymbolContext &SymCtx) {
     }
   }
 
+  const bool IsLinkerSymbol = SymbolName.starts_with("$ld$");
+
+  if (R.isVerified()) {
+    // Check for unavailable symbols.
+    // This should only occur in the zippered case where we ignored
+    // availability until all headers have been parsed.
+    auto It = DeferredZipperedSymbols.find(SymCtx.SymbolName);
+    if (It == DeferredZipperedSymbols.end()) {
+      updateState(Result::Valid);
+      return;
+    }
+
+    ZipperedDeclSources Locs;
+    for (const ZipperedDeclSource &ZSource : It->second) {
+      if (ZSource.FA->Avail.isObsoleted()) {
+        updateState(Result::Ignore);
+        return;
+      }
+      if (ZSource.T.Arch != Ctx.Target.Arch)
+        continue;
+      Locs.emplace_back(ZSource);
+    }
+    assert(Locs.size() == 2 && "Expected two decls for zippered symbol");
+
+    // Print violating declarations per platform.
+    for (const ZipperedDeclSource &ZSource : Locs) {
+      unsigned DiagID = 0;
+      if (Mode == VerificationMode::Pedantic || IsLinkerSymbol) {
+        updateState(Result::Invalid);
+        DiagID = diag::err_header_availability_mismatch;
+      } else if (Mode == VerificationMode::ErrorsAndWarnings) {
+        updateState(Result::Ignore);
+        DiagID = diag::warn_header_availability_mismatch;
+      } else {
+        updateState(Result::Ignore);
+        return;
+      }
+      // Bypass emitDiag banner and print the target everytime.
+      Ctx.Diag->setSourceManager(ZSource.SrcMgr);
+      Ctx.Diag->Report(diag::warn_target) << getTargetTripleName(ZSource.T);
+      Ctx.Diag->Report(ZSource.FA->Loc, DiagID)
+          << getAnnotatedName(&R, SymCtx) << ZSource.FA->Avail.isUnavailable()
+          << ZSource.FA->Avail.isUnavailable();
+    }
+    return;
+  }
+
   if (shouldIgnoreCpp(SymbolName, R.isWeakDefined())) {
     updateState(Result::Valid);
     return;
   }
 
-  const bool IsLinkerSymbol = SymbolName.starts_with("$ld$");
-
   // All checks at this point classify as some kind of violation.
   // The different verification modes dictate whether they are reported to the
   // user.
@@ -647,8 +726,6 @@ void DylibVerifier::visitSymbolInDylib(const Record &R, SymbolContext &SymCtx) {
 }
 
 void DylibVerifier::visitGlobal(const GlobalRecord &R) {
-  if (R.isVerified())
-    return;
   SymbolContext SymCtx;
   SimpleSymbol Sym = parseSymbol(R.getName());
   SymCtx.SymbolName = Sym.Name;
@@ -658,8 +735,6 @@ void DylibVerifier::visitGlobal(const GlobalRecord &R) {
 
 void DylibVerifier::visitObjCIVar(const ObjCIVarRecord &R,
                                   const StringRef Super) {
-  if (R.isVerified())
-    return;
   SymbolContext SymCtx;
   SymCtx.SymbolName = ObjCIVarRecord::createScopedName(Super, R.getName());
   SymCtx.Kind = EncodeKind::ObjectiveCInstanceVariable;
@@ -679,8 +754,6 @@ void DylibVerifier::accumulateSrcLocForDylibSymbols() {
 }
 
 void DylibVerifier::visitObjCInterface(const ObjCInterfaceRecord &R) {
-  if (R.isVerified())
-    return;
   SymbolContext SymCtx;
   SymCtx.SymbolName = R.getName();
   SymCtx.ObjCIFKind = assignObjCIFSymbolKind(&R);
@@ -713,9 +786,12 @@ DylibVerifier::Result DylibVerifier::verifyRemainingSymbols() {
 
   DWARFContext DWARFInfo;
   DWARFCtx = &DWARFInfo;
-  Ctx.DiscoveredFirstError = false;
-  Ctx.PrintArch = true;
+  Ctx.Target = Target(Architecture::AK_unknown, PlatformType::PLATFORM_UNKNOWN);
   for (std::shared_ptr<RecordsSlice> Slice : Dylib) {
+    if (Ctx.Target.Arch == Slice->getTarget().Arch)
+      continue;
+    Ctx.DiscoveredFirstError = false;
+    Ctx.PrintArch = true;
     Ctx.Target = Slice->getTarget();
     Ctx.DylibSlice = Slice.get();
     Slice->visit(*this);

diff --git a/clang/test/InstallAPI/Inputs/MacOSX13.0.sdk/SDKSettings.json b/clang/test/InstallAPI/Inputs/MacOSX13.0.sdk/SDKSettings.json
@@ -0,0 +1,19 @@
+{
+  "DefaultVariant": "macos", "DisplayName": "macOS 13",
+  "Version": "13.0",
+  "MaximumDeploymentTarget": "13.0.99",
+  "PropertyConditionFallbackNames": [], "VersionMap": {
+    "iOSMac_macOS": {
+      "16.1": "13.0",
+      "15.0": "12.0",
+      "13.1": "10.15",
+      "14.0": "11.0"
+    },
+    "macOS_iOSMac": {
+      "13.0": "16.1",
+      "12.0": "15.0",
+      "11.0": "14.0",
+      "10.15": "13.1"
+    }
+  }
+}