[Lldb-commits] [lldb] Change debugserver to report the cpu(sub)type of process, not the host. (PR #82938)
Adrian Prantl via lldb-commits
lldb-commits at lists.llvm.org
Sun Feb 25 15:15:37 PST 2024
https://github.com/adrian-prantl updated https://github.com/llvm/llvm-project/pull/82938
>From 5c9e53d45ba948b8a5e8416aa9b3322c87fc46c8 Mon Sep 17 00:00:00 2001
From: Adrian Prantl <aprantl at apple.com>
Date: Fri, 23 Feb 2024 09:58:17 -0800
Subject: [PATCH 1/2] Replace ArchSpec::PiecewiseCompare() with
Triple::operator==()
Looking ast the definition of both functions this is *almost* an NFC
change, except that Triple also looks at the SubArch (important) and
ObjectFormat (less so).
This fixes a bug that only manifests with how Xcode uses the SBAPI to
attach to a process by name: it guesses the architecture based on the
system. If the system is arm64 and the Process is arm64e Target fails
to update the triple because it deemed the two to be equivalent.
rdar://123338218
---
lldb/include/lldb/Utility/ArchSpec.h | 5 ----
lldb/source/Target/Target.cpp | 8 +-----
lldb/source/Utility/ArchSpec.cpp | 17 -----------
lldb/test/API/macosx/arm64e-attach/Makefile | 2 ++
.../macosx/arm64e-attach/TestArm64eAttach.py | 28 +++++++++++++++++++
lldb/test/API/macosx/arm64e-attach/main.c | 2 ++
6 files changed, 33 insertions(+), 29 deletions(-)
create mode 100644 lldb/test/API/macosx/arm64e-attach/Makefile
create mode 100644 lldb/test/API/macosx/arm64e-attach/TestArm64eAttach.py
create mode 100644 lldb/test/API/macosx/arm64e-attach/main.c
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..e982a30a3ae4ff 100644
--- a/lldb/source/Target/Target.cpp
+++ b/lldb/source/Target/Target.cpp
@@ -1568,14 +1568,8 @@ 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;
}
}
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..c9319d6e6888a4
--- /dev/null
+++ b/lldb/test/API/macosx/arm64e-attach/Makefile
@@ -0,0 +1,2 @@
+C_SOURCES := main.c
+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..0dc8700ed02dd8
--- /dev/null
+++ b/lldb/test/API/macosx/arm64e-attach/TestArm64eAttach.py
@@ -0,0 +1,28 @@
+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.")
+ self.build()
+ popen = self.spawnSubprocess(self.getBuildArtifact(), [])
+ error = lldb.SBError()
+ # This simulates how Xcode attaches to a process by pid/name.
+ 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)
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(); }
>From aecc7f751328a11c12bb910b1e30cab41b98a146 Mon Sep 17 00:00:00 2001
From: Adrian Prantl <aprantl at apple.com>
Date: Sun, 25 Feb 2024 15:08:21 -0800
Subject: [PATCH 2/2] Change debugserver to report the cpu(sub)type of process,
not the host.
This way debugserver can correctly report qProcessInfo for arm64
processes on arm64e-capable hosts.
Patch implemented with help from Jason Molenda!
---
lldb/source/Target/Target.cpp | 8 ++
lldb/test/API/macosx/arm64e-attach/Makefile | 5 +
.../macosx/arm64e-attach/TestArm64eAttach.py | 12 +-
lldb/tools/debugserver/source/DNB.cpp | 8 ++
lldb/tools/debugserver/source/DNB.h | 2 +
.../debugserver/source/MacOSX/MachProcess.h | 2 +
.../debugserver/source/MacOSX/MachProcess.mm | 17 +++
lldb/tools/debugserver/source/RNBRemote.cpp | 106 ++++++++++--------
8 files changed, 111 insertions(+), 49 deletions(-)
diff --git a/lldb/source/Target/Target.cpp b/lldb/source/Target/Target.cpp
index e982a30a3ae4ff..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"
@@ -1571,6 +1572,13 @@ bool Target::SetArchitecture(const ArchSpec &arch_spec, bool set_platform,
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/test/API/macosx/arm64e-attach/Makefile b/lldb/test/API/macosx/arm64e-attach/Makefile
index c9319d6e6888a4..a2465b51ea1571 100644
--- a/lldb/test/API/macosx/arm64e-attach/Makefile
+++ b/lldb/test/API/macosx/arm64e-attach/Makefile
@@ -1,2 +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
index 0dc8700ed02dd8..8abb7ae3bec0d3 100644
--- a/lldb/test/API/macosx/arm64e-attach/TestArm64eAttach.py
+++ b/lldb/test/API/macosx/arm64e-attach/TestArm64eAttach.py
@@ -13,10 +13,15 @@ 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(), [])
- error = lldb.SBError()
+
# 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)
@@ -26,3 +31,8 @@ def test(self):
"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/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
More information about the lldb-commits
mailing list