https://github.com/Michael137 created https://github.com/llvm/llvm-project/pull/182002
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`). >From bab822ac817a6ac8d55047d6db82ff391f73f74f Mon Sep 17 00:00:00 2001 From: Michael Buch <[email protected]> Date: Wed, 18 Feb 2026 11:46:57 +0000 Subject: [PATCH 1/2] [lldb][PlatformDarwin][NFC] Use early-return style in LocateExecutableScriptingResources I'm planning on adding more to this function. It'll be easier to review/reason about if we un-nested the if-blocks (as the LLVM style guide recommends). --- .../Platform/MacOSX/PlatformDarwin.cpp | 220 +++++++++--------- 1 file changed, 113 insertions(+), 107 deletions(-) diff --git a/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp b/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp index 3e085e993cad7..ae0f6cabe90f8 100644 --- a/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp +++ b/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp @@ -201,120 +201,126 @@ PlatformDarwin::PutFile(const lldb_private::FileSpec &source, FileSpecList PlatformDarwin::LocateExecutableScriptingResources( Target *target, Module &module, Stream &feedback_stream) { - 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; - } + if (!target) + return {}; - 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()); - } + // For now only Python scripts supported for auto-loading. + if (target->GetDebugger().GetScriptLanguage() != eScriptLanguagePython) + return {}; - if (FileSystem::Instance().Exists(script_fspec)) { - file_list.Append(script_fspec); - break; - } + // 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. - // 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; + FileSpec module_spec = module.GetFileSpec(); - module_spec.SetFilename(filename_no_extension); - } - } - } + if (!module_spec) + return {}; + + SymbolFile *symfile = module.GetSymbolFile(); + if (!symfile) + return {}; + + ObjectFile *objfile = symfile->GetObjectFile(); + if (!objfile) + return {}; + + FileSpecList file_list; + 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; + } + + 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 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); } } + return file_list; } >From 34078973309b9cd20f4fdea405763e6cb5a7afb4 Mon Sep 17 00:00:00 2001 From: Michael Buch <[email protected]> Date: Wed, 18 Feb 2026 11:59:17 +0000 Subject: [PATCH 2/2] [lldb][PlatformDarwin][NFCI] Factor out dSYM script auto-loading into helper function 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`). --- .../Platform/MacOSX/PlatformDarwin.cpp | 190 +++++++++--------- 1 file changed, 99 insertions(+), 91 deletions(-) diff --git a/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp b/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp index ae0f6cabe90f8..1991777a6cb29 100644 --- a/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp +++ b/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp @@ -199,6 +199,99 @@ PlatformDarwin::PutFile(const lldb_private::FileSpec &source, return PlatformPOSIX::PutFile(source, destination, uid, gid); } +static FileSpecList LoadExecutableScriptingResourceFromDSYM( + Stream &feedback_stream, FileSpec module_spec, const Target &target, + const FileSpec &symfile_spec, ) { + FileSpecList file_list; + 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()); + } + + 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; + + module_spec.SetFilename(filename_no_extension); + } + + return file_list; +} + FileSpecList PlatformDarwin::LocateExecutableScriptingResources( Target *target, Module &module, Stream &feedback_stream) { if (!target) @@ -215,7 +308,7 @@ FileSpecList PlatformDarwin::LocateExecutableScriptingResources( // extensions (".exe", ".app", ".dSYM", ".framework") which should be // stripped while leaving "this.binary.file" as-is. - FileSpec module_spec = module.GetFileSpec(); + const FileSpec &module_spec = module.GetFileSpec(); if (!module_spec) return {}; @@ -228,100 +321,15 @@ FileSpecList PlatformDarwin::LocateExecutableScriptingResources( if (!objfile) return {}; - FileSpecList file_list; - FileSpec symfile_spec(objfile->GetFileSpec()); + 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)) { - 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()); - } - - if (FileSystem::Instance().Exists(script_fspec)) { - file_list.Append(script_fspec); - break; - } + FileSystem::Instance().Exists(symfile_spec)) + return LoadExecutableScriptingResourceFromDSYM(feedback_stream, module_spec, + *target, symfile_spec); - // 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); - } - } - - return file_list; + return {}; } Status PlatformDarwin::ResolveSymbolFile(Target &target, _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
