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

2009-12-30 Thread Daniel Jacobowitz
On Wed, Dec 30, 2009 at 06:32:14PM +0100, Laurent Desnogues wrote:
  @@ -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;
 
 Did you check this works?  I think it doesn't since host_argv
 field is being assigned target_argv, defined in main.  And
 target_argv is freed in main before starting simulation...

Well, that's possible - but that code was there already; I only moved
the CONFIG_USER_ONLY case down a couple of lines.

I don't recall why there's user-mode support in this file.

-- 
Daniel Jacobowitz
CodeSourcery




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

2009-12-30 Thread Daniel Jacobowitz
From: Daniel Jacobowitz d...@codesourcery.com

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 d...@codesourcery.com

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_OPEN0x01
@@ -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  04) {
+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




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

2009-12-30 Thread Jamie Lokier
Daniel Jacobowitz wrote:
 From: Daniel Jacobowitz d...@codesourcery.com
 
 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.

I see that you didn't start the semihosting support, but what's the
purpose of it?  Why would you use it to run programs like cc1 instead
of qemu-arm, the user mode simulation?

Thanks,
-- Jamie




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

2009-12-30 Thread Daniel Jacobowitz
On Wed, Dec 30, 2009 at 06:38:16PM +, Jamie Lokier wrote:
 I see that you didn't start the semihosting support, but what's the
 purpose of it?  Why would you use it to run programs like cc1 instead
 of qemu-arm, the user mode simulation?

You use it to run an arm-none-eabi cc1, not an arm-none-linux-gnueabi
cc1.  In my case that was useful because there was no dynamic memory
allocation.  We also rely on semihosting for GCC testing.

-- 
Daniel Jacobowitz
CodeSourcery