[LLDB][RISCV] Make software single stepping work

[Emmmer]

Summary

Add:

• EmulateInstructionRISCV, which can be used for riscv32 and riscv64.
• Add unittests for EmulateInstructionRISCV.

Note: Compressed instructions set (RVC) was still not supported in this patch.

[DavidSpickett]

Initial reactions, since I won't get time to review this today.

EmulateInstructionRISCV, which can be used for riscv32 and riscv64.

I know that ARM has tests for EmulateInstructionARM, and I would like to know this is tested. Either the way that does it or with unittests.

handle resume thread error.
Fix possible nullptr dereference.

Are these two generic changes (not specific to riscv)? If so can you make a patch for each. If it can be tested as well then great, if it's some theoretical corner case then should be fine without.

I presume that with this change a bunch of tests started passing? Would be good to summarise that in the commit message. "after this change a further N tests that use single stepping now pass".

[Emmmer]

Address review comments:

• Add unittests for EmulateInstructionRISCV.
• Split "thread error" and "nullptr dereference" as separate PRs.

[DavidSpickett]

lldb/source/Plugins/Instruction/RISCV/EmulateInstructionRISCV.cpp

36

Move this next to first use if possible.

38

What part of the encoding does this help you extract? Can you add a comment with the name of the field as the spec calls it.

39

Also what does this mean?

83

Here we know that reg_encode is not 0, and reg_encode is unsigned so it cannot be negative. So it could just be if (reg_encode <= 31).

109

Does compressed come into this at any point? Just checking since I see a lot of +4. It's fine to say it's not supported at this time.

120

JALR clears the bottom bit, why is this?

On Arm thumb we have bx which can change modes between arm and thumb code. In thumb code the bottom bit is used as a mode bit to say you're in thumb. Is it anything like that?

Please add comments to explain.

126

Is funct3 the name of a field from the spec? If so can you add a comment like "funct3 is the type of the compare", whatever it means.

139

Do you ever expect to hit this? Add an assert if you don't.

146

Nit: put this next to the ReadPC call.

Generally, order the declarations as they are used.

150

Do this check one line up immediately after the ReadPC.

164

I would just merge these into the CompareB below.

182

Can you explain in the comments what RVI and RVA are?

183

What is not certain at this point? It would be good to record that at any point where the spec being in flux is an issue.

E.g. "this code may change once the spec is ratified, at the moment X and Y aspects are not fixed"

Just in case we wanted to fixup individual bits as they get decided on later (or find out where we diverge from the final spec).

188

Just put the array_lengthof call in the for line.

190

I would write this inst & pat.type_mask. I know it works either way but it looks a bit strange in my opinion.

198

0x%x and does not branch:

223

This comment seems redundant given that the function is called DecodeAndExecute.

250

Please add comments to explain this.

I think what you're doing is checking whether the compressed encoding could be valid. We do similair things for Thumb encodings for Arm.

251

Seems easier to write this as != 3.

260

I would change this into an early return style like this:

  bool success = false;
  m_addr = ReadPC(&success);
  if (!success)
    m_addr = LLDB_INVALID_ADDRESS;

    Context ctx;
    ctx.type = eContextReadOpcode;
    ctx.SetNoArgs();
    uint32_t inst = (uint32_t)ReadMemoryUnsigned(ctx, m_addr, 4, 0, &success);
    uint16_t try_rvc = (uint16_t)(inst & 0x0000ffff);
    uint16_t mask = try_rvc & 0b11;
    if (try_rvc != 0 && (mask == 0 || mask == 1 || mask == 2)) {
      m_opcode.SetOpcode16(try_rvc, GetByteOrder());
    } else {
      m_opcode.SetOpcode32(inst, GetByteOrder());
    }

  return true;
}

lldb/source/Plugins/Instruction/RISCV/EmulateInstructionRISCV.h

56

Do we need this wrapper/forwarder? It just passes on the arg unchanged.

lldb/unittests/Instruction/CMakeLists.txt

1–26

