llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-lldb Author: Tal Keren (talkeren) <details> <summary>Changes</summary> This change remove the call to GetPlatform from GetDeploymentInfo and instead pass it as an argument,so GetPlatform will be called only once for all the load commands. The reason is that if we try to query the platform too early we will get an invalid response which we won't cache, and since each call to `GetProcessPlatformViaDYLDSPI` is slow, handling the load commands is getting increasnigly slower when there are many of them. See #<!-- -->140610 --- Full diff: https://github.com/llvm/llvm-project/pull/140853.diff 5 Files Affected: - (modified) lldb/tools/debugserver/source/DNB.cpp (+10-3) - (modified) lldb/tools/debugserver/source/DNB.h (+10-9) - (modified) lldb/tools/debugserver/source/MacOSX/MachProcess.h (+2-1) - (modified) lldb/tools/debugserver/source/MacOSX/MachProcess.mm (+7-7) - (modified) lldb/tools/debugserver/source/RNBRemote.cpp (+4-3) ``````````diff diff --git a/lldb/tools/debugserver/source/DNB.cpp b/lldb/tools/debugserver/source/DNB.cpp index f541134b43a1b..b59218456741f 100644 --- a/lldb/tools/debugserver/source/DNB.cpp +++ b/lldb/tools/debugserver/source/DNB.cpp @@ -1498,7 +1498,7 @@ nub_bool_t DNBProcessSharedLibrariesUpdated(nub_process_t pid) { std::optional<std::string> DNBGetDeploymentInfo(nub_process_t pid, bool is_executable, - const struct load_command &lc, + uint32_t dyld_platform, const struct load_command &lc, uint64_t load_command_address, uint32_t &major_version, uint32_t &minor_version, uint32_t &patch_version) { MachProcessSP procSP; @@ -1507,8 +1507,8 @@ DNBGetDeploymentInfo(nub_process_t pid, bool is_executable, // macOS binary) is loaded with the macCatalyst dyld platform // override. The image info corrects for this, but qProcessInfo // will return what is in the binary. - auto info = - procSP->GetDeploymentInfo(lc, load_command_address, is_executable); + auto info = procSP->GetDeploymentInfo(dyld_platform, lc, + load_command_address, is_executable); major_version = info.major_version; minor_version = info.minor_version; patch_version = info.patch_version; @@ -1521,6 +1521,13 @@ DNBGetDeploymentInfo(nub_process_t pid, bool is_executable, return {}; } +uint32_t DNBGetPlatform(nub_process_t pid) { + MachProcessSP procSP; + if (GetProcessSP(pid, procSP)) + return procSP->GetPlatform(); + return 0; +} + // Get the current shared library information for a process. Only return // the shared libraries that have changed since the last shared library // state changed event if only_changed is non-zero. diff --git a/lldb/tools/debugserver/source/DNB.h b/lldb/tools/debugserver/source/DNB.h index 10d1f68794355..5475e691c7a93 100644 --- a/lldb/tools/debugserver/source/DNB.h +++ b/lldb/tools/debugserver/source/DNB.h @@ -52,15 +52,15 @@ nub_process_t DNBProcessLaunch( nub_process_t DNBProcessGetPIDByName(const char *name); nub_process_t DNBProcessAttach(nub_process_t pid, struct timespec *timeout, - const RNBContext::IgnoredExceptions - &ignored_exceptions, - char *err_str, - size_t err_len); + const RNBContext::IgnoredExceptions +&ignored_exceptions, + char *err_str, +size_t err_len); nub_process_t DNBProcessAttachByName(const char *name, struct timespec *timeout, - const RNBContext::IgnoredExceptions - &ignored_exceptions, - char *err_str, - size_t err_len); + const RNBContext::IgnoredExceptions +&ignored_exceptions, + char *err_str, +size_t err_len); nub_process_t DNBProcessAttachWait(RNBContext *ctx, const char *wait_name, bool ignore_existing, struct timespec *timeout, @@ -137,9 +137,10 @@ DNBProcessGetSharedLibraryInfo(nub_process_t pid, nub_bool_t only_changed, DNBExecutableImageInfo **image_infos) DNB_EXPORT; std::optional<std::string> DNBGetDeploymentInfo(nub_process_t pid, bool is_executable, - const struct load_command &lc, + uint32_t dyld_platform, const struct load_command &lc, uint64_t load_command_address, uint32_t &major_version, uint32_t &minor_version, uint32_t &patch_version); +uint32_t DNBGetPlatform(nub_process_t pid); nub_bool_t DNBProcessSetNameToAddressCallback(nub_process_t pid, DNBCallbackNameToAddress callback, void *baton) DNB_EXPORT; diff --git a/lldb/tools/debugserver/source/MacOSX/MachProcess.h b/lldb/tools/debugserver/source/MacOSX/MachProcess.h index 56bc9d6c7461e..453130ddeab45 100644 --- a/lldb/tools/debugserver/source/MacOSX/MachProcess.h +++ b/lldb/tools/debugserver/source/MacOSX/MachProcess.h @@ -255,7 +255,8 @@ class MachProcess { uint32_t minor_version = 0; uint32_t patch_version = 0; }; - DeploymentInfo GetDeploymentInfo(const struct load_command &, + DeploymentInfo GetDeploymentInfo(uint32_t dyld_platform, + const struct load_command &, uint64_t load_command_address, bool is_executable); static std::optional<std::string> GetPlatformString(unsigned char platform); diff --git a/lldb/tools/debugserver/source/MacOSX/MachProcess.mm b/lldb/tools/debugserver/source/MacOSX/MachProcess.mm index 3afaaa2f64c00..c72947598f760 100644 --- a/lldb/tools/debugserver/source/MacOSX/MachProcess.mm +++ b/lldb/tools/debugserver/source/MacOSX/MachProcess.mm @@ -638,10 +638,9 @@ static bool FBSAddEventDataToOptions(NSMutableDictionary *options, plo_pthread_tsd_entry_size); } -MachProcess::DeploymentInfo -MachProcess::GetDeploymentInfo(const struct load_command &lc, - uint64_t load_command_address, - bool is_executable) { +MachProcess::DeploymentInfo MachProcess::GetDeploymentInfo( + uint32_t dyld_platform, const struct load_command &lc, + uint64_t load_command_address, bool is_executable) { DeploymentInfo info; uint32_t cmd = lc.cmd & ~LC_REQ_DYLD; @@ -712,7 +711,7 @@ static bool FBSAddEventDataToOptions(NSMutableDictionary *options, // DYLD_FORCE_PLATFORM=6. In that case, force the platform to // macCatalyst and use the macCatalyst version of the host OS // instead of the macOS deployment target. - if (is_executable && GetPlatform() == PLATFORM_MACCATALYST) { + if (is_executable && dyld_platform == PLATFORM_MACCATALYST) { info.platform = PLATFORM_MACCATALYST; std::string catalyst_version = GetMacCatalystVersionString(); const char *major = catalyst_version.c_str(); @@ -883,8 +882,9 @@ static bool mach_header_validity_test(uint32_t magic, uint32_t cputype) { sizeof(struct uuid_command)) uuid_copy(inf.uuid, uuidcmd.uuid); } - if (DeploymentInfo deployment_info = GetDeploymentInfo( - lc, load_cmds_p, inf.mach_header.filetype == MH_EXECUTE)) { + if (DeploymentInfo deployment_info = + GetDeploymentInfo(dyld_platform, lc, load_cmds_p, + inf.mach_header.filetype == MH_EXECUTE)) { std::optional<std::string> lc_platform = GetPlatformString(deployment_info.platform); if (dyld_platform != PLATFORM_MACCATALYST && diff --git a/lldb/tools/debugserver/source/RNBRemote.cpp b/lldb/tools/debugserver/source/RNBRemote.cpp index e0831023e7ae4..47c73571360b9 100644 --- a/lldb/tools/debugserver/source/RNBRemote.cpp +++ b/lldb/tools/debugserver/source/RNBRemote.cpp @@ -6231,6 +6231,7 @@ rnb_err_t RNBRemote::HandlePacket_qProcessInfo(const char *p) { nub_addr_t exe_mach_header_addr = GetMachHeaderForMainExecutable(pid, addr_size, mh); if (exe_mach_header_addr != INVALID_NUB_ADDRESS) { + uint32_t dyld_platform = DNBGetPlatform(pid); uint64_t load_command_addr = exe_mach_header_addr + ((addr_size == 8) ? sizeof(mach_header_64) : sizeof(mach_header)); @@ -6242,9 +6243,9 @@ rnb_err_t RNBRemote::HandlePacket_qProcessInfo(const char *p) { bool is_executable = true; uint32_t major_version, minor_version, patch_version; - std::optional<std::string> platform = - DNBGetDeploymentInfo(pid, is_executable, lc, load_command_addr, - major_version, minor_version, patch_version); + std::optional<std::string> platform = DNBGetDeploymentInfo( + pid, is_executable, dyld_platform, lc, load_command_addr, + major_version, minor_version, patch_version); if (platform) { os_handled = true; rep << "ostype:" << *platform << ";"; `````````` </details> https://github.com/llvm/llvm-project/pull/140853 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
