[RISCV][LLDB] Add initial SysV ABI support

[kasper81]

Summary

Supersedes https://reviews.llvm.org/D62732, since author is not working on it anymore (https://reviews.llvm.org/D62732?vs=on&id=341471#3561717).

This is to get the basic stuff working with DefaultUnwindPlan. We can incrementally add more plans in the future and make the implementation more robust. |

[jasonmolenda]

I think this DefaultUnwindPlan has the same problem that the previous patch did -- this looks like the unwind state on function entry. There is an ABI::CreateFunctionEntryUnwindPlan() for the unwind state on the first instruction of a function, but the idea of DefaultUnwindPlan is for backtracing through jitted code, once the stack frame has been set up -- usually the stack pointer is copied into the frame pointer, maybe with a decrement, and the original frame pointer & return address registers are spilled to stack memory. Any of the arm ABI plugins are good examples of this kind of architecture.

Beyond DefaultUnwindPlan, usually the next place we get unwind information is from eh_frame, and lldb uses an instruction emulation system as a source of unwind information. A static scan of all the instructions in the function, tracking branches and register spills, stack pointer/frame pointer manipulations, and creates an UnwindPlan from those. That's a pretty big chunk of work though. although lol I missed that this has already been started, v. lldb/source/Plugins/Instruction/RISCV/EmulateInstructionRISCV.cpp.

[Emmmer]

Currently, lldb/source/Plugins/Instruction/RISCV/EmulateInstructionRISCV.cpp only handles a few jump instructions, we can refine it further if needed.

lldb/source/Plugins/ABI/RISCV/ABISysV_riscv.cpp

39

This enum can be included from lldb/source/Utility/RISCV_DWARF_Registers.h

[kasper81]

@jasonmolenda the problem with original review is that we were waiting for the wholesale support for 3.5 years, and it rendered into an impossible task for the author. I don't want to make this one "all or none" kind of a deal as well. This patch is neither bringing 100% lldb support nor regressing. It is an incremental step forward to unblock a few more scenarios to initialize SysV ABI.

After this, we probably first need RISCV support in lldb/source/Plugins/Architecture which will then be used in Core to unlock some other scenarios and then we can further enrich these features in small and fast iterations. @Emmmer has a better understanding of what is left.

[kasper81]

This comment was removed by kasper81.

[kasper81]

lldb/source/Plugins/ABI/RISCV/ABISysV_riscv.cpp

39

Thanks, I have reused that header.

[jasonmolenda]

In D132510#3745412, @kasper81 wrote:

@jasonmolenda the problem with original review is that we were waiting for the wholesale support for 3.5 years, and it rendered into an impossible task for the author. I don't want to make this one "all or none" kind of a deal as well. This patch is neither bringing 100% lldb support nor regressing. It is an incremental step forward to unblock a few more scenarios to initialize SysV ABI.

Perfectly reasonable. I don't have any problems with this patch, but you might want to add a little FIXME comment in CreateDefaultUnwindPlan noting that it should create a frame-pointer-based unwind plan a la any of the ARM ABI plugins, that this unwind plan is only correct on the first instruction or a leaf function that does not set up a stack frame. I'd also duplicate that method under the name CreateFunctionEntryUnwindPlan() because I believe this is a correct implementation of that ABI method.

[kasper81]

Added a FIXME comment in CreateDefaultUnwindPlan, based on advice from @jasonmolenda.

[kasper81]

@jasonmolenda. @Emmmer, I've addressed your review feedback. Could you please take another look?

[Emmmer]

I have to say sorry after reading your update but not giving you feedback.
I pulled your patch and found it does not compile, and I have fixed them for you :)

In D62732#2306055, @labath wrote:

ABI plugins are one of the hardest things to test in lldb, particularly without actual hardware. That's why we've let them be added in the past without any accompanying tests. The situation is not ideal though, because we've accumulated various ABI plugins which are hard to change without breaking (or even to know if they work) because they only work with third-party (possibly proprietary) stubs.

