[InstallAPI] Add support for parsing dSYMs (#86852)

InstallAPI does not directly look at object files in the dylib for
verification. To help diagnose violations where a declaration is
undiscovered in headers, parse the dSYM and look up the source location
for symbols. Emitting out the source location with a diagnostic is
enough for some IDE's (e.g. Xcode) to have them map back to editable
source files.

Author: cyndyishida

clang/include/clang/InstallAPI/DylibVerifier.h

@@ -31,6 +31,7 @@ enum class VerificationMode {
 class DylibVerifier : llvm::MachO::RecordVisitor {
 private:
   struct SymbolContext;
+  struct DWARFContext;
 
 public:
   enum class Result { NoVerify, Ignore, Valid, Invalid };
@@ -54,7 +55,7 @@ class DylibVerifier : llvm::MachO::RecordVisitor {
     DiagnosticsEngine *Diag = nullptr;
 
     // Handle diagnostics reporting for target level violations.
-    void emitDiag(llvm::function_ref<void()> Report);
+    void emitDiag(llvm::function_ref<void()> Report, RecordLoc *Loc = nullptr);
 
     VerifierContext() = default;
     VerifierContext(DiagnosticsEngine *Diag) : Diag(Diag) {}
@@ -63,9 +64,10 @@ class DylibVerifier : llvm::MachO::RecordVisitor {
   DylibVerifier() = default;
 
   DylibVerifier(llvm::MachO::Records &&Dylib, DiagnosticsEngine *Diag,
-                VerificationMode Mode, bool Demangle)
+                VerificationMode Mode, bool Demangle, StringRef DSYMPath)
       : Dylib(std::move(Dylib)), Mode(Mode), Demangle(Demangle),
-        Exports(std::make_unique<SymbolSet>()), Ctx(VerifierContext{Diag}) {}
+        DSYMPath(DSYMPath), Exports(std::make_unique<SymbolSet>()),
+        Ctx(VerifierContext{Diag}) {}
 
   Result verify(GlobalRecord *R, const FrontendAttrs *FA);
   Result verify(ObjCInterfaceRecord *R, const FrontendAttrs *FA);
@@ -143,6 +145,12 @@ class DylibVerifier : llvm::MachO::RecordVisitor {
   std::string getAnnotatedName(const Record *R, SymbolContext &SymCtx,
                                bool ValidSourceLoc = true);
 
+  /// Extract source location for symbol implementations.
+  /// As this is a relatively expensive operation, it is only used
+  /// when there is a violation to report and there is not a known declaration
+  /// in the interface.
+  void accumulateSrcLocForDylibSymbols();
+
   // Symbols in dylib.
   llvm::MachO::Records Dylib;
 
@@ -152,11 +160,17 @@ class DylibVerifier : llvm::MachO::RecordVisitor {
   // Attempt to demangle when reporting violations.
   bool Demangle = false;
 
+  // File path to DSYM file.
+  StringRef DSYMPath;
+
   // 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;
+
+  // Track DWARF provided source location for dylibs.
+  DWARFContext *DWARFCtx = nullptr;
 };
 
 } // namespace installapi

clang/include/clang/InstallAPI/MachO.h

@@ -34,6 +34,7 @@ using ObjCCategoryRecord = llvm::MachO::ObjCCategoryRecord;
 using ObjCIVarRecord = llvm::MachO::ObjCIVarRecord;
 using ObjCIFSymbolKind = llvm::MachO::ObjCIFSymbolKind;
 using Records = llvm::MachO::Records;
+using RecordLoc = llvm::MachO::RecordLoc;
 using RecordsSlice = llvm::MachO::RecordsSlice;
 using BinaryAttrs = llvm::MachO::RecordsSlice::BinaryAttrs;
 using SymbolSet = llvm::MachO::SymbolSet;

clang/lib/InstallAPI/CMakeLists.txt

@@ -1,6 +1,7 @@
 set(LLVM_LINK_COMPONENTS
   Support
   TextAPI
+  TextAPIBinaryReader
   Demangle
   Core
   )

clang/lib/InstallAPI/DylibVerifier.cpp

@@ -10,6 +10,7 @@
 #include "clang/InstallAPI/FrontendRecords.h"
 #include "clang/InstallAPI/InstallAPIDiagnostic.h"
 #include "llvm/Demangle/Demangle.h"
+#include "llvm/TextAPI/DylibReader.h"
 
 using namespace llvm::MachO;
 
@@ -35,6 +36,14 @@ struct DylibVerifier::SymbolContext {
   bool Inlined = false;
 };
 
+struct DylibVerifier::DWARFContext {
+  // Track whether DSYM parsing has already been attempted to avoid re-parsing.
+  bool ParsedDSYM{false};
+
+  // Lookup table for source locations by symbol name.
+  DylibReader::SymbolToSourceLocMap SourceLocs{};
+};
+
 static bool isCppMangled(StringRef Name) {
   // InstallAPI currently only supports itanium manglings.
   return (Name.starts_with("_Z") || Name.starts_with("__Z") ||
@@ -511,14 +520,16 @@ DylibVerifier::Result DylibVerifier::verify(GlobalRecord *R,
   return verifyImpl(R, SymCtx);
 }
 
-void DylibVerifier::VerifierContext::emitDiag(
-    llvm::function_ref<void()> Report) {
+void DylibVerifier::VerifierContext::emitDiag(llvm::function_ref<void()> Report,
+                                              RecordLoc *Loc) {
   if (!DiscoveredFirstError) {
     Diag->Report(diag::warn_target)
         << (PrintArch ? getArchitectureName(Target.Arch)
                       : getTargetTripleName(Target));
     DiscoveredFirstError = true;
   }
+  if (Loc && Loc->isValid())
+    llvm::errs() << Loc->File << ":" << Loc->Line << ":" << 0 << ": ";
 
   Report();
 }
@@ -561,37 +572,49 @@ void DylibVerifier::visitSymbolInDylib(const Record &R, SymbolContext &SymCtx) {
     return;
   }
 
-  // All checks at this point classify as some kind of violation that should be
-  // reported.
+  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.
+  if (IsLinkerSymbol || (Mode > VerificationMode::ErrorsOnly))
+    accumulateSrcLocForDylibSymbols();
+  RecordLoc Loc = DWARFCtx->SourceLocs.lookup(SymCtx.SymbolName);
 
   // 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);
-    });
+  if (IsLinkerSymbol) {
+    Ctx.emitDiag(
+        [&]() {
+          Ctx.Diag->Report(diag::err_header_symbol_missing)
+              << getAnnotatedName(&R, SymCtx, Loc.isValid());
+        },
+        &Loc);
     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);
-    });
+    Ctx.emitDiag(
+        [&]() {
+          Ctx.Diag->Report(diag::err_header_symbol_missing)
+              << getAnnotatedName(&R, SymCtx, Loc.isValid());
+        },
+        &Loc);
     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);
-    });
+    Ctx.emitDiag(
+        [&]() {
+          Ctx.Diag->Report(diag::warn_header_symbol_missing)
+              << getAnnotatedName(&R, SymCtx, Loc.isValid());
+        },
+        &Loc);
     updateState(Result::Ignore);
     return;
   }
