[lldb][riscv] Fix setting breakpoint for undecoded instruction

Copy gdb behaviour:
For RISCV we can set breakpoint even for unknown instruction, as
RISCV encoding have information about size of instruction.

Author: ita-sc

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

@@ -624,9 +624,26 @@ std::optional<DecodeResult> EmulateInstructionRISCV::Decode(uint32_t inst) {
   uint16_t try_rvc = uint16_t(inst & 0x0000ffff);
   // check whether the compressed encode could be valid
   uint16_t mask = try_rvc & 0b11;
-  bool is_rvc = try_rvc != 0 && mask != 3;
   uint8_t inst_type = RV64;
 
+  // Try to get size of RISCV instruction.
+  // 1.2 Instruction Length Encoding
+  bool is_16b = (inst & 0b11) != 0b11;
+  bool is_32b = (inst & 0x1f) != 0x1f;
+  bool is_48b = (inst & 0x3f) != 0x1f;
+  bool is_64b = (inst & 0x7f) != 0x3f;
+  if (is_16b)
+    m_last_size = 2;
+  else if (is_32b)
+    m_last_size = 4;
+  else if (is_48b)
+    m_last_size = 6;
+  else if (is_64b)
+    m_last_size = 8;
+  else
+    // Not Valid
+    m_last_size = std::nullopt;
+
   // if we have ArchSpec::eCore_riscv128 in the future,
   // we also need to check it here
   if (m_arch.GetCore() == ArchSpec::eCore_riscv32)
@@ -638,8 +655,8 @@ std::optional<DecodeResult> EmulateInstructionRISCV::Decode(uint32_t inst) {
       LLDB_LOGF(
           log, "EmulateInstructionRISCV::%s: inst(%x at %" PRIx64 ") was decoded to %s",
           __FUNCTION__, inst, m_addr, pat.name);
-      auto decoded = is_rvc ? pat.decode(try_rvc) : pat.decode(inst);
-      return DecodeResult{decoded, inst, is_rvc, pat};
+      auto decoded = is_16b ? pat.decode(try_rvc) : pat.decode(inst);
+      return DecodeResult{decoded, inst, is_16b, pat};
     }
   }
   LLDB_LOGF(log, "EmulateInstructionRISCV::%s: inst(0x%x) was unsupported",

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

@@ -60,6 +60,7 @@ class EmulateInstructionRISCV : public EmulateInstruction {
 
   bool SetTargetTriple(const ArchSpec &arch) override;
   bool ReadInstruction() override;
+  virtual std::optional<uint32_t> GetLastInstrSize() { return std::nullopt; }
   bool EvaluateInstruction(uint32_t options) override;
   bool TestEmulation(Stream &out_stream, ArchSpec &arch,
                      OptionValueDictionary *test_data) override;
@@ -99,6 +100,8 @@ class EmulateInstructionRISCV : public EmulateInstruction {
 private:
   /// Last decoded instruction from m_opcode
   DecodeResult m_decoded;
+  /// Last decoded instruction size estimate.
+  std::optional<uint32_t> m_last_size;
 };
 
 } // namespace lldb_private

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

@@ -94,6 +94,39 @@ static lldb::addr_t ReadFlags(NativeRegisterContext &regsiter_context) {
                                                  LLDB_INVALID_ADDRESS);
 }
 
+static int GetSoftwareWatchpointSize(const ArchSpec &arch,
+                                     lldb::addr_t next_flags) {
+  if (arch.GetMachine() == llvm::Triple::arm) {
+    if (next_flags & 0x20)
+      // Thumb mode
+      return 2;
+    else
+      // Arm mode
+      return 4;
+  }
+  if (arch.IsMIPS() || arch.GetTriple().isPPC64() ||
+      arch.GetTriple().isRISCV() || arch.GetTriple().isLoongArch())
+    return 4;
+  return 0;
+}
+
+static Status SetSoftwareBreakPointOnPC(const ArchSpec &arch, lldb::addr_t pc,
+                                        lldb::addr_t next_flags,
+                                        NativeProcessProtocol &process) {
+  int size_hint = GetSoftwareWatchpointSize(arch, next_flags);
+  Status error;
+  error = process.SetBreakpoint(pc, size_hint, /*hardware=*/false);
+
+  // If setting the breakpoint fails because 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;
+
+  return Status();
+}
+
 Status NativeProcessSoftwareSingleStep::SetupSoftwareSingleStepping(
     NativeThreadProtocol &thread) {
   Status error;
@@ -115,8 +148,23 @@ Status NativeProcessSoftwareSingleStep::SetupSoftwareSingleStepping(
   emulator_up->SetWriteMemCallback(&WriteMemoryCallback);
   emulator_up->SetWriteRegCallback(&WriteRegisterCallback);
 
-  if (!emulator_up->ReadInstruction())
-    return Status("Read instruction failed!");
+  if (!emulator_up->ReadInstruction()) {
+    // try to get at least the size of next instruction to set breakpoint.
+    auto instrSizeOpt = emulator_up->GetLastInstrSize();
+    if (!instrSizeOpt)
+      return Status("Read instruction failed!");
+    bool success = false;
+    auto pc = emulator_up->ReadRegisterUnsigned(eRegisterKindGeneric,
+                                                LLDB_REGNUM_GENERIC_PC,
+                                                LLDB_INVALID_ADDRESS, &success);
+    if (!success)
+      return Status("Reading pc failed!");
+    lldb::addr_t next_pc = pc + *instrSizeOpt;
+    auto Result =
+        SetSoftwareBreakPointOnPC(arch, next_pc, /* next_flags */ 0x0, process);
+    m_threads_stepping_with_breakpoint.insert({thread.GetID(), next_pc});
+    return Result;
+  }
 
   bool emulation_result =
       emulator_up->EvaluateInstruction(eEmulateInstructionOptionAutoAdvancePC);
@@ -157,29 +205,7 @@ Status NativeProcessSoftwareSingleStep::SetupSoftwareSingleStepping(
     // modifying the PC but we don't  know how.
     return Status("Instruction emulation failed unexpectedly.");
   }
-
-  int size_hint = 0;
-  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() ||
-             arch.GetTriple().isRISCV() || arch.GetTriple().isLoongArch())
-    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;
-
+  auto Result = SetSoftwareBreakPointOnPC(arch, next_pc, next_flags, process);
   m_threads_stepping_with_breakpoint.insert({thread.GetID(), next_pc});
-
-  return Status();
+  return Result;
 }