Also for AArch64 this should be "AARCH64" or "ARM64" (I think it's the former).

In this context ARM == 32 bit Arm and AArch64 == 64 bit Arm (where elsewhere Arm means both, and I agree, it's confusing).

34

Why do we need this conditional logic here?

Surely unittests don't need a riscv or arm64 host and shouldn't clash with each other, or did you find some issue including both?

[DavidSpickett]

I know this is a flood of comments but the overall impression is that this is mostly fine. A bunch of small things.

Thanks for splitting out the other patches and adding the test cases here.

lldb/source/Plugins/Instruction/RISCV/EmulateInstructionRISCV.cpp

299

Assert here if you do not expect to hit this code. (even if that's just temporary until you emulate more instructions, it's better to know that something needs implementing)

lldb/source/Plugins/Instruction/RISCV/EmulateInstructionRISCV.h

21

You have 2 public markers in the same class with no private between them. Intentional?

lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp

886

Could you use isRISCV here instead?

lldb/unittests/Instruction/RISCV/TestRISCVEmulator.cpp

10

This appears unused.

40

Please explain what this does. Just on the face of it, something a bit dodgy :) (maybe there is a better way to express it, even if it is legit)

46

if (reg == gpr_x0_riscv)
      reg_value.SetUInt(0, reg_info->byte_size);
else
    reg_value.SetUInt(tester->gpr.gpr[reg], reg_info->byte_size);

return true;

56–60

if (reg != gpr_x0_riscv)
  tester->gpr.gpr[reg] = reg_value.GetAsUInt64();

return true;

69

Assert that this was successful.

80

EncodeInstruction? I don't think we need to shorten the names quite this much.

104

You check the ~1 here but your input values don't have the bottom bit set.

Does that mean this test is not covering that aspect of the emulation? Seems like it should be.

107

EncodeBranch

141

testBranch (you get the idea by now)

165

Add comments to explain what these numbers are. With the macro wrapper it's hard to tell what they mean and what this is actually checking.

[DavidSpickett]

Oh and please note in the commit message that this does not support compressed instructions (or does it, I assume no but make it obvious).

[Emmmer]

lldb/source/Plugins/Instruction/RISCV/EmulateInstructionRISCV.cpp

83

Just to make the if condition explicitly exhaustive. Not relying on implicitly derived conditions helps the code survive larger refactorings that may break the implicit conditions.

Sure it can be simplified to reg_encode <= 31, but we should let compilers do it for us rather than doing it manually.

109

No. jal/jalr/b are always 4-byte instructions. Supporting RVC does not need to
change the +4 here.

120

It is like the mode bit in ARM but more flexible since riscv does not have two modes.

The spec says auxiliary information can be stored to LSB in function addresses (like JIT compilers can use this bit). It is open to developers.

126

Yes. funct3 is a widely used name in spec meaning 3-bits function selector

183

It's a pity that the whole debug spec was not useable (not implemented by any emulator nor hardware).

We don't use this code for platforms which implemented the debug spec. But for platforms without the debug spec we still need this emulation. We don't need to change the code here in either case in the future. So I guess it's fine to not record the issue.

223

A sense of ceremony haha. Will delete the comment accordingly.

250

Exactly! Comments are added now.

lldb/source/Plugins/Instruction/RISCV/EmulateInstructionRISCV.h

21

Yes, to distinguish public static members from public instance members.

56

Yes. SupportsThisInstructionType is also used in CreateInstance which is a static method.

lldb/unittests/Instruction/CMakeLists.txt

1–26

This code confused me a lot and thank you for the explanation.
I am going to check ARM and AARCH64 explicitly.

34

I don't know either. I see the original code checking TARGETS_TO_BUILD and there might be special consideration I suppose?

lldb/unittests/Instruction/RISCV/TestRISCVEmulator.cpp

10

nice catch!

40

It is actually an always-success dynamic_cast because:

• The RISCVEmulatorTester is derived from EmulateInstruction
• These callbacks are set in the constructor of RISCVEmulatorTester
• All EmulateInstruction instances created in this unittest are always RISCVEmulatorTester

Signature of these callbacks is not polymorphic (like for Java we have <T extends EmulateInstruction> with bounded constraints.) and that's the reason why we need this cast. We may turn it into a template but it requires more effort.

I also noticed that there's a void *baton in parameters. But using that field looks more terrible. I guess we could have a userdata whose type was parametrized.

80

The I stands for I-Type, a riscv instruction format. Like EncodeB below.

104

0x1024 - 255 = 3877
(0x1024 - 255) & ~1 = 3876

I think it is covered 😃

107

The B stands for B-Type which works for all B-type instructions.

Branch instructions are all of B-type so renaming it to EncodeBranch seems Okay right now, but we may add more B-type instructions in the future.

141

Yes this should be Branch since it only tests branch instructions.

[Emmmer]

address review comments

[DavidSpickett]

Are we expecting this code to only ever be given JAL or JALR at this time?

If it's the case that anything else will just return false or do nothing, might be worth adding a couple of tests that check that encodings that aren't branches just do nothing or fail. Obviously not every encoding maybe just flip 1 bit in the instruction type field just as a smoke test for each of jal and jalr.

lldb/source/Plugins/Instruction/RISCV/EmulateInstructionRISCV.cpp

223

Now worries. Usually I end up with these because I wrote out the steps as comments first and forget to delete them.

lldb/source/Plugins/Instruction/RISCV/EmulateInstructionRISCV.h

21

Got it, I missed that.

lldb/unittests/Instruction/CMakeLists.txt

34

Sorry I completely missed that first line (the diff split at that point). Right, so you're just adding the riscv equivalent of what's already there. So actually just leave the ARM bit as is, I'll do some builds and find out whether it needs to change for AArch64.

lldb/unittests/Instruction/RISCV/TestRISCVEmulator.cpp

40

But using that field looks more terrible. I guess we could have a userdata whose type was parametrized.

Yeah there's a lot of...not ideal patterns in this area but I wouldn't pay much attention to them.

Without opting into llvm's rtti equivalent I guess this is the best way to do it.

80

Cool. It's mostly that EncodeB looks like the author got distracted half way through the name. But anyway I'm not familiar with the spec so if it matches that then great.

99

Add a comment like "JALR will always zero the bottom bit of the target".

104

Doh, yes I wasn't looking at the offset.

162

Now I understand what's going on. I thought that this somehow included non branch encodings when in fact what it's doing is saying for a branch equals we should only branch if the operands are equal.

[Emmmer]

lldb/unittests/Instruction/CMakeLists.txt

34

Oh... thank you! I am gonna revert the change for AArch64.

lldb/unittests/Instruction/RISCV/TestRISCVEmulator.cpp

80

You're right, the name looks unfriendly to other developers. So I changed its name to EncodeIType (also for EncodeBType) to avoid confusion.

[Emmmer]

address review comments

[DavidSpickett]

LGTM

lldb/source/Plugins/Instruction/CMakeLists.txt

 add_subdirectory(ARM)
 add_subdirectory(ARM64)
 add_subdirectory(MIPS)
 add_subdirectory(MIPS64)
 add_subdirectory(PPC64)
+add_subdirectory(RISCV)

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

+add_lldb_library(lldbPluginInstructionRISCV PLUGIN
+  EmulateInstructionRISCV.cpp
+
+  LINK_LIBS
+    lldbCore
+    lldbInterpreter
+    lldbSymbol
+    lldbPluginProcessUtility
+  LINK_COMPONENTS
+    Support
+  )

lldb/source/Plugins/Instruction/RISCV/EmulateInstructionRISCV.h

+//===-- EmulateInstructionRISCV.h -----------------------------------------===//
+//
+// 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_INSTRUCTION_RISCV_EMULATEINSTRUCTIONRISCV_H
+#define LLDB_SOURCE_PLUGINS_INSTRUCTION_RISCV_EMULATEINSTRUCTIONRISCV_H
+
+#include "lldb/Core/EmulateInstruction.h"
+#include "lldb/Interpreter/OptionValue.h"
+#include "lldb/Utility/Log.h"
+#include "lldb/Utility/Status.h"
+
+namespace lldb_private {
+
+class EmulateInstructionRISCV : public EmulateInstruction {
+public:
+  static llvm::StringRef GetPluginNameStatic() { return "riscv"; }
+
+  static llvm::StringRef GetPluginDescriptionStatic() {
+    return "Emulate instructions for the RISC-V architecture.";
+  }
+
+  static bool SupportsThisInstructionType(InstructionType inst_type) {
+    switch (inst_type) {
+    case eInstructionTypeAny:
+    case eInstructionTypePCModifying:
+      return true;
+    case eInstructionTypePrologueEpilogue:
+    case eInstructionTypeAll:
+    default:
+      return false;
+    }
+  }
+
+  static bool SupportsThisArch(const ArchSpec &arch);
+
+  static lldb_private::EmulateInstruction *
+  CreateInstance(const lldb_private::ArchSpec &arch, InstructionType inst_type);
+
+  static void Initialize();
+
+  static void Terminate();
+
+public:
+  EmulateInstructionRISCV(const ArchSpec &arch) : EmulateInstruction(arch) {}
+
+  llvm::StringRef GetPluginName() override { return GetPluginNameStatic(); }
+
+  bool SupportsEmulatingInstructionsOfType(InstructionType inst_type) override {
+    return SupportsThisInstructionType(inst_type);
+  }
+
+  bool SetTargetTriple(const ArchSpec &arch) override;
+  bool ReadInstruction() override;
+  bool EvaluateInstruction(uint32_t options) override;
+  bool TestEmulation(Stream *out_stream, ArchSpec &arch,
+                     OptionValueDictionary *test_data) override;
+  bool GetRegisterInfo(lldb::RegisterKind reg_kind, uint32_t reg_num,
+                       RegisterInfo &reg_info) override;
+
+  lldb::addr_t ReadPC(bool *success);
+  bool WritePC(lldb::addr_t pc);
+  bool DecodeAndExecute(uint32_t inst, bool ignore_cond);
+};
+
+} // namespace lldb_private
+
+#endif // LLDB_SOURCE_PLUGINS_INSTRUCTION_RISCV_EMULATEINSTRUCTIONRISCV_H

lldb/source/Plugins/Instruction/RISCV/EmulateInstructionRISCV.cpp

+//===-- EmulateInstructionRISCV.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 <cstdlib>
+
+#include "EmulateInstructionRISCV.h"
+#include "Plugins/Process/Utility/RegisterInfoPOSIX_riscv64.h"
+#include "Plugins/Process/Utility/lldb-riscv-register-enums.h"
+
+#include "lldb/Core/Address.h"
+#include "lldb/Core/PluginManager.h"
+#include "lldb/Interpreter/OptionValueArray.h"
+#include "lldb/Interpreter/OptionValueDictionary.h"
+#include "lldb/Symbol/UnwindPlan.h"
+#include "lldb/Utility/ArchSpec.h"
+#include "lldb/Utility/LLDBLog.h"
+#include "lldb/Utility/RegisterValue.h"
+#include "lldb/Utility/Stream.h"
+
+#include "llvm/ADT/STLExtras.h"
+#include "llvm/Support/MathExtras.h"
+
+using namespace lldb;
+using namespace lldb_private;
+
+LLDB_PLUGIN_DEFINE_ADV(EmulateInstructionRISCV, InstructionRISCV)
+
+namespace lldb_private {
+
+// Masks for detecting instructions types. According to riscv-spec Chap 26.
+constexpr uint32_t I_MASK = 0b111000001111111;
+constexpr uint32_t J_MASK = 0b000000001111111;
+// no funct3 in the b-mask because the logic executing B<CMP> is quite similar.
+constexpr uint32_t B_MASK = 0b000000001111111;
+
+// The funct3 is the type of compare in B<CMP> instructions.
+// funct3 means "3-bits function selector", which RISC-V ISA uses as minor
+// opcode. It reuses the major opcode encoding space.
+constexpr uint32_t BEQ = 0b000;
+constexpr uint32_t BNE = 0b001;
+constexpr uint32_t BLT = 0b100;
+constexpr uint32_t BGE = 0b101;
+constexpr uint32_t BLTU = 0b110;
+constexpr uint32_t BGEU = 0b111;
+
+constexpr uint32_t DecodeRD(uint32_t inst) { return (inst & 0xF80) >> 7; }
+constexpr uint32_t DecodeRS1(uint32_t inst) { return (inst & 0xF8000) >> 15; }
+constexpr uint32_t DecodeRS2(uint32_t inst) { return (inst & 0x1F00000) >> 20; }
+constexpr uint32_t DecodeFunct3(uint32_t inst) { return (inst & 0x7000) >> 12; }
+
+constexpr int32_t SignExt(uint32_t imm) { return int32_t(imm); }
+
+constexpr uint32_t DecodeJImm(uint32_t inst) {
+  return (uint64_t(int64_t(int32_t(inst & 0x80000000)) >> 11)) // imm[20]
+         | (inst & 0xff000)                                    // imm[19:12]
+         | ((inst >> 9) & 0x800)                               // imm[11]
+         | ((inst >> 20) & 0x7fe);                             // imm[10:1]
+}
+
+constexpr uint32_t DecodeIImm(uint32_t inst) {
+  return int64_t(int32_t(inst)) >> 20; // imm[11:0]
+}
+
+constexpr uint32_t DecodeBImm(uint32_t inst) {
+  return (uint64_t(int64_t(int32_t(inst & 0x80000000)) >> 19)) // imm[12]
+         | ((inst & 0x80) << 4)                                // imm[11]
+         | ((inst >> 20) & 0x7e0)                              // imm[10:5]
+         | ((inst >> 7) & 0x1e);                               // imm[4:1]
+}
+
+static uint32_t GPREncodingToLLDB(uint32_t reg_encode) {
+  if (reg_encode == 0)
+    return gpr_x0_riscv;
+  if (reg_encode >= 1 && reg_encode <= 31)
+    return gpr_x1_riscv + reg_encode - 1;
+  return LLDB_INVALID_REGNUM;
+}
+
+static bool ReadRegister(EmulateInstructionRISCV *emulator, uint32_t reg_encode,
+                         RegisterValue &value) {
+  uint32_t lldb_reg = GPREncodingToLLDB(reg_encode);
+  return emulator->ReadRegister(eRegisterKindLLDB, lldb_reg, value);
+}
+
+static bool WriteRegister(EmulateInstructionRISCV *emulator,
+                          uint32_t reg_encode, const RegisterValue &value) {
+  uint32_t lldb_reg = GPREncodingToLLDB(reg_encode);
+  EmulateInstruction::Context ctx;
+  ctx.type = EmulateInstruction::eContextRegisterStore;
+  ctx.SetNoArgs();
+  return emulator->WriteRegister(ctx, eRegisterKindLLDB, lldb_reg, value);
+}
+
+static bool ExecJAL(EmulateInstructionRISCV *emulator, uint32_t inst, bool) {
+  bool success = false;
+  int64_t offset = SignExt(DecodeJImm(inst));
+  int64_t pc = emulator->ReadPC(&success);
+  return success && emulator->WritePC(pc + offset) &&
+         WriteRegister(emulator, DecodeRD(inst),
+                       RegisterValue(uint64_t(pc + 4)));
+}
+
+static bool ExecJALR(EmulateInstructionRISCV *emulator, uint32_t inst, bool) {
+  int64_t offset = SignExt(DecodeIImm(inst));
+  RegisterValue value;
+  if (!ReadRegister(emulator, DecodeRS1(inst), value))
+    return false;
+  bool success = false;
+  int64_t pc = emulator->ReadPC(&success);
+  int64_t rs1 = int64_t(value.GetAsUInt64());
+  // JALR clears the bottom bit. According to riscv-spec:
+  // "The JALR instruction now clears the lowest bit of the calculated target
+  // address, to simplify hardware and to allow auxiliary information to be
+  // stored in function pointers."
+  return emulator->WritePC((rs1 + offset) & ~1) &&
+         WriteRegister(emulator, DecodeRD(inst),
+                       RegisterValue(uint64_t(pc + 4)));
+}
+
+static bool CompareB(uint64_t rs1, uint64_t rs2, uint32_t funct3) {
+  switch (funct3) {
+  case BEQ:
+    return rs1 == rs2;
+  case BNE:
+    return rs1 != rs2;
+  case BLT:
+    return int64_t(rs1) < int64_t(rs2);
+  case BGE:
+    return int64_t(rs1) >= int64_t(rs2);
+  case BLTU:
+    return rs1 < rs2;
+  case BGEU:
+    return rs1 >= rs2;
+  default:
+    llvm_unreachable("unexpected funct3");
+  }
+}
+
+static bool ExecB(EmulateInstructionRISCV *emulator, uint32_t inst,
+                  bool ignore_cond) {
+  bool success = false;
+  uint64_t pc = emulator->ReadPC(&success);
+  if (!success)
+    return false;
+
+  uint64_t offset = SignExt(DecodeBImm(inst));
+  uint64_t target = pc + offset;
+  if (ignore_cond)
+    return emulator->WritePC(target);
+
+  RegisterValue value1;
+  RegisterValue value2;
+  if (!ReadRegister(emulator, DecodeRS1(inst), value1) ||
+      !ReadRegister(emulator, DecodeRS2(inst), value2))
+    return false;
+
+  uint32_t funct3 = DecodeFunct3(inst);
+  if (CompareB(value1.GetAsUInt64(), value2.GetAsUInt64(), funct3))
+    return emulator->WritePC(target);
+
+  return true;
+}
+
+struct InstrPattern {
+  const char *name;
+  /// Bit mask to check the type of a instruction (B-Type, I-Type, J-Type, etc.)
+  uint32_t type_mask;
+  /// Characteristic value after bitwise-and with type_mask.
+  uint32_t eigen;
+  bool (*exec)(EmulateInstructionRISCV *emulator, uint32_t inst,
+               bool ignore_cond);
+};
+
+static InstrPattern PATTERNS[] = {
+    {"JAL", J_MASK, 0b1101111, ExecJAL},
+    {"JALR", I_MASK, 0b000000001100111, ExecJALR},
+    {"B<CMP>", B_MASK, 0b1100011, ExecB},
+    // TODO: {LR/SC}.{W/D} and ECALL
+};
+
+/// This function only determines the next instruction address for software
+/// sigle stepping by emulating branching instructions including:
+/// - from Base Instruction Set  : JAL, JALR, B<CMP>, ECALL
+/// - from Atomic Instruction Set: LR -> BNE -> SC -> BNE
+/// We will get rid of this tedious code when the riscv debug spec is ratified.
+bool EmulateInstructionRISCV::DecodeAndExecute(uint32_t inst,
+                                               bool ignore_cond) {
+  Log *log = GetLog(LLDBLog::Process | LLDBLog::Breakpoints);
+  for (int i = 0; i < llvm::array_lengthof(PATTERNS); ++i) {
+    const InstrPattern &pat = PATTERNS[i];
+    if ((inst & pat.type_mask) == pat.eigen) {
+      LLDB_LOGF(log, "EmulateInstructionRISCV::%s: inst(%x) was decoded to %s",
+                __FUNCTION__, inst, pat.name);
+      return pat.exec(this, inst, ignore_cond);
+    }
+  }
+
+  LLDB_LOGF(log,
+            "EmulateInstructionRISCV::%s: inst(0x%x) does not branch: "
+            "no need to calculate the next pc address which is trivial.",
+            __FUNCTION__, inst);
+  return true;
+}
+
+bool EmulateInstructionRISCV::EvaluateInstruction(uint32_t options) {
+  uint32_t inst_size = m_opcode.GetByteSize();
+  uint32_t inst = m_opcode.GetOpcode32();
+  bool increase_pc = options & eEmulateInstructionOptionAutoAdvancePC;
+  bool ignore_cond = options & eEmulateInstructionOptionIgnoreConditions;
+  bool success = false;
+
+  lldb::addr_t old_pc = 0;
+  if (increase_pc) {
+    old_pc = ReadPC(&success);
+    if (!success)
+      return false;
+  }
+
+  if (inst_size == 2) {
+    // TODO: execute RVC
+    return false;
+  }
+
+  success = DecodeAndExecute(inst, ignore_cond);
+  if (!success)
+    return false;
+
+  if (increase_pc) {
+    lldb::addr_t new_pc = ReadPC(&success);
+    if (!success)
+      return false;
+
+    if (new_pc == old_pc) {
+      if (!WritePC(old_pc + inst_size))
+        return false;
+    }
+  }
+  return true;
+}
+
+bool EmulateInstructionRISCV::ReadInstruction() {
+  bool success = false;
+  m_addr = ReadPC(&success);
+  if (!success) {
+    m_addr = LLDB_INVALID_ADDRESS;
+    return false;
+  }
+
+  Context ctx;
+  ctx.type = eContextReadOpcode;
+  ctx.SetNoArgs();
+  uint32_t inst = (uint32_t)ReadMemoryUnsigned(ctx, m_addr, 4, 0, &success);
+  uint16_t try_rvc = (uint16_t)(inst & 0x0000ffff);
+  // check whether the compressed encode could be valid
+  uint16_t mask = try_rvc & 0b11;
+  if (try_rvc != 0 && mask != 3) {
+    m_opcode.SetOpcode16(try_rvc, GetByteOrder());
+  } else {
+    m_opcode.SetOpcode32(inst, GetByteOrder());
+  }
+
+  return true;
+}
+
+lldb::addr_t EmulateInstructionRISCV::ReadPC(bool *success) {
+  return ReadRegisterUnsigned(eRegisterKindGeneric, LLDB_REGNUM_GENERIC_PC,
+                              LLDB_INVALID_ADDRESS, success);
+}
+
+bool EmulateInstructionRISCV::WritePC(lldb::addr_t pc) {
+  EmulateInstruction::Context ctx;
+  ctx.type = eContextAdvancePC;
+  ctx.SetNoArgs();
+  return WriteRegisterUnsigned(ctx, eRegisterKindGeneric,
+                               LLDB_REGNUM_GENERIC_PC, pc);
+}
+
+bool EmulateInstructionRISCV::GetRegisterInfo(lldb::RegisterKind reg_kind,
+                                              uint32_t reg_index,
+                                              RegisterInfo &reg_info) {
+  if (reg_kind == eRegisterKindGeneric) {
+    switch (reg_index) {
+    case LLDB_REGNUM_GENERIC_PC:
+      reg_kind = eRegisterKindLLDB;
+      reg_index = gpr_pc_riscv;
+      break;
+    case LLDB_REGNUM_GENERIC_SP:
+      reg_kind = eRegisterKindLLDB;
+      reg_index = gpr_sp_riscv;
+      break;
+    case LLDB_REGNUM_GENERIC_FP:
+      reg_kind = eRegisterKindLLDB;
+      reg_index = gpr_fp_riscv;
+      break;
+    case LLDB_REGNUM_GENERIC_RA:
+      reg_kind = eRegisterKindLLDB;
+      reg_index = gpr_ra_riscv;
+      break;
+    // We may handle LLDB_REGNUM_GENERIC_ARGx when more instructions are
+    // supported.
+    default:
+      llvm_unreachable("unsupported register");
+    }
+  }
+
+  const RegisterInfo *array =
+      RegisterInfoPOSIX_riscv64::GetRegisterInfoPtr(m_arch);
+  const uint32_t length =
+      RegisterInfoPOSIX_riscv64::GetRegisterInfoCount(m_arch);
+
+  if (reg_index >= length || reg_kind != eRegisterKindLLDB)
+    return false;
+
+  reg_info = array[reg_index];
+  return true;
+}
+
+bool EmulateInstructionRISCV::SetTargetTriple(const ArchSpec &arch) {
+  return SupportsThisArch(arch);
+}
+
+bool EmulateInstructionRISCV::TestEmulation(Stream *out_stream, ArchSpec &arch,
+                                            OptionValueDictionary *test_data) {
+  return false;
+}
+
+void EmulateInstructionRISCV::Initialize() {
+  PluginManager::RegisterPlugin(GetPluginNameStatic(),
+                                GetPluginDescriptionStatic(), CreateInstance);
+}
+
+void EmulateInstructionRISCV::Terminate() {
+  PluginManager::UnregisterPlugin(CreateInstance);
+}
+
+lldb_private::EmulateInstruction *
+EmulateInstructionRISCV::CreateInstance(const ArchSpec &arch,
+                                        InstructionType inst_type) {
+  if (EmulateInstructionRISCV::SupportsThisInstructionType(inst_type) &&
+      SupportsThisArch(arch)) {
+    return new EmulateInstructionRISCV(arch);
+  }
+
+  return nullptr;
+}
+
+bool EmulateInstructionRISCV::SupportsThisArch(const ArchSpec &arch) {
+  return arch.GetTriple().isRISCV();
+}
+
+} // namespace lldb_private

lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp

bool NativeProcessLinux::MonitorClone(NativeThreadLinux &parent,
   default:
     llvm_unreachable("unknown clone_info.event");
   }
 
   return true;
 }
 
 bool NativeProcessLinux::SupportHardwareSingleStepping() const {
-  if (m_arch.GetMachine() == llvm::Triple::arm || m_arch.IsMIPS())
+  if (m_arch.IsMIPS() || m_arch.GetMachine() == llvm::Triple::arm ||
+      m_arch.GetTriple().isRISCV())
     return false;
   return true;
 }
 
 Status NativeProcessLinux::Resume(const ResumeActionList &resume_actions) {
   Log *log = GetLog(POSIXLog::Process);
   LLDB_LOG(log, "pid {0}", GetID());
 

lldb/source/Plugins/Process/Utility/NativeProcessSoftwareSingleStep.cpp

Status NativeProcessSoftwareSingleStep::SetupSoftwareSingleStepping(
   if (arch.GetMachine() == llvm::Triple::arm) {
     if (next_flags & 0x20) {
       // Thumb mode
       size_hint = 2;
     } else {
       // Arm mode
       size_hint = 4;
     }
-  } else if (arch.IsMIPS() || arch.GetTriple().isPPC64())
+  } else if (arch.IsMIPS() || arch.GetTriple().isPPC64() ||
+             arch.GetTriple().isRISCV())
     size_hint = 4;
   error = process.SetBreakpoint(next_pc, size_hint, /*hardware=*/false);
 
   // If setting the breakpoint fails because next_pc is out of the address
   // space, ignore it and let the debugee segfault.
   if (error.GetError() == EIO || error.GetError() == EFAULT) {
     return Status();
   } else if (error.Fail())
     return error;
 
   m_threads_stepping_with_breakpoint.insert({thread.GetID(), next_pc});
 
   return Status();
 }

