Browse Source

believe argument passing in runprogram working, have base execv without agrs working. with args is still not great

tarfeef101 6 years ago
parent
commit
b9192e35bf

+ 1 - 0
kern/arch/mips/syscall/syscall.c

@@ -137,6 +137,7 @@ syscall(struct trapframe * tf)
 	  break;
 	 case SYS_execv:
 	  err = sys_execv((char *)tf->tf_a0,(userptr_t)tf->tf_a1);
+    break;
  
 	default:
 	  kprintf("Unknown syscall %d\n", callno);

BIN
kern/compile/ASST2/kernel


+ 1 - 1
kern/compile/ASST2/vers.c

@@ -1,3 +1,3 @@
 /* This file is automatically generated. Edits will be lost.*/
-const int buildversion = 125;
+const int buildversion = 159;
 const char buildconfig[] = "ASST2";

+ 1 - 1
kern/compile/ASST2/version

@@ -1 +1 @@
-125
+159

+ 1 - 0
kern/include/syscall.h

@@ -65,6 +65,7 @@ int sys_getpid(pid_t *  retval);
 int sys_waitpid(pid_t pid, userptr_t status, int options, pid_t * retval);
 int sys_fork(struct trapframe * tf, int * retval);
 int sys_execv(const char * program, userptr_t args);
+int copyArgs(int argc, char ** argv, userptr_t * argvAddr, vaddr_t * stackptr);
 
 #endif // UW
 

+ 1 - 1
kern/include/test.h

@@ -82,7 +82,7 @@ int mallocstress(int, char **);
 int nettest(int, char **);
 
 /* Routine for running a user-level program. */
-int runprogram(char *progname);
+int runprogram(char * progname, int argc, char ** argv);
 
 /* Kernel menu system. */
 void menu(char *argstr);

+ 1 - 5
kern/startup/menu.c

@@ -93,16 +93,12 @@ cmd_progthread(void *ptr, unsigned long nargs)
 
 	KASSERT(nargs >= 1);
 
-	if (nargs > 2) {
-		kprintf("Warning: argument passing from menu not supported\n");
-	}
-
 	/* Hope we fit. */
 	KASSERT(strlen(args[0]) < sizeof(progname));
 
 	strcpy(progname, args[0]);
 
-	result = runprogram(progname);
+	result = runprogram(progname, nargs, args);
 	if (result) {
 		kprintf("Running program %s failed: %s\n", args[0],
 			strerror(result));

+ 104 - 15
kern/syscall/proc_syscalls.c

@@ -44,11 +44,11 @@ void sys__exit(int exitcode)
   listelete(p->kids);
   
   // VFS fields
-	if (p->p_cwd)
-	{
-		VOP_DECREF(p->p_cwd);
-		p->p_cwd = NULL;
-	}
+  if (p->p_cwd)
+  {
+    VOP_DECREF(p->p_cwd);
+    p->p_cwd = NULL;
+  }
   
   as_deactivate();
   as = curproc_setas(NULL);
@@ -59,7 +59,7 @@ void sys__exit(int exitcode)
   proc_remthread(curthread);
 
   threadarray_cleanup(&p->p_threads);
-	spinlock_cleanup(&p->p_lock);
+  spinlock_cleanup(&p->p_lock);
 
   lock_acquire(p->waitlock);
   cv_broadcast(p->waiting, p->waitlock);
@@ -139,6 +139,56 @@ int sys_fork(struct trapframe * tf, int * retval)
   return 0;
 }
 
