From 2d70e7f16696640e563c25c854bc3aad7dd62d87 Mon Sep 17 00:00:00 2001 From: Andrew Waterman Date: Sun, 4 Dec 2022 21:46:59 -1000 Subject: [PATCH 1/3] For trap_t::name, return an std::string instead of a C string This is not a performance-critical case, since this method is invoked only when printing the log and when an exception occurs in the target. This works towards eliminating all uses of sprintf. I could have simply switched to snprintf, but this path seems cleaner. --- riscv/trap.h | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/riscv/trap.h b/riscv/trap.h index b5afce9a..0ef645af 100644 --- a/riscv/trap.h +++ b/riscv/trap.h @@ -5,6 +5,7 @@ #include "decode.h" #include +#include struct state_t; @@ -26,7 +27,7 @@ class trap_t virtual reg_t get_tinst() { return 0; } reg_t cause() { return which; } - virtual const char* name() + virtual std::string name() { const char* fmt = uint8_t(which) == which ? "trap #%u" : "interrupt #%u"; sprintf(_name, fmt, uint8_t(which)); @@ -73,31 +74,31 @@ class mem_trap_t : public trap_t #define DECLARE_TRAP(n, x) class trap_##x : public trap_t { \ public: \ trap_##x() : trap_t(n) {} \ - const char* name() { return "trap_"#x; } \ + std::string name() { return "trap_"#x; } \ }; #define DECLARE_INST_TRAP(n, x) class trap_##x : public insn_trap_t { \ public: \ trap_##x(reg_t tval) : insn_trap_t(n, /*gva*/false, tval) {} \ - const char* name() { return "trap_"#x; } \ + std::string name() { return "trap_"#x; } \ }; #define DECLARE_INST_WITH_GVA_TRAP(n, x) class trap_##x : public insn_trap_t { \ public: \ trap_##x(bool gva, reg_t tval) : insn_trap_t(n, gva, tval) {} \ - const char* name() { return "trap_"#x; } \ + std::string name() { return "trap_"#x; } \ }; #define DECLARE_MEM_TRAP(n, x) class trap_##x : public mem_trap_t { \ public: \ trap_##x(bool gva, reg_t tval, reg_t tval2, reg_t tinst) : mem_trap_t(n, gva, tval, tval2, tinst) {} \ - const char* name() { return "trap_"#x; } \ + std::string name() { return "trap_"#x; } \ }; #define DECLARE_MEM_GVA_TRAP(n, x) class trap_##x : public mem_trap_t { \ public: \ trap_##x(reg_t tval, reg_t tval2, reg_t tinst) : mem_trap_t(n, true, tval, tval2, tinst) {} \ - const char* name() { return "trap_"#x; } \ + std::string name() { return "trap_"#x; } \ }; DECLARE_MEM_TRAP(CAUSE_MISALIGNED_FETCH, instruction_address_misaligned) From 69bfc0261847861517fab5174ce38df48f8c4b4a Mon Sep 17 00:00:00 2001 From: Andrew Waterman Date: Sun, 4 Dec 2022 22:06:36 -1000 Subject: [PATCH 2/3] Avoid use of sprintf in trap_t --- riscv/trap.h | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/riscv/trap.h b/riscv/trap.h index 0ef645af..2b223710 100644 --- a/riscv/trap.h +++ b/riscv/trap.h @@ -4,7 +4,6 @@ #define _RISCV_TRAP_H #include "decode.h" -#include #include struct state_t; @@ -29,15 +28,14 @@ class trap_t virtual std::string name() { - const char* fmt = uint8_t(which) == which ? "trap #%u" : "interrupt #%u"; - sprintf(_name, fmt, uint8_t(which)); - return _name; + const uint8_t code = uint8_t(which); + const bool is_interrupt = code != which; + return (is_interrupt ? "interrupt #" : "trap #") + std::to_string(code); } virtual ~trap_t() = default; private: - char _name[16]; reg_t which; }; From 9dc874288cb6dcfa1b09411f70cc44f5c532e067 Mon Sep 17 00:00:00 2001 From: Andrew Waterman Date: Sun, 4 Dec 2022 21:38:17 -1000 Subject: [PATCH 3/3] Avoid use of sprintf in disassembler None of these cases are perf-critical. It was easy to change one of them to use std::string, but the others would have required more refactoring. So, simply change them to use snprintf instead. --- disasm/disasm.cc | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/disasm/disasm.cc b/disasm/disasm.cc index 856a651c..f522df9d 100644 --- a/disasm/disasm.cc +++ b/disasm/disasm.cc @@ -1289,14 +1289,12 @@ void disassembler_t::add_instructions(const isa_parser_t* isa) 0x10000000, 0x10005000, 0x10006000, 0x10007000}; for (unsigned nf = 0; nf <= 7; ++nf) { - char seg_str[8] = ""; - if (nf) - sprintf(seg_str, "seg%u", nf + 1); + const auto seg_str = nf ? "seg" + std::to_string(nf + 1) : ""; for (auto item : template_insn) { const reg_t match_nf = nf << 29; char buf[128]; - sprintf(buf, item.fmt, seg_str, 8 << elt); + snprintf(buf, sizeof(buf), item.fmt, seg_str.c_str(), 8 << elt); add_insn(new disasm_insn_t( buf, ((item.match | match_nf) & ~mask_vldst) | elt_map[elt], @@ -1314,7 +1312,7 @@ void disassembler_t::add_instructions(const isa_parser_t* isa) for (auto item : template_insn2) { const reg_t match_nf = nf << 29; char buf[128]; - sprintf(buf, item.fmt, nf + 1, 8 << elt); + snprintf(buf, sizeof(buf), item.fmt, nf + 1, 8 << elt); add_insn(new disasm_insn_t( buf, item.match | match_nf | elt_map[elt], @@ -1675,7 +1673,7 @@ void disassembler_t::add_instructions(const isa_parser_t* isa) for (size_t idx = 0; idx < sizeof(amo_map) / sizeof(amo_map[0]); ++idx) { for (auto item : template_insn) { char buf[128]; - sprintf(buf, item.fmt, amo_map[idx].first, 8 << elt); + snprintf(buf, sizeof(buf), item.fmt, amo_map[idx].first, 8 << elt); add_insn(new disasm_insn_t(buf, item.match | amo_map[idx].second | elt_map[elt], item.mask,