[Lldb-commits] [lldb] [lldb] Prevent passing a nullptr to std::string in ObjectFileMachO (PR #100421)
via lldb-commits
lldb-commits at lists.llvm.org
Wed Jul 24 09:47:50 PDT 2024
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-lldb
Author: Jonas Devlieghere (JDevlieghere)
<details>
<summary>Changes</summary>
Prevent passing a nullptr to std::string::insert in
ObjectFileMachO::GetDependentModules. Calling GetCString on an empty
ConstString will return a nullptr, which is undefined behavior. Instead,
use the GetString helper which will return an empty string in that case.
rdar://132388027
---
Full diff: https://github.com/llvm/llvm-project/pull/100421.diff
1 Files Affected:
- (modified) lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp (+104-103)
``````````diff
diff --git a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
index 2c7005449f9d7..ce095bcc48374 100644
--- a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
+++ b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
@@ -137,6 +137,9 @@ using namespace lldb;
using namespace lldb_private;
using namespace llvm::MachO;
+static constexpr llvm::StringLiteral g_loader_path = "@loader_path";
+static constexpr llvm::StringLiteral g_executable_path = "@executable_path";
+
LLDB_PLUGIN_DEFINE(ObjectFileMachO)
static void PrintRegisterValue(RegisterContext *reg_ctx, const char *name,
@@ -5116,123 +5119,121 @@ UUID ObjectFileMachO::GetUUID() {
}
uint32_t ObjectFileMachO::GetDependentModules(FileSpecList &files) {
+ ModuleSP module_sp = GetModule();
+ if (!module_sp)
+ return 0;
+
uint32_t count = 0;
- ModuleSP module_sp(GetModule());
- if (module_sp) {
- std::lock_guard<std::recursive_mutex> guard(module_sp->GetMutex());
- llvm::MachO::load_command load_cmd;
- lldb::offset_t offset = MachHeaderSizeFromMagic(m_header.magic);
- std::vector<std::string> rpath_paths;
- std::vector<std::string> rpath_relative_paths;
- std::vector<std::string> at_exec_relative_paths;
- uint32_t i;
- for (i = 0; i < m_header.ncmds; ++i) {
- const uint32_t cmd_offset = offset;
- if (m_data.GetU32(&offset, &load_cmd, 2) == nullptr)
- break;
+ std::lock_guard<std::recursive_mutex> guard(module_sp->GetMutex());
+ llvm::MachO::load_command load_cmd;
+ lldb::offset_t offset = MachHeaderSizeFromMagic(m_header.magic);
+ std::vector<std::string> rpath_paths;
+ std::vector<std::string> rpath_relative_paths;
+ std::vector<std::string> at_exec_relative_paths;
+ uint32_t i;
+ for (i = 0; i < m_header.ncmds; ++i) {
+ const uint32_t cmd_offset = offset;
+ if (m_data.GetU32(&offset, &load_cmd, 2) == nullptr)
+ break;
- switch (load_cmd.cmd) {
- case LC_RPATH:
- case LC_LOAD_DYLIB:
- case LC_LOAD_WEAK_DYLIB:
- case LC_REEXPORT_DYLIB:
- case LC_LOAD_DYLINKER:
- case LC_LOADFVMLIB:
- case LC_LOAD_UPWARD_DYLIB: {
- uint32_t name_offset = cmd_offset + m_data.GetU32(&offset);
- // For LC_LOAD_DYLIB there is an alternate encoding
- // which adds a uint32_t `flags` field for `DYLD_USE_*`
- // flags. This can be detected by a timestamp field with
- // the `DYLIB_USE_MARKER` constant value.
- bool is_delayed_init = false;
- uint32_t use_command_marker = m_data.GetU32(&offset);
- if (use_command_marker == 0x1a741800 /* DYLIB_USE_MARKER */) {
- offset += 4; /* uint32_t current_version */
- offset += 4; /* uint32_t compat_version */
- uint32_t flags = m_data.GetU32(&offset);
- // If this LC_LOAD_DYLIB is marked delay-init,
- // don't report it as a dependent library -- it
- // may be loaded in the process at some point,
- // but will most likely not be load at launch.
- if (flags & 0x08 /* DYLIB_USE_DELAYED_INIT */)
- is_delayed_init = true;
- }
- const char *path = m_data.PeekCStr(name_offset);
- if (path && !is_delayed_init) {
- if (load_cmd.cmd == LC_RPATH)
- rpath_paths.push_back(path);
- else {
- if (path[0] == '@') {
- if (strncmp(path, "@rpath", strlen("@rpath")) == 0)
- rpath_relative_paths.push_back(path + strlen("@rpath"));
- else if (strncmp(path, "@executable_path",
- strlen("@executable_path")) == 0)
- at_exec_relative_paths.push_back(path +
- strlen("@executable_path"));
- } else {
- FileSpec file_spec(path);
- if (files.AppendIfUnique(file_spec))
- count++;
- }
+ switch (load_cmd.cmd) {
+ case LC_RPATH:
+ case LC_LOAD_DYLIB:
+ case LC_LOAD_WEAK_DYLIB:
+ case LC_REEXPORT_DYLIB:
+ case LC_LOAD_DYLINKER:
+ case LC_LOADFVMLIB:
+ case LC_LOAD_UPWARD_DYLIB: {
+ uint32_t name_offset = cmd_offset + m_data.GetU32(&offset);
+ // For LC_LOAD_DYLIB there is an alternate encoding
+ // which adds a uint32_t `flags` field for `DYLD_USE_*`
+ // flags. This can be detected by a timestamp field with
+ // the `DYLIB_USE_MARKER` constant value.
+ bool is_delayed_init = false;
+ uint32_t use_command_marker = m_data.GetU32(&offset);
+ if (use_command_marker == 0x1a741800 /* DYLIB_USE_MARKER */) {
+ offset += 4; /* uint32_t current_version */
+ offset += 4; /* uint32_t compat_version */
+ uint32_t flags = m_data.GetU32(&offset);
+ // If this LC_LOAD_DYLIB is marked delay-init,
+ // don't report it as a dependent library -- it
+ // may be loaded in the process at some point,
+ // but will most likely not be load at launch.
+ if (flags & 0x08 /* DYLIB_USE_DELAYED_INIT */)
+ is_delayed_init = true;
+ }
+ const char *path = m_data.PeekCStr(name_offset);
+ if (path && !is_delayed_init) {
+ if (load_cmd.cmd == LC_RPATH)
+ rpath_paths.push_back(path);
+ else {
+ if (path[0] == '@') {
+ if (strncmp(path, "@rpath", strlen("@rpath")) == 0)
+ rpath_relative_paths.push_back(path + strlen("@rpath"));
+ else if (strncmp(path, "@executable_path",
+ strlen("@executable_path")) == 0)
+ at_exec_relative_paths.push_back(path +
+ strlen("@executable_path"));
+ } else {
+ FileSpec file_spec(path);
+ if (files.AppendIfUnique(file_spec))
+ count++;
}
}
- } break;
-
- default:
- break;
}
- offset = cmd_offset + load_cmd.cmdsize;
- }
+ } break;
- FileSpec this_file_spec(m_file);
- FileSystem::Instance().Resolve(this_file_spec);
-
- if (!rpath_paths.empty()) {
- // Fixup all LC_RPATH values to be absolute paths
- std::string loader_path("@loader_path");
- std::string executable_path("@executable_path");
- for (auto &rpath : rpath_paths) {
- if (llvm::StringRef(rpath).starts_with(loader_path)) {
- rpath.erase(0, loader_path.size());
- rpath.insert(0, this_file_spec.GetDirectory().GetCString());
- } else if (llvm::StringRef(rpath).starts_with(executable_path)) {
- rpath.erase(0, executable_path.size());
- rpath.insert(0, this_file_spec.GetDirectory().GetCString());
- }
- }
+ default:
+ break;
+ }
+ offset = cmd_offset + load_cmd.cmdsize;
+ }
- for (const auto &rpath_relative_path : rpath_relative_paths) {
- for (const auto &rpath : rpath_paths) {
- std::string path = rpath;
- path += rpath_relative_path;
- // It is OK to resolve this path because we must find a file on disk
- // for us to accept it anyway if it is rpath relative.
- FileSpec file_spec(path);
- FileSystem::Instance().Resolve(file_spec);
- if (FileSystem::Instance().Exists(file_spec) &&
- files.AppendIfUnique(file_spec)) {
- count++;
- break;
- }
- }
- }
+ FileSpec this_file_spec(m_file);
+ FileSystem::Instance().Resolve(this_file_spec);
+
+ if (!rpath_paths.empty()) {
+ // Fixup all LC_RPATH values to be absolute paths.
+ const std::string this_directory =
+ this_file_spec.GetDirectory().GetString();
+ for (auto &rpath : rpath_paths) {
+ if (llvm::StringRef(rpath).starts_with(g_loader_path))
+ rpath = this_directory + rpath.substr(g_loader_path.size());
+ else if (llvm::StringRef(rpath).starts_with(g_executable_path))
+ rpath = this_directory + rpath.substr(g_executable_path.size());
}
- // We may have @executable_paths but no RPATHS. Figure those out here.
- // Only do this if this object file is the executable. We have no way to
- // get back to the actual executable otherwise, so we won't get the right
- // path.
- if (!at_exec_relative_paths.empty() && CalculateType() == eTypeExecutable) {
- FileSpec exec_dir = this_file_spec.CopyByRemovingLastPathComponent();
- for (const auto &at_exec_relative_path : at_exec_relative_paths) {
- FileSpec file_spec =
- exec_dir.CopyByAppendingPathComponent(at_exec_relative_path);
+ for (const auto &rpath_relative_path : rpath_relative_paths) {
+ for (const auto &rpath : rpath_paths) {
+ std::string path = rpath;
+ path += rpath_relative_path;
+ // It is OK to resolve this path because we must find a file on disk
+ // for us to accept it anyway if it is rpath relative.
+ FileSpec file_spec(path);
+ FileSystem::Instance().Resolve(file_spec);
if (FileSystem::Instance().Exists(file_spec) &&
- files.AppendIfUnique(file_spec))
+ files.AppendIfUnique(file_spec)) {
count++;
+ break;
+ }
}
}
}
+
+ // We may have @executable_paths but no RPATHS. Figure those out here.
+ // Only do this if this object file is the executable. We have no way to
+ // get back to the actual executable otherwise, so we won't get the right
+ // path.
+ if (!at_exec_relative_paths.empty() && CalculateType() == eTypeExecutable) {
+ FileSpec exec_dir = this_file_spec.CopyByRemovingLastPathComponent();
+ for (const auto &at_exec_relative_path : at_exec_relative_paths) {
+ FileSpec file_spec =
+ exec_dir.CopyByAppendingPathComponent(at_exec_relative_path);
+ if (FileSystem::Instance().Exists(file_spec) &&
+ files.AppendIfUnique(file_spec))
+ count++;
+ }
+ }
return count;
}
``````````
</details>
https://github.com/llvm/llvm-project/pull/100421
More information about the lldb-commits
mailing list