This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH, RFC] Enable libsanitizer on powerpc{,64}


On Fri, 2012-11-16 at 15:47 -0800, Konstantin Serebryany wrote:
> On Fri, Nov 16, 2012 at 3:08 PM, Peter Bergner <bergner@vnet.ibm.com> wrote:
> > The lone ASAN
> > test case does fail, but it seems to be related to us using
> > _Unwind_Backtrace() and we end up with two extra frames at the
> > bottom of our stack frame, so we don't match the expected
> > results.
> 
> Maybe just drop those two frames before reporting them?
> (if we always have them)

I was worried about whether we always have to extra frames from the
ASAN runtime or not, but looking at the code, it seems we do, so I
added a StackTrace::PopStackFrames(uptr count) method that can be
called after the call to _Unwind_Backtrace() pop off the offending
frames.  With this change, we now pass the ASAN testsuite...all 1
of them. :)



> > One question that I have is that the toplev.c test for port support
> > tests for !FRAME_GROWS_DOWNWARD.  The rs6000 port has FRAME_GROWS_DOWNWARD
> > defined as (flag_stack_protect != 0), so ASAN only works when we use
> > -fstack-protector.  Is there a technical reason why FRAME_GROWS_DOWNWARD
> > must be false?

Jakub answered this and I took his suggestion and redefined our target's
FRAME_GROWS_DOWNWARD to include !flag_asan so we no longer need to use
-fstack-protector.




> > I also don't like all the #ifdef's sprinkling the code.  Can't we
> > use some configure fragments to help remove many/most of these?
> 
> We'll probably have to add some config-like headers as we get more platforms.
> Not necessarily now.

With the SANITIZER_LINUX_USES_64BIT_SYSCALLS patch, I see you started
to address this.  Nice.  I look forward to more of that.



> > +#if defined(__powerpc__) || defined(__powerpc64__)
> > +// Current PPC64 kernels use 64K pages sizes, but they can be
> > +// configured with 4K or even other sizes, so we should probably
> > +// use getpagesize() or sysconf(_SC_PAGESIZE) here rather than
> > +// hardcoding the values.
> > +const uptr kPageSizeBits = 16;
> 
> Interesting. This may have some unexpected side effects. (or maybe not?)
> It's of course ok to try like this and change it later if something got broken.

Well, we _have_ to make this change, since without it, we attempt to do
an mmap of 4K and that will fail on any system with a page size larger
than 4k, since page size is the minimum mmap quantity.


I have attached our current patch which does not include your change that
added SANITIZER_LINUX_USES_64BIT_SYSCALLS, since that isn't upstream yet.

Peter


Index: libsanitizer/sanitizer_common/sanitizer_stacktrace.h
===================================================================
--- libsanitizer/sanitizer_common/sanitizer_stacktrace.h	(revision 193626)
+++ libsanitizer/sanitizer_common/sanitizer_stacktrace.h	(working copy)
@@ -43,6 +43,8 @@
 
   void FastUnwindStack(uptr pc, uptr bp, uptr stack_top, uptr stack_bottom);
 
