[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

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

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

tools/dsymutil/CFBundle.h

1

Missing license?

[aprantl]

tools/dsymutil/CFBundle.cpp

119

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

tools/dsymutil/CFBundle.h

5

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

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 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

55

comment what the class is for?

65

comment what this function is doing?

77

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

87

move into declaration?

100

comment what this function is doing?

183

perhaps move into declaration?

189

move into declaration?

194

move into declaration?

239

move into declaration?

245

move into declaration?

267

should this be the doxygen comment for the function?

tools/dsymutil/CFBundle.h

5

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

[JDevlieghere]

• Feedback Adrian
• Remove dead code

[vsk]

Thanks, lgtm!

tools/dsymutil/CFBundle.h

+
+#include "llvm/ADT/StringRef.h"
+#include <string>
+
+void getBundleInfo(llvm::StringRef ExePath, std::string &bundleVersionStr,
+                   std::string &bundleShortVersionStr, std::string &bundleIDStr,
+                   bool &OmitShortVersion);

tools/dsymutil/CFBundle.cpp

+
+#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>
+
+/// Templatized CF helper class that can own any CF pointer and will call
+/// CFRelease() on any valid pointer it owns unless that pointer is explicitly
+/// released using the release() member function.
+///
+/// This class is designed to mimic the std::unique_ptr<T> class and has a
+/// similar interface. The one thing to watch out for is the
+/// CFReleaser<T>::release() function won't actually CFRelease any owned
+/// pointer, it is designed to relinquish ownership of the pointer just like
+/// std:unique_ptr<T>::release() does.
+template <class T> class CFReleaser {
+public:
+  /// Constructor that takes a pointer to a CF object that is to be released
+  /// when this object goes out of scope.
+  CFReleaser(T ptr = nullptr) : _ptr(ptr) {}
+
+  /// The copy constructor will retain the pointer contained in the RHS if it
+  /// contains a valid pointer value.
+  CFReleaser(const CFReleaser &rhs) : _ptr(rhs.get()) {
+    if (get())
+      ::CFRetain(get());
+  }
+
+  /// The destructor will release the pointer that it contains if it has a
+  /// valid pointer.
+  virtual ~CFReleaser() { reset(); }
+
+  CFReleaser &operator=(const CFReleaser<T> &rhs) {
+    if (this != &rhs) {
+      // Replace our owned pointer with the new one.
+      reset(rhs.get());
+      // Retain the current pointer that we own.
+      if (get())
+        ::CFRetain(get());
+    }
+    return *this;
+  }
+
+  /// Get the address of the contained type in case it needs to be passed to a
+  /// function that will fill in a pointer value. The function currently will
+  /// assert if _ptr is not nullptr because the only time this method should be
+  /// used is if another function will modify the contents, and we could leak a
+  /// pointer if this is not nullptr. If the assertion fires, check the
+  /// offending code, or call reset() prior to using the "ptr_address()" member
+  /// to make sure any owned objects has CFRelease called on it.
+  T *ptr_address() {
+    assert(_ptr == nullptr);
+    return &_ptr;
+  }
+
+  T get() { return _ptr; }
+
+  const T get() const { return _ptr; }
+
+  /// Set a new value for the pointer and CFRelease our old value if we had a
+  /// valid one.
+  void reset(T ptr = nullptr) {
+    if ((_ptr != nullptr) && (ptr != _ptr))
+      ::CFRelease(_ptr);
+    _ptr = ptr;
+  }
+
+  /// Release ownership without calling CFRelease. This class is designed to
+  /// mimic std::unique_ptr<T>, so the release method releases ownership of the
+  /// contained pointer without calling CFRelease.
+  T release() {
+    T tmp = _ptr;
+    _ptr = nullptr;
+    return tmp;
+  }
+
+private:
+  T _ptr;
+};
+
+class CFString : public CFReleaser<CFStringRef> {
+public:
+  CFString(CFStringRef cf_str = nullptr);
+  CFString(const char *s, CFStringEncoding encoding);
+  CFString(const CFString &rhs);
+  CFString &operator=(const CFString &rhs);
+  virtual ~CFString();
+
+  const char *GetFileSystemRepresentation(std::string &str);
+  CFStringRef SetFileSystemRepresentation(const char *path);
+  CFStringRef SetFileSystemRepresentationFromCFType(CFTypeRef cf_type);
+  CFStringRef SetFileSystemRepresentationAndExpandTilde(const char *path);
+  const char *UTF8(std::string &str);
+  CFIndex GetLength() const;
+  static const char *UTF8(CFStringRef cf_str, std::string &str);
+  static const char *FileSystemRepresentation(CFStringRef cf_str,
+                                              std::string &str);
+};
+
+class CFBundle : public CFReleaser<CFBundleRef> {
+public:
+  CFBundle(const char *path = nullptr);
+  CFBundle(const CFBundle &rhs);
+  CFBundle(CFURLRef url);
+  CFBundle &operator=(const CFBundle &rhs);
+  virtual ~CFBundle();
+
+  bool SetPath(const char *path);
+  CFStringRef GetIdentifier() const;
+  CFTypeRef GetValueForInfoDictionaryKey(CFStringRef key) const;
+};
+
+static const char *CreatePathByExpandingTildePath(const char *path,
+                                                  std::string &expanded_path) {
+  glob_t globbuf;
+  if (glob(path, GLOB_TILDE, nullptr, &globbuf) == 0) {
+    expanded_path = globbuf.gl_pathv[0];
+    globfree(&globbuf);
+  } else
+    expanded_path.clear();
+
+  return expanded_path.c_str();
+}
+
+CFString::CFString(CFStringRef s) : CFReleaser<CFStringRef>(s) {}
+
+CFString::CFString(const CFString &rhs) : CFReleaser<CFStringRef>(rhs) {}
+
+CFString &CFString::operator=(const CFString &rhs) {
+  if (this != &rhs)
+    *this = rhs;
+  return *this;
+}
+
+CFString::CFString(const char *cstr, CFStringEncoding cstr_encoding)
+    : CFReleaser<CFStringRef>() {
+  if (cstr && cstr[0]) {
+    reset(
+        ::CFStringCreateWithCString(kCFAllocatorDefault, cstr, cstr_encoding));
+  }
+}
+
+CFString::~CFString() {}
+
+const char *CFString::GetFileSystemRepresentation(std::string &s) {
+  return CFString::FileSystemRepresentation(get(), s);
+}
+
+CFStringRef CFString::SetFileSystemRepresentation(const char *path) {
+  CFStringRef new_value = nullptr;
+  if (path && path[0])
+    new_value =
+        ::CFStringCreateWithFileSystemRepresentation(kCFAllocatorDefault, path);
+  reset(new_value);
+  return get();
+}
+
+CFStringRef CFString::SetFileSystemRepresentationFromCFType(CFTypeRef cf_type) {
+  CFStringRef new_value = nullptr;
+  if (cf_type != nullptr) {
+    CFTypeID cf_type_id = ::CFGetTypeID(cf_type);
+
+    if (cf_type_id == ::CFStringGetTypeID()) {
+      // Retain since we are using the existing object
+      new_value = (CFStringRef)::CFRetain(cf_type);
+    } else if (cf_type_id == ::CFURLGetTypeID()) {
+      new_value =
+          ::CFURLCopyFileSystemPath((CFURLRef)cf_type, kCFURLPOSIXPathStyle);
+    }
+  }
+  reset(new_value);
+  return get();
+}
+
+CFStringRef
+CFString::SetFileSystemRepresentationAndExpandTilde(const char *path) {
+  std::string expanded_path;
+  if (CreatePathByExpandingTildePath(path, expanded_path))
+    SetFileSystemRepresentation(expanded_path.c_str());
+  else
+    reset();
+  return get();
+}
+
+const char *CFString::UTF8(std::string &str) {
+  return CFString::UTF8(get(), str);
+}
+
+/// Static function that puts a copy of the UTF8 contents of CF_STR into STR
+/// and returns the C string pointer that is contained in STR when successful,
+/// else nullptr is returned. 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 cf_str, std::string &str) {
+  if (cf_str) {
+    const CFStringEncoding encoding = kCFStringEncodingUTF8;
+    CFIndex max_utf8_str_len = CFStringGetLength(cf_str);
+    max_utf8_str_len =
+        CFStringGetMaximumSizeForEncoding(max_utf8_str_len, encoding);
+    if (max_utf8_str_len > 0) {
+      str.resize(max_utf8_str_len);
+      if (!str.empty()) {
+        if (CFStringGetCString(cf_str, &str[0], str.size(), encoding)) {
+          str.resize(strlen(str.c_str()));
+          return str.c_str();
+        }
+      }
+    }
+  }
+  return nullptr;
+}
+
+/// Static function that puts a copy of the file system representation of
+/// CF_STR into STR and returns the C string pointer that is contained in STR
+/// when successful, else nullptr is returned. 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::FileSystemRepresentation(CFStringRef cf_str,
+                                               std::string &str) {
+  if (cf_str) {
+    CFIndex max_length =
+        ::CFStringGetMaximumSizeOfFileSystemRepresentation(cf_str);
+    if (max_length > 0) {
+      str.resize(max_length);
+      if (!str.empty()) {
+        if (::CFStringGetFileSystemRepresentation(cf_str, &str[0],
+                                                  str.size())) {
+          str.erase(::strlen(str.c_str()));
+          return str.c_str();
+        }
+      }
+    }
+  }
+  str.erase();
+  return nullptr;
+}
+
+CFIndex CFString::GetLength() const {
+  CFStringRef str = get();
+  if (str)
+    return CFStringGetLength(str);
+  return 0;
+}
+
+CFBundle::CFBundle(const char *path) : CFReleaser<CFBundleRef>() {
+  if (path && path[0])
+    SetPath(path);
+}
+
+CFBundle::CFBundle(const CFBundle &rhs) : CFReleaser<CFBundleRef>(rhs) {}
+
+CFBundle::CFBundle(CFURLRef url)
+    : CFReleaser<CFBundleRef>(url ? CFBundleCreate(nullptr, url) : nullptr) {}
+
+CFBundle &CFBundle::operator=(const CFBundle &rhs) {
+  if (this != &rhs)
+    *this = rhs;
+  return *this;
+}
+
+CFBundle::~CFBundle() {}
+
+bool CFBundle::SetPath(const char *in_path) {
+  // Release our old bundle and ULR
+  reset(); // This class is a CFReleaser<CFBundleRef>
+
+  if (in_path && in_path[0]) {
+    char resolved_path[PATH_MAX];
+    const char *path = ::realpath(in_path, resolved_path);
+    if (path == nullptr)
+      path = in_path;
+
+    CFAllocatorRef alloc = kCFAllocatorDefault;
+    // Make our Bundle URL
+    CFReleaser<CFURLRef> bundle_url(::CFURLCreateFromFileSystemRepresentation(
+        alloc, (const UInt8 *)path, strlen(path), false));
+    if (bundle_url.get()) {
+      CFIndex last_length = LONG_MAX;
+
+      while (bundle_url.get() != nullptr) {
+        // Check the Path range and make sure we didn't make it to just
+        // "/", ".", or ".."
+        CFRange rangeIncludingSeparators;
+        CFRange range = ::CFURLGetByteRangeForComponent(
+            bundle_url.get(), kCFURLComponentPath, &rangeIncludingSeparators);
+        if (range.length > last_length)
+          break;
+
+        reset(::CFBundleCreate(alloc, bundle_url.get()));
+        if (get() != nullptr) {
+          if (GetIdentifier() != nullptr)
+            break;
+          reset();
+        }
+        bundle_url.reset(::CFURLCreateCopyDeletingLastPathComponent(
+            alloc, bundle_url.get()));
+
+        last_length = range.length;
+      }
+    }
+  }
+  return get() != nullptr;
+}
+
+CFStringRef CFBundle::GetIdentifier() const {
+  CFBundleRef bundle = get();
+  if (bundle != nullptr)
+    return ::CFBundleGetIdentifier(bundle);
+  return nullptr;
+}
+
+CFTypeRef CFBundle::GetValueForInfoDictionaryKey(CFStringRef key) const {
+  CFBundleRef bundle = get();
+  if (bundle != nullptr)
+    return ::CFBundleGetValueForInfoDictionaryKey(bundle, key);
+  return nullptr;
+}
+#endif
+
+void getBundleInfo(llvm::StringRef ExePath, std::string &bundleVersionStr,
+                   std::string &bundleShortVersionStr, std::string &bundleIDStr,
+                   bool &OmitShortVersion) {
+#ifdef __APPLE__
+  CFTypeRef cf = nullptr;
+  CFTypeID cf_type_id = 0;
+
+  // 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.
+  if (!ExePath.empty() && llvm::sys::fs::exists(ExePath)) {
+    const char *exe_path = ExePath.data();
+    CFBundle bundle(exe_path);
+    CFStringRef bundleID = bundle.GetIdentifier();
+    if (bundleID != nullptr) {
+      CFString::UTF8(bundleID, bundleIDStr);
+      cf = bundle.GetValueForInfoDictionaryKey(CFSTR("CFBundleVersion"));
+      if (cf != nullptr) {
+        cf_type_id = ::CFGetTypeID(cf);
+        if (cf_type_id == ::CFStringGetTypeID())
+          CFString::UTF8((CFStringRef)cf, bundleVersionStr);
+        else {
+          CFString cf_type_id_cfstr(::CFCopyTypeIDDescription(cf_type_id));
+          std::string cf_type_id_str;
+          llvm::errs() << "The Info.plist key \"CFBundleVersion\" is a "
+                       << cf_type_id_cfstr.UTF8(cf_type_id_str)
+                       << "but it should be a string in: " << exe_path << ".\n";
+        }
+      }
+      cf = bundle.GetValueForInfoDictionaryKey(
+          CFSTR("CFBundleShortVersionString"));
+      if (cf != nullptr) {
+        cf_type_id = ::CFGetTypeID(cf);
+        if (::CFGetTypeID(cf) == ::CFStringGetTypeID())
+          CFString::UTF8((CFStringRef)cf, bundleShortVersionStr);
+        else {
+          CFString cf_type_id_cfstr(::CFCopyTypeIDDescription(cf_type_id));
+          std::string cf_type_id_str;
+          llvm::errs() << "The Info.plist key \"CFBundleShortVersionString\" is"
+                       << "a " << cf_type_id_cfstr.UTF8(cf_type_id_str)
+                       << ", but it should be a string in: " << exe_path
+                       << ".\n";
+        }
+      }
+      OmitShortVersion = bundleShortVersionStr.empty();
+    }
+  }
+
+#endif
+  if (bundleVersionStr.empty())
+    bundleVersionStr = "1";
+
+  if (bundleShortVersionStr.empty() && !OmitShortVersion)
+    bundleShortVersionStr = "1.0";
+}

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)

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;
+  std::string BundleVersionStr, BundleShortVersionStr, BundleIDStr;
+  bool OmitShortVersion = false;
 
+  getBundleInfo(Bin, BundleVersionStr, BundleShortVersionStr, BundleIDStr,
+                OmitShortVersion);
+
+  if (BundleIDStr.empty()) {
     llvm::StringRef BundleID = *llvm::sys::path::rbegin(BundleRoot);
     if (llvm::sys::path::extension(BundleRoot) == ".dSYM")
-    bundleIDStr = llvm::sys::path::stem(BundleID);
+      BundleIDStr = llvm::sys::path::stem(BundleID);
     else
-    bundleIDStr = BundleID;
+      BundleIDStr = 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." << BundleIDStr << "</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 (!OmitShortVersion)
+    PL << "\t\t<key>CFBundleShortVersionString</key>\n"
+       << "\t\t<string>" << BundleShortVersionStr << "</string>\n";
+
+  PL << "\t\t<key>CFBundleVersion</key>\n"
+     << "\t\t<string>" << BundleVersionStr << "</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() {