[Lldb-commits] [lldb] 015c6cd - Re-land "[lldb/Reproducers] Always collect the whole dSYM in the reproducer"
Jonas Devlieghere via lldb-commits
lldb-commits at lists.llvm.org
Tue Mar 31 12:47:19 PDT 2020
Author: Jonas Devlieghere
Date: 2020-03-31T12:47:12-07:00
New Revision: 015c6cd47557272bb8b92fbf9f5bd2bcb8fa8989
URL: https://github.com/llvm/llvm-project/commit/015c6cd47557272bb8b92fbf9f5bd2bcb8fa8989
DIFF: https://github.com/llvm/llvm-project/commit/015c6cd47557272bb8b92fbf9f5bd2bcb8fa8989.diff
LOG: Re-land "[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
Added:
lldb/test/Shell/Reproducer/TestDSYM.test
Modified:
lldb/include/lldb/Utility/Reproducer.h
lldb/source/Plugins/SymbolVendor/MacOSX/SymbolVendorMacOSX.cpp
lldb/source/Utility/Reproducer.cpp
Removed:
################################################################################
diff --git a/lldb/include/lldb/Utility/Reproducer.h b/lldb/include/lldb/Utility/Reproducer.h
index bffb0f7c0647..b8b897d02ea2 100644
--- a/lldb/include/lldb/Utility/Reproducer.h
+++ b/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.
diff --git a/lldb/source/Plugins/SymbolVendor/MacOSX/SymbolVendorMacOSX.cpp b/lldb/source/Plugins/SymbolVendor/MacOSX/SymbolVendorMacOSX.cpp
index 2b67fee70617..fc426d1c9c37 100644
--- a/lldb/source/Plugins/SymbolVendor/MacOSX/SymbolVendorMacOSX.cpp
+++ b/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"
@@ -145,6 +146,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 =
@@ -154,136 +160,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 =
- 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;
- }
+ 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 =
- 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();
- }
+ 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);
}
}
}
@@ -293,6 +295,13 @@ SymbolVendorMacOSX::CreateInstance(const lldb::ModuleSP &module_sp,
}
symbol_vendor->AddSymbolFileRepresentation(dsym_objfile_sp);
+ if (!dsym_root.empty()) {
+ if (repro::Generator *g =
+ repro::Reproducer::Instance().GetGenerator()) {
+ repro::FileProvider &fp = g->GetOrCreate<repro::FileProvider>();
+ fp.recordInterestingDirectory(dsym_root);
+ }
+ }
return symbol_vendor;
}
}
diff --git a/lldb/source/Utility/Reproducer.cpp b/lldb/source/Utility/Reproducer.cpp
index 8daa3216f7b1..bbf399e599bf 100644
--- a/lldb/source/Utility/Reproducer.cpp
+++ b/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;
diff --git a/lldb/test/Shell/Reproducer/TestDSYM.test b/lldb/test/Shell/Reproducer/TestDSYM.test
new file mode 100644
index 000000000000..bddacda3242f
--- /dev/null
+++ b/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
More information about the lldb-commits
mailing list