+  void PopStackFrames(uptr count);
+
   static uptr GetCurrentPc();
 
   static uptr CompressStack(StackTrace *stack,
Index: libsanitizer/sanitizer_common/sanitizer_common.h
===================================================================
--- libsanitizer/sanitizer_common/sanitizer_common.h	(revision 193626)
+++ libsanitizer/sanitizer_common/sanitizer_common.h	(working copy)
@@ -21,12 +21,24 @@
 // Constants.
 const uptr kWordSize = __WORDSIZE / 8;
 const uptr kWordSizeInBits = 8 * kWordSize;
+#if defined(__powerpc__) || defined(__powerpc64__)
+// Current PPC64 kernels use 64K pages sizes, but they can be
+// configured with 4K or even other sizes, so we should probably
+// use getpagesize() or sysconf(_SC_PAGESIZE) here rather than
+// hardcoding the values.
+const uptr kPageSizeBits = 16;
+const uptr kPageSize = 1UL << kPageSizeBits;
+const uptr kCacheLineSize = 128;
+const uptr kMmapGranularity = kPageSize;
+#elif !defined( _WIN32)
 const uptr kPageSizeBits = 12;
 const uptr kPageSize = 1UL << kPageSizeBits;
 const uptr kCacheLineSize = 64;
-#ifndef _WIN32
 const uptr kMmapGranularity = kPageSize;
 #else
+const uptr kPageSizeBits = 12;
+const uptr kPageSize = 1UL << kPageSizeBits;
+const uptr kCacheLineSize = 64;
 const uptr kMmapGranularity = 1UL << 16;
 #endif
 
Index: libsanitizer/sanitizer_common/sanitizer_linux.cc
===================================================================
--- libsanitizer/sanitizer_common/sanitizer_linux.cc	(revision 193626)
+++ libsanitizer/sanitizer_common/sanitizer_linux.cc	(working copy)
@@ -34,7 +34,7 @@
 // --------------- sanitizer_libc.h
 void *internal_mmap(void *addr, uptr length, int prot, int flags,
                     int fd, u64 offset) {
-#if defined __x86_64__
+#if defined(__x86_64__) || defined(__powerpc64__)
   return (void *)syscall(__NR_mmap, addr, length, prot, flags, fd, offset);
 #else
   return (void *)syscall(__NR_mmap2, addr, length, prot, flags, fd, offset);
@@ -67,7 +67,7 @@
 }
 
 uptr internal_filesize(fd_t fd) {
-#if defined __x86_64__
+#if defined(__x86_64__) || defined(__powerpc64__)
   struct stat st;
   if (syscall(__NR_fstat, fd, &st))
     return -1;
Index: libsanitizer/sanitizer_common/sanitizer_stacktrace.cc
===================================================================
--- libsanitizer/sanitizer_common/sanitizer_stacktrace.cc	(revision 193626)
+++ libsanitizer/sanitizer_common/sanitizer_stacktrace.cc	(working copy)
@@ -31,7 +31,12 @@
   // Cancel Thumb bit.
   pc = pc & (~1);
 #endif
+#if defined(__powerpc__) || defined(__powerpc64__)
+  // PCs are always 4 byte aligned.
+  return pc - 4;
+#else
   return pc - 1;
+#endif
 }
 
 static void PrintStackFramePrefix(uptr frame_num, uptr pc) {
@@ -135,6 +140,16 @@
   }
 }
 
+void StackTrace::PopStackFrames(uptr count) {
+  CHECK(size > count);
+  size -= count;
+  uptr i;
+
+  for (i = 0; i < size; i++) {
+    trace[i] = trace[i+count];
+  }
+}
+
 // On 32-bits we don't compress stack traces.
 // On 64-bits we compress stack traces: if a given pc differes slightly from
 // the previous one, we record a 31-bit offset instead of the full pc.
Index: libsanitizer/asan/asan_mapping.h
===================================================================
--- libsanitizer/asan/asan_mapping.h	(revision 193626)
+++ libsanitizer/asan/asan_mapping.h	(working copy)
@@ -31,7 +31,11 @@
 #  if __WORDSIZE == 32
 #   define SHADOW_OFFSET (1 << 29)
 #  else
-#   define SHADOW_OFFSET (1ULL << 44)
+#   if defined(__powerpc64__)
+#    define SHADOW_OFFSET (1ULL << 41)
+#   else
+#    define SHADOW_OFFSET (1ULL << 44)
+#   endif
 #  endif
 # endif
 #endif  // ASAN_FLEXIBLE_MAPPING_AND_OFFSET
@@ -41,7 +45,11 @@
 #define SHADOW_TO_MEM(shadow) (((shadow) - SHADOW_OFFSET) << SHADOW_SCALE)
 
 #if __WORDSIZE == 64
+# if defined(__powerpc64__)
+  static const uptr kHighMemEnd = 0x00000fffffffffffUL;
+# else
   static const uptr kHighMemEnd = 0x00007fffffffffffUL;
+# endif
 #else  // __WORDSIZE == 32
   static const uptr kHighMemEnd = 0xffffffff;
 #endif  // __WORDSIZE
