[dsymutil] Upstream getBundleInfo implementation

[JDevlieghere]

Summary

This patch implements getBundleInfo, which uses CoreFoundation to
obtain information about the CFBundle. This information is needed to
populate the Plist in the dSYM bundle.

This change only applies to darwin and is an NFC as far as other
platforms are concerned.

[vsk]

Thanks for sending this out! Are there tests?

tools/dsymutil/CFBundle.cpp

20 ↗

(On Diff #123574)

Why not define CFReleaser as a std::unique_ptr with a custom deleter? That would drop support for get_address, but it looks dead anyway.

91 ↗

(On Diff #123574)

Why do the subclasses of CFReleaser<T> need virtual destructors?

tools/dsymutil/CFBundle.h

1 ↗

(On Diff #123574)

Missing license?

[aprantl]

tools/dsymutil/CFBundle.cpp

119 ↗

(On Diff #123574)

Capitalize according to LLVM coding guidelines? (This applies to the entire file)

tools/dsymutil/CFBundle.h

5 ↗

(On Diff #123574)

does this only work when __APPLE__ is set? Otherwise, this would be a better fit for Support since it might be useful for other tools, too.

7 ↗

(On Diff #123574)

Should this return a struct instead?

[JDevlieghere]

• Use std::unique_ptr with customer deleter.
• Add ASCII art for license.
• Conform CF code to LLVM's coding standards.

[JDevlieghere]

tools/dsymutil/CFBundle.h

5 ↗

(On Diff #123574)

On non __APPLE__ the function returns the default ("1" and "1.0"). The advantage of keeping this here is that we only have to link dsymutil against CoreFoundation. Do you think it's worth the trade-off?

[aprantl]

Couple of stylistic comments, otherwise this looks good.

tools/dsymutil/CFBundle.cpp

54 ↗

(On Diff #124731)

comment what the class is for?

64 ↗

(On Diff #124731)

comment what this function is doing?

76 ↗

(On Diff #124731)

It might make sense to move all these short and undocumented functions into the class declaration.

86 ↗

(On Diff #124731)

move into declaration?

99 ↗

(On Diff #124731)

comment what this function is doing?

182 ↗

(On Diff #124731)

perhaps move into declaration?

188 ↗

(On Diff #124731)

move into declaration?

193 ↗

(On Diff #124731)

move into declaration?

238 ↗

(On Diff #124731)

move into declaration?

244 ↗

(On Diff #124731)

move into declaration?

266 ↗

(On Diff #124731)

should this be the doxygen comment for the function?

tools/dsymutil/CFBundle.h

5 ↗

(On Diff #123574)

The extra dependency is a pretty strong argument for keeping it in dsymutil, thanks!

[JDevlieghere]

• Feedback Adrian
• Remove dead code

[vsk]

Thanks, lgtm!

llvm/trunk/test/tools/dsymutil/Inputs/Info.plist

+<?xml version="1.0" encoding="UTF-8"?>
+<!DOCTYPE plist PUBLIC "-//Apple Computer//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd">
+<plist version="1.0">
+  <dict>
+    <key>CFBundleDevelopmentRegion</key>
+    <string>English</string>
+    <key>CFBundleIdentifier</key>
+    <string>custom</string>
+    <key>CFBundleInfoDictionaryVersion</key>
+    <string>6.0</string>
+    <key>CFBundlePackageType</key>
+    <string>dSYM</string>
+    <key>CFBundleSignature</key>
+    <string>????</string>
+    <key>CFBundleShortVersionString</key>
+    <string>2.0</string>
+    <key>CFBundleVersion</key>
+    <string>2</string>
+  </dict>
+</plist>

llvm/trunk/test/tools/dsymutil/X86/darwin-bundle.test

+REQUIRES: system-darwin
+
+RUN: rm -rf %t
+RUN: mkdir -p %t/dsymdest
+RUN: cat %p/../Inputs/basic.macho.x86_64 > %t/basic.macho.x86_64
+RUN: cat %p/../Inputs/Info.plist > %t/Info.plist
+
+RUN: llvm-dsymutil -oso-prepend-path=%p/.. %t/basic.macho.x86_64 -o %t/dsymdest/basic.macho.x86_64.dSYM
+RUN: FileCheck %s --input-file %t/dsymdest/basic.macho.x86_64.dSYM/Contents/Info.plist
+
+CHECK: <?xml version="1.0" encoding="UTF-8"?>
+CHECK-NEXT: <!DOCTYPE plist PUBLIC "-//Apple Computer//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd">
+CHECK-NEXT: <plist version="1.0">
+CHECK-NEXT:         <dict>
+CHECK-NEXT:                 <key>CFBundleDevelopmentRegion</key>
+CHECK-NEXT:                 <string>English</string>
+CHECK-NEXT:                 <key>CFBundleIdentifier</key>
+CHECK-NEXT:                 <string>com.apple.xcode.dsym.custom</string>
+CHECK-NEXT:                 <key>CFBundleInfoDictionaryVersion</key>
+CHECK-NEXT:                 <string>6.0</string>
+CHECK-NEXT:                 <key>CFBundlePackageType</key>
+CHECK-NEXT:                 <string>dSYM</string>
+CHECK-NEXT:                 <key>CFBundleSignature</key>
+CHECK-NEXT:                 <string>????</string>
+CHECK-NEXT:                 <key>CFBundleShortVersionString</key>
+CHECK-NEXT:                 <string>2.0</string>
+CHECK-NEXT:                 <key>CFBundleVersion</key>
+CHECK-NEXT:                 <string>2</string>
+CHECK-NEXT:         </dict>
+CHECK-NEXT: </plist>

llvm/trunk/tools/dsymutil/CFBundle.h

+//===- tools/dsymutil/CFBundle.h - CFBundle helper --------------*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "llvm/ADT/StringRef.h"
+#include <string>
+
+namespace llvm {
+namespace dsymutil {
+
+struct CFBundleInfo {
+  std::string VersionStr = "1";
+  std::string ShortVersionStr = "1.0";
+  std::string IDStr;
+  bool OmitShortVersion() const { return ShortVersionStr.empty(); }
+};
+
+CFBundleInfo getBundleInfo(llvm::StringRef ExePath);
+
+} // end namespace dsymutil
+} // end namespace llvm

llvm/trunk/tools/dsymutil/CFBundle.cpp

+//===- tools/dsymutil/CFBundle.cpp - CFBundle helper ------------*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "CFBundle.h"
+
+#ifdef __APPLE__
+#include "llvm/Support/FileSystem.h"
+#include "llvm/Support/raw_ostream.h"
+#include <CoreFoundation/CoreFoundation.h>
+#include <assert.h>
+#include <glob.h>
+#include <memory>
+
+namespace llvm {
+namespace dsymutil {
+
+/// Deleter that calls CFRelease rather than deleting the pointer.
+template <typename T> struct CFDeleter {
+  void operator()(T *P) {
+    if (P)
+      ::CFRelease(P);
+  }
+};
+
+/// This helper owns any CoreFoundation pointer and will call CFRelease() on
+/// any valid pointer it owns unless that pointer is explicitly released using
+/// the release() member function.
+template <typename T>
+using CFReleaser =
+    std::unique_ptr<typename std::remove_pointer<T>::type,
+                    CFDeleter<typename std::remove_pointer<T>::type>>;
+
+/// RAII wrapper around CFBundleRef.
+class CFString : public CFReleaser<CFStringRef> {
+public:
+  CFString(CFStringRef CFStr = nullptr) : CFReleaser<CFStringRef>(CFStr) {}
+
+  const char *UTF8(std::string &Str) const {
+    return CFString::UTF8(get(), Str);
+  }
+
+  CFIndex GetLength() const {
+    if (CFStringRef Str = get())
+      return CFStringGetLength(Str);
+    return 0;
+  }
+
+  static const char *UTF8(CFStringRef CFStr, std::string &Str);
+};
+
+/// Static function that puts a copy of the UTF8 contents of CFStringRef into
+/// std::string and returns the C string pointer that is contained in the
+/// std::string when successful, nullptr otherwise.
+///
+/// This allows the std::string parameter to own the extracted string, and also
+/// allows that string to be returned as a C string pointer that can be used.
+const char *CFString::UTF8(CFStringRef CFStr, std::string &Str) {
+  if (!CFStr)
+    return nullptr;
+
+  const CFStringEncoding Encoding = kCFStringEncodingUTF8;
+  CFIndex MaxUTF8StrLength = CFStringGetLength(CFStr);
+  MaxUTF8StrLength =
+      CFStringGetMaximumSizeForEncoding(MaxUTF8StrLength, Encoding);
+  if (MaxUTF8StrLength > 0) {
+    Str.resize(MaxUTF8StrLength);
+    if (!Str.empty() &&
+        CFStringGetCString(CFStr, &Str[0], Str.size(), Encoding)) {
+      Str.resize(strlen(Str.c_str()));
+      return Str.c_str();
+    }
+  }
+
+  return nullptr;
+}
+
+/// RAII wrapper around CFBundleRef.
+class CFBundle : public CFReleaser<CFBundleRef> {
+public:
+  CFBundle(const char *Path = nullptr) : CFReleaser<CFBundleRef>() {
+    if (Path && Path[0])
+      SetFromPath(Path);
+  }
+
+  CFBundle(CFURLRef url)
+      : CFReleaser<CFBundleRef>(url ? ::CFBundleCreate(nullptr, url)
+                                    : nullptr) {}
+
+  /// Return the bundle identifier.
+  CFStringRef GetIdentifier() const {
+    if (CFBundleRef bundle = get())
+      return ::CFBundleGetIdentifier(bundle);
+    return nullptr;
+  }
+
+  /// Return value for key.
+  CFTypeRef GetValueForInfoDictionaryKey(CFStringRef key) const {
+    if (CFBundleRef bundle = get())
+      return ::CFBundleGetValueForInfoDictionaryKey(bundle, key);
+    return nullptr;
+  }
+
+private:
+  /// Update this instance with a new bundle created from the given path.
+  bool SetFromPath(const char *Path);
+};
+
+bool CFBundle::SetFromPath(const char *InPath) {
+  // Release our old bundle and URL.
+  reset();
+
+  if (InPath && InPath[0]) {
+    char ResolvedPath[PATH_MAX];
+    const char *Path = ::realpath(InPath, ResolvedPath);
+    if (Path == nullptr)
+      Path = InPath;
+
+    CFAllocatorRef Allocator = kCFAllocatorDefault;
+    // Make our Bundle URL.
+    CFReleaser<CFURLRef> BundleURL(::CFURLCreateFromFileSystemRepresentation(
+        Allocator, (const UInt8 *)Path, strlen(Path), false));
+    if (BundleURL.get()) {
+      CFIndex LastLength = LONG_MAX;
+
+      while (BundleURL.get() != nullptr) {
+        // Check the Path range and make sure we didn't make it to just "/",
+        // ".", or "..".
+        CFRange rangeIncludingSeparators;
+        CFRange range = ::CFURLGetByteRangeForComponent(
+            BundleURL.get(), kCFURLComponentPath, &rangeIncludingSeparators);
+        if (range.length > LastLength)
+          break;
+
+        reset(::CFBundleCreate(Allocator, BundleURL.get()));
+        if (get() != nullptr) {
+          if (GetIdentifier() != nullptr)
+            break;
+          reset();
+        }
+        BundleURL.reset(::CFURLCreateCopyDeletingLastPathComponent(
+            Allocator, BundleURL.get()));
+
+        LastLength = range.length;
+      }
+    }
+  }
+
+  return get() != nullptr;
+}
+
+#endif
+
+/// On Darwin, try and find the original executable's Info.plist information
+/// using CoreFoundation calls by creating a URL for the executable and
+/// chopping off the last Path component. The CFBundle can then get the
+/// identifier and grab any needed information from it directly. Return default
+/// CFBundleInfo on other platforms.
+CFBundleInfo getBundleInfo(StringRef ExePath) {
+  CFBundleInfo BundleInfo;
+
+#ifdef __APPLE__
+  if (ExePath.empty() || !sys::fs::exists(ExePath))
+    return BundleInfo;
+
+  auto PrintError = [&](CFTypeID TypeID) {
+    CFString TypeIDCFStr(::CFCopyTypeIDDescription(TypeID));
+    std::string TypeIDStr;
+    errs() << "The Info.plist key \"CFBundleShortVersionString\" is"
+           << "a " << TypeIDCFStr.UTF8(TypeIDStr)
+           << ", but it should be a string in: " << ExePath << ".\n";
+  };
+
+  CFBundle Bundle(ExePath.data());
+  if (CFStringRef BundleID = Bundle.GetIdentifier()) {
+    CFString::UTF8(BundleID, BundleInfo.IDStr);
+    if (CFTypeRef TypeRef =
+            Bundle.GetValueForInfoDictionaryKey(CFSTR("CFBundleVersion"))) {
+      CFTypeID TypeID = ::CFGetTypeID(TypeRef);
+      if (TypeID == ::CFStringGetTypeID())
+        CFString::UTF8((CFStringRef)TypeRef, BundleInfo.VersionStr);
+      else
+        PrintError(TypeID);
+    }
+    if (CFTypeRef TypeRef = Bundle.GetValueForInfoDictionaryKey(
+            CFSTR("CFBundleShortVersionString"))) {
+      CFTypeID TypeID = ::CFGetTypeID(TypeRef);
+      if (TypeID == ::CFStringGetTypeID())
+        CFString::UTF8((CFStringRef)TypeRef, BundleInfo.ShortVersionStr);
+      else
+        PrintError(TypeID);
+    }
+  }
+#endif
+
+  return BundleInfo;
+}
+
+} // end namespace dsymutil
+} // end namespace llvm

llvm/trunk/tools/dsymutil/CMakeLists.txt

 set(LLVM_LINK_COMPONENTS
   ${LLVM_TARGETS_TO_BUILD}
   AsmPrinter
   DebugInfoDWARF
   MC
   Object
   Support
   Target
   )
 
 add_llvm_tool(llvm-dsymutil
   dsymutil.cpp
   BinaryHolder.cpp
+  CFBundle.cpp
   DebugMap.cpp
   DwarfLinker.cpp
   MachODebugMapParser.cpp
   MachOUtils.cpp
 
   DEPENDS
   intrinsics_gen
   )
 
+IF(APPLE)
+  target_link_libraries(llvm-dsymutil "-framework CoreFoundation")
+ENDIF(APPLE)

llvm/trunk/tools/dsymutil/dsymutil.cpp

 //===- dsymutil.cpp - Debug info dumping utility for llvm -----------------===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 //
 // This program is a utility that aims to be a dropin replacement for
 // Darwin's dsymutil.
 //
 //===----------------------------------------------------------------------===//
 
 #include "dsymutil.h"
+#include "CFBundle.h"
 #include "DebugMap.h"
 #include "MachOUtils.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/Triple.h"
 #include "llvm/Object/MachO.h"
 #include "llvm/Support/CommandLine.h"
static opt<bool> DumpDebugMap(
     desc("Parse and dump the debug map to standard output. Not DWARF link "
          "will take place."),
     init(false), cat(DsymCategory));
 
 static opt<bool> InputIsYAMLDebugMap(
     "y", desc("Treat the input file is a YAML debug map rather than a binary."),
     init(false), cat(DsymCategory));
 
-static bool createPlistFile(llvm::StringRef BundleRoot) {
+static bool createPlistFile(llvm::StringRef Bin, llvm::StringRef BundleRoot) {
   if (NoOutput)
     return true;
 
   // Create plist file to write to.
   llvm::SmallString<128> InfoPlist(BundleRoot);
   llvm::sys::path::append(InfoPlist, "Contents/Info.plist");
   std::error_code EC;
   llvm::raw_fd_ostream PL(InfoPlist, EC, llvm::sys::fs::F_Text);
   if (EC) {
     llvm::errs() << "error: cannot create plist file " << InfoPlist << ": "
                  << EC.message() << '\n';
     return false;
   }
 
-  // FIXME: Use CoreFoundation to get executable bundle info. Use
-  // dummy values for now.
-  std::string bundleVersionStr = "1", bundleShortVersionStr = "1.0",
-              bundleIDStr;
+  CFBundleInfo BI = getBundleInfo(Bin);
 
+  if (BI.IDStr.empty()) {
     llvm::StringRef BundleID = *llvm::sys::path::rbegin(BundleRoot);
     if (llvm::sys::path::extension(BundleRoot) == ".dSYM")
-    bundleIDStr = llvm::sys::path::stem(BundleID);
+      BI.IDStr = llvm::sys::path::stem(BundleID);
     else
-    bundleIDStr = BundleID;
+      BI.IDStr = BundleID;
+  }
 
   // Print out information to the plist file.
   PL << "<?xml version=\"1.0\" encoding=\"UTF-8\"\?>\n"
      << "<!DOCTYPE plist PUBLIC \"-//Apple Computer//DTD PLIST 1.0//EN\" "
      << "\"http://www.apple.com/DTDs/PropertyList-1.0.dtd\">\n"
      << "<plist version=\"1.0\">\n"
      << "\t<dict>\n"
      << "\t\t<key>CFBundleDevelopmentRegion</key>\n"
      << "\t\t<string>English</string>\n"
      << "\t\t<key>CFBundleIdentifier</key>\n"
-     << "\t\t<string>com.apple.xcode.dsym." << bundleIDStr << "</string>\n"
+     << "\t\t<string>com.apple.xcode.dsym." << BI.IDStr << "</string>\n"
      << "\t\t<key>CFBundleInfoDictionaryVersion</key>\n"
      << "\t\t<string>6.0</string>\n"
      << "\t\t<key>CFBundlePackageType</key>\n"
      << "\t\t<string>dSYM</string>\n"
      << "\t\t<key>CFBundleSignature</key>\n"
-     << "\t\t<string>\?\?\?\?</string>\n"
-     << "\t\t<key>CFBundleShortVersionString</key>\n"
-     << "\t\t<string>" << bundleShortVersionStr << "</string>\n"
-     << "\t\t<key>CFBundleVersion</key>\n"
-     << "\t\t<string>" << bundleVersionStr << "</string>\n"
+     << "\t\t<string>\?\?\?\?</string>\n";
+
+  if (!BI.OmitShortVersion())
+    PL << "\t\t<key>CFBundleShortVersionString</key>\n"
+       << "\t\t<string>" << BI.ShortVersionStr << "</string>\n";
+
+  PL << "\t\t<key>CFBundleVersion</key>\n"
+     << "\t\t<string>" << BI.VersionStr << "</string>\n"
      << "\t</dict>\n"
      << "</plist>\n";
 
   PL.close();
   return true;
 }
 
 static bool createBundleDir(llvm::StringRef BundleBase) {
static std::string getOutputFileName(llvm::StringRef InputFile) {
   //          Resources/
   //             DWARF/
   //                <DWARF file(s)>
   std::string DwarfFile =
       InputFile == "-" ? llvm::StringRef("a.out") : InputFile;
   llvm::SmallString<128> BundleDir(OutputFileOpt);
   if (BundleDir.empty())
     BundleDir = DwarfFile + ".dSYM";
-  if (!createBundleDir(BundleDir) || !createPlistFile(BundleDir))
+  if (!createBundleDir(BundleDir) || !createPlistFile(DwarfFile, BundleDir))
     return "";
 
   llvm::sys::path::append(BundleDir, "Contents", "Resources", "DWARF",
                           llvm::sys::path::filename(DwarfFile));
   return BundleDir.str();
 }
 
 static Expected<sys::fs::TempFile> createTempFile() {