[Lldb-commits] [lldb] [lldb][AArch64][Linux] Add field information for the CPSR register (PR #70300)

David Spickett via lldb-commits lldb-commits at lists.llvm.org
Tue Nov 7 02:25:12 PST 2023


https://github.com/DavidSpickett updated https://github.com/llvm/llvm-project/pull/70300

>From 45a9d131ce6c9fb31355519cd810ceff32c05ee7 Mon Sep 17 00:00:00 2001
From: David Spickett <david.spickett at linaro.org>
Date: Wed, 11 Oct 2023 14:54:07 +0100
Subject: [PATCH 1/8] [lldb][AArch64][Linux] Add field information for the CPSR
 register

The contents of which are mostly SPSR_EL1 as shown in the Arm manual,
with a few adjustments for things Linux says userspace shouldn't concern
itself with.

```
(lldb) register read cpsr
    cpsr = 0x80001000
         = (N = 1, Z = 0, C = 0, V = 0, SS = 0, IL = 0, ...
```

Some fields are always present, some depend on extensions. I've checked
for those extensions using HWCAP and HWCAP2.

To provide this for core files and live processes I've added a new class
LinuxArm64RegisterFlags. This is a container for all the registers we'll want
to have fields and handles detecting fields and updating register info.

This is used by the native process as follows:
* There is a global LinuxArm64RegisterFlags object.
* The first thread takes a mutex on it, and updates the fields.
* Subsequent threads see that detection is already done, and skip it.
* All threads then update their own copy of the register information
  with pointers to the field information contained in the global object.

This means that even though every thread will have the same fields,
we only detect them once and have one copy of the information.

Core files instead have a LinuxArm64RegisterFlags as a member, because
each core file could have different saved capabilities. The logic from
there is the same but we get HWACP values from the corefile note.

This handler class is Linux specific right now, but it can easily be made
more generic if needed. For example by using LLVM's FeatureBitset
instead of HWCAPs.

Updating register info is done with string comparison, which isn't ideal.
For CPSR, we do know the register number ahead of time but we do not for
other registers in dynamic register sets. So in the interest of
consistency, I'm going to use string comparison for all registers including cpsr.

I've added tests with a core file and live process. Only checking for
fields that are always present to account for CPU variance.
---
 lldb/include/lldb/Target/RegisterFlags.h      |   5 +
 lldb/include/lldb/lldb-private-types.h        |   5 +-
 .../NativeRegisterContextLinux_arm64.cpp      |  18 ++++
 .../Plugins/Process/Utility/CMakeLists.txt    |   1 +
 .../Utility/RegisterFlagsLinux_arm64.cpp      | 100 ++++++++++++++++++
 .../Utility/RegisterFlagsLinux_arm64.h        |  75 +++++++++++++
 .../RegisterContextPOSIXCore_arm64.cpp        |  17 +++
 .../elf-core/RegisterContextPOSIXCore_arm64.h |   3 +
 lldb/source/Target/RegisterFlags.cpp          |  14 ++-
 .../register_command/TestRegisters.py         |  12 +++
 .../postmortem/elf-core/TestLinuxCore.py      |   4 +
 11 files changed, 249 insertions(+), 5 deletions(-)
 create mode 100644 lldb/source/Plugins/Process/Utility/RegisterFlagsLinux_arm64.cpp
 create mode 100644 lldb/source/Plugins/Process/Utility/RegisterFlagsLinux_arm64.h

diff --git a/lldb/include/lldb/Target/RegisterFlags.h b/lldb/include/lldb/Target/RegisterFlags.h
index 7c5b97c2265fda3..1d8c8e943813e77 100644
--- a/lldb/include/lldb/Target/RegisterFlags.h
+++ b/lldb/include/lldb/Target/RegisterFlags.h
@@ -83,6 +83,11 @@ class RegisterFlags {
   RegisterFlags(std::string id, unsigned size,
                 const std::vector<Field> &fields);
 
+  /// Replace all the fields with the new set of fields. All the assumptions
+  /// and checks apply as when you use the constructor. Intended to only be used
+  /// when runtime field detection is needed.
+  void SetFields(const std::vector<Field> &fields);
+
   // Reverse the order of the fields, keeping their values the same.
   // For example a field from bit 31 to 30 with value 0b10 will become bits
   // 1 to 0, with the same 0b10 value.
diff --git a/lldb/include/lldb/lldb-private-types.h b/lldb/include/lldb/lldb-private-types.h
index e6717836331f590..f2ced61b7cc315b 100644
--- a/lldb/include/lldb/lldb-private-types.h
+++ b/lldb/include/lldb/lldb-private-types.h
@@ -62,7 +62,10 @@ struct RegisterInfo {
   /// rax ax, ah, and al.
   uint32_t *invalidate_regs;
   /// If not nullptr, a type defined by XML descriptions.
-  const RegisterFlags *flags_type;
+  /// This is mutable so that it may be updated after the register info tables
+  /// have been constructed. For example a specific target OS may have a
+  /// different layout.
+  mutable const RegisterFlags *flags_type;
 
   llvm::ArrayRef<uint8_t> data(const uint8_t *context_base) const {
     return llvm::ArrayRef<uint8_t>(context_base + byte_offset, byte_size);
diff --git a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
index b5210c368144206..b55d60f3d9bbd13 100644
--- a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
+++ b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
@@ -23,6 +23,7 @@
 #include "Plugins/Process/Linux/Procfs.h"
 #include "Plugins/Process/POSIX/ProcessPOSIXLog.h"
 #include "Plugins/Process/Utility/MemoryTagManagerAArch64MTE.h"
+#include "Plugins/Process/Utility/RegisterFlagsLinux_arm64.h"
 #include "Plugins/Process/Utility/RegisterInfoPOSIX_arm64.h"
 
 // System includes - They have to be included after framework includes because
@@ -30,6 +31,7 @@
 #include <sys/uio.h>
 // NT_PRSTATUS and NT_FPREGSET definition
 #include <elf.h>
+#include <mutex>
 #include <optional>
 
 #ifndef NT_ARM_SVE
@@ -54,12 +56,20 @@
 #endif
 
 #define HWCAP_PACA (1 << 30)
+
 #define HWCAP2_MTE (1 << 18)
 
 using namespace lldb;
 using namespace lldb_private;
 using namespace lldb_private::process_linux;
 
+// A NativeRegisterContext is constructed per thread, but all threads' registers
+// will contain the same fields. Therefore this mutex prevents each instance
+// competing with the other, and subsequent instances from having to detect the
+// fields all over again.
+static std::mutex g_register_flags_mutex;
+static LinuxArm64RegisterFlags g_register_flags;
+
 std::unique_ptr<NativeRegisterContextLinux>
 NativeRegisterContextLinux::CreateHostNativeRegisterContextLinux(
     const ArchSpec &target_arch, NativeThreadLinux &native_thread) {
@@ -118,6 +128,10 @@ NativeRegisterContextLinux::CreateHostNativeRegisterContextLinux(
 
     opt_regsets.Set(RegisterInfoPOSIX_arm64::eRegsetMaskTLS);
 
+    std::lock_guard<std::mutex> lock(g_register_flags_mutex);
+    if (!g_register_flags.HasDetected())
+      g_register_flags.DetectFields(auxv_at_hwcap, auxv_at_hwcap2);
+
     auto register_info_up =
         std::make_unique<RegisterInfoPOSIX_arm64>(target_arch, opt_regsets);
     return std::make_unique<NativeRegisterContextLinux_arm64>(
@@ -140,6 +154,10 @@ NativeRegisterContextLinux_arm64::NativeRegisterContextLinux_arm64(
     : NativeRegisterContextRegisterInfo(native_thread,
                                         register_info_up.release()),
       NativeRegisterContextLinux(native_thread) {
+  g_register_flags.UpdateRegisterInfo(
+      GetRegisterInfoInterface().GetRegisterInfo(),
+      GetRegisterInfoInterface().GetRegisterCount());
+
   ::memset(&m_fpr, 0, sizeof(m_fpr));
   ::memset(&m_gpr_arm64, 0, sizeof(m_gpr_arm64));
   ::memset(&m_hwp_regs, 0, sizeof(m_hwp_regs));
diff --git a/lldb/source/Plugins/Process/Utility/CMakeLists.txt b/lldb/source/Plugins/Process/Utility/CMakeLists.txt
index 1ebd0484f0210a6..37b53b7e3e7edde 100644
--- a/lldb/source/Plugins/Process/Utility/CMakeLists.txt
+++ b/lldb/source/Plugins/Process/Utility/CMakeLists.txt
@@ -47,6 +47,7 @@ add_lldb_library(lldbPluginProcessUtility
   RegisterContextThreadMemory.cpp
   RegisterContextWindows_i386.cpp
   RegisterContextWindows_x86_64.cpp
+  RegisterFlagsLinux_arm64.cpp
   RegisterInfos_x86_64_with_base_shared.cpp
   RegisterInfoPOSIX_arm.cpp
   RegisterInfoPOSIX_arm64.cpp
diff --git a/lldb/source/Plugins/Process/Utility/RegisterFlagsLinux_arm64.cpp b/lldb/source/Plugins/Process/Utility/RegisterFlagsLinux_arm64.cpp
new file mode 100644
index 000000000000000..13f265e0fe91e22
--- /dev/null
+++ b/lldb/source/Plugins/Process/Utility/RegisterFlagsLinux_arm64.cpp
@@ -0,0 +1,100 @@
+//===-- RegisterFlagsLinux_arm64.cpp --------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "RegisterFlagsLinux_arm64.h"
+#include "lldb/lldb-private-types.h"
+
+// This file is built on all systems because it is used by native processes and
+// core files, so we manually define the needed HWCAP values here.
+
+#define HWCAP_DIT (1 << 24)
+#define HWCAP_SSBS (1 << 28)
+
+#define HWCAP2_BTI (1 << 17)
+#define HWCAP2_MTE (1 << 18)
+
+using namespace lldb_private;
+
+LinuxArm64RegisterFlags::Fields
+LinuxArm64RegisterFlags::DetectCPSRFields(std::optional<uint64_t> hwcap,
+                                          std::optional<uint64_t> hwcap2) {
+  // The fields here are a combination of the Arm manual's SPSR_EL1,
+  // plus a few changes where Linux has decided not to make use of them at all,
+  // or at least not from userspace.
+
+  // Status bits that are always present.
+  std::vector<RegisterFlags::Field> cpsr_fields{
+      {"N", 31}, {"Z", 30}, {"C", 29}, {"V", 28},
+      // Bits 27-26 reserved.
+  };
+
+  if (hwcap2 && (*hwcap2 & HWCAP2_MTE))
+    cpsr_fields.push_back({"TCO", 25});
+  if (hwcap && (*hwcap & HWCAP_DIT))
+    cpsr_fields.push_back({"DIT", 24});
+
+  // UAO and PAN are bits 23 and 22 and have no meaning for userspace so
+  // are treated as reserved by the kernel.
+
+  cpsr_fields.push_back({"SS", 21});
+  cpsr_fields.push_back({"IL", 20});
+  // Bits 19-14 reserved.
+
+  // Bit 13, ALLINT, requires FEAT_NMI that isn't relevant to userspace, and we
+  // can't detect either, don't show this field.
+  if (hwcap && (*hwcap & HWCAP_SSBS))
+    cpsr_fields.push_back({"SSBS", 12});
+  if (hwcap2 && (*hwcap2 & HWCAP2_BTI))
+    cpsr_fields.push_back({"BTYPE", 10, 11});
+
+  cpsr_fields.push_back({"D", 9});
+  cpsr_fields.push_back({"A", 8});
+  cpsr_fields.push_back({"I", 7});
+  cpsr_fields.push_back({"F", 6});
+  // Bit 5 reserved
+  // Called "M" in the ARMARM.
+  cpsr_fields.push_back({"nRW", 4});
+  // This is a 4 bit field M[3:0] in the ARMARM, we split it into parts.
+  cpsr_fields.push_back({"EL", 2, 3});
+  // Bit 1 is unused and expected to be 0.
+  cpsr_fields.push_back({"SP", 0});
+
+  return cpsr_fields;
+}
+
+void LinuxArm64RegisterFlags::DetectFields(std::optional<uint64_t> hwcap,
+                                           std::optional<uint64_t> hwcap2) {
+  for (auto &reg : m_registers)
+    reg.m_flags.SetFields(reg.m_detector(hwcap, hwcap2));
+  m_has_detected = true;
+}
+
+void LinuxArm64RegisterFlags::UpdateRegisterInfo(const RegisterInfo *reg_info,
+                                                 uint32_t num_regs) {
+  assert(m_has_detected &&
+         "Must call DetectFields before updating register info.");
+
+  // Register names will not be duplicated, so we do not want to compare against
+  // one if it has already been found. Each time we find one, we erase it from
+  // this list.
+  std::vector<std::pair<const char *, const RegisterFlags *>> search_registers;
+  for (const auto &reg : m_registers)
+    search_registers.push_back({reg.m_name, &reg.m_flags});
+
+  uint32_t idx = 0;
+  for (; idx < num_regs && search_registers.size(); ++idx, ++reg_info) {
+    auto end = search_registers.cend();
+    for (auto it = search_registers.cbegin(); it != end; ++it) {
+      if (std::strcmp(reg_info->name, it->first) == 0) {
+        reg_info->flags_type = it->second;
+        search_registers.erase(it);
+        break;
+      }
+    }
+  }
+}
\ No newline at end of file
diff --git a/lldb/source/Plugins/Process/Utility/RegisterFlagsLinux_arm64.h b/lldb/source/Plugins/Process/Utility/RegisterFlagsLinux_arm64.h
new file mode 100644
index 000000000000000..5371ed2f208e24e
--- /dev/null
+++ b/lldb/source/Plugins/Process/Utility/RegisterFlagsLinux_arm64.h
@@ -0,0 +1,75 @@
+//===-- RegisterFlagsLinux_arm64.h ------------------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLDB_SOURCE_PLUGINS_PROCESS_UTILITY_REGISTERFLAGSLINUX_ARM64_H
+#define LLDB_SOURCE_PLUGINS_PROCESS_UTILITY_REGISTERFLAGSLINUX_ARM64_H
+
+#include "lldb/Target/RegisterFlags.h"
+#include <functional>
+#include <optional>
+
+namespace lldb_private {
+
+struct RegisterInfo;
+
+/// This class manages the storage and detection of register field information
+/// for Arm64 Linux registers. The same register may have different fields on
+/// different CPUs. This class abstracts out the field detection process so we
+/// can use it on live processes and core files.
+///
+/// The general way to use this class is:
+/// * Make an instance somewhere that will last as long as the debug session
+///   (because your final register info will point to this instance).
+/// * Read hardware capabilities from a core note, binary, prctl, etc.
+/// * Pass those to DetectFields.
+/// * Call UpdateRegisterInfo with your RegisterInfo to add pointers
+///   to the detected fields for all registers listed in this class.
+///
+/// This must be done in that order, and you should ensure that if multiple
+/// threads will reference the information, a mutex is used to make sure only
+/// one calls DetectFields.
+class LinuxArm64RegisterFlags {
+public:
+  /// For the registers listed in this class, detect which fields are
+  /// present. Must be called before UpdateRegisterInfos.
+  /// If called more than once, fields will be redetected each time from
+  /// scratch.
+  void DetectFields(std::optional<uint64_t> hwcap,
+                    std::optional<uint64_t> hwcap2);
+
+  /// Add the field information of any registers named in this class,
+  /// to the relevant RegisterInfo instances. Note that this will be done
+  /// with a pointer to the instance of this class that you call this on, so
+  /// the lifetime of that instance must be at least that of the register info.
+  void UpdateRegisterInfo(const RegisterInfo *reg_info, uint32_t num_regs);
+
+  /// Returns true if field detection has been run at least once.
+  bool HasDetected() const { return m_has_detected; }
+
+private:
+  using Fields = std::vector<RegisterFlags::Field>;
+
+  static Fields DetectCPSRFields(std::optional<uint64_t> hwcap,
+                                 std::optional<uint64_t> hwcap2);
+
+  struct RegisterEntry {
+    const char *m_name;
+    RegisterFlags m_flags;
+    std::function<Fields(std::optional<uint64_t>, std::optional<uint64_t>)>
+        m_detector;
+  } m_registers[1] = {
+      {"cpsr", {"cpsr_flags", 4, {{"replace_me", 0, 0}}}, DetectCPSRFields},
+  };
+
+  // Becomes true once field detection has been run for all registers.
+  bool m_has_detected = false;
+};
+
+} // namespace lldb_private
+
+#endif // LLDB_SOURCE_PLUGINS_PROCESS_UTILITY_REGISTERFLAGSLINUX_ARM64_H
\ No newline at end of file
diff --git a/lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.cpp b/lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.cpp
index 99cee83eed12515..56edfd4d8676219 100644
--- a/lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.cpp
+++ b/lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.cpp
@@ -9,6 +9,9 @@
 #include "RegisterContextPOSIXCore_arm64.h"
 #include "Plugins/Process/Utility/RegisterInfoPOSIX_arm64.h"
 
+#include "Plugins/Process/Utility/AuxVector.h"
+#include "Plugins/Process/Utility/RegisterFlagsLinux_arm64.h"
+#include "Plugins/Process/elf-core/ProcessElfCore.h"
 #include "Plugins/Process/elf-core/RegisterUtilities.h"
 #include "lldb/Target/Thread.h"
 #include "lldb/Utility/RegisterValue.h"
@@ -67,6 +70,20 @@ RegisterContextCorePOSIX_arm64::RegisterContextCorePOSIX_arm64(
     : RegisterContextPOSIX_arm64(thread, std::move(register_info)) {
   ::memset(&m_sme_pseudo_regs, 0, sizeof(m_sme_pseudo_regs));
 
+  ProcessElfCore *process =
+      static_cast<ProcessElfCore *>(thread.GetProcess().get());
+  if (process->GetArchitecture().GetTriple().getOS() == llvm::Triple::Linux) {
+    AuxVector aux_vec(process->GetAuxvData());
+    std::optional<uint64_t> auxv_at_hwcap =
+        aux_vec.GetAuxValue(AuxVector::AUXV_AT_HWCAP);
+    std::optional<uint64_t> auxv_at_hwcap2 =
+        aux_vec.GetAuxValue(AuxVector::AUXV_AT_HWCAP2);
+
+    m_linux_register_flags.DetectFields(auxv_at_hwcap, auxv_at_hwcap2);
+    m_linux_register_flags.UpdateRegisterInfo(GetRegisterInfo(),
+                                              GetRegisterCount());
+  }
+
   m_gpr_data.SetData(std::make_shared<DataBufferHeap>(gpregset.GetDataStart(),
                                                       gpregset.GetByteSize()));
   m_gpr_data.SetByteOrder(gpregset.GetByteOrder());
diff --git a/lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.h b/lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.h
index 86bdeb426ab3fe0..52116884f0da983 100644
--- a/lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.h
+++ b/lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.h
@@ -11,6 +11,7 @@
 
 #include "Plugins/Process/Utility/LinuxPTraceDefines_arm64sve.h"
 #include "Plugins/Process/Utility/RegisterContextPOSIX_arm64.h"
+#include "Plugins/Process/Utility/RegisterFlagsLinux_arm64.h"
 
 #include "Plugins/Process/elf-core/RegisterUtilities.h"
 #include "lldb/Utility/DataBufferHeap.h"
@@ -73,6 +74,8 @@ class RegisterContextCorePOSIX_arm64 : public RegisterContextPOSIX_arm64 {
 
   struct sme_pseudo_regs m_sme_pseudo_regs;
 
+  lldb_private::LinuxArm64RegisterFlags m_linux_register_flags;
+
   const uint8_t *GetSVEBuffer(uint64_t offset = 0);
 
   void ConfigureRegisterContext();
diff --git a/lldb/source/Target/RegisterFlags.cpp b/lldb/source/Target/RegisterFlags.cpp
index 49974718ccb514a..b1669b85fd2fe7b 100644
--- a/lldb/source/Target/RegisterFlags.cpp
+++ b/lldb/source/Target/RegisterFlags.cpp
@@ -53,9 +53,7 @@ unsigned RegisterFlags::Field::PaddingDistance(const Field &other) const {
   return lhs_start - rhs_end - 1;
 }
 
-RegisterFlags::RegisterFlags(std::string id, unsigned size,
-                             const std::vector<Field> &fields)
-    : m_id(std::move(id)), m_size(size) {
+void RegisterFlags::SetFields(const std::vector<Field> &fields) {
   // We expect that the XML processor will discard anything describing flags but
   // with no fields.
   assert(fields.size() && "Some fields must be provided.");
@@ -63,6 +61,8 @@ RegisterFlags::RegisterFlags(std::string id, unsigned size,
   // We expect that these are unsorted but do not overlap.
   // They could fill the register but may have gaps.
   std::vector<Field> provided_fields = fields;
+
+  m_fields.clear();
   m_fields.reserve(provided_fields.size());
 
   // ProcessGDBRemote should have sorted these in descending order already.
@@ -71,7 +71,7 @@ RegisterFlags::RegisterFlags(std::string id, unsigned size,
   // Build a new list of fields that includes anonymous (empty name) fields
   // wherever there is a gap. This will simplify processing later.
   std::optional<Field> previous_field;
-  unsigned register_msb = (size * 8) - 1;
+  unsigned register_msb = (m_size * 8) - 1;
   for (auto field : provided_fields) {
     if (previous_field) {
       unsigned padding = previous_field->PaddingDistance(field);
@@ -96,6 +96,12 @@ RegisterFlags::RegisterFlags(std::string id, unsigned size,
     m_fields.push_back(Field("", 0, previous_field->GetStart() - 1));
 }
 
+RegisterFlags::RegisterFlags(std::string id, unsigned size,
+                             const std::vector<Field> &fields)
+    : m_id(std::move(id)), m_size(size) {
+  SetFields(fields);
+}
+
 void RegisterFlags::log(Log *log) const {
   LLDB_LOG(log, "ID: \"{0}\" Size: {1}", m_id.c_str(), m_size);
   for (const Field &field : m_fields)
diff --git a/lldb/test/API/commands/register/register/register_command/TestRegisters.py b/lldb/test/API/commands/register/register/register_command/TestRegisters.py
index f2ee3c4a047a269..f29b2ab5d8f259d 100644
--- a/lldb/test/API/commands/register/register/register_command/TestRegisters.py
+++ b/lldb/test/API/commands/register/register/register_command/TestRegisters.py
@@ -618,6 +618,18 @@ def test_info_register(self):
         # This has an alternative name according to the ABI.
         self.expect("register info x30", substrs=["Name: lr (x30)"])
 
+    @skipIfXmlSupportMissing
+    @skipUnlessPlatform(["linux"])
+    @skipIf(archs=no_match(["aarch64"]))
+    def test_register_read_fields(self):
+        """Test that when debugging a live process, we see the fields of the
+        CPSR register."""
+        self.build()
+        self.common_setup()
+
+        # N/Z/C/V bits will always be present, so check only for those.
+        self.expect("register read cpsr", substrs=["= (N = 0, Z = 1, C = 1, V = 0"])
+
     @skipUnlessPlatform(["linux"])
     @skipIf(archs=no_match(["x86_64"]))
     def test_fs_gs_base(self):
diff --git a/lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py b/lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
index 4ff288ad49c0155..a083fab18eabcbc 100644
--- a/lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
+++ b/lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
@@ -574,6 +574,10 @@ def test_aarch64_sve_regs_full(self):
 
         self.expect("register read --all")
 
+        # Register field information should work with core files as it does a live process.
+        # The N/Z/C/V bits are always present so just check for those.
+        self.expect("register read cpsr", substrs=["= (N = 0, Z = 0, C = 0, V = 0"])
+
     @skipIfLLVMTargetMissing("AArch64")
     def test_aarch64_pac_regs(self):
         # Test AArch64/Linux Pointer Authentication register read

>From 3acbb68b8610c147f5516834d078452ec6dcfdb2 Mon Sep 17 00:00:00 2001
From: David Spickett <david.spickett at linaro.org>
Date: Fri, 27 Oct 2023 08:36:12 +0000
Subject: [PATCH 2/8] Make _flags name and dummy field automatically.

---
 .../Process/Utility/RegisterFlagsLinux_arm64.h        | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/lldb/source/Plugins/Process/Utility/RegisterFlagsLinux_arm64.h b/lldb/source/Plugins/Process/Utility/RegisterFlagsLinux_arm64.h
index 5371ed2f208e24e..a7170a7138c5023 100644
--- a/lldb/source/Plugins/Process/Utility/RegisterFlagsLinux_arm64.h
+++ b/lldb/source/Plugins/Process/Utility/RegisterFlagsLinux_arm64.h
@@ -53,17 +53,22 @@ class LinuxArm64RegisterFlags {
 
 private:
   using Fields = std::vector<RegisterFlags::Field>;
+  using DetectorFn =
+      std::function<Fields(std::optional<uint64_t>, std::optional<uint64_t>)>;
 
   static Fields DetectCPSRFields(std::optional<uint64_t> hwcap,
                                  std::optional<uint64_t> hwcap2);
 
   struct RegisterEntry {
+    RegisterEntry(const char *name, unsigned size, DetectorFn detector)
+        : m_name(name), m_flags(std::string(name) + "_flags", size, {{"", 0}}),
+          m_detector(detector) {}
+
     const char *m_name;
     RegisterFlags m_flags;
-    std::function<Fields(std::optional<uint64_t>, std::optional<uint64_t>)>
-        m_detector;
+    DetectorFn m_detector;
   } m_registers[1] = {
-      {"cpsr", {"cpsr_flags", 4, {{"replace_me", 0, 0}}}, DetectCPSRFields},
+      RegisterEntry("cpsr", 4, DetectCPSRFields),
   };
 
   // Becomes true once field detection has been run for all registers.

>From 447c9307fa71d024b2f9fbb2cf286ebc8f18babb Mon Sep 17 00:00:00 2001
From: David Spickett <david.spickett at linaro.org>
Date: Fri, 27 Oct 2023 08:42:43 +0000
Subject: [PATCH 3/8] Don't patch register info if detection found 0 fields.

---
 .../Plugins/Process/Utility/RegisterFlagsLinux_arm64.cpp  | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/lldb/source/Plugins/Process/Utility/RegisterFlagsLinux_arm64.cpp b/lldb/source/Plugins/Process/Utility/RegisterFlagsLinux_arm64.cpp
index 13f265e0fe91e22..195827ac34f549b 100644
--- a/lldb/source/Plugins/Process/Utility/RegisterFlagsLinux_arm64.cpp
+++ b/lldb/source/Plugins/Process/Utility/RegisterFlagsLinux_arm64.cpp
@@ -83,8 +83,12 @@ void LinuxArm64RegisterFlags::UpdateRegisterInfo(const RegisterInfo *reg_info,
   // one if it has already been found. Each time we find one, we erase it from
   // this list.
   std::vector<std::pair<const char *, const RegisterFlags *>> search_registers;
-  for (const auto &reg : m_registers)
-    search_registers.push_back({reg.m_name, &reg.m_flags});
+  for (const auto &reg : m_registers) {
+    // It is possible that a register is all extension dependent fields, and
+    // none of them are present.
+    if (reg.m_flags.GetFields().size())
+      search_registers.push_back({reg.m_name, &reg.m_flags});
+  }
 
   uint32_t idx = 0;
   for (; idx < num_regs && search_registers.size(); ++idx, ++reg_info) {

>From 3a72da47576fff6573363350bccaaf0cf14adb81 Mon Sep 17 00:00:00 2001
From: David Spickett <david.spickett at linaro.org>
Date: Fri, 27 Oct 2023 08:49:47 +0000
Subject: [PATCH 4/8] As HWCAP bits are 0 if feature not present, just pass 0
 to detectors if we don't have a HWCAP value.

---
 .../Linux/NativeRegisterContextLinux_arm64.cpp     |  3 ++-
 .../Process/Utility/RegisterFlagsLinux_arm64.cpp   | 14 ++++++--------
 .../Process/Utility/RegisterFlagsLinux_arm64.h     | 13 +++++--------
 .../elf-core/RegisterContextPOSIXCore_arm64.cpp    |  3 ++-
 4 files changed, 15 insertions(+), 18 deletions(-)

diff --git a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
index b55d60f3d9bbd13..0c598c5a83e7fa3 100644
--- a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
+++ b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
@@ -130,7 +130,8 @@ NativeRegisterContextLinux::CreateHostNativeRegisterContextLinux(
 
     std::lock_guard<std::mutex> lock(g_register_flags_mutex);
     if (!g_register_flags.HasDetected())
-      g_register_flags.DetectFields(auxv_at_hwcap, auxv_at_hwcap2);
+      g_register_flags.DetectFields(auxv_at_hwcap.value_or(0),
+                                    auxv_at_hwcap2.value_or(0));
 
     auto register_info_up =
         std::make_unique<RegisterInfoPOSIX_arm64>(target_arch, opt_regsets);
diff --git a/lldb/source/Plugins/Process/Utility/RegisterFlagsLinux_arm64.cpp b/lldb/source/Plugins/Process/Utility/RegisterFlagsLinux_arm64.cpp
index 195827ac34f549b..5102a8ae39b26c1 100644
--- a/lldb/source/Plugins/Process/Utility/RegisterFlagsLinux_arm64.cpp
+++ b/lldb/source/Plugins/Process/Utility/RegisterFlagsLinux_arm64.cpp
@@ -21,8 +21,7 @@
 using namespace lldb_private;
 
 LinuxArm64RegisterFlags::Fields
-LinuxArm64RegisterFlags::DetectCPSRFields(std::optional<uint64_t> hwcap,
-                                          std::optional<uint64_t> hwcap2) {
+LinuxArm64RegisterFlags::DetectCPSRFields(uint64_t hwcap, uint64_t hwcap2) {
   // The fields here are a combination of the Arm manual's SPSR_EL1,
   // plus a few changes where Linux has decided not to make use of them at all,
   // or at least not from userspace.
@@ -33,9 +32,9 @@ LinuxArm64RegisterFlags::DetectCPSRFields(std::optional<uint64_t> hwcap,
       // Bits 27-26 reserved.
   };
 
-  if (hwcap2 && (*hwcap2 & HWCAP2_MTE))
+  if (hwcap2 & HWCAP2_MTE)
     cpsr_fields.push_back({"TCO", 25});
-  if (hwcap && (*hwcap & HWCAP_DIT))
+  if (hwcap & HWCAP_DIT)
     cpsr_fields.push_back({"DIT", 24});
 
   // UAO and PAN are bits 23 and 22 and have no meaning for userspace so
@@ -47,9 +46,9 @@ LinuxArm64RegisterFlags::DetectCPSRFields(std::optional<uint64_t> hwcap,
 
   // Bit 13, ALLINT, requires FEAT_NMI that isn't relevant to userspace, and we
   // can't detect either, don't show this field.
-  if (hwcap && (*hwcap & HWCAP_SSBS))
+  if (hwcap & HWCAP_SSBS)
     cpsr_fields.push_back({"SSBS", 12});
-  if (hwcap2 && (*hwcap2 & HWCAP2_BTI))
+  if (hwcap2 & HWCAP2_BTI)
     cpsr_fields.push_back({"BTYPE", 10, 11});
 
   cpsr_fields.push_back({"D", 9});
@@ -67,8 +66,7 @@ LinuxArm64RegisterFlags::DetectCPSRFields(std::optional<uint64_t> hwcap,
   return cpsr_fields;
 }
 
-void LinuxArm64RegisterFlags::DetectFields(std::optional<uint64_t> hwcap,
-                                           std::optional<uint64_t> hwcap2) {
+void LinuxArm64RegisterFlags::DetectFields(uint64_t hwcap, uint64_t hwcap2) {
   for (auto &reg : m_registers)
     reg.m_flags.SetFields(reg.m_detector(hwcap, hwcap2));
   m_has_detected = true;
diff --git a/lldb/source/Plugins/Process/Utility/RegisterFlagsLinux_arm64.h b/lldb/source/Plugins/Process/Utility/RegisterFlagsLinux_arm64.h
index a7170a7138c5023..318ce826f481cbe 100644
--- a/lldb/source/Plugins/Process/Utility/RegisterFlagsLinux_arm64.h
+++ b/lldb/source/Plugins/Process/Utility/RegisterFlagsLinux_arm64.h
@@ -11,7 +11,6 @@
 
 #include "lldb/Target/RegisterFlags.h"
 #include <functional>
-#include <optional>
 
 namespace lldb_private {
 
@@ -38,9 +37,9 @@ class LinuxArm64RegisterFlags {
   /// For the registers listed in this class, detect which fields are
   /// present. Must be called before UpdateRegisterInfos.
   /// If called more than once, fields will be redetected each time from
-  /// scratch.
-  void DetectFields(std::optional<uint64_t> hwcap,
-                    std::optional<uint64_t> hwcap2);
+  /// scratch. If you do not have access to hwcap, just pass 0 for each one, you
+  /// will only get unconditional fields.
+  void DetectFields(uint64_t hwcap, uint64_t hwcap2);
 
   /// Add the field information of any registers named in this class,
   /// to the relevant RegisterInfo instances. Note that this will be done
@@ -53,11 +52,9 @@ class LinuxArm64RegisterFlags {
 
 private:
   using Fields = std::vector<RegisterFlags::Field>;
-  using DetectorFn =
-      std::function<Fields(std::optional<uint64_t>, std::optional<uint64_t>)>;
+  using DetectorFn = std::function<Fields(uint64_t, uint64_t)>;
 
-  static Fields DetectCPSRFields(std::optional<uint64_t> hwcap,
-                                 std::optional<uint64_t> hwcap2);
+  static Fields DetectCPSRFields(uint64_t hwcap, uint64_t hwcap2);
 
   struct RegisterEntry {
     RegisterEntry(const char *name, unsigned size, DetectorFn detector)
diff --git a/lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.cpp b/lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.cpp
index 56edfd4d8676219..b23a31e1b102c68 100644
--- a/lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.cpp
+++ b/lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.cpp
@@ -79,7 +79,8 @@ RegisterContextCorePOSIX_arm64::RegisterContextCorePOSIX_arm64(
     std::optional<uint64_t> auxv_at_hwcap2 =
         aux_vec.GetAuxValue(AuxVector::AUXV_AT_HWCAP2);
 
-    m_linux_register_flags.DetectFields(auxv_at_hwcap, auxv_at_hwcap2);
+    m_linux_register_flags.DetectFields(auxv_at_hwcap.value_or(0),
+                                        auxv_at_hwcap2.value_or(0));
     m_linux_register_flags.UpdateRegisterInfo(GetRegisterInfo(),
                                               GetRegisterCount());
   }

>From bf7417e7a0d63b73c449069c4ba7a184186fbd1e Mon Sep 17 00:00:00 2001
From: David Spickett <david.spickett at linaro.org>
Date: Tue, 7 Nov 2023 09:50:41 +0000
Subject: [PATCH 5/8] StringRef

---
 .../Plugins/Process/Utility/RegisterFlagsLinux_arm64.cpp     | 4 ++--
 .../Plugins/Process/Utility/RegisterFlagsLinux_arm64.h       | 5 +++--
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/lldb/source/Plugins/Process/Utility/RegisterFlagsLinux_arm64.cpp b/lldb/source/Plugins/Process/Utility/RegisterFlagsLinux_arm64.cpp
index 5102a8ae39b26c1..e7d63f8271fd0d4 100644
--- a/lldb/source/Plugins/Process/Utility/RegisterFlagsLinux_arm64.cpp
+++ b/lldb/source/Plugins/Process/Utility/RegisterFlagsLinux_arm64.cpp
@@ -80,7 +80,7 @@ void LinuxArm64RegisterFlags::UpdateRegisterInfo(const RegisterInfo *reg_info,
   // Register names will not be duplicated, so we do not want to compare against
   // one if it has already been found. Each time we find one, we erase it from
   // this list.
-  std::vector<std::pair<const char *, const RegisterFlags *>> search_registers;
+  std::vector<std::pair<llvm::StringRef, const RegisterFlags *>> search_registers;
   for (const auto &reg : m_registers) {
     // It is possible that a register is all extension dependent fields, and
     // none of them are present.
@@ -92,7 +92,7 @@ void LinuxArm64RegisterFlags::UpdateRegisterInfo(const RegisterInfo *reg_info,
   for (; idx < num_regs && search_registers.size(); ++idx, ++reg_info) {
     auto end = search_registers.cend();
     for (auto it = search_registers.cbegin(); it != end; ++it) {
-      if (std::strcmp(reg_info->name, it->first) == 0) {
+      if (reg_info->name == it->first) {
         reg_info->flags_type = it->second;
         search_registers.erase(it);
         break;
diff --git a/lldb/source/Plugins/Process/Utility/RegisterFlagsLinux_arm64.h b/lldb/source/Plugins/Process/Utility/RegisterFlagsLinux_arm64.h
index 318ce826f481cbe..6c7a3b61a1425ee 100644
--- a/lldb/source/Plugins/Process/Utility/RegisterFlagsLinux_arm64.h
+++ b/lldb/source/Plugins/Process/Utility/RegisterFlagsLinux_arm64.h
@@ -10,6 +10,7 @@
 #define LLDB_SOURCE_PLUGINS_PROCESS_UTILITY_REGISTERFLAGSLINUX_ARM64_H
 
 #include "lldb/Target/RegisterFlags.h"
+#include "llvm/ADT/StringRef.h"
 #include <functional>
 
 namespace lldb_private {
@@ -57,11 +58,11 @@ class LinuxArm64RegisterFlags {
   static Fields DetectCPSRFields(uint64_t hwcap, uint64_t hwcap2);
 
   struct RegisterEntry {
-    RegisterEntry(const char *name, unsigned size, DetectorFn detector)
+    RegisterEntry(llvm::StringRef name, unsigned size, DetectorFn detector)
         : m_name(name), m_flags(std::string(name) + "_flags", size, {{"", 0}}),
           m_detector(detector) {}
 
-    const char *m_name;
+    llvm::StringRef m_name;
     RegisterFlags m_flags;
     DetectorFn m_detector;
   } m_registers[1] = {

>From efeedcbf1ac6071f0cf480301cbc11f82db25b22 Mon Sep 17 00:00:00 2001
From: David Spickett <david.spickett at linaro.org>
Date: Tue, 7 Nov 2023 10:05:30 +0000
Subject: [PATCH 6/8] Explain the register info walk with an example.

---
 .../Process/Utility/RegisterFlagsLinux_arm64.cpp   | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/lldb/source/Plugins/Process/Utility/RegisterFlagsLinux_arm64.cpp b/lldb/source/Plugins/Process/Utility/RegisterFlagsLinux_arm64.cpp
index e7d63f8271fd0d4..1019d5d0797debe 100644
--- a/lldb/source/Plugins/Process/Utility/RegisterFlagsLinux_arm64.cpp
+++ b/lldb/source/Plugins/Process/Utility/RegisterFlagsLinux_arm64.cpp
@@ -88,12 +88,22 @@ void LinuxArm64RegisterFlags::UpdateRegisterInfo(const RegisterInfo *reg_info,
       search_registers.push_back({reg.m_name, &reg.m_flags});
   }
 
-  uint32_t idx = 0;
-  for (; idx < num_regs && search_registers.size(); ++idx, ++reg_info) {
+  // Walk register information while there are registers we know need
+  // to be updated. Example:
+  // Register information: [a, b, c, d]
+  // To be patched: [b, c]
+  // * a != b, a != c, do nothing and move on.
+  // * b == b, patch b, new patch list is [c], move on.
+  // * c == c, patch c, patch list is empty, exit early without looking at d.
+  for (uint32_t idx = 0; idx < num_regs && search_registers.size();
+       ++idx, ++reg_info) {
     auto end = search_registers.cend();
     for (auto it = search_registers.cbegin(); it != end; ++it) {
+      // If this register is one that must be patched.
       if (reg_info->name == it->first) {
+        // Attach the field information.
         reg_info->flags_type = it->second;
+        // We do not expect to see this name again, so don't look for it.
         search_registers.erase(it);
         break;
       }

>From b229c6667c4e8ae3852db5662e4e855f4a308872 Mon Sep 17 00:00:00 2001
From: David Spickett <david.spickett at linaro.org>
Date: Tue, 7 Nov 2023 10:21:01 +0000
Subject: [PATCH 7/8] Use std:: stuff to do patching.

---
 .../Utility/RegisterFlagsLinux_arm64.cpp      | 23 +++++++++++--------
 1 file changed, 13 insertions(+), 10 deletions(-)

diff --git a/lldb/source/Plugins/Process/Utility/RegisterFlagsLinux_arm64.cpp b/lldb/source/Plugins/Process/Utility/RegisterFlagsLinux_arm64.cpp
index 1019d5d0797debe..f5df4a7cfacc8c3 100644
--- a/lldb/source/Plugins/Process/Utility/RegisterFlagsLinux_arm64.cpp
+++ b/lldb/source/Plugins/Process/Utility/RegisterFlagsLinux_arm64.cpp
@@ -97,16 +97,19 @@ void LinuxArm64RegisterFlags::UpdateRegisterInfo(const RegisterInfo *reg_info,
   // * c == c, patch c, patch list is empty, exit early without looking at d.
   for (uint32_t idx = 0; idx < num_regs && search_registers.size();
        ++idx, ++reg_info) {
-    auto end = search_registers.cend();
-    for (auto it = search_registers.cbegin(); it != end; ++it) {
-      // If this register is one that must be patched.
-      if (reg_info->name == it->first) {
-        // Attach the field information.
-        reg_info->flags_type = it->second;
-        // We do not expect to see this name again, so don't look for it.
-        search_registers.erase(it);
-        break;
-      }
+    auto reg_it = std::find_if(
+        search_registers.cbegin(), search_registers.cend(),
+        [reg_info](auto reg) { return reg.first == reg_info->name; });
+
+    if (reg_it != search_registers.end()) {
+      // Attach the field information.
+      reg_info->flags_type = reg_it->second;
+      // We do not expect to see this name again so don't look for it again.
+      search_registers.erase(reg_it);
     }
   }
+
+  // We do not assert that search_registers is empty here, because it may
+  // contain registers from optional extensions that are not present on the
+  // current target.
 }
\ No newline at end of file

>From 5c22c4ea3e769fb666836a8d4ad8a912f2b8aeb3 Mon Sep 17 00:00:00 2001
From: David Spickett <david.spickett at linaro.org>
Date: Tue, 7 Nov 2023 10:24:48 +0000
Subject: [PATCH 8/8] Format

---
 .../Plugins/Process/Utility/RegisterFlagsLinux_arm64.cpp       | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/lldb/source/Plugins/Process/Utility/RegisterFlagsLinux_arm64.cpp b/lldb/source/Plugins/Process/Utility/RegisterFlagsLinux_arm64.cpp
index f5df4a7cfacc8c3..043789bd6d21e47 100644
--- a/lldb/source/Plugins/Process/Utility/RegisterFlagsLinux_arm64.cpp
+++ b/lldb/source/Plugins/Process/Utility/RegisterFlagsLinux_arm64.cpp
@@ -80,7 +80,8 @@ void LinuxArm64RegisterFlags::UpdateRegisterInfo(const RegisterInfo *reg_info,
   // Register names will not be duplicated, so we do not want to compare against
   // one if it has already been found. Each time we find one, we erase it from
   // this list.
-  std::vector<std::pair<llvm::StringRef, const RegisterFlags *>> search_registers;
+  std::vector<std::pair<llvm::StringRef, const RegisterFlags *>>
+      search_registers;
   for (const auto &reg : m_registers) {
     // It is possible that a register is all extension dependent fields, and
     // none of them are present.



More information about the lldb-commits mailing list