[Lldb-commits] [lldb] Change debugserver to report the cpu(sub)type of process, not the host. (PR #82938)
via lldb-commits
lldb-commits at lists.llvm.org
Sun Feb 25 15:15:23 PST 2024
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
More information about the lldb-commits
mailing list