Author: jmolenda Date: Thu Oct 1 00:37:22 2015 New Revision: 248985 URL: http://llvm.org/viewvc/llvm-project?rev=248985&view=rev Log: Fixing a subtle issue on Mac OS X systems with dSYMs (possibly introduced by r235737 but I didn't look into it too closely).
A dSYM can have a per-UUID plist in it which tells lldb where to find an executable binary for the dSYM (DBGSymbolRichExecutable) - other information can be included in this plist, like how to remap the source file paths from their build pathnames to their long-term storage pathnames. This per-UUID plist is a unusual; it is used probably exclusively inside apple with our build system. It is not created by default in normal dSYMs. The problem was like this: 1. lldb wants to find an executable, given only a UUID (this happens when lldb is doing cross-host debugging and doesn't have a copy of the target system's binaries) 2. It eventually calls LocateMacOSXFilesUsingDebugSymbols which does a spotlight search for the dSYM on the local system, and failing that, tries the DBGShellCommands command to find the dSYM. 3. It gets a dSYM. It reads the per-UUID plist in the dSYM. The dSYM has a DBGSymbolRichExecutable kv pair pointing to the binary on a network filesystem. 4. Using the binary on the network filesystem, lldb now goes to find the dSYM. 5. It starts by looking for a dSYM next to the binary it found. 6. lldb is now reading the dSYM over a network filesystem, ignoring the one it found on its local filesystem earlier. Everything still *works* but it's much slower. This would be a tricky one to write up in a testsuite case; you really need the binary to not exist on the local system. And LocateMacOSXFilesUsingDebugSymbols will only compile on Mac OS X - even if I found a way to write up a test case, it would not run anywhere but on a mac. One change Greg wanted while I was touching this code was to have LocateMacOSXFilesUsingDebugSymbols (which could be asked to find a binary OR find a dSYM) to instead return a ModuleSpec with the sum total of everything it could find. This change of passing around a ModuleSpec instead of a FileSpec was percolated up into ModuleList::GetSharedModule. The changes to LocateMacOSXFilesUsingDebugSymbols look larger than they really are - there's a lot of simple whitespace changes in there. I ran the testsuites on mac, no new regressions introduced <rdar://problem/21993813> Modified: lldb/trunk/include/lldb/Host/Symbols.h lldb/trunk/source/Core/ModuleList.cpp lldb/trunk/source/Host/common/Symbols.cpp lldb/trunk/source/Host/macosx/Symbols.cpp lldb/trunk/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp Modified: lldb/trunk/include/lldb/Host/Symbols.h URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Host/Symbols.h?rev=248985&r1=248984&r2=248985&view=diff ============================================================================== --- lldb/trunk/include/lldb/Host/Symbols.h (original) +++ lldb/trunk/include/lldb/Host/Symbols.h Thu Oct 1 00:37:22 2015 @@ -29,7 +29,7 @@ public: // Locating the file should happen only on the local computer or using // the current computers global settings. //---------------------------------------------------------------------- - static FileSpec + static ModuleSpec LocateExecutableObjectFile (const ModuleSpec &module_spec); //---------------------------------------------------------------------- Modified: lldb/trunk/source/Core/ModuleList.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Core/ModuleList.cpp?rev=248985&r1=248984&r2=248985&view=diff ============================================================================== --- lldb/trunk/source/Core/ModuleList.cpp (original) +++ lldb/trunk/source/Core/ModuleList.cpp Thu Oct 1 00:37:22 2015 @@ -1045,19 +1045,19 @@ ModuleList::GetSharedModule // Fixup the incoming path in case the path points to a valid file, yet // the arch or UUID (if one was passed in) don't match. - FileSpec file_spec = Symbols::LocateExecutableObjectFile (module_spec); + ModuleSpec located_binary_modulespec = Symbols::LocateExecutableObjectFile (module_spec); // Don't look for the file if it appears to be the same one we already // checked for above... - if (file_spec != module_file_spec) + if (located_binary_modulespec.GetFileSpec() != module_file_spec) { - if (!file_spec.Exists()) + if (!located_binary_modulespec.GetFileSpec().Exists()) { - file_spec.GetPath(path, sizeof(path)); + located_binary_modulespec.GetFileSpec().GetPath(path, sizeof(path)); if (path[0] == '\0') module_file_spec.GetPath(path, sizeof(path)); // How can this check ever be true? This branch it is false, and we haven't modified file_spec. - if (file_spec.Exists()) + if (located_binary_modulespec.GetFileSpec().Exists()) { std::string uuid_str; if (uuid_ptr && uuid_ptr->IsValid()) @@ -1084,9 +1084,8 @@ ModuleList::GetSharedModule // Make sure no one else can try and get or create a module while this // function is actively working on it by doing an extra lock on the // global mutex list. - ModuleSpec platform_module_spec(module_spec); - platform_module_spec.GetFileSpec() = file_spec; - platform_module_spec.GetPlatformFileSpec() = file_spec; + ModuleSpec platform_module_spec(located_binary_modulespec); + platform_module_spec.GetPlatformFileSpec() = located_binary_modulespec.GetFileSpec(); ModuleList matching_module_list; if (shared_module_list.FindModules (platform_module_spec, matching_module_list) > 0) { @@ -1096,7 +1095,7 @@ ModuleList::GetSharedModule // then we should make sure the modification time hasn't changed! if (platform_module_spec.GetUUIDPtr() == NULL) { - TimeValue file_spec_mod_time(file_spec.GetModificationTime()); + TimeValue file_spec_mod_time(located_binary_modulespec.GetFileSpec().GetModificationTime()); if (file_spec_mod_time.IsValid()) { if (file_spec_mod_time != module_sp->GetModificationTime()) @@ -1125,9 +1124,9 @@ ModuleList::GetSharedModule } else { - file_spec.GetPath(path, sizeof(path)); + located_binary_modulespec.GetFileSpec().GetPath(path, sizeof(path)); - if (file_spec) + if (located_binary_modulespec.GetFileSpec()) { if (arch.IsValid()) error.SetErrorStringWithFormat("unable to open %s architecture in '%s'", arch.GetArchitectureName(), path); Modified: lldb/trunk/source/Host/common/Symbols.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Host/common/Symbols.cpp?rev=248985&r1=248984&r2=248985&view=diff ============================================================================== --- lldb/trunk/source/Host/common/Symbols.cpp (original) +++ lldb/trunk/source/Host/common/Symbols.cpp Thu Oct 1 00:37:22 2015 @@ -38,8 +38,7 @@ int LocateMacOSXFilesUsingDebugSymbols ( const ModuleSpec &module_spec, - FileSpec *out_exec_fspec, // If non-NULL, try and find the executable - FileSpec *out_dsym_fspec // If non-NULL try and find the debug symbol file + ModuleSpec &return_module_spec ); #else @@ -48,8 +47,7 @@ int LocateMacOSXFilesUsingDebugSymbols ( const ModuleSpec &module_spec, - FileSpec *out_exec_fspec, // If non-NULL, try and find the executable - FileSpec *out_dsym_fspec // If non-NULL try and find the debug symbol file + ModuleSpec &return_module_spec ) { // Cannot find MacOSX files using debug symbols on non MacOSX. return 0; @@ -178,19 +176,25 @@ LocateExecutableSymbolFileDsym (const Mo (const void*)uuid); FileSpec symbol_fspec; + ModuleSpec dsym_module_spec; // First try and find the dSYM in the same directory as the executable or in // an appropriate parent directory if (LocateDSYMInVincinityOfExecutable (module_spec, symbol_fspec) == false) { // We failed to easily find the dSYM above, so use DebugSymbols - LocateMacOSXFilesUsingDebugSymbols (module_spec, NULL, &symbol_fspec); + LocateMacOSXFilesUsingDebugSymbols (module_spec, dsym_module_spec); } - return symbol_fspec; + else + { + dsym_module_spec.GetSymbolFileSpec() = symbol_fspec; + } + return dsym_module_spec.GetSymbolFileSpec(); } -FileSpec +ModuleSpec Symbols::LocateExecutableObjectFile (const ModuleSpec &module_spec) { + ModuleSpec result = module_spec; const FileSpec *exec_fspec = module_spec.GetFileSpecPtr(); const ArchSpec *arch = module_spec.GetArchitecturePtr(); const UUID *uuid = module_spec.GetUUIDPtr(); @@ -200,20 +204,19 @@ Symbols::LocateExecutableObjectFile (con arch ? arch->GetArchitectureName() : "<NULL>", (const void*)uuid); - FileSpec objfile_fspec; ModuleSpecList module_specs; ModuleSpec matched_module_spec; if (exec_fspec && ObjectFile::GetModuleSpecifications(*exec_fspec, 0, 0, module_specs) && module_specs.FindMatchingModuleSpec(module_spec, matched_module_spec)) { - objfile_fspec = exec_fspec; + result.GetFileSpec() = exec_fspec; } else { - LocateMacOSXFilesUsingDebugSymbols (module_spec, &objfile_fspec, NULL); + LocateMacOSXFilesUsingDebugSymbols (module_spec, result); } - return objfile_fspec; + return result; } FileSpec Modified: lldb/trunk/source/Host/macosx/Symbols.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Host/macosx/Symbols.cpp?rev=248985&r1=248984&r2=248985&view=diff ============================================================================== --- lldb/trunk/source/Host/macosx/Symbols.cpp (original) +++ lldb/trunk/source/Host/macosx/Symbols.cpp Thu Oct 1 00:37:22 2015 @@ -55,17 +55,14 @@ int LocateMacOSXFilesUsingDebugSymbols ( const ModuleSpec &module_spec, - FileSpec *out_exec_fspec, // If non-NULL, try and find the executable - FileSpec *out_dsym_fspec // If non-NULL try and find the debug symbol file + ModuleSpec &return_module_spec ) { - int items_found = 0; - - if (out_exec_fspec) - out_exec_fspec->Clear(); + return_module_spec = module_spec; + return_module_spec.GetFileSpec().Clear(); + return_module_spec.GetSymbolFileSpec().Clear(); - if (out_dsym_fspec) - out_dsym_fspec->Clear(); + int items_found = 0; #if !defined (__arm__) && !defined (__arm64__) && !defined (__aarch64__) // No DebugSymbols on the iOS devices @@ -110,151 +107,132 @@ LocateMacOSXFilesUsingDebugSymbols strlen(exec_cf_path), FALSE)); } - if (log) - { - std::string searching_for; - if (out_exec_fspec && out_dsym_fspec) - { - searching_for = "executable binary and dSYM"; - } - else if (out_exec_fspec) - { - searching_for = "executable binary"; - } - else - { - searching_for = "dSYM bundle"; - } - log->Printf ("Calling DebugSymbols framework to locate dSYM bundle for UUID %s, searching for %s", uuid->GetAsString().c_str(), searching_for.c_str()); - } CFCReleaser<CFURLRef> dsym_url (::DBGCopyFullDSYMURLForUUID(module_uuid_ref.get(), exec_url.get())); char path[PATH_MAX]; if (dsym_url.get()) { - if (out_dsym_fspec) + if (::CFURLGetFileSystemRepresentation (dsym_url.get(), true, (UInt8*)path, sizeof(path)-1)) + { + if (log) + { + log->Printf ("DebugSymbols framework returned dSYM path of %s for UUID %s -- looking for the dSYM", path, uuid->GetAsString().c_str()); + } + FileSpec dsym_filespec(path, path[0] == '~'); + + if (dsym_filespec.GetFileType () == FileSpec::eFileTypeDirectory) + { + dsym_filespec = Symbols::FindSymbolFileInBundle (dsym_filespec, uuid, arch); + ++items_found; + } + else + { + ++items_found; + } + return_module_spec.GetSymbolFileSpec() = dsym_filespec; + } + + bool success = false; + if (log) { if (::CFURLGetFileSystemRepresentation (dsym_url.get(), true, (UInt8*)path, sizeof(path)-1)) { - if (log) - { - log->Printf ("DebugSymbols framework returned dSYM path of %s for UUID %s -- looking for the dSYM", path, uuid->GetAsString().c_str()); - } - out_dsym_fspec->SetFile(path, path[0] == '~'); + log->Printf ("DebugSymbols framework returned dSYM path of %s for UUID %s -- looking for an exec file", path, uuid->GetAsString().c_str()); + } - if (out_dsym_fspec->GetFileType () == FileSpec::eFileTypeDirectory) + } + + CFCReleaser<CFDictionaryRef> dict(::DBGCopyDSYMPropertyLists (dsym_url.get())); + CFDictionaryRef uuid_dict = NULL; + if (dict.get()) + { + CFCString uuid_cfstr (uuid->GetAsString().c_str()); + uuid_dict = static_cast<CFDictionaryRef>(::CFDictionaryGetValue (dict.get(), uuid_cfstr.get())); + } + if (uuid_dict) + { + CFStringRef exec_cf_path = static_cast<CFStringRef>(::CFDictionaryGetValue (uuid_dict, CFSTR("DBGSymbolRichExecutable"))); + if (exec_cf_path && ::CFStringGetFileSystemRepresentation (exec_cf_path, path, sizeof(path))) + { + if (log) { - *out_dsym_fspec = Symbols::FindSymbolFileInBundle (*out_dsym_fspec, uuid, arch); - if (*out_dsym_fspec) - ++items_found; + log->Printf ("plist bundle has exec path of %s for UUID %s", path, uuid->GetAsString().c_str()); } - else + ++items_found; + FileSpec exec_filespec (path, path[0] == '~'); + if (exec_filespec.Exists()) { - ++items_found; + success = true; + return_module_spec.GetFileSpec() = exec_filespec; } } } - if (out_exec_fspec) + if (!success) { - bool success = false; - if (log) - { - if (::CFURLGetFileSystemRepresentation (dsym_url.get(), true, (UInt8*)path, sizeof(path)-1)) - { - log->Printf ("DebugSymbols framework returned dSYM path of %s for UUID %s -- looking for an exec file", path, uuid->GetAsString().c_str()); - } - - } - CFCReleaser<CFDictionaryRef> dict(::DBGCopyDSYMPropertyLists (dsym_url.get())); - CFDictionaryRef uuid_dict = NULL; - if (dict.get()) - { - CFCString uuid_cfstr (uuid->GetAsString().c_str()); - uuid_dict = static_cast<CFDictionaryRef>(::CFDictionaryGetValue (dict.get(), uuid_cfstr.get())); - } - if (uuid_dict) + // No dictionary, check near the dSYM bundle for an executable that matches... + if (::CFURLGetFileSystemRepresentation (dsym_url.get(), true, (UInt8*)path, sizeof(path)-1)) { - CFStringRef exec_cf_path = static_cast<CFStringRef>(::CFDictionaryGetValue (uuid_dict, CFSTR("DBGSymbolRichExecutable"))); - if (exec_cf_path && ::CFStringGetFileSystemRepresentation (exec_cf_path, path, sizeof(path))) + char *dsym_extension_pos = ::strstr (path, ".dSYM"); + if (dsym_extension_pos) { + *dsym_extension_pos = '\0'; if (log) { - log->Printf ("plist bundle has exec path of %s for UUID %s", path, uuid->GetAsString().c_str()); + log->Printf ("Looking for executable binary next to dSYM bundle with name with name %s", path); } - ++items_found; - out_exec_fspec->SetFile(path, path[0] == '~'); - if (out_exec_fspec->Exists()) - success = true; - } - } - - if (!success) - { - // No dictionary, check near the dSYM bundle for an executable that matches... - if (::CFURLGetFileSystemRepresentation (dsym_url.get(), true, (UInt8*)path, sizeof(path)-1)) - { - char *dsym_extension_pos = ::strstr (path, ".dSYM"); - if (dsym_extension_pos) + FileSpec file_spec (path, true); + ModuleSpecList module_specs; + ModuleSpec matched_module_spec; + switch (file_spec.GetFileType()) { - *dsym_extension_pos = '\0'; - if (log) - { - log->Printf ("Looking for executable binary next to dSYM bundle with name with name %s", path); - } - FileSpec file_spec (path, true); - ModuleSpecList module_specs; - ModuleSpec matched_module_spec; - switch (file_spec.GetFileType()) - { - case FileSpec::eFileTypeDirectory: // Bundle directory? + case FileSpec::eFileTypeDirectory: // Bundle directory? + { + CFCBundle bundle (path); + CFCReleaser<CFURLRef> bundle_exe_url (bundle.CopyExecutableURL ()); + if (bundle_exe_url.get()) { - CFCBundle bundle (path); - CFCReleaser<CFURLRef> bundle_exe_url (bundle.CopyExecutableURL ()); - if (bundle_exe_url.get()) + if (::CFURLGetFileSystemRepresentation (bundle_exe_url.get(), true, (UInt8*)path, sizeof(path)-1)) { - if (::CFURLGetFileSystemRepresentation (bundle_exe_url.get(), true, (UInt8*)path, sizeof(path)-1)) - { - FileSpec bundle_exe_file_spec (path, true); - if (ObjectFile::GetModuleSpecifications(bundle_exe_file_spec, 0, 0, module_specs) && - module_specs.FindMatchingModuleSpec(module_spec, matched_module_spec)) + FileSpec bundle_exe_file_spec (path, true); + if (ObjectFile::GetModuleSpecifications(bundle_exe_file_spec, 0, 0, module_specs) && + module_specs.FindMatchingModuleSpec(module_spec, matched_module_spec)) + { + ++items_found; + return_module_spec.GetFileSpec() = bundle_exe_file_spec; + if (log) { - ++items_found; - *out_exec_fspec = bundle_exe_file_spec; - if (log) - { - log->Printf ("Executable binary %s next to dSYM is compatible; using", path); - } + log->Printf ("Executable binary %s next to dSYM is compatible; using", path); } } } } - break; - - case FileSpec::eFileTypePipe: // Forget pipes - case FileSpec::eFileTypeSocket: // We can't process socket files - case FileSpec::eFileTypeInvalid: // File doesn't exist... - break; - - case FileSpec::eFileTypeUnknown: - case FileSpec::eFileTypeRegular: - case FileSpec::eFileTypeSymbolicLink: - case FileSpec::eFileTypeOther: - if (ObjectFile::GetModuleSpecifications(file_spec, 0, 0, module_specs) && - module_specs.FindMatchingModuleSpec(module_spec, matched_module_spec)) + } + break; + case FileSpec::eFileTypePipe: // Forget pipes + case FileSpec::eFileTypeSocket: // We can't process socket files + case FileSpec::eFileTypeInvalid: // File doesn't exist... + break; + + case FileSpec::eFileTypeUnknown: + case FileSpec::eFileTypeRegular: + case FileSpec::eFileTypeSymbolicLink: + case FileSpec::eFileTypeOther: + if (ObjectFile::GetModuleSpecifications(file_spec, 0, 0, module_specs) && + module_specs.FindMatchingModuleSpec(module_spec, matched_module_spec)) + + { + ++items_found; + return_module_spec.GetFileSpec() = file_spec; + if (log) { - ++items_found; - *out_exec_fspec = file_spec; - if (log) - { - log->Printf ("Executable binary %s next to dSYM is compatible; using", path); - } + log->Printf ("Executable binary %s next to dSYM is compatible; using", path); } - break; - } + } + break; } } } Modified: lldb/trunk/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp?rev=248985&r1=248984&r2=248985&view=diff ============================================================================== --- lldb/trunk/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp (original) +++ lldb/trunk/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp Thu Oct 1 00:37:22 2015 @@ -321,7 +321,13 @@ ProcessKDP::DoConnectRemote (Stream *str // Lookup UUID locally, before attempting dsymForUUID like action module_spec.GetSymbolFileSpec() = Symbols::LocateExecutableSymbolFile(module_spec); if (module_spec.GetSymbolFileSpec()) - module_spec.GetFileSpec() = Symbols::LocateExecutableObjectFile (module_spec); + { + ModuleSpec executable_module_spec = Symbols::LocateExecutableObjectFile (module_spec); + if (executable_module_spec.GetFileSpec().Exists()) + { + module_spec.GetFileSpec() = executable_module_spec.GetFileSpec(); + } + } if (!module_spec.GetSymbolFileSpec() || !module_spec.GetSymbolFileSpec()) Symbols::DownloadObjectAndSymbolFile (module_spec, true); _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits