Fix crashes because of are frame allocator copy

8 files changed, 54 insertions, 41 deletions

diff --git a/arch/x86_64/include/arch/context_switching/syscall/main.hpp b/arch/x86_64/include/arch/context_switching/syscall/main.hpp
index 8587ab2..9d61f97 100644
--- a/arch/x86_64/include/arch/context_switching/syscall/main.hpp
+++ b/arch/x86_64/include/arch/context_switching/syscall/main.hpp
@@ -60,8 +60,8 @@ namespace teachos::arch::context_switching::syscall
    */
   struct response
   {
-    error error_code;  ///< Error code returned by the syscall. If it failed all the values will be 0.
-    arguments values;  ///< Optional return values of the syscall implementation.
+    error error_code;       ///< Error code returned by the syscall. If it failed all the values will be 0.
+    arguments values = {};  ///< Optional return values of the syscall implementation.
   };
 
   /**
@@ -78,7 +78,7 @@ namespace teachos::arch::context_switching::syscall
    * in the arguments struct. So the value can be read and used for further processing.
    */
   [[gnu::section(".user_text")]]
-  auto syscall(type syscall_number, arguments args) -> response;
+  auto syscall(type syscall_number, arguments args = {}) -> response;
 
 }  // namespace teachos::arch::context_switching::syscall
 
diff --git a/arch/x86_64/include/arch/memory/heap/memory_block.hpp b/arch/x86_64/include/arch/memory/heap/memory_block.hpp
index e1cd288..9d1fb02 100644
--- a/arch/x86_64/include/arch/memory/heap/memory_block.hpp
+++ b/arch/x86_64/include/arch/memory/heap/memory_block.hpp
@@ -18,6 +18,7 @@ namespace teachos::arch::memory::heap
      * @param size Amount of free memory of this specific hole.
      * @param next Optional pointer to the next free memory.
      */
+    [[gnu::section(".user_text")]]
     memory_block(std::size_t size, memory_block * next);
 
     /**
@@ -26,6 +27,7 @@ namespace teachos::arch::memory::heap
      * @note Used so the memory can be reused to construct other classes into, without having the old values.
      * Required because we cannot call delete, because it causes "undefined reference to `sbrk`".
      */
+    [[gnu::section(".user_text")]]
     ~memory_block();
 
     std::size_t size;     ///< Amount of free memory this hole contains, has to always be atleast 16 bytes to hold the
diff --git a/arch/x86_64/include/arch/memory/paging/kernel_mapper.hpp b/arch/x86_64/include/arch/memory/paging/kernel_mapper.hpp
index 756eeb1..ca7e2f9 100644
--- a/arch/x86_64/include/arch/memory/paging/kernel_mapper.hpp
+++ b/arch/x86_64/include/arch/memory/paging/kernel_mapper.hpp
@@ -45,7 +45,7 @@ namespace teachos::arch::memory::paging
       kernel::cpu::write_control_register(kernel::cpu::control_register::CR4, cr4 | 0x80);
 
       temporary_page temporary_page{virtual_page{0xCAFEBABE}, allocator};
-      auto & active_table = active_page_table::create_or_get();
+      decltype(auto) active_table = active_page_table::create_or_get();
       auto const frame = allocator.allocate_frame();
       exception_handling::assert(frame.has_value(),
                                  "[Kernel Mapper] Frame could not be allocated and therefore kernel not mapped");
diff --git a/arch/x86_64/src/context_switching/syscall/main.cpp b/arch/x86_64/src/context_switching/syscall/main.cpp
index 996d7fb..e291c10 100644
--- a/arch/x86_64/src/context_switching/syscall/main.cpp
+++ b/arch/x86_64/src/context_switching/syscall/main.cpp
@@ -18,9 +18,6 @@ namespace teachos::arch::context_switching::syscall
 
     asm volatile("syscall");
 
-    error error_code{};
-    asm volatile("mov %%rax, %[output]" : [output] "=m"(error_code));
-
     arguments values{};
     asm volatile("mov %%rdi, %[output]" : [output] "=m"(values.arg_0));
     asm volatile("mov %%rsi, %[output]" : [output] "=m"(values.arg_1));
