[Lldb-commits] [lldb] [lldb] [debugserver] address preprocessor warning, extra arg (PR #90808)

Jason Molenda via lldb-commits lldb-commits at lists.llvm.org
Wed May 1 18:12:35 PDT 2024


https://github.com/jasonmolenda created https://github.com/llvm/llvm-project/pull/90808

In DNBArchImplARM64.cpp I'm doing

And the preprocessor warns that this is not defined behavior.  This checks if ptrauth_calls is available and if this is being compiled 64-bit (i.e. arm64e), and defines a single DEBUGSERVER_IS_ARM64E when those are both true.

I did have to duplicate one DNBLogThreaded() call which itself is a macro, and using an ifdef in the middle of macro arguments also got me a warning from the preprocessor.

While testing this for all the different targets, I found a DNBError initialization that accepts a c-string but I'm passing in a printf-style formatter c-string and an argument.  Create the string before the call and pass in the constructed string.

rdar://127129242

>From 8145e9faaa52209f9800d473fb75f7cfbd2a1185 Mon Sep 17 00:00:00 2001
From: Jason Molenda <jmolenda at apple.com>
Date: Wed, 1 May 2024 18:06:50 -0700
Subject: [PATCH] [lldb] [debugserver] address preprocessor warning, extra arg

In DNBArchImplARM64.cpp I'm doing

And the preprocessor warns that this is not defined behavior.  This
checks if ptrauth_calls is available and if this is being compiled
64-bit (i.e. arm64e), and defines a single DEBUGSERVER_IS_ARM64E
when those are both true.

I did have to duplicate one DNBLogThreaded() call which itself is a
macro, and using an ifdef in the middle of macro arguments also got
me a warning from the preprocessor.

While testing this for all the different targets, I found a DNBError
initialization that accepts a c-string but I'm passing in a
printf-style formatter c-string and an argument.  Create the string
before the call and pass in the constructed string.

rdar://127129242
---
 .../debugserver/source/MacOSX/MachProcess.mm  |  8 ++---
 .../source/MacOSX/arm64/DNBArchImplARM64.cpp  | 31 ++++++++++++-------
 2 files changed, 23 insertions(+), 16 deletions(-)

diff --git a/lldb/tools/debugserver/source/MacOSX/MachProcess.mm b/lldb/tools/debugserver/source/MacOSX/MachProcess.mm
index 70b4564a027b1b..cbe3c5459e91fc 100644
--- a/lldb/tools/debugserver/source/MacOSX/MachProcess.mm
+++ b/lldb/tools/debugserver/source/MacOSX/MachProcess.mm
@@ -4070,10 +4070,10 @@ static CFStringRef CopyBundleIDForPath(const char *app_bundle_path,
       m_flags |= eMachProcessFlagsAttached;
       DNBLog("[LaunchAttach] successfully attached to pid %d", m_pid);
     } else {
-      launch_err.SetErrorString(
-          "Failed to attach to pid %d, BoardServiceLaunchForDebug() unable to "
-          "ptrace(PT_ATTACHEXC)",
-          m_pid);
+      std::string errmsg = "Failed to attach to pid ";
+      errmsg += std::to_string(m_pid);
+      errmsg += ", BoardServiceLaunchForDebug() unable to ptrace(PT_ATTACHEXC)";
+      launch_err.SetErrorString(errmsg.c_str());
       SetState(eStateExited);
       DNBLog("[LaunchAttach] END (%d) error: failed to attach to pid %d",
              getpid(), m_pid);
diff --git a/lldb/tools/debugserver/source/MacOSX/arm64/DNBArchImplARM64.cpp b/lldb/tools/debugserver/source/MacOSX/arm64/DNBArchImplARM64.cpp
index 57dd2dce6bf5ad..3b1147c69c666b 100644
--- a/lldb/tools/debugserver/source/MacOSX/arm64/DNBArchImplARM64.cpp
+++ b/lldb/tools/debugserver/source/MacOSX/arm64/DNBArchImplARM64.cpp
@@ -26,8 +26,12 @@
 #include <cinttypes>
 #include <sys/sysctl.h>
 
+#undef DEBUGSERVER_IS_ARM64E
 #if __has_feature(ptrauth_calls)
 #include <ptrauth.h>
+#if defined(__LP64__)
+#define DEBUGSERVER_IS_ARM64E 1
+#endif
 #endif
 
 // Break only in privileged or user mode
@@ -115,7 +119,7 @@ static uint64_t clear_pac_bits(uint64_t value) {
 uint64_t DNBArchMachARM64::GetPC(uint64_t failValue) {
   // Get program counter
   if (GetGPRState(false) == KERN_SUCCESS)
-#if __has_feature(ptrauth_calls) && defined(__LP64__)
+#if defined(DEBUGSERVER_IS_ARM64E)
     return clear_pac_bits(
         reinterpret_cast<uint64_t>(m_state.context.gpr.__opaque_pc));
 #else
@@ -147,7 +151,7 @@ kern_return_t DNBArchMachARM64::SetPC(uint64_t value) {
 uint64_t DNBArchMachARM64::GetSP(uint64_t failValue) {
   // Get stack pointer
   if (GetGPRState(false) == KERN_SUCCESS)
-#if __has_feature(ptrauth_calls) && defined(__LP64__)
+#if defined(DEBUGSERVER_IS_ARM64E)
     return clear_pac_bits(
         reinterpret_cast<uint64_t>(m_state.context.gpr.__opaque_sp));
 #else
@@ -169,25 +173,28 @@ kern_return_t DNBArchMachARM64::GetGPRState(bool force) {
                          (thread_state_t)&m_state.context.gpr, &count);
   if (DNBLogEnabledForAny(LOG_THREAD)) {
     uint64_t *x = &m_state.context.gpr.__x[0];
+
+#if defined(DEBUGSERVER_IS_ARM64E)
     DNBLogThreaded("thread_get_state signed regs "
                    "\n   fp=%16.16llx"
                    "\n   lr=%16.16llx"
                    "\n   sp=%16.16llx"
                    "\n   pc=%16.16llx",
-#if __has_feature(ptrauth_calls) && defined(__LP64__)
                    reinterpret_cast<uint64_t>(m_state.context.gpr.__opaque_fp),
                    reinterpret_cast<uint64_t>(m_state.context.gpr.__opaque_lr),
                    reinterpret_cast<uint64_t>(m_state.context.gpr.__opaque_sp),
-                   reinterpret_cast<uint64_t>(m_state.context.gpr.__opaque_pc)
+                   reinterpret_cast<uint64_t>(m_state.context.gpr.__opaque_pc));
 #else
-                   m_state.context.gpr.__fp,
-                   m_state.context.gpr.__lr, 
-                   m_state.context.gpr.__sp,
-                   m_state.context.gpr.__pc
+    DNBLogThreaded("thread_get_state signed regs "
+                   "\n   fp=%16.16llx"
+                   "\n   lr=%16.16llx"
+                   "\n   sp=%16.16llx"
+                   "\n   pc=%16.16llx",
+                   m_state.context.gpr.__fp, m_state.context.gpr.__lr,
+                   m_state.context.gpr.__sp, m_state.context.gpr.__pc);
 #endif
-    );
 
-#if __has_feature(ptrauth_calls) && defined(__LP64__)
+#if defined(DEBUGSERVER_IS_ARM64E)
     uint64_t log_fp = clear_pac_bits(
         reinterpret_cast<uint64_t>(m_state.context.gpr.__opaque_fp));
     uint64_t log_lr = clear_pac_bits(
@@ -661,7 +668,7 @@ kern_return_t DNBArchMachARM64::EnableHardwareSingleStep(bool enable) {
     return err.Status();
   }
 
-#if __has_feature(ptrauth_calls) && defined(__LP64__)
+#if defined(DEBUGSERVER_IS_ARM64E)
   uint64_t pc = clear_pac_bits(
       reinterpret_cast<uint64_t>(m_state.context.gpr.__opaque_pc));
 #else
@@ -2187,7 +2194,7 @@ bool DNBArchMachARM64::GetRegisterValue(uint32_t set, uint32_t reg,
     case e_regSetGPR:
       if (reg <= gpr_pc) {
         switch (reg) {
-#if __has_feature(ptrauth_calls) && defined(__LP64__)
+#if defined(DEBUGSERVER_IS_ARM64E)
         case gpr_pc:
           value->value.uint64 = clear_pac_bits(
               reinterpret_cast<uint64_t>(m_state.context.gpr.__opaque_pc));



More information about the lldb-commits mailing list