On Fri, Nov 16, 2012 at 3:08 PM, Peter Bergner <berg...@vnet.ibm.com> wrote:
> Attached below is an initial port of ASAN to powerpc*-linux.
> With the patch below along with Jakub's ASAN testsuite patch:
>
> http://gcc.gnu.org/ml/gcc-patches/2012-11/msg01365.html
>
> ASAN not only builds, but seems to be working.
Outstanding!
> 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)
>
> ==47550== ERROR: AddressSanitizer stack-buffer-overflow on address
> 0x0fffe65540a4 at pc 0xfff769d0d70 bp 0xfffe6553ed0 sp 0xfffe6553fe8
> READ of size 1 at 0x0fffe65540a4 thread T0
> #0 0xfff769dc388 in
> _ZN6__asan13GetStackTraceEPN11__sanitizer10StackTraceEmmm
> /home/bergner/gcc/gcc-fsf-mainline-asan/libsanitizer/asan/asan_linux.cc:160
> #1 0xfff769df4f0 in __asan_report_error
> /home/bergner/gcc/gcc-fsf-mainline-asan/libsanitizer/asan/asan_report.cc:471
> (discriminator 5)
> #2 0xfff769d0dbc in __interceptor_memcmp
> /home/bergner/gcc/gcc-fsf-mainline-asan/libsanitizer/asan/asan_interceptors.cc:218
> (discriminator 1)
> #3 0x10000b4c in main
> /home/bergner/gcc/gcc-fsf-mainline-asan/gcc/testsuite/c-c++-common/asan/memcmp-1.c:12
>
> ...which doesn't match:
>
> /* { dg-output " #0 0x\[0-9a-f\]+ (in _*(interceptor_|)memcmp
> |\[(\])\[^\n\r]*(\n|\r\n|\r)" } */
> /* { dg-output " #1 0x\[0-9a-f\]+ (in _*main|\[(\])\[^\n\r]*(\n|\r\n|\r)"
> } */
>
> because the frame numbers are off. Any ideas on how to fix that?
>
> We cannot use the FastUnwindStack, since that uses exclusively the
> frame pointer which ppc almost never uses. We also can have stack
> frames above or below the stack pointer, depending on whether the
> function is a leaf or not. And with shrink wrapping, can we even
> be sure if the frame pointer is even setup even on those systems
> that do use the frame pointer? I've seen others say we should
> use libbacktrace, and I think I have to agree with them for the
> reasons above.
FastUnwindStack is clearly x86-only thing.
I'd love to have fast solutions for other architectures, but for now
libbacktrace (or any other library call)
is ok for non-x86.
We just need to make sure that those functions do not call malloc/etc.
>
> 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?
<Hopefully some one else can comment here>
>
> I should also note, the code setting the page size is very fragile.
> On PPC/PPC64 kernels, you can configure the kernel to use different
> page sizes. My systems are running 64K page sizes, but could just
> as easily be 4K or some other number. Shouldn't we be calling
> getpagesize() or sysconf() to query the page size and then computing
> the other values from that?
>
> 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.
>
> Does anyone have any thoughts on the patch? Does it look reasonable?
>
> Peter
>
>
> gcc/
> * config/rs6000/rs6000.c (TARGET_ASAN_SHADOW_OFFSET): Define.
> (rs6000_asan_shadow_offset): New function.
>
> libsanitizer/
> * configure.tgt: Enable build on powerpc and powerpc64 linux.
> * sanitizer_common/sanitizer_common.h (kPageSizeBits): Set for powerpc
> and powerpc64.
> (kPageSize): Likewise.
> (kCacheLineSize): Likewise.
> (kMmapGranularity): Likewise.
> * sanitizer_common/sanitizer_linux.cc (__NR_mmap) Call for powerpc64.
> (__NR_fstat): Likewise.
> * sanitizer_common/sanitizer_stacktrace.cc (patch_pc): Align to 4
> bytes.
> * asan/asan_mapping.h (SHADOW_OFFSET): Define for powerpc and
> powerpc64.
> (kHighMemEnd): Set for powerpc and powerpc64.
> * asan/asan_linux.cc (GetPcSpBp): Add powerpc and powerpc64 support.
> (GetStackTrace): Use _Unwind_Backtrace for powerpc and powerpc64.
>
> Index: libsanitizer/configure.tgt
> ===================================================================
> --- libsanitizer/configure.tgt (revision 193575)
> +++ 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: libsanitizer/sanitizer_common/sanitizer_common.h
> ===================================================================
> --- libsanitizer/sanitizer_common/sanitizer_common.h (revision 193575)
> +++ 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;
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.
> +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 193575)
> +++ 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
> 193575)
> +++ 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) {
> Index: libsanitizer/asan/asan_mapping.h
> ===================================================================
> --- libsanitizer/asan/asan_mapping.h (revision 193575)
> +++ 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 193575)
> +++ 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,7 +156,7 @@
> 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);
> #else
> if (!asan_inited) return;
> Index: gcc/config/rs6000/rs6000.c
> ===================================================================
> --- gcc/config/rs6000/rs6000.c (revision 193575)
> +++ 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
>
> @@ -27372,6 +27375,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
>
>
overall the patch looks great.
I really want it to go through LLVM tree first (otherwise we'll lose
control of the code very soon),
but I am boarding a plane and will not be available for a few days after that.
Hopefully other asan folks (CC-ed) can finish the review and commit to llvm.
Thanks!
--kcc