lldb/tools/lldb-server/CMakeLists.txt

add_lldb_tool(lldb-server
     LINK_LIBS
       lldbHost
       lldbInitialization
       lldbVersion
       ${LLDB_PLUGINS}
       lldbPluginInstructionARM
       lldbPluginInstructionMIPS
       lldbPluginInstructionMIPS64
+      lldbPluginInstructionRISCV
       ${LLDB_SYSTEM_LIBS}
 
     LINK_COMPONENTS
       Option
       Support
 )
 
 add_dependencies(lldb-server
   LLGSOptionsTableGen
   ${tablegen_deps}
 )
 target_include_directories(lldb-server PRIVATE "${LLDB_SOURCE_DIR}/source")
 target_link_libraries(lldb-server PRIVATE ${LLDB_SYSTEM_LIBS})

lldb/tools/lldb-server/SystemInitializerLLGS.cpp

 #endif
 
 #if defined(__mips__) || defined(mips) || defined(__mips) ||                   \
     defined(__MIPS__) || defined(_M_MIPS) || defined(LLDB_TARGET_MIPS64)
 #define LLDB_TARGET_MIPS
 #include "Plugins/Instruction/MIPS/EmulateInstructionMIPS.h"
 #endif
 
+#if defined(__riscv)
+#define LLDB_TARGET_RISCV
+#include "Plugins/Instruction/RISCV/EmulateInstructionRISCV.h"
+#endif
+
 using namespace lldb_private;
 
 llvm::Error SystemInitializerLLGS::Initialize() {
   if (auto e = SystemInitializerCommon::Initialize())
     return e;
 
   HostObjectFile::Initialize();
 
 #if defined(LLDB_TARGET_ARM) || defined(LLDB_TARGET_ARM64)
   EmulateInstructionARM::Initialize();
 #endif
 #if defined(LLDB_TARGET_MIPS) || defined(LLDB_TARGET_MIPS64)
   EmulateInstructionMIPS::Initialize();
 #endif
 #if defined(LLDB_TARGET_MIPS64)
   EmulateInstructionMIPS64::Initialize();
 #endif
+#if defined(LLDB_TARGET_RISCV)
+  EmulateInstructionRISCV::Initialize();
+#endif
 
   return llvm::Error::success();
 }
 
 void SystemInitializerLLGS::Terminate() {
   HostObjectFile::Terminate();
 
 #if defined(LLDB_TARGET_ARM) || defined(LLDB_TARGET_ARM64)
   EmulateInstructionARM::Terminate();
 #endif
 #if defined(LLDB_TARGET_MIPS) || defined(LLDB_TARGET_MIPS64)
   EmulateInstructionMIPS::Terminate();
 #endif
 #if defined(LLDB_TARGET_MIPS64)
   EmulateInstructionMIPS64::Terminate();
 #endif
+#if defined(LLDB_TARGET_RISCV)
+  EmulateInstructionRISCV::Terminate();
+#endif
 
   SystemInitializerCommon::Terminate();
 }

