Skip to content

[lldb] [debugserver] address preprocessor warning, extra arg #90808

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
May 2, 2024
Merged

[lldb] [debugserver] address preprocessor warning, extra arg #90808

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8145e9f
[lldb] [debugserver] address preprocessor warning, extra arg
jasonmolenda May 2, 2024
2122664
Use Alex's suggestion for the duplicated DNBLogThreaded
jasonmolenda May 2, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions lldb/tools/debugserver/source/MacOSX/MachProcess.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
37 changes: 20 additions & 17 deletions lldb/tools/debugserver/source/MacOSX/arm64/DNBArchImplARM64.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -169,25 +173,24 @@ 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];
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__)

const char *log_str = "thread_get_state signed regs "
"\n fp=%16.16llx"
"\n lr=%16.16llx"
"\n sp=%16.16llx"
"\n pc=%16.16llx";
#if defined(DEBUGSERVER_IS_ARM64E)
DNBLogThreaded(log_str,
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(log_str, m_state.context.gpr.__fp, m_state.context.gpr.__lr,
m_state.context.gpr.__sp, m_state.context.gpr.__pc);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good idea, done.

#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(
Expand Down Expand Up @@ -661,7 +664,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
Expand Down Expand Up @@ -2187,7 +2190,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));
Expand Down