[Qemu-devel] [2 0/2] Linux user for 3.1 patches

[Qemu-devel] [2 0/2] Linux user for 3.1 patches

[Laurent Vivier]

The following changes since commit 77f7c747193662edfadeeb3118d63eed0eac51a6:

  Merge remote-tracking branch 'remotes/huth-gitlab/tags/pull-request-2018-10-17' into staging (2018-10-18 13:40:19 +0100)

are available in the Git repository at:

  git://github.com/vivier/qemu.git tags/linux-user-for-3.1-pull-request

for you to fetch changes up to 6d5bb0b896f9954c98d85c8835a75bb3a041de41:

  linux-user/sparc/signal.c: Remove unnecessary comment (2018-10-18 20:17:49 +0200)

----------------------------------------------------------------
Add a workaround for clang bug and remove misleading comment (sparc)

----------------------------------------------------------------

Peter Maydell (2):
  linux-user: Suppress address-of-packed-member warnings in
    __get/put_user_e
  linux-user/sparc/signal.c: Remove unnecessary comment

 linux-user/qemu.h         | 74 ++++++++++++++++++++++++++++-----------
 linux-user/sparc/signal.c |  4 ---
 2 files changed, 53 insertions(+), 25 deletions(-)

-- 
2.17.2

[Qemu-devel] [2 1/2] linux-user: Suppress address-of-packed-member warnings in __get/put_user_e

[Laurent Vivier]

From: Peter Maydell <peter.maydell@linaro.org>

