i#4717: Fix hang in api.static_prepop test on aarchxx (#4721)

Fixes a bug in the api.static_prepop assembly where the link register is clobbered, leading to an infinite loop on aarchxx. Enables the test for aarchxx. Fixes a bug on identifying stopping points on ARM where LSB Thumb decoration on the two sides of the comparison was not consistent. Fixes a corresponding bug on going native where the LSB was not set. Tested manually on ARM. Issue: #1578, #4717, #4720 Fixes #4717
DynamoRIO · Feb 5, 2021 · 9be64f0 · 9be64f0
1 parent 795c849
commit 9be64f0
Show file tree

Hide file tree

Showing 3 changed files with 38 additions and 11 deletions.
diff --git a/core/dispatch.c b/core/dispatch.c
@@ -248,9 +248,13 @@ d_r_dispatch(dcontext_t *dcontext)
 
 /* returns true if pc is a point at which DynamoRIO should stop interpreting */
 bool
-is_stopping_point(dcontext_t *dcontext, app_pc pc)
+is_stopping_point(dcontext_t *dcontext, app_pc arg_pc)
 {
-    if ((pc == BACK_TO_NATIVE_AFTER_SYSCALL &&
+#ifdef DR_APP_EXPORTS
+    /* TODO i#4720: Find and update other comparisons to function pointers. */
+    app_pc pc = PC_AS_JMP_TGT(dr_get_isa_mode(dcontext), arg_pc);
+#endif
+    if ((arg_pc /*undecorated*/ == BACK_TO_NATIVE_AFTER_SYSCALL &&
          /* case 6253: app may xfer to this "address" in which case pass
           * exception to app
           */
@@ -699,6 +703,7 @@ dispatch_enter_native(dcontext_t *dcontext)
         enter_nolinking(dcontext, NULL, true);
     } else {
 #if defined(DR_APP_EXPORTS) || defined(UNIX)
+        dcontext->next_tag = PC_AS_JMP_TGT(dr_get_isa_mode(dcontext), dcontext->next_tag);
         dispatch_at_stopping_point(dcontext);
         enter_nolinking(dcontext, NULL, false);
 #else
@@ -1482,9 +1487,10 @@ dispatch_exit_fcache_stats(dcontext_t *dcontext)
             STATS_INC(num_bb_exits);
     });
 
-    LOG(THREAD, LOG_DISPATCH, 2, "%s%s",
-        IF_X64_ELSE(FRAG_IS_32(last_f->flags) ? " (32-bit)" : "", ""),
-        TEST(FRAG_SHARED, last_f->flags) ? " (shared)" : "");
+    LOG(THREAD, LOG_DISPATCH, 2, " %s%s",
+        IF_X86_ELSE(IF_X64_ELSE(FRAG_IS_32(last_f->flags) ? "(32-bit)" : "", ""),
+                    IF_ARM_ELSE(FRAG_IS_THUMB(last_f->flags) ? "(T32)" : "(A32)", "")),
+        TEST(FRAG_SHARED, last_f->flags) ? "(shared)" : "");
     DOLOG(2, LOG_SYMBOLS, {
         char symbuf[MAXIMUM_SYMBOL_LENGTH];
         print_symbolic_address(last_f->tag, symbuf, sizeof(symbuf), true);

diff --git a/suite/tests/CMakeLists.txt b/suite/tests/CMakeLists.txt
@@ -2950,10 +2950,8 @@ if (CLIENT_INTERFACE)
     target_link_libraries(api.static_noinit ${libmath})
     tobuild_api(api.static_detach api/static_detach.c "" "" OFF ON)
     target_link_libraries(api.static_detach ${libmath})
-    if (NOT AARCHXX) # TODO i#1578: Crashing on AArch64.
-      tobuild_api(api.static_prepop api/static_prepop.c "" "" OFF ON)
-      target_link_libraries(api.static_prepop ${libmath})
-    endif ()
+    tobuild_api(api.static_prepop api/static_prepop.c "" "" OFF ON)
+    target_link_libraries(api.static_prepop ${libmath})
 
     if (NOT WIN32)
       tobuild_api(api.static_signal api/static_signal.c "" "" OFF ON)

diff --git a/suite/tests/api/static_prepop.c b/suite/tests/api/static_prepop.c
@@ -1,5 +1,5 @@
 /* **********************************************************
- * Copyright (c) 2017-2018 Google, Inc.  All rights reserved.
+ * Copyright (c) 2017-2021 Google, Inc.  All rights reserved.
  * **********************************************************/
 
 /*
@@ -135,7 +135,15 @@ main(int argc, const char *argv[])
         // At that point, the stats should have been reset at reattach.
         if (i == 0)
             assert(stats.basic_block_count == 0);
+#    ifdef ARM
+        /* Our asm is arm, not thumb. */
+        dr_isa_mode_t old_mode;
+        dr_set_isa_mode(dr_get_current_drcontext(), DR_ISA_ARM_A32, &old_mode);
+#    endif
         success = dr_prepopulate_cache(tags, sizeof(tags) / sizeof(tags[0]));
+#    ifdef ARM
+        dr_set_isa_mode(dr_get_current_drcontext(), old_mode, NULL);
+#    endif
         assert(success);
         success =
             dr_prepopulate_indirect_targets(DR_INDIRECT_RETURN, return_tags,
@@ -195,8 +203,23 @@ DECLARE_GLOBAL(asm_return)
         DECLARE_FUNC(FUNCNAME)
 GLOBAL_LABEL(FUNCNAME:)
         INC(ARG1)
+#   ifdef AARCH64
+        stp      x29, x30, [sp, #-16]!
+#   elif defined(ARM)
+        push     {lr}
+#   endif
+#   ifdef ARM
+        /* We don't use CALLC0 b/c it uses blx which swaps to Thumb. */
+        bl       asm_label1
+#   else
         CALLC0(asm_label1)
+#   endif
 ADDRTAKEN_LABEL(asm_return:)
+#   ifdef AARCH64
+        ldp      x29, x30, [sp], #16
+#   elif defined(ARM)
+        pop      {lr}
+#   endif
         JUMP     asm_label2
 
 ADDRTAKEN_LABEL(asm_label1:)
@@ -214,5 +237,5 @@ ADDRTAKEN_LABEL(asm_label3:)
 
 
 END_FILE
-/* clang-format on */
+/* Not turning clang format back on b/c of a clang-format-diff wart. */
 #endif