[LLVMdev] Compiler-RT - Enabling ThreadSanitizer on PPC64(BE/LE) platforms

[simoatze]

Summary

Enabling ThreadSaniitizer on PPC64(BE/LE) platforms. Currently the porting is at about 95%, considering that it's failing only 7 tests out of 257.
It looks like that all the failing tests are related to the system call setjmp.

[samsonov]

Please update the next version of patch with more context (see http://llvm.org/docs/Phabricator.html).

cmake/config-ix.cmake

146

This code seems to be used only on Android, why do you need to modify it?

162

Ditto

257

Why do you need this replacement? These variables were added to special-case Apple systems, where we want to target several variants of x86_64 architectures, unlike on another platforms. Listing both powerpc64 and powerpc64le below would be more clear IMO.

lib/sanitizer_common/sanitizer_linux.h

48

Is __powerpc64le__ a thing? It doesn't seem to be used anywhere in sanitizer code, and we support PowerPC support in ASan, for instance.

lib/tsan/rtl/tsan_interceptors.cc

2464

We only care about powerpc64

2470

Looks like it's better to fix TSAN_INTERCEPT_VER instead?

test/tsan/CMakeLists.txt

2

You should probably invert this check to work on x86 only

test/tsan/mmap_large.cc

19

See above - we only care about powerpc64, right?

[wschmidt]

lib/sanitizer_common/sanitizer_linux.cc

983

Remove LE version

1123

Remove LE version

lib/sanitizer_common/sanitizer_linux.h

48

Right, this is not a thing. Where endianness matters, there are other macros to be used.

__powerpc64le__

is #defined for both BE and LE systems.

lib/sanitizer_common/sanitizer_linux_libcdep.cc

182

Remove LE version, and so on to the end

[simoatze]

• Added more context.
• Addressed all the comments

[simoatze]

Ignore previous update.

[wschmidt]

test/tsan/mmap_large.cc

19

Looks like you missed addressing this comment. __powerpc is 32-bit PowerPC, and there's no intent to support TSan for that, that I'm aware of.

[simoatze]

Fixed test, ppc32 won't support Tsan.

[zatrazz]

lib/sanitizer_common/sanitizer_linux_libcdep.cc

272

You do not need the 'if' statement, just store as AArch64 does.

lib/tsan/rtl/tsan_interceptors.cc

73

This is only valid for ELFv2. BE is GLIBC_2.3.

[simoatze]

Inserted control for BE and LE when selecting the GLIBC version.

[samsonov]

LGTM with a couple of nits

lib/sanitizer_common/sanitizer_linux.cc

984

Please make a comment more reasonable

// FIXME: properly implement clone for PowerPC

lib/sanitizer_common/sanitizer_linux_libcdep.cc

344

Probably you don't need line break here

[simoatze]

Patch fixed according to last comments.

[zatrazz]

lib/sanitizer_common/sanitizer_linux.cc

989

Here is a working clone implementation for powerpc64 if you want to incorporate in your patch.

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
#else
#define FRAME_MIN_SIZE         32
#define FRAME_TOC_SAVE         24
#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;

  __asm__ __volatile__(
            /* fn, arg, child_stack are saved acrVoss the syscall */
            "mr 28, %5\n\t"
            "mr 29, %6\n\t"
            "mr 27, %8\n\t"

            /* syscall
              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"

            /* Do the function call */
            "std   2, %13(1)\n\t"
#if _CALL_ELF != 2
            "ld    0, 0(28)\n\t"
            "ld    2, 8(28)\n\t"
            "mtctr 0\n\t"
#else
            "mr    12, 28\n\t"
            "mtctr 12\n\t"
#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");
  return res;
}

Checked on powerpc64le, no regressions found.

[simoatze]

Updated patch with "internal_clone", thanks to zatrazz for the implementation.

[simoatze]

Fixed indentation spaces in "internal_clone" function. It made it fail the tests.

[simoatze]

Updated patch for supporting multiple VMA (44 and 46 bits). To support these multiple VMAS transformations I followed the same mechanism used for AARCH64. By default the system will build for VMA=46, in order to build for VMA=44 will be still necessary to define the macro SANITIZER_PPC64_VMA at build time, for example for CMake:

-D CMAKE_C_FLAGS='-D SANITIZER_PPC64_VMA=44' -D CMAKE_CXX_FLAGS='-D SANITIZER_PPC64_VMA=44'

Applying also the patch D13729, it is failing only a few tests, here a summary:

Failing tests with VMA=44:
cond_cancel.c
java_race_pc.cc
signal_longjmp.cc
signal_errno.cc

Failing tests with VMA=46:
java_race_pc.cc
race_on_mutex.c (this is not failing for VMA=44)

I am already working on these last few problems.

Simone

[simoatze]

I added XFAIL in the tests that are currently failing.
I added a comment in each test so the failure can be addressed later more easily.

This patch together with D13729 should be ready to be committed.

[simoatze]

Hi,

is ok to commit the patch in this status?
I talked to Bill, and he agreed to commit the patch as it is, since in the future they are more concerned about LE systems (VMA=46).

Currently XFAILing the tests with suspicious problems in libc (for VMA=44 bits) will produce XPASSes on ppc64le (VMA=46 bits).
It looks like FileCheck does not allow to discriminate this two different cases, do you have any suggestion?
Otherwise if we can just XFAIL those tests and have 3 XPASSes for ppc64le, I think the patch is ready to be committed unless you have some other comment.

Simone

[simoatze]

Ping...

[samsonov]

I'll let Dmitry should sign this off.

[dvyukov]

LGTM with a nit

lib/tsan/rtl/tsan_platform_linux.cc

247

Please split this check per-architecture. Current numbers (39 and 42) are for aarch64. Otherwise we will end up allowing just any vma size.

[simoatze]

Addressed Dmitry's comment, patch ready to land.

Bill, will you take care of the commit?

Thanks!
Simone

[wschmidt]

In D12841#304084, @simoatze wrote:

Addressed Dmitry's comment, patch ready to land.

Bill, will you take care of the commit?

Thanks!
Simone

Yes, I will do some quick testing and commit this patch, my setjmp/longjmp patch, and your Clang patch to enable things. If all goes well this will all land later today.

[wschmidt]

Committed as r255057.

cmake/config-ix.cmake

   check_symbol_exists(__i386__ "" __I386)
   check_symbol_exists(__mips__ "" __MIPS)
   check_symbol_exists(__mips64__ "" __MIPS64)
+  check_symbol_exists(__powerpc64__ "" __POWERPC64)
+  check_symbol_exists(__powerpc64le__ "" __POWERPC64LE)
   if(__ARM)
     add_default_target_arch(arm)
   elseif(__AARCH64)

     add_default_target_arch(mips64)
   elseif(__MIPS)
     add_default_target_arch(mips)
+  elseif(__POWERPC64)
+    add_default_target_arch(powerpc64)
+  elseif(__POWERPC64LE)
+    add_default_target_arch(powerpc64le)
   endif()
 endmacro()
 

 set(ARM64 aarch64)
 set(ARM32 arm)
 set(X86_64 x86_64)
+set(PPC64 powerpc64 powerpc64le)
 if(APPLE)
   set(ARM64 arm64)
   set(ARM32 armv7 armv7s)

 endif()
 
 set(ALL_BUILTIN_SUPPORTED_ARCH i386 i686 ${X86_64} ${ARM32} ${ARM64})
-set(ALL_SANITIZER_COMMON_SUPPORTED_ARCH ${X86_64} i386 i686 powerpc64
-  powerpc64le ${ARM32} ${ARM64} mips mips64 mipsel mips64el)
-set(ALL_ASAN_SUPPORTED_ARCH ${X86_64} i386 i686 powerpc64 powerpc64le ${ARM32}
+set(ALL_SANITIZER_COMMON_SUPPORTED_ARCH ${X86_64} i386 i686 ${PPC64}
+  ${ARM32} ${ARM64} mips mips64 mipsel mips64el)
+set(ALL_ASAN_SUPPORTED_ARCH ${X86_64} i386 i686 ${PPC64} ${ARM32}
   ${ARM64} mips mipsel mips64 mips64el)
 set(ALL_DFSAN_SUPPORTED_ARCH ${X86_64} mips64 mips64el ${ARM64})
 set(ALL_LSAN_SUPPORTED_ARCH ${X86_64} mips64 mips64el)
 set(ALL_MSAN_SUPPORTED_ARCH ${X86_64} mips64 mips64el)
 set(ALL_PROFILE_SUPPORTED_ARCH ${X86_64} i386 i686 ${ARM32} mips mips64
-    mipsel mips64el ${ARM64} powerpc64 powerpc64le)
-set(ALL_TSAN_SUPPORTED_ARCH ${X86_64} mips64 mips64el ${ARM64})
+    mipsel mips64el ${ARM64} ${PPC64})
+set(ALL_TSAN_SUPPORTED_ARCH ${X86_64} mips64 mips64el ${ARM64} ${PPC64})
 set(ALL_UBSAN_SUPPORTED_ARCH ${X86_64} i386 i686 ${ARM32} ${ARM64} mips
-    mipsel mips64 mips64el powerpc64 powerpc64le)
+    mipsel mips64 mips64el ${PPC64})
 set(ALL_SAFESTACK_SUPPORTED_ARCH ${X86_64} i386 i686)
 
 if(APPLE)

