[Lldb-commits] [lldb] [lldb] Add RegisterContextPOSIXCore for RISC-V 64 (PR #93297)

[Alexey Merzlyakov via lldb-commits]

================
@@ -0,0 +1,84 @@
+//===-- RegisterContextPOSIXCore_riscv64.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 "RegisterContextPOSIXCore_riscv64.h"
+
+#include "lldb/Utility/DataBufferHeap.h"
+
+using namespace lldb_private;
+
+std::unique_ptr<RegisterContextCorePOSIX_riscv64>
+RegisterContextCorePOSIX_riscv64::Create(
+    lldb_private::Thread &thread, const lldb_private::ArchSpec &arch,
+    const lldb_private::DataExtractor &gpregset,
+    llvm::ArrayRef<lldb_private::CoreNote> notes) {
+  Flags flags = 0;
+
+  auto register_info_up =
+      std::make_unique<RegisterInfoPOSIX_riscv64>(arch, flags);
+  return std::unique_ptr<RegisterContextCorePOSIX_riscv64>(
+      new RegisterContextCorePOSIX_riscv64(thread, std::move(register_info_up),
+                                           gpregset, notes));
+}
+
+RegisterContextCorePOSIX_riscv64::RegisterContextCorePOSIX_riscv64(
+    Thread &thread, std::unique_ptr<RegisterInfoPOSIX_riscv64> register_info,
+    const DataExtractor &gpregset, llvm::ArrayRef<CoreNote> notes)
+    : RegisterContextPOSIX_riscv64(thread, std::move(register_info)) {
+
+  m_gpr_buffer = std::make_shared<DataBufferHeap>(gpregset.GetDataStart(),
+                                                  gpregset.GetByteSize());
+  m_gpr.SetData(m_gpr_buffer);
+  m_gpr.SetByteOrder(gpregset.GetByteOrder());
+
+  ArchSpec arch = m_register_info_up->GetTargetArchitecture();
+  DataExtractor fpregset = getRegset(notes, arch.GetTriple(), FPR_Desc);
+  m_fpr_buffer = std::make_shared<DataBufferHeap>(fpregset.GetDataStart(),
----------------
AlexeyMerzlyakov wrote:

Yes, this is good point, since some architectures (like RV64IMAC) might not contain FP-registers at all.

However, it seems that in current state of implementation of RegisterInfo RISC-V plugin FP registers are always  enabled as GP ones. For example, [here](https://github.com/llvm/llvm-project/blob/78cc9cbba23fd1783a9b233ae745f126ece56cc7/lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_riscv64.cpp#L56) and [here](https://github.com/llvm/llvm-project/blob/78cc9cbba23fd1783a9b233ae745f126ece56cc7/lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_riscv64.cpp#L123-L125). If we will just add the check in `RegisterContextCorePOSIX_riscv64()` contstructor / `Create()` routine, it won't be enough: LLDB will fail in run-time when try to iterate all the registers and will get wrong number from RegisterInfo plugin. The Info plugin is also should be re-worked itself, to have FP optionally enabled if `lldb_private::getRegset()` will confirm it.

As it seems to me and if you don't mind, I'd like to move this work to separate PR ticket, as it related more to RegisterInfo RISC-V plugin re-work rather than coredumps. I have some initial patch on it, but I'd like to test it well before having the final solution.

https://github.com/llvm/llvm-project/pull/93297