llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-lldb @llvm/pr-subscribers-backend-risc-v Author: None (daniilavdeev) <details> <summary>Changes</summary> This patch replaces the hardcoded RISCV feature flags in disassembler with SubtargetFeatures provided by ArchSpec, which are derived from the .riscv.attributes ELF section. This ensures the disassembler uses exactly the RISCV extensions present in the executable, improving the accuracy and reliability of the disassembly output. --- Full diff: https://github.com/llvm/llvm-project/pull/173047.diff 4 Files Affected: - (modified) lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp (+22-17) - (added) lldb/test/API/riscv/disassembler/Makefile (+3) - (added) lldb/test/API/riscv/disassembler/TestDisassembler.py (+71) - (added) lldb/test/API/riscv/disassembler/main.cpp (+12) ``````````diff diff --git a/lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp b/lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp index e8bb706f7aab6..04cd48f1f5dcb 100644 --- a/lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp +++ b/lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp @@ -1590,23 +1590,28 @@ DisassemblerLLVMC::DisassemblerLLVMC(const ArchSpec &arch, } if (triple.isRISCV() && !cpu_or_features_overriden) { - uint32_t arch_flags = arch.GetFlags(); - if (arch_flags & ArchSpec::eRISCV_rvc) - features_str += "+c,"; - if (arch_flags & ArchSpec::eRISCV_rve) - features_str += "+e,"; - if ((arch_flags & ArchSpec::eRISCV_float_abi_single) == - ArchSpec::eRISCV_float_abi_single) - features_str += "+f,"; - if ((arch_flags & ArchSpec::eRISCV_float_abi_double) == - ArchSpec::eRISCV_float_abi_double) - features_str += "+f,+d,"; - if ((arch_flags & ArchSpec::eRISCV_float_abi_quad) == - ArchSpec::eRISCV_float_abi_quad) - features_str += "+f,+d,+q,"; - // FIXME: how do we detect features such as `+a`, `+m`? - // Turn them on by default now, since everyone seems to use them - features_str += "+a,+m,"; + auto subtarget_features = arch.GetSubtargetFeatures().getString(); + if (!subtarget_features.empty()) { + features_str += subtarget_features; + } else { + uint32_t arch_flags = arch.GetFlags(); + if (arch_flags & ArchSpec::eRISCV_rvc) + features_str += "+c,"; + if (arch_flags & ArchSpec::eRISCV_rve) + features_str += "+e,"; + if ((arch_flags & ArchSpec::eRISCV_float_abi_single) == + ArchSpec::eRISCV_float_abi_single) + features_str += "+f,"; + if ((arch_flags & ArchSpec::eRISCV_float_abi_double) == + ArchSpec::eRISCV_float_abi_double) + features_str += "+f,+d,"; + if ((arch_flags & ArchSpec::eRISCV_float_abi_quad) == + ArchSpec::eRISCV_float_abi_quad) + features_str += "+f,+d,+q,"; + // FIXME: how do we detect features such as `+a`, `+m`? + // Turn them on by default now, since everyone seems to use them + features_str += "+a,+m,"; + } } // We use m_disasm_up.get() to tell whether we are valid or not, so if this diff --git a/lldb/test/API/riscv/disassembler/Makefile b/lldb/test/API/riscv/disassembler/Makefile new file mode 100644 index 0000000000000..99998b20bcb05 --- /dev/null +++ b/lldb/test/API/riscv/disassembler/Makefile @@ -0,0 +1,3 @@ +CXX_SOURCES := main.cpp + +include Makefile.rules diff --git a/lldb/test/API/riscv/disassembler/TestDisassembler.py b/lldb/test/API/riscv/disassembler/TestDisassembler.py new file mode 100644 index 0000000000000..796136cedc4e1 --- /dev/null +++ b/lldb/test/API/riscv/disassembler/TestDisassembler.py @@ -0,0 +1,71 @@ +""" +Tests that LLDB can correctly set up a disassembler using extensions from the .riscv.attributes section. +""" + +import lldb +from lldbsuite.test.decorators import * +from lldbsuite.test.lldbtest import * +from lldbsuite.test import lldbutil + + +class TestDisassembler(TestBase): + expected_zbb_instrs = ["andn", "orn", "xnor", "rol", "ror", "ret"] + + def _get_llvm_tool(self, tool): + clang = self.getCompiler() + bindir = os.path.dirname(clang) + candidate = os.path.join(bindir, tool) + if os.path.exists(candidate): + return candidate + return lldbutil.which(tool) + + def _strip_riscv_attributes(self): + """ + Strips the .riscv.attributes section. + """ + exe = self.getBuildArtifact("a.out") + stripped = self.getBuildArtifact("stripped.out") + + objcopy_path = self._get_llvm_tool("llvm-objcopy") + self.assertTrue(objcopy_path, "llvm-objcopy not found") + + out = subprocess.run( + [objcopy_path, "--remove-section=.riscv.attributes", exe, stripped], + check=True, + ) + + return os.path.basename(stripped) + + @skipIf(archs=no_match("^riscv.*")) + def test_without_riscv_attributes(self): + """ + Tests disassembly of a riscv binary without the .riscv.attributes. + Without the .riscv.attributes section lldb won't set up a disassembler to + handle the bitmanip extension, so it is not expected to see zbb instructions + in the output. + """ + self.build(dictionary={"CFLAGS_EXTRAS": "-march=rv64gc_zbb"}) + stripped_exe = self._strip_riscv_attributes() + + lldbutil.run_to_name_breakpoint(self, "main", exe_name=stripped_exe) + + self.expect("disassemble --name do_zbb_stuff") + output = self.res.GetOutput() + + for instr in self.expected_zbb_instrs: + self.assertFalse(instr in output, "Zbb instructions should not be disassembled") + + @skipIf(archs=no_match("^riscv.*")) + def test_with_riscv_attributes(self): + """ + Tests disassembly of a riscv binary with the .riscv.attributes. + """ + self.build(dictionary={"CFLAGS_EXTRAS": "-march=rv64gc_zbb"}) + + lldbutil.run_to_name_breakpoint(self, "main") + + self.expect("disassemble --name do_zbb_stuff") + output = self.res.GetOutput() + + for instr in self.expected_zbb_instrs: + self.assertTrue(instr in output, "Invalid disassembler output") diff --git a/lldb/test/API/riscv/disassembler/main.cpp b/lldb/test/API/riscv/disassembler/main.cpp new file mode 100644 index 0000000000000..1d1f22d602e71 --- /dev/null +++ b/lldb/test/API/riscv/disassembler/main.cpp @@ -0,0 +1,12 @@ +void do_zbb_stuff() { + asm volatile("andn a2, a0, a1\n" + "orn a2, a0, a1\n" + "xnor a2, a0, a1\n" + "rol a2, a0, a1\n" + "ror a2, a0, a1\n"); +} + +int main() { + do_zbb_stuff(); + return 0; +} `````````` </details> https://github.com/llvm/llvm-project/pull/173047 _______________________________________________ llvm-branch-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