lib/sanitizer_common/sanitizer_linux.h

 // internal_sigaction instead.
 int internal_sigaction_norestorer(int signum, const void *act, void *oldact);
 void internal_sigdelset(__sanitizer_sigset_t *set, int signum);
-#if defined(__x86_64__) || defined(__mips__) || defined(__aarch64__)
+#if defined(__x86_64__) || defined(__mips__) || defined(__aarch64__) || \
+  defined(__powerpc64__)  || defined(__powerpc64le__)
 uptr internal_clone(int (*fn)(void *), void *child_stack, int flags, void *arg,
                     int *parent_tidptr, void *newtls, int *child_tidptr);
 #endif

lib/sanitizer_common/sanitizer_linux.cc

 // Are we using 32-bit or 64-bit Linux syscalls?
 // x32 (which defines __x86_64__) has SANITIZER_WORDSIZE == 32
 // but it still needs to use 64-bit syscalls.
-#if SANITIZER_LINUX && (defined(__x86_64__) || SANITIZER_WORDSIZE == 64)
+#if SANITIZER_LINUX && (defined(__x86_64__) || defined(__powerpc64__) \
+|| defined(__powerpc64le__) || SANITIZER_WORDSIZE == 64)
 # define SANITIZER_LINUX_USES_64BIT_SYSCALLS 1
 #else
 # define SANITIZER_LINUX_USES_64BIT_SYSCALLS 0

                        : "x30", "memory");
   return res;
 }
