Add memory clobbers to several asm statements,
[pintos-anon] / src / userprog / process.c
index 78b3122b1b6e1dcb1db9ce12d804e46383785a71..675dc254af3db38cfea22ec26d77250aeb9206a9 100644 (file)
@@ -3,6 +3,7 @@
 #include <inttypes.h>
 #include <round.h>
 #include <stdio.h>
+#include <stdlib.h>
 #include <string.h>
 #include "userprog/gdt.h"
 #include "userprog/pagedir.h"
 #include "threads/flags.h"
 #include "threads/init.h"
 #include "threads/interrupt.h"
-#include "threads/mmu.h"
 #include "threads/palloc.h"
 #include "threads/thread.h"
+#include "threads/vaddr.h"
 
 static thread_func execute_thread NO_RETURN;
 static bool load (const char *cmdline, void (**eip) (void), void **esp);
 
 /* Starts a new thread running a user program loaded from
-   FILENAME.  The new thread may be scheduled before
-   process_execute() returns.*/
+   FILENAME.  The new thread may be scheduled (and may even exit)
+   before process_execute() returns.  Returns the new process's
+   thread id, or TID_ERROR if the thread cannot be created. */
 tid_t
-process_execute (const char *filename) 
+process_execute (const char *file_name) 
 {
   char *fn_copy;
   tid_t tid;
 
-  /* Make a copy of FILENAME.
+  /* Make a copy of FILE_NAME.
      Otherwise there's a race between the caller and load(). */
   fn_copy = palloc_get_page (0);
   if (fn_copy == NULL)
     return TID_ERROR;
-  strlcpy (fn_copy, filename, PGSIZE);
+  strlcpy (fn_copy, file_name, PGSIZE);
 
-  /* Create a new thread to execute FILENAME. */
-  tid = thread_create (filename, PRI_DEFAULT, execute_thread, fn_copy);
+  /* Create a new thread to execute FILE_NAME. */
+  tid = thread_create (file_name, PRI_DEFAULT, execute_thread, fn_copy);
   if (tid == TID_ERROR)
     palloc_free_page (fn_copy); 
   return tid;
@@ -46,42 +48,49 @@ process_execute (const char *filename)
 /* A thread function that loads a user process and starts it
    running. */
 static void
-execute_thread (void *filename_)
+execute_thread (void *file_name_)
 {
-  char *filename = filename_;
+  char *file_name = file_name_;
   struct intr_frame if_;
   bool success;
 
   /* Initialize interrupt frame and load executable. */
   memset (&if_, 0, sizeof if_);
-  if_.es = SEL_UDSEG;
-  if_.ds = SEL_UDSEG;
+  if_.gs = if_.fs = if_.es = if_.ds = if_.ss = SEL_UDSEG;
   if_.cs = SEL_UCSEG;
   if_.eflags = FLAG_IF | FLAG_MBS;
-  if_.ss = SEL_UDSEG;
-  success = load (filename, &if_.eip, &if_.esp);
+  success = load (file_name, &if_.eip, &if_.esp);
 
   /* If load failed, quit. */
-  palloc_free_page (filename);
+  palloc_free_page (file_name);
   if (!success) 
     thread_exit ();
 
-  /* Switch page tables. */
-  process_activate ();
-
   /* Start the user process by simulating a return from an
      interrupt, implemented by intr_exit (in
-     threads/intr-stubs.pl).  Because intr_exit takes all of its
+     threads/intr-stubs.S).  Because intr_exit takes all of its
      arguments on the stack in the form of a `struct intr_frame',
      we just point the stack pointer (%esp) to our stack frame
      and jump to it. */
-  asm ("mov %0, %%esp\n"
-       "jmp intr_exit\n"
-       : /* no outputs */
-       : "g" (&if_));
+  asm volatile ("movl %0, %%esp; jmp intr_exit" : : "g" (&if_) : "memory");
   NOT_REACHED ();
 }
 
+/* Waits for thread TID to die and returns its exit status.  If
+   it was terminated by the kernel (i.e. killed due to an
+   exception), returns -1.  If TID is invalid or if it was not a
+   child of the calling process, or if process_wait() has already
+   been successfully called for the given TID, returns -1
+   immediately, without waiting.
+
+   This function will be implemented in problem 2-2.  For now, it
+   does nothing. */
+int
+process_wait (tid_t child_tid UNUSED) 
+{
+  return -1;
+}
+
 /* Free the current process's resources. */
 void
 process_exit (void)
@@ -90,13 +99,17 @@ process_exit (void)
   uint32_t *pd;
 
   /* Destroy the current process's page directory and switch back
-     to the kernel-only page directory.  We have to set
-     cur->pagedir to NULL before switching page directories, or a
-     timer interrupt might switch back to the process page
-     directory. */
+     to the kernel-only page directory. */
   pd = cur->pagedir;
   if (pd != NULL) 
     {
+      /* Correct ordering here is crucial.  We must set
+         cur->pagedir to NULL before switching page directories,
+         so that a timer interrupt can't switch back to the
+         process page directory.  We must activate the base page
+         directory before destroying the process's page
+         directory, or our active page directory will be one
+         that's been freed (and cleared). */
       cur->pagedir = NULL;
       pagedir_activate (NULL);
       pagedir_destroy (pd);
@@ -181,24 +194,18 @@ struct Elf32_Phdr
 #define PF_W 2          /* Writable. */
 #define PF_R 4          /* Readable. */
 
-static bool load_segment (struct file *, const struct Elf32_Phdr *);
 static bool setup_stack (void **esp);
+static bool validate_segment (const struct Elf32_Phdr *, struct file *);
+static bool load_segment (struct file *file, off_t ofs, uint8_t *upage,
+                          uint32_t read_bytes, uint32_t zero_bytes,
+                          bool writable);
 
-/* Aborts loading an executable, with an error message. */
-#define LOAD_ERROR(MSG)                                         \
-        do {                                                    \
-                printf ("load: %s: ", filename);      \
-                printf MSG;                                     \
-                printf ("\n");                                  \
-                goto done;                                     \
-        } while (0)
-
-/* Loads an ELF executable from FILENAME into the current thread.
+/* Loads an ELF executable from FILE_NAME into the current thread.
    Stores the executable's entry point into *EIP
    and its initial stack pointer into *ESP.
    Returns true if successful, false otherwise. */
 bool
-load (const char *filename, void (**eip) (void), void **esp) 
+load (const char *file_name, void (**eip) (void), void **esp) 
 {
   struct thread *t = thread_current ();
   struct Elf32_Ehdr ehdr;
@@ -207,32 +214,32 @@ load (const char *filename, void (**eip) (void), void **esp)
   bool success = false;
   int i;
 
-  /* Allocate page directory. */
+  /* Allocate and activate page directory. */
   t->pagedir = pagedir_create ();
-  if (t->pagedir == NULL)
-    LOAD_ERROR (("page directory allocation failed"));
+  if (t->pagedir == NULL) 
+    goto done;
+  process_activate ();
 
   /* Open executable file. */
-  file = filesys_open (filename);
-  if (file == NULL)
-    LOAD_ERROR (("open failed"));
+  file = filesys_open (file_name);
+  if (file == NULL) 
+    {
+      printf ("load: %s: open failed\n", file_name);
+      goto done; 
+    }
 
   /* Read and verify executable header. */
-  if (file_read (file, &ehdr, sizeof ehdr) != sizeof ehdr) 
-    LOAD_ERROR (("error reading executable header"));
-  if (memcmp (ehdr.e_ident, "\177ELF\1\1\1", 7) != 0)
-    LOAD_ERROR (("file is not ELF"));
-  if (ehdr.e_type != 2)
-    LOAD_ERROR (("ELF file is not an executable"));
-  if (ehdr.e_machine != 3)
-    LOAD_ERROR (("ELF executable is not x86"));
-  if (ehdr.e_version != 1)
-    LOAD_ERROR (("ELF executable has unknown version %d",
-                 (int) ehdr.e_version));
-  if (ehdr.e_phentsize != sizeof (struct Elf32_Phdr))
-    LOAD_ERROR (("bad ELF program header size"));
-  if (ehdr.e_phnum > 1024)
-    LOAD_ERROR (("too many ELF program headers"));
+  if (file_read (file, &ehdr, sizeof ehdr) != sizeof ehdr
+      || memcmp (ehdr.e_ident, "\177ELF\1\1\1", 7)
+      || ehdr.e_type != 2
+      || ehdr.e_machine != 3
+      || ehdr.e_version != 1
+      || ehdr.e_phentsize != sizeof (struct Elf32_Phdr)
+      || ehdr.e_phnum > 1024) 
+    {
+      printf ("load: %s: error loading executable\n", file_name);
+      goto done; 
+    }
 
   /* Read program headers. */
   file_ofs = ehdr.e_phoff;
@@ -240,9 +247,12 @@ load (const char *filename, void (**eip) (void), void **esp)
     {
       struct Elf32_Phdr phdr;
 
+      if (file_ofs < 0 || file_ofs > file_length (file))
+        goto done;
       file_seek (file, file_ofs);
+
       if (file_read (file, &phdr, sizeof phdr) != sizeof phdr)
-        LOAD_ERROR (("error reading program header"));
+        goto done;
       file_ofs += sizeof phdr;
       switch (phdr.p_type) 
         {
@@ -250,19 +260,41 @@ load (const char *filename, void (**eip) (void), void **esp)
         case PT_NOTE:
         case PT_PHDR:
         case PT_STACK:
+        default:
           /* Ignore this segment. */
           break;
         case PT_DYNAMIC:
         case PT_INTERP:
         case PT_SHLIB:
-          /* Reject the executable. */
-          LOAD_ERROR (("unsupported ELF segment type %d\n", phdr.p_type));
-          break;
-        default:
-          printf ("unknown ELF segment type %08x\n", phdr.p_type);
-          break;
+          goto done;
         case PT_LOAD:
-          if (!load_segment (file, &phdr))
+          if (validate_segment (&phdr, file)) 
+            {
+              bool writable = (phdr.p_flags & PF_W) != 0;
+              uint32_t file_page = phdr.p_offset & ~PGMASK;
+              uint32_t mem_page = phdr.p_vaddr & ~PGMASK;
+              uint32_t page_offset = phdr.p_vaddr & PGMASK;
+              uint32_t read_bytes, zero_bytes;
+              if (phdr.p_filesz > 0)
+                {
+                  /* Normal segment.
+                     Read initial part from disk and zero the rest. */
+                  read_bytes = page_offset + phdr.p_filesz;
+                  zero_bytes = (ROUND_UP (page_offset + phdr.p_memsz, PGSIZE)
+                                - read_bytes);
+                }
+              else 
+                {
+                  /* Entirely zero.
+                     Don't read anything from disk. */
+                  read_bytes = 0;
+                  zero_bytes = ROUND_UP (page_offset + phdr.p_memsz, PGSIZE);
+                }
+              if (!load_segment (file, file_page, (void *) mem_page,
+                                 read_bytes, zero_bytes, writable))
+                goto done;
+            }
+          else
             goto done;
           break;
         }
@@ -285,89 +317,109 @@ load (const char *filename, void (**eip) (void), void **esp)
 \f
 /* load() helpers. */
 
-static bool install_page (void *upage, void *kpage);
+static bool install_page (void *upage, void *kpage, bool writable);
 
-/* Loads the segment described by PHDR from FILE into user
-   address space.  Return true if successful, false otherwise. */
+/* Checks whether PHDR describes a valid, loadable segment in
+   FILE and returns true if so, false otherwise. */
 static bool
-load_segment (struct file *file, const struct Elf32_Phdr *phdr
+validate_segment (const struct Elf32_Phdr *phdr, struct file *file
 {
-  void *start, *end;  /* Page-rounded segment start and end. */
-  uint8_t *upage;     /* Iterator from start to end. */
-  off_t filesz_left;  /* Bytes left of file data (as opposed to
-                         zero-initialized bytes). */
-
-  /* Is this a read-only segment?  Not currently used, so it's
-     commented out.  You'll want to use it when implementing VM
-     to decide whether to page the segment from its executable or
-     from swap. */
-  //bool read_only = (phdr->p_flags & PF_W) == 0;
-
-  ASSERT (file != NULL);
-  ASSERT (phdr != NULL);
-  ASSERT (phdr->p_type == PT_LOAD);
-
-  /* [ELF1] 2-2 says that p_offset and p_vaddr must be congruent
-     modulo PGSIZE. */
-  if (phdr->p_offset % PGSIZE != phdr->p_vaddr % PGSIZE) 
-    {
-      printf ("%#08"PE32Ox" and %#08"PE32Ax" not congruent modulo %#x\n",
-              phdr->p_offset, phdr->p_vaddr, (unsigned) PGSIZE);
-      return false; 
-    }
+  /* p_offset and p_vaddr must have the same page offset. */
+  if ((phdr->p_offset & PGMASK) != (phdr->p_vaddr & PGMASK)) 
+    return false; 
+
+  /* p_offset must point within FILE. */
+  if (phdr->p_offset > (Elf32_Off) file_length (file)) 
+    return false;
 
-  /* [ELF1] 2-3 says that p_memsz must be at least as big as
-     p_filesz. */
+  /* p_memsz must be at least as big as p_filesz. */
   if (phdr->p_memsz < phdr->p_filesz) 
-    {
-      printf ("p_memsz (%08"PE32Wx") < p_filesz (%08"PE32Wx")\n",
-              phdr->p_memsz, phdr->p_filesz);
-      return false; 
-    }
+    return false; 
+
+  /* The segment must not be empty. */
+  if (phdr->p_memsz == 0)
+    return false;
+  
+  /* The virtual memory region must both start and end within the
+     user address space range. */
+  if (!is_user_vaddr ((void *) phdr->p_vaddr))
+    return false;
+  if (!is_user_vaddr ((void *) (phdr->p_vaddr + phdr->p_memsz)))
+    return false;
+
+  /* The region cannot "wrap around" across the kernel virtual
+     address space. */
+  if (phdr->p_vaddr + phdr->p_memsz < phdr->p_vaddr)
+    return false;
+
+  /* Disallow mapping page 0.
+     Not only is it a bad idea to map page 0, but if we allowed
+     it then user code that passed a null pointer to system calls
+     could quite likely panic the kernel by way of null pointer
+     assertions in memcpy(), etc. */
+  if (phdr->p_vaddr < PGSIZE)
+    return false;
+
+  /* It's okay. */
+  return true;
+}
 
-  /* Validate virtual memory region to be mapped.
-     The region must both start and end within the user address
-     space range starting at 0 and ending at PHYS_BASE (typically
-     3 GB == 0xc0000000). */
-  start = pg_round_down ((void *) phdr->p_vaddr);
-  end = pg_round_up ((void *) (phdr->p_vaddr + phdr->p_memsz));
-  if (start >= PHYS_BASE || end >= PHYS_BASE || end < start) 
-    {
-      printf ("bad virtual region %08lx...%08lx\n",
-              (unsigned long) start, (unsigned long) end);
-      return false; 
-    }
+/* Loads a segment starting at offset OFS in FILE at address
+   UPAGE.  In total, READ_BYTES + ZERO_BYTES bytes of virtual
+   memory are initialized, as follows:
+
+        - READ_BYTES bytes at UPAGE must be read from FILE
+          starting at offset OFS.
 
-  /* Load the segment page-by-page into memory. */
-  filesz_left = phdr->p_filesz + (phdr->p_vaddr & PGMASK);
-  file_seek (file, ROUND_DOWN (phdr->p_offset, PGSIZE));
-  for (upage = start; upage < (uint8_t *) end; upage += PGSIZE) 
+        - ZERO_BYTES bytes at UPAGE + READ_BYTES must be zeroed.
+
+   The pages initialized by this function must be writable by the
+   user process if WRITABLE is true, read-only otherwise.
+
+   Return true if successful, false if a memory allocation error
+   or disk read error occurs. */
+static bool
+load_segment (struct file *file, off_t ofs, uint8_t *upage,
+              uint32_t read_bytes, uint32_t zero_bytes, bool writable) 
+{
+  ASSERT ((read_bytes + zero_bytes) % PGSIZE == 0);
+  ASSERT (pg_ofs (upage) == 0);
+  ASSERT (ofs % PGSIZE == 0);
+
+  file_seek (file, ofs);
+  while (read_bytes > 0 || zero_bytes > 0) 
     {
-      /* We want to read min(PGSIZE, filesz_left) bytes from the
-         file into the page and zero the rest. */
-      size_t read_bytes = filesz_left >= PGSIZE ? PGSIZE : filesz_left;
-      size_t zero_bytes = PGSIZE - read_bytes;
+      /* Calculate how to fill this page.
+         We will read PAGE_READ_BYTES bytes from FILE
+         and zero the final PAGE_ZERO_BYTES bytes. */
+      size_t page_read_bytes = read_bytes < PGSIZE ? read_bytes : PGSIZE;
+      size_t page_zero_bytes = PGSIZE - page_read_bytes;
+
+      /* Get a page of memory. */
       uint8_t *kpage = palloc_get_page (PAL_USER);
       if (kpage == NULL)
         return false;
 
-      /* Do the reading and zeroing. */
-      if (file_read (file, kpage, read_bytes) != (int) read_bytes) 
+      /* Load this page. */
+      if (file_read (file, kpage, page_read_bytes) != (int) page_read_bytes)
         {
           palloc_free_page (kpage);
           return false; 
         }
-      memset (kpage + read_bytes, 0, zero_bytes);
-      filesz_left -= read_bytes;
+      memset (kpage + page_read_bytes, 0, page_zero_bytes);
 
       /* Add the page to the process's address space. */
-      if (!install_page (upage, kpage)) 
+      if (!install_page (upage, kpage, writable)) 
         {
           palloc_free_page (kpage);
           return false; 
         }
-    }
 
+      /* Advance. */
+      read_bytes -= page_read_bytes;
+      zero_bytes -= page_zero_bytes;
+      upage += PGSIZE;
+    }
   return true;
 }
 
@@ -382,28 +434,31 @@ setup_stack (void **esp)
   kpage = palloc_get_page (PAL_USER | PAL_ZERO);
   if (kpage != NULL) 
     {
-      success = install_page (((uint8_t *) PHYS_BASE) - PGSIZE, kpage);
+      success = install_page (((uint8_t *) PHYS_BASE) - PGSIZE, kpage, true);
       if (success)
         *esp = PHYS_BASE;
       else
         palloc_free_page (kpage);
     }
-  else
-    printf ("failed to allocate process stack\n");
-
   return success;
 }
 
 /* Adds a mapping from user virtual address UPAGE to kernel
-   virtual address KPAGE to the page table.  Fails if UPAGE is
-   already mapped or if memory allocation fails. */
+   virtual address KPAGE to the page table.
+   If WRITABLE is true, the user process may modify the page;
+   otherwise, it is read-only.
+   UPAGE must not already be mapped.
+   KPAGE should probably be a page obtained from the user pool
+   with palloc_get_page().
+   Returns true on success, false if UPAGE is already mapped or
+   if memory allocation fails. */
 static bool
-install_page (void *upage, void *kpage)
+install_page (void *upage, void *kpage, bool writable)
 {
   struct thread *t = thread_current ();
 
   /* Verify that there's not already a page at that virtual
      address, then map our page there. */
   return (pagedir_get_page (t->pagedir, upage) == NULL
-          && pagedir_set_page (t->pagedir, upage, kpage, true));
+          && pagedir_set_page (t->pagedir, upage, kpage, writable));
 }