From eb458efa59d65cc2b06011c78cd3b9e126e2b495 Mon Sep 17 00:00:00 2001 From: Themis Demetriades Date: Sun, 3 Nov 2024 17:38:38 +0000 Subject: [PATCH 01/41] 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/41] 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 0bf5fdb0e564213159ece8aa9682866d509d6998 Mon Sep 17 00:00:00 2001 From: sBubshait Date: Sun, 3 Nov 2024 22:28:17 +0000 Subject: [PATCH 03/41] Add syscall_function type definition for the different syscall handlers, w/ E --- src/userprog/syscall.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/userprog/syscall.c b/src/userprog/syscall.c index 370c89b..6f8f660 100644 --- a/src/userprog/syscall.c +++ b/src/userprog/syscall.c @@ -6,6 +6,11 @@ static void syscall_handler (struct intr_frame *); +/* A syscall_function is a function that receives up to 3 arguments, the + arguments to the functions are either ints or pointers taking up to 32 bits + in size. */ +typedef uintptr_t syscall_function (uintptr_t, uintptr_t, uintptr_t); + void syscall_init (void) { From fa6dac21089676b48b3a4433e55ffa3b022b0f6e Mon Sep 17 00:00:00 2001 From: EDiasAlberto Date: Sun, 3 Nov 2024 22:34:50 +0000 Subject: [PATCH 04/41] Implement halt system call w/ S. --- src/userprog/syscall.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/userprog/syscall.c b/src/userprog/syscall.c index 6f8f660..1d3ef62 100644 --- a/src/userprog/syscall.c +++ b/src/userprog/syscall.c @@ -1,4 +1,5 @@ #include "userprog/syscall.h" +#include "devices/shutdown.h" #include #include #include "threads/interrupt.h" @@ -11,6 +12,9 @@ static void syscall_handler (struct intr_frame *); in size. */ typedef uintptr_t syscall_function (uintptr_t, uintptr_t, uintptr_t); +/* System Call Functions */ +static void halt (void); + void syscall_init (void) { @@ -23,3 +27,9 @@ syscall_handler (struct intr_frame *f UNUSED) printf ("system call!\n"); thread_exit (); } + +static void +halt (void) +{ + shutdown_power_off (); +} From c0f85a6bccd74349293151038bbed222b6782a7c Mon Sep 17 00:00:00 2001 From: EDiasAlberto Date: Sun, 3 Nov 2024 22:36:22 +0000 Subject: [PATCH 05/41] Implement skeleton for exit command w/ S. --- src/userprog/syscall.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/userprog/syscall.c b/src/userprog/syscall.c index 1d3ef62..5fec7d3 100644 --- a/src/userprog/syscall.c +++ b/src/userprog/syscall.c @@ -14,6 +14,7 @@ typedef uintptr_t syscall_function (uintptr_t, uintptr_t, uintptr_t); /* System Call Functions */ static void halt (void); +static void exit (int status); void syscall_init (void) @@ -33,3 +34,9 @@ halt (void) { shutdown_power_off (); } + +static void +exit (int status) +{ + //TODO +} \ No newline at end of file From 62453ef432877ec31ea8b72b810d321260301982 Mon Sep 17 00:00:00 2001 From: sBubshait Date: Sun, 3 Nov 2024 22:54:03 +0000 Subject: [PATCH 06/41] Add a look up table from system call numbers to their handler functions, w/ E --- src/userprog/syscall.c | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/src/userprog/syscall.c b/src/userprog/syscall.c index 5fec7d3..6e7fb17 100644 --- a/src/userprog/syscall.c +++ b/src/userprog/syscall.c @@ -10,12 +10,24 @@ static void syscall_handler (struct intr_frame *); /* A syscall_function is a function that receives up to 3 arguments, the arguments to the functions are either ints or pointers taking up to 32 bits in size. */ -typedef uintptr_t syscall_function (uintptr_t, uintptr_t, uintptr_t); +typedef uintptr_t (*syscall_function) (uintptr_t, uintptr_t, uintptr_t); /* System Call Functions */ static void halt (void); static void exit (int status); +/* A struct defining a pair of a syscall_function along with its arity. */ +typedef struct { + syscall_function function; + int arity; +} syscall_arguments; + +/* A look-up table for system call functions mapped using their numbers. */ +static const syscall_arguments syscall_lookup[] = { + [SYS_HALT] = {(syscall_function) halt, 0}, + [SYS_EXIT] = {(syscall_function) exit, 1}, +}; + void syscall_init (void) { From 87126237ad7ac56cac1062982d05b13c4179690f Mon Sep 17 00:00:00 2001 From: EDiasAlberto Date: Sun, 3 Nov 2024 23:20:42 +0000 Subject: [PATCH 07/41] Implement function to validate user memory pointers w/ S. --- src/userprog/syscall.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/userprog/syscall.c b/src/userprog/syscall.c index 6e7fb17..9164142 100644 --- a/src/userprog/syscall.c +++ b/src/userprog/syscall.c @@ -1,5 +1,6 @@ #include "userprog/syscall.h" #include "devices/shutdown.h" +#include "threads/vaddr.h" #include #include #include "threads/interrupt.h" @@ -51,4 +52,15 @@ static void exit (int status) { //TODO +} + +/* A function to validate if a block of memory starting at PTR and of + size SIZE bytes is fully contained within user virtual memory. */ +static void * +validate_user_pointer (void *ptr, size_t size) +{ + if (ptr == NULL || !is_user_vaddr (ptr) || !is_user_vaddr (ptr + size - 1)) + thread_exit (); + + return ptr; } \ No newline at end of file From d626b7a392aca74c5327daf493d5183a24571106 Mon Sep 17 00:00:00 2001 From: sBubshait Date: Sun, 3 Nov 2024 23:47:22 +0000 Subject: [PATCH 08/41] Implement basic syscall_handler using the lookup table, w/ E --- src/userprog/syscall.c | 24 +++++++++++++++++++++--- 1 file changed, 21 insertions(+), 3 deletions(-) diff --git a/src/userprog/syscall.c b/src/userprog/syscall.c index 9164142..5e53cd3 100644 --- a/src/userprog/syscall.c +++ b/src/userprog/syscall.c @@ -17,6 +17,8 @@ typedef uintptr_t (*syscall_function) (uintptr_t, uintptr_t, uintptr_t); static void halt (void); static void exit (int status); +static void *validate_user_pointer(void *ptr, size_t size); + /* A struct defining a pair of a syscall_function along with its arity. */ typedef struct { syscall_function function; @@ -29,6 +31,9 @@ static const syscall_arguments syscall_lookup[] = { [SYS_EXIT] = {(syscall_function) exit, 1}, }; +static const int lookup_size + = sizeof (syscall_lookup) / sizeof (syscall_arguments); + void syscall_init (void) { @@ -36,10 +41,23 @@ syscall_init (void) } static void -syscall_handler (struct intr_frame *f UNUSED) +syscall_handler (struct intr_frame *f) { - printf ("system call!\n"); - thread_exit (); + validate_user_pointer(f->esp, 1); + int syscall_number = *(int *) f->esp; + + if (syscall_number < 0 || syscall_number >= lookup_size) + thread_exit (); + + syscall_arguments syscall = syscall_lookup[syscall_number]; + + validate_user_pointer (f->esp, syscall.arity); + uintptr_t args[3]; + + for (int i=0; i < syscall.arity; i++) + args[i] = *(uintptr_t *) (f->esp + 1 + i); + + f->eax = syscall.function(args[0], args[1], args[2]); } static void From 34d6c15d739ab8804bb4fc0775f3025729f6667d Mon Sep 17 00:00:00 2001 From: Themis Demetriades Date: Mon, 4 Nov 2024 00:00:56 +0000 Subject: [PATCH 09/41] 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 10/41] 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 11/41] 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 12/41] 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 ade8faf0f4abe285c103f8085dd2d8fe22c58197 Mon Sep 17 00:00:00 2001 From: sBubshait Date: Mon, 4 Nov 2024 00:20:09 +0000 Subject: [PATCH 13/41] Update syscall to add more comments explaining the basic handler --- src/userprog/syscall.c | 25 ++++++++++++++++--------- 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/src/userprog/syscall.c b/src/userprog/syscall.c index 5e53cd3..2926bbd 100644 --- a/src/userprog/syscall.c +++ b/src/userprog/syscall.c @@ -13,24 +13,27 @@ static void syscall_handler (struct intr_frame *); in size. */ typedef uintptr_t (*syscall_function) (uintptr_t, uintptr_t, uintptr_t); -/* System Call Functions */ +/* System call function prototypes */ static void halt (void); static void exit (int status); static void *validate_user_pointer(void *ptr, size_t size); -/* A struct defining a pair of a syscall_function along with its arity. */ +/* A struct defining a syscall_function pointer along with its arity. */ typedef struct { - syscall_function function; - int arity; + syscall_function function; /* Function pointer. */ + int arity; /* Number of arguments of the function. */ } syscall_arguments; -/* A look-up table for system call functions mapped using their numbers. */ +/* A look-up table mapping numbers to system call functions with their number of + arguments. */ static const syscall_arguments syscall_lookup[] = { [SYS_HALT] = {(syscall_function) halt, 0}, [SYS_EXIT] = {(syscall_function) exit, 1}, }; +/* The number of syscall functions (i.e, number of elements) within the + syscall_lookup table. */ static const int lookup_size = sizeof (syscall_lookup) / sizeof (syscall_arguments); @@ -43,20 +46,24 @@ syscall_init (void) static void syscall_handler (struct intr_frame *f) { + /* First, read the system call number from the stack. */ validate_user_pointer(f->esp, 1); int syscall_number = *(int *) f->esp; + /* Ensures the number corresponds to a system call that can be handled. */ if (syscall_number < 0 || syscall_number >= lookup_size) thread_exit (); syscall_arguments syscall = syscall_lookup[syscall_number]; + /* Next, read and copy the arguments from the stack pointer. */ validate_user_pointer (f->esp, syscall.arity); uintptr_t args[3]; - for (int i=0; i < syscall.arity; i++) args[i] = *(uintptr_t *) (f->esp + 1 + i); + /* Call the function that handles this system call with the arguments. When + there is a return value it is stored in f->eax. */ f->eax = syscall.function(args[0], args[1], args[2]); } @@ -67,13 +74,13 @@ halt (void) } static void -exit (int status) +exit (int status UNUSED) { //TODO } -/* A function to validate if a block of memory starting at PTR and of - size SIZE bytes is fully contained within user virtual memory. */ +/* Validates if a block of memory starting at PTR and of size SIZE bytes is + fully contained within user virtual memory. */ static void * validate_user_pointer (void *ptr, size_t size) { From e718159ed89acf205213caf4cb218a52bb5312d0 Mon Sep 17 00:00:00 2001 From: sBubshait Date: Mon, 4 Nov 2024 00:29:07 +0000 Subject: [PATCH 14/41] Update syscall to use screaming uppercase casing for a constant --- src/userprog/syscall.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/userprog/syscall.c b/src/userprog/syscall.c index 2926bbd..ddbbd39 100644 --- a/src/userprog/syscall.c +++ b/src/userprog/syscall.c @@ -34,7 +34,7 @@ static const syscall_arguments syscall_lookup[] = { /* The number of syscall functions (i.e, number of elements) within the syscall_lookup table. */ -static const int lookup_size +static const int LOOKUP_SIZE = sizeof (syscall_lookup) / sizeof (syscall_arguments); void @@ -51,7 +51,7 @@ syscall_handler (struct intr_frame *f) int syscall_number = *(int *) f->esp; /* Ensures the number corresponds to a system call that can be handled. */ - if (syscall_number < 0 || syscall_number >= lookup_size) + if (syscall_number < 0 || syscall_number >= LOOKUP_SIZE) thread_exit (); syscall_arguments syscall = syscall_lookup[syscall_number]; From 3a258cf064c51f6f69df48a5bf37de9097f337c7 Mon Sep 17 00:00:00 2001 From: sBubshait Date: Mon, 4 Nov 2024 00:38:58 +0000 Subject: [PATCH 15/41] Update validate_user_pointer to perform no memory checks when size is 0 --- src/userprog/syscall.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/userprog/syscall.c b/src/userprog/syscall.c index ddbbd39..fc8c28e 100644 --- a/src/userprog/syscall.c +++ b/src/userprog/syscall.c @@ -80,11 +80,15 @@ exit (int status UNUSED) } /* Validates if a block of memory starting at PTR and of size SIZE bytes is - fully contained within user virtual memory. */ + fully contained within user virtual memory. Kills the thread (by calling + thread_exit) if the memory is invalid. Otherwise, returns the PTR given. + If the size is 0, the function does no checks and returns PTR.*/ static void * validate_user_pointer (void *ptr, size_t size) { - if (ptr == NULL || !is_user_vaddr (ptr) || !is_user_vaddr (ptr + size - 1)) + if (size > 0 && (ptr == NULL || + !is_user_vaddr (ptr) || + !is_user_vaddr (ptr + size - 1))) thread_exit (); return ptr; From 79f6a8e8080dfd2fc15fd9bd31dc0365600c4600 Mon Sep 17 00:00:00 2001 From: sBubshait Date: Mon, 4 Nov 2024 00:44:55 +0000 Subject: [PATCH 16/41] Fix Bug in syscall handler related to pointer arithmetic: add sizeof uintptr_t instead of 1 --- src/userprog/syscall.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/userprog/syscall.c b/src/userprog/syscall.c index fc8c28e..e2f0163 100644 --- a/src/userprog/syscall.c +++ b/src/userprog/syscall.c @@ -57,10 +57,11 @@ syscall_handler (struct intr_frame *f) syscall_arguments syscall = syscall_lookup[syscall_number]; /* Next, read and copy the arguments from the stack pointer. */ - validate_user_pointer (f->esp, syscall.arity); - uintptr_t args[3]; + validate_user_pointer (f->esp + sizeof (uintptr_t), + syscall.arity * sizeof (uintptr_t)); + uintptr_t args[3] = {0}; for (int i=0; i < syscall.arity; i++) - args[i] = *(uintptr_t *) (f->esp + 1 + i); + args[i] = *(uintptr_t *) (f->esp + sizeof (uintptr_t) * (i + 1)); /* Call the function that handles this system call with the arguments. When there is a return value it is stored in f->eax. */ From 0d057da3dc56dd90fa487df96daf3410ac5ae65d Mon Sep 17 00:00:00 2001 From: sBubshait Date: Mon, 4 Nov 2024 00:48:36 +0000 Subject: [PATCH 17/41] Refactor syscall to follow PintOS style in adding space after after function name in calls --- src/userprog/syscall.c | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/src/userprog/syscall.c b/src/userprog/syscall.c index e2f0163..f225b41 100644 --- a/src/userprog/syscall.c +++ b/src/userprog/syscall.c @@ -17,17 +17,19 @@ typedef uintptr_t (*syscall_function) (uintptr_t, uintptr_t, uintptr_t); static void halt (void); static void exit (int status); -static void *validate_user_pointer(void *ptr, size_t size); +static void *validate_user_pointer (void *ptr, size_t size); /* A struct defining a syscall_function pointer along with its arity. */ -typedef struct { - syscall_function function; /* Function pointer. */ - int arity; /* Number of arguments of the function. */ -} syscall_arguments; +typedef struct + { + syscall_function function; /* Function pointer. */ + int arity; /* Number of arguments of the function. */ + } syscall_arguments; /* A look-up table mapping numbers to system call functions with their number of arguments. */ -static const syscall_arguments syscall_lookup[] = { +static const syscall_arguments syscall_lookup[] = +{ [SYS_HALT] = {(syscall_function) halt, 0}, [SYS_EXIT] = {(syscall_function) exit, 1}, }; @@ -47,7 +49,7 @@ static void syscall_handler (struct intr_frame *f) { /* First, read the system call number from the stack. */ - validate_user_pointer(f->esp, 1); + validate_user_pointer (f->esp, 1); int syscall_number = *(int *) f->esp; /* Ensures the number corresponds to a system call that can be handled. */ @@ -65,7 +67,7 @@ syscall_handler (struct intr_frame *f) /* Call the function that handles this system call with the arguments. When there is a return value it is stored in f->eax. */ - f->eax = syscall.function(args[0], args[1], args[2]); + f->eax = syscall.function (args[0], args[1], args[2]); } static void From 5e2342fad781a75a70bf6f7bda65d8b2b3d71e1a Mon Sep 17 00:00:00 2001 From: sBubshait Date: Mon, 4 Nov 2024 00:49:47 +0000 Subject: [PATCH 18/41] Update syscall to make syscall_number an unsigned integer instead of an int --- src/userprog/syscall.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/userprog/syscall.c b/src/userprog/syscall.c index f225b41..2b504ec 100644 --- a/src/userprog/syscall.c +++ b/src/userprog/syscall.c @@ -50,10 +50,10 @@ syscall_handler (struct intr_frame *f) { /* First, read the system call number from the stack. */ validate_user_pointer (f->esp, 1); - int syscall_number = *(int *) f->esp; + unsigned syscall_number = *(int *) f->esp; /* Ensures the number corresponds to a system call that can be handled. */ - if (syscall_number < 0 || syscall_number >= LOOKUP_SIZE) + if (syscall_number >= LOOKUP_SIZE) thread_exit (); syscall_arguments syscall = syscall_lookup[syscall_number]; From 4c27aa020309f756de85b39b9879dfe2f544b123 Mon Sep 17 00:00:00 2001 From: EDiasAlberto Date: Mon, 4 Nov 2024 00:57:19 +0000 Subject: [PATCH 19/41] Complete syscall lookup table, and syscall stubs and skeletons w/ S. --- src/userprog/syscall.c | 97 ++++++++++++++++++++++++++++++++++++++++++ src/userprog/syscall.h | 2 + 2 files changed, 99 insertions(+) diff --git a/src/userprog/syscall.c b/src/userprog/syscall.c index 5e53cd3..0e0a050 100644 --- a/src/userprog/syscall.c +++ b/src/userprog/syscall.c @@ -16,6 +16,17 @@ typedef uintptr_t (*syscall_function) (uintptr_t, uintptr_t, uintptr_t); /* System Call Functions */ static void halt (void); static void exit (int status); +static pid_t exec (const char *cmd_line); +static int wait (pid_t pid); +static bool file_create (const char *file, unsigned initial_size); +static bool file_remove (const char *file); +static int open (const char *file); +static int filesize (int fd); +static int read (int fd, void *buffer, unsigned size); +static int write (int fd, const void *buffer, unsigned size); +static void seek (int fd, unsigned position); +static unsigned tell (int fd); +static void close (int fd); static void *validate_user_pointer(void *ptr, size_t size); @@ -29,6 +40,17 @@ typedef struct { static const syscall_arguments syscall_lookup[] = { [SYS_HALT] = {(syscall_function) halt, 0}, [SYS_EXIT] = {(syscall_function) exit, 1}, + [SYS_EXEC] = {(syscall_function) exec, 1}, + [SYS_WAIT] = {(syscall_function) wait, 1}, + [SYS_CREATE] = {(syscall_function) file_create, 2}, + [SYS_REMOVE] = {(syscall_function) file_remove, 1}, + [SYS_OPEN] = {(syscall_function) open, 1}, + [SYS_FILESIZE] = {(syscall_function) filesize, 1}, + [SYS_READ] = {(syscall_function) read, 3}, + [SYS_WRITE] = {(syscall_function) write, 3}, + [SYS_SEEK] = {(syscall_function) seek, 2}, + [SYS_TELL] = {(syscall_function) tell, 1}, + [SYS_CLOSE] = {(syscall_function) close, 1}, }; static const int lookup_size @@ -72,6 +94,81 @@ exit (int status) //TODO } +static pid_t +exec (const char *cmd_line) +{ + //TODO + return 0; +} + +static int +wait (pid_t pid) +{ + //TODO + return 0; +} + +static bool +file_create (const char *file, unsigned initial_size) +{ + //TODO + return 0; +} + +static bool +file_remove (const char *file) +{ + //TODO + return 0; +} + +static int +open (const char *file) +{ + //TODO + return 0; +} + +static int +filesize (int fd) +{ + //TODO + return 0; +} + +static int +read (int fd, void *buffer, unsigned size) +{ + //TODO + return 0; +} + +static int +write (int fd, const void *buffer, unsigned size) +{ + //TODO + return 0; +} + +static void +seek (int fd, unsigned position) +{ + //TODO +} + +static unsigned +tell (int fd) +{ + //TODO + return 0; +} + +static void +close (int fd) +{ + //TODO +} + /* A function to validate if a block of memory starting at PTR and of size SIZE bytes is fully contained within user virtual memory. */ static void * diff --git a/src/userprog/syscall.h b/src/userprog/syscall.h index 9059096..702a6c7 100644 --- a/src/userprog/syscall.h +++ b/src/userprog/syscall.h @@ -1,6 +1,8 @@ #ifndef USERPROG_SYSCALL_H #define USERPROG_SYSCALL_H +typedef int pid_t; + void syscall_init (void); #endif /* userprog/syscall.h */ From f8e529e87748ae9c0a9c69544259425ca3fbe59b Mon Sep 17 00:00:00 2001 From: EDiasAlberto Date: Mon, 4 Nov 2024 01:02:04 +0000 Subject: [PATCH 20/41] Add UNUSED tag to system call function skeletons w/ S. --- src/userprog/syscall.c | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/src/userprog/syscall.c b/src/userprog/syscall.c index 61682aa..fedd1a4 100644 --- a/src/userprog/syscall.c +++ b/src/userprog/syscall.c @@ -105,76 +105,76 @@ exit (int status UNUSED) } static pid_t -exec (const char *cmd_line) +exec (const char *cmd_line UNUSED) { //TODO return 0; } static int -wait (pid_t pid) +wait (pid_t pid UNUSED) { //TODO return 0; } static bool -file_create (const char *file, unsigned initial_size) +file_create (const char *file UNUSED, unsigned initial_size UNUSED) { //TODO return 0; } static bool -file_remove (const char *file) +file_remove (const char *file UNUSED) { //TODO return 0; } static int -open (const char *file) +open (const char *file UNUSED) { //TODO return 0; } static int -filesize (int fd) +filesize (int fd UNUSED) { //TODO return 0; } static int -read (int fd, void *buffer, unsigned size) +read (int fd UNUSED, void *buffer UNUSED, unsigned size UNUSED) { //TODO return 0; } static int -write (int fd, const void *buffer, unsigned size) +write (int fd UNUSED, const void *buffer UNUSED, unsigned size UNUSED) { //TODO return 0; } static void -seek (int fd, unsigned position) +seek (int fd UNUSED, unsigned position UNUSED) { //TODO } static unsigned -tell (int fd) +tell (int fd UNUSED) { //TODO return 0; } static void -close (int fd) +close (int fd UNUSED) { //TODO } From 6c6ce77824b5a9338e59d2fcf24c9bc3e2059146 Mon Sep 17 00:00:00 2001 From: Themis Demetriades Date: Mon, 4 Nov 2024 01:11:19 +0000 Subject: [PATCH 21/41] 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 22/41] 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 23/41] 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 2dccd87a76992159c2540604d5f8bd4ec8527982 Mon Sep 17 00:00:00 2001 From: sBubshait Date: Tue, 5 Nov 2024 22:38:09 +0000 Subject: [PATCH 24/41] Update thread to add exit_status, intialised to -1, into the thread structure, w/ E --- src/threads/thread.c | 2 ++ src/threads/thread.h | 1 + 2 files changed, 3 insertions(+) diff --git a/src/threads/thread.c b/src/threads/thread.c index 7102ad2..457f7b9 100644 --- a/src/threads/thread.c +++ b/src/threads/thread.c @@ -659,6 +659,8 @@ init_thread (struct thread *t, const char *name, int nice, int priority, t->recent_cpu = recent_cpu; t->priority = t->base_priority; + t->exit_status = -1; + old_level = intr_disable (); list_push_back (&all_list, &t->allelem); intr_set_level (old_level); diff --git a/src/threads/thread.h b/src/threads/thread.h index c7cc364..1c05030 100644 --- a/src/threads/thread.h +++ b/src/threads/thread.h @@ -111,6 +111,7 @@ struct thread /* Shared between thread.c and synch.c. */ struct list_elem elem; /* List element. */ + int exit_status; /* Exit Status: 0 = successful exit. */ #ifdef USERPROG /* Owned by userprog/process.c. */ From e9c4061531a26621b24e04eac0280384cd509182 Mon Sep 17 00:00:00 2001 From: sBubshait Date: Tue, 5 Nov 2024 22:38:45 +0000 Subject: [PATCH 25/41] Implement the exit system call, w/ E --- src/userprog/syscall.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/userprog/syscall.c b/src/userprog/syscall.c index fedd1a4..5113292 100644 --- a/src/userprog/syscall.c +++ b/src/userprog/syscall.c @@ -99,9 +99,12 @@ halt (void) } static void -exit (int status UNUSED) +exit (int status) { - //TODO + /* Sets exit_status of the thread to status. thread_exit () will call + process_exit () if user programs are allowed. */ + thread_current ()->exit_status = status; + thread_exit (); } static pid_t From 421f2c1206933624a58e9ad82bcf51c24e3ffcc4 Mon Sep 17 00:00:00 2001 From: EDiasAlberto Date: Tue, 5 Nov 2024 22:46:21 +0000 Subject: [PATCH 26/41] Refactor function names and includes in syscall.c to avoid conflicts w/ S. --- src/userprog/syscall.c | 82 +++++++++++++++++++++--------------------- 1 file changed, 41 insertions(+), 41 deletions(-) diff --git a/src/userprog/syscall.c b/src/userprog/syscall.c index 5113292..3cb3d69 100644 --- a/src/userprog/syscall.c +++ b/src/userprog/syscall.c @@ -1,10 +1,10 @@ #include "userprog/syscall.h" #include "devices/shutdown.h" #include "threads/vaddr.h" -#include -#include #include "threads/interrupt.h" #include "threads/thread.h" +#include +#include static void syscall_handler (struct intr_frame *); @@ -14,19 +14,19 @@ static void syscall_handler (struct intr_frame *); typedef uintptr_t (*syscall_function) (uintptr_t, uintptr_t, uintptr_t); /* System call function prototypes */ -static void halt (void); -static void exit (int status); -static pid_t exec (const char *cmd_line); -static int wait (pid_t pid); -static bool file_create (const char *file, unsigned initial_size); -static bool file_remove (const char *file); -static int open (const char *file); -static int filesize (int fd); -static int read (int fd, void *buffer, unsigned size); -static int write (int fd, const void *buffer, unsigned size); -static void seek (int fd, unsigned position); -static unsigned tell (int fd); -static void close (int fd); +static void syscall_halt (void); +static void syscall_exit (int status); +static pid_t syscall_exec (const char *cmd_line); +static int syscall_wait (pid_t pid); +static bool syscall_create (const char *file, unsigned initial_size); +static bool syscall_remove (const char *file); +static int syscall_open (const char *file); +static int syscall_filesize (int fd); +static int syscall_read (int fd, void *buffer, unsigned size); +static int syscall_write (int fd, const void *buffer, unsigned size); +static void syscall_seek (int fd, unsigned position); +static unsigned syscall_tell (int fd); +static void syscall_close (int fd); static void *validate_user_pointer (void *ptr, size_t size); @@ -41,19 +41,19 @@ typedef struct arguments. */ static const syscall_arguments syscall_lookup[] = { - [SYS_HALT] = {(syscall_function) halt, 0}, - [SYS_EXIT] = {(syscall_function) exit, 1}, - [SYS_EXEC] = {(syscall_function) exec, 1}, - [SYS_WAIT] = {(syscall_function) wait, 1}, - [SYS_CREATE] = {(syscall_function) file_create, 2}, - [SYS_REMOVE] = {(syscall_function) file_remove, 1}, - [SYS_OPEN] = {(syscall_function) open, 1}, - [SYS_FILESIZE] = {(syscall_function) filesize, 1}, - [SYS_READ] = {(syscall_function) read, 3}, - [SYS_WRITE] = {(syscall_function) write, 3}, - [SYS_SEEK] = {(syscall_function) seek, 2}, - [SYS_TELL] = {(syscall_function) tell, 1}, - [SYS_CLOSE] = {(syscall_function) close, 1}, + [SYS_HALT] = {(syscall_function) syscall_halt, 0}, + [SYS_EXIT] = {(syscall_function) syscall_exit, 1}, + [SYS_EXEC] = {(syscall_function) syscall_exec, 1}, + [SYS_WAIT] = {(syscall_function) syscall_wait, 1}, + [SYS_CREATE] = {(syscall_function) syscall_create, 2}, + [SYS_REMOVE] = {(syscall_function) syscall_remove, 1}, + [SYS_OPEN] = {(syscall_function) syscall_open, 1}, + [SYS_FILESIZE] = {(syscall_function) syscall_filesize, 1}, + [SYS_READ] = {(syscall_function) syscall_read, 3}, + [SYS_WRITE] = {(syscall_function) syscall_write, 3}, + [SYS_SEEK] = {(syscall_function) syscall_seek, 2}, + [SYS_TELL] = {(syscall_function) syscall_tell, 1}, + [SYS_CLOSE] = {(syscall_function) syscall_close, 1}, }; /* The number of syscall functions (i.e, number of elements) within the @@ -93,13 +93,13 @@ syscall_handler (struct intr_frame *f) } static void -halt (void) +syscall_halt (void) { shutdown_power_off (); } static void -exit (int status) +syscall_exit (int status) { /* Sets exit_status of the thread to status. thread_exit () will call process_exit () if user programs are allowed. */ @@ -108,76 +108,76 @@ exit (int status) } static pid_t -exec (const char *cmd_line UNUSED) +syscall_exec (const char *cmd_line UNUSED) { //TODO return 0; } static int -wait (pid_t pid UNUSED) +syscall_wait (pid_t pid UNUSED) { //TODO return 0; } static bool -file_create (const char *file UNUSED, unsigned initial_size UNUSED) +syscall_create (const char *file UNUSED, unsigned initial_size UNUSED) { //TODO return 0; } static bool -file_remove (const char *file UNUSED) +syscall_remove (const char *file UNUSED) { //TODO return 0; } static int -open (const char *file UNUSED) +syscall_open (const char *file UNUSED) { //TODO return 0; } static int -filesize (int fd UNUSED) +syscall_filesize (int fd UNUSED) { //TODO return 0; } static int -read (int fd UNUSED, void *buffer UNUSED, unsigned size UNUSED) +syscall_read (int fd UNUSED, void *buffer UNUSED, unsigned size UNUSED) { //TODO return 0; } static int -write (int fd UNUSED, const void *buffer UNUSED, unsigned size UNUSED) +syscall_write (int fd UNUSED, const void *buffer UNUSED, unsigned size UNUSED) { //TODO return 0; } static void -seek (int fd UNUSED, unsigned position UNUSED) +syscall_seek (int fd UNUSED, unsigned position UNUSED) { //TODO } static unsigned -tell (int fd UNUSED) +syscall_tell (int fd UNUSED) { //TODO return 0; } static void -close (int fd UNUSED) +syscall_close (int fd UNUSED) { //TODO } From b3e23eb1ccf68a828175db9cbf1f32f870a652ef Mon Sep 17 00:00:00 2001 From: EDiasAlberto Date: Tue, 5 Nov 2024 22:48:35 +0000 Subject: [PATCH 27/41] Implement system call wait w/ S. --- src/userprog/syscall.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/userprog/syscall.c b/src/userprog/syscall.c index 3cb3d69..6eb1bce 100644 --- a/src/userprog/syscall.c +++ b/src/userprog/syscall.c @@ -3,6 +3,7 @@ #include "threads/vaddr.h" #include "threads/interrupt.h" #include "threads/thread.h" +#include "userprog/process.h" #include #include @@ -115,10 +116,9 @@ syscall_exec (const char *cmd_line UNUSED) } static int -syscall_wait (pid_t pid UNUSED) +syscall_wait (pid_t pid) { - //TODO - return 0; + return process_wait (pid); } static bool From 01933cb5de852bcb14b1b225c7512eb0aa8c2255 Mon Sep 17 00:00:00 2001 From: sBubshait Date: Tue, 5 Nov 2024 23:07:07 +0000 Subject: [PATCH 28/41] Implement the write system call, w/ E --- src/userprog/syscall.c | 26 +++++++++++++++++++++----- 1 file changed, 21 insertions(+), 5 deletions(-) diff --git a/src/userprog/syscall.c b/src/userprog/syscall.c index 6eb1bce..dbcd3b8 100644 --- a/src/userprog/syscall.c +++ b/src/userprog/syscall.c @@ -29,7 +29,7 @@ static void syscall_seek (int fd, unsigned position); static unsigned syscall_tell (int fd); static void syscall_close (int fd); -static void *validate_user_pointer (void *ptr, size_t size); +static void *validate_user_pointer (const void *ptr, size_t size); /* A struct defining a syscall_function pointer along with its arity. */ typedef struct @@ -157,10 +157,26 @@ syscall_read (int fd UNUSED, void *buffer UNUSED, unsigned size UNUSED) } static int -syscall_write (int fd UNUSED, const void *buffer UNUSED, unsigned size UNUSED) +syscall_write (int fd, const void *buffer, unsigned size) { - //TODO - return 0; + /* Only console (fd = 1) or other files, not including STDIN, (fd > 1) are + allowed. */ + if (fd <= 0) + return 0; + + validate_user_pointer (buffer, size); + + if (fd == STDOUT_FILENO) + { + /* Writing to the console. */ + putbuf (buffer, size); + return size; + } + else + { + /* Writing to a file. */ + return 0; // TODO: Implement Write to Files + } } static void @@ -187,7 +203,7 @@ syscall_close (int fd UNUSED) thread_exit) if the memory is invalid. Otherwise, returns the PTR given. If the size is 0, the function does no checks and returns PTR.*/ static void * -validate_user_pointer (void *ptr, size_t size) +validate_user_pointer (const void *ptr, size_t size) { if (size > 0 && (ptr == NULL || !is_user_vaddr (ptr) || From f4290c31f325b734b0d1acca1df44b048746b99f Mon Sep 17 00:00:00 2001 From: EDiasAlberto Date: Tue, 5 Nov 2024 23:20:18 +0000 Subject: [PATCH 29/41] Implement syscall_read for console input w/ S. --- src/userprog/syscall.c | 26 +++++++++++++++++++++++--- 1 file changed, 23 insertions(+), 3 deletions(-) diff --git a/src/userprog/syscall.c b/src/userprog/syscall.c index dbcd3b8..6027de2 100644 --- a/src/userprog/syscall.c +++ b/src/userprog/syscall.c @@ -1,5 +1,6 @@ #include "userprog/syscall.h" #include "devices/shutdown.h" +#include "devices/input.h" #include "threads/vaddr.h" #include "threads/interrupt.h" #include "threads/thread.h" @@ -150,10 +151,29 @@ syscall_filesize (int fd UNUSED) } static int -syscall_read (int fd UNUSED, void *buffer UNUSED, unsigned size UNUSED) +syscall_read (int fd, void *buffer, unsigned size) { - //TODO - return 0; + /* Only console (fd = 0) or other files, not including STDOUT, (fd > 1) are + allowed. */ + if (fd < 0 && fd != STDOUT_FILENO) + return -1; + + validate_user_pointer (buffer, size); + + if (fd == STDIN_FILENO) + { + /* Reading from the console. */ + char *write_buffer = buffer; + for (int i = 0; i < size; i++) + write_buffer[i] = input_getc (); + + return size; + } + else + { + /* Reading from a file. */ + return 0; // TODO: Implement Write to Files + } } static int From 02fff62ca2dd77af8f59a9630337a0200dae040b Mon Sep 17 00:00:00 2001 From: sBubshait Date: Tue, 5 Nov 2024 23:24:41 +0000 Subject: [PATCH 30/41] Refactor syscall.c to follow PintOS styling, w/ E --- src/userprog/syscall.c | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/src/userprog/syscall.c b/src/userprog/syscall.c index 6027de2..1dbe73b 100644 --- a/src/userprog/syscall.c +++ b/src/userprog/syscall.c @@ -161,19 +161,19 @@ syscall_read (int fd, void *buffer, unsigned size) validate_user_pointer (buffer, size); if (fd == STDIN_FILENO) - { - /* Reading from the console. */ - char *write_buffer = buffer; - for (int i = 0; i < size; i++) - write_buffer[i] = input_getc (); + { + /* Reading from the console. */ + char *write_buffer = buffer; + for (int i = 0; i < size; i++) + write_buffer[i] = input_getc (); - return size; - } + return size; + } else - { - /* Reading from a file. */ - return 0; // TODO: Implement Write to Files - } + { + /* Reading from a file. */ + return 0; // TODO: Implement Write to Files + } } static int @@ -231,4 +231,4 @@ validate_user_pointer (const void *ptr, size_t size) thread_exit (); return ptr; -} \ No newline at end of file +} From 5aac37d1674dfc5b3b32ea5c53b5d80376283914 Mon Sep 17 00:00:00 2001 From: sBubshait Date: Tue, 5 Nov 2024 23:28:17 +0000 Subject: [PATCH 31/41] Add temporary fixes to process_wait and setup stack, w/ E --- src/userprog/process.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/userprog/process.c b/src/userprog/process.c index 49b9bd5..c9f6339 100644 --- a/src/userprog/process.c +++ b/src/userprog/process.c @@ -14,6 +14,7 @@ #include "threads/flags.h" #include "threads/init.h" #include "threads/interrupt.h" +#include "threads/synch.h" #include "threads/palloc.h" #include "threads/thread.h" #include "threads/vaddr.h" @@ -88,7 +89,8 @@ start_process (void *file_name_) int process_wait (tid_t child_tid UNUSED) { - return -1; + for (;;) + barrier (); } /* Free the current process's resources. */ @@ -451,7 +453,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 b0c1923d44ab971f77401ad9066144e3c60f379d Mon Sep 17 00:00:00 2001 From: Themis Demetriades Date: Wed, 6 Nov 2024 12:28:58 +0000 Subject: [PATCH 32/41] 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 ab716de0a6ed8dcb7b2f880dd3466d14fd727c11 Mon Sep 17 00:00:00 2001 From: sBubshait Date: Wed, 6 Nov 2024 15:46:47 +0000 Subject: [PATCH 33/41] Update process_wait to temporarily sleep for 1 second to allow user programs to run --- 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..017e14b 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 "devices/timer.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; + /* As a temporary wait, waiting will just put the thread to sleep for one + second (TIMER_FREQ = 100 ticks ~ 1 second). */ + /* TODO: Implement process_wait () correctly. Remove the next line. */ + timer_sleep (TIMER_FREQ); + + return 0; /* TODO: Change this too */ } /* Free the current process's resources. */ From fcb7e9e4414d320bef117be95326c81f69d35b98 Mon Sep 17 00:00:00 2001 From: sBubshait Date: Wed, 6 Nov 2024 15:48:27 +0000 Subject: [PATCH 34/41] Update setup_stack to temporarily fake set-up for a stack to prevent page faults in no arg user programs --- 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 017e14b..cde4b4a 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 4020a140d245e28905c051de94f2299c9b389233 Mon Sep 17 00:00:00 2001 From: Themis Demetriades Date: Wed, 6 Nov 2024 22:36:43 +0000 Subject: [PATCH 35/41] 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 36/41] 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 37/41] 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 38/41] 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 39/41] 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)); From 6a3cf67d33497826128e40729c5054747ffa038b Mon Sep 17 00:00:00 2001 From: sBubshait Date: Thu, 7 Nov 2024 11:55:26 +0000 Subject: [PATCH 40/41] Fix Bug in process.c initialising a char to NULL --- src/userprog/process.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/userprog/process.c b/src/userprog/process.c index 0e9fc75..01f30d5 100644 --- a/src/userprog/process.c +++ b/src/userprog/process.c @@ -147,7 +147,7 @@ start_process (void *file_name_) /* Push a null pointer sentinel inside argv. */ if_.esp -= sizeof (char *); - *(char *) if_.esp = NULL; + *(char *) if_.esp = 0; /* Push pointer to the process file name to the stack. */ char **argv; @@ -173,7 +173,7 @@ start_process (void *file_name_) /* Push fake return address (null pointer). */ if_.esp -= sizeof (char *); - *(char *) if_.esp = NULL; + *(char *) if_.esp = 0; /* If load failed, quit. */ palloc_free_page (file_name_); From 39018419cde7bf6dc153eeb1f13fbbba935a8acc Mon Sep 17 00:00:00 2001 From: sBubshait Date: Thu, 7 Nov 2024 12:15:29 +0000 Subject: [PATCH 41/41] Fix Bug in Process.c to print the exit statement in process_exit instead of exit syscall to handle unexpected/bad exits --- src/userprog/process.c | 2 ++ src/userprog/syscall.c | 1 - 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/src/userprog/process.c b/src/userprog/process.c index 01f30d5..6c8ef2d 100644 --- a/src/userprog/process.c +++ b/src/userprog/process.c @@ -217,6 +217,8 @@ process_exit (void) struct thread *cur = thread_current (); uint32_t *pd; + printf ("%s: exit(%d)\n", cur->name, cur->exit_status); + /* Destroy the current process's page directory and switch back to the kernel-only page directory. */ pd = cur->pagedir; diff --git a/src/userprog/syscall.c b/src/userprog/syscall.c index 9991156..1dbe73b 100644 --- a/src/userprog/syscall.c +++ b/src/userprog/syscall.c @@ -105,7 +105,6 @@ 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 (); }