[Lldb-commits] [lldb] [lldb] [debugserver] address preprocessor warning, extra arg (PR #90808)
https://github.com/jasonmolenda closed https://github.com/llvm/llvm-project/pull/90808 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] [debugserver] address preprocessor warning, extra arg (PR #90808)
https://github.com/JDevlieghere approved this pull request. https://github.com/llvm/llvm-project/pull/90808 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] [debugserver] address preprocessor warning, extra arg (PR #90808)
@@ -169,25 +173,28 @@ kern_return_t DNBArchMachARM64::GetGPRState(bool force) {
(thread_state_t)_state.context.gpr, );
if (DNBLogEnabledForAny(LOG_THREAD)) {
uint64_t *x = _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(m_state.context.gpr.__opaque_fp),
reinterpret_cast(m_state.context.gpr.__opaque_lr),
reinterpret_cast(m_state.context.gpr.__opaque_sp),
- reinterpret_cast(m_state.context.gpr.__opaque_pc)
+
reinterpret_cast(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);
jasonmolenda wrote:
good idea, done.
https://github.com/llvm/llvm-project/pull/90808
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] [debugserver] address preprocessor warning, extra arg (PR #90808)
https://github.com/jasonmolenda updated
https://github.com/llvm/llvm-project/pull/90808
>From 8145e9faaa52209f9800d473fb75f7cfbd2a1185 Mon Sep 17 00:00:00 2001
From: Jason Molenda
Date: Wed, 1 May 2024 18:06:50 -0700
Subject: [PATCH 1/2] [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
#include
+#undef DEBUGSERVER_IS_ARM64E
#if __has_feature(ptrauth_calls)
#include
+#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(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(m_state.context.gpr.__opaque_sp));
#else
@@ -169,25 +173,28 @@ kern_return_t DNBArchMachARM64::GetGPRState(bool force) {
(thread_state_t)_state.context.gpr, );
if (DNBLogEnabledForAny(LOG_THREAD)) {
uint64_t *x = _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(m_state.context.gpr.__opaque_fp),
reinterpret_cast(m_state.context.gpr.__opaque_lr),
reinterpret_cast(m_state.context.gpr.__opaque_sp),
- reinterpret_cast(m_state.context.gpr.__opaque_pc)
+
reinterpret_cast(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,
[Lldb-commits] [lldb] [lldb] [debugserver] address preprocessor warning, extra arg (PR #90808)
@@ -169,25 +173,28 @@ kern_return_t DNBArchMachARM64::GetGPRState(bool force) {
(thread_state_t)_state.context.gpr, );
if (DNBLogEnabledForAny(LOG_THREAD)) {
uint64_t *x = _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(m_state.context.gpr.__opaque_fp),
reinterpret_cast(m_state.context.gpr.__opaque_lr),
reinterpret_cast(m_state.context.gpr.__opaque_sp),
- reinterpret_cast(m_state.context.gpr.__opaque_pc)
+
reinterpret_cast(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);
bulbazord wrote:
Suggestion: The format string is the same between these two, why not pull out
the common factor?
```
const char *format = "thread_get_state signed regs\n fp=%16.16llx\n
lr=%16.16llx\n sp=%16.16llx\n pc=%16.16llx";
#if defined(...)
DNBLogThreaded(format, ...);
#else
DNBLogThreaded(format, ...);
#endif
```
You could also pull out the arguments.
https://github.com/llvm/llvm-project/pull/90808
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] [debugserver] address preprocessor warning, extra arg (PR #90808)
https://github.com/jasonmolenda edited https://github.com/llvm/llvm-project/pull/90808 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] [debugserver] address preprocessor warning, extra arg (PR #90808)
llvmbot wrote:
@llvm/pr-subscribers-lldb
Author: Jason Molenda (jasonmolenda)
Changes
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
---
Full diff: https://github.com/llvm/llvm-project/pull/90808.diff
2 Files Affected:
- (modified) lldb/tools/debugserver/source/MacOSX/MachProcess.mm (+4-4)
- (modified) lldb/tools/debugserver/source/MacOSX/arm64/DNBArchImplARM64.cpp
(+19-12)
``diff
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
#include
+#undef DEBUGSERVER_IS_ARM64E
#if __has_feature(ptrauth_calls)
#include
+#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(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(m_state.context.gpr.__opaque_sp));
#else
@@ -169,25 +173,28 @@ kern_return_t DNBArchMachARM64::GetGPRState(bool force) {
(thread_state_t)_state.context.gpr, );
if (DNBLogEnabledForAny(LOG_THREAD)) {
uint64_t *x = _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(m_state.context.gpr.__opaque_fp),
reinterpret_cast(m_state.context.gpr.__opaque_lr),
reinterpret_cast(m_state.context.gpr.__opaque_sp),
- reinterpret_cast(m_state.context.gpr.__opaque_pc)
+
reinterpret_cast(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)
[Lldb-commits] [lldb] [lldb] [debugserver] address preprocessor warning, extra arg (PR #90808)
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
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
#include
+#undef DEBUGSERVER_IS_ARM64E
#if __has_feature(ptrauth_calls)
#include
+#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(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(m_state.context.gpr.__opaque_sp));
#else
@@ -169,25 +173,28 @@ kern_return_t DNBArchMachARM64::GetGPRState(bool force) {
(thread_state_t)_state.context.gpr, );
if (DNBLogEnabledForAny(LOG_THREAD)) {
uint64_t *x = _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(m_state.context.gpr.__opaque_fp),
reinterpret_cast(m_state.context.gpr.__opaque_lr),
