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] Add support for parsing dSYMs #86852

Merged
merged 4 commits into from
Mar 29, 2024

Conversation

cyndyishida
Copy link
Member

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.

InstallAPI does not directly look at object files in the dylib. To help
diagnose violations where a declaration is undiscovered in headers,
parse the dSYM and lookup 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.
@llvmbot llvmbot added the clang Clang issues not falling into any other category label Mar 27, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented Mar 27, 2024

@llvm/pr-subscribers-clang

Author: Cyndy Ishida (cyndyishida)

Changes

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.


Full diff: https://github.com/llvm/llvm-project/pull/86852.diff

14 Files Affected:

  • (modified) clang/include/clang/InstallAPI/DylibVerifier.h (+17-3)
  • (modified) clang/include/clang/InstallAPI/MachO.h (+1)
  • (modified) clang/lib/InstallAPI/CMakeLists.txt (+1)
  • (modified) clang/lib/InstallAPI/DylibVerifier.cpp (+54-17)
  • (modified) clang/test/CMakeLists.txt (+1)
  • (added) clang/test/InstallAPI/diagnostics-dsym.test (+40)
  • (modified) clang/test/lit.cfg.py (+1)
  • (modified) clang/tools/clang-installapi/InstallAPIOpts.td (+2)
  • (modified) clang/tools/clang-installapi/Options.cpp (+5-1)
  • (modified) clang/tools/clang-installapi/Options.h (+3)
  • (modified) llvm/include/llvm/TextAPI/DylibReader.h (+9)
  • (modified) llvm/include/llvm/TextAPI/Record.h (+17)
  • (modified) llvm/lib/TextAPI/BinaryReader/CMakeLists.txt (+1)
  • (modified) llvm/lib/TextAPI/BinaryReader/DylibReader.cpp (+111-1)
diff --git a/clang/include/clang/InstallAPI/DylibVerifier.h b/clang/include/clang/InstallAPI/DylibVerifier.h
index 49de24763f1f93..22cdc234486cf3 100644
--- a/clang/include/clang/InstallAPI/DylibVerifier.h
+++ b/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
diff --git a/clang/include/clang/InstallAPI/MachO.h b/clang/include/clang/InstallAPI/MachO.h
index 4961c596fd68ae..827220dbf39fb8 100644
--- a/clang/include/clang/InstallAPI/MachO.h
+++ b/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;
diff --git a/clang/lib/InstallAPI/CMakeLists.txt b/clang/lib/InstallAPI/CMakeLists.txt
index 894db699578f20..e0bc8d969ecb3a 100644
--- a/clang/lib/InstallAPI/CMakeLists.txt
+++ b/clang/lib/InstallAPI/CMakeLists.txt
@@ -1,6 +1,7 @@
 set(LLVM_LINK_COMPONENTS
   Support
   TextAPI
+  TextAPIBinaryReader
   Demangle
   Core
   )
diff --git a/clang/lib/InstallAPI/DylibVerifier.cpp b/clang/lib/InstallAPI/DylibVerifier.cpp
index ba25e4183a9b89..c0eda1d81b9b98 100644
--- a/clang/lib/InstallAPI/DylibVerifier.cpp
+++ b/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,26 +572,36 @@ 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;
   }