+// this function takes an argc, argv, and puts argv into the userptr argvaddr, on the stack stackptr
+int copyArgs(int argc, char ** argv, userptr_t * argvAddr, vaddr_t * stackptr)
+{
+  vaddr_t stack = *stackptr; // for ease of referencing
+  char ** newArgv = kmalloc(sizeof(char *) * (argc + 1)); // need argc + 1 space since we want a final address to be null
+  size_t wasteOfSpace;
+  int errcode;
+  
+  for(int i = 0; i < argc; ++i)
+  {
+    int arglen = strlen(*(argv + i)) + 1; // length of char array for this arg
+    stack -= ROUNDUP(arglen, 8); // make the stack bigger by this length, but rounded to 8 cause bytes
+  errcode = copyoutstr(*(argv + i), (userptr_t)stack, arglen, &wasteOfSpace); // take the string in argv at i, put it at the stack position we calculated just before as the start for a string of our length
+    if(errcode)
+    {
+      kfree(newArgv);
+      return errcode; // if an error, fuck life
+    }
+    *(newArgv + i) = (char *)stack; // our argv kernel array is going to contain the user space address
+  }
+  
+  *(newArgv + argc) = NULL; // set final address to NULL
+  
+  for(int i = 0; i <= argc; ++i) // after we store the array contents, we want to store the array of pointers to those contents. which is "before" in the stack becuase fuck you, that's why
+  {
+    stack -= sizeof(char *); // move the stack pointer back one pointer worth of space
+    errcode = copyout(newArgv + (argc - i), (userptr_t)stack, sizeof(char *)); // copy the pointers in reverse order into the stack, since we are going backwards in the stack.
+    if(errcode)
+    {
+      kfree(newArgv);
+      return errcode; // cry. just cry. then return your error.
+    }
+  }
+
+  *argvAddr = (userptr_t)stack; // set the argv array in userland to start at where we put it (current stackptr location)
+  if (stack % 8 == 0) stack -= 8; // move the stack pointer back the minimum amount (if we are at an 8 byte address already, a full 8 bytes. otherwise we must be at a 4 byte address, so just half that)
+  else stack -= 4;
+  *stackptr = stack; // set the real stack pointer to the one we've been fucking with
+  kfree(newArgv); // all data copied over, can free our temp array now
+  return 0; // return no error
+}
+
+static void freeArgv(char ** argv, int len)
+{
+  for (int i = 0; i <= len; ++i)
+  {
+    kfree(argv[i]);
+  }
+}
+
 // args is an array of null terminated strings, with the last element being a null pointer so we don't overflow if iterating
 // since this is a userptr type, we need to use copyin and copyout to get data properly
 // hopefully the path in program is actually a full filepath