Our __get_user_e() and __put_user_e() macros cause newer versions
of clang to generate false-positive -Waddress-of-packed-member
warnings if they are passed the address of a member of a packed
struct (see https://bugs.llvm.org/show_bug.cgi?id=39113).
Suppress these using the _Pragma() operator. Unfortunately
_Pragma() support in gcc is broken in some gcc versions and
in some usage contexts, so we limit the pragma usage here to clang.

To put in the pragmas we need to convert the macros from
expressions to statements, but all the callsites effectively
treat them as statements already so this is OK.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Laurent Vivier <laurent@vivier.eu>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-Id: <20181009161814.21257-1-peter.maydell@linaro.org>
Signed-off-by: Laurent Vivier <laurent@vivier.eu>
---
 linux-user/qemu.h | 74 +++++++++++++++++++++++++++++++++--------------
 1 file changed, 53 insertions(+), 21 deletions(-)

diff --git a/linux-user/qemu.h b/linux-user/qemu.h
index b4959e41c6..1beb6a2cfc 100644
--- a/linux-user/qemu.h
+++ b/linux-user/qemu.h
@@ -461,27 +461,59 @@ static inline int access_ok(int type, abi_ulong addr, abi_ulong size)
    These are usually used to access struct data members once the struct has
    been locked - usually with lock_user_struct.  */
 
-/* Tricky points:
-   - Use __builtin_choose_expr to avoid type promotion from ?:,
-   - Invalid sizes result in a compile time error stemming from
-     the fact that abort has no parameters.
-   - It's easier to use the endian-specific unaligned load/store
-     functions than host-endian unaligned load/store plus tswapN.  */
-
-#define __put_user_e(x, hptr, e)                                        \
-  (__builtin_choose_expr(sizeof(*(hptr)) == 1, stb_p,                   \
-   __builtin_choose_expr(sizeof(*(hptr)) == 2, stw_##e##_p,             \
-   __builtin_choose_expr(sizeof(*(hptr)) == 4, stl_##e##_p,             \
-   __builtin_choose_expr(sizeof(*(hptr)) == 8, stq_##e##_p, abort))))   \
-     ((hptr), (x)), (void)0)
-
-#define __get_user_e(x, hptr, e)                                        \
-  ((x) = (typeof(*hptr))(                                               \
-   __builtin_choose_expr(sizeof(*(hptr)) == 1, ldub_p,                  \
-   __builtin_choose_expr(sizeof(*(hptr)) == 2, lduw_##e##_p,            \
-   __builtin_choose_expr(sizeof(*(hptr)) == 4, ldl_##e##_p,             \
-   __builtin_choose_expr(sizeof(*(hptr)) == 8, ldq_##e##_p, abort))))   \
-     (hptr)), (void)0)
+/*
+ * Tricky points:
+ * - Use __builtin_choose_expr to avoid type promotion from ?:,
+ * - Invalid sizes result in a compile time error stemming from
+ *   the fact that abort has no parameters.
+ * - It's easier to use the endian-specific unaligned load/store
+ *   functions than host-endian unaligned load/store plus tswapN.
+ * - The pragmas are necessary only to silence a clang false-positive
+ *   warning: see https://bugs.llvm.org/show_bug.cgi?id=39113 .
+ * - We have to disable -Wpragmas warnings to avoid a complaint about
+ *   an unknown warning type from older compilers that don't know about
+ *   -Waddress-of-packed-member.
+ * - gcc has bugs in its _Pragma() support in some versions, eg
+ *   https://gcc.gnu.org/bugzilla/show_bug.cgi?id=83256 -- so we only
+ *   include the warning-suppression pragmas for clang
+ */
+#ifdef __clang__
+#define PRAGMA_DISABLE_PACKED_WARNING                                   \
+    _Pragma("GCC diagnostic push");                                     \
+    _Pragma("GCC diagnostic ignored \"-Wpragmas\"");                    \
+    _Pragma("GCC diagnostic ignored \"-Waddress-of-packed-member\"")
+
+#define PRAGMA_REENABLE_PACKED_WARNING          \
+    _Pragma("GCC diagnostic pop")
+
+#else
+#define PRAGMA_DISABLE_PACKED_WARNING
+#define PRAGMA_REENABLE_PACKED_WARNING
+#endif
+
+#define __put_user_e(x, hptr, e)                                            \
+    do {                                                                    \
+        PRAGMA_DISABLE_PACKED_WARNING;                                      \
+        (__builtin_choose_expr(sizeof(*(hptr)) == 1, stb_p,                 \
+        __builtin_choose_expr(sizeof(*(hptr)) == 2, stw_##e##_p,            \
+        __builtin_choose_expr(sizeof(*(hptr)) == 4, stl_##e##_p,            \
+        __builtin_choose_expr(sizeof(*(hptr)) == 8, stq_##e##_p, abort))))  \
+            ((hptr), (x)), (void)0);                                        \
+        PRAGMA_REENABLE_PACKED_WARNING;                                     \
+    } while (0)
+
+#define __get_user_e(x, hptr, e)                                            \
+    do {                                                                    \
+        PRAGMA_DISABLE_PACKED_WARNING;                                      \
+        ((x) = (typeof(*hptr))(                                             \
+        __builtin_choose_expr(sizeof(*(hptr)) == 1, ldub_p,                 \
+        __builtin_choose_expr(sizeof(*(hptr)) == 2, lduw_##e##_p,           \
+        __builtin_choose_expr(sizeof(*(hptr)) == 4, ldl_##e##_p,            \
+        __builtin_choose_expr(sizeof(*(hptr)) == 8, ldq_##e##_p, abort))))  \
+            (hptr)), (void)0);                                              \
+        PRAGMA_REENABLE_PACKED_WARNING;                                     \
+    } while (0)
+
 
 #ifdef TARGET_WORDS_BIGENDIAN
 # define __put_user(x, hptr)  __put_user_e(x, hptr, be)
-- 
2.17.2

[Qemu-devel] [2 2/2] linux-user/sparc/signal.c: Remove unnecessary comment

[Laurent Vivier]

From: Peter Maydell <peter.maydell@linaro.org>

Remove a comment suggesting that we need to call tb_flush()
after writing the SPARC signal frame trampoline insns.
This isn't necessary in QEMU, because (even if the guest
architecture requires explicit icache maintenance) we
ensure that memory writes result in invalidation of
translated code from that memory.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Message-Id: <20181009184017.15675-1-peter.maydell@linaro.org>
Signed-off-by: Laurent Vivier <laurent@vivier.eu>
---
 linux-user/sparc/signal.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/linux-user/sparc/signal.c b/linux-user/sparc/signal.c
index b4c60aa446..e44e99993c 100644
--- a/linux-user/sparc/signal.c
+++ b/linux-user/sparc/signal.c
@@ -258,10 +258,6 @@ void setup_frame(int sig, struct target_sigaction *ka,
         __put_user(val32, &sf->insns[1]);
         if (err)
             goto sigsegv;
-
-        /* Flush instruction space. */
-        // flush_sig_insns(current->mm, (unsigned long) &(sf->insns[0]));
-        // tb_flush(env);
     }
     unlock_user(sf, sf_addr, sizeof(struct target_signal_frame));
     return;
-- 
2.17.2

Re: [Qemu-devel] [2 0/2] Linux user for 3.1 patches

[Peter Maydell]

On 18 October 2018 at 20:00, Laurent Vivier <laurent@vivier.eu> wrote:
> The following changes since commit 77f7c747193662edfadeeb3118d63eed0eac51a6:
>
>   Merge remote-tracking branch 'remotes/huth-gitlab/tags/pull-request-2018-10-17' into staging (2018-10-18 13:40:19 +0100)
>
> are available in the Git repository at:
>
>   git://github.com/vivier/qemu.git tags/linux-user-for-3.1-pull-request
>
> for you to fetch changes up to 6d5bb0b896f9954c98d85c8835a75bb3a041de41:
>
>   linux-user/sparc/signal.c: Remove unnecessary comment (2018-10-18 20:17:49 +0200)
>
> ----------------------------------------------------------------
> Add a workaround for clang bug and remove misleading comment (sparc)
>
> ----------------------------------------------------------------
>
> Peter Maydell (2):
>   linux-user: Suppress address-of-packed-member warnings in
>     __get/put_user_e
>   linux-user/sparc/signal.c: Remove unnecessary comment
>
>  linux-user/qemu.h         | 74 ++++++++++++++++++++++++++++-----------
>  linux-user/sparc/signal.c |  4 ---
>  2 files changed, 53 insertions(+), 25 deletions(-)

Applied, thanks.

-- PMM