On Fri, 2012-11-16 at 15:47 -0800, Konstantin Serebryany wrote:
> On Fri, Nov 16, 2012 at 3:08 PM, Peter Bergner <berg...@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 \
