[lldb/Reproducers] Always collect the whole dSYM in the reproducer

The FileCollector in LLDB collects every files that's used during a
debug session when capture is enabled. This ensures that the reproducer
only contains the files necessary to reproduce. This approach is not a
good fit for the dSYM bundle, which is a directory on disk, but should
be treated as a single unit.

On macOS LLDB have automatically find the matching dSYM for a binary by
its UUID. Having a incomplete dSYM in a reproducer can break debugging
even when reproducers are disabled.

This patch adds a was to specify a directory of interest to the
reproducers. It is called from SymbolVendorMacOSX with the path of the
dSYMs used by LLDB.

Differential revision: https://reviews.llvm.org/D76672

(cherry picked from commit 38ddb49)

lldb/include/lldb/Utility/Reproducer.h

@@ -98,6 +98,8 @@ class FileProvider : public Provider<FileProvider> {
     return m_collector;
   }
 
+  void recordInterestingDirectory(const llvm::Twine &dir);
+
   void Keep() override {
     auto mapping = GetRoot().CopyByAppendingPathComponent(Info::file);
     // Temporary files that are removed during execution can cause copy errors.

lldb/source/Plugins/SymbolVendor/MacOSX/SymbolVendorMacOSX.cpp

@@ -20,6 +20,7 @@
 #include "lldb/Symbol/LocateSymbolFile.h"
 #include "lldb/Symbol/ObjectFile.h"
 #include "lldb/Target/Target.h"
+#include "lldb/Utility/Reproducer.h"
 #include "lldb/Utility/StreamString.h"
 #include "lldb/Utility/Timer.h"
 
@@ -143,6 +144,11 @@ SymbolVendorMacOSX::CreateInstance(const lldb::ModuleSP &module_sp,
     }
 
     if (dsym_fspec) {
+      // Compute dSYM root.
+      std::string dsym_root = dsym_fspec.GetPath();
+      const size_t pos = dsym_root.find("/Contents/Resources/");
+      dsym_root = pos != std::string::npos ? dsym_root.substr(0, pos) : "";
+
       DataBufferSP dsym_file_data_sp;
       lldb::offset_t dsym_file_data_offset = 0;
       dsym_objfile_sp =
@@ -152,136 +158,132 @@ SymbolVendorMacOSX::CreateInstance(const lldb::ModuleSP &module_sp,
       if (UUIDsMatch(module_sp.get(), dsym_objfile_sp.get(), feedback_strm)) {
         // We need a XML parser if we hope to parse a plist...
         if (XMLDocument::XMLEnabled()) {
-          char dsym_path[PATH_MAX];
-          if (module_sp->GetSourceMappingList().IsEmpty() &&
-              dsym_fspec.GetPath(dsym_path, sizeof(dsym_path))) {
+          if (module_sp->GetSourceMappingList().IsEmpty()) {
             lldb_private::UUID dsym_uuid = dsym_objfile_sp->GetUUID();
             if (dsym_uuid) {
               std::string uuid_str = dsym_uuid.GetAsString();
-              if (!uuid_str.empty()) {
-                char *resources = strstr(dsym_path, "/Contents/Resources/");
-                if (resources) {
-                  char dsym_uuid_plist_path[PATH_MAX];
-                  resources[strlen("/Contents/Resources/")] = '\0';
-                  snprintf(dsym_uuid_plist_path, sizeof(dsym_uuid_plist_path),
-                           "%s%s.plist", dsym_path, uuid_str.c_str());
-                  FileSpec dsym_uuid_plist_spec(dsym_uuid_plist_path);
-                  if (FileSystem::Instance().Exists(dsym_uuid_plist_spec)) {
-                    ApplePropertyList plist(dsym_uuid_plist_path);
-                    if (plist) {
-                      std::string DBGBuildSourcePath;
-                      std::string DBGSourcePath;
-
-                      // DBGSourcePathRemapping is a dictionary in the plist
-                      // with keys which are DBGBuildSourcePath file paths and
-                      // values which are DBGSourcePath file paths
-
-                      StructuredData::ObjectSP plist_sp =
-                          plist.GetStructuredData();
-                      if (plist_sp.get() && plist_sp->GetAsDictionary() &&
-                          plist_sp->GetAsDictionary()->HasKey(
-                              "DBGSourcePathRemapping") &&
-                          plist_sp->GetAsDictionary()
-                              ->GetValueForKey("DBGSourcePathRemapping")
-                              ->GetAsDictionary()) {
-
-                        // If DBGVersion 1 or DBGVersion missing, ignore DBGSourcePathRemapping.
-                        // If DBGVersion 2, strip last two components of path remappings from
-                        //                  entries to fix an issue with a specific set of
-                        //                  DBGSourcePathRemapping entries that lldb worked
-                        //                  with.
-                        // If DBGVersion 3, trust & use the source path remappings as-is.
-                        //
-
-                        bool new_style_source_remapping_dictionary = false;
-                        bool do_truncate_remapping_names = false;
-                        std::string original_DBGSourcePath_value =
-                            DBGSourcePath;
-                        if (plist_sp->GetAsDictionary()->HasKey("DBGVersion")) {
-                          std::string version_string =
-                              plist_sp->GetAsDictionary()
-                                  ->GetValueForKey("DBGVersion")
-                                  ->GetStringValue("");
-                          if (!version_string.empty() &&
-                              isdigit(version_string[0])) {
-                            int version_number = atoi(version_string.c_str());
-                            if (version_number > 1) {
-                              new_style_source_remapping_dictionary = true;
-                            }
-                            if (version_number == 2) {
-                                do_truncate_remapping_names = true;
-                            }
+              if (!uuid_str.empty() && !dsym_root.empty()) {
+                char dsym_uuid_plist_path[PATH_MAX];
+                snprintf(dsym_uuid_plist_path, sizeof(dsym_uuid_plist_path),
+                         "%s/Contents/Resources/%s.plist", dsym_root.c_str(),
+                         uuid_str.c_str());
+                FileSpec dsym_uuid_plist_spec(dsym_uuid_plist_path);
+                if (FileSystem::Instance().Exists(dsym_uuid_plist_spec)) {
+                  ApplePropertyList plist(dsym_uuid_plist_path);
+                  if (plist) {
+                    std::string DBGBuildSourcePath;
+                    std::string DBGSourcePath;
+
+                    // DBGSourcePathRemapping is a dictionary in the plist
+                    // with keys which are DBGBuildSourcePath file paths and
+                    // values which are DBGSourcePath file paths
+
+                    StructuredData::ObjectSP plist_sp =
+                        plist.GetStructuredData();
+                    if (plist_sp.get() && plist_sp->GetAsDictionary() &&
+                        plist_sp->GetAsDictionary()->HasKey(
+                            "DBGSourcePathRemapping") &&
+                        plist_sp->GetAsDictionary()
+                            ->GetValueForKey("DBGSourcePathRemapping")
+                            ->GetAsDictionary()) {
+
+                      // If DBGVersion 1 or DBGVersion missing, ignore
+                      // DBGSourcePathRemapping. If DBGVersion 2, strip last two
+                      // components of path remappings from
+                      //                  entries to fix an issue with a
+                      //                  specific set of DBGSourcePathRemapping
+                      //                  entries that lldb worked with.
+                      // If DBGVersion 3, trust & use the source path remappings
+                      // as-is.
+                      //
+
+                      bool new_style_source_remapping_dictionary = false;
+                      bool do_truncate_remapping_names = false;
+                      std::string original_DBGSourcePath_value = DBGSourcePath;
+                      if (plist_sp->GetAsDictionary()->HasKey("DBGVersion")) {
+                        std::string version_string =
+                            std::string(plist_sp->GetAsDictionary()
+                                            ->GetValueForKey("DBGVersion")
+                                            ->GetStringValue(""));
+                        if (!version_string.empty() &&
+                            isdigit(version_string[0])) {
+                          int version_number = atoi(version_string.c_str());
+                          if (version_number > 1) {
+                            new_style_source_remapping_dictionary = true;
+                          }
+                          if (version_number == 2) {
+                            do_truncate_remapping_names = true;
                           }
                         }
+                      }
 
-                        StructuredData::Dictionary *remappings_dict =
-                            plist_sp->GetAsDictionary()
-                                ->GetValueForKey("DBGSourcePathRemapping")
-                                ->GetAsDictionary();
-                        remappings_dict->ForEach(
-                            [&module_sp, new_style_source_remapping_dictionary,
-                             original_DBGSourcePath_value, do_truncate_remapping_names](
-                                ConstString key,
-                                StructuredData::Object *object) -> bool {
-                              if (object && object->GetAsString()) {
-
-                                // key is DBGBuildSourcePath
-                                // object is DBGSourcePath
-                                std::string DBGSourcePath =
-                                    object->GetStringValue();
-                                if (!new_style_source_remapping_dictionary &&
-                                    !original_DBGSourcePath_value.empty()) {
-                                  DBGSourcePath = original_DBGSourcePath_value;
-                                }
-                                if (DBGSourcePath[0] == '~') {
-                                  FileSpec resolved_source_path(
-                                      DBGSourcePath.c_str());
-                                  FileSystem::Instance().Resolve(
-                                      resolved_source_path);
-                                  DBGSourcePath =
-                                      resolved_source_path.GetPath();
-                                }
+                      StructuredData::Dictionary *remappings_dict =
+                          plist_sp->GetAsDictionary()
+                              ->GetValueForKey("DBGSourcePathRemapping")
+                              ->GetAsDictionary();
+                      remappings_dict->ForEach(
+                          [&module_sp, new_style_source_remapping_dictionary,
+                           original_DBGSourcePath_value,
+                           do_truncate_remapping_names](
+                              ConstString key,
+                              StructuredData::Object *object) -> bool {
+                            if (object && object->GetAsString()) {
+
+                              // key is DBGBuildSourcePath
+                              // object is DBGSourcePath
+                              std::string DBGSourcePath =
+                                  std::string(object->GetStringValue());
+                              if (!new_style_source_remapping_dictionary &&
+                                  !original_DBGSourcePath_value.empty()) {
+                                DBGSourcePath = original_DBGSourcePath_value;
+                              }
+                              if (DBGSourcePath[0] == '~') {
+                                FileSpec resolved_source_path(
+                                    DBGSourcePath.c_str());
+                                FileSystem::Instance().Resolve(
+                                    resolved_source_path);
+                                DBGSourcePath = resolved_source_path.GetPath();
+                              }
+                              module_sp->GetSourceMappingList().Append(
+                                  key, ConstString(DBGSourcePath), true);
+                              // With version 2 of DBGSourcePathRemapping, we
+                              // can chop off the last two filename parts
+                              // from the source remapping and get a more
+                              // general source remapping that still works.
+                              // Add this as another option in addition to
+                              // the full source path remap.
+                              if (do_truncate_remapping_names) {
+                                FileSpec build_path(key.AsCString());
+                                FileSpec source_path(DBGSourcePath.c_str());
+                                build_path.RemoveLastPathComponent();
+                                build_path.RemoveLastPathComponent();
+                                source_path.RemoveLastPathComponent();
+                                source_path.RemoveLastPathComponent();
                                 module_sp->GetSourceMappingList().Append(
-                                    key, ConstString(DBGSourcePath), true);
-                                // With version 2 of DBGSourcePathRemapping, we
-                                // can chop off the last two filename parts
-                                // from the source remapping and get a more
-                                // general source remapping that still works.
-                                // Add this as another option in addition to
-                                // the full source path remap.
-                                if (do_truncate_remapping_names) {
-                                  FileSpec build_path(key.AsCString());
-                                  FileSpec source_path(DBGSourcePath.c_str());
-                                  build_path.RemoveLastPathComponent();
-                                  build_path.RemoveLastPathComponent();
-                                  source_path.RemoveLastPathComponent();
-                                  source_path.RemoveLastPathComponent();
-                                  module_sp->GetSourceMappingList().Append(
-                                      ConstString(build_path.GetPath().c_str()),
-                                      ConstString(source_path.GetPath().c_str()), true);
-                                }
+                                    ConstString(build_path.GetPath().c_str()),
+                                    ConstString(source_path.GetPath().c_str()),
+                                    true);
                               }
-                              return true;
-                            });
-                      }
+                            }
+                            return true;
+                          });
+                    }
 
-                      // If we have a DBGBuildSourcePath + DBGSourcePath pair,
-                      // append those to the source path remappings.
-
-                      plist.GetValueAsString("DBGBuildSourcePath",
-                                             DBGBuildSourcePath);
-                      plist.GetValueAsString("DBGSourcePath", DBGSourcePath);
-                      if (!DBGBuildSourcePath.empty() &&
-                          !DBGSourcePath.empty()) {
-                        if (DBGSourcePath[0] == '~') {
-                          FileSpec resolved_source_path(DBGSourcePath.c_str());
-                          FileSystem::Instance().Resolve(resolved_source_path);
-                          DBGSourcePath = resolved_source_path.GetPath();
-                        }
-                        module_sp->GetSourceMappingList().Append(
-                            ConstString(DBGBuildSourcePath),
-                            ConstString(DBGSourcePath), true);
+                    // If we have a DBGBuildSourcePath + DBGSourcePath pair,
+                    // append those to the source path remappings.
+
+                    plist.GetValueAsString("DBGBuildSourcePath",
+                                           DBGBuildSourcePath);
+                    plist.GetValueAsString("DBGSourcePath", DBGSourcePath);
+                    if (!DBGBuildSourcePath.empty() && !DBGSourcePath.empty()) {
+                      if (DBGSourcePath[0] == '~') {
+                        FileSpec resolved_source_path(DBGSourcePath.c_str());
+                        FileSystem::Instance().Resolve(resolved_source_path);
+                        DBGSourcePath = resolved_source_path.GetPath();
                       }
+                      module_sp->GetSourceMappingList().Append(
+                          ConstString(DBGBuildSourcePath),
+                          ConstString(DBGSourcePath), true);
                     }
                   }
                 }
@@ -291,6 +293,11 @@ SymbolVendorMacOSX::CreateInstance(const lldb::ModuleSP &module_sp,
         }
 
         symbol_vendor->AddSymbolFileRepresentation(dsym_objfile_sp);
+        if (repro::Generator *g =
+                repro::Reproducer::Instance().GetGenerator()) {
+          repro::FileProvider &fp = g->GetOrCreate<repro::FileProvider>();
+          fp.recordInterestingDirectory(dsym_root);
+        }
         return symbol_vendor;
       }
     }

lldb/source/Utility/Reproducer.cpp

@@ -321,6 +321,11 @@ void WorkingDirectoryProvider::Keep() {
   os << m_cwd << "\n";
 }
 
+void FileProvider::recordInterestingDirectory(const llvm::Twine &dir) {
+  if (m_collector)
+    m_collector->addDirectory(dir);
+}
+
 void ProviderBase::anchor() {}
 char CommandProvider::ID = 0;
 char FileProvider::ID = 0;

lldb/test/Shell/Reproducer/TestDSYM.test

@@ -0,0 +1,11 @@
+# REQUIRES: system-darwin
+# Ensure that the reproducers captures the whole dSYM bundle.
+
+# RUN: rm -rf %t.repro
+# RUN: %clang_host %S/Inputs/simple.c -g -o %t.out
+# RUN: touch %t.out.dSYM/foo.bar
+
+# RUN: %lldb -x -b --capture --capture-path %t.repro %t.out -o 'b main' -o 'run' -o 'reproducer generate'
+
+# RUN: %lldb -b -o 'reproducer dump -p files -f %t.repro' | FileCheck %s --check-prefix FILES
+# FILES: foo.bar