lldb/unittests/Instruction/CMakeLists.txt

-if("ARM" IN_LIST LLVM_TARGETS_TO_BUILD)
+set(FILES "")
+set(DEPS "")
+
+if ("ARM" IN_LIST LLVM_TARGETS_TO_BUILD)
+  list(APPEND FILES ARM64/TestAArch64Emulator.cpp)
+  list(APPEND DEPS lldbPluginInstructionARM64)
+endif ()
+
+if ("RISCV" IN_LIST LLVM_TARGETS_TO_BUILD)
+  list(APPEND FILES RISCV/TestRISCVEmulator.cpp)
+  list(APPEND DEPS lldbPluginInstructionRISCV)
+endif ()
+
+list(LENGTH FILES LISTLEN)
+
+if (LISTLEN GREATER 0)
   add_lldb_unittest(EmulatorTests
-    TestAArch64Emulator.cpp
+    ${FILES}
+
     LINK_LIBS
       lldbCore
       lldbSymbol
       lldbTarget
-      lldbPluginInstructionARM64
+      ${DEPS}
     LINK_COMPONENTS
       Support
-      ${LLVM_TARGETS_TO_BUILD})
-endif()
+      ${LLVM_TARGETS_TO_BUILD}
+    )
+endif ()

lldb/unittests/Instruction/RISCV/TestRISCVEmulator.cpp

