From eb458efa59d65cc2b06011c78cd3b9e126e2b495 Mon Sep 17 00:00:00 2001 From: Themis Demetriades Date: Sun, 3 Nov 2024 17:38:38 +0000 Subject: [PATCH 01/15] Update process_wait skeleton to loop infinitely for testing purposes --- src/userprog/process.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/userprog/process.c b/src/userprog/process.c index 49b9bd5..fdcf2c4 100644 --- a/src/userprog/process.c +++ b/src/userprog/process.c @@ -17,6 +17,7 @@ #include "threads/palloc.h" #include "threads/thread.h" #include "threads/vaddr.h" +#include "threads/synch.h" static thread_func start_process NO_RETURN; static bool load (const char *cmdline, void (**eip) (void), void **esp); @@ -88,7 +89,12 @@ start_process (void *file_name_) int process_wait (tid_t child_tid UNUSED) { - return -1; + /* TODO: Implement correct process waiting behaviour. + Currently an infinite loop for testing purposes. */ + while (1) + { + barrier (); + } } /* Free the current process's resources. */ From e26b11cce6593ad1de799615ad869192b31e5dba Mon Sep 17 00:00:00 2001 From: Themis Demetriades Date: Sun, 3 Nov 2024 18:04:42 +0000 Subject: [PATCH 02/15] Update setup_stack skeleton to fake minimal stack set-up for testing purposes --- src/userprog/process.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/userprog/process.c b/src/userprog/process.c index fdcf2c4..701078a 100644 --- a/src/userprog/process.c +++ b/src/userprog/process.c @@ -457,7 +457,7 @@ setup_stack (void **esp) { success = install_page (((uint8_t *) PHYS_BASE) - PGSIZE, kpage, true); if (success) - *esp = PHYS_BASE; + *esp = PHYS_BASE - 12; else palloc_free_page (kpage); } From 34d6c15d739ab8804bb4fc0775f3025729f6667d Mon Sep 17 00:00:00 2001 From: Themis Demetriades Date: Mon, 4 Nov 2024 00:00:56 +0000 Subject: [PATCH 03/15] Update start_process to push process argument values to its thread's stack --- src/userprog/process.c | 75 ++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 72 insertions(+), 3 deletions(-) diff --git a/src/userprog/process.c b/src/userprog/process.c index 701078a..8aff29b 100644 --- a/src/userprog/process.c +++ b/src/userprog/process.c @@ -15,10 +15,19 @@ #include "threads/init.h" #include "threads/interrupt.h" #include "threads/palloc.h" +#include "threads/malloc.h" #include "threads/thread.h" #include "threads/vaddr.h" #include "threads/synch.h" +/* Keeps track of the position of pointers to user program arguments + within a linked list. */ +struct arg_elem + { + char* arg; + struct list_elem elem; + }; + static thread_func start_process NO_RETURN; static bool load (const char *cmdline, void (**eip) (void), void **esp); @@ -37,6 +46,9 @@ process_execute (const char *file_name) fn_copy = palloc_get_page (0); if (fn_copy == NULL) return TID_ERROR; + + /* Imposing implicit limit that the command line arguments + including the user program name fit within a single page. */ strlcpy (fn_copy, file_name, PGSIZE); /* Create a new thread to execute FILE_NAME. */ @@ -51,19 +63,76 @@ process_execute (const char *file_name) static void start_process (void *file_name_) { - char *file_name = file_name_; struct intr_frame if_; bool success; + /* Retrieve first argument of command, which is the file name + of the process. */ + char *saveptr; + char *arg = strtok_r (file_name_, " ", &saveptr); + /* Initialize interrupt frame and load executable. */ memset (&if_, 0, sizeof if_); if_.gs = if_.fs = if_.es = if_.ds = if_.ss = SEL_UDSEG; if_.cs = SEL_UCSEG; if_.eflags = FLAG_IF | FLAG_MBS; - success = load (file_name, &if_.eip, &if_.esp); + success = load (arg, &if_.eip, &if_.esp); + + /* Load command line argument *data* to user process stack. + This can't cause overflow due to enforcing that the size of + command line input must fit in a page. Also keep track + of pointers to the argument data within a linked list. */ + struct list arg_list; + list_init(&arg_list); + + unsigned int arg_count = 1; + while (arg != NULL) + { + size_t arg_size = (strlen(arg) + 1) * sizeof (char); + if_.esp -= arg_size; + memcpy (if_.esp, arg, arg_size); + + struct arg_elem *arg_elem = malloc(sizeof (struct arg_elem)); + ASSERT (arg_elem != NULL); + arg_elem->arg = arg; + list_push_front (&arg_list, &arg_elem->elem); + + arg_count++; + arg = strtok_r (NULL, " ", &saveptr); + } + + /* Calculate the remaining number of bytes that need to be written + to the user process stack in order to check for possible overflow. */ + size_t align_size = (unsigned int) if_.esp % 4; + size_t argv_data_size = (arg_count + 1) * sizeof (char *); + size_t argv_size = sizeof (char **); + size_t argc_size = sizeof (int); + size_t return_addr_size = sizeof (void *); + size_t remaining_size = align_size + argv_data_size + argv_size + argc_size + + return_addr_size; + + /* If pushing the rest of the data required for the stack would cause + overflow, allocate an extra page. */ + if (PHYS_BASE - if_.esp + remaining_size > PGSIZE) + { + /* TODO: Allocate an extra page for the rest of the process stack. */ + } + + /* Push pointers to process arguments from argument linked list */ + struct list_elem *e = list_begin (&arg_list); + struct list_elem *tail = list_tail (&arg_list); + while (e != tail) + { + /* TODO: Push argument pointers to process stack. */ + + struct list_elem *prev_e = e; + e = list_next (e); + + free (list_entry (prev_e, struct arg_elem, elem)); + } /* If load failed, quit. */ - palloc_free_page (file_name); + palloc_free_page (file_name_); if (!success) thread_exit (); From 92c681ff0274d94144ae19bf58d433be83294064 Mon Sep 17 00:00:00 2001 From: Themis Demetriades Date: Mon, 4 Nov 2024 00:04:08 +0000 Subject: [PATCH 04/15] Reformat start_process stack initialization code to follow style --- src/userprog/process.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/userprog/process.c b/src/userprog/process.c index 8aff29b..122325c 100644 --- a/src/userprog/process.c +++ b/src/userprog/process.c @@ -83,16 +83,16 @@ start_process (void *file_name_) command line input must fit in a page. Also keep track of pointers to the argument data within a linked list. */ struct list arg_list; - list_init(&arg_list); + list_init (&arg_list); unsigned int arg_count = 1; while (arg != NULL) { - size_t arg_size = (strlen(arg) + 1) * sizeof (char); + size_t arg_size = (strlen (arg) + 1) * sizeof (char); if_.esp -= arg_size; memcpy (if_.esp, arg, arg_size); - struct arg_elem *arg_elem = malloc(sizeof (struct arg_elem)); + struct arg_elem *arg_elem = malloc (sizeof (struct arg_elem)); ASSERT (arg_elem != NULL); arg_elem->arg = arg; list_push_front (&arg_list, &arg_elem->elem); From 62d2cb54e5440eea2893d735dff2e84c8b1a8468 Mon Sep 17 00:00:00 2001 From: Themis Demetriades Date: Mon, 4 Nov 2024 00:16:04 +0000 Subject: [PATCH 05/15] Update start_process to push pointers to process arguments onto the process thread's stack --- src/userprog/process.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/userprog/process.c b/src/userprog/process.c index 122325c..ffd964b 100644 --- a/src/userprog/process.c +++ b/src/userprog/process.c @@ -123,12 +123,13 @@ start_process (void *file_name_) struct list_elem *tail = list_tail (&arg_list); while (e != tail) { - /* TODO: Push argument pointers to process stack. */ + struct arg_elem *arg_elem = list_entry (e, struct arg_elem, elem); + + if_.esp -= sizeof (char *); + memcpy (if_.esp, arg_elem->arg, sizeof (char *)); - struct list_elem *prev_e = e; e = list_next (e); - - free (list_entry (prev_e, struct arg_elem, elem)); + free (arg_elem); } /* If load failed, quit. */ From 6f9c911ebe998d4b464f091140664ff556e2829c Mon Sep 17 00:00:00 2001 From: Themis Demetriades Date: Mon, 4 Nov 2024 00:19:36 +0000 Subject: [PATCH 06/15] Update start_process to pad process stack before pushing argv elements for performance --- src/userprog/process.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/userprog/process.c b/src/userprog/process.c index ffd964b..452f020 100644 --- a/src/userprog/process.c +++ b/src/userprog/process.c @@ -118,6 +118,10 @@ start_process (void *file_name_) /* TODO: Allocate an extra page for the rest of the process stack. */ } + /* Align stack pointer to word size before pushing argv elements for + performance. */ + if_.esp -= align_size * sizeof (uint8_t); + /* Push pointers to process arguments from argument linked list */ struct list_elem *e = list_begin (&arg_list); struct list_elem *tail = list_tail (&arg_list); From 6c6ce77824b5a9338e59d2fcf24c9bc3e2059146 Mon Sep 17 00:00:00 2001 From: Themis Demetriades Date: Mon, 4 Nov 2024 01:11:19 +0000 Subject: [PATCH 07/15] Implement complete stack initialization for user processes, without accounting for overflow --- src/userprog/process.c | 44 +++++++++++++++++++++++++++++++++++------- 1 file changed, 37 insertions(+), 7 deletions(-) diff --git a/src/userprog/process.c b/src/userprog/process.c index 452f020..7b6ebe9 100644 --- a/src/userprog/process.c +++ b/src/userprog/process.c @@ -58,6 +58,14 @@ process_execute (const char *file_name) return tid; } +static void * +push_to_stack (void **esp, void *data, size_t data_size) +{ + *esp -= data_size; + memcpy (*esp, data, data_size); + return *esp; +} + /* A thread function that loads a user process and starts it running. */ static void @@ -70,27 +78,32 @@ start_process (void *file_name_) of the process. */ char *saveptr; char *arg = strtok_r (file_name_, " ", &saveptr); + + char file_name[15]; + strlcpy (file_name, arg, 15); /* Initialize interrupt frame and load executable. */ memset (&if_, 0, sizeof if_); if_.gs = if_.fs = if_.es = if_.ds = if_.ss = SEL_UDSEG; if_.cs = SEL_UCSEG; if_.eflags = FLAG_IF | FLAG_MBS; - success = load (arg, &if_.eip, &if_.esp); + success = load (file_name, &if_.eip, &if_.esp); /* Load command line argument *data* to user process stack. This can't cause overflow due to enforcing that the size of command line input must fit in a page. Also keep track of pointers to the argument data within a linked list. */ + char *file_name_ptr = push_to_stack (&if_.esp, file_name, + (strlen (file_name) + 1) + * sizeof (char)); + struct list arg_list; list_init (&arg_list); - unsigned int arg_count = 1; + int arg_count = 1; while (arg != NULL) { - size_t arg_size = (strlen (arg) + 1) * sizeof (char); - if_.esp -= arg_size; - memcpy (if_.esp, arg, arg_size); + push_to_stack (&if_.esp, arg, (strlen (arg) + 1) * sizeof (char)); struct arg_elem *arg_elem = malloc (sizeof (struct arg_elem)); ASSERT (arg_elem != NULL); @@ -122,6 +135,10 @@ start_process (void *file_name_) performance. */ if_.esp -= align_size * sizeof (uint8_t); + /* Push a null pointer sentinel inside argv. */ + if_.esp -= sizeof (char *); + *(char *) if_.esp = NULL; + /* Push pointers to process arguments from argument linked list */ struct list_elem *e = list_begin (&arg_list); struct list_elem *tail = list_tail (&arg_list); @@ -129,13 +146,26 @@ start_process (void *file_name_) { struct arg_elem *arg_elem = list_entry (e, struct arg_elem, elem); - if_.esp -= sizeof (char *); - memcpy (if_.esp, arg_elem->arg, sizeof (char *)); + push_to_stack (&if_.esp, &arg_elem->arg, sizeof (arg_elem->arg)); e = list_next (e); free (arg_elem); } + /* Push pointer to the process file name to the stack. */ + char **argv = push_to_stack (&if_.esp, &file_name_ptr, + sizeof (file_name_ptr)); + + /* Push pointer to the start of argv array. */ + push_to_stack (&if_.esp, &argv, sizeof(argv)); + + /* Push the number of arguments to the stack. */ + push_to_stack (&if_.esp, &arg_count, sizeof (arg_count)); + + /* Push fake return address (null pointer). */ + if_.esp -= sizeof (char *); + *(char *) if_.esp = NULL; + /* If load failed, quit. */ palloc_free_page (file_name_); if (!success) From 2a890d5bd20930ae65dc1fc1e175d8f894bf1dd0 Mon Sep 17 00:00:00 2001 From: Themis Demetriades Date: Mon, 4 Nov 2024 12:54:18 +0000 Subject: [PATCH 08/15] Reformat calculation of padding size in stack set-up to increase clarity --- src/userprog/process.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/userprog/process.c b/src/userprog/process.c index 7b6ebe9..810b449 100644 --- a/src/userprog/process.c +++ b/src/userprog/process.c @@ -2,6 +2,7 @@ #include #include #include +#include #include #include #include @@ -116,7 +117,7 @@ start_process (void *file_name_) /* Calculate the remaining number of bytes that need to be written to the user process stack in order to check for possible overflow. */ - size_t align_size = (unsigned int) if_.esp % 4; + size_t align_size = ((unsigned int) if_.esp % 4) * sizeof (uint8_t); size_t argv_data_size = (arg_count + 1) * sizeof (char *); size_t argv_size = sizeof (char **); size_t argc_size = sizeof (int); @@ -133,7 +134,7 @@ start_process (void *file_name_) /* Align stack pointer to word size before pushing argv elements for performance. */ - if_.esp -= align_size * sizeof (uint8_t); + if_.esp -= align_size; /* Push a null pointer sentinel inside argv. */ if_.esp -= sizeof (char *); From b4c41b0a6a8d766bf2a014de402e4c44d98987ae Mon Sep 17 00:00:00 2001 From: Themis Demetriades Date: Mon, 4 Nov 2024 12:57:29 +0000 Subject: [PATCH 09/15] Remove superfluous include in process.c --- src/userprog/process.c | 1 - 1 file changed, 1 deletion(-) diff --git a/src/userprog/process.c b/src/userprog/process.c index 810b449..aaac9ee 100644 --- a/src/userprog/process.c +++ b/src/userprog/process.c @@ -2,7 +2,6 @@ #include #include #include -#include #include #include #include From b0c1923d44ab971f77401ad9066144e3c60f379d Mon Sep 17 00:00:00 2001 From: Themis Demetriades Date: Wed, 6 Nov 2024 12:28:58 +0000 Subject: [PATCH 10/15] Update stack argument initialization behaviour to terminate creation of process on failing memory allocations --- src/userprog/process.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/userprog/process.c b/src/userprog/process.c index aaac9ee..7f38374 100644 --- a/src/userprog/process.c +++ b/src/userprog/process.c @@ -106,7 +106,15 @@ start_process (void *file_name_) push_to_stack (&if_.esp, arg, (strlen (arg) + 1) * sizeof (char)); struct arg_elem *arg_elem = malloc (sizeof (struct arg_elem)); - ASSERT (arg_elem != NULL); + if (arg_elem == NULL) + { + printf("ERROR: Couldn't allocate argument pointer memory for %s!\n", + file_name); + palloc_free_page (file_name_); + if (success) process_exit (); + thread_exit (); + } + arg_elem->arg = arg; list_push_front (&arg_list, &arg_elem->elem); From 4020a140d245e28905c051de94f2299c9b389233 Mon Sep 17 00:00:00 2001 From: Themis Demetriades Date: Wed, 6 Nov 2024 22:36:43 +0000 Subject: [PATCH 11/15] Fix removal of 'timer.h' include needed for calling timer_sleep in process module --- src/userprog/process.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/userprog/process.c b/src/userprog/process.c index 20e2677..4f72dc0 100644 --- a/src/userprog/process.c +++ b/src/userprog/process.c @@ -20,6 +20,7 @@ #include "threads/thread.h" #include "threads/vaddr.h" #include "threads/synch.h" +#include "devices/timer.h" /* Keeps track of the position of pointers to user program arguments within a linked list. */ From b2764cfa0c67b416355684c02bcce1b6de6e9c22 Mon Sep 17 00:00:00 2001 From: Themis Demetriades Date: Wed, 6 Nov 2024 22:46:11 +0000 Subject: [PATCH 12/15] Revert setup_stack pointer decrement 'hack' faking stack initialization --- src/userprog/process.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/userprog/process.c b/src/userprog/process.c index 4f72dc0..fd07a07 100644 --- a/src/userprog/process.c +++ b/src/userprog/process.c @@ -571,7 +571,7 @@ setup_stack (void **esp) { success = install_page (((uint8_t *) PHYS_BASE) - PGSIZE, kpage, true); if (success) - *esp = PHYS_BASE - 12; + *esp = PHYS_BASE; else palloc_free_page (kpage); } From 1ca9d095128252a22a80301f1b92206f20ad94bf Mon Sep 17 00:00:00 2001 From: Themis Demetriades Date: Wed, 6 Nov 2024 23:01:10 +0000 Subject: [PATCH 13/15] Update exit () syscall to print correct termination message --- src/userprog/syscall.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/userprog/syscall.c b/src/userprog/syscall.c index 1dbe73b..9991156 100644 --- a/src/userprog/syscall.c +++ b/src/userprog/syscall.c @@ -105,6 +105,7 @@ syscall_exit (int status) { /* Sets exit_status of the thread to status. thread_exit () will call process_exit () if user programs are allowed. */ + printf ("%s: exit(%d)\n", thread_current()->name, status); thread_current ()->exit_status = status; thread_exit (); } From 26ae7ac02e9e2b569c6f7b6f40fba1ace80bac83 Mon Sep 17 00:00:00 2001 From: Themis Demetriades Date: Wed, 6 Nov 2024 23:57:48 +0000 Subject: [PATCH 14/15] Fix bug in stack creation which would count one extra argument for argc --- src/userprog/process.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/userprog/process.c b/src/userprog/process.c index fd07a07..d5f0621 100644 --- a/src/userprog/process.c +++ b/src/userprog/process.c @@ -102,7 +102,7 @@ start_process (void *file_name_) struct list arg_list; list_init (&arg_list); - int arg_count = 1; + int arg_count = 0; while (arg != NULL) { push_to_stack (&if_.esp, arg, (strlen (arg) + 1) * sizeof (char)); From 273fb48b31f0829c35a8042d0a7fddb1fe9ccf01 Mon Sep 17 00:00:00 2001 From: Themis Demetriades Date: Thu, 7 Nov 2024 00:40:52 +0000 Subject: [PATCH 15/15] Fix stack initialization to pass stack addreses (rather than thread addresses) for the arguments and only pass name a single time --- src/userprog/process.c | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/src/userprog/process.c b/src/userprog/process.c index d5f0621..0e9fc75 100644 --- a/src/userprog/process.c +++ b/src/userprog/process.c @@ -84,6 +84,10 @@ start_process (void *file_name_) char file_name[15]; strlcpy (file_name, arg, 15); + /* TODO: Move naming of thread to process_execute, so start + tokenizing there. */ + strlcpy (thread_current ()->name, file_name, 15); + /* Initialize interrupt frame and load executable. */ memset (&if_, 0, sizeof if_); if_.gs = if_.fs = if_.es = if_.ds = if_.ss = SEL_UDSEG; @@ -95,10 +99,6 @@ start_process (void *file_name_) This can't cause overflow due to enforcing that the size of command line input must fit in a page. Also keep track of pointers to the argument data within a linked list. */ - char *file_name_ptr = push_to_stack (&if_.esp, file_name, - (strlen (file_name) + 1) - * sizeof (char)); - struct list arg_list; list_init (&arg_list); @@ -117,7 +117,7 @@ start_process (void *file_name_) thread_exit (); } - arg_elem->arg = arg; + arg_elem->arg = if_.esp; list_push_front (&arg_list, &arg_elem->elem); arg_count++; @@ -148,6 +148,9 @@ start_process (void *file_name_) /* Push a null pointer sentinel inside argv. */ if_.esp -= sizeof (char *); *(char *) if_.esp = NULL; + + /* Push pointer to the process file name to the stack. */ + char **argv; /* Push pointers to process arguments from argument linked list */ struct list_elem *e = list_begin (&arg_list); @@ -156,16 +159,12 @@ start_process (void *file_name_) { struct arg_elem *arg_elem = list_entry (e, struct arg_elem, elem); - push_to_stack (&if_.esp, &arg_elem->arg, sizeof (arg_elem->arg)); + argv = push_to_stack (&if_.esp, &arg_elem->arg, sizeof (arg_elem->arg)); e = list_next (e); free (arg_elem); } - /* Push pointer to the process file name to the stack. */ - char **argv = push_to_stack (&if_.esp, &file_name_ptr, - sizeof (file_name_ptr)); - /* Push pointer to the start of argv array. */ push_to_stack (&if_.esp, &argv, sizeof(argv));