@@ -29,6 +26,9 @@ namespace teachos::arch::context_switching::syscall
     asm volatile("mov %%r8, %[output]" : [output] "=m"(values.arg_4));
     asm volatile("mov %%r9, %[output]" : [output] "=m"(values.arg_5));
 
+    error error_code{};
+    asm volatile("mov %%rax, %[output]" : [output] "=m"(error_code));
+
     return {error_code, values};
   }
 
diff --git a/arch/x86_64/src/context_switching/syscall/syscall_handler.cpp b/arch/x86_64/src/context_switching/syscall/syscall_handler.cpp
index 7272e9e..9cc6edf 100644
--- a/arch/x86_64/src/context_switching/syscall/syscall_handler.cpp
+++ b/arch/x86_64/src/context_switching/syscall/syscall_handler.cpp
@@ -11,30 +11,21 @@ namespace teachos::arch::context_switching::syscall
 
   namespace
   {
-    auto write_to_vga_buffer(uint64_t buffer) -> error
+    auto write_to_vga_buffer(uint64_t buffer) -> response
     {
       video::vga::text::write(reinterpret_cast<const char *>(buffer),
                               video::vga::text::common_attributes::green_on_black);
       video::vga::text::newline();
-      return error::OK;
+      return {error::OK};
     }
 
-    auto expand_user_heap() -> error
+    auto expand_user_heap() -> response
     {
       static auto current_heap_end = memory::heap::USER_HEAP_START;
-      memory::remap_heap(current_heap_end, memory::heap::USER_HEAP_SIZE, memory::paging::entry::USER_ACCESSIBLE);
-
-      arguments args{current_heap_end, memory::heap::USER_HEAP_SIZE};
-      asm volatile("mov %[input], %%rdi"
-                   : /* no output from call */
-                   : [input] "m"(args.arg_0)
-                   : "memory");
-      asm volatile("mov %[input], %%rsi"
-                   : /* no output from call */
-                   : [input] "m"(args.arg_1)
-                   : "memory");
+      uint64_t const heap_start = current_heap_end;
+      memory::remap_heap(heap_start, memory::heap::USER_HEAP_SIZE, memory::paging::entry::USER_ACCESSIBLE);
       current_heap_end += memory::heap::USER_HEAP_SIZE;
-      return error::OK;
+      return {error::OK, {heap_start, memory::heap::USER_HEAP_SIZE}};
     }
   }  // namespace
 
@@ -60,7 +51,7 @@ namespace teachos::arch::context_switching::syscall
     // and now.
     asm volatile("mov %%rax, %[output]" : [output] "=m"(syscall_number));
 