+//===-- TestRISCVEmulator.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 "gtest/gtest.h"
+
+#include "lldb/Core/Address.h"
+#include "lldb/Core/Disassembler.h"
+#include "lldb/Core/PluginManager.h"
+#include "lldb/Target/ExecutionContext.h"
+#include "lldb/Utility/ArchSpec.h"
+#include "lldb/Utility/RegisterValue.h"
+
+#include "Plugins/Instruction/RISCV/EmulateInstructionRISCV.h"
+#include "Plugins/Process/Utility/RegisterInfoPOSIX_riscv64.h"
+#include "Plugins/Process/Utility/lldb-riscv-register-enums.h"
+
+using namespace lldb;
+using namespace lldb_private;
+
+struct RISCVEmulatorTester : public EmulateInstructionRISCV, testing::Test {
+  RegisterInfoPOSIX_riscv64::GPR gpr;
+
+  RISCVEmulatorTester()
+      : EmulateInstructionRISCV(ArchSpec("riscv64-unknown-linux-gnu")) {
+    EmulateInstruction::SetReadRegCallback(ReadRegisterCallback);
+    EmulateInstruction::SetWriteRegCallback(WriteRegisterCallback);
+  }
+
+  static bool ReadRegisterCallback(EmulateInstruction *instruction, void *baton,
+                                   const RegisterInfo *reg_info,
+                                   RegisterValue &reg_value) {
+    RISCVEmulatorTester *tester = (RISCVEmulatorTester *)instruction;
+    uint32_t reg = reg_info->kinds[eRegisterKindLLDB];
+    if (reg == gpr_x0_riscv)
+      reg_value.SetUInt(0, reg_info->byte_size);
+    else
+      reg_value.SetUInt(tester->gpr.gpr[reg], reg_info->byte_size);
+    return true;
+  }
+
+  static bool WriteRegisterCallback(EmulateInstruction *instruction,
+                                    void *baton, const Context &context,
+                                    const RegisterInfo *reg_info,
+                                    const RegisterValue &reg_value) {
+    RISCVEmulatorTester *tester = (RISCVEmulatorTester *)instruction;
+    uint32_t reg = reg_info->kinds[eRegisterKindLLDB];
+    if (reg != gpr_x0_riscv)
+      tester->gpr.gpr[reg] = reg_value.GetAsUInt64();
+    return true;
+  }
+};
+
+TEST_F(RISCVEmulatorTester, testJAL) {
+  lldb::addr_t old_pc = 0x114514;
+  WritePC(old_pc);
+  // jal x1, -6*4
+  uint32_t inst = 0b11111110100111111111000011101111;
+  ASSERT_TRUE(DecodeAndExecute(inst, false));
+  auto x1 = gpr.gpr[1];
+
+  bool success = false;
+  auto pc = ReadPC(&success);
+
+  ASSERT_TRUE(success);
+  ASSERT_EQ(x1, old_pc + 4);
+  ASSERT_EQ(pc, old_pc + (-6 * 4));
+}
+
+constexpr uint32_t EncodeIType(uint32_t opcode, uint32_t funct3, uint32_t rd,
+                               uint32_t rs1, uint32_t imm) {
+  return imm << 20 | rs1 << 15 | funct3 << 12 | rd << 7 | opcode;
+}
+
+constexpr uint32_t JALR(uint32_t rd, uint32_t rs1, int32_t offset) {
+  return EncodeIType(0b1100111, 0, rd, rs1, uint32_t(offset));
+}
+
+TEST_F(RISCVEmulatorTester, testJALR) {
+  lldb::addr_t old_pc = 0x114514;
+  lldb::addr_t old_x2 = 0x1024;
+  WritePC(old_pc);
+  gpr.gpr[2] = old_x2;
+  // jalr x1, x2(-255)
+  uint32_t inst = JALR(1, 2, -255);
+  ASSERT_TRUE(DecodeAndExecute(inst, false));
+  auto x1 = gpr.gpr[1];
+
+  bool success = false;
+  auto pc = ReadPC(&success);
+
+  ASSERT_TRUE(success);
+  ASSERT_EQ(x1, old_pc + 4);
+  // JALR always zeros the bottom bit of the target address.
+  ASSERT_EQ(pc, (old_x2 + (-255)) & (~1));
+}
+
+constexpr uint32_t EncodeBType(uint32_t opcode, uint32_t funct3, uint32_t rs1,
+                               uint32_t rs2, uint32_t imm) {
+  uint32_t bimm = (imm & (0b1 << 11)) >> 4 | (imm & (0b11110)) << 7 |
+                  (imm & (0b111111 << 5)) << 20 | (imm & (0b1 << 12)) << 19;
+
+  return rs2 << 20 | rs1 << 15 | funct3 << 12 | opcode | bimm;
+}
+
+constexpr uint32_t BEQ(uint32_t rs1, uint32_t rs2, int32_t offset) {
+  return EncodeBType(0b1100011, 0b000, rs1, rs2, uint32_t(offset));
+}
+
+constexpr uint32_t BNE(uint32_t rs1, uint32_t rs2, int32_t offset) {
+  return EncodeBType(0b1100011, 0b001, rs1, rs2, uint32_t(offset));
+}
+
+constexpr uint32_t BLT(uint32_t rs1, uint32_t rs2, int32_t offset) {
+  return EncodeBType(0b1100011, 0b100, rs1, rs2, uint32_t(offset));
+}
+
+constexpr uint32_t BGE(uint32_t rs1, uint32_t rs2, int32_t offset) {
+  return EncodeBType(0b1100011, 0b101, rs1, rs2, uint32_t(offset));
+}
+
+constexpr uint32_t BLTU(uint32_t rs1, uint32_t rs2, int32_t offset) {
+  return EncodeBType(0b1100011, 0b110, rs1, rs2, uint32_t(offset));
+}
+
+constexpr uint32_t BGEU(uint32_t rs1, uint32_t rs2, int32_t offset) {
+  return EncodeBType(0b1100011, 0b111, rs1, rs2, uint32_t(offset));
+}
+
+using EncoderB = uint32_t (*)(uint32_t rs1, uint32_t rs2, int32_t offset);
+
+void testBranch(RISCVEmulatorTester *tester, EncoderB encoder, bool branched,
+                uint64_t rs1, uint64_t rs2) {
+  // prepare test registers
+  lldb::addr_t old_pc = 0x114514;
+  tester->WritePC(old_pc);
+  tester->gpr.gpr[1] = rs1;
+  tester->gpr.gpr[2] = rs2;
+  // b<cmp> x1, x2, (-256)
+  uint32_t inst = encoder(1, 2, -256);
+  ASSERT_TRUE(tester->DecodeAndExecute(inst, false));
+  bool success = false;
+  auto pc = tester->ReadPC(&success);
+  ASSERT_TRUE(success);
+  ASSERT_EQ(pc, old_pc + (branched ? (-256) : 0));
+}
+
+#define GEN_BRANCH_TEST(name, rs1, rs2_branched, rs2_continued)                \
+  TEST_F(RISCVEmulatorTester, test##name##Branched) {                          \
+    testBranch(this, name, true, rs1, rs2_branched);                           \
+  }                                                                            \
+  TEST_F(RISCVEmulatorTester, test##name##Continued) {                         \
+    testBranch(this, name, false, rs1, rs2_continued);                         \
+  }
+
+// GEN_BRANCH_TEST(opcode, imm1, imm2, imm3):
+// It should branch for instruction `opcode imm1, imm2`
+// It should do nothing for instruction `opcode imm1, imm3`
+GEN_BRANCH_TEST(BEQ, 1, 1, 0)
+GEN_BRANCH_TEST(BNE, 1, 0, 1)
+GEN_BRANCH_TEST(BLT, -2, 1, -3)
+GEN_BRANCH_TEST(BGE, -2, -3, 1)
+GEN_BRANCH_TEST(BLTU, -2, -1, 1)
+GEN_BRANCH_TEST(BGEU, -2, 1, -1)
+
+void testNothing(RISCVEmulatorTester *tester, uint32_t inst) {
+  lldb::addr_t old_pc = 0x114514;
+  tester->WritePC(old_pc);
+  tester->SetInstruction(Opcode(inst, tester->GetByteOrder()),
+                         LLDB_INVALID_ADDRESS, nullptr);
+  ASSERT_TRUE(tester->EvaluateInstruction(0));
+  bool success = false;
+  auto pc = tester->ReadPC(&success);
+  ASSERT_TRUE(success);
+  ASSERT_EQ(pc, old_pc);
+  ASSERT_TRUE(
+      tester->EvaluateInstruction(eEmulateInstructionOptionAutoAdvancePC));
+  pc = tester->ReadPC(&success);
+  ASSERT_TRUE(success);
+  ASSERT_EQ(pc, old_pc + 4);
+}
+
+#define GEN_NOTHING_TEST(name, inst)                                           \
+  TEST_F(RISCVEmulatorTester, testDoNothing_##name) { testNothing(this, inst); }
+
+// GEN_NOTHING_TEST(name, inst):
+// It should do nothing (except increasing pc) for instruction `inst`
+GEN_NOTHING_TEST(mv, 0x01813083)   // mv a0, a5
+GEN_NOTHING_TEST(li, 0x00078513)   // li a5, 0
+GEN_NOTHING_TEST(sd, 0x02010413)   // sd s0, sp(16)
+GEN_NOTHING_TEST(lw, 0x0007879b)   // lw a5, s0(-20)
+GEN_NOTHING_TEST(addi, 0x00113423) // addi sp, sp, -16