llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-lldb Author: Michael Buch (Michael137) <details> <summary>Changes</summary> Depends on: * https://github.com/llvm/llvm-project/pull/182001 (only second commit is relevant for this review) This patch factors out the logic to load dSYM scripting resources into a helper function. In the process we eliminate some redundant copying of `FileSpec` and pass it to the helper by `const-ref` instead (specifically the `symfile_spec`). --- Full diff: https://github.com/llvm/llvm-project/pull/182002.diff 1 Files Affected: - (modified) lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp (+124-110) ``````````diff diff --git a/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp b/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp index 3e085e993cad7..1991777a6cb29 100644 --- a/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp +++ b/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp @@ -199,125 +199,139 @@ PlatformDarwin::PutFile(const lldb_private::FileSpec &source, return PlatformPOSIX::PutFile(source, destination, uid, gid); } -FileSpecList PlatformDarwin::LocateExecutableScriptingResources( - Target *target, Module &module, Stream &feedback_stream) { +static FileSpecList LoadExecutableScriptingResourceFromDSYM( + Stream &feedback_stream, FileSpec module_spec, const Target &target, + const FileSpec &symfile_spec, ) { FileSpecList file_list; - if (target && - target->GetDebugger().GetScriptLanguage() == eScriptLanguagePython) { - // NB some extensions might be meaningful and should not be stripped - - // "this.binary.file" - // should not lose ".file" but GetFileNameStrippingExtension() will do - // precisely that. Ideally, we should have a per-platform list of - // extensions (".exe", ".app", ".dSYM", ".framework") which should be - // stripped while leaving "this.binary.file" as-is. - - FileSpec module_spec = module.GetFileSpec(); - - if (module_spec) { - if (SymbolFile *symfile = module.GetSymbolFile()) { - ObjectFile *objfile = symfile->GetObjectFile(); - if (objfile) { - FileSpec symfile_spec(objfile->GetFileSpec()); - if (symfile_spec && - llvm::StringRef(symfile_spec.GetPath()) - .contains_insensitive(".dSYM/Contents/Resources/DWARF") && - FileSystem::Instance().Exists(symfile_spec)) { - while (module_spec.GetFilename()) { - std::string module_basename( - module_spec.GetFilename().GetCString()); - std::string original_module_basename(module_basename); - - bool was_keyword = false; - - // FIXME: for Python, we cannot allow certain characters in - // module - // filenames we import. Theoretically, different scripting - // languages may have different sets of forbidden tokens in - // filenames, and that should be dealt with by each - // ScriptInterpreter. For now, we just replace dots with - // underscores, but if we ever support anything other than - // Python we will need to rework this - llvm::replace(module_basename, '.', '_'); - llvm::replace(module_basename, ' ', '_'); - llvm::replace(module_basename, '-', '_'); - ScriptInterpreter *script_interpreter = - target->GetDebugger().GetScriptInterpreter(); - if (script_interpreter && - script_interpreter->IsReservedWord(module_basename.c_str())) { - module_basename.insert(module_basename.begin(), '_'); - was_keyword = true; - } + while (module_spec.GetFilename()) { + std::string module_basename(module_spec.GetFilename().GetCString()); + std::string original_module_basename(module_basename); + + bool was_keyword = false; + + // FIXME: for Python, we cannot allow certain characters in + // module + // filenames we import. Theoretically, different scripting + // languages may have different sets of forbidden tokens in + // filenames, and that should be dealt with by each + // ScriptInterpreter. For now, we just replace dots with + // underscores, but if we ever support anything other than + // Python we will need to rework this + llvm::replace(module_basename, '.', '_'); + llvm::replace(module_basename, ' ', '_'); + llvm::replace(module_basename, '-', '_'); + ScriptInterpreter *script_interpreter = + target.GetDebugger().GetScriptInterpreter(); + if (script_interpreter && + script_interpreter->IsReservedWord(module_basename.c_str())) { + module_basename.insert(module_basename.begin(), '_'); + was_keyword = true; + } - StreamString path_string; - StreamString original_path_string; - // for OSX we are going to be in - // .dSYM/Contents/Resources/DWARF/<basename> let us go to - // .dSYM/Contents/Resources/Python/<basename>.py and see if the - // file exists - path_string.Printf("%s/../Python/%s.py", - symfile_spec.GetDirectory().GetCString(), - module_basename.c_str()); - original_path_string.Printf( - "%s/../Python/%s.py", - symfile_spec.GetDirectory().GetCString(), - original_module_basename.c_str()); - FileSpec script_fspec(path_string.GetString()); - FileSystem::Instance().Resolve(script_fspec); - FileSpec orig_script_fspec(original_path_string.GetString()); - FileSystem::Instance().Resolve(orig_script_fspec); - - // if we did some replacements of reserved characters, and a - // file with the untampered name exists, then warn the user - // that the file as-is shall not be loaded - if (module_basename != original_module_basename && - FileSystem::Instance().Exists(orig_script_fspec)) { - const char *reason_for_complaint = - was_keyword ? "conflicts with a keyword" - : "contains reserved characters"; - if (FileSystem::Instance().Exists(script_fspec)) - feedback_stream.Printf( - "warning: the symbol file '%s' contains a debug " - "script. However, its name" - " '%s' %s and as such cannot be loaded. LLDB will" - " load '%s' instead. Consider removing the file with " - "the malformed name to" - " eliminate this warning.\n", - symfile_spec.GetPath().c_str(), - original_path_string.GetData(), reason_for_complaint, - path_string.GetData()); - else - feedback_stream.Printf( - "warning: the symbol file '%s' contains a debug " - "script. However, its name" - " %s and as such cannot be loaded. If you intend" - " to have this script loaded, please rename '%s' to " - "'%s' and retry.\n", - symfile_spec.GetPath().c_str(), reason_for_complaint, - original_path_string.GetData(), path_string.GetData()); - } + StreamString path_string; + StreamString original_path_string; + // for OSX we are going to be in + // .dSYM/Contents/Resources/DWARF/<basename> let us go to + // .dSYM/Contents/Resources/Python/<basename>.py and see if the + // file exists + path_string.Printf("%s/../Python/%s.py", + symfile_spec.GetDirectory().GetCString(), + module_basename.c_str()); + original_path_string.Printf("%s/../Python/%s.py", + symfile_spec.GetDirectory().GetCString(), + original_module_basename.c_str()); + FileSpec script_fspec(path_string.GetString()); + FileSystem::Instance().Resolve(script_fspec); + FileSpec orig_script_fspec(original_path_string.GetString()); + FileSystem::Instance().Resolve(orig_script_fspec); + + // if we did some replacements of reserved characters, and a + // file with the untampered name exists, then warn the user + // that the file as-is shall not be loaded + if (module_basename != original_module_basename && + FileSystem::Instance().Exists(orig_script_fspec)) { + const char *reason_for_complaint = was_keyword + ? "conflicts with a keyword" + : "contains reserved characters"; + if (FileSystem::Instance().Exists(script_fspec)) + feedback_stream.Printf( + "warning: the symbol file '%s' contains a debug " + "script. However, its name" + " '%s' %s and as such cannot be loaded. LLDB will" + " load '%s' instead. Consider removing the file with " + "the malformed name to" + " eliminate this warning.\n", + symfile_spec.GetPath().c_str(), original_path_string.GetData(), + reason_for_complaint, path_string.GetData()); + else + feedback_stream.Printf( + "warning: the symbol file '%s' contains a debug " + "script. However, its name" + " %s and as such cannot be loaded. If you intend" + " to have this script loaded, please rename '%s' to " + "'%s' and retry.\n", + symfile_spec.GetPath().c_str(), reason_for_complaint, + original_path_string.GetData(), path_string.GetData()); + } - if (FileSystem::Instance().Exists(script_fspec)) { - file_list.Append(script_fspec); - break; - } + if (FileSystem::Instance().Exists(script_fspec)) { + file_list.Append(script_fspec); + break; + } - // If we didn't find the python file, then keep stripping the - // extensions and try again - ConstString filename_no_extension( - module_spec.GetFileNameStrippingExtension()); - if (module_spec.GetFilename() == filename_no_extension) - break; + // If we didn't find the python file, then keep stripping the + // extensions and try again + ConstString filename_no_extension( + module_spec.GetFileNameStrippingExtension()); + if (module_spec.GetFilename() == filename_no_extension) + break; - module_spec.SetFilename(filename_no_extension); - } - } - } - } - } + module_spec.SetFilename(filename_no_extension); } + return file_list; } +FileSpecList PlatformDarwin::LocateExecutableScriptingResources( + Target *target, Module &module, Stream &feedback_stream) { + if (!target) + return {}; + + // For now only Python scripts supported for auto-loading. + if (target->GetDebugger().GetScriptLanguage() != eScriptLanguagePython) + return {}; + + // NB some extensions might be meaningful and should not be stripped - + // "this.binary.file" + // should not lose ".file" but GetFileNameStrippingExtension() will do + // precisely that. Ideally, we should have a per-platform list of + // extensions (".exe", ".app", ".dSYM", ".framework") which should be + // stripped while leaving "this.binary.file" as-is. + + const FileSpec &module_spec = module.GetFileSpec(); + + if (!module_spec) + return {}; + + SymbolFile *symfile = module.GetSymbolFile(); + if (!symfile) + return {}; + + ObjectFile *objfile = symfile->GetObjectFile(); + if (!objfile) + return {}; + + const FileSpec &symfile_spec = objfile->GetFileSpec(); + if (symfile_spec && + llvm::StringRef(symfile_spec.GetPath()) + .contains_insensitive(".dSYM/Contents/Resources/DWARF") && + FileSystem::Instance().Exists(symfile_spec)) + return LoadExecutableScriptingResourceFromDSYM(feedback_stream, module_spec, + *target, symfile_spec); + + return {}; +} + Status PlatformDarwin::ResolveSymbolFile(Target &target, const ModuleSpec &sym_spec, FileSpec &sym_file) { `````````` </details> https://github.com/llvm/llvm-project/pull/182002 _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