+#elif defined(__powerpc64__) || defined(__powerpc64le__)
+// TSAN_PPC64 - not completed
+uptr internal_clone(int (*fn)(void *), void *child_stack, int flags, void *arg,
+                    int *parent_tidptr, void *newtls, int *child_tidptr) {
+  long long res;
+
+  res = clone(fn, child_stack, flags, arg, parent_tidptr, newtls, child_tidptr);
+
+  return res;
+}
 #endif  // defined(__x86_64__) && SANITIZER_LINUX
 
 #if SANITIZER_ANDROID

   *bp = ucontext->uc_mcontext.gregs[REG_EBP];
   *sp = ucontext->uc_mcontext.gregs[REG_ESP];
 # endif
-#elif defined(__powerpc__) || defined(__powerpc64__)
+#elif defined(__powerpc__) || defined(__powerpc64__) \
+  || defined(__powerpc64le__)
   ucontext_t *ucontext = (ucontext_t*)context;
   *pc = ucontext->uc_mcontext.regs->nip;
   *sp = ucontext->uc_mcontext.regs->gpr[PT_R1];

lib/sanitizer_common/sanitizer_linux_libcdep.cc

 # define DL_INTERNAL_FUNCTION
 #endif
 
-#if defined(__mips__)
+#if defined(__mips__) || defined(__powerpc64__) || defined(__powerpc64le__)
 // TlsPreTcbSize includes size of struct pthread_descr and size of tcb
 // head structure. It lies before the static tls blocks.
 static uptr TlsPreTcbSize() {

 }
 
 #if (defined(__x86_64__) || defined(__i386__) || defined(__mips__) \
