From 6adf2e743b0e6045b721dd3e4d3d0185f2e4f460 Mon Sep 17 00:00:00 2001
From: Themis Demetriades <td1223@ic.ac.uk>
Date: Mon, 2 Dec 2024 19:50:40 +0000
Subject: [PATCH] refactor: dynamic stack growth functions to follow code style

---
 src/userprog/exception.c |  4 ++--
 src/vm/stackgrowth.c     | 50 ++++++++++++++++++++++++----------------
 src/vm/stackgrowth.h     |  2 +-
 3 files changed, 33 insertions(+), 23 deletions(-)

diff --git a/src/userprog/exception.c b/src/userprog/exception.c
index 2e812ec..8988c0a 100644
--- a/src/userprog/exception.c
+++ b/src/userprog/exception.c
@@ -148,12 +148,12 @@ page_fault (struct intr_frame *f)
 
   if (user && not_present)
     {
-      if (try_alloc_new_page (fault_addr, f->esp))
+      if (handle_stack_fault (fault_addr, f->esp))
         return;
     }
   else
     {
-      if (try_alloc_new_page (fault_addr, thread_current ()->curr_esp))
+      if (handle_stack_fault (fault_addr, thread_current ()->curr_esp))
         return;
 
       f->eip = (void *)f->eax;
diff --git a/src/vm/stackgrowth.c b/src/vm/stackgrowth.c
index bc9717c..8dae21a 100644
--- a/src/vm/stackgrowth.c
+++ b/src/vm/stackgrowth.c
@@ -1,5 +1,6 @@
 #include <stdio.h>
 #include "stackgrowth.h"
+#include "frame.h"
 #include "threads/palloc.h"
 #include "threads/thread.h"
 #include "threads/vaddr.h"
@@ -7,44 +8,53 @@
 
 #define MAX_STACK_ACCESS_DIST 32
 
-static bool needs_new_page (const void *addr, const void *esp);
+static bool is_stack_fault (const void *addr, const void *esp);
 static bool grow_stack (const void *addr);
 
+/* Determine whether a particular page fault occured due to a stack
+   access below the stack pointer that should induce stack growth, and
+   if so grow the stack by a single page (capped at MAX_STACK_SIZE). */
 bool
-try_alloc_new_page (const void *ptr, const void *esp)
+handle_stack_fault (const void *ptr, const void *esp)
 {
-  return needs_new_page (ptr, esp) && grow_stack (ptr);
+  return is_stack_fault (ptr, esp) && grow_stack (ptr);
 }
 
-/* Validates a given address for being a stack query and not a generic erroneous
-   address
- */
+/* Determines whether a particular page fault appears to be caused by
+   a stack access that should induce dynamic stack growth. Stack size
+   is capped at MAX_STACK_SIZE. */
 static bool
-needs_new_page (const void *addr, const void *esp)
+is_stack_fault (const void *addr, const void *esp)
 {
-    return (is_user_vaddr (addr) &&
-            (uint32_t*)addr >= ((uint32_t*)esp - MAX_STACK_ACCESS_DIST) &&
-            ((PHYS_BASE - pg_round_down (addr))
-                          <= MAX_STACK_SIZE));
+  return (is_user_vaddr (addr) &&
+          (uint32_t*)addr >= ((uint32_t*)esp - MAX_STACK_ACCESS_DIST) &&
+          ((PHYS_BASE - pg_round_down (addr)) <= MAX_STACK_SIZE));
 }
 
-/* Extends the stack by the necessary number of pages */
+/* Grows the stack of the process running inside the current thread by a single
+   page given a user virtual address inside of the page wherein the new section
+   of the stack should be allocated. */
 static bool
 grow_stack (const void *addr)
 {
   struct thread *t = thread_current ();
   void *last_page = pg_round_down (addr);
 
-  uint8_t *new_page = palloc_get_page (PAL_USER | PAL_ZERO);
-  if ( new_page == NULL)
+  /* This function should only be called when dealing with a faulting stack
+     access that induces stack growth, so the provided address shouldn't be
+     present in a page within the current thread's page directory. */
+  ASSERT (pagedir_get_page (t->pagedir, last_page) == NULL);
+
+  uint8_t *new_page = frame_alloc (PAL_ZERO, last_page, t);
+  if (new_page == NULL)
     return false;
 
-  bool added_page = pagedir_get_page (t->pagedir, last_page) == NULL
-                    && pagedir_set_page (t->pagedir, last_page, new_page, true);
-
-  if (!added_page) {
-    palloc_free_page (new_page);
+  if (!pagedir_set_page (t->pagedir, last_page, new_page, true))
+  {
+    frame_free (new_page);
     return false;
   }
+
   return true;
-}
\ No newline at end of file
+}
+
diff --git a/src/vm/stackgrowth.h b/src/vm/stackgrowth.h
index acd123e..a19366e 100644
--- a/src/vm/stackgrowth.h
+++ b/src/vm/stackgrowth.h
@@ -5,6 +5,6 @@
 
 #define MAX_STACK_SIZE 8388608 // (8MB)
 
-bool try_alloc_new_page (const void *ptr, const void *esp);
+bool handle_stack_fault (const void *ptr, const void *esp);
 
 #endif /* vm/frame.h */