Fix some bugs in execve() and make sh use it for process launching.

Interrupting children of sh now always works with ^C :^)

Kernel/MemoryManager.cpp

@@ -152,8 +152,6 @@ auto MemoryManager::ensurePTE(PageDirectory* page_directory, LinearAddress laddr
                 laddr.get(),
                 page_table);
 #endif
-            if (page_table.get() == 0x71d000)
-                ASSERT(page_directory == m_kernel_page_directory);
             page_directory->physical_addresses[page_directory_index] = page_table;
             pde.setPageTableBase(page_table.get());
             pde.setUserAllowed(true);

Kernel/Process.cpp

@@ -346,8 +346,7 @@ int Process::sys$execve(const char* filename, const char** argv, const char** en
         new_page_directory = reinterpret_cast<PageDirectory*>(kmalloc_page_aligned(sizeof(PageDirectory)));
         MM.populate_page_directory(*new_page_directory);
         m_page_directory = new_page_directory;
-
+        MM.enter_process_paging_scope(*this);
-        ProcessPagingScope pagingScope(*this);
         space.hookableAlloc = [&] (const String& name, size_t size) {
             if (!size)
                 return (void*)nullptr;
@@ -357,11 +356,12 @@ int Process::sys$execve(const char* filename, const char** argv, const char** en
         };
         bool success = space.loadELF(move(elfData));
         if (!success) {
-            MM.release_page_directory(*new_page_directory);
             m_page_directory = old_page_directory;
+            MM.enter_process_paging_scope(*this);
+            MM.release_page_directory(*new_page_directory);
             m_regions = move(old_regions);
             m_subregions = move(old_subregions);
-            kprintf("Failure loading ELF %s\n", path.characters());
+            kprintf("sys$execve: Failure loading %s\n", path.characters());
             return -ENOEXEC;
         }
 
@@ -378,8 +378,9 @@ int Process::sys$execve(const char* filename, const char** argv, const char** en
 
         entry_eip = (dword)space.symbolPtr("_start");
         if (!entry_eip) {
-            MM.release_page_directory(*new_page_directory);
             m_page_directory = old_page_directory;
+            MM.enter_process_paging_scope(*this);
+            MM.release_page_directory(*new_page_directory);
             m_regions = move(old_regions);
             m_subregions = move(old_subregions);
             return -ENOEXEC;

Userland/cat.cpp

@@ -5,7 +5,6 @@
 
 int main(int argc, char** argv)
 {
-    setpgid(0, 0);
     if (argc != 2) {
         printf("usage: cat <file>\n");
         return 1;

Userland/sh.cpp

@@ -142,16 +142,15 @@ static bool handle_builtin(int argc, const char** argv, int& retval)
     return false;
 }
 
-static int try_spawn(const char* path, const char** argv)
+static int try_exec(const char* path, const char** argv)
 {
-    int ret = spawn(path, argv);
+    int ret = execve(path, argv, nullptr);
-    if (ret >= 0)
+    assert(ret < 0);
-        return ret;
 
     const char* search_path = "/bin";
     char pathbuf[128];
     sprintf(pathbuf, "%s/%s", search_path, argv[0]);
-    ret = spawn(pathbuf, argv);
+    ret = execve(pathbuf, argv, nullptr);
     if (ret == -1)
         return -1;
     return ret;
@@ -181,21 +180,24 @@ static int runcmd(char* cmd)
         return 0;
     }
 
-    int ret = try_spawn(argv[0], argv);
+    pid_t child = fork();
-    if (ret < 0) {
+    if (!child) {
-        printf("spawn failed: %s (%s)\n", cmd, strerror(errno));
+        setpgid(0, 0);
-        return 1;
+        tcsetpgrp(0, getpid());
+        int ret = try_exec(argv[0], argv);
+        if (ret < 0) {
+            printf("exec failed: %s (%s)\n", cmd, strerror(errno));
+            exit(1);
+        }
+        // We should never get here!
+        assert(false);
     }
 
-    // FIXME: This is racy, since spawn has already started the new process.
-    //        Once I have fork()+exec(), pgrp setup can be done in the child before exec()ing.
-    tcsetpgrp(0, ret);
-
-    // FIXME: waitpid should give us the spawned process's exit status
     int wstatus = 0;
-    waitpid(ret, &wstatus, 0);
+    waitpid(child, &wstatus, 0);
 
-    // FIXME: Racy as mentioned above. Rework once we have fork()+exec().
+    // FIXME: Should I really have to tcsetpgrp() after my child has exited?
+    //        Is the terminal controlling pgrp really still the PGID of the dead process?
     tcsetpgrp(0, getpid());
 
     if (WIFEXITED(wstatus)) {