@@ -622,6 +645,18 @@ void DylibVerifier::visitObjCIVar(const ObjCIVarRecord &R,
   visitSymbolInDylib(R, SymCtx);
 }
 
+void DylibVerifier::accumulateSrcLocForDylibSymbols() {
+  if (DSYMPath.empty())
+    return;
+
+  assert(DWARFCtx != nullptr && "Expected an initialized DWARFContext");
+  if (DWARFCtx->ParsedDSYM)
+    return;
+  DWARFCtx->ParsedDSYM = true;
+  DWARFCtx->SourceLocs =
+      DylibReader::accumulateSourceLocFromDSYM(DSYMPath, Ctx.Target);
+}
+
 void DylibVerifier::visitObjCInterface(const ObjCInterfaceRecord &R) {
   if (R.isVerified())
     return;
@@ -655,6 +690,8 @@ DylibVerifier::Result DylibVerifier::verifyRemainingSymbols() {
     return Result::NoVerify;
   assert(!Dylib.empty() && "No binary to verify against");
 
+  DWARFContext DWARFInfo;
+  DWARFCtx = &DWARFInfo;
   Ctx.DiscoveredFirstError = false;
   Ctx.PrintArch = true;
   for (std::shared_ptr<RecordsSlice> Slice : Dylib) {

clang/test/InstallAPI/diagnostics-dsym.test

@@ -0,0 +1,43 @@
+; REQUIRES: system-darwin
+
+; RUN: rm -rf %t
+; RUN: split-file %s %t
+
+// Build a simple dylib with debug info.
+; RUN: %clang --target=arm64-apple-macos13 -g -dynamiclib %t/foo.c \
+; RUN: -current_version 1 -compatibility_version 1 -L%t/usr/lib \
+; RUN: -save-temps \
+; RUN: -o %t/foo.dylib -install_name %t/foo.dylib
+; RUN: dsymutil %t/foo.dylib -o %t/foo.dSYM
+
+; RUN: not clang-installapi -x c++ --target=arm64-apple-macos13 \
+; RUN: -install_name %t/foo.dylib  \
+; RUN: -current_version 1 -compatibility_version 1 \
+; RUN: -o %t/output.tbd \
+; RUN: --verify-against=%t/foo.dylib --dsym=%t/foo.dSYM \
+; RUN: --verify-mode=Pedantic 2>&1 | FileCheck %s
+
+; CHECK: violations found for arm64 
+; CHECK: foo.c:5:0: error: no declaration found for exported symbol 'bar' in dynamic library
+; CHECK: foo.c:1:0: error: no declaration found for exported symbol 'foo' in dynamic library
+
+;--- foo.c
+int foo(void) {
+  return 1;
+}
+extern char bar;
+char bar = 'a';
+
+;--- usr/lib/libSystem.tbd
+{
+  "main_library": {
+    "install_names": [
+      {"name": "/usr/lib/libSystem.B.dylib"}
+    ],
+    "target_info": [
+      {"target": "arm64-macos"}
+    ]
+  },
+  "tapi_tbd_version": 5
+}
+

clang/tools/clang-installapi/InstallAPIOpts.td

@@ -29,6 +29,8 @@ def verify_mode_EQ : Joined<["--"], "verify-mode=">,
   HelpText<"Specify the severity and extend of the validation. Valid modes are ErrorsOnly, ErrorsAndWarnings, and Pedantic.">;
 def demangle : Flag<["--", "-"], "demangle">,
   HelpText<"Demangle symbols when printing warnings and errors">;
+def dsym: Joined<["--"], "dsym=">,
+  MetaVarName<"<path>">, HelpText<"Specify dSYM path for enriched diagnostics.">;
 
 // Additional input options.
 def extra_project_header : Separate<["-"], "extra-project-header">,

clang/tools/clang-installapi/Options.cpp

@@ -241,6 +241,9 @@ Options::processAndFilterOutInstallAPIOptions(ArrayRef<const char *> Args) {
   if (const Arg *A = ParsedArgs.getLastArg(OPT_verify_against))
     DriverOpts.DylibToVerify = A->getValue();
 
+  if (const Arg *A = ParsedArgs.getLastArg(OPT_dsym))
+    DriverOpts.DSYMPath = A->getValue();
+
   // Handle exclude & extra header directories or files.
   auto handleAdditionalInputArgs = [&](PathSeq &Headers,
                                        clang::installapi::ID OptID) {
@@ -522,7 +525,8 @@ InstallAPIContext Options::createContext() {
   }
 
   Ctx.Verifier = std::make_unique<DylibVerifier>(
-      std::move(*Slices), Diags, DriverOpts.VerifyMode, DriverOpts.Demangle);
+      std::move(*Slices), Diags, DriverOpts.VerifyMode, DriverOpts.Demangle,
+      DriverOpts.DSYMPath);
   return Ctx;
 }
 

clang/tools/clang-installapi/Options.h

@@ -67,6 +67,9 @@ struct DriverOptions {
   /// \brief Output path.
   std::string OutputPath;
 
+  /// \brief DSYM path.
+  std::string DSYMPath;
+
   /// \brief File encoding to print.
   FileType OutFT = FileType::TBD_V5;
 

llvm/include/llvm/TextAPI/DylibReader.h

@@ -13,6 +13,7 @@
 #ifndef LLVM_TEXTAPI_DYLIBREADER_H
 #define LLVM_TEXTAPI_DYLIBREADER_H
 
+#include "llvm/ADT/StringMap.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/TextAPI/ArchitectureSet.h"
@@ -43,6 +44,14 @@ Expected<Records> readFile(MemoryBufferRef Buffer, const ParseOption &Opt);
 /// \param Buffer Data that points to dylib.
 Expected<std::unique_ptr<InterfaceFile>> get(MemoryBufferRef Buffer);
 
+using SymbolToSourceLocMap = llvm::StringMap<RecordLoc>;
+/// Get the source location for each symbol from dylib.
+///
+/// \param DSYM Path to DSYM file.
+/// \param T Requested target slice for dylib.
+SymbolToSourceLocMap accumulateSourceLocFromDSYM(const StringRef DSYM,
+                                                 const Target &T);
+
 } // namespace llvm::MachO::DylibReader
 
 #endif // LLVM_TEXTAPI_DYLIBREADER_H

llvm/include/llvm/TextAPI/Record.h

@@ -27,6 +27,23 @@ LLVM_ENABLE_BITMASK_ENUMS_IN_NAMESPACE();
 
 class RecordsSlice;
 
+// Defines lightweight source location for records.
+struct RecordLoc {
+  RecordLoc() = default;
+  RecordLoc(std::string File, unsigned Line)
+      : File(std::move(File)), Line(Line) {}
+
+  /// Whether there is source location tied to the RecordLoc object.
+  bool isValid() const { return !File.empty(); }
+
+  bool operator==(const RecordLoc &O) const {
+    return std::tie(File, Line) == std::tie(O.File, O.Line);
+  }
+
+  const std::string File;
+  const unsigned Line = 0;
+};
+
 // Defines a list of linkage types.
 enum class RecordLinkage : uint8_t {
   // Unknown linkage.

llvm/lib/TextAPI/BinaryReader/CMakeLists.txt

@@ -2,6 +2,7 @@ add_llvm_component_library(LLVMTextAPIBinaryReader
   DylibReader.cpp
 
   LINK_COMPONENTS
+  DebugInfoDWARF
   Support
   Object
   TextAPI