Re: [Qemu-devel] [PATCH] ARM semihosting improvements

[Daniel Jacobowitz]

From: Daniel Jacobowitz <address@hidden>

This patch improves ARM semihosting to the point where qemu-system-arm
can simulate cc1 from GCC.  It can't simulate GCC itself, which
requires POSIXy bits like execve, but the backend works, including the
preprocessor.

* Use -kernel and -append for SYS_GET_CMDLINE.  This lets semihosted
programs receive command line options.

* Correctly return errno values without gdb attached.  Previously
most system calls discarded errno.

* Set errno == ENOENT after SYS_FLEN of a directory.  This is a
workaround for the absence of stat in the ARM semihosting protocol.
Now stat on directories will report that they do not exist, which
causes most applications to skip the missing directory.

Also fixes a use-after-free when using semihosting with linux-user,
as pointed out by Laurent Desnogues.

Signed-off-by: Daniel Jacobowitz <address@hidden>

diff --git a/arm-semi.c b/arm-semi.c
index 5239ffc..e4d1ae5 100644
--- a/arm-semi.c
+++ b/arm-semi.c
@@ -35,6 +35,7 @@
 #include "qemu-common.h"
 #include "sysemu.h"
 #include "gdbstub.h"
+#include "hw/arm-misc.h"
 #endif
 
 #define SYS_OPEN        0x01
@@ -108,8 +109,12 @@ static inline uint32_t set_swi_errno(TaskState *ts, 
uint32_t code)
     return code;
 }
 #else
+static target_ulong syscall_err;
+
 static inline uint32_t set_swi_errno(CPUState *env, uint32_t code)
 {
+    if (code == (uint32_t)-1)
+        syscall_err = errno;
     return code;
 }
 
@@ -118,10 +123,6 @@ static inline uint32_t set_swi_errno(CPUState *env, 
uint32_t code)
 
 static target_ulong arm_semi_syscall_len;
 
-#if !defined(CONFIG_USER_ONLY)
-static target_ulong syscall_err;
-#endif
-
 static void arm_semi_cb(CPUState *env, target_ulong ret, target_ulong err)
 {
 #ifdef CONFIG_USER_ONLY
@@ -156,8 +157,17 @@ static void arm_semi_flen_cb(CPUState *env, target_ulong 
ret, target_ulong err)
 {
     /* The size is always stored in big-endian order, extract
        the value. We assume the size always fit in 32 bits.  */
-    uint32_t size;
+    uint32_t size, mode;
     cpu_memory_rw_debug(env, env->regs[13]-64+32, (uint8_t *)&size, 4, 0);
+
+    /* Report that all directories do not exist.  */
+    cpu_memory_rw_debug(env, env->regs[13]-64+8, (uint8_t *)&mode, 4, 0);
+    mode = be32_to_cpu(mode);
+    if (mode & 040000) {
+        err = 2;
+        size = -1;
+    }
+
     env->regs[0] = be32_to_cpu(size);
 #ifdef CONFIG_USER_ONLY
     ((TaskState *)env->opaque)->swi_errno = err;
@@ -310,6 +320,11 @@ uint32_t do_arm_semihosting(CPUState *env)
             ret = set_swi_errno(ts, fstat(ARG(0), &buf));
             if (ret == (uint32_t)-1)
                 return -1;
+            if (S_ISDIR (buf.st_mode)) {
+                errno = ENOENT;
+                set_swi_errno(ts, -1);
+                return -1;
+            }
             return buf.st_size;
         }
     case SYS_TMPNAM:
@@ -370,13 +385,21 @@ uint32_t do_arm_semihosting(CPUState *env)
         return syscall_err;
 #endif
     case SYS_GET_CMDLINE:
-#ifdef CONFIG_USER_ONLY
-        /* Build a commandline from the original argv.  */
         {
-            char **arg = ts->info->host_argv;
             int len = ARG(1);
             /* lock the buffer on the ARM side */
             char *cmdline_buffer = (char*)lock_user(VERIFY_WRITE, ARG(0), len, 
0);
+#ifdef CONFIG_USER_ONLY
+            /* Build a commandline from the original argv.  */
+            char **arg = ts->info->host_argv;
+#else
+            /* Build a commandline from -kernel and -append.  */
+            /* This is simple but only because we do no escaping.  */
+            const char *arglist[3] = { 0 }, **arg = arglist;
+
+            arglist[0] = env->boot_info->kernel_filename;
+            arglist[1] = env->boot_info->kernel_cmdline;
+#endif
 
             if (!cmdline_buffer)
                 /* FIXME - should this error code be -TARGET_EFAULT ? */
@@ -410,9 +433,6 @@ uint32_t do_arm_semihosting(CPUState *env)
             /* Return success if commandline fit into buffer.  */
             return *arg ? -1 : 0;
         }
-#else
-      return -1;
-#endif
     case SYS_HEAPINFO:
         {
             uint32_t *ptr;
diff --git a/linux-user/main.c b/linux-user/main.c
index a0d8ce7..fbff44a 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -2782,11 +2782,6 @@ int main(int argc, char **argv, char **envp)
         _exit(1);
     }
 
-    for (i = 0; i < target_argc; i++) {
-        free(target_argv[i]);
-    }
-    free(target_argv);
-
     for (wrk = target_environ; *wrk; wrk++) {
         free(*wrk);
     }

-- 
Daniel Jacobowitz
CodeSourcery