I have tried to work on ABISys_V plugins, but I think it is difficult because I am not able to check whether ABIPlugin works well.
It will be nice if we have some tests.

lldb/source/Plugins/ABI/RISCV/ABISysV_riscv.cpp

38–40

modify target name.

158–163

Using .isRISCV() instead of
arch.GetTriple().getArch() == llvm::Triple::riscv32 || arch.GetTriple().getArch() == llvm::Triple::riscv64
and remove braces from if statment

176–185

We can remove this since we defined it in ABISysV_riscv.h

lldb/source/Plugins/ABI/RISCV/ABISysV_riscv.h

95–99

GetPluginVersion() is unused.

lldb/source/Plugins/ABI/RISCV/CMakeLists.txt

2

modify target name

[kasper81]

Addressed review feedback.

[kasper81]

@Emmmer, agreed that automated tests for ABI plugins would be a nice thing to have at some point in the future, but it is out of the scope of this initial bring up for RISCV, as tests are missing for all architectures. Also we need Plugins/Architectureport for RISCV to get the exotic use cases working. So best way is to make individual short patches with fast iterations rather than stuck on one thing for years.

[kasper81]

@jasonmolenda, @Emmmer, any other feedback, or good to merge? i will work on Plugins/Architecture/RISCV, unless someone else beat me to it. :)

[jasonmolenda]

In D132510#3771030, @kasper81 wrote:

@jasonmolenda, @Emmmer, any other feedback, or good to merge? i will work on Plugins/Architecture/RISCV, unless someone else beat me to it. :)

FWIW looks good to me, @Emmmer 's feedback is more relevant than mine here but IMO any shortcomings in the current patch can be iterated on later; getting this landed would be good.

[kasper81]

@MaskRay could you please take a look?

[kasper81]

@labath, if there is anything needed by me, please let me know. i'm new to phabricator patch system. :)

[baoyuxu]

@kasper81, I just tried this patch with lastest repo in github and can bulid patched lldb. But when I try to remote debug with riscv qemu, it still get all 1s frame. Could you please see what I'm doing wrong？

lldb/source/Plugins/ABI/CMakeLists.txt

-foreach(target AArch64 ARM ARC Hexagon Mips PowerPC SystemZ X86)
+foreach(target AArch64 ARM ARC Hexagon Mips PowerPC RISCV SystemZ X86)
   if (${target} IN_LIST LLVM_TARGETS_TO_BUILD)
     add_subdirectory(${target})
   endif()

lldb/source/Plugins/ABI/RISCV/ABISysV_riscv.h

