llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-lldb Author: Adrian Prantl (adrian-prantl) <details> <summary>Changes</summary> This reapplies [Replace ArchSpec::PiecewiseCompare() with Triple::operator==()](https://github.com/llvm/llvm-project/commit/5c9e53d45ba948b8a5e8416aa9b3322c87fc46c8), with an additional fix in debugserver. [Change debugserver to report the cpu(sub)type of process, not the host.](https://github.com/llvm/llvm-project/commit/3ca8611705613f4c483e34c75dfa1dfad1572f77) This way debugserver can correctly report qProcessInfo for arm64 processes on arm64e-capable hosts. Patch implemented with help from Jason Molenda! --- Full diff: https://github.com/llvm/llvm-project/pull/82938.diff 11 Files Affected: - (modified) lldb/include/lldb/Utility/ArchSpec.h (-5) - (modified) lldb/source/Target/Target.cpp (+9-7) - (modified) lldb/source/Utility/ArchSpec.cpp (-17) - (added) lldb/test/API/macosx/arm64e-attach/Makefile (+7) - (added) lldb/test/API/macosx/arm64e-attach/TestArm64eAttach.py (+38) - (added) lldb/test/API/macosx/arm64e-attach/main.c (+2) - (modified) lldb/tools/debugserver/source/DNB.cpp (+8) - (modified) lldb/tools/debugserver/source/DNB.h (+2) - (modified) lldb/tools/debugserver/source/MacOSX/MachProcess.h (+2) - (modified) lldb/tools/debugserver/source/MacOSX/MachProcess.mm (+17) - (modified) lldb/tools/debugserver/source/RNBRemote.cpp (+58-48) ``````````diff diff --git a/lldb/include/lldb/Utility/ArchSpec.h b/lldb/include/lldb/Utility/ArchSpec.h index a226a3a5a9b71d..50830b889b9115 100644 --- a/lldb/include/lldb/Utility/ArchSpec.h +++ b/lldb/include/lldb/Utility/ArchSpec.h @@ -505,11 +505,6 @@ class ArchSpec { bool IsFullySpecifiedTriple() const; - void PiecewiseTripleCompare(const ArchSpec &other, bool &arch_different, - bool &vendor_different, bool &os_different, - bool &os_version_different, - bool &env_different) const; - /// Detect whether this architecture uses thumb code exclusively /// /// Some embedded ARM chips (e.g. the ARM Cortex M0-7 line) can only execute diff --git a/lldb/source/Target/Target.cpp b/lldb/source/Target/Target.cpp index e17bfcb5d5e2ad..089915cab4915a 100644 --- a/lldb/source/Target/Target.cpp +++ b/lldb/source/Target/Target.cpp @@ -33,6 +33,7 @@ #include "lldb/Expression/UtilityFunction.h" #include "lldb/Host/Host.h" #include "lldb/Host/PosixApi.h" +#include "lldb/Host/SafeMachO.h" #include "lldb/Host/StreamFile.h" #include "lldb/Interpreter/CommandInterpreter.h" #include "lldb/Interpreter/CommandReturnObject.h" @@ -1568,15 +1569,16 @@ bool Target::SetArchitecture(const ArchSpec &arch_spec, bool set_platform, if (m_arch.GetSpec().IsCompatibleMatch(other)) { compatible_local_arch = true; - bool arch_changed, vendor_changed, os_changed, os_ver_changed, - env_changed; - m_arch.GetSpec().PiecewiseTripleCompare(other, arch_changed, - vendor_changed, os_changed, - os_ver_changed, env_changed); - - if (!arch_changed && !vendor_changed && !os_changed && !env_changed) + if (m_arch.GetSpec().GetTriple() == other.GetTriple()) replace_local_arch = false; + // Workaround for for pre-2024 debugserver, which always + // returns arm64e on arm64e-capable hardware regardless of + // what the process is. This can be deleted at some point in + // the future. + if (!m_arch.GetSpec().GetMachOCPUSubType() && + other.GetMachOCPUSubType() == llvm::MachO::CPU_SUBTYPE_ARM64E) + replace_local_arch = true; } } } diff --git a/lldb/source/Utility/ArchSpec.cpp b/lldb/source/Utility/ArchSpec.cpp index fb0e985a0d5657..07ef435ef451d2 100644 --- a/lldb/source/Utility/ArchSpec.cpp +++ b/lldb/source/Utility/ArchSpec.cpp @@ -1421,23 +1421,6 @@ bool ArchSpec::IsFullySpecifiedTriple() const { return true; } -void ArchSpec::PiecewiseTripleCompare( - const ArchSpec &other, bool &arch_different, bool &vendor_different, - bool &os_different, bool &os_version_different, bool &env_different) const { - const llvm::Triple &me(GetTriple()); - const llvm::Triple &them(other.GetTriple()); - - arch_different = (me.getArch() != them.getArch()); - - vendor_different = (me.getVendor() != them.getVendor()); - - os_different = (me.getOS() != them.getOS()); - - os_version_different = (me.getOSMajorVersion() != them.getOSMajorVersion()); - - env_different = (me.getEnvironment() != them.getEnvironment()); -} - bool ArchSpec::IsAlwaysThumbInstructions() const { std::string Status; if (GetTriple().getArch() == llvm::Triple::arm || diff --git a/lldb/test/API/macosx/arm64e-attach/Makefile b/lldb/test/API/macosx/arm64e-attach/Makefile new file mode 100644 index 00000000000000..248a93e0324883 --- /dev/null +++ b/lldb/test/API/macosx/arm64e-attach/Makefile @@ -0,0 +1,7 @@ +C_SOURCES := main.c + +# Uncomment this for q&d local debugging. +all: + xcrun clang -g $(SRCDIR)/main.c -o a.out -target arm64e-apple-macosx + +include Makefile.rules diff --git a/lldb/test/API/macosx/arm64e-attach/TestArm64eAttach.py b/lldb/test/API/macosx/arm64e-attach/TestArm64eAttach.py new file mode 100644 index 00000000000000..8abb7ae3bec0d3 --- /dev/null +++ b/lldb/test/API/macosx/arm64e-attach/TestArm64eAttach.py @@ -0,0 +1,38 @@ +import lldb +from lldbsuite.test.decorators import * +from lldbsuite.test.lldbtest import * +from lldbsuite.test import lldbutil + + +class TestArm64eAttach(TestBase): + NO_DEBUG_INFO_TESTCASE = True + + # On Darwin systems, arch arm64e means ARMv8.3 with ptrauth ABI used. + @skipIf(archs=no_match(["arm64e"])) + def test(self): + # Skip this test if not running on AArch64 target that supports PAC + if not self.isAArch64PAuth(): + self.skipTest("Target must support pointer authentication.") + + packets = self.getBuildArtifact('packet.log') + self.expect('log enable -f "%s" gdb-remote packets' % packets) + + self.build() + popen = self.spawnSubprocess(self.getBuildArtifact(), []) + + # This simulates how Xcode attaches to a process by pid/name. + error = lldb.SBError() + target = self.dbg.CreateTarget("", "arm64", "", True, error) + listener = lldb.SBListener("my.attach.listener") + process = target.AttachToProcessWithID(listener, popen.pid, error) + self.assertSuccess(error) + self.assertTrue(process, PROCESS_IS_VALID) + self.assertEqual(target.GetTriple().split('-')[0], "arm64e", + "target triple is updated correctly") + error = process.Kill() + self.assertSuccess(error) + + # Test debugserver behavior. + self.filecheck('platform shell cat "%s"' % packets, __file__) + # CHECK: cputype:100000c;cpusubtype:2;ptrsize:8;ostype:macosx;vendor:apple;endian:little; + diff --git a/lldb/test/API/macosx/arm64e-attach/main.c b/lldb/test/API/macosx/arm64e-attach/main.c new file mode 100644 index 00000000000000..7baf2ffd8f2899 --- /dev/null +++ b/lldb/test/API/macosx/arm64e-attach/main.c @@ -0,0 +1,2 @@ +int getchar(); +int main() { return getchar(); } diff --git a/lldb/tools/debugserver/source/DNB.cpp b/lldb/tools/debugserver/source/DNB.cpp index 0ec50df42d1fed..1ac9a8040c6163 100644 --- a/lldb/tools/debugserver/source/DNB.cpp +++ b/lldb/tools/debugserver/source/DNB.cpp @@ -1062,6 +1062,14 @@ DNBGetTSDAddressForThread(nub_process_t pid, nub_thread_t tid, return INVALID_NUB_ADDRESS; } +std::optional<std::pair<cpu_type_t, cpu_subtype_t>> +DNBGetMainBinaryCPUTypes(nub_process_t pid) { + MachProcessSP procSP; + if (GetProcessSP(pid, procSP)) + return procSP->GetMainBinaryCPUTypes(pid); + return {}; +} + JSONGenerator::ObjectSP DNBGetAllLoadedLibrariesInfos(nub_process_t pid, bool report_load_commands) { MachProcessSP procSP; diff --git a/lldb/tools/debugserver/source/DNB.h b/lldb/tools/debugserver/source/DNB.h index 97de83ef9ff80d..10d1f687943556 100644 --- a/lldb/tools/debugserver/source/DNB.h +++ b/lldb/tools/debugserver/source/DNB.h @@ -210,6 +210,8 @@ DNBGetTSDAddressForThread(nub_process_t pid, nub_thread_t tid, uint64_t plo_pthread_tsd_base_address_offset, uint64_t plo_pthread_tsd_base_offset, uint64_t plo_pthread_tsd_entry_size); +std::optional<std::pair<cpu_type_t, cpu_subtype_t>> +DNBGetMainBinaryCPUTypes(nub_process_t pid); JSONGenerator::ObjectSP DNBGetAllLoadedLibrariesInfos(nub_process_t pid, bool report_load_commands); JSONGenerator::ObjectSP diff --git a/lldb/tools/debugserver/source/MacOSX/MachProcess.h b/lldb/tools/debugserver/source/MacOSX/MachProcess.h index 8432bdb36c0cf8..db673693a1b21a 100644 --- a/lldb/tools/debugserver/source/MacOSX/MachProcess.h +++ b/lldb/tools/debugserver/source/MacOSX/MachProcess.h @@ -103,6 +103,8 @@ class MachProcess { const char *stdin_path, const char *stdout_path, const char *stderr_path, bool no_stdio, MachProcess *process, int disable_aslr, DNBError &err); nub_addr_t GetDYLDAllImageInfosAddress(); + std::optional<std::pair<cpu_type_t, cpu_subtype_t>> + GetMainBinaryCPUTypes(nub_process_t pid); static const void *PrepareForAttach(const char *path, nub_launch_flavor_t launch_flavor, bool waitfor, DNBError &err_str); diff --git a/lldb/tools/debugserver/source/MacOSX/MachProcess.mm b/lldb/tools/debugserver/source/MacOSX/MachProcess.mm index 3a4dfb9ea6ea22..87bdbf835bfd10 100644 --- a/lldb/tools/debugserver/source/MacOSX/MachProcess.mm +++ b/lldb/tools/debugserver/source/MacOSX/MachProcess.mm @@ -1111,6 +1111,23 @@ static bool mach_header_validity_test(uint32_t magic, uint32_t cputype) { return FormatDynamicLibrariesIntoJSON(image_infos, report_load_commands); } +std::optional<std::pair<cpu_type_t, cpu_subtype_t>> +MachProcess::GetMainBinaryCPUTypes(nub_process_t pid) { + int pointer_size = GetInferiorAddrSize(pid); + std::vector<struct binary_image_information> image_infos; + GetAllLoadedBinariesViaDYLDSPI(image_infos); + uint32_t platform = GetPlatform(); + for (auto &image_info : image_infos) + if (GetMachOInformationFromMemory(platform, image_info.load_address, + pointer_size, image_info.macho_info)) + if (image_info.macho_info.mach_header.filetype == MH_EXECUTE) + return { + {static_cast<cpu_type_t>(image_info.macho_info.mach_header.cputype), + static_cast<cpu_subtype_t>( + image_info.macho_info.mach_header.cpusubtype)}}; + return {}; +} + // Fetch information about the shared libraries at the given load addresses // using the // dyld SPIs that exist in macOS 10.12, iOS 10, tvOS 10, watchOS 3 and newer. diff --git a/lldb/tools/debugserver/source/RNBRemote.cpp b/lldb/tools/debugserver/source/RNBRemote.cpp index feea4c914ec536..03d427d3fc5954 100644 --- a/lldb/tools/debugserver/source/RNBRemote.cpp +++ b/lldb/tools/debugserver/source/RNBRemote.cpp @@ -6195,59 +6195,14 @@ rnb_err_t RNBRemote::HandlePacket_qSymbol(const char *command) { } } -// Note that all numeric values returned by qProcessInfo are hex encoded, -// including the pid and the cpu type. - -rnb_err_t RNBRemote::HandlePacket_qProcessInfo(const char *p) { - nub_process_t pid; - std::ostringstream rep; - - // If we haven't run the process yet, return an error. - if (!m_ctx.HasValidProcessID()) - return SendPacket("E68"); - - pid = m_ctx.ProcessID(); - - rep << "pid:" << std::hex << pid << ';'; - - int procpid_mib[4]; - procpid_mib[0] = CTL_KERN; - procpid_mib[1] = KERN_PROC; - procpid_mib[2] = KERN_PROC_PID; - procpid_mib[3] = pid; - struct kinfo_proc proc_kinfo; - size_t proc_kinfo_size = sizeof(struct kinfo_proc); - - if (::sysctl(procpid_mib, 4, &proc_kinfo, &proc_kinfo_size, NULL, 0) == 0) { - if (proc_kinfo_size > 0) { - rep << "parent-pid:" << std::hex << proc_kinfo.kp_eproc.e_ppid << ';'; - rep << "real-uid:" << std::hex << proc_kinfo.kp_eproc.e_pcred.p_ruid - << ';'; - rep << "real-gid:" << std::hex << proc_kinfo.kp_eproc.e_pcred.p_rgid - << ';'; - rep << "effective-uid:" << std::hex << proc_kinfo.kp_eproc.e_ucred.cr_uid - << ';'; - if (proc_kinfo.kp_eproc.e_ucred.cr_ngroups > 0) - rep << "effective-gid:" << std::hex - << proc_kinfo.kp_eproc.e_ucred.cr_groups[0] << ';'; - } - } - +static std::pair<cpu_type_t, cpu_subtype_t> +GetCPUTypesFromHost(nub_process_t pid) { cpu_type_t cputype = DNBProcessGetCPUType(pid); if (cputype == 0) { DNBLog("Unable to get the process cpu_type, making a best guess."); cputype = best_guess_cpu_type(); } - uint32_t addr_size = 0; - if (cputype != 0) { - rep << "cputype:" << std::hex << cputype << ";"; - if (cputype & CPU_ARCH_ABI64) - addr_size = 8; - else - addr_size = 4; - } - bool host_cpu_is_64bit = false; uint32_t is64bit_capable; size_t is64bit_capable_len = sizeof(is64bit_capable); @@ -6288,14 +6243,69 @@ rnb_err_t RNBRemote::HandlePacket_qProcessInfo(const char *p) { if (cputype == CPU_TYPE_ARM64_32 && cpusubtype == 2) cpusubtype = CPU_SUBTYPE_ARM64_32_V8; #endif + } + + return {cputype, cpusubtype}; +} + +// Note that all numeric values returned by qProcessInfo are hex encoded, +// including the pid and the cpu type. +rnb_err_t RNBRemote::HandlePacket_qProcessInfo(const char *p) { + nub_process_t pid; + std::ostringstream rep; + + // If we haven't run the process yet, return an error. + if (!m_ctx.HasValidProcessID()) + return SendPacket("E68"); + + pid = m_ctx.ProcessID(); + + rep << "pid:" << std::hex << pid << ';'; + + int procpid_mib[4]; + procpid_mib[0] = CTL_KERN; + procpid_mib[1] = KERN_PROC; + procpid_mib[2] = KERN_PROC_PID; + procpid_mib[3] = pid; + struct kinfo_proc proc_kinfo; + size_t proc_kinfo_size = sizeof(struct kinfo_proc); + + if (::sysctl(procpid_mib, 4, &proc_kinfo, &proc_kinfo_size, NULL, 0) == 0) { + if (proc_kinfo_size > 0) { + rep << "parent-pid:" << std::hex << proc_kinfo.kp_eproc.e_ppid << ';'; + rep << "real-uid:" << std::hex << proc_kinfo.kp_eproc.e_pcred.p_ruid + << ';'; + rep << "real-gid:" << std::hex << proc_kinfo.kp_eproc.e_pcred.p_rgid + << ';'; + rep << "effective-uid:" << std::hex << proc_kinfo.kp_eproc.e_ucred.cr_uid + << ';'; + if (proc_kinfo.kp_eproc.e_ucred.cr_ngroups > 0) + rep << "effective-gid:" << std::hex + << proc_kinfo.kp_eproc.e_ucred.cr_groups[0] << ';'; + } + } + + cpu_type_t cputype; + cpu_subtype_t cpusubtype; + if (auto cputypes = DNBGetMainBinaryCPUTypes(pid)) + std::tie(cputype, cpusubtype) = *cputypes; + else + std::tie(cputype, cpusubtype) = GetCPUTypesFromHost(pid); + + uint32_t addr_size = 0; + if (cputype != 0) { + rep << "cputype:" << std::hex << cputype << ";"; rep << "cpusubtype:" << std::hex << cpusubtype << ';'; + if (cputype & CPU_ARCH_ABI64) + addr_size = 8; + else + addr_size = 4; } bool os_handled = false; if (addr_size > 0) { rep << "ptrsize:" << std::dec << addr_size << ';'; - #if defined(TARGET_OS_OSX) && TARGET_OS_OSX == 1 // Try and get the OS type by looking at the load commands in the main // executable and looking for a LC_VERSION_MIN load command. This is the `````````` </details> https://github.com/llvm/llvm-project/pull/82938 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
