Author: Jonas Devlieghere Date: 2024-07-24T15:07:38-07:00 New Revision: e846fb48038a34d8df3ad7412bbdcf37e9e7acc9
URL: https://github.com/llvm/llvm-project/commit/e846fb48038a34d8df3ad7412bbdcf37e9e7acc9 DIFF: https://github.com/llvm/llvm-project/commit/e846fb48038a34d8df3ad7412bbdcf37e9e7acc9.diff LOG: [lldb] Prevent passing a nullptr to std::string in ObjectFileMachO (#100421) 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 Added: Modified: lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp Removed: ################################################################################ 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; } _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