+//===-- ABISysV_riscv.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_ABI_RISCV_ABISYSV_RISCV_H
+#define LLDB_SOURCE_PLUGINS_ABI_RISCV_ABISYSV_RISCV_H
+
+#include "lldb/Target/ABI.h"
+#include "lldb/lldb-private.h"
+
+class ABISysV_riscv : public lldb_private::MCBasedABI {
+  bool isRV64;
+
+public:
+  ~ABISysV_riscv() override = default;
+
+  size_t GetRedZoneSize() const override { return 0; }
+
+  bool PrepareTrivialCall(lldb_private::Thread &thread, lldb::addr_t sp,
+                          lldb::addr_t functionAddress,
+                          lldb::addr_t returnAddress,
+                          llvm::ArrayRef<lldb::addr_t> args) const override {
+    // TODO: Implement
+    return false;
+  }
+
+  bool GetArgumentValues(lldb_private::Thread &thread,
+                         lldb_private::ValueList &values) const override {
+    // TODO: Implement
+    return false;
+  }
+
+  lldb_private::Status
+  SetReturnValueObject(lldb::StackFrameSP &frame_sp,
+                       lldb::ValueObjectSP &new_value) override {
+    // TODO: Implement
+    lldb_private::Status error;
+    error.SetErrorString("Not yet implemented");
+    return error;
+  }
+
+  lldb::ValueObjectSP
+  GetReturnValueObjectImpl(lldb_private::Thread &thread,
+                           lldb_private::CompilerType &type) const override {
+    // TODO: Implement
+    lldb::ValueObjectSP return_valobj;
+    return return_valobj;
+  }
+
+  bool
+  CreateFunctionEntryUnwindPlan(lldb_private::UnwindPlan &unwind_plan) override;
+
+  bool CreateDefaultUnwindPlan(lldb_private::UnwindPlan &unwind_plan) override;
+
+  bool RegisterIsVolatile(const lldb_private::RegisterInfo *reg_info) override;
+
+  bool CallFrameAddressIsValid(lldb::addr_t cfa) override {
+    // Assume any address except zero is valid
+    if (cfa == 0)
+      return false;
+    return true;
+  }
+
+  bool CodeAddressIsValid(lldb::addr_t pc) override {
+    // Ensure addresses are smaller than XLEN bits wide. Calls can use the least
+    // significant bit to store auxiliary information, so no strict check is
+    // done for alignment.
+    if (!isRV64)
+      return (pc <= UINT32_MAX);
+    return (pc <= UINT64_MAX);
+  }
+
+  lldb::addr_t FixCodeAddress(lldb::addr_t pc) override {
+    // Since the least significant bit of a code address can be used to store
+    // auxiliary information, that bit must be zeroed in any addresses.
+    return pc & ~(lldb::addr_t)1;
+  }
+
+  // Static Functions
+
+  static void Initialize();
+
+  static void Terminate();
+
+  static lldb::ABISP CreateInstance(lldb::ProcessSP process_sp,
+                                    const lldb_private::ArchSpec &arch);
+
+  // PluginInterface protocol
+
+  static llvm::StringRef GetPluginNameStatic() { return "sysv-riscv"; }
+
+  llvm::StringRef GetPluginName() override { return GetPluginNameStatic(); }
+
+protected:
+  bool RegisterIsCalleeSaved(const lldb_private::RegisterInfo *reg_info);
+
+  std::pair<uint32_t, uint32_t>
+  GetEHAndDWARFNums(llvm::StringRef name) override;
+
+  uint32_t GetGenericNum(llvm::StringRef reg) override;
+
+  bool IsHardFloatProcess() const;
+
+private:
+  ABISysV_riscv(lldb::ProcessSP process_sp,
+                std::unique_ptr<llvm::MCRegisterInfo> info_up, bool _isRV64)
+      : lldb_private::MCBasedABI(std::move(process_sp), std::move(info_up)),
+        isRV64(_isRV64) {
+    // Call CreateInstance instead.
+  }
+};
+
+#endif // LLDB_SOURCE_PLUGINS_ABI_RISCV_ABISYSV_RISCV_H

lldb/source/Plugins/ABI/RISCV/ABISysV_riscv.cpp

