[Lldb-commits] [lldb] a1ffabc - [lldb][riscv] Fix setting breakpoint for undecoded instruction (#90075)
via lldb-commits
lldb-commits at lists.llvm.org
Tue Jul 16 02:03:45 PDT 2024
Author: ita-sc
Date: 2024-07-16T10:03:42+01:00
New Revision: a1ffabc403d4ce55ab2e665511b0b68a16d4850b
URL: https://github.com/llvm/llvm-project/commit/a1ffabc403d4ce55ab2e665511b0b68a16d4850b
DIFF: https://github.com/llvm/llvm-project/commit/a1ffabc403d4ce55ab2e665511b0b68a16d4850b.diff
LOG: [lldb][riscv] Fix setting breakpoint for undecoded instruction (#90075)
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 supported
by `EmulateInstructionRISCV`.
Added:
lldb/test/API/riscv/break-undecoded/Makefile
lldb/test/API/riscv/break-undecoded/TestBreakpointIllegal.py
lldb/test/API/riscv/break-undecoded/compressed.c
lldb/test/API/riscv/break-undecoded/main.c
Modified:
lldb/include/lldb/Core/EmulateInstruction.h
lldb/source/Plugins/Instruction/RISCV/EmulateInstructionRISCV.cpp
lldb/source/Plugins/Instruction/RISCV/EmulateInstructionRISCV.h
lldb/source/Plugins/Process/Utility/NativeProcessSoftwareSingleStep.cpp
Removed:
################################################################################
diff --git a/lldb/include/lldb/Core/EmulateInstruction.h b/lldb/include/lldb/Core/EmulateInstruction.h
index 93c16537adba1..b459476883fc5 100644
--- a/lldb/include/lldb/Core/EmulateInstruction.h
+++ b/lldb/include/lldb/Core/EmulateInstruction.h
@@ -369,6 +369,8 @@ class EmulateInstruction : public PluginInterface {
virtual bool ReadInstruction() = 0;
+ virtual std::optional<uint32_t> GetLastInstrSize() { return std::nullopt; }
+
virtual bool EvaluateInstruction(uint32_t evaluate_options) = 0;
virtual InstructionCondition GetInstructionCondition() {
diff --git a/lldb/source/Plugins/Instruction/RISCV/EmulateInstructionRISCV.cpp b/lldb/source/Plugins/Instruction/RISCV/EmulateInstructionRISCV.cpp
index 6c46618b337c2..3f61e011d620a 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 8bca73a7f589d..53ac11c2e1102 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;
+ std::optional<uint32_t> GetLastInstrSize() override { return m_last_size; }
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
diff --git a/lldb/source/Plugins/Process/Utility/NativeProcessSoftwareSingleStep.cpp b/lldb/source/Plugins/Process/Utility/NativeProcessSoftwareSingleStep.cpp
index 6bf8a0dc28b22..ef71a964eaf20 100644
--- a/lldb/source/Plugins/Process/Utility/NativeProcessSoftwareSingleStep.cpp
+++ b/lldb/source/Plugins/Process/Utility/NativeProcessSoftwareSingleStep.cpp
@@ -94,6 +94,38 @@ static lldb::addr_t ReadFlags(NativeRegisterContext ®siter_context) {
LLDB_INVALID_ADDRESS);
}
+static int GetSoftwareBreakpointSize(const ArchSpec &arch,
+ lldb::addr_t next_flags) {
+ if (arch.GetMachine() == llvm::Triple::arm) {
+ if (next_flags & 0x20)
+ // Thumb mode
+ return 2;
+ // 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 = GetSoftwareBreakpointSize(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();
+ if (error.Fail())
+ return error;
+
+ return Status();
+}
+
Status NativeProcessSoftwareSingleStep::SetupSoftwareSingleStepping(
NativeThreadProtocol &thread) {
Status error;
@@ -115,8 +147,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 instr_size = emulator_up->GetLastInstrSize();
+ if (!instr_size)
+ 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 + *instr_size;
+ 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 +204,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;
}
diff --git a/lldb/test/API/riscv/break-undecoded/Makefile b/lldb/test/API/riscv/break-undecoded/Makefile
new file mode 100644
index 0000000000000..10495940055b6
--- /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/TestBreakpointIllegal.py b/lldb/test/API/riscv/break-undecoded/TestBreakpointIllegal.py
new file mode 100644
index 0000000000000..41e8901bf84ab
--- /dev/null
+++ b/lldb/test/API/riscv/break-undecoded/TestBreakpointIllegal.py
@@ -0,0 +1,44 @@
+"""
+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 TestBreakpointIllegal(TestBase):
+ @skipIf(archs=no_match(["rv64gc"]))
+ def test_4(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"],
+ )
+
+ @skipIf(archs=no_match(["rv64gc"]))
+ def test_2(self):
+ self.build(dictionary={"C_SOURCES": "compressed.c", "EXE": "compressed.x"})
+ (target, process, cur_thread, bkpt) = lldbutil.run_to_source_breakpoint(
+ self, "main", lldb.SBFileSpec("compressed.c"), exe_name="compressed.x"
+ )
+ 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/compressed.c b/lldb/test/API/riscv/break-undecoded/compressed.c
new file mode 100644
index 0000000000000..a82ce9893cdb6
--- /dev/null
+++ b/lldb/test/API/riscv/break-undecoded/compressed.c
@@ -0,0 +1,7 @@
+int main() {
+ // This instruction is not valid, but we have an ability to set
+ // software breakpoint.
+ // This results in illegal instruction during execution, not fail to set
+ // breakpoint
+ asm volatile(".2byte 0xaf");
+}
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 0000000000000..747923071e632
--- /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 in illegal instruction during execution, not fail to set
+ // breakpoint
+ asm volatile(".4byte 0xc58573" : :);
+}
More information about the lldb-commits
mailing list