@@ -588,10 +609,12 @@ void DylibVerifier::visitSymbolInDylib(const Record &R, SymbolContext &SymCtx) {
   // 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) {
diff --git a/clang/test/CMakeLists.txt b/clang/test/CMakeLists.txt
index fcfca354f4a75f..1efda8462b7481 100644
--- a/clang/test/CMakeLists.txt
+++ b/clang/test/CMakeLists.txt
@@ -111,6 +111,7 @@ set(CLANG_TEST_PARAMS
 
 if( NOT CLANG_BUILT_STANDALONE )
   list(APPEND CLANG_TEST_DEPS
+    dsymutil
     llvm-config
     FileCheck count not
     llc
diff --git a/clang/test/InstallAPI/diagnostics-dsym.test b/clang/test/InstallAPI/diagnostics-dsym.test
new file mode 100644
index 00000000000000..29f47c4e581448
--- /dev/null
+++ b/clang/test/InstallAPI/diagnostics-dsym.test
@@ -0,0 +1,40 @@
+; 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:4: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 = 'a';
+
+;--- usr/lib/libSystem.tbd
+{
+  "main_library": {
+    "install_names": [
+      {"name": "/usr/lib/libSystem.B.dylib"}
+    ],
+    "target_info": [
+      {"target": "arm64-macos"}
+    ]
+  },
+  "tapi_tbd_version": 5
+}
+
diff --git a/clang/test/lit.cfg.py b/clang/test/lit.cfg.py
index e5630a07424c7c..5462beb5bb94b2 100644
--- a/clang/test/lit.cfg.py
+++ b/clang/test/lit.cfg.py
@@ -99,6 +99,7 @@
     "llvm-lto2",
     "llvm-profdata",
     "llvm-readtapi",
+    "dsymutil",
     ToolSubst(
         "%clang_extdef_map",
         command=FindTool("clang-extdef-mapping"),
diff --git a/clang/tools/clang-installapi/InstallAPIOpts.td b/clang/tools/clang-installapi/InstallAPIOpts.td
index 71532c9cf24d17..010f2507a1d1f3 100644
--- a/clang/tools/clang-installapi/InstallAPIOpts.td
+++ b/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">,
diff --git a/clang/tools/clang-installapi/Options.cpp b/clang/tools/clang-installapi/Options.cpp
index 8e4a1b019fd816..c4f39b7c841742 100644
--- a/clang/tools/clang-installapi/Options.cpp
+++ b/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;
 }
 
diff --git a/clang/tools/clang-installapi/Options.h b/clang/tools/clang-installapi/Options.h
index 3671e4c8274bd3..82e04b49d12596 100644
--- a/clang/tools/clang-installapi/Options.h
+++ b/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;
 
diff --git a/llvm/include/llvm/TextAPI/DylibReader.h b/llvm/include/llvm/TextAPI/DylibReader.h
index b556fbf6832a93..6861d3cb1591b7 100644
--- a/llvm/include/llvm/TextAPI/DylibReader.h
+++ b/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
diff --git a/llvm/include/llvm/TextAPI/Record.h b/llvm/include/llvm/TextAPI/Record.h
index ef152ce433877c..7d721988ec3dab 100644
--- a/llvm/include/llvm/TextAPI/Record.h
+++ b/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.
diff --git a/llvm/lib/TextAPI/BinaryReader/CMakeLists.txt b/llvm/lib/TextAPI/BinaryReader/CMakeLists.txt
index cbdf7b2c969698..c4535310d91c17 100644
--- a/llvm/lib/TextAPI/BinaryReader/CMakeLists.txt
+++ b/llvm/lib/TextAPI/BinaryReader/CMakeLists.txt
@@ -2,6 +2,7 @@ add_llvm_component_library(LLVMTextAPIBinaryReader
   DylibReader.cpp
 
   LINK_COMPONENTS
+  DebugInfoDWARF
   Support
   Object
   TextAPI
diff --git a/llvm/lib/TextAPI/BinaryReader/DylibReader.cpp b/llvm/lib/TextAPI/BinaryReader/DylibReader.cpp
index 2e36d4a8b98ce0..2f8a2a277d3a6c 100644
--- a/llvm/lib/TextAPI/BinaryReader/DylibReader.cpp
+++ b/llvm/lib/TextAPI/BinaryReader/DylibReader.cpp
@@ -12,7 +12,8 @@
 
 #include "llvm/TextAPI/DylibReader.h"
 #include "llvm/ADT/STLExtras.h"
-#include "llvm/ADT/StringMap.h"
+#include "llvm/DebugInfo/DWARF/DWARFCompileUnit.h"
+#include "llvm/DebugInfo/DWARF/DWARFContext.h"
 #include "llvm/Object/Binary.h"
 #include "llvm/Object/MachOUniversal.h"
 #include "llvm/Support/Endian.h"
@@ -432,3 +433,112 @@ DylibReader::get(MemoryBufferRef Buffer) {
 
   return convertToInterfaceFile(*SlicesOrErr);
 }
+
+static void DWARFErrorHandler(Error err) { /**/
+}
+
+static SymbolToSourceLocMap
+accumulateLocs(MachOObjectFile &Obj,
+               const std::unique_ptr<DWARFContext> &DiCtx) {
+  SymbolToSourceLocMap LocMap;
+  for (const auto &Symbol : Obj.symbols()) {
+    Expected<uint32_t> FlagsOrErr = Symbol.getFlags();
+    if (!FlagsOrErr) {
+      consumeError(FlagsOrErr.takeError());
+      continue;
+    }
+
+    if (!(*FlagsOrErr & SymbolRef::SF_Exported))
+      continue;
+
+    Expected<uint64_t> AddressOrErr = Symbol.getAddress();
+    if (!AddressOrErr) {
+      consumeError(AddressOrErr.takeError());
+      continue;
+    }
+    const uint64_t Address = *AddressOrErr;
+
+    auto TypeOrErr = Symbol.getType();
+    if (!TypeOrErr) {
+      consumeError(TypeOrErr.takeError());
+      continue;
+    }
+    const bool IsCode = (*TypeOrErr & SymbolRef::ST_Function);
+
+    auto *DWARFCU = IsCode ? DiCtx->getCompileUnitForCodeAddress(Address)
+                           : DiCtx->getCompileUnitForDataAddress(Address);
+    if (!DWARFCU)
+      continue;
+
+    const DWARFDie &DIE = IsCode ? DWARFCU->getSubroutineForAddress(Address)
+                                 : DWARFCU->getVariableForAddress(Address);
+    const std::string File = DIE.getDeclFile(
+        llvm::DILineInfoSpecifier::FileLineInfoKind::AbsoluteFilePath);
+    const uint64_t Line = DIE.getDeclLine();
+
+    auto NameOrErr = Symbol.getName();
+    if (!NameOrErr) {
+      consumeError(NameOrErr.takeError());
+      continue;
+    }
+    auto Name = *NameOrErr;
+    auto Sym = parseSymbol(Name);
+
+    if (!File.empty() && Line != 0)
+      LocMap.insert({Sym.Name.str(), RecordLoc(File, Line)});
+  }
+
+  return LocMap;
+}
+
+SymbolToSourceLocMap
+DylibReader::accumulateSourceLocFromDSYM(const StringRef DSYM,
+                                         const Target &T) {
+  // Find sidecar file.
+  auto DSYMsOrErr = MachOObjectFile::findDsymObjectMembers(DSYM);
+  if (!DSYMsOrErr) {
+    consumeError(DSYMsOrErr.takeError());
+    return SymbolToSourceLocMap();
+  }
+  if (DSYMsOrErr->empty())
+    return SymbolToSourceLocMap();
+
+  const StringRef Path = DSYMsOrErr->front();
+  auto BufOrErr = MemoryBuffer::getFile(Path);
+  if (auto Err = BufOrErr.getError())
+    return SymbolToSourceLocMap();
+
+  auto BinOrErr = createBinary(*BufOrErr.get());
+  if (!BinOrErr) {
+    consumeError(BinOrErr.takeError());
+    return SymbolToSourceLocMap();
+  }
+  // Handle single arch.
+  if (auto *Single = dyn_cast<MachOObjectFile>(BinOrErr->get())) {
+    auto DiCtx = DWARFContext::create(
+        *Single, DWARFContext::ProcessDebugRelocations::Process, nullptr, "",
+        DWARFErrorHandler, DWARFErrorHandler);
+
+    return accumulateLocs(*Single, DiCtx);
+  }
+  // Handle universal companion file.
+  if (auto *Fat = dyn_cast<MachOUniversalBinary>(BinOrErr->get())) {
+    auto ObjForArch = Fat->getObjectForArch(getArchitectureName(T.Arch));
+    if (!ObjForArch) {
+      consumeError(ObjForArch.takeError());
+      return SymbolToSourceLocMap();
+    }
+    auto MachOOrErr = ObjForArch->getAsObjectFile();
+    if (!MachOOrErr) {
+      consumeError(MachOOrErr.takeError());
+      return SymbolToSourceLocMap();
+    }
+    auto &Obj = **MachOOrErr;
+    auto DiCtx = DWARFContext::create(
+        Obj, DWARFContext::ProcessDebugRelocations::Process, nullptr, "",
+        DWARFErrorHandler, DWARFErrorHandler);
+
+    return accumulateLocs(Obj, DiCtx);
+  }
+  return SymbolToSourceLocMap();
+}

; RUN: split-file %s %t

// Build a simple dylib with debug info.
; RUN: %clang --target=arm64-apple-macos13 -g -dynamiclib %t/foo.c \
Copy link
Member Author

Choose a reason for hiding this comment

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

@JDevlieghere Is there a better/known practice for generating full dylibs with debug info for tests? Hopefully, PR CI will let me know, but I'm a little worried about different linker semantics.
e.g. Xcode's ld requires all userspace dylibs link against libSystem.

Copy link
Member Author

Choose a reason for hiding this comment

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

Looks like the linker on the linux bot doesn't support building Darwin anyway

/usr/bin/ld: unrecognised emulation mode: llvm
Supported emulations: elf_x86_64 elf32_x86_64 elf_i386 elf_iamcu elf_l1om elf_k1om i386pep i386pe
clang: error: linker command failed with exit code 1 (use -v to see invocation)

Perhaps I'll require Darwin env to run the test and then I won't need to introduce a dsymutil dependency at all since it can come from xcode.

Copy link

github-actions bot commented Mar 27, 2024

✅ With the latest revision this PR passed the C/C++ code formatter.

Require the test to run on darwin, where an xcode or command
line tools install should be
present on any supported configuration. Dsymutil can be used from there.
@cyndyishida cyndyishida merged commit a4de589 into llvm:main Mar 29, 2024
4 checks passed
@cyndyishida cyndyishida deleted the eng/PR-clangInstallAPIDsyms branch March 29, 2024 15:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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