-    error result = error::OK;
+    response result;
     switch (static_cast<type>(syscall_number))
     {
       case type::WRITE:
@@ -76,7 +67,32 @@ namespace teachos::arch::context_switching::syscall
 
     asm volatile("mov %[input], %%rax"
                  : /* no output from call */
-                 : [input] "m"(result)
+                 : [input] "m"(result.error_code)
+                 : "memory");
+
+    asm volatile("mov %[input], %%rdi"
+                 : /* no output from call */
+                 : [input] "m"(result.values.arg_0)
+                 : "memory");
+    asm volatile("mov %[input], %%rsi"
+                 : /* no output from call */
+                 : [input] "m"(result.values.arg_1)
+                 : "memory");
+    asm volatile("mov %[input], %%rdx"
+                 : /* no output from call */
+                 : [input] "m"(result.values.arg_2)
+                 : "memory");
+    asm volatile("mov %[input], %%r10"
+                 : /* no output from call */
+                 : [input] "m"(result.values.arg_3)
+                 : "memory");
+    asm volatile("mov %[input], %%r8"
+                 : /* no output from call */
+                 : [input] "m"(result.values.arg_4)
+                 : "memory");
+    asm volatile("mov %[input], %%r9"
+                 : /* no output from call */
+                 : [input] "m"(result.values.arg_5)
                  : "memory");
 
     asm volatile("mov %[input], %%rcx"
diff --git a/arch/x86_64/src/memory/heap/memory_block.cpp b/arch/x86_64/src/memory/heap/memory_block.cpp
index 6ee675a..bc97bd6 100644
--- a/arch/x86_64/src/memory/heap/memory_block.cpp
+++ b/arch/x86_64/src/memory/heap/memory_block.cpp
@@ -6,11 +6,10 @@ namespace teachos::arch::memory::heap
 {
   memory_block::memory_block(std::size_t size, memory_block * next)
   {
-    // TODO: Figure out why this memset fails
-    // memset(static_cast<void *>(this), 0, size);
+    memset(static_cast<void *>(this), 0U, size);
     this->size = size;
     this->next = next;
   }
 
-  memory_block::~memory_block() { /*memset(static_cast<void *>(this), 0, sizeof(memory_block));*/ }
+  memory_block::~memory_block() { memset(static_cast<void *>(this), 0U, sizeof(memory_block)); }
 }  // namespace teachos::arch::memory::heap
diff --git a/arch/x86_64/src/memory/heap/user_heap_allocator.cpp b/arch/x86_64/src/memory/heap/user_heap_allocator.cpp
index 9cb6c17..f3fe1c2 100644
--- a/arch/x86_64/src/memory/heap/user_heap_allocator.cpp
+++ b/arch/x86_64/src/memory/heap/user_heap_allocator.cpp
@@ -104,16 +104,11 @@ namespace teachos::arch::memory::heap
 
   auto user_heap_allocator::expand_heap_if_full() -> memory_block *
   {
-    context_switching::syscall::arguments args{};
-    auto const result = context_switching::syscall::syscall(context_switching::syscall::type::EXPAND_HEAP, args);
+    auto const result = context_switching::syscall::syscall(context_switching::syscall::type::EXPAND_HEAP);
 
-    if (!result.error_code)
-    {
-      uint64_t const heap_start = result.values.arg_0;
-      uint64_t const heap_size = result.values.arg_1;
-      return new (reinterpret_cast<void *>(heap_start)) memory_block(heap_size, nullptr);
-    }
-    return nullptr;
+    uint64_t const heap_start = result.values.arg_0;
+    uint64_t const heap_size = result.values.arg_1;
+    return !result.error_code ? new (reinterpret_cast<void *>(heap_start)) memory_block(heap_size, nullptr) : nullptr;
   }
 
   auto user_heap_allocator::remove_free_memory_block(memory_block * previous_block, memory_block * current_block)
diff --git a/arch/x86_64/src/memory/main.cpp b/arch/x86_64/src/memory/main.cpp
index 5e671ac..2746a71 100644
--- a/arch/x86_64/src/memory/main.cpp
+++ b/arch/x86_64/src/memory/main.cpp
@@ -18,22 +18,23 @@ namespace teachos::arch::memory
     static std::optional<allocator::area_frame_allocator> frame_allocator;
 
     auto create_frame_allocator(multiboot::memory_information const & memory_information)
-        -> allocator::area_frame_allocator
+        -> allocator::area_frame_allocator &
     {
       frame_allocator.emplace(memory_information);
       return frame_allocator.value();
     }
 
-    auto get_frame_allocator() -> allocator::area_frame_allocator
+    auto get_frame_allocator() -> allocator::area_frame_allocator &
     {
-      exception_handling::assert(frame_allocator.has_value(), "[Memory main] Frame allocator has not been created yet");
+      exception_handling::assert(frame_allocator.has_value(),
+                                 "[Initialization] Frame allocator has not been created yet");
       return frame_allocator.value();
     }
   }  // namespace
 
   auto remap_heap(std::size_t heap_start, std::size_t heap_size, paging::entry::bitset additional_flags = {}) -> void
   {
-    auto allocator = get_frame_allocator();
+    decltype(auto) allocator = get_frame_allocator();
     decltype(auto) active_table = paging::active_page_table::create_or_get();
     auto const start_page = paging::virtual_page::containing_address(heap_start);
     auto const end_page = ++(paging::virtual_page::containing_address(heap_start + heap_size - 1));
@@ -59,7 +60,7 @@ namespace teachos::arch::memory
     has_been_called = true;
 
     auto const memory_information = multiboot::read_multiboot2();
-    auto allocator = create_frame_allocator(memory_information);
+    decltype(auto) allocator = create_frame_allocator(memory_information);
 
     kernel::cpu::set_cr0_bit(kernel::cpu::cr0_flags::WRITE_PROTECT);
     kernel::cpu::set_efer_bit(kernel::cpu::efer_flags::NXE);