From 91a82eec7c5e437fbc9e203eaf5924a79fdea507 Mon Sep 17 00:00:00 2001 From: Amneesh Singh Date: Sat, 23 Sep 2023 21:09:44 +0530 Subject: [PATCH] log: encapsulate logger Signed-off-by: Amneesh Singh --- .clang-tidy | 1 + apps/target/main.cc | 3 ++ include/meson.build | 1 + include/util/loglevel.hh | 14 ++++++ include/util/meson.build | 3 ++ src/cpu/arm/exec.cc | 50 +++++++++--------- src/cpu/cpu-impl.cc | 9 ++-- src/memory.cc | 41 ++++++++------- src/meson.build | 2 +- src/util/log.cc | 8 +++ src/util/log.hh | 106 ++++++++++++++++++++++++--------------- src/util/meson.build | 3 ++ tests/main.cc | 8 +++ tests/meson.build | 6 ++- 14 files changed, 158 insertions(+), 97 deletions(-) create mode 100644 include/util/loglevel.hh create mode 100644 include/util/meson.build create mode 100644 src/util/log.cc create mode 100644 src/util/meson.build create mode 100644 tests/main.cc diff --git a/.clang-tidy b/.clang-tidy index 4271e9b..d31e5f6 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -6,4 +6,5 @@ Checks: ' , -cppcoreguidelines-macro-usage , -cppcoreguidelines-avoid-const-or-ref-data-members , -cppcoreguidelines-non-private-member-variables-in-classes + , -cppcoreguidelines-avoid-non-const-global-variables ' \ No newline at end of file diff --git a/apps/target/main.cc b/apps/target/main.cc index cf272e3..1dcf994 100644 --- a/apps/target/main.cc +++ b/apps/target/main.cc @@ -1,6 +1,7 @@ #include "bus.hh" #include "cpu/cpu.hh" #include "memory.hh" +#include "util/loglevel.hh" #include #include #include @@ -84,6 +85,8 @@ main(int argc, const char* argv[]) { std::flush(std::cout); std::flush(std::cout); + matar::set_log_level(matar::LogLevel::Debug); + try { matar::Memory memory(std::move(bios), std::move(rom)); matar::Bus bus(memory); diff --git a/include/meson.build b/include/meson.build index a12baca..5862056 100644 --- a/include/meson.build +++ b/include/meson.build @@ -7,5 +7,6 @@ headers = files( inc = include_directories('.') subdir('cpu') +subdir('util') install_headers(headers, subdir: meson.project_name(), preserve_path: true) \ No newline at end of file diff --git a/include/util/loglevel.hh b/include/util/loglevel.hh new file mode 100644 index 0000000..f5d10f5 --- /dev/null +++ b/include/util/loglevel.hh @@ -0,0 +1,14 @@ +#pragma once + +namespace matar { +enum class LogLevel { + Off = 1 << 0, + Error = 1 << 1, + Warn = 1 << 2, + Info = 1 << 3, + Debug = 1 << 4 +}; + +void +set_log_level(LogLevel level); +} diff --git a/include/util/meson.build b/include/util/meson.build new file mode 100644 index 0000000..d811d36 --- /dev/null +++ b/include/util/meson.build @@ -0,0 +1,3 @@ +headers += files( + 'loglevel.hh' +) \ No newline at end of file diff --git a/src/cpu/arm/exec.cc b/src/cpu/arm/exec.cc index 9af0251..d9af8fe 100644 --- a/src/cpu/arm/exec.cc +++ b/src/cpu/arm/exec.cc @@ -2,27 +2,24 @@ #include "util/bits.hh" #include "util/log.hh" -using namespace logger; - namespace matar { void CpuImpl::exec_arm(const arm::Instruction instruction) { Condition cond = instruction.condition; arm::InstructionData data = instruction.data; - debug(cpsr.condition(cond)); if (!cpsr.condition(cond)) { return; } auto pc_error = [](uint8_t r) { if (r == PC_INDEX) - log_error("Using PC (R15) as operand register"); + glogger.error("Using PC (R15) as operand register"); }; auto pc_warn = [](uint8_t r) { if (r == PC_INDEX) - log_warn("Using PC (R15) as operand register"); + glogger.warn("Using PC (R15) as operand register"); }; using namespace arm; @@ -62,8 +59,8 @@ CpuImpl::exec_arm(const arm::Instruction instruction) { }, [this, pc_error](Multiply& data) { if (data.rd == data.rm) - log_error("rd and rm are not distinct in {}", - typeid(data).name()); + glogger.error("rd and rm are not distinct in {}", + typeid(data).name()); pc_error(data.rd); pc_error(data.rd); @@ -81,8 +78,8 @@ CpuImpl::exec_arm(const arm::Instruction instruction) { [this, pc_error](MultiplyLong& data) { if (data.rdhi == data.rdlo || data.rdhi == data.rm || data.rdlo == data.rm) - log_error("rdhi, rdlo and rm are not distinct in {}", - typeid(data).name()); + glogger.error("rdhi, rdlo and rm are not distinct in {}", + typeid(data).name()); pc_error(data.rdhi); pc_error(data.rdlo); @@ -123,7 +120,7 @@ CpuImpl::exec_arm(const arm::Instruction instruction) { cpsr.set_v(0); } }, - [](Undefined) { log_warn("Undefined instruction"); }, + [](Undefined) { glogger.warn("Undefined instruction"); }, [this, pc_error](SingleDataSwap& data) { pc_error(data.rm); pc_error(data.rn); @@ -142,12 +139,12 @@ CpuImpl::exec_arm(const arm::Instruction instruction) { uint32_t address = gpr[data.rn]; if (!data.pre && data.write) - log_warn("Write-back enabled with post-indexing in {}", - typeid(data).name()); + glogger.warn("Write-back enabled with post-indexing in {}", + typeid(data).name()); if (data.rn == PC_INDEX && data.write) - log_warn("Write-back enabled with base register as PC {}", - typeid(data).name()); + glogger.warn("Write-back enabled with base register as PC {}", + typeid(data).name()); if (data.write) pc_warn(data.rn); @@ -216,11 +213,11 @@ CpuImpl::exec_arm(const arm::Instruction instruction) { uint32_t offset = 0; if (!data.pre && data.write) - log_error("Write-back enabled with post-indexing in {}", - typeid(data).name()); + glogger.error("Write-back enabled with post-indexing in {}", + typeid(data).name()); if (data.sign && !data.load) - log_error("Signed data found in {}", typeid(data).name()); + glogger.error("Signed data found in {}", typeid(data).name()); if (data.write) pc_warn(data.rn); @@ -294,8 +291,8 @@ CpuImpl::exec_arm(const arm::Instruction instruction) { pc_error(data.rn); if (cpsr.mode() == Mode::User && data.s) { - log_error("Bit S is set outside priviliged modes in {}", - typeid(data).name()); + glogger.error("Bit S is set outside priviliged modes in {}", + typeid(data).name()); } // we just change modes to load user registers @@ -304,8 +301,9 @@ CpuImpl::exec_arm(const arm::Instruction instruction) { chg_mode(Mode::User); if (data.write) { - log_error("Write-back enable for user bank registers in {}", - typeid(data).name()); + glogger.error( + "Write-back enable for user bank registers in {}", + typeid(data).name()); } } @@ -358,8 +356,8 @@ CpuImpl::exec_arm(const arm::Instruction instruction) { }, [this, pc_error](PsrTransfer& data) { if (data.spsr && cpsr.mode() == Mode::User) { - log_error("Accessing SPSR in User mode in {}", - typeid(data).name()); + glogger.error("Accessing SPSR in User mode in {}", + typeid(data).name()); } Psr& psr = data.spsr ? spsr : cpsr; @@ -513,8 +511,8 @@ CpuImpl::exec_arm(const arm::Instruction instruction) { if (data.set) { if (data.rd == PC_INDEX) { if (cpsr.mode() == Mode::User) - log_error("Running {} in User mode", - typeid(data).name()); + glogger.error("Running {} in User mode", + typeid(data).name()); spsr = cpsr; } else { set_conditions(); @@ -536,7 +534,7 @@ CpuImpl::exec_arm(const arm::Instruction instruction) { spsr = cpsr; }, [](auto& data) { - log_error("Unimplemented {} instruction", typeid(data).name()); + glogger.error("Unimplemented {} instruction", typeid(data).name()); } }, data); } diff --git a/src/cpu/cpu-impl.cc b/src/cpu/cpu-impl.cc index 948f740..fe3aa7d 100644 --- a/src/cpu/cpu-impl.cc +++ b/src/cpu/cpu-impl.cc @@ -4,8 +4,6 @@ #include #include -using namespace logger; - namespace matar { CpuImpl::CpuImpl(const Bus& bus) noexcept : bus(std::make_shared(bus)) @@ -19,7 +17,7 @@ CpuImpl::CpuImpl(const Bus& bus) noexcept cpsr.set_irq_disabled(true); cpsr.set_fiq_disabled(true); cpsr.set_state(State::Arm); - log_info("CPU successfully initialised"); + glogger.info("CPU successfully initialised"); // PC always points to two instructions ahead // PC - 2 is the instruction being executed @@ -121,14 +119,13 @@ CpuImpl::step() { uint32_t cur_pc = pc - 2 * arm::INSTRUCTION_SIZE; if (cpsr.state() == State::Arm) { - debug(cur_pc); uint32_t x = bus->read_word(cur_pc); arm::Instruction instruction(x); - log_info("{:#034b}", x); + glogger.info("{:#034b}", x); exec_arm(instruction); - log_info("0x{:08X} : {}", cur_pc, instruction.disassemble()); + glogger.info("0x{:08X} : {}", cur_pc, instruction.disassemble()); if (is_flushed) { // if flushed, do not increment the PC, instead set it to two diff --git a/src/memory.cc b/src/memory.cc index de1dcea..98ac91c 100644 --- a/src/memory.cc +++ b/src/memory.cc @@ -6,8 +6,6 @@ #include #include -using namespace logger; - namespace matar { Memory::Memory(std::array&& bios, std::vector&& rom) @@ -23,17 +21,17 @@ Memory::Memory(std::array&& bios, "fd2547724b505f487e6dcb29ec2ecff3af35a841a77ab2e85fd87350abd36570"; if (bios_hash != expected_hash) { - log_warn("BIOS hash failed to match, run at your own risk" - "\nExpected : {} " - "\nGot : {}", - expected_hash, - bios_hash); + glogger.warn("BIOS hash failed to match, run at your own risk" + "\nExpected : {} " + "\nGot : {}", + expected_hash, + bios_hash); } parse_header(); - log_info("Memory successfully initialised"); - log_info("Cartridge Title: {}", header.title); + glogger.info("Memory successfully initialised"); + glogger.info("Cartridge Title: {}", header.title); }; #define MATCHES(area) address >= area##_START&& address <= area##_END @@ -59,7 +57,7 @@ Memory::read(size_t address) const { } else if (MATCHES(ROM_2)) { return rom[address - ROM_2_START]; } else { - log_error("Invalid memory region accessed"); + glogger.error("Invalid memory region accessed"); return 0xFF; } } @@ -85,7 +83,7 @@ Memory::write(size_t address, uint8_t byte) { } else if (MATCHES(ROM_2)) { rom[address - ROM_2_START] = byte; } else { - log_error("Invalid memory region accessed"); + glogger.error("Invalid memory region accessed"); } } @@ -94,7 +92,7 @@ Memory::write(size_t address, uint8_t byte) { uint16_t Memory::read_halfword(size_t address) const { if (address & 0b01) - log_warn("Reading a non aligned halfword address"); + glogger.warn("Reading a non aligned halfword address"); return read(address) | read(address + 1) << 8; } @@ -102,7 +100,7 @@ Memory::read_halfword(size_t address) const { void Memory::write_halfword(size_t address, uint16_t halfword) { if (address & 0b01) - log_warn("Writing to a non aligned halfword address"); + glogger.warn("Writing to a non aligned halfword address"); write(address, halfword & 0xFF); write(address + 1, halfword >> 8 & 0xFF); @@ -111,7 +109,7 @@ Memory::write_halfword(size_t address, uint16_t halfword) { uint32_t Memory::read_word(size_t address) const { if (address & 0b11) - log_warn("Reading a non aligned word address"); + glogger.warn("Reading a non aligned word address"); return read(address) | read(address + 1) << 8 | read(address + 2) << 16 | read(address + 3) << 24; @@ -120,7 +118,7 @@ Memory::read_word(size_t address) const { void Memory::write_word(size_t address, uint32_t word) { if (address & 0b11) - log_warn("Writing to a non aligned word address"); + glogger.warn("Writing to a non aligned word address"); write(address, word & 0xFF); write(address + 1, word >> 8 & 0xFF); @@ -142,7 +140,7 @@ Memory::parse_header() { // nintendo logo if (rom[0x9C] != 0x21) - log_info("HEADER: BIOS debugger bits not set to 0"); + glogger.info("HEADER: BIOS debugger bits not set to 0"); // game info header.title = std::string(&rom[0xA0], &rom[0xA0 + 12]); @@ -177,7 +175,7 @@ Memory::parse_header() { break; default: - log_error("HEADER: invalid unique code: {}", rom[0xAC]); + glogger.error("HEADER: invalid unique code: {}", rom[0xAC]); } header.title_code = std::string(&rom[0xAD], &rom[0xAE]); @@ -206,15 +204,16 @@ Memory::parse_header() { break; default: - log_error("HEADER: invalid destination/language: {}", rom[0xAF]); + glogger.error("HEADER: invalid destination/language: {}", + rom[0xAF]); } if (rom[0xB2] != 0x96) - log_error("HEADER: invalid fixed byte at 0xB2"); + glogger.error("HEADER: invalid fixed byte at 0xB2"); for (size_t i = 0xB5; i < 0xBC; i++) { if (rom[i] != 0x00) - log_error("HEADER: invalid fixed bytes at 0xB5"); + glogger.error("HEADER: invalid fixed bytes at 0xB5"); } header.version = rom[0xBC]; @@ -228,7 +227,7 @@ Memory::parse_header() { chk &= 0xFF; if (chk != rom[0xBD]) - log_error("HEADER: checksum does not match"); + glogger.error("HEADER: checksum does not match"); } // multiboot not required right now diff --git a/src/meson.build b/src/meson.build index 81b23d2..b1eff10 100644 --- a/src/meson.build +++ b/src/meson.build @@ -3,9 +3,9 @@ lib_sources = files( 'bus.cc' ) +subdir('util') subdir('cpu') - lib_cpp_args = [ ] fmt = dependency('fmt', version : '>=10.1.0', static: true) diff --git a/src/util/log.cc b/src/util/log.cc new file mode 100644 index 0000000..b11a57c --- /dev/null +++ b/src/util/log.cc @@ -0,0 +1,8 @@ +#include "log.hh" + +logging::Logger glogger = logging::Logger(); + +void +matar::set_log_level(LogLevel level) { + glogger.set_level(level); +} diff --git a/src/util/log.hh b/src/util/log.hh index 8ecd504..1b003c5 100644 --- a/src/util/log.hh +++ b/src/util/log.hh @@ -1,12 +1,10 @@ #pragma once +#include "util/loglevel.hh" #include #include -using fmt::print; -using std::clog; - -namespace logger { +namespace logging { namespace ansi { static constexpr std::string_view RED = "\033[31m"; static constexpr std::string_view YELLOW = "\033[33m"; @@ -16,43 +14,69 @@ static constexpr std::string_view BOLD = "\033[1m"; static constexpr std::string_view RESET = "\033[0m"; } -template -inline void -log_raw(const fmt::format_string& fmt, Args&&... args) { - fmt::println(clog, fmt, std::forward(args)...); +using fmt::print; + +class Logger { + using LogLevel = matar::LogLevel; + + public: + Logger(LogLevel level = LogLevel::Debug) + : level(0) { + set_level(level); + } + + template + void log(const fmt::format_string& fmt, Args&&... args) { + fmt::println(stream, fmt, std::forward(args)...); + } + + template + void debug(const fmt::format_string& fmt, Args&&... args) { + if (level & static_cast(LogLevel::Debug)) { + print(stream, "{}{}[DEBUG] ", ansi::MAGENTA, ansi::BOLD); + log(fmt, std::forward(args)...); + print(stream, ansi::RESET); + } + } + + template + void info(const fmt::format_string& fmt, Args&&... args) { + if (level & static_cast(LogLevel::Info)) { + print(stream, "{}[INFO] ", ansi::WHITE); + log(fmt, std::forward(args)...); + print(stream, ansi::RESET); + } + } + + template + void warn(const fmt::format_string& fmt, Args&&... args) { + if (level & static_cast(LogLevel::Warn)) { + print(stream, "{}[WARN] ", ansi::YELLOW); + log(fmt, std::forward(args)...); + print(stream, ansi::RESET); + } + } + + template + void error(const fmt::format_string& fmt, Args&&... args) { + if (level & static_cast(LogLevel::Error)) { + print(stream, "{}{}[ERROR] ", ansi::RED, ansi::BOLD); + log(fmt, std::forward(args)...); + print(stream, ansi::RESET); + } + } + + void set_level(LogLevel level) { + this->level = (static_cast(level) << 1) - 1; + } + void set_stream(std::ostream& stream) { this->stream = stream; } + + private: + uint8_t level; + std::reference_wrapper stream = std::clog; +}; } -template -inline void -log_debug(const fmt::format_string& fmt, Args&&... args) { - print(clog, "{}{}[DEBUG] ", ansi::MAGENTA, ansi::BOLD); - log_raw(fmt, std::forward(args)...); - print(clog, ansi::RESET); -} +extern logging::Logger glogger; -template -inline void -log_info(const fmt::format_string& fmt, Args&&... args) { - print(clog, "{}[INFO] ", ansi::WHITE); - log_raw(fmt, std::forward(args)...); - print(clog, ansi::RESET); -} - -template -inline void -log_warn(const fmt::format_string& fmt, Args&&... args) { - print(clog, "{}[WARN] ", ansi::YELLOW); - log_raw(fmt, std::forward(args)...); - print(clog, ansi::RESET); -} - -template -inline void -log_error(const fmt::format_string& fmt, Args&&... args) { - print(clog, "{}{}[ERROR] ", ansi::RED, ansi::BOLD); - log_raw(fmt, std::forward(args)...); - print(clog, ansi::RESET); -} -} - -#define debug(value) logger::log_debug("{} = {}", #value, value) +#define debug(x) logger.debug("{} = {}", #x, x); diff --git a/src/util/meson.build b/src/util/meson.build new file mode 100644 index 0000000..d25f0c9 --- /dev/null +++ b/src/util/meson.build @@ -0,0 +1,3 @@ +lib_sources += files( + 'log.cc' +) \ No newline at end of file diff --git a/tests/main.cc b/tests/main.cc new file mode 100644 index 0000000..83e9f1b --- /dev/null +++ b/tests/main.cc @@ -0,0 +1,8 @@ +#include "util/loglevel.hh" +#include + +int +main(int argc, char* argv[]) { + matar::set_log_level(matar::LogLevel::Off); + return Catch::Session().run(argc, argv); +} diff --git a/tests/meson.build b/tests/meson.build index 35ded4d..d06ab7c 100644 --- a/tests/meson.build +++ b/tests/meson.build @@ -4,11 +4,13 @@ tests_deps = [ src = include_directories('../src') -tests_sources = files() +tests_sources = files( + 'main.cc' +) subdir('cpu') -catch2 = dependency('catch2-with-main', version: '>=3.4.0', static: true) +catch2 = dependency('catch2', version: '>=3.4.0', static: true) catch2_tests = executable( 'matar_tests', tests_sources,