Patchwork [4/6] linux-user: Rewrite __get_user/__put_user with __builtin_choose_expr

login
register
mail settings
Submitter Richard Henderson
Date Oct. 11, 2012, 7:22 p.m.
Message ID <1349983336-9974-5-git-send-email-rth@twiddle.net>
Download mbox | patch
Permalink /patch/190988/
State New
Headers show

Comments

Richard Henderson - Oct. 11, 2012, 7:22 p.m.
The previous formuation with multiple assignments to __typeof(*hptr) falls
down when hptr is qualified const.  E.g. with const struct S *p, p->f is
also qualified const.

With this formulation, there's no assignment to any local variable.

Signed-off-by: Richard Henderson <rth@twiddle.net>
---
 linux-user/qemu.h | 50 ++++++++++++++++++++------------------------------
 1 file changed, 20 insertions(+), 30 deletions(-)
Riku Voipio - Oct. 12, 2012, 11:10 a.m.
Hi,

On Thu, Oct 11, 2012 at 12:52:27PM -0700, Richard Henderson wrote:
> On 10/11/2012 12:22 PM, Richard Henderson wrote:
> > The previous formuation with multiple assignments to __typeof(*hptr) falls
> > down when hptr is qualified const.  E.g. with const struct S *p, p->f is
> > also qualified const.
> > 
> > With this formulation, there's no assignment to any local variable.
> > 
> > Signed-off-by: Richard Henderson <rth@twiddle.net>
 
> Scratch that... I lost the unaligned access capability.
> I'll have to re-work the patch.

I've just prepared the rest of your patch for pending pull request[1].
Since changing __get_user/__put_user impacts more than just sigaction,
do you mind if we put this patch (and thus sigaction change as well)
to a later pull request?

Riku

[1] http://git.linaro.org/gitweb?p=people/rikuvoipio/qemu.git;a=shortlog;h=refs/heads/linux-user-for-upstream
Richard Henderson - Oct. 12, 2012, 2:43 p.m.
On 10/12/2012 04:10 AM, Riku Voipio wrote:
> Since changing __get_user/__put_user impacts more than just sigaction,
> do you mind if we put this patch (and thus sigaction change as well)
> to a later pull request?

Certainly.


r~

Patch

diff --git a/linux-user/qemu.h b/linux-user/qemu.h
index fc4cc00..39b2c9c 100644
--- a/linux-user/qemu.h
+++ b/linux-user/qemu.h
@@ -284,36 +284,26 @@  static inline int access_ok(int type, abi_ulong addr, abi_ulong size)
                             (type == VERIFY_READ) ? PAGE_READ : (PAGE_READ | PAGE_WRITE)) == 0;
 }
 
-/* NOTE __get_user and __put_user use host pointers and don't check access. */
-/* These are usually used to access struct data members once the
- * struct has been locked - usually with lock_user_struct().
- */
-#define __put_user(x, hptr)\
-({ __typeof(*hptr) pu_ = (x);\
-    switch(sizeof(*hptr)) {\
-    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: break; \
-    case 2: gu_ = tswap16(gu_); break; \
-    case 4: gu_ = tswap32(gu_); break; \
-    case 8: gu_ = tswap64(gu_); break; \
-    default: abort();\
-    }\
-    (x) = gu_; \
-    0;\
-})
+/* NOTE __get_user and __put_user use host pointers and don't check access.
+   These are usually used to access struct data members once the struct has
+   been locked - usually with lock_user_struct.  */
+#define __put_user(x, hptr)                                          \
+(*(hptr) =                                                           \
+ __builtin_choose_expr(sizeof(*(hptr)) == 1, (uint8_t)(x),           \
+ __builtin_choose_expr(sizeof(*(hptr)) == 2, tswap16((uint16_t)(x)), \
+ __builtin_choose_expr(sizeof(*(hptr)) == 4, tswap32((uint32_t)(x)), \
+ __builtin_choose_expr(sizeof(*(hptr)) == 8, tswap64((uint64_t)(x)), \
+                       abort())))),                                  \
+ 0)
+
+#define __get_user(x, hptr)                                     \
+((x) =                                                          \
+ __builtin_choose_expr(sizeof(*(hptr)) == 1, *(hptr),           \
+ __builtin_choose_expr(sizeof(*(hptr)) == 2, tswap16(*(hptr)),  \
+ __builtin_choose_expr(sizeof(*(hptr)) == 4, tswap32(*(hptr)),  \
+ __builtin_choose_expr(sizeof(*(hptr)) == 8, tswap64(*(hptr)),  \
+                       abort())))),                             \
+ 0)
 
 /* put_user()/get_user() take a guest address and check access */
 /* These are usually used to access an atomic data type, such as an int,