From 8825c5c9b4e304516b9e69e29fffbc30bd5a70b1 Mon Sep 17 00:00:00 2001 From: Jason Molenda Date: Thu, 8 Oct 2015 21:48:35 +0000 Subject: Re-commit the (fixed) changes from r248985 which were reverted by Pavel when they introduced android testsuite regressions. Pavel has run the testsuite against the updated patch and it completes cleanly now. The original commit message: 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 llvm-svn: 249755 --- lldb/source/Core/ModuleList.cpp | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) (limited to 'lldb/source/Core/ModuleList.cpp') diff --git a/lldb/source/Core/ModuleList.cpp b/lldb/source/Core/ModuleList.cpp index 9f92913aa52..0116bf67a75 100644 --- a/lldb/source/Core/ModuleList.cpp +++ b/lldb/source/Core/ModuleList.cpp @@ -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()) @@ -1085,8 +1085,9 @@ ModuleList::GetSharedModule // 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; + platform_module_spec.GetFileSpec() = located_binary_modulespec.GetFileSpec(); + platform_module_spec.GetPlatformFileSpec() = located_binary_modulespec.GetFileSpec(); + platform_module_spec.GetSymbolFileSpec() = located_binary_modulespec.GetSymbolFileSpec(); ModuleList matching_module_list; if (shared_module_list.FindModules (platform_module_spec, matching_module_list) > 0) { @@ -1096,7 +1097,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 +1126,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); -- cgit v1.2.3