From: Ben Pfaff <blp@cs.stanford.edu>
Date: Mon, 20 Sep 2004 04:33:01 +0000 (+0000)
Subject: Change interface of addrspace_load() to provide initial stack pointer.
X-Git-Url: https://pintos-os.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=a2a884ae32b2a9e793c0d6540608bac20e855d86;p=pintos-anon

Change interface of addrspace_load() to provide initial stack pointer.
destroy_thread() doesn't really need thread in dying state.
---

diff --git a/src/threads/thread.c b/src/threads/thread.c
index 80c01fd..bdf99e3 100644
--- a/src/threads/thread.c
+++ b/src/threads/thread.c
@@ -2,6 +2,7 @@
 #include <debug.h>
 #include <stddef.h>
 #include <random.h>
+#include <stdio.h>
 #include <string.h>
 #include "threads/flags.h"
 #include "threads/interrupt.h"
@@ -151,7 +152,6 @@ thread_execute (const char *filename)
   struct intr_frame *if_;
   struct switch_entry_frame *ef;
   struct switch_threads_frame *sf;
-  void (*start) (void);
   tid_t tid;
 
   ASSERT (filename != NULL);
@@ -161,17 +161,12 @@ thread_execute (const char *filename)
     return TID_ERROR;
   tid = t->tid;
   
-  if (!addrspace_load (t, filename, &start)) 
-    PANIC ("%s: program load failed", filename);
-
   /* Interrupt frame. */
   if_ = alloc_frame (t, sizeof *if_);
   if_->es = SEL_UDSEG;
   if_->ds = SEL_UDSEG;
-  if_->eip = start;
   if_->cs = SEL_UCSEG;
   if_->eflags = FLAG_IF | FLAG_MBS;
-  if_->esp = PHYS_BASE;
   if_->ss = SEL_UDSEG;
 
   /* Stack frame for switch_entry(). */
@@ -182,6 +177,13 @@ thread_execute (const char *filename)
   sf = alloc_frame (t, sizeof *sf);
   sf->eip = switch_entry;
 
+  /* Load. */
+  if (!addrspace_load (t, filename, &if_->eip, &if_->esp))
+    {
+      destroy_thread (t);
+      return TID_ERROR;
+    }
+
   /* Add to run queue. */
   thread_unblock (t);
 
@@ -397,13 +399,11 @@ next_thread_to_run (void)
     return list_entry (list_pop_front (&ready_list), struct thread, elem);
 }
 
-/* Destroys T, which must be in the dying state and must not be
-   the running thread. */
+/* Destroys T, which must not be the running thread. */
 static void
 destroy_thread (struct thread *t) 
 {
   ASSERT (is_thread (t));
-  ASSERT (t->status == THREAD_DYING);
   ASSERT (t != thread_current ());
 
 #ifdef USERPROG
diff --git a/src/userprog/addrspace.c b/src/userprog/addrspace.c
index f535bad..ccbb356 100644
--- a/src/userprog/addrspace.c
+++ b/src/userprog/addrspace.c
@@ -6,6 +6,7 @@
 #include <string.h>
 #include "userprog/pagedir.h"
 #include "userprog/tss.h"
+#include "filesys/directory.h"
 #include "filesys/file.h"
 #include "filesys/filesys.h"
 #include "threads/init.h"
@@ -78,7 +79,7 @@ struct Elf32_Phdr
 
 static bool load_segment (struct thread *, struct file *,
                           const struct Elf32_Phdr *);
-static bool setup_stack (struct thread *);
+static bool setup_stack (struct thread *, void **esp);
 
 /* Aborts loading an executable, with an error message. */
 #define LOAD_ERROR(MSG)                                         \
@@ -90,10 +91,12 @@ static bool setup_stack (struct thread *);
         } while (0)
 
 /* Loads an ELF executable from FILENAME into T,
-   and stores the executable's entry point into *START.
+   Stores the executable's entry point into *EIP
+   and its initial stack pointer into *ESP.
    Returns true if successful, false otherwise. */
 bool
-addrspace_load (struct thread *t, const char *filename, void (**start) (void)) 
+addrspace_load (struct thread *t, const char *filename,
+                void (**eip) (void), void **esp) 
 {
   struct Elf32_Ehdr ehdr;
   struct file *file = NULL;
@@ -163,11 +166,11 @@ addrspace_load (struct thread *t, const char *filename, void (**start) (void))
     }
 
   /* Set up stack. */
-  if (!setup_stack (t))
+  if (!setup_stack (t, esp))
     goto done;
 
   /* Start address. */
-  *start = (void (*) (void)) ehdr.e_entry;
+  *eip = (void (*) (void)) ehdr.e_entry;
 
   success = true;
 
@@ -299,7 +302,7 @@ load_segment (struct thread *t, struct file *file,
 /* Create a minimal stack for T by mapping a zeroed page at the
    top of user virtual memory. */
 static bool
-setup_stack (struct thread *t) 
+setup_stack (struct thread *t, void **esp) 
 {
   uint8_t *kpage;
   bool success = false;
@@ -308,7 +311,9 @@ setup_stack (struct thread *t)
   if (kpage != NULL) 
     {
       success = install_page (t, ((uint8_t *) PHYS_BASE) - PGSIZE, kpage);
-      if (!success)
+      if (success)
+        *esp = PHYS_BASE;
+      else
         palloc_free (kpage);
     }
   else
diff --git a/src/userprog/addrspace.h b/src/userprog/addrspace.h
index 0ab70b8..0b4b372 100644
--- a/src/userprog/addrspace.h
+++ b/src/userprog/addrspace.h
@@ -4,7 +4,8 @@
 #include <stdbool.h>
 
 struct thread;
-bool addrspace_load (struct thread *, const char *, void (**start) (void));
+bool addrspace_load (struct thread *, const char *,
+                     void (**eip) (void), void **esp);
 void addrspace_destroy (struct thread *);
 void addrspace_activate (struct thread *);