+//===-- ABISysV_riscv.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 "ABISysV_riscv.h"
+
+#include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/StringSwitch.h"
+#include "llvm/ADT/Triple.h"
+
+#include "lldb/Core/Module.h"
+#include "lldb/Core/PluginManager.h"
+#include "lldb/Core/Value.h"
+#include "lldb/Core/ValueObjectConstResult.h"
+#include "lldb/Core/ValueObjectMemory.h"
+#include "lldb/Core/ValueObjectRegister.h"
+#include "lldb/Symbol/UnwindPlan.h"
+#include "lldb/Target/Process.h"
+#include "lldb/Target/RegisterContext.h"
+#include "lldb/Target/StackFrame.h"
+#include "lldb/Target/Target.h"
+#include "lldb/Target/Thread.h"
+#include "lldb/Utility/ConstString.h"
+#include "lldb/Utility/DataExtractor.h"
+#include "lldb/Utility/Log.h"
+#include "lldb/Utility/RegisterValue.h"
+#include "lldb/Utility/Status.h"
+
+#include "Utility/RISCV_DWARF_Registers.h"
+
+using namespace lldb;
+using namespace lldb_private;
+using namespace riscv_dwarf;
+
+LLDB_PLUGIN_DEFINE_ADV(ABISysV_riscv, ABIRISCV)
+
+bool ABISysV_riscv::CreateFunctionEntryUnwindPlan(UnwindPlan &unwind_plan) {
+  unwind_plan.Clear();
+  unwind_plan.SetRegisterKind(eRegisterKindGeneric);
+
+  uint32_t pc_reg_num = LLDB_REGNUM_GENERIC_PC;
+  uint32_t sp_reg_num = LLDB_REGNUM_GENERIC_SP;
+  uint32_t ra_reg_num = LLDB_REGNUM_GENERIC_RA;
+
+  UnwindPlan::RowSP row(new UnwindPlan::Row);
+
+  // Define CFA as the current stack pointer.
+  row->GetCFAValue().SetIsRegisterPlusOffset(sp_reg_num, 0);
+  row->SetOffset(0);
+
+  // Previous frame's pc is stored in ra.
+  row->SetRegisterLocationToRegister(pc_reg_num, ra_reg_num, true);
+
+  unwind_plan.AppendRow(row);
+  unwind_plan.SetSourceName("riscv function-entry unwind plan");
+  unwind_plan.SetSourcedFromCompiler(eLazyBoolNo);
+  unwind_plan.SetUnwindPlanValidAtAllInstructions(eLazyBoolNo);
+  return true;
+}
+
+bool ABISysV_riscv::CreateDefaultUnwindPlan(UnwindPlan &unwind_plan) {
+  // FIXME: Currently this implementation is a duplicate of CreateFunctionEntryUnwindPlan(), which is incorrect.
+  //        It should create a frame-pointer-based unwind plan as it is only correct on the first instruction or
+  //        a leaf function that does not set up a stack frame. See any of the ARM ABI for good example.
+
+  unwind_plan.Clear();
+  unwind_plan.SetRegisterKind(eRegisterKindGeneric);
+
+  uint32_t pc_reg_num = LLDB_REGNUM_GENERIC_PC;
+  uint32_t sp_reg_num = LLDB_REGNUM_GENERIC_SP;
+  uint32_t ra_reg_num = LLDB_REGNUM_GENERIC_RA;
+
+  UnwindPlan::RowSP row(new UnwindPlan::Row);
+
+  // Define CFA as the current stack pointer.
+  row->GetCFAValue().SetIsRegisterPlusOffset(sp_reg_num, 0);
+  row->SetOffset(0);
+
+  // Previous frame's pc is stored in ra.
+  row->SetRegisterLocationToRegister(pc_reg_num, ra_reg_num, true);
+
+  unwind_plan.AppendRow(row);
+  unwind_plan.SetSourceName("riscv default unwind plan");
+  unwind_plan.SetSourcedFromCompiler(eLazyBoolNo);
+  unwind_plan.SetUnwindPlanValidAtAllInstructions(eLazyBoolNo);
+  return true;
+}
+
+bool ABISysV_riscv::RegisterIsVolatile(
+    const lldb_private::RegisterInfo *reg_info) {
+  return !RegisterIsCalleeSaved(reg_info);
+}
+
+// See "Register Convention" in the RISC-V psABI documentation, which is
+// maintained at https://github.com/riscv/riscv-elf-psabi-doc
+bool ABISysV_riscv::RegisterIsCalleeSaved(
+    const lldb_private::RegisterInfo *reg_info) {
+  if (!reg_info)
+    return false;
+
+  bool IsCalleeSaved =
+      llvm::StringSwitch<bool>(reg_info->name)
+          .Cases("x1", "x2", "x8", "x9", "x18", "x19", "x20", "x21", true)
+          .Cases("x22", "x23", "x24", "x25", "x26", "x27", true)
+          .Cases("f8", "f9", "f18", "f19", "f20", "f21", IsHardFloatProcess())
+          .Cases("f22", "f23", "f24", "f25", "f26", "f27", IsHardFloatProcess())
+          .Default(false);
+  return IsCalleeSaved;
+}
+
+std::pair<uint32_t, uint32_t>
+ABISysV_riscv::GetEHAndDWARFNums(llvm::StringRef name) {
+  if (name == "ra")
+    return {LLDB_INVALID_REGNUM, dwarf_gpr_x1};
+  if (name == "sp")
+    return {LLDB_INVALID_REGNUM, dwarf_gpr_x2};
+  if (name == "fp")
+    return {LLDB_INVALID_REGNUM, dwarf_gpr_x8};
+  return MCBasedABI::GetEHAndDWARFNums(name);
+}
+
+uint32_t ABISysV_riscv::GetGenericNum(llvm::StringRef name) {
+  return llvm::StringSwitch<uint32_t>(name)
+      .Case("pc", LLDB_REGNUM_GENERIC_PC)
+      .Case("ra", LLDB_REGNUM_GENERIC_RA)
+      .Case("sp", LLDB_REGNUM_GENERIC_SP)
+      .Case("fp", LLDB_REGNUM_GENERIC_FP)
+      .Case("a0", LLDB_REGNUM_GENERIC_ARG1)
+      .Case("a1", LLDB_REGNUM_GENERIC_ARG2)
+      .Case("a2", LLDB_REGNUM_GENERIC_ARG3)
+      .Case("a3", LLDB_REGNUM_GENERIC_ARG4)
+      .Case("a4", LLDB_REGNUM_GENERIC_ARG5)
+      .Case("a5", LLDB_REGNUM_GENERIC_ARG6)
+      .Case("a6", LLDB_REGNUM_GENERIC_ARG7)
+      .Case("a7", LLDB_REGNUM_GENERIC_ARG8)
+      .Default(LLDB_INVALID_REGNUM);
+}
+
+bool ABISysV_riscv::IsHardFloatProcess() const {
+  bool is_hardfloat = false;
+  ProcessSP process_sp(GetProcessSP());
+  if (process_sp) {
+    const ArchSpec &arch(process_sp->GetTarget().GetArchitecture());
+    if (arch.GetFlags() & ArchSpec::eRISCV_float_abi_single ||
+        arch.GetFlags() & ArchSpec::eRISCV_float_abi_double)
+      is_hardfloat = true;
+  }
+  return is_hardfloat;
+}
+
+ABISP
+ABISysV_riscv::CreateInstance(lldb::ProcessSP process_sp,
+                              const ArchSpec &arch) {
+  if (arch.GetTriple().isRISCV())
+    return ABISP(
+        new ABISysV_riscv(std::move(process_sp), MakeMCRegisterInfo(arch),
+                          arch.GetTriple().getArch() == llvm::Triple::riscv64));
+  return ABISP();
+}
+
+void ABISysV_riscv::Initialize() {
+  PluginManager::RegisterPlugin(
+      GetPluginNameStatic(), "System V ABI for riscv targets", CreateInstance);
+}
+
+void ABISysV_riscv::Terminate() {
+  PluginManager::UnregisterPlugin(CreateInstance);
+}

lldb/source/Plugins/ABI/RISCV/CMakeLists.txt

+add_lldb_library(lldbPluginABIRISCV PLUGIN
+  ABISysV_riscv.cpp
+
+  LINK_LIBS
+    lldbCore
+    lldbSymbol
+    lldbTarget
+  LINK_COMPONENTS
+    Support
+)

lldb/source/Target/Platform.cpp

 
   case llvm::Triple::riscv32:
   case llvm::Triple::riscv64: {
+    static const uint8_t g_riscv_c_opcode[] = {0x02, 0x90}; // c_ebreak
     static const uint8_t g_riscv_opcode[] = {0x73, 0x00, 0x10, 0x00}; // ebreak
-    trap_opcode = g_riscv_opcode;
-    trap_opcode_size = sizeof(g_riscv_opcode);
-  } break;
+    if (arch.GetFlags() & ArchSpec::eRISCV_rvc) {
+      trap_opcode = g_riscv_c_opcode;
+      trap_opcode_size = sizeof(g_riscv_c_opcode);
+    } else {
+      trap_opcode = g_riscv_opcode;
+      trap_opcode_size = sizeof(g_riscv_opcode);
+    }
+    break;
+  }
 
   default:
     return 0;