[InstallAPI] Report exports discovered in binary but not in interface (…

…#86025)

This patch completes the classes of errors installapi can detect.

clang/include/clang/Basic/DiagnosticInstallAPIKinds.td

@@ -26,6 +26,7 @@ def warn_library_hidden_symbol : Warning<"declaration has external linkage, but
 def warn_header_hidden_symbol : Warning<"symbol exported in dynamic library, but marked hidden in declaration '%0'">, InGroup<InstallAPIViolation>;
 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_symbol_missing : Warning<"no declaration was found for exported symbol '%0' in dynamic library">, InGroup<InstallAPIViolation>;
 def warn_header_availability_mismatch : Warning<"declaration '%0' is marked %select{available|unavailable}1,"
   " but symbol is %select{not |}2exported in dynamic library">, InGroup<InstallAPIViolation>;
 def err_header_availability_mismatch : Error<"declaration '%0' is marked %select{available|unavailable}1,"

clang/include/clang/InstallAPI/DylibVerifier.h

@@ -28,7 +28,7 @@ enum class VerificationMode {
 /// 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 {
+class DylibVerifier : llvm::MachO::RecordVisitor {
 private:
   struct SymbolContext;
 
@@ -72,6 +72,9 @@ class DylibVerifier {
   Result verify(ObjCIVarRecord *R, const FrontendAttrs *FA,
                 const StringRef SuperClass);
 
+  // Scan through dylib slices and report any remaining missing exports.
+  Result verifyRemainingSymbols();
+
   /// Initialize target for verification.
   void setTarget(const Target &T);
 
@@ -128,6 +131,14 @@ class DylibVerifier {
   /// Find matching dylib slice for target triple that is being parsed.
   void assignSlice(const Target &T);
 
+  /// Shared implementation for verifying exported symbols in dylib.
+  void visitSymbolInDylib(const Record &R, SymbolContext &SymCtx);
+
+  void visitGlobal(const GlobalRecord &R) override;
+  void visitObjCInterface(const ObjCInterfaceRecord &R) override;
+  void visitObjCCategory(const ObjCCategoryRecord &R) override;
+  void visitObjCIVar(const ObjCIVarRecord &R, const StringRef Super);
+
   /// Gather annotations for symbol for error reporting.
   std::string getAnnotatedName(const Record *R, SymbolContext &SymCtx,
                                bool ValidSourceLoc = true);

clang/lib/InstallAPI/DylibVerifier.cpp

@@ -66,17 +66,15 @@ std::string DylibVerifier::getAnnotatedName(const Record *R,
     Annotation += "(tlv) ";
 
   // Check if symbol represents only part of a @interface declaration.
-  const bool IsAnnotatedObjCClass =
-      ((SymCtx.ObjCIFKind != ObjCIFSymbolKind::None) &&
-       (SymCtx.ObjCIFKind <= ObjCIFSymbolKind::EHType));
-
-  if (IsAnnotatedObjCClass) {
-    if (SymCtx.ObjCIFKind == ObjCIFSymbolKind::EHType)
-      Annotation += "Exception Type of ";
-    if (SymCtx.ObjCIFKind == ObjCIFSymbolKind::MetaClass)
-      Annotation += "Metaclass of ";
-    if (SymCtx.ObjCIFKind == ObjCIFSymbolKind::Class)
-      Annotation += "Class of ";
+  switch (SymCtx.ObjCIFKind) {
+  default:
+    break;
+  case ObjCIFSymbolKind::EHType:
+    return Annotation + "Exception Type of " + PrettyName;
+  case ObjCIFSymbolKind::MetaClass:
+    return Annotation + "Metaclass of " + PrettyName;
+  case ObjCIFSymbolKind::Class:
+    return Annotation + "Class of " + PrettyName;
   }
 
   // Only print symbol type prefix or leading "_" if there is no source location
@@ -90,9 +88,6 @@ std::string DylibVerifier::getAnnotatedName(const Record *R,
     return Annotation + PrettyName;
   }
 
-  if (IsAnnotatedObjCClass)
-    return Annotation + PrettyName;
-
   switch (SymCtx.Kind) {
   case EncodeKind::GlobalSymbol:
     return Annotation + PrettyName;
@@ -332,9 +327,9 @@ bool DylibVerifier::compareSymbolFlags(const Record *R, SymbolContext &SymCtx,
   }
   if (!DR->isThreadLocalValue() && R->isThreadLocalValue()) {
     Ctx.emitDiag([&]() {
-      SymCtx.FA->D->getLocation(),
-          Ctx.Diag->Report(diag::err_header_symbol_flags_mismatch)
-              << getAnnotatedName(DR, SymCtx) << R->isThreadLocalValue();
+      Ctx.Diag->Report(SymCtx.FA->D->getLocation(),
+                       diag::err_header_symbol_flags_mismatch)
+          << getAnnotatedName(R, SymCtx) << R->isThreadLocalValue();
     });
     return false;
   }
@@ -520,5 +515,147 @@ void DylibVerifier::VerifierContext::emitDiag(
   Report();
 }
 
+// The existence of weak-defined RTTI can not always be inferred from the
+// header files because they can be generated as part of an implementation
+// file.
+// InstallAPI doesn't warn about weak-defined RTTI, because this doesn't affect
+// static linking and so can be ignored for text-api files.
+static bool shouldIgnoreCpp(StringRef Name, bool IsWeakDef) {
+  return (IsWeakDef &&
+          (Name.starts_with("__ZTI") || Name.starts_with("__ZTS")));
+}
+void DylibVerifier::visitSymbolInDylib(const Record &R, SymbolContext &SymCtx) {
+  // Undefined symbols should not be in InstallAPI generated text-api files.
+  if (R.isUndefined()) {
+    updateState(Result::Valid);
+    return;
+  }
+
+  // Internal symbols should not be in InstallAPI generated text-api files.
+  if (R.isInternal()) {
+    updateState(Result::Valid);
+    return;
+  }
+
+  // Allow zippered symbols with potentially mismatching availability
+  // between macOS and macCatalyst in the final text-api file.
+  const StringRef SymbolName(SymCtx.SymbolName);
+  if (const Symbol *Sym = Exports->findSymbol(SymCtx.Kind, SymCtx.SymbolName,
+                                              SymCtx.ObjCIFKind)) {
+    if (Sym->hasArchitecture(Ctx.Target.Arch)) {
+      updateState(Result::Ignore);
+      return;
+    }
+  }
+
+  if (shouldIgnoreCpp(SymbolName, R.isWeakDefined())) {
+    updateState(Result::Valid);
+    return;
+  }
+
+  // All checks at this point classify as some kind of violation that should be
+  // reported.
+
+  // Regardless of verification mode, error out on mismatched special linker
+  // symbols.
+  if (SymbolName.starts_with("$ld$")) {
+    Ctx.emitDiag([&]() {
+      Ctx.Diag->Report(diag::err_header_symbol_missing)
+          << getAnnotatedName(&R, SymCtx, /*ValidSourceLoc=*/false);
+    });
+    updateState(Result::Invalid);
+    return;
+  }
+
+  // Missing declarations for exported symbols are hard errors on Pedantic mode.
+  if (Mode == VerificationMode::Pedantic) {
+    Ctx.emitDiag([&]() {
+      Ctx.Diag->Report(diag::err_header_symbol_missing)
+          << getAnnotatedName(&R, SymCtx, /*ValidSourceLoc=*/false);
+    });
+    updateState(Result::Invalid);
+    return;
+  }
+
+  // Missing declarations for exported symbols are warnings on ErrorsAndWarnings
+  // mode.
+  if (Mode == VerificationMode::ErrorsAndWarnings) {
+    Ctx.emitDiag([&]() {
+      Ctx.Diag->Report(diag::warn_header_symbol_missing)
+          << getAnnotatedName(&R, SymCtx, /*ValidSourceLoc=*/false);
+    });
+    updateState(Result::Ignore);
+    return;
+  }
+
+  // Missing declarations are dropped for ErrorsOnly mode. It is the last
+  // remaining mode.
+  updateState(Result::Ignore);
+  return;
+}
+
+void DylibVerifier::visitGlobal(const GlobalRecord &R) {
+  if (R.isVerified())
+    return;
+  SymbolContext SymCtx;
+  SimpleSymbol Sym = parseSymbol(R.getName());
+  SymCtx.SymbolName = Sym.Name;
+  SymCtx.Kind = Sym.Kind;
+  visitSymbolInDylib(R, SymCtx);
+}
+
+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;
+  visitSymbolInDylib(R, SymCtx);
+}
+
+void DylibVerifier::visitObjCInterface(const ObjCInterfaceRecord &R) {
+  if (R.isVerified())
+    return;
+  SymbolContext SymCtx;
+  SymCtx.SymbolName = R.getName();
+  SymCtx.ObjCIFKind = assignObjCIFSymbolKind(&R);
+  if (SymCtx.ObjCIFKind > ObjCIFSymbolKind::EHType) {
+    if (R.hasExceptionAttribute()) {
+      SymCtx.Kind = EncodeKind::ObjectiveCClassEHType;
+      visitSymbolInDylib(R, SymCtx);
+    }
+    SymCtx.Kind = EncodeKind::ObjectiveCClass;
+    visitSymbolInDylib(R, SymCtx);
+  } else {
+    SymCtx.Kind = R.hasExceptionAttribute() ? EncodeKind::ObjectiveCClassEHType
+                                            : EncodeKind::ObjectiveCClass;
+    visitSymbolInDylib(R, SymCtx);
+  }
+
+  for (const ObjCIVarRecord *IV : R.getObjCIVars())
+    visitObjCIVar(*IV, R.getName());
+}
+
+void DylibVerifier::visitObjCCategory(const ObjCCategoryRecord &R) {
+  for (const ObjCIVarRecord *IV : R.getObjCIVars())
+    visitObjCIVar(*IV, R.getSuperClassName());
+}
+
+DylibVerifier::Result DylibVerifier::verifyRemainingSymbols() {
+  if (getState() == Result::NoVerify)
+    return Result::NoVerify;
+  assert(!Dylib.empty() && "No binary to verify against");
+
+  Ctx.DiscoveredFirstError = false;
+  Ctx.PrintArch = true;
+  for (std::shared_ptr<RecordsSlice> Slice : Dylib) {
+    Ctx.Target = Slice->getTarget();
+    Ctx.DylibSlice = Slice.get();
+    Slice->visit(*this);
+  }
+  return getState();
+}
+
 } // namespace installapi
 } // namespace clang

clang/test/InstallAPI/diagnostics-cpp.test

@@ -21,6 +21,8 @@ CHECK-NEXT: CPP.h:5:7: error: declaration has external linkage, but symbol has i
 CHECK-NEXT: CPP.h:6:7: error: dynamic library symbol '(weak-def) Bar::init()' is weak defined, but its declaration is not
 CHECK-NEXT:   int init();
 CHECK-NEXT:       ^
+CHECK-NEXT: warning: violations found for arm64
+CHECK-NEXT: error: no declaration found for exported symbol 'int foo<unsigned int>(unsigned int)' in dynamic library
 
 //--- inputs.json.in
 {