[Lldb-commits] [lldb] 75cfd38 - Revert "[lldb/Reproducers] Always collect the whole dSYM in the reproducer"
Davide Italiano via lldb-commits
lldb-commits at lists.llvm.org
Tue Mar 31 10:56:10 PDT 2020
Author: Davide Italiano
Date: 2020-03-31T10:56:02-07:00
New Revision: 75cfd382201978615cca1c91c2d9f14f8b7af56d
URL: https://github.com/llvm/llvm-project/commit/75cfd382201978615cca1c91c2d9f14f8b7af56d
DIFF: https://github.com/llvm/llvm-project/commit/75cfd382201978615cca1c91c2d9f14f8b7af56d.diff
LOG: Revert "[lldb/Reproducers] Always collect the whole dSYM in the reproducer"
This reverts commit 38ddb49e5242920e44a982cff7bbe2e86bd23a69 as it
breaks the macOS bots.
Added:
Modified:
lldb/include/lldb/Utility/Reproducer.h
lldb/source/Plugins/SymbolVendor/MacOSX/SymbolVendorMacOSX.cpp
lldb/source/Utility/Reproducer.cpp
Removed:
lldb/test/Shell/Reproducer/TestDSYM.test
################################################################################
diff --git a/lldb/include/lldb/Utility/Reproducer.h b/lldb/include/lldb/Utility/Reproducer.h
index b8b897d02ea2..bffb0f7c0647 100644
--- a/lldb/include/lldb/Utility/Reproducer.h
+++ b/lldb/include/lldb/Utility/Reproducer.h
@@ -98,8 +98,6 @@ 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 e819c342c6ec..2b67fee70617 100644
--- a/lldb/source/Plugins/SymbolVendor/MacOSX/SymbolVendorMacOSX.cpp
+++ b/lldb/source/Plugins/SymbolVendor/MacOSX/SymbolVendorMacOSX.cpp
@@ -20,7 +20,6 @@
#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"
@@ -146,11 +145,6 @@ 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 =
@@ -160,132 +154,136 @@ 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()) {
- if (module_sp->GetSourceMappingList().IsEmpty()) {
+ char dsym_path[PATH_MAX];
+ if (module_sp->GetSourceMappingList().IsEmpty() &&
+ dsym_fspec.GetPath(dsym_path, sizeof(dsym_path))) {
lldb_private::UUID dsym_uuid = dsym_objfile_sp->GetUUID();
if (dsym_uuid) {
std::string uuid_str = dsym_uuid.GetAsString();
- 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;
+ 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;
+ }
}
}
- }
- 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();
+ 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(
- ConstString(build_path.GetPath().c_str()),
- ConstString(source_path.GetPath().c_str()),
- true);
+ 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);
+ }
}
- }
- 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();
+ // 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);
}
- module_sp->GetSourceMappingList().Append(
- ConstString(DBGBuildSourcePath),
- ConstString(DBGSourcePath), true);
}
}
}
@@ -295,11 +293,6 @@ 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;
}
}
diff --git a/lldb/source/Utility/Reproducer.cpp b/lldb/source/Utility/Reproducer.cpp
index bbf399e599bf..8daa3216f7b1 100644
--- a/lldb/source/Utility/Reproducer.cpp
+++ b/lldb/source/Utility/Reproducer.cpp
@@ -321,11 +321,6 @@ 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
deleted file mode 100644
index bddacda3242f..000000000000
--- a/lldb/test/Shell/Reproducer/TestDSYM.test
+++ /dev/null
@@ -1,11 +0,0 @@
-# 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