Emmmer added a subscriber: labath. Emmmer added a comment. I have to say sorry after reading your update but not giving you feedback. I pulled your patch and found it does not compile, and I have fixed them for you :)
In D62732#2306055 <https://reviews.llvm.org/D62732#2306055>, @labath wrote: > ABI plugins are one of the hardest things to test in lldb, particularly > without actual hardware. That's why we've let them be added in the past > without any accompanying tests. The situation is not ideal though, because > we've accumulated various ABI plugins which are hard to change without > breaking (or even to know if they work) because they only work with > third-party (possibly proprietary) stubs. I have tried to work on ABISys_V plugins, but I think it is difficult because I am not able to check whether ABIPlugin works well. It will be nice if we have some tests. ================ Comment at: lldb/source/Plugins/ABI/RISCV/ABISysV_riscv.cpp:37-39 + +LLDB_PLUGIN_DEFINE(ABISysV_riscv) + ---------------- modify target name. ================ Comment at: lldb/source/Plugins/ABI/RISCV/ABISysV_riscv.cpp:157-162 + if (arch.GetTriple().getArch() == llvm::Triple::riscv32 || + arch.GetTriple().getArch() == llvm::Triple::riscv64) { + return ABISP( + new ABISysV_riscv(std::move(process_sp), MakeMCRegisterInfo(arch), + arch.GetTriple().getArch() == llvm::Triple::riscv64)); + } ---------------- Using `.isRISCV()` instead of `arch.GetTriple().getArch() == llvm::Triple::riscv32 || arch.GetTriple().getArch() == llvm::Triple::riscv64` and remove braces from `if` statment ================ Comment at: lldb/source/Plugins/ABI/RISCV/ABISysV_riscv.cpp:175-184 +// PluginInterface protocol + +lldb_private::ConstString ABISysV_riscv::GetPluginNameStatic() { + static ConstString g_name("sysv-riscv"); + return g_name; +} + ---------------- We can remove this since we defined it in `ABISysV_riscv.h` ================ Comment at: lldb/source/Plugins/ABI/RISCV/ABISysV_riscv.h:94-98 + static lldb_private::ConstString GetPluginNameStatic(); + + lldb_private::ConstString GetPluginName() override; + + uint32_t GetPluginVersion() override { return 1; } ---------------- GetPluginVersion() is unused. ================ Comment at: lldb/source/Plugins/ABI/RISCV/CMakeLists.txt:1 +add_lldb_library(lldbPluginABISysV_riscv PLUGIN + ABISysV_riscv.cpp ---------------- modify target name Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132510/new/ https://reviews.llvm.org/D132510 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