-    || defined(__aarch64__)) \
-    && SANITIZER_LINUX && !SANITIZER_ANDROID
+    || defined(__aarch64__) || defined(__powerpc64__) \
+  || defined(__powerpc64le__)) && SANITIZER_LINUX && !SANITIZER_ANDROID
 // sizeof(struct thread) from glibc.
 static atomic_uintptr_t kThreadDescriptorSize;
 

   val = 1776;
   atomic_store(&kThreadDescriptorSize, val, memory_order_relaxed);
   return val;
+#elif defined(__powerpc64__) || defined(__powerpc64le__)
+  val = FIRST_32_SECOND_64(1872, 1872);
+  if (val)
+    atomic_store(&kThreadDescriptorSize, val, memory_order_relaxed);
+  return val;
 #endif
   return 0;
 }

   descr_addr = thread_pointer - kTlsTcbOffset - TlsPreTcbSize();
 # elif defined(__aarch64__)
   descr_addr = reinterpret_cast<uptr>(__builtin_thread_pointer());
+# elif defined(__powerpc64__) || defined(__powerpc64le__)
+  const uptr kTlsTcbOffset = 0x7000;
+  uptr thread_pointer;
+  asm("addi %0,%%r13,0" : "=r" (thread_pointer));
+  descr_addr = thread_pointer - kTlsTcbOffset - TlsPreTcbSize();
 # else
 #  error "unsupported CPU arch"
 # endif

   *size = GetTlsSize();
   *addr -= *size;
   *addr += ThreadDescriptorSize();
-# elif defined(__mips__) || defined(__aarch64__)
+# elif defined(__mips__) || defined(__aarch64__) \
+  || defined(__powerpc64__) || defined(__powerpc64le__)
   *addr = ThreadSelf();
   *size = GetTlsSize();
 # else

lib/sanitizer_common/sanitizer_stoptheworld_linux_libcdep.cc

 
 #include "sanitizer_platform.h"
 #if SANITIZER_LINUX && (defined(__x86_64__) || defined(__mips__) || \
-                        defined(__aarch64__))
+                        defined(__aarch64__) || defined(__powerpc64__) || \
+                        defined(__powerpc64le__))
 
 #include "sanitizer_stoptheworld.h"
 

lib/tsan/rtl/tsan_interceptors.cc

   TSAN_INTERCEPT(pthread_join);
   TSAN_INTERCEPT(pthread_detach);
 
+#if defined(__powerpc__) || defined(__powerpc64__) \
+  || defined(__powerpc64le__)
+  TSAN_INTERCEPT(pthread_cond_init);
+  TSAN_INTERCEPT(pthread_cond_signal);
+  TSAN_INTERCEPT(pthread_cond_broadcast);
+  TSAN_INTERCEPT(pthread_cond_wait);
+  TSAN_INTERCEPT(pthread_cond_timedwait);
+  TSAN_INTERCEPT(pthread_cond_destroy);
+#else
   TSAN_INTERCEPT_VER(pthread_cond_init, PTHREAD_ABI_BASE);
   TSAN_INTERCEPT_VER(pthread_cond_signal, PTHREAD_ABI_BASE);
   TSAN_INTERCEPT_VER(pthread_cond_broadcast, PTHREAD_ABI_BASE);
   TSAN_INTERCEPT_VER(pthread_cond_wait, PTHREAD_ABI_BASE);
   TSAN_INTERCEPT_VER(pthread_cond_timedwait, PTHREAD_ABI_BASE);
   TSAN_INTERCEPT_VER(pthread_cond_destroy, PTHREAD_ABI_BASE);
+#endif
 
   TSAN_INTERCEPT(pthread_mutex_init);
   TSAN_INTERCEPT(pthread_mutex_destroy);

lib/tsan/rtl/tsan_platform.h

 const uptr kAppMemXor     = 0x04000000000ull;
 const uptr kVdsoBeg       = 0x37f00000000ull;
 # endif
+#elif defined(__powerpc64__) || defined(__powerpc64le__)
+/*
+C/C++ on linux/powerpc64
+0000 0000 1000 - 0100 0000 0000: main binary and/or MAP_32BIT mappings
+0100 0000 0000 - 0200 0000 0000: -
+0200 0000 0000 - 1000 0000 0000: shadow
[phone] - 3000 0000 0000: -
[phone] - 4000 0000 0000: metainfo (memory blocks and sync objects)
[phone] - 6000 0000 0000: -
[phone] - 6200 0000 0000: traces
[phone] - 7d00 0000 0000: -
+7d00 0000 0000 - 7e00 0000 0000: heap
+7e00 0000 0000 - 7e80 0000 0000: -
+7e80 0000 0000 - 8000 0000 0000: modules and main thread stack
+*/
+const uptr kMetaShadowBeg = 0x100000000000ull;
+const uptr kMetaShadowEnd = 0x200000000000ull;
+const uptr kTraceMemBeg   = 0x200000000000ull;
+const uptr kTraceMemEnd   = 0x220000000000ull;
+const uptr kShadowBeg     = 0x020000000000ull;
+const uptr kShadowEnd     = 0x100000000000ull;
+const uptr kHeapMemBeg    = 0x3d0000000000ull;
+const uptr kHeapMemEnd    = 0x3e0000000000ull;
+const uptr kLoAppMemBeg   = 0x000000001000ull;
+const uptr kLoAppMemEnd   = 0x010000000000ull;
+const uptr kHiAppMemBeg   = 0x3e8000000000ull;
+const uptr kHiAppMemEnd   = 0x400000000000ull;
+const uptr kAppMemMsk     = 0x3c0000000000ull;
+const uptr kAppMemXor     = 0x020000000000ull;
+const uptr kVdsoBeg       = 0xf000000000000000ull;
 #endif
 
 ALWAYS_INLINE

lib/tsan/rtl/tsan_platform_linux.cc

 #elif defined(__aarch64__)
   const uptr kMadviseRangeBeg  = 0x7e00000000ull;
   const uptr kMadviseRangeSize = 0x0100000000ull;
+#elif defined(__powerpc__) || defined(__powerpc64__) || \
+  defined(__powerpc64le__)
+  const uptr kMadviseRangeBeg  = 0x3f0000000000ull;
+  const uptr kMadviseRangeSize = 0x010000000000ull;
 #endif
   NoHugePagesInRegion(MemToShadow(kMadviseRangeBeg),
                       kMadviseRangeSize * kShadowMultiplier);

test/tsan/CMakeLists.txt

 set(TSAN_TEST_DEPS ${SANITIZER_COMMON_LIT_TEST_DEPS})
-if(NOT ${COMPILER_RT_DEFAULT_TARGET_ARCH} MATCHES "mips")
+if(NOT ${COMPILER_RT_DEFAULT_TARGET_ARCH} MATCHES "mips|powerpc64|powerpc64le")
   list(APPEND TSAN_TEST_DEPS GotsanRuntimeCheck)
 endif()
 if(NOT COMPILER_RT_STANDALONE_BUILD)

test/tsan/map32bit.cc

 // MAP_32BIT flag for mmap is supported only for x86_64.
 // XFAIL: mips64
 // XFAIL: aarch64
+// XFAIL: powerpc64
+// XFAIL: powerpc64le
 
 void *Thread(void *ptr) {
   *(int*)ptr = 42;

test/tsan/mmap_large.cc

   const size_t kLog2Size = 39;
 #elif defined(__mips64) || defined(__aarch64__)
   const size_t kLog2Size = 32;
+#elif defined(__powerpc)
+  const size_t kLog2Size = 39;
+#elif defined(__powerpc64__)
+  const size_t kLog2Size = 39;
 #endif
   const uintptr_t kLocation = 0x40ULL << kLog2Size;
   void *p = mmap(

test/tsan/test.h

   }
 
   fprintf(stderr, format, (unsigned long) address);
+#elif defined(__powerpc64__)
+  fprintf(stderr, "0x%012lx", (unsigned long) address);
 #endif
 }