[PPC64, Sanitizers] Proper stack frame for the thread spawned in internal_clone

[alekseyshl]

Summary

Set up the proper stack frame for the thread spawned in internal_clone,
the current code does not follow ABI (and causes SEGV trying to use this
malformed frame).

[echristo]

Couple of inline comments. I think this looks right but I've added Kit and Nemanja on here so one of them can give an ACK if Bill doesn't.

-eric

lib/sanitizer_common/sanitizer_linux.cc

1117 ↗

(On Diff #93725)

Do you mean elfv1 vs elfv2 here? The standard one would be just to check for _CALL_ELF == 1 and _CALL_ELF == 2.

1137 ↗

(On Diff #93725)

If we're just talking about power64 we can probably just hard code it to N rather than N * sizeof(some type).

[alekseyshl]

• Address comments

lib/sanitizer_common/sanitizer_linux.cc

1117 ↗

(On Diff #93725)

That's common practice in sanitizers, sanitizer_platform.h defines SANITIZER_... symbols and those get to be used everywhere. SANITIZER_PPC64V1/2 defined in terms of _CALL_ELF

1137 ↗

(On Diff #93725)

Makes sense

[alekseyshl]

Ping

[nemanjai]

I can't claim I really understand this sanitizer code, but won't this depend on the decision made as part of this code:
https://reviews.llvm.org/D29881
https://reviews.llvm.org/rL296771

Should we make a modification that always creates a parameter save area for sanitizers? Or is this completely orthogonal?

[alekseyshl]

• Address comments
• Drop optional parameter save area on PPC64 ELFv2.

[alekseyshl]

No, it's not a sanitizer specific requirement, I was trying to be generic, unnecessary so, I'd say. Thank you for pointing to those revisions, let's be consistent and drop the parameter save area for ELFv2.

[alekseyshl]

Ping

[alekseyshl]

Ping

[hfinkel]

LGTM

compiler-rt/trunk/lib/sanitizer_common/sanitizer_linux.cc

__asm__ __volatile__(
                          "i"(__NR_clone), "i"(__NR_exit)
                        : "x30", "memory");
   return res;
 }
 #elif defined(__powerpc64__)
 uptr internal_clone(int (*fn)(void *), void *child_stack, int flags, void *arg,
                    int *parent_tidptr, void *newtls, int *child_tidptr) {
   long long res;
-/* Stack frame offsets.  */
-#if _CALL_ELF != 2
-#define FRAME_MIN_SIZE         112
-#define FRAME_TOC_SAVE         40
+// Stack frame structure.
+#if SANITIZER_PPC64V1
+//   Back chain == 0        (SP + 112)
+// Frame (112 bytes):
+//   Parameter save area    (SP + 48), 8 doublewords
+//   TOC save area          (SP + 40)
+//   Link editor doubleword (SP + 32)
+//   Compiler doubleword    (SP + 24)
+//   LR save area           (SP + 16)
+//   CR save area           (SP + 8)
+//   Back chain             (SP + 0)
+# define FRAME_SIZE 112
+# define FRAME_TOC_SAVE_OFFSET 40
+#elif SANITIZER_PPC64V2
+//   Back chain == 0        (SP + 32)
+// Frame (32 bytes):
+//   TOC save area          (SP + 24)
+//   LR save area           (SP + 16)
+//   CR save area           (SP + 8)
+//   Back chain             (SP + 0)
+# define FRAME_SIZE 32
+# define FRAME_TOC_SAVE_OFFSET 24
 #else
-#define FRAME_MIN_SIZE         32
-#define FRAME_TOC_SAVE         24
+# error "Unsupported PPC64 ABI"
 #endif
   if (!fn || !child_stack)
     return -EINVAL;
   CHECK_EQ(0, (uptr)child_stack % 16);
-  child_stack = (char *)child_stack - 2 * sizeof(unsigned long long);
-  ((unsigned long long *)child_stack)[0] = (uptr)fn;
-  ((unsigned long long *)child_stack)[1] = (uptr)arg;
 
   register int (*__fn)(void *) __asm__("r3") = fn;
   register void *__cstack      __asm__("r4") = child_stack;
   register int __flags         __asm__("r5") = flags;
-  register void * __arg        __asm__("r6") = arg;
-  register int * __ptidptr     __asm__("r7") = parent_tidptr;
-  register void * __newtls     __asm__("r8") = newtls;
-  register int * __ctidptr     __asm__("r9") = child_tidptr;
+  register void *__arg         __asm__("r6") = arg;
+  register int *__ptidptr      __asm__("r7") = parent_tidptr;
+  register void *__newtls      __asm__("r8") = newtls;
+  register int *__ctidptr      __asm__("r9") = child_tidptr;
 
  __asm__ __volatile__(
-           /* fn, arg, child_stack are saved acrVoss the syscall */
+           /* fn and arg are saved across the syscall */
            "mr 28, %5\n\t"
-           "mr 29, %6\n\t"
            "mr 27, %8\n\t"
 
            /* syscall
+             r0 == __NR_clone
              r3 == flags
              r4 == child_stack
              r5 == parent_tidptr
              r6 == newtls
              r7 == child_tidptr */
            "mr 3, %7\n\t"
            "mr 5, %9\n\t"
            "mr 6, %10\n\t"
            "mr 7, %11\n\t"
            "li 0, %3\n\t"
            "sc\n\t"
 
            /* Test if syscall was successful */
            "cmpdi  cr1, 3, 0\n\t"
            "crandc cr1*4+eq, cr1*4+eq, cr0*4+so\n\t"
            "bne-   cr1, 1f\n\t"
 
+           /* Set up stack frame */
+           "li    29, 0\n\t"
+           "stdu  29, -8(1)\n\t"
+           "stdu  1, -%12(1)\n\t"
            /* Do the function call */
            "std   2, %13(1)\n\t"
-#if _CALL_ELF != 2
+#if SANITIZER_PPC64V1
            "ld    0, 0(28)\n\t"
            "ld    2, 8(28)\n\t"
            "mtctr 0\n\t"
-#else
+#elif SANITIZER_PPC64V2
            "mr    12, 28\n\t"
            "mtctr 12\n\t"
+#else
+# error "Unsupported PPC64 ABI"
 #endif
            "mr    3, 27\n\t"
            "bctrl\n\t"
            "ld    2, %13(1)\n\t"
 
            /* Call _exit(r3) */
            "li 0, %4\n\t"
            "sc\n\t"
 
            /* Return to parent */
            "1:\n\t"
            "mr %0, 3\n\t"
              : "=r" (res)
-             : "0" (-1), "i" (EINVAL),
-               "i" (__NR_clone), "i" (__NR_exit),
-               "r" (__fn), "r" (__cstack), "r" (__flags),
-               "r" (__arg), "r" (__ptidptr), "r" (__newtls),
-               "r" (__ctidptr), "i" (FRAME_MIN_SIZE), "i" (FRAME_TOC_SAVE)
-             : "cr0", "cr1", "memory", "ctr",
-               "r0", "r29", "r27", "r28");
+             : "0" (-1),
+               "i" (EINVAL),
+               "i" (__NR_clone),
+               "i" (__NR_exit),
+               "r" (__fn),
+               "r" (__cstack),
+               "r" (__flags),
+               "r" (__arg),
+               "r" (__ptidptr),
+               "r" (__newtls),
+               "r" (__ctidptr),
+               "i" (FRAME_SIZE),
+               "i" (FRAME_TOC_SAVE_OFFSET)
+             : "cr0", "cr1", "memory", "ctr", "r0", "r27", "r28", "r29");
   return res;
 }
 #elif defined(__i386__) && SANITIZER_LINUX
 uptr internal_clone(int (*fn)(void *), void *child_stack, int flags, void *arg,
                     int *parent_tidptr, void *newtls, int *child_tidptr) {
   int res;
   if (!fn || !child_stack)
     return -EINVAL;