[Lldb-commits] [lldb] [lldb][PlatformDarwin][NFC] Move logic to emit warning on invalid/conflicting Python script names into helper function (PR #185669)
via lldb-commits
lldb-commits at lists.llvm.org
Tue Mar 10 08:27:44 PDT 2026
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/185666
* https://github.com/llvm/llvm-project/pull/185627
I'm planning on re-using this logic for a different API. Hence move it into a common helper.
---
Full diff: https://github.com/llvm/llvm-project/pull/185669.diff
2 Files Affected:
- (modified) lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp (+93-51)
- (modified) lldb/unittests/Platform/PlatformDarwinTest.cpp (+14-14)
``````````diff
diff --git a/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp b/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
index c49bd69618227..d81ea6c31614e 100644
--- a/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
+++ b/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
@@ -80,6 +80,92 @@ static Status ExceptionMaskValidator(const char *string, void *unused) {
return {};
}
+namespace {
+/// Holds an lldb_private::Module name and a "sanitized" version
+/// of it for the purposes of loading a script of that name by
+/// the relevant ScriptInterpreter.
+///
+/// E.g., for Python the sanitized name can't include:
+/// * Special characters: '-', ' ', '.'
+/// * Python keywords
+class SanitizedScriptingModuleName {
+public:
+ SanitizedScriptingModuleName(llvm::StringRef name,
+ ScriptInterpreter *script_interpreter)
+ : m_original_name(name), m_sanitized_name(name.str()) {
+ // FIXME: for Python, don't allow certain characters in imported module
+ // filenames. Theoretically, different scripting languages may have
+ // different sets of forbidden tokens in filenames, and that should
+ // be dealt with by each ScriptInterpreter. For now, just replace dots
+ // with underscores. In order to support anything other than Python
+ // this will need to be reworked.
+ llvm::replace(m_sanitized_name, '.', '_');
+ llvm::replace(m_sanitized_name, ' ', '_');
+ llvm::replace(m_sanitized_name, '-', '_');
+
+ if (script_interpreter &&
+ script_interpreter->IsReservedWord(m_sanitized_name.c_str())) {
+ m_sanitized_name.insert(m_sanitized_name.begin(), '_');
+ m_name_is_keyword = true;
+ }
+ }
+
+ /// Returns \c true if this name is a keyword in the associated scripting
+ /// language.
+ bool IsKeyword() const { return m_name_is_keyword; }
+
+ /// Returns \c true if the original name has been sanitized (i.e., required
+ /// changes).
+ bool RequiredSanitization() const {
+ return m_sanitized_name != m_original_name;
+ }
+
+ llvm::StringRef GetSanitizedName() const { return m_sanitized_name; }
+ llvm::StringRef GetOriginalName() const { return m_original_name; }
+
+ /// 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.
+ void WarnIfInvalidUnsanitizedScriptExists(Stream &os,
+ const FileSpec &original_fspec,
+ const FileSpec &fspec) const {
+ if (!RequiredSanitization())
+ return;
+
+ // Path to unsanitized script name doesn't exist. Nothing to warn about.
+ if (!FileSystem::Instance().Exists(original_fspec))
+ return;
+
+ llvm::StringRef reason_for_complaint = IsKeyword()
+ ? "conflicts with a keyword"
+ : "contains reserved characters";
+
+ if (FileSystem::Instance().Exists(fspec))
+ os.Format("warning: found a debug script '{0}'. However, its name"
+ " {1} and as such cannot be loaded. LLDB will"
+ " load '{2}' instead. Consider removing the file with"
+ " the malformed name to eliminate this warning.\n",
+ original_fspec.GetPath(), reason_for_complaint,
+ fspec.GetPath());
+ else
+ os.Format("warning: found a debug script '{0}'. However, its name"
+ " {1} and as such cannot be loaded. If you intend"
+ " to have this script loaded, please rename it to "
+ "'{2}' and retry.\n",
+ original_fspec.GetPath(), reason_for_complaint,
+ fspec.GetPath());
+ }
+
+private:
+ llvm::StringRef m_original_name;
+ std::string m_sanitized_name;
+
+ /// \c true if m_sanitized_name is a keyword for the ScriptInterpreter
+ /// language associated with this SanitizedScriptingModuleName.
+ bool m_name_is_keyword = false;
+};
+} // namespace
+
/// Destructor.
///
/// The destructor is virtual since this class is designed to be
@@ -201,27 +287,9 @@ FileSpecList PlatformDarwin::LocateExecutableScriptingResourcesFromDSYM(
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, don't allow certain characters in imported module
- // filenames. Theoretically, different scripting languages may have
- // different sets of forbidden tokens in filenames, and that should
- // be dealt with by each ScriptInterpreter. For now, just replace dots
- // with underscores. In order to support anything other than Python
- // this will need to be reworked.
- 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;
- }
+ SanitizedScriptingModuleName sanitized_name(
+ module_spec.GetFilename().GetStringRef(),
+ target.GetDebugger().GetScriptInterpreter());
StreamString path_string;
StreamString original_path_string;
@@ -231,44 +299,18 @@ FileSpecList PlatformDarwin::LocateExecutableScriptingResourcesFromDSYM(
// file exists
path_string.Format("{0}/../Python/{1}.py",
symfile_spec.GetDirectory().GetStringRef(),
- module_basename);
+ sanitized_name.GetSanitizedName());
original_path_string.Format("{0}/../Python/{1}.py",
symfile_spec.GetDirectory().GetStringRef(),
- original_module_basename);
+ sanitized_name.GetOriginalName());
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.Format(
- "warning: the symbol file '{0}' contains a debug "
- "script. However, its name"
- " '{1}' {2} and as such cannot be loaded. LLDB will"
- " load '{3}' instead. Consider removing the file with "
- "the malformed name to"
- " eliminate this warning.\n",
- symfile_spec.GetPath(), original_path_string.GetString(),
- reason_for_complaint, path_string.GetString());
- else
- feedback_stream.Format(
- "warning: the symbol file '{0}' contains a debug "
- "script. However, its name"
- " {1} and as such cannot be loaded. If you intend"
- " to have this script loaded, please rename '{2}' to "
- "'{3}' and retry.\n",
- symfile_spec.GetPath(), reason_for_complaint,
- original_path_string.GetString(), path_string.GetString());
- }
+ sanitized_name.WarnIfInvalidUnsanitizedScriptExists(
+ feedback_stream, orig_script_fspec, script_fspec);
if (FileSystem::Instance().Exists(script_fspec)) {
file_list.Append(script_fspec);
diff --git a/lldb/unittests/Platform/PlatformDarwinTest.cpp b/lldb/unittests/Platform/PlatformDarwinTest.cpp
index 30a37f7aa2b25..da17aa980df1b 100644
--- a/lldb/unittests/Platform/PlatformDarwinTest.cpp
+++ b/lldb/unittests/Platform/PlatformDarwinTest.cpp
@@ -351,11 +351,11 @@ TEST_F(PlatformDarwinLocateTest,
std::string fixed_script =
(m_tmp_dsym_dwarf_dir + "/../Python/_import.py").str();
std::string expected = llvm::formatv(
- "warning: the symbol file '{0}' contains a debug script. However, its "
+ "warning: found a debug script '{0}'. However, its "
"name conflicts with a keyword and as such cannot be loaded. If you "
- "intend to have this script loaded, please rename '{1}' to '{2}' and "
+ "intend to have this script loaded, please rename it to '{1}' and "
"retry.\n",
- dsym_module_fpec.GetPath(), orig_script, fixed_script);
+ orig_script, fixed_script);
EXPECT_EQ(ss.GetString(), expected);
}
@@ -391,11 +391,11 @@ TEST_F(PlatformDarwinLocateTest,
std::string fixed_script =
(m_tmp_dsym_dwarf_dir + "/../Python/_import.py").str();
std::string expected = llvm::formatv(
- "warning: the symbol file '{0}' contains a debug script. However, its "
- "name '{1}' conflicts with a keyword and as such cannot be loaded. LLDB "
- "will load '{2}' instead. Consider removing the file with the malformed "
+ "warning: found a debug script '{0}'. However, its "
+ "name conflicts with a keyword and as such cannot be loaded. LLDB "
+ "will load '{1}' instead. Consider removing the file with the malformed "
"name to eliminate this warning.\n",
- dsym_module_fpec.GetPath(), orig_script, fixed_script);
+ orig_script, fixed_script);
EXPECT_EQ(ss.GetString(), expected);
}
@@ -458,11 +458,11 @@ TEST_F(
std::string fixed_script =
(m_tmp_dsym_dwarf_dir + "/../Python/TestModule_1_1_1.py").str();
std::string expected = llvm::formatv(
- "warning: the symbol file '{0}' contains a debug script. However, its "
+ "warning: found a debug script '{0}'. However, its "
"name contains reserved characters and as such cannot be loaded. If you "
- "intend to have this script loaded, please rename '{1}' to '{2}' and "
+ "intend to have this script loaded, please rename it to '{1}' and "
"retry.\n",
- dsym_module_fpec.GetPath(), orig_script, fixed_script);
+ orig_script, fixed_script);
EXPECT_EQ(ss.GetString(), expected);
}
@@ -499,11 +499,11 @@ TEST_F(
std::string fixed_script =
(m_tmp_dsym_dwarf_dir + "/../Python/TestModule_1_1_1.py").str();
std::string expected = llvm::formatv(
- "warning: the symbol file '{0}' contains a debug script. However, its "
- "name '{1}' contains reserved characters and as such cannot be loaded. "
- "LLDB will load '{2}' instead. Consider removing the file with the "
+ "warning: found a debug script '{0}'. However, its "
+ "name contains reserved characters and as such cannot be loaded. "
+ "LLDB will load '{1}' instead. Consider removing the file with the "
"malformed name to eliminate this warning.\n",
- dsym_module_fpec.GetPath(), orig_script, fixed_script);
+ orig_script, fixed_script);
EXPECT_EQ(ss.GetString(), expected);
}
``````````
</details>
https://github.com/llvm/llvm-project/pull/185669
More information about the lldb-commits
mailing list