i#1312 AVX-512 support: AVX-512 compiled client handling, get_xmm_val…

…s() support. (#3751) Compile time detection has been added for whether a client was compiled with AVX-512. In this case, and if DynamoRIO is not deployed via "earliest" inject, we initialize the lazy AVX-512 code detection to true, in order to prevent the client from clobbering potential AVX-512 application state. Adds two new tests to check for the initialized state of AVX-512 lazy code detection. Provides support for zmm context in get_xmm_vals(). Provides support to dump zmm SIMD mcontext state. No tests for this have been added as this is used for log files. Issue: #1312
DynamoRIO · Aug 1, 2019 · d63795e · d63795e
1 parent 3f417e3
commit d63795e
Show file tree

Hide file tree

Showing 11 changed files with 234 additions and 11 deletions.
diff --git a/core/arch/arch.c b/core/arch/arch.c
@@ -87,6 +87,7 @@ static bool sysenter_hook_failed = false;
 
 #ifdef X86
 bool *d_r_avx512_code_in_use = NULL;
+bool d_r_client_avx512_code_in_use = false;
 #endif
 
 /* static functions forward references */
@@ -3660,7 +3661,12 @@ dump_mcontext(priv_mcontext_t *context, file_t f, bool dump_xml)
     if (preserve_xmm_caller_saved()) {
         int i, j;
         for (i = 0; i < proc_num_simd_saved(); i++) {
-            if (YMM_ENABLED()) {
+            if (ZMM_ENABLED()) {
+                print_file(f, dump_xml ? "\t\tzmm%d= \"0x" : "\tzmm%d= 0x", i);
+                for (j = 0; j < 16; j++) {
+                    print_file(f, "%08x", context->simd[i].u32[j]);
+                }
+            } else if (YMM_ENABLED()) {
                 print_file(f, dump_xml ? "\t\tymm%d= \"0x" : "\tymm%d= 0x", i);
                 for (j = 0; j < 8; j++) {
                     print_file(f, "%08x", context->simd[i].u32[j]);

diff --git a/core/arch/arch.h b/core/arch/arch.h
@@ -235,6 +235,12 @@ preserve_xmm_caller_saved(void)
  */
 extern bool *d_r_avx512_code_in_use;
 
+/* This flag indicates a client that had been compiled with AVX-512. In all other than
+ * "earliest" inject methods, the initial value of d_r_is_avx512_code_in_use() will be
+ * set to true, to prevent a client from clobbering potential application state.
+ */
+extern bool d_r_client_avx512_code_in_use;
+
 /* This routine determines whether zmm registers should be saved. */
 static inline bool
 d_r_is_avx512_code_in_use()
@@ -250,6 +256,19 @@ d_r_set_avx512_code_in_use(bool in_use)
     SELF_PROTECT_DATASEC(DATASEC_RARELY_PROT);
 }
 
+static inline bool
+d_r_is_client_avx512_code_in_use()
+{
+    return d_r_client_avx512_code_in_use;
+}
+
+static inline void
+d_r_set_client_avx512_code_in_use()
+{
+    SELF_UNPROTECT_DATASEC(DATASEC_RARELY_PROT);
+    ATOMIC_1BYTE_WRITE(&d_r_client_avx512_code_in_use, (bool)true, false);
+    SELF_PROTECT_DATASEC(DATASEC_RARELY_PROT);
+}
 #endif
 
 typedef enum {
@@ -1311,7 +1330,7 @@ new_bsdthread_setup(priv_mcontext_t *mc);
 #endif
 
 void
-get_xmm_vals(priv_mcontext_t *mc);
+get_simd_vals(priv_mcontext_t *mc);
 
 /* i#350: Fast safe_read without dcontext.  On success or failure, returns the
  * current source pointer.  Requires fault handling to be set up.

diff --git a/core/arch/x86/x86.asm b/core/arch/x86/x86.asm
@@ -121,7 +121,7 @@ START_FILE
 /* Pushes a priv_mcontext_t on the stack, with an xsp value equal to the
  * xsp before the pushing.  Clobbers xax!
  * Does fill in xmm0-5, if necessary, for PR 264138.
- * Assumes that DR has been initialized (get_xmm_vals() checks proc feature bits).
+ * Assumes that DR has been initialized (get_simd_vals() checks proc feature bits).
  * Caller should ensure 16-byte stack alignment prior to the push (PR 306421).
  */
 #define PUSH_PRIV_MCXT(pc)                              \
@@ -130,7 +130,7 @@ START_FILE
         PUSHF                                          @N@\
         PUSHGPR                                        @N@\
         lea      REG_XAX, [REG_XSP]                    @N@\
-        CALLC1(GLOBAL_REF(get_xmm_vals), REG_XAX)      @N@\
+        CALLC1(GLOBAL_REF(get_simd_vals), REG_XAX)      @N@\
         lea      REG_XAX, [PRIV_MCXT_SIZE + REG_XSP]   @N@\
         mov      [PUSHGPR_XSP_OFFS + REG_XSP], REG_XAX
 
@@ -148,7 +148,7 @@ START_FILE
 DECL_EXTERN(unexpected_return)
 
 DECL_EXTERN(get_own_context_integer_control)
-DECL_EXTERN(get_xmm_vals)
+DECL_EXTERN(get_simd_vals)
 DECL_EXTERN(auto_setup)
 DECL_EXTERN(return_from_native)
 DECL_EXTERN(native_module_callout)

diff --git a/core/arch/x86_code.c b/core/arch/x86_code.c
@@ -48,15 +48,19 @@
  * (or all for linux) (or ymm) only if necessary.
  */
 void
-get_xmm_vals(priv_mcontext_t *mc)
+get_simd_vals(priv_mcontext_t *mc)
 {
 #ifdef X86
     if (preserve_xmm_caller_saved()) {
         ASSERT(proc_has_feature(FEATURE_SSE));
-        if (YMM_ENABLED())
+        if (d_r_is_avx512_code_in_use()) {
+            get_zmm_caller_saved(&mc->simd[0]);
+            get_opmask_caller_saved(&mc->opmask[0]);
+        } else if (YMM_ENABLED()) {
             get_ymm_caller_saved(&mc->simd[0]);
-        else
+        } else {
             get_xmm_caller_saved(&mc->simd[0]);
+        }
     }
 #elif defined(ARM)
     /* FIXME i#1551: no xmm but SIMD regs on ARM */

diff --git a/core/lib/dr_api.h b/core/lib/dr_api.h
@@ -123,6 +123,20 @@ DR_EXPORT LINK_ONCE int _USES_DR_VERSION_ = ${VERSION_NUMBER_INTEGER};
 /* clang-format on */
 #endif
 
+/* AVX-512 client code detection. Compiling the client with AVX-512 will cause
+ * DynamoRIO to assume that AVX-512 code is in use when deploying DynamoRIO after
+ * the application has started.
+ */
+#ifndef DYNAMORIO_STANDALONE
+#    ifdef __AVX512F__
+DR_EXPORT LINK_ONCE bool _DR_CLIENT_AVX512_CODE_IN_USE_ = true;
+#    else
+DR_EXPORT LINK_ONCE bool _DR_CLIENT_AVX512_CODE_IN_USE_ = false;
+#    endif
+#else
+#    define _DR_CLIENT_AVX512_CODE_IN_USE_ false
+#endif
+
 /* A flag that can be used to identify whether this file was included */
 #define DYNAMORIO_API
 

diff --git a/core/lib/instrument.c b/core/lib/instrument.c
@@ -112,6 +112,9 @@ do_file_write(file_t f, const char *fmt, va_list ap);
  */
 DR_API const char *unique_build_number = STRINGIFY(UNIQUE_BUILD_NUMBER);
 
+/* The flag d_r_client_avx512_code_in_use is described in arch.h. */
+#    define DR_CLIENT_AVX512_CODE_IN_USE_NAME "_DR_CLIENT_AVX512_CODE_IN_USE_"
+
 /* Acquire when registering or unregistering event callbacks
  * Also held when invoking events, which happens much more often
  * than registration changes, so we use rwlock
@@ -619,6 +622,14 @@ add_client_lib(const char *path, const char *id_str, const char *options)
                         BUFFER_SIZE_ELEMENTS(client_libs[idx].options));
                 NULL_TERMINATE_BUFFER(client_libs[idx].options);
             }
+#    ifdef X86
+            bool *client_avx512_code_in_use = (bool *)lookup_library_routine(
+                client_lib, DR_CLIENT_AVX512_CODE_IN_USE_NAME);
+            if (client_avx512_code_in_use != NULL) {
+                if (*client_avx512_code_in_use)
+                    d_r_set_client_avx512_code_in_use();
+            }
+#    endif
 
             /* We'll look up dr_client_main and call it in instrument_init */
         }
@@ -694,6 +705,16 @@ instrument_init(void)
 
     init_client_aux_libs();
 
+#    ifdef X86
+    if (IF_WINDOWS_ELSE(!dr_earliest_injected, !DYNAMO_OPTION(early_inject))) {
+        /* A client that had been compiled with AVX-512 may clobber an application's
+         * state. AVX-512 context switching will not be lazy in this case.
+         */
+        if (d_r_is_client_avx512_code_in_use())
+            d_r_set_avx512_code_in_use(true);
+    }
+#    endif
+
     if (num_client_libs > 0) {
         /* We no longer distinguish in-DR vs in-client crashes, as many crashes in
          * the DR lib are really client bugs.

diff --git a/suite/tests/CMakeLists.txt b/suite/tests/CMakeLists.txt
@@ -2585,12 +2585,33 @@ endif ()
     tobuild_ci(client.avx512lazy client-interface/avx512lazy.c "" "" "")
     set_target_properties(client.avx512lazy PROPERTIES COMPILE_FLAGS "${CFLAGS_AVX512}")
 
+    # XXX i#1312: This is untested on Windows.
+    tobuild_ci(client.avx512lazy-initial client-interface/avx512lazy.c "" "" "")
+    set_target_properties(client.avx512lazy-initial PROPERTIES COMPILE_FLAGS
+      "${CFLAGS_AVX512}")
+    append_property_string(TARGET client.avx512lazy-initial.dll COMPILE_FLAGS
+      "${CFLAGS_AVX512} -DCLIENT_COMPILED_WITH_AVX512")
+
     if (UNIX)
       # XXX i#1312: we do not yet support AVX-512 Windows context switching.
       tobuild_ci(client.avx512ctx client-interface/avx512ctx.c "" "" "")
       set_target_properties(client.avx512ctx PROPERTIES COMPILE_FLAGS "${CFLAGS_AVX512}")
       target_include_directories(client.avx512ctx PRIVATE
         ${CMAKE_CURRENT_SOURCE_DIR}/client-interface)
+
+      # This could be generalized in a new function. Currently this is the only api-style
+      # test that also has a client library.
+      add_library(api.startstop_avx512lazy.dll SHARED api/startstop_avx512lazy.dll.c)
+      setup_test_client_dll_basics(api.startstop_avx512lazy.dll)
+      append_property_string(TARGET api.startstop_avx512lazy.dll COMPILE_FLAGS
+        "${CFLAGS_AVX512}")
+      get_client_path(client_path api.startstop_avx512lazy.dll api.startstop_avx512lazy)
+      tobuild_api(api.startstop_avx512lazy api/startstop_avx512lazy.c
+        "-client_lib ${client_path}" "" OFF OFF)
+      append_property_string(TARGET api.startstop_avx512lazy COMPILE_FLAGS
+        "${CFLAGS_AVX512}")
+      append_property_string(TARGET api.startstop_avx512lazy.dll COMPILE_FLAGS
+        "${CFLAGS_AVX512}")
     endif (UNIX)
   endif ()
 

diff --git a/suite/tests/api/startstop_avx512lazy.c b/suite/tests/api/startstop_avx512lazy.c
@@ -0,0 +1,66 @@
+/* **********************************************************
+ * Copyright (c) 2019 Google, Inc.  All rights reserved.
+ * **********************************************************/
+
+/*
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are met:
+ *
+ * * Redistributions of source code must retain the above copyright notice,
+ *   this list of conditions and the following disclaimer.
+ *
+ * * Redistributions in binary form must reproduce the above copyright notice,
+ *   this list of conditions and the following disclaimer in the documentation
+ *   and/or other materials provided with the distribution.
+ *
+ * * Neither the name of VMware, Inc. nor the names of its contributors may be
+ *   used to endorse or promote products derived from this software without
+ *   specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
+ * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED. IN NO EVENT SHALL VMWARE, INC. OR CONTRIBUTORS BE LIABLE
+ * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
+ * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR
+ * SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
+ * CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
+ * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
+ * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH
+ * DAMAGE.
+ */
+
+#include <assert.h>
+#include <stdio.h>
+#include <math.h>
+#include <stdint.h>
+#include "configure.h"
+#include "dr_api.h"
+#include "tools.h"
+
+int
+main(void)
+{
+    print("Initializing DynamoRIO\n");
+
+    /* Initialize DR */
+    dr_app_setup();
+
+    print("Starting DynamoRIO\n");
+
+    dr_app_start();
+
+    if (!dr_app_running_under_dynamorio())
+        print("ERROR: should be under DynamoRIO after dr_app_start!\n");
+
+    dr_app_stop();
+
+    if (dr_app_running_under_dynamorio())
+        print("ERROR: should not be under DynamoRIO after dr_app_stop!\n");
+
+    dr_app_cleanup();
+
+    print("Ok\n");
+
+    return 0;
+}
diff --git a/suite/tests/api/startstop_avx512lazy.dll.c b/suite/tests/api/startstop_avx512lazy.dll.c
@@ -0,0 +1,51 @@
+/* **********************************************************
+ * Copyright (c) 2019 Google, Inc.  All rights reserved.
+ * **********************************************************/
+
+/*
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are met:
+ *
+ * * Redistributions of source code must retain the above copyright notice,
+ *   this list of conditions and the following disclaimer.
+ *
+ * * Redistributions in binary form must reproduce the above copyright notice,
+ *   this list of conditions and the following disclaimer in the documentation
+ *   and/or other materials provided with the distribution.
+ *
+ * * Neither the name of Google, Inc. nor the names of its contributors may be
+ *   used to endorse or promote products derived from this software without
+ *   specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
+ * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED. IN NO EVENT SHALL GOOGLE, INC. OR CONTRIBUTORS BE LIABLE
+ * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
+ * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR
+ * SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
+ * CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
+ * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
+ * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH
+ * DAMAGE.
+ */
+
+#include "dr_api.h"
+#include "client_tools.h"
+#include <string.h>
+
+#define CHECK(x, msg)                                                                \
+    do {                                                                             \
+        if (!(x)) {                                                                  \
+            dr_fprintf(STDERR, "CHECK failed %s:%d: %s\n", __FILE__, __LINE__, msg); \
+            dr_abort();                                                              \
+        }                                                                            \
+    } while (0);
+
+DR_EXPORT void
+dr_init(client_id_t id)
+{
+    dr_fprintf(STDERR, "Initializing client\n");
+    CHECK(dr_mcontext_zmm_fields_valid(),
+          "Error: dr_mcontext_zmm_fields_valid() should return true.");
+}
diff --git a/suite/tests/api/startstop_avx512lazy.templatex b/suite/tests/api/startstop_avx512lazy.templatex
@@ -0,0 +1,5 @@
+(<Application .*client\.avx512ctx.*AVX-512 was detected at PC 0x[0-9a-f]+. AVX-512 is not fully supported yet.>
+)?Initializing DynamoRIO
+Initializing client
+Starting DynamoRIO
+Ok
diff --git a/suite/tests/client-interface/avx512lazy.dll.c b/suite/tests/client-interface/avx512lazy.dll.c
@@ -34,6 +34,15 @@
 #include "client_tools.h"
 #include <string.h>
 
+#ifdef CLIENT_COMPILED_WITH_AVX512
+/* This just double-checks the implicit __AVX512__ flag if the client was compiled
+ * with AVX-512.
+ */
+#    ifndef __AVX512F__
+#        error "Compiler provided __AVX512F__ define not set"
+#    endif
+#endif
+
 #define CHECK(x, msg)                                                                \
     do {                                                                             \
         if (!(x)) {                                                                  \
@@ -43,7 +52,7 @@
     } while (0);
 
 /* This library assumes a single-threaded test. */
-static bool before_seen;
+static bool seen_before;
 
 static dr_emit_flags_t
 bb_event(void *drcontext, void *tag, instrlist_t *bb, bool for_trace, bool translating)
@@ -59,16 +68,23 @@ bb_event(void *drcontext, void *tag, instrlist_t *bb, bool for_trace, bool trans
                 val1 != 0 && /* rule out xor w/ self */
                 opnd_is_reg(instr_get_dst(instr, 0)) &&
                 opnd_get_reg(instr_get_dst(instr, 0)) == REG_XAX) {
-                if (before_seen) {
+                if (seen_before) {
                     CHECK(dr_mcontext_zmm_fields_valid(),
                           "Error: dr_mcontext_zmm_fields_valid() should return true.");
                     dr_fprintf(STDERR, "After\n");
                 } else {
+                    /* The *-initial version of this test runs the client compiled with
+                     * AVX-512. Even in this case, the initial value of
+                     * dr_mcontext_zmm_fields_valid() is expected to be false. The only
+                     * time it should be true before application AVX-512 code has actually
+                     * been seen is the "attach" case. This is tested by
+                     * api.startstop_avx512lazy.
+                     */
                     CHECK(!dr_mcontext_zmm_fields_valid(),
                           "Error: dr_mcontext_zmm_fields_valid() should return false.");
                     dr_fprintf(STDERR, "Before\n");
                 }
-                before_seen = true;
+                seen_before = true;
             } else
                 prev_was_mov_const = true;
         } else