From da2341ec12128d3b4983a67d39aeaf76b1781fa8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matteo=20Gm=C3=BCr?= Date: Sun, 20 Oct 2024 12:02:20 +0000 Subject: Add printf like behaviour to assert --- arch/x86_64/CMakeLists.txt | 1 - .../include/arch/exception_handling/assert.hpp | 52 +++++++++++++++++++--- arch/x86_64/src/exception_handling/abort.cpp | 3 +- arch/x86_64/src/exception_handling/assert.cpp | 16 ------- arch/x86_64/src/exception_handling/panic.cpp | 3 +- arch/x86_64/src/memory/multiboot/reader.cpp | 24 +++++++--- arch/x86_64/src/memory/paging/page_entry.cpp | 2 +- arch/x86_64/src/memory/paging/page_mapper.cpp | 24 +++++----- arch/x86_64/src/memory/paging/page_table.cpp | 2 +- 9 files changed, 79 insertions(+), 48 deletions(-) delete mode 100644 arch/x86_64/src/exception_handling/assert.cpp (limited to 'arch') diff --git a/arch/x86_64/CMakeLists.txt b/arch/x86_64/CMakeLists.txt index 018fe61..645660a 100644 --- a/arch/x86_64/CMakeLists.txt +++ b/arch/x86_64/CMakeLists.txt @@ -58,7 +58,6 @@ target_sources("_memory" PRIVATE target_sources("_exception" PRIVATE "src/exception_handling/abort.cpp" - "src/exception_handling/assert.cpp" "src/exception_handling/panic.cpp" ) diff --git a/arch/x86_64/include/arch/exception_handling/assert.hpp b/arch/x86_64/include/arch/exception_handling/assert.hpp index f355a61..152e653 100644 --- a/arch/x86_64/include/arch/exception_handling/assert.hpp +++ b/arch/x86_64/include/arch/exception_handling/assert.hpp @@ -1,16 +1,56 @@ #ifndef TEACHOS_ARCH_X86_64_EXCEPTION_HANDLING_ASSERT_HPP #define TEACHOS_ARCH_X86_64_EXCEPTION_HANDLING_ASSERT_HPP +#include "arch/exception_handling/panic.hpp" + +#include + namespace teachos::arch::exception_handling { + namespace + { + char constexpr FAILED_MESSAGE[] = "Invalid arguments passed to format specifiers (%) in exception_handling::assert"; + } + /** - * @brief assert a condition to be true, if not do not continue - * execution of the code and print message to screen + * @brief Assert a condition to be true, if not do not continue + * execution of the code and print the formatted message to screen. * - * @param condition - * @param message + * @param condition Condition we want to be true. + * @param format Formatting message that the given arguments will be inserted into. + * @param ...args Arguments that will be formatted and inserted into the resulting string, replacing their respective + * specifiers. Uses the printf specifiers see https://cplusplus.com/reference/cstdio/printf/ for more information. */ - auto assert(bool condition, char const * message) -> void; + template + auto assert(bool condition, char const * format, Args const &... args) -> void + { + if (condition) + { + return; + } + else if constexpr (sizeof...(args) == 0) + { + panic("Assertion Violation: ", format); + } + + // Result is what would have been written if the passed buffer would have been large enough not counting null + // character, or if an error occured while creating the string a negative number is returned instead. To ensure this + // will not crash the system when creating an array with negative size we assert beforehand with a clear error + // message. + int const size = snprintf(nullptr, 0U, format, args...) + 1U; + if (size < 0) + { + panic("Assertion Violation: ", FAILED_MESSAGE); + } +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wvla" + char arguments[size] = {}; +#pragma GCC diagnostic pop + int const written_characters = snprintf(arguments, size, format, args...); + // Written characters is expected to be one less, because of the null termination character. + bool const result = (written_characters == (size - 1)); + panic("Assertion Violation: ", result ? arguments : FAILED_MESSAGE); + } } // namespace teachos::arch::exception_handling -#endif \ No newline at end of file +#endif diff --git a/arch/x86_64/src/exception_handling/abort.cpp b/arch/x86_64/src/exception_handling/abort.cpp index dc40008..77576b1 100644 --- a/arch/x86_64/src/exception_handling/abort.cpp +++ b/arch/x86_64/src/exception_handling/abort.cpp @@ -4,7 +4,6 @@ namespace teachos::arch::exception_handling { - /** * @brief Override for the newlib abort function. * @@ -14,4 +13,4 @@ namespace teachos::arch::exception_handling */ extern "C" auto abort() -> void { panic("Terminate was called, possibly due to an unhandled exception"); } -} // namespace teachos::arch::exception_handling \ No newline at end of file +} // namespace teachos::arch::exception_handling diff --git a/arch/x86_64/src/exception_handling/assert.cpp b/arch/x86_64/src/exception_handling/assert.cpp deleted file mode 100644 index 86696f8..0000000 --- a/arch/x86_64/src/exception_handling/assert.cpp +++ /dev/null @@ -1,16 +0,0 @@ -#include "arch/exception_handling/assert.hpp" - -#include "arch/exception_handling/panic.hpp" - -namespace teachos::arch::exception_handling -{ - auto assert(bool condition, char const * message) -> void - { - if (condition) - { - return; - } - - panic("Assertion Violation: ", message); - } -} // namespace teachos::arch::exception_handling \ No newline at end of file diff --git a/arch/x86_64/src/exception_handling/panic.cpp b/arch/x86_64/src/exception_handling/panic.cpp index 56edfd5..8e3802a 100644 --- a/arch/x86_64/src/exception_handling/panic.cpp +++ b/arch/x86_64/src/exception_handling/panic.cpp @@ -5,7 +5,6 @@ namespace teachos::arch::exception_handling { - extern "C" char const message_prefix_panic[]; auto panic(char const * reason) -> void { panic(message_prefix_panic, reason); } @@ -20,4 +19,4 @@ namespace teachos::arch::exception_handling kernel::halt(); }; -} // namespace teachos::arch::exception_handling \ No newline at end of file +} // namespace teachos::arch::exception_handling diff --git a/arch/x86_64/src/memory/multiboot/reader.cpp b/arch/x86_64/src/memory/multiboot/reader.cpp index 565f604..4e78a06 100644 --- a/arch/x86_64/src/memory/multiboot/reader.cpp +++ b/arch/x86_64/src/memory/multiboot/reader.cpp @@ -19,7 +19,9 @@ namespace teachos::arch::memory::multiboot { auto expected_entry_size = mminfo->entry_size; constexpr auto actual_entry_size = sizeof(memory_area); - exception_handling::assert(expected_entry_size == actual_entry_size, "Unexpected memory_area entry size"); + exception_handling::assert(expected_entry_size == actual_entry_size, + "[Multiboot Reader] Expected memory area entry size (%u) but got (%u)", + expected_entry_size, actual_entry_size); auto total_size = mminfo->info.size; auto total_entries_size = total_size - sizeof(memory_map_header) + actual_entry_size; @@ -34,18 +36,22 @@ namespace teachos::arch::memory::multiboot { auto expected_entry_size = symbol->entry_size; constexpr auto actual_entry_size = sizeof(elf_section_header); - exception_handling::assert(expected_entry_size == actual_entry_size, "Unexpected elf_section_header entry size"); + exception_handling::assert(expected_entry_size == actual_entry_size, + "[Multiboot Reader] Expected elf section header entry size (%u) but got (%u)", + expected_entry_size, actual_entry_size); auto expected_total_size = symbol->info.size; auto actual_total_entry_size = actual_entry_size * symbol->number_of_sections; constexpr auto actual_total_section_size = sizeof(elf_symbols_section_header) - sizeof(uint32_t); auto actual_total_size = actual_total_entry_size + actual_total_section_size; exception_handling::assert(expected_total_size == actual_total_size, - "Unexpected elf_symbols_section_header total size"); + "[Multiboot Reader] Expected elf symbols section header total size (%u) but got (%u)", + expected_total_size, actual_total_size); auto begin = reinterpret_cast(&symbol->end); auto end = begin + symbol->number_of_sections; - exception_handling::assert(begin->is_null(), "Missing elf_section_header begin"); + exception_handling::assert(begin->is_null(), + "[Multiboot Reader] Elf symbols section not starting with SHT_NULL section"); std::size_t symbol_table_section_count = 0U; std::size_t dynamic_section_count = 0U; @@ -79,8 +85,14 @@ namespace teachos::arch::memory::multiboot } } - exception_handling::assert(symbol_table_section_count == 1U, "Unexpected symbol_table_count value"); - exception_handling::assert(dynamic_section_count <= 1U, "Unexpected dynamic_section_count value"); + exception_handling::assert( + symbol_table_section_count == 1U, + "[Multiboot Reader] ELF Specifications allows only (1) symbol table section, but got (%u)", + symbol_table_section_count); + exception_handling::assert( + dynamic_section_count <= 1U, + "[Multiboot Reader] ELF Specifications allows only (1) or less dynamic sections, but got (%u)", + dynamic_section_count); } } // namespace diff --git a/arch/x86_64/src/memory/paging/page_entry.cpp b/arch/x86_64/src/memory/paging/page_entry.cpp index 0dbbae1..30a8961 100644 --- a/arch/x86_64/src/memory/paging/page_entry.cpp +++ b/arch/x86_64/src/memory/paging/page_entry.cpp @@ -36,7 +36,7 @@ namespace teachos::arch::memory::paging auto entry::set_entry(allocator::physical_frame frame, std::bitset<64U> additional_flags) -> void { exception_handling::assert((frame.start_address() & ~0x000fffff'fffff000) == 0, - "Start address is not aligned with Page"); + "[Paging Entry] Start address is not aligned with page"); flags = std::bitset<64U>(frame.start_address()) | additional_flags; } } // namespace teachos::arch::memory::paging diff --git a/arch/x86_64/src/memory/paging/page_mapper.cpp b/arch/x86_64/src/memory/paging/page_mapper.cpp index eb1e18c..8c64f22 100644 --- a/arch/x86_64/src/memory/paging/page_mapper.cpp +++ b/arch/x86_64/src/memory/paging/page_mapper.cpp @@ -81,28 +81,26 @@ namespace teachos::arch::memory::paging allocator::physical_frame const & frame, entry::bitset flags) -> void { page_table page_table{}; - bool is_valid = false; + bool table_exists = false; for (auto level = page_table::LEVEL4; level != page_table::LEVEL1; level--) { std::size_t level_index = page.get_level_index(level); - is_valid = page_table.next_table(level_index); + table_exists = page_table.next_table(level_index); - if (!is_valid) + if (!table_exists) { - break; + auto allocated_frame = allocator.allocate_frame(); + exception_handling::assert(!allocated_frame.has_value(), "[Page mapper]: Unable to allocate frame"); + page_table[level_index].set_entry(allocated_frame.value(), entry::PRESENT | entry::WRITABLE); + page_table.zero_entries(); } - - auto allocated_frame = allocator.allocate_frame(); - exception_handling::assert(!allocated_frame.has_value(), "[Page mapper]: Unable to allocate frame"); - page_table[level_index].set_entry(allocated_frame.value(), entry::PRESENT | entry::WRITABLE); - page_table.zero_entries(); } - entry page_table_entry = page_table[page.get_level_index(page_table::LEVEL1)]; - arch::exception_handling::assert(!page_table_entry.contains_flags(entry::HUGE_PAGE), + auto level1_entry = page_table[page.get_level_index(page_table::LEVEL1)]; + arch::exception_handling::assert(!level1_entry.contains_flags(entry::HUGE_PAGE), "[Page Mapper]: Unable to map huge pages"); - arch::exception_handling::assert(!page_table_entry.is_unused(), "[Page Mapper]: Page table entry is already used"); - page_table_entry.set_entry(frame, flags | entry::PRESENT); + arch::exception_handling::assert(!level1_entry.is_unused(), "[Page Mapper]: Page table entry is already used"); + level1_entry.set_entry(frame, flags | entry::PRESENT); } } // namespace teachos::arch::memory::paging \ No newline at end of file diff --git a/arch/x86_64/src/memory/paging/page_table.cpp b/arch/x86_64/src/memory/paging/page_table.cpp index 02ababe..ea2e9c2 100644 --- a/arch/x86_64/src/memory/paging/page_table.cpp +++ b/arch/x86_64/src/memory/paging/page_table.cpp @@ -39,7 +39,7 @@ namespace teachos::arch::memory::paging { // C array is not bounds checked, therefore we have to check ourselves, to ensure no out of bounds reads, which // could be incredibly hard to debug later. - exception_handling::assert(index < PAGE_TABLE_ENTRY_COUNT, "[Page Table] index out of bounds"); + exception_handling::assert(index < PAGE_TABLE_ENTRY_COUNT, "[Page Table] Index out of bounds"); return current_table->entries[index]; } -- cgit v1.2.3