diff mbox

[01/13] linux-user: Use memcpy in get_user/put_user.

Message ID 1347895732-22212-2-git-send-email-rth@twiddle.net
State New
Headers show

Commit Message

Richard Henderson Sept. 17, 2012, 3:28 p.m. UTC
Not sure whether this is a gcc bug, but on sparc we wind up
trying to use std to an unaligned address.  The patch does
work around the problem.

Signed-off-by: Richard Henderson <rth@twiddle.net>
---
 linux-user/qemu.h | 47 +++++++++++++++--------------------------------
 1 file changed, 15 insertions(+), 32 deletions(-)

Comments

Peter Maydell Sept. 17, 2012, 3:35 p.m. UTC | #1
On 17 September 2012 16:28, Richard Henderson <rth@twiddle.net> wrote:
> Not sure whether this is a gcc bug, but on sparc we wind up
> trying to use std to an unaligned address.  The patch does
> work around the problem.

Hmm. I guess this is guest and host having differing alignment
requirements for 64 bit types? What case does it fail on?

-- PMM
Richard Henderson Sept. 17, 2012, 3:40 p.m. UTC | #2
On 09/17/2012 08:35 AM, Peter Maydell wrote:
> On 17 September 2012 16:28, Richard Henderson <rth@twiddle.net> wrote:
>> Not sure whether this is a gcc bug, but on sparc we wind up
>> trying to use std to an unaligned address.  The patch does
>> work around the problem.
> 
> Hmm. I guess this is guest and host having differing alignment
> requirements for 64 bit types? What case does it fail on?

I'll have to re-check.  I'm pretty sure this was sparc32 hosting arm-linux-user.


r~
Peter Maydell Sept. 17, 2012, 4:06 p.m. UTC | #3
On 17 September 2012 16:28, Richard Henderson <rth@twiddle.net> wrote:
> Not sure whether this is a gcc bug, but on sparc we wind up
> trying to use std to an unaligned address.  The patch does
> work around the problem.
>
> Signed-off-by: Richard Henderson <rth@twiddle.net>

As far as the code is concerned:
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

but please rewrite the commit message to be more sure of itself :-)

Specifically, this is required because the guest's alignment
requirements may be laxer than those of the host, so we can't
take a hptr for a 16/32/64 bit type (which only has the alignment
restrictions of the guest) and do a gcc pointer dereference on it
(which may assume the alignment restrictions of the host).

Does anybody know the semantics of the kernel's copy_from_user()?
Does it fail on misaligned userspace structs or does it allow
them?

-- PMM
diff mbox

Patch

diff --git a/linux-user/qemu.h b/linux-user/qemu.h
index 69b27d7..fc4cc00 100644
--- a/linux-user/qemu.h
+++ b/linux-user/qemu.h
@@ -289,46 +289,29 @@  static inline int access_ok(int type, abi_ulong addr, abi_ulong size)
  * struct has been locked - usually with lock_user_struct().
  */
 #define __put_user(x, hptr)\
-({\
+({ __typeof(*hptr) pu_ = (x);\
     switch(sizeof(*hptr)) {\
-    case 1:\
-        *(uint8_t *)(hptr) = (uint8_t)(typeof(*hptr))(x);\
-        break;\
-    case 2:\
-        *(uint16_t *)(hptr) = tswap16((uint16_t)(typeof(*hptr))(x));\
-        break;\
-    case 4:\
-        *(uint32_t *)(hptr) = tswap32((uint32_t)(typeof(*hptr))(x));\
-        break;\
-    case 8:\
-        *(uint64_t *)(hptr) = tswap64((typeof(*hptr))(x));\
-        break;\
-    default:\
-        abort();\
+    case 1: break;\
+    case 2: pu_ = tswap16(pu_); break; \
+    case 4: pu_ = tswap32(pu_); break; \
+    case 8: pu_ = tswap64(pu_); break; \
+    default: abort();\
     }\
+    memcpy(hptr, &pu_, sizeof(pu_)); \
     0;\
 })
 
 #define __get_user(x, hptr) \
-({\
+({ __typeof(*hptr) gu_; \
+    memcpy(&gu_, hptr, sizeof(gu_)); \
     switch(sizeof(*hptr)) {\
-    case 1:\
-        x = (typeof(*hptr))*(uint8_t *)(hptr);\
-        break;\
-    case 2:\
-        x = (typeof(*hptr))tswap16(*(uint16_t *)(hptr));\
-        break;\
-    case 4:\
-        x = (typeof(*hptr))tswap32(*(uint32_t *)(hptr));\
-        break;\
-    case 8:\
-        x = (typeof(*hptr))tswap64(*(uint64_t *)(hptr));\
-        break;\
-    default:\
-        /* avoid warning */\
-        x = 0;\
-        abort();\
+    case 1: break; \
+    case 2: gu_ = tswap16(gu_); break; \
+    case 4: gu_ = tswap32(gu_); break; \
+    case 8: gu_ = tswap64(gu_); break; \
+    default: abort();\
     }\
+    (x) = gu_; \
     0;\
 })