[Lldb-commits] [lldb] [lldb][riscv] Fix setting breakpoint for undecoded instruction (PR #90075)
via lldb-commits
lldb-commits at lists.llvm.org
Thu Apr 25 09:11:40 PDT 2024
https://github.com/ita-sc created https://github.com/llvm/llvm-project/pull/90075
Hi
This patch adds an interface GetLastInstrSize to get information about the size of last tried to be decoded instruction and uses it to set software breakpoint if the memory can be decoded as instruction.
RISC-V architecture instruction format specifies the length of instruction in first bits, so we can set a breakpoint for these cases. This is needed as RISCV have a lot of extensions, that are not suppored by `EmulateInstructionRISCV`.
>From e144b99f2200ba362f149b5552eaeef9a45fb747 Mon Sep 17 00:00:00 2001
From: Ivan Tetyushkin <ivan.tetyushkin at syntacore.com>
Date: Fri, 19 Apr 2024 18:47:05 +0300
Subject: [PATCH 1/2] [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.
---
.../RISCV/EmulateInstructionRISCV.cpp | 23 +++++-
.../RISCV/EmulateInstructionRISCV.h | 3 +
.../NativeProcessSoftwareSingleStep.cpp | 78 ++++++++++++-------
3 files changed, 75 insertions(+), 29 deletions(-)
diff --git a/lldb/source/Plugins/Instruction/RISCV/EmulateInstructionRISCV.cpp b/lldb/source/Plugins/Instruction/RISCV/EmulateInstructionRISCV.cpp
index 6c46618b337c23..3f61e011d620a2 100644
--- a/lldb/source/Plugins/Instruction/RISCV/EmulateInstructionRISCV.cpp
+++ b/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",
diff --git a/lldb/source/Plugins/Instruction/RISCV/EmulateInstructionRISCV.h b/lldb/source/Plugins/Instruction/RISCV/EmulateInstructionRISCV.h
index 8bca73a7f589df..7eac59d9a127b5 100644
--- a/lldb/source/Plugins/Instruction/RISCV/EmulateInstructionRISCV.h
+++ b/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 tried to be decoded instruction expected size.
+ std::optional<uint32_t> m_last_size;
};
} // namespace lldb_private
diff --git a/lldb/source/Plugins/Process/Utility/NativeProcessSoftwareSingleStep.cpp b/lldb/source/Plugins/Process/Utility/NativeProcessSoftwareSingleStep.cpp
index 6bf8a0dc28b22e..a6019d09737472 100644
--- a/lldb/source/Plugins/Process/Utility/NativeProcessSoftwareSingleStep.cpp
+++ b/lldb/source/Plugins/Process/Utility/NativeProcessSoftwareSingleStep.cpp
@@ -94,6 +94,39 @@ static lldb::addr_t ReadFlags(NativeRegisterContext ®siter_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;
}
>From cb5511a3009f3fd9fe96e47b13151c137d32ccb4 Mon Sep 17 00:00:00 2001
From: Ivan Tetyushkin <ivan.tetyushkin at syntacore.com>
Date: Thu, 18 Apr 2024 17:11:13 +0300
Subject: [PATCH 2/2] [lldb][riscv][test] Add test for stepi with illegal instr
---
lldb/test/API/riscv/break-undecoded/Makefile | 3 +++
.../break-undecoded/TestBreakpointIlligal.py | 27 +++++++++++++++++++
lldb/test/API/riscv/break-undecoded/main.c | 7 +++++
3 files changed, 37 insertions(+)
create mode 100644 lldb/test/API/riscv/break-undecoded/Makefile
create mode 100644 lldb/test/API/riscv/break-undecoded/TestBreakpointIlligal.py
create mode 100644 lldb/test/API/riscv/break-undecoded/main.c
diff --git a/lldb/test/API/riscv/break-undecoded/Makefile b/lldb/test/API/riscv/break-undecoded/Makefile
new file mode 100644
index 00000000000000..10495940055b63
--- /dev/null
+++ b/lldb/test/API/riscv/break-undecoded/Makefile
@@ -0,0 +1,3 @@
+C_SOURCES := main.c
+
+include Makefile.rules
diff --git a/lldb/test/API/riscv/break-undecoded/TestBreakpointIlligal.py b/lldb/test/API/riscv/break-undecoded/TestBreakpointIlligal.py
new file mode 100644
index 00000000000000..a934b5024eacc8
--- /dev/null
+++ b/lldb/test/API/riscv/break-undecoded/TestBreakpointIlligal.py
@@ -0,0 +1,27 @@
+"""
+Test that we can set up software breakpoint even if we failed to decode and execute instruction
+"""
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class TestBreakpointIlligal(TestBase):
+ @skipIf(archs=no_match(["rv64gc"]))
+ def test(self):
+ self.build()
+ (target, process, cur_thread, bkpt) = lldbutil.run_to_source_breakpoint(
+ self, "main", lldb.SBFileSpec("main.c")
+ )
+ self.runCmd("thread step-inst")
+ # we need to step more, as some compilers do not set appropriate debug info.
+ while cur_thread.GetStopDescription(256) == "instruction step into":
+ self.runCmd("thread step-inst")
+ # The stop reason of the thread should be illegal opcode.
+ self.expect(
+ "thread list",
+ STOPPED_DUE_TO_SIGNAL,
+ substrs=["stopped", "stop reason = signal SIGILL: illegal opcode"],
+ )
diff --git a/lldb/test/API/riscv/break-undecoded/main.c b/lldb/test/API/riscv/break-undecoded/main.c
new file mode 100644
index 00000000000000..ba85f4b9fa86d6
--- /dev/null
+++ b/lldb/test/API/riscv/break-undecoded/main.c
@@ -0,0 +1,7 @@
+int main() {
+ // This instruction is not valid, but we have an ability to set
+ // software breakpoint.
+ // This results illegal instruction during execution, not fail to set
+ // breakpoint
+ asm volatile(".insn r 0x73, 0, 0, a0, a1, a2" : :);
+}
More information about the lldb-commits
mailing list