i#6451 AArch64: Fix large page support (#6868)

PR #5835 inadvertently broke support for large pages on AArch64 by
introducing code that assumed a 4K page size.

The core options unit_tests were also failing on large page systems
because a lot of options need to be overridden and the test was not
accounting for extra options being set in the options string.

Issues: #1680, #6451
Fixes: #6451

core/options.c

@@ -2785,7 +2785,17 @@ unit_test_options(void)
      * We include a smaller option to ensure we avoid printing out "0G".
      */
     get_dynamo_options_string(&dynamo_options, opstring, sizeof(opstring), true);
-    EXPECT_EQ(0, strcmp(opstring, "-vmheap_size 16G -persist_short_digest 8K "));
+#        define EXPECT_OPT(opt, val)                                                     \
+            do {                                                                         \
+                const char *start = strstr(opstring, opt);                               \
+                EXPECT_NE(start, NULL);                                                  \
+                const char expected[] = opt " " val;                                     \
+                EXPECT_STR(start, expected, sizeof(expected) / sizeof(expected[0]) - 1); \
+            } while (0)
+    EXPECT_OPT("-vmheap_size", "16G");
+    EXPECT_OPT("-persist_short_digest", "8K");
+#        undef EXPECT_OPT
+
 #    endif
 
     SELF_PROTECT_OPTIONS();

core/unix/os.c

@@ -4061,12 +4061,6 @@ client_thread_run(void)
     dcontext_t *dcontext;
     byte *xsp;
     GET_STACK_PTR(xsp);
-#    ifdef AARCH64
-    /* AArch64's Scalable Vector Extension (SVE) requires more space on the
-     * stack. Align to page boundary, similar to that in get_clone_record().
-     */
-    xsp = (app_pc)ALIGN_BACKWARD(xsp, PAGE_SIZE);
-#    endif
     void *crec = get_clone_record((reg_t)xsp);
     /* i#2335: we support setup separate from start, and we want to allow a client
      * to create a client thread during init, but we do not support that thread

core/unix/signal.c

@@ -928,26 +928,43 @@ get_clone_record(reg_t xsp)
     /* xsp should be in a dstack, i.e., dynamorio heap.  */
     ASSERT(is_dynamo_address((app_pc)xsp));
 
-    /* The (size of the clone record +
+    /* The stack usage when this is called on most platforms is less than one page so we
+     * can straightforwardly find the clone record by forward aligning xsp.
+     *
+     * However when this function is called by new_thread_setup() on AArch64 the stack
+     * usage is > 4K:
+     *     (size of the clone record +
      *      stack used by new_thread_start (only for setting up priv_mcontext_t) +
      *      stack used by new_thread_setup before calling get_clone_record())
-     * is less than a page for X86 and 2 pages for AArch64. This is verified by
-     * the assert below. If it does exceed 1 page for X86 and 2 for AArch64, it
-     * won't happen at random during runtime, but in a predictable way during
-     * development, which will be caught by the assert.
+     * so if we have 4k pages we need to account for the extra page we are using by
+     * adding PAGE_SIZE to dstack_base.
+     *
+     * When this function is called by client_thread_run() there is no priv_mcontext_t on
+     * the stack so we don't have to make this adjustment.
      *
-     * The current usage is about 800 bytes (X86) or 1920 bytes (AArch64) for
+     * The current usage is about 800 bytes (X86) or 4400 bytes (AArch64) for
      * clone_record + sizeof(priv_mcontext_t) + few words in new_thread_setup
      * before get_clone_record() is called.
      */
+    const size_t page_size = PAGE_SIZE;
+    dstack_base = (byte *)ALIGN_FORWARD(xsp, page_size);
+
+    record = (clone_record_t *)(dstack_base - sizeof(clone_record_t));
 #ifdef AARCH64
-    dstack_base = (byte *)ALIGN_FORWARD(xsp, PAGE_SIZE) + PAGE_SIZE;
-#else
-    dstack_base = (byte *)ALIGN_FORWARD(xsp, PAGE_SIZE);
+    if (sizeof(clone_record_t) + sizeof(priv_mcontext_t) > page_size &&
+        dstack_base != record->dstack) {
+        /* Looks like there is a priv_mcontext_t on the stack so we need to skip forward
+         * another page.
+         */
+        dstack_base += page_size;
+        record = (clone_record_t *)(dstack_base - sizeof(clone_record_t));
+    }
 #endif
-    record = (clone_record_t *)(dstack_base - sizeof(clone_record_t));
 
-    /* dstack_base and the dstack in the clone record should be the same. */
+    /* dstack_base and the dstack in the clone record should be the same.
+     * This verifies the assumptions we have made above about the stack useage fitting
+     * into 1 or 2 pages.
+     */
     ASSERT(dstack_base == record->dstack);
 #ifdef MACOS
     ASSERT(record->app_thread_xsp != 0); /* else it's not in dstack */