Skip to content

Commit 015c6cd

Browse files
JDevlieghereJDevlieghere
committed
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
1 parent 11ccad6 commit 015c6cd

File tree

4 files changed

+147
-120
lines changed

4 files changed

+147
-120
lines changed

‎lldb/include/lldb/Utility/Reproducer.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,8 @@ class FileProvider : public Provider<FileProvider> {
9898
return m_collector;
9999
}
100100

101+
void recordInterestingDirectory(const llvm::Twine &dir);
102+
101103
void Keep() override {
102104
auto mapping = GetRoot().CopyByAppendingPathComponent(Info::file);
103105
// Temporary files that are removed during execution can cause copy errors.

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

Lines changed: 129 additions & 120 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
#include "lldb/Symbol/LocateSymbolFile.h"
2121
#include "lldb/Symbol/ObjectFile.h"
2222
#include "lldb/Target/Target.h"
23+
#include "lldb/Utility/Reproducer.h"
2324
#include "lldb/Utility/StreamString.h"
2425
#include "lldb/Utility/Timer.h"
2526

@@ -145,6 +146,11 @@ SymbolVendorMacOSX::CreateInstance(const lldb::ModuleSP &module_sp,
145146
}
146147

147148
if (dsym_fspec) {
149+
// Compute dSYM root.
150+
std::string dsym_root = dsym_fspec.GetPath();
151+
const size_t pos = dsym_root.find("/Contents/Resources/");
152+
dsym_root = pos != std::string::npos ? dsym_root.substr(0, pos) : "";
153+
148154
DataBufferSP dsym_file_data_sp;
149155
lldb::offset_t dsym_file_data_offset = 0;
150156
dsym_objfile_sp =
@@ -154,136 +160,132 @@ SymbolVendorMacOSX::CreateInstance(const lldb::ModuleSP &module_sp,
154160
if (UUIDsMatch(module_sp.get(), dsym_objfile_sp.get(), feedback_strm)) {
155161
// We need a XML parser if we hope to parse a plist...
156162
if (XMLDocument::XMLEnabled()) {
157-
char dsym_path[PATH_MAX];
158-
if (module_sp->GetSourceMappingList().IsEmpty() &&
159-
dsym_fspec.GetPath(dsym_path, sizeof(dsym_path))) {
163+
if (module_sp->GetSourceMappingList().IsEmpty()) {
160164
lldb_private::UUID dsym_uuid = dsym_objfile_sp->GetUUID();
161165
if (dsym_uuid) {
162166
std::string uuid_str = dsym_uuid.GetAsString();
163-
if (!uuid_str.empty()) {
164-
char *resources = strstr(dsym_path, "/Contents/Resources/");
165-
if (resources) {
166-
char dsym_uuid_plist_path[PATH_MAX];
167-
resources[strlen("/Contents/Resources/")] = '\0';
168-
snprintf(dsym_uuid_plist_path, sizeof(dsym_uuid_plist_path),
169-
"%s%s.plist", dsym_path, uuid_str.c_str());
170-
FileSpec dsym_uuid_plist_spec(dsym_uuid_plist_path);
171-
if (FileSystem::Instance().Exists(dsym_uuid_plist_spec)) {
172-
ApplePropertyList plist(dsym_uuid_plist_path);
173-
if (plist) {
174-
std::string DBGBuildSourcePath;
175-
std::string DBGSourcePath;
176-
177-
// DBGSourcePathRemapping is a dictionary in the plist
178-
// with keys which are DBGBuildSourcePath file paths and
179-
// values which are DBGSourcePath file paths
180-
181-
StructuredData::ObjectSP plist_sp =
182-
plist.GetStructuredData();
183-
if (plist_sp.get() && plist_sp->GetAsDictionary() &&
184-
plist_sp->GetAsDictionary()->HasKey(
185-
"DBGSourcePathRemapping") &&
186-
plist_sp->GetAsDictionary()
187-
->GetValueForKey("DBGSourcePathRemapping")
188-
->GetAsDictionary()) {
189-
190-
// If DBGVersion 1 or DBGVersion missing, ignore DBGSourcePathRemapping.
191-
// If DBGVersion 2, strip last two components of path remappings from
192-
// entries to fix an issue with a specific set of
193-
// DBGSourcePathRemapping entries that lldb worked
194-
// with.
195-
// If DBGVersion 3, trust & use the source path remappings as-is.
196-
//
197-
198-
bool new_style_source_remapping_dictionary = false;
199-
bool do_truncate_remapping_names = false;
200-
std::string original_DBGSourcePath_value =
201-
DBGSourcePath;
202-
if (plist_sp->GetAsDictionary()->HasKey("DBGVersion")) {
203-
std::string version_string =
204-
std::string(plist_sp->GetAsDictionary()
205-
->GetValueForKey("DBGVersion")
206-
->GetStringValue(""));
207-
if (!version_string.empty() &&
208-
isdigit(version_string[0])) {
209-
int version_number = atoi(version_string.c_str());
210-
if (version_number > 1) {
211-
new_style_source_remapping_dictionary = true;
212-
}
213-
if (version_number == 2) {
214-
do_truncate_remapping_names = true;
215-
}
167+
if (!uuid_str.empty() && !dsym_root.empty()) {
168+
char dsym_uuid_plist_path[PATH_MAX];
169+
snprintf(dsym_uuid_plist_path, sizeof(dsym_uuid_plist_path),
170+
"%s/Contents/Resources/%s.plist", dsym_root.c_str(),
171+
uuid_str.c_str());
172+
FileSpec dsym_uuid_plist_spec(dsym_uuid_plist_path);
173+
if (FileSystem::Instance().Exists(dsym_uuid_plist_spec)) {
174+
ApplePropertyList plist(dsym_uuid_plist_path);
175+
if (plist) {
176+
std::string DBGBuildSourcePath;
177+
std::string DBGSourcePath;
178+
179+
// DBGSourcePathRemapping is a dictionary in the plist
180+
// with keys which are DBGBuildSourcePath file paths and
181+
// values which are DBGSourcePath file paths
182+
183+
StructuredData::ObjectSP plist_sp =
184+
plist.GetStructuredData();
185+
if (plist_sp.get() && plist_sp->GetAsDictionary() &&
186+
plist_sp->GetAsDictionary()->HasKey(
187+
"DBGSourcePathRemapping") &&
188+
plist_sp->GetAsDictionary()
189+
->GetValueForKey("DBGSourcePathRemapping")
190+
->GetAsDictionary()) {
191+
192+
// If DBGVersion 1 or DBGVersion missing, ignore
193+
// DBGSourcePathRemapping. If DBGVersion 2, strip last two
194+
// components of path remappings from
195+
// entries to fix an issue with a
196+
// specific set of DBGSourcePathRemapping
197+
// entries that lldb worked with.
198+
// If DBGVersion 3, trust & use the source path remappings
199+
// as-is.
200+
//
201+
202+
bool new_style_source_remapping_dictionary = false;
203+
bool do_truncate_remapping_names = false;
204+
std::string original_DBGSourcePath_value = DBGSourcePath;
205+
if (plist_sp->GetAsDictionary()->HasKey("DBGVersion")) {
206+
std::string version_string =
207+
std::string(plist_sp->GetAsDictionary()
208+
->GetValueForKey("DBGVersion")
209+
->GetStringValue(""));
210+
if (!version_string.empty() &&
211+
isdigit(version_string[0])) {
212+
int version_number = atoi(version_string.c_str());
213+
if (version_number > 1) {
214+
new_style_source_remapping_dictionary = true;
215+
}
216+
if (version_number == 2) {
217+
do_truncate_remapping_names = true;
216218
}
217219
}
220+
}
218221

219-
StructuredData::Dictionary *remappings_dict =
220-
plist_sp->GetAsDictionary()
221-
->GetValueForKey("DBGSourcePathRemapping")
222-
->GetAsDictionary();
223-
remappings_dict->ForEach(
224-
[&module_sp, new_style_source_remapping_dictionary,
225-
original_DBGSourcePath_value, do_truncate_remapping_names](
226-
ConstString key,
227-
StructuredData::Object *object) -> bool {
228-
if (object && object->GetAsString()) {
229-
230-
// key is DBGBuildSourcePath
231-
// object is DBGSourcePath
232-
std::string DBGSourcePath =
233-
std::string(object->GetStringValue());
234-
if (!new_style_source_remapping_dictionary &&
235-
!original_DBGSourcePath_value.empty()) {
236-
DBGSourcePath = original_DBGSourcePath_value;
237-
}
238-
if (DBGSourcePath[0] == '~') {
239-
FileSpec resolved_source_path(
240-
DBGSourcePath.c_str());
241-
FileSystem::Instance().Resolve(
242-
resolved_source_path);
243-
DBGSourcePath =
244-
resolved_source_path.GetPath();
245-
}
222+
StructuredData::Dictionary *remappings_dict =
223+
plist_sp->GetAsDictionary()
224+
->GetValueForKey("DBGSourcePathRemapping")
225+
->GetAsDictionary();
226+
remappings_dict->ForEach(
227+
[&module_sp, new_style_source_remapping_dictionary,
228+
original_DBGSourcePath_value,
229+
do_truncate_remapping_names](
230+
ConstString key,
231+
StructuredData::Object *object) -> bool {
232+
if (object && object->GetAsString()) {
233+
234+
// key is DBGBuildSourcePath
235+
// object is DBGSourcePath
236+
std::string DBGSourcePath =
237+
std::string(object->GetStringValue());
238+
if (!new_style_source_remapping_dictionary &&
239+
!original_DBGSourcePath_value.empty()) {
240+
DBGSourcePath = original_DBGSourcePath_value;
241+
}
242+
if (DBGSourcePath[0] == '~') {
243+
FileSpec resolved_source_path(
244+
DBGSourcePath.c_str());
245+
FileSystem::Instance().Resolve(
246+
resolved_source_path);
247+
DBGSourcePath = resolved_source_path.GetPath();
248+
}
249+
module_sp->GetSourceMappingList().Append(
250+
key, ConstString(DBGSourcePath), true);
251+
// With version 2 of DBGSourcePathRemapping, we
252+
// can chop off the last two filename parts
253+
// from the source remapping and get a more
254+
// general source remapping that still works.
255+
// Add this as another option in addition to
256+
// the full source path remap.
257+
if (do_truncate_remapping_names) {
258+
FileSpec build_path(key.AsCString());
259+
FileSpec source_path(DBGSourcePath.c_str());
260+
build_path.RemoveLastPathComponent();
261+
build_path.RemoveLastPathComponent();
262+
source_path.RemoveLastPathComponent();
263+
source_path.RemoveLastPathComponent();
246264
module_sp->GetSourceMappingList().Append(
247-
key, ConstString(DBGSourcePath), true);
248-
// With version 2 of DBGSourcePathRemapping, we
249-
// can chop off the last two filename parts
250-
// from the source remapping and get a more
251-
// general source remapping that still works.
252-
// Add this as another option in addition to
253-
// the full source path remap.
254-
if (do_truncate_remapping_names) {
255-
FileSpec build_path(key.AsCString());
256-
FileSpec source_path(DBGSourcePath.c_str());
257-
build_path.RemoveLastPathComponent();
258-
build_path.RemoveLastPathComponent();
259-
source_path.RemoveLastPathComponent();
260-
source_path.RemoveLastPathComponent();
261-
module_sp->GetSourceMappingList().Append(
262-
ConstString(build_path.GetPath().c_str()),
263-
ConstString(source_path.GetPath().c_str()), true);
264-
}
265+
ConstString(build_path.GetPath().c_str()),
266+
ConstString(source_path.GetPath().c_str()),
267+
true);
265268
}
266-
return true;
267-
});
268-
}
269+
}
270+
return true;
271+
});
272+
}
269273

270-
// If we have a DBGBuildSourcePath + DBGSourcePath pair,
271-
// append those to the source path remappings.
272-
273-
plist.GetValueAsString("DBGBuildSourcePath",
274-
DBGBuildSourcePath);
275-
plist.GetValueAsString("DBGSourcePath", DBGSourcePath);
276-
if (!DBGBuildSourcePath.empty() &&
277-
!DBGSourcePath.empty()) {
278-
if (DBGSourcePath[0] == '~') {
279-
FileSpec resolved_source_path(DBGSourcePath.c_str());
280-
FileSystem::Instance().Resolve(resolved_source_path);
281-
DBGSourcePath = resolved_source_path.GetPath();
282-
}
283-
module_sp->GetSourceMappingList().Append(
284-
ConstString(DBGBuildSourcePath),
285-
ConstString(DBGSourcePath), true);
274+
// If we have a DBGBuildSourcePath + DBGSourcePath pair,
275+
// append those to the source path remappings.
276+
277+
plist.GetValueAsString("DBGBuildSourcePath",
278+
DBGBuildSourcePath);
279+
plist.GetValueAsString("DBGSourcePath", DBGSourcePath);
280+
if (!DBGBuildSourcePath.empty() && !DBGSourcePath.empty()) {
281+
if (DBGSourcePath[0] == '~') {
282+
FileSpec resolved_source_path(DBGSourcePath.c_str());
283+
FileSystem::Instance().Resolve(resolved_source_path);
284+
DBGSourcePath = resolved_source_path.GetPath();
286285
}
286+
module_sp->GetSourceMappingList().Append(
287+
ConstString(DBGBuildSourcePath),
288+
ConstString(DBGSourcePath), true);
287289
}
288290
}
289291
}
@@ -293,6 +295,13 @@ SymbolVendorMacOSX::CreateInstance(const lldb::ModuleSP &module_sp,
293295
}
294296

295297
symbol_vendor->AddSymbolFileRepresentation(dsym_objfile_sp);
298+
if (!dsym_root.empty()) {
299+
if (repro::Generator *g =
300+
repro::Reproducer::Instance().GetGenerator()) {
301+
repro::FileProvider &fp = g->GetOrCreate<repro::FileProvider>();
302+
fp.recordInterestingDirectory(dsym_root);
303+
}
304+
}
296305
return symbol_vendor;
297306
}
298307
}

‎lldb/source/Utility/Reproducer.cpp

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -321,6 +321,11 @@ void WorkingDirectoryProvider::Keep() {
321321
os << m_cwd << "\n";
322322
}
323323

324+
void FileProvider::recordInterestingDirectory(const llvm::Twine &dir) {
325+
if (m_collector)
326+
m_collector->addDirectory(dir);
327+
}
328+
324329
void ProviderBase::anchor() {}
325330
char CommandProvider::ID = 0;
326331
char FileProvider::ID = 0;

‎lldb/test/Shell/Reproducer/TestDSYM.test

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
# REQUIRES: system-darwin
2+
# Ensure that the reproducers captures the whole dSYM bundle.
3+
4+
# RUN: rm -rf %t.repro
5+
# RUN: %clang_host %S/Inputs/simple.c -g -o %t.out
6+
# RUN: touch %t.out.dSYM/foo.bar
7+
8+
# RUN: %lldb -x -b --capture --capture-path %t.repro %t.out -o 'b main' -o 'run' -o 'reproducer generate'
9+
10+
# RUN: %lldb -b -o 'reproducer dump -p files -f %t.repro' | FileCheck %s --check-prefix FILES
11+
# FILES: foo.bar

0 commit comments

Comments
 (0)