@@ -146,14 +196,13 @@ int sys_execv(const char * program, userptr_t args)
 {
   userptr_t temp = args;
   int argcount = 0;
-  char ** argv;
   int errcode; // why don't we have errno!!! my beautful one-liners!!!
-  
+
   // see how long args is
   while (1)
   {
     char * temp2;
-    errcode = copyin((const userptr_t)temp, temp2, sizeof(char *));
+    errcode = copyin(temp, temp2, sizeof(char *));
     if (errcode) return errcode;
     if (!(temp2)) break;
     temp += sizeof(char *);
@@ -161,20 +210,40 @@ int sys_execv(const char * program, userptr_t args)
   }
   
   // allocate space for argv
-  argv = kmalloc(argcount * sizeof(char *));
+  char ** argv = kmalloc(argcount * sizeof(char *));
   if (!argv) return ENOMEM;
   
   // get kernel copy of args in argv
   temp = args;
   for (int i = 0; i < argcount; ++i)
   {
-    errcode = copyin(temp, argv[i], sizeof(char *));
+    char * useraddr;
+
+    // useraddr is now the address of the string in userland
+    errcode = copyin(temp, useraddr, sizeof(char *));
+    if (errcode)
+    {
+      kfree(argv);
+      return errcode;
+    }
+ 
+    // change argv[i] to be a string of length enough
+    int strLen = strlen(useraddr) + 1;
+    size_t wasteOfSpace;
+    argv[i] = kmalloc(strLen * sizeof(char));
+    if (!(argv[i]))
+    {
+      freeArgv(argv, i);
+      kfree(argv);
+      return ENOMEM;
+    }
+    errcode = copyinstr(temp, argv[i], strLen, &wasteOfSpace);
     if (errcode)
     {
+      freeArgv(argv, i);
       kfree(argv);
       return errcode;
     }
-    
     temp += sizeof(char *);
   }
   
@@ -182,6 +251,7 @@ int sys_execv(const char * program, userptr_t args)
   char * filepath = kmalloc(sizeof(program));
   if (!(filepath))
   {
+    freeArgv(argv, argcount - 1);
     kfree(argv);
     return ENOMEM;
   }
@@ -191,6 +261,7 @@ int sys_execv(const char * program, userptr_t args)
   errcode = copyinstr((const_userptr_t)program, filepath, strlen(program) + 1, &useless);
   if (errcode)
   {
+    freeArgv(argv, argcount - 1);
     kfree(argv);
     kfree(filepath);
     return errcode;
@@ -201,6 +272,7 @@ int sys_execv(const char * program, userptr_t args)
   errcode = vfs_open(filepath, O_RDONLY, 0, &v);
   if (errcode)
   {
+    freeArgv(argv, argcount - 1);
     kfree(argv);
     kfree(filepath);
     return errcode;
@@ -211,6 +283,7 @@ int sys_execv(const char * program, userptr_t args)
   if (!(as))
   {
     vfs_close(v);
+    freeArgv(argv, argcount - 1);
     kfree(argv);
     kfree(filepath);
     return ENOMEM;
@@ -227,6 +300,7 @@ int sys_execv(const char * program, userptr_t args)
   if (errcode)
   {
     vfs_close(v);
+    freeArgv(argv, argcount - 1);
     kfree(argv);
     kfree(filepath);
     curproc_setas(oldas);
@@ -242,20 +316,35 @@ int sys_execv(const char * program, userptr_t args)
   errcode = as_define_stack(as, &newstack);
   if (errcode)
   {
+    freeArgv(argv, argcount - 1);
+    kfree(argv);
+    kfree(filepath);
+    curproc_setas(oldas);
+    as_destroy(as);
+    return errcode;
+  }
+
+  userptr_t userV;
+  errcode = copyArgs(argcount, argv, &userV, &newstack);
+  if (errcode)
+  {
+    freeArgv(argv, argcount - 1);
     kfree(argv);
     kfree(filepath);
     curproc_setas(oldas);
     as_destroy(as);
     return errcode;
   }
-  
-  userptr_t userArgv;
   
   // need to copy data over into the new address space, as rn it is all in kernel heap
 
   // delete old addrspace, enter new process
   as_destroy(oldas);
-  enter_new_process(argcount, userArgv, newstack, entrypoint);
+  kfree(filepath);
+  freeArgv(argv, argcount - 1);
+  kfree(argv);
+  //kprintf("agrc: %d, userv: %p, stack: %p, entry: %p\n", argcount, userV, (void *)newstack, (void *)entrypoint);
+  enter_new_process(argcount, userV, newstack, entrypoint);
   panic("Enter new process returned!\n");
   return EINVAL;
 }

+ 12 - 13
kern/syscall/runprogram.c

@@ -51,8 +51,7 @@
  *
  * Calls vfs_open on progname and thus may destroy it.
  */
-int
-runprogram(char *progname)
+int runprogram(char * progname, int argc, char ** argv)
 {
 	struct addrspace *as;
 	struct vnode *v;
@@ -61,16 +60,15 @@ runprogram(char *progname)
 
 	/* Open the file. */
 	result = vfs_open(progname, O_RDONLY, 0, &v);
-	if (result) {
-		return result;
-	}
+	if (result) return result;
 
 	/* We should be a new process. */
 	KASSERT(curproc_getas() == NULL);
 
 	/* Create a new address space. */
 	as = as_create();
-	if (as ==NULL) {
+	if (!(as))
+  {
 		vfs_close(v);
 		return ENOMEM;
 	}
@@ -81,7 +79,8 @@ runprogram(char *progname)
 
 	/* Load the executable. */
 	result = load_elf(v, &entrypoint);
-	if (result) {
+	if (result)
+  {
 		/* p_addrspace will go away when curproc is destroyed */
 		vfs_close(v);
 		return result;
@@ -92,14 +91,14 @@ runprogram(char *progname)
 
 	/* Define the user stack in the address space */
 	result = as_define_stack(as, &stackptr);
-	if (result) {
-		/* p_addrspace will go away when curproc is destroyed */
-		return result;
-	}
+	if (result) return result;
+
+  // copy args into a new argv
+  userptr_t userv;
+  copyArgs(argc, argv, &userv, &stackptr);
 
 	/* Warp to user mode. */
-	enter_new_process(0 /*argc*/, NULL /*userspace addr of argv*/,
-			  stackptr, entrypoint);
+	enter_new_process(argc, userv, stackptr, entrypoint);
 	
 	/* enter_new_process does not return. */
 	panic("enter_new_process returned\n");