Aim debugserver workaround more precisely.

[adrian-prantl]

No description provided.

[llvmbot]

@llvm/pr-subscribers-lldb

Author: Adrian Prantl (adrian-prantl)

Changes

---

Full diff: https://github.com/llvm/llvm-project/pull/83099.diff

2 Files Affected:

• (modified) lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp (+14-1)
• (modified) lldb/source/Target/Target.cpp (+3-1)

diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
index 6f8aa262289946..c38d74ca8e6c95 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
@@ -17,6 +17,7 @@
 
 #include "lldb/Core/ModuleSpec.h"
 #include "lldb/Host/HostInfo.h"
+#include "lldb/Host/SafeMachO.h"
 #include "lldb/Host/XML.h"
 #include "lldb/Symbol/Symbol.h"
 #include "lldb/Target/MemoryRegionInfo.h"
@@ -2147,8 +2148,20 @@ bool GDBRemoteCommunicationClient::GetCurrentProcessInfo(bool allow_lazy) {
           if (!value.getAsInteger(16, cpu))
             ++num_keys_decoded;
         } else if (name.equals("cpusubtype")) {
-          if (!value.getAsInteger(16, sub))
+          if (!value.getAsInteger(16, sub)) {
             ++num_keys_decoded;
+            // Workaround for for pre-2024 Apple 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 (cpu == llvm::MachO::CPU_TYPE_ARM64 &&
+                sub == llvm::MachO::CPU_SUBTYPE_ARM64E) {
+              if (GetGDBServerVersion())
+                if (m_gdb_server_version >= 1000 &&
+                    m_gdb_server_version <= 1504)
+                  sub = 0;
+            }
+          }
         } else if (name.equals("triple")) {
           StringExtractor extractor(value);
           extractor.GetHexByteString(triple);
diff --git a/lldb/source/Target/Target.cpp b/lldb/source/Target/Target.cpp
index 089915cab4915a..b592016e7a9824 100644
--- a/lldb/source/Target/Target.cpp
+++ b/lldb/source/Target/Target.cpp
@@ -33,7 +33,6 @@
 #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"
@@ -1572,6 +1571,7 @@ bool Target::SetArchitecture(const ArchSpec &arch_spec, bool set_platform,
 
         if (m_arch.GetSpec().GetTriple() == other.GetTriple())
           replace_local_arch = false;
+<<<<<<< HEAD
         // 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
@@ -1579,6 +1579,8 @@ bool Target::SetArchitecture(const ArchSpec &arch_spec, bool set_platform,
         if (!m_arch.GetSpec().GetMachOCPUSubType() &&
             other.GetMachOCPUSubType() == llvm::MachO::CPU_SUBTYPE_ARM64E)
           replace_local_arch = true;
+=======
+>>>>>>> e491dde5dab4 (Aim debugserver workaround more precisely.)
       }
     }
   }

[jasonmolenda]

I'm a little split because the current packet is not super useful as it's constructed today - I'd really like to see a domain or vendor along with a version number And instead of a simple number, maybe a major/minor version number and/or a version string? I'm also fine with landing this now, detecting an "old" apple debugserver as 1000 < version number < 1504, and I can do the version packet fix separately and update the check.

[jasonmolenda]

I'm not fishing for Adrian to dig in to that, I'm happy to make the change. but I'm curious what @JDevlieghere and @adrian-prantl think, if it's worth changing this packet. It's not used anywhere yet, so overhauling it now before/shortly after we actually start using it, is the right way to go.

[jasonmolenda]

LGTM, let's land this patch. I will extend the stub version packet with additional fields (Jonas points out that other stubs implement this packet already as-is) and update this once that PR is approved and merged. But we don't need to block this PR on getting that done.

[JDevlieghere]

I'm happy if Jason is happy!