Index: libsanitizer/asan/asan_linux.cc
===================================================================
--- libsanitizer/asan/asan_linux.cc	(revision 193626)
+++ libsanitizer/asan/asan_linux.cc	(working copy)
@@ -66,6 +66,13 @@
   *pc = ucontext->uc_mcontext.gregs[REG_EIP];
   *bp = ucontext->uc_mcontext.gregs[REG_EBP];
   *sp = ucontext->uc_mcontext.gregs[REG_ESP];
+# elif defined(__powerpc__) || defined(__powerpc64__)
+  ucontext_t *ucontext = (ucontext_t*)context;
+  *pc = ucontext->uc_mcontext.regs->nip;
+  *sp = ucontext->uc_mcontext.regs->gpr[PT_R1];
+  // The powerpc{,64}-linux ABIs do not specify r31 as the frame
+  // pointer, but GCC always uses r31 when we need a frame pointer.
+  *bp = ucontext->uc_mcontext.regs->gpr[PT_R31];
 # elif defined(__sparc__)
   ucontext_t *ucontext = (ucontext_t*)context;
   uptr *stk_ptr;
@@ -149,8 +156,10 @@
   stack->trace[0] = pc;
   if ((max_s) > 1) {
     stack->max_size = max_s;
-#ifdef __arm__
+#if defined(__arm__) || defined(__powerpc__) || defined(__powerpc64__)
     _Unwind_Backtrace(Unwind_Trace, stack);
+    // Pop off the two ASAN functions from the backtrace.
+    stack->PopStackFrames(2);
 #else
     if (!asan_inited) return;
     if (AsanThread *t = asanThreadRegistry().GetCurrent())
Index: libsanitizer/configure.tgt
===================================================================
--- libsanitizer/configure.tgt	(revision 193626)
+++ libsanitizer/configure.tgt	(working copy)
@@ -20,6 +20,8 @@
 
 # Filter out unsupported systems.
 case "${target}" in
+  powerpc*-*-linux*)
+	;;
   x86_64-*-linux* | i?86-*-linux* | sparc*-*-linux*)
 	;;
   *)
Index: gcc/config/rs6000/rs6000.c
===================================================================
--- gcc/config/rs6000/rs6000.c	(revision 193626)
+++ gcc/config/rs6000/rs6000.c	(working copy)
@@ -1186,6 +1186,9 @@
 #undef TARGET_DELEGITIMIZE_ADDRESS
 #define TARGET_DELEGITIMIZE_ADDRESS rs6000_delegitimize_address
 
+#undef TARGET_ASAN_SHADOW_OFFSET
+#define TARGET_ASAN_SHADOW_OFFSET rs6000_asan_shadow_offset
+
 #undef TARGET_CONST_NOT_OK_FOR_DEBUG_P
 #define TARGET_CONST_NOT_OK_FOR_DEBUG_P rs6000_const_not_ok_for_debug_p
 
@@ -27370,6 +27373,13 @@
     }
 }
 
+/* Implement the TARGET_ASAN_SHADOW_OFFSET hook.  */
+
+static unsigned HOST_WIDE_INT
+rs6000_asan_shadow_offset (void)
+{
+  return (unsigned HOST_WIDE_INT) 1 << (TARGET_64BIT ? 41 : 29);
+}
 
 /* Mask options that we want to support inside of attribute((target)) and
    #pragma GCC target operations.  Note, we do not include things like
Index: gcc/config/rs6000/rs6000.h
===================================================================
--- gcc/config/rs6000/rs6000.h	(revision 193626)
+++ gcc/config/rs6000/rs6000.h	(working copy)
@@ -1406,7 +1406,7 @@
 
    On the RS/6000, we grow upwards, from the area after the outgoing
    arguments.  */
-#define FRAME_GROWS_DOWNWARD (flag_stack_protect != 0)
+#define FRAME_GROWS_DOWNWARD (flag_stack_protect != 0 || flag_asan != 0)
 
 /* Size of the outgoing register save area */
 #define RS6000_REG_SAVE ((DEFAULT_ABI == ABI_AIX			\




Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]