Skip to content

Commit 1d9a5be

Browse files
JDevlieghereJDevlieghere
committed
[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)
1 parent 83dca65 commit 1d9a5be

File tree

4 files changed

+145
-120
lines changed

4 files changed

+145
-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: 127 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

@@ -143,6 +144,11 @@ SymbolVendorMacOSX::CreateInstance(const lldb::ModuleSP &module_sp,
143144
}
144145

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

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

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

293295
symbol_vendor->AddSymbolFileRepresentation(dsym_objfile_sp);
296+
if (repro::Generator *g =
297+
repro::Reproducer::Instance().GetGenerator()) {
298+
repro::FileProvider &fp = g->GetOrCreate<repro::FileProvider>();
299+
fp.recordInterestingDirectory(dsym_root);
300+
}
294301
return symbol_vendor;
295302
}
296303
}

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