[Qemu-devel] [RFC] Fixes for random Qemu crashes

["J. Mayer" <l_indien@magic.fr>]

[-- Attachment #1: Type: text/plain, Size: 1063 bytes --]

Following the discussion about the inlining problems encountered while
compiling gcc, I tried another set of patches to address those issues.

I first moved all common definitions to osdep.h, changed the
always_inline definition into:
#define always_inline __attribute__ (( always_inline )) __inline__
(as suggested by Ben Pfaff) and added:
#define inline always_inline
It then appeared that translate-op.c did not include osdep.h. As it
seems not great that inlining would not be done in this part of the
code, I did add the include.
This patch also triggered that there are some recursive functions
declared as inline in thunk.h / thunk.c. Defining inline as
always_inline makes gcc emit an error as those functions can obviously
not be inlined.

Here are the 3 diffs, which are not too invasive. Even if gcc still emit
inlining warnings (when setting the -Winline flag), then not inlining
some functions declared as always_inline, this patch seems to allow the
PowerPC 64 target not to crash.

Please comment.

-- 
J. Mayer <l_indien@magic.fr>
Never organized

[-- Attachment #2: always_inline.diff --]
[-- Type: text/x-patch, Size: 3857 bytes --]

Index: exec-all.h
===================================================================
RCS file: /sources/qemu/qemu/exec-all.h,v
retrieving revision 1.70
diff -u -d -d -p -r1.70 exec-all.h
--- exec-all.h	4 Nov 2007 02:24:57 -0000	1.70
+++ exec-all.h	17 Nov 2007 09:37:20 -0000
@@ -21,36 +21,6 @@
 /* allow to see translation results - the slowdown should be negligible, so we leave it */
 #define DEBUG_DISAS
 
-#ifndef glue
-#define xglue(x, y) x ## y
-#define glue(x, y) xglue(x, y)
-#define stringify(s)	tostring(s)
-#define tostring(s)	#s
-#endif
-
-#ifndef likely
-#if __GNUC__ < 3
-#define __builtin_expect(x, n) (x)
-#endif
-
-#define likely(x)   __builtin_expect(!!(x), 1)
-#define unlikely(x)   __builtin_expect(!!(x), 0)
-#endif
-
-#ifndef always_inline
-#if (__GNUC__ < 3) || defined(__APPLE__)
-#define always_inline inline
-#else
-#define always_inline __attribute__ (( always_inline )) inline
-#endif
-#endif
-
-#ifdef __i386__
-#define REGPARM(n) __attribute((regparm(n)))
-#else
-#define REGPARM(n)
-#endif
-
 /* is_jmp field values */
 #define DISAS_NEXT    0 /* next instruction can be analyzed */
 #define DISAS_JUMP    1 /* only pc was modified dynamically */
Index: osdep.h
===================================================================
RCS file: /sources/qemu/qemu/osdep.h,v
retrieving revision 1.10
diff -u -d -d -p -r1.10 osdep.h
--- osdep.h	7 Jun 2007 23:09:47 -0000	1.10
+++ osdep.h	17 Nov 2007 09:37:20 -0000
@@ -3,6 +3,44 @@
 
 #include <stdarg.h>
 
+#ifndef glue
+#define xglue(x, y) x ## y
+#define glue(x, y) xglue(x, y)
+#define stringify(s)	tostring(s)
+#define tostring(s)	#s
+#endif
+
+#ifndef likely
+#if __GNUC__ < 3
+#define __builtin_expect(x, n) (x)
+#endif
+
+#define likely(x)   __builtin_expect(!!(x), 1)
+#define unlikely(x)   __builtin_expect(!!(x), 0)
+#endif
+
+#ifndef MIN
+#define MIN(a, b) (((a) < (b)) ? (a) : (b))
+#endif
+#ifndef MAX
+#define MAX(a, b) (((a) > (b)) ? (a) : (b))
+#endif
+
+#ifndef always_inline
+#if (__GNUC__ < 3) || defined(__APPLE__)
+#define always_inline inline
+#else
+#define always_inline __attribute__ (( always_inline )) __inline__
+#endif
+#endif
+#define inline always_inline
+
+#ifdef __i386__
+#define REGPARM(n) __attribute((regparm(n)))
+#else
+#define REGPARM(n)
+#endif
+
 #define qemu_printf printf
 
 void *qemu_malloc(size_t size);
Index: vl.h
===================================================================
RCS file: /sources/qemu/qemu/vl.h,v
retrieving revision 1.296
diff -u -d -d -p -r1.296 vl.h
--- vl.h	17 Nov 2007 09:04:09 -0000	1.296
+++ vl.h	17 Nov 2007 09:37:20 -0000
@@ -29,37 +29,6 @@
 /* FIXME: Remove this.  */
 #include "block.h"
 
-#ifndef glue
-#define xglue(x, y) x ## y
-#define glue(x, y) xglue(x, y)
-#define stringify(s)	tostring(s)
-#define tostring(s)	#s
-#endif
-
-#ifndef likely
-#if __GNUC__ < 3
-#define __builtin_expect(x, n) (x)
-#endif
-
-#define likely(x)   __builtin_expect(!!(x), 1)
-#define unlikely(x)   __builtin_expect(!!(x), 0)
-#endif
-
-#ifndef MIN
-#define MIN(a, b) (((a) < (b)) ? (a) : (b))
-#endif
-#ifndef MAX
-#define MAX(a, b) (((a) > (b)) ? (a) : (b))
-#endif
-
-#ifndef always_inline
-#if (__GNUC__ < 3) || defined(__APPLE__)
-#define always_inline inline
-#else
-#define always_inline __attribute__ (( always_inline )) inline
-#endif
-#endif
-
 #include "audio/audio.h"
 
 /* vl.c */
Index: darwin-user/qemu.h
===================================================================
RCS file: /sources/qemu/qemu/darwin-user/qemu.h,v
retrieving revision 1.1
diff -u -d -d -p -r1.1 qemu.h
--- darwin-user/qemu.h	18 Jan 2007 20:06:33 -0000	1.1
+++ darwin-user/qemu.h	17 Nov 2007 09:37:20 -0000
@@ -1,13 +1,13 @@
 #ifndef GEMU_H
 #define GEMU_H
 
-#include "thunk.h"
-
 #include <signal.h>
 #include <string.h>
 
 #include "cpu.h"
 
+#include "thunk.h"
+
 #include "gdbstub.h"
 
 typedef siginfo_t target_siginfo_t;

[-- Attachment #3: translate-op.diff --]
[-- Type: text/x-patch, Size: 434 bytes --]

Index: translate-op.c
===================================================================
RCS file: /sources/qemu/qemu/translate-op.c,v
retrieving revision 1.2
diff -u -d -d -p -r1.2 translate-op.c
--- translate-op.c	16 Sep 2007 21:07:51 -0000	1.2
+++ translate-op.c	17 Nov 2007 09:38:06 -0000
@@ -24,6 +24,7 @@
 #include <inttypes.h>
 
 #include "config.h"
+#include "osdep.h"
 
 enum {
 #define DEF(s, n, copy_size) INDEX_op_ ## s,

[-- Attachment #4: thunk.diff --]
[-- Type: text/x-patch, Size: 4502 bytes --]

Index: thunk.c
===================================================================
RCS file: /sources/qemu/qemu/thunk.c,v
retrieving revision 1.10
diff -u -d -d -p -r1.10 thunk.c
--- thunk.c	11 Nov 2007 19:31:34 -0000	1.10
+++ thunk.c	17 Nov 2007 09:38:24 -0000
@@ -31,7 +31,7 @@
 /* XXX: make it dynamic */
 StructEntry struct_entries[MAX_STRUCTS];
 
-static inline const argtype *thunk_type_next(const argtype *type_ptr)
+static const argtype *thunk_type_next(const argtype *type_ptr)
 {
     int type;
 
@@ -267,3 +267,78 @@ unsigned int host_to_target_bitmask(unsi
     }
     return(x86_mask);
 }
+
+#ifndef NO_THUNK_TYPE_SIZE
+int thunk_type_size(const argtype *type_ptr, int is_host)
+{
+    int type, size;
+    const StructEntry *se;
+
+    type = *type_ptr;
+    switch(type) {
+    case TYPE_CHAR:
+        return 1;
+    case TYPE_SHORT:
+        return 2;
+    case TYPE_INT:
+        return 4;
+    case TYPE_LONGLONG:
+    case TYPE_ULONGLONG:
+        return 8;
+    case TYPE_LONG:
+    case TYPE_ULONG:
+    case TYPE_PTRVOID:
+    case TYPE_PTR:
+        if (is_host) {
+            return HOST_LONG_SIZE;
+        } else {
+            return TARGET_ABI_BITS / 8;
+        }
+        break;
+    case TYPE_ARRAY:
+        size = type_ptr[1];
+        return size * thunk_type_size(type_ptr + 2, is_host);
+    case TYPE_STRUCT:
+        se = struct_entries + type_ptr[1];
+        return se->size[is_host];
+    default:
+        return -1;
+    }
+}
+
+int thunk_type_align(const argtype *type_ptr, int is_host)
+{
+    int type;
+    const StructEntry *se;
+
+    type = *type_ptr;
+    switch(type) {
+    case TYPE_CHAR:
+        return 1;
+    case TYPE_SHORT:
+        return 2;
+    case TYPE_INT:
+        return 4;
+    case TYPE_LONGLONG:
+    case TYPE_ULONGLONG:
+        return 8;
+    case TYPE_LONG:
+    case TYPE_ULONG:
+    case TYPE_PTRVOID:
+    case TYPE_PTR:
+        if (is_host) {
+            return HOST_LONG_SIZE;
+        } else {
+            return TARGET_ABI_BITS / 8;
+        }
+        break;
+    case TYPE_ARRAY:
+        return thunk_type_align(type_ptr + 2, is_host);
+    case TYPE_STRUCT:
+        se = struct_entries + type_ptr[1];
+        return se->align[is_host];
+    default:
+        return -1;
+    }
+}
+#endif /* ndef NO_THUNK_TYPE_SIZE */
Index: thunk.h
===================================================================
RCS file: /sources/qemu/qemu/thunk.h,v
retrieving revision 1.15
diff -u -d -d -p -r1.15 thunk.h
--- thunk.h	14 Oct 2007 16:27:28 -0000	1.15
+++ thunk.h	17 Nov 2007 09:38:24 -0000
@@ -75,78 +75,8 @@ const argtype *thunk_convert(void *dst, 
 
 extern StructEntry struct_entries[];
 
-static inline int thunk_type_size(const argtype *type_ptr, int is_host)
-{
-    int type, size;
-    const StructEntry *se;
-
-    type = *type_ptr;
-    switch(type) {
-    case TYPE_CHAR:
-        return 1;
-    case TYPE_SHORT:
-        return 2;
-    case TYPE_INT:
-        return 4;
-    case TYPE_LONGLONG:
-    case TYPE_ULONGLONG:
-        return 8;
-    case TYPE_LONG:
-    case TYPE_ULONG:
-    case TYPE_PTRVOID:
-    case TYPE_PTR:
-        if (is_host) {
-            return HOST_LONG_SIZE;
-        } else {
-            return TARGET_ABI_BITS / 8;
-        }
-        break;
-    case TYPE_ARRAY:
-        size = type_ptr[1];
-        return size * thunk_type_size(type_ptr + 2, is_host);
-    case TYPE_STRUCT:
-        se = struct_entries + type_ptr[1];
-        return se->size[is_host];
-    default:
-        return -1;
-    }
-}
-
-static inline int thunk_type_align(const argtype *type_ptr, int is_host)
-{
-    int type;
-    const StructEntry *se;
-
-    type = *type_ptr;
-    switch(type) {
-    case TYPE_CHAR:
-        return 1;
-    case TYPE_SHORT:
-        return 2;
-    case TYPE_INT:
-        return 4;
-    case TYPE_LONGLONG:
-    case TYPE_ULONGLONG:
-        return 8;
-    case TYPE_LONG:
-    case TYPE_ULONG:
-    case TYPE_PTRVOID:
-    case TYPE_PTR:
-        if (is_host) {
-            return HOST_LONG_SIZE;
-        } else {
-            return TARGET_ABI_BITS / 8;
-        }
-        break;
-    case TYPE_ARRAY:
-        return thunk_type_align(type_ptr + 2, is_host);
-    case TYPE_STRUCT:
-        se = struct_entries + type_ptr[1];
-        return se->align[is_host];
-    default:
-        return -1;
-    }
-}
+int thunk_type_size(const argtype *type_ptr, int is_host);
+int thunk_type_align(const argtype *type_ptr, int is_host);
 
 #endif /* NO_THUNK_TYPE_SIZE */