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

[Konstantin Serebryany <konstantin dot s dot serebryany at gmail dot com>]

On Tue, Nov 20, 2012 at 11:08 PM, Peter Bergner <bergner@vnet.ibm.com> wrote:
> On Tue, 2012-11-20 at 18:18 +0400, Evgeniy Stepanov wrote:
>
>> I wonder if under some conditions we may get a different number of
>> extra frames (inlining comes to mind). What do you think of removing
>> any number of frames that belong to the runtime library - we have
>> memory layout info for that?
>
> How about the following hack that needs to be cleaned up, but does
> work for me.  It scans through the trace looking for the frame pointer
> that is passed in and if it finds it, it pops the stack up to that
> point.  If it doesn't find it (a bug?), it just leaves the trace
> alone.
>
> Maybe it fixes the ARM Android issue you just ran into?
>
> Peter
>
>
>
> diff -urpN -X /home/bergner/cvs/dontdiff gcc-fsf-mainline-kcc/libsanitizer/asan/asan_linux.cc gcc-fsf-mainline-asan/libsanitizer/asan/asan_linux.cc
> --- gcc-fsf-mainline-kcc/libsanitizer/asan/asan_linux.cc        2012-11-20 12:52:33.961664485 -0600
> +++ gcc-fsf-mainline-asan/libsanitizer/asan/asan_linux.cc       2012-11-20 11:28:00.646245908 -0600
> @@ -141,12 +141,19 @@ uptr Unwind_GetIP(struct _Unwind_Context
>  #endif
>  }
>
> +uptr Unwind_GetBP(struct _Unwind_Context *ctx) {
> +  return _Unwind_GetCFA(ctx);
> +}
> +
>  _Unwind_Reason_Code Unwind_Trace(struct _Unwind_Context *ctx,
>      void *param) {
>    StackTrace *b = (StackTrace*)param;
>    CHECK(b->size < b->max_size);
>    uptr pc = Unwind_GetIP(ctx);
> -  b->trace[b->size++] = pc;
> +  uptr bp = Unwind_GetBP(ctx);
> +  b->trace[b->size] = pc;
> +  b->frame[b->size] = bp;
> +  b->size++;
>    if (b->size == b->max_size) return UNWIND_STOP;
>    return UNWIND_CONTINUE;
>  }
> @@ -158,8 +165,13 @@ void GetStackTrace(StackTrace *stack, up
>      stack->max_size = max_s;
>  #if defined(__arm__) || defined(__powerpc__) || defined(__powerpc64__)
>      _Unwind_Backtrace(Unwind_Trace, stack);
> -    // Pop off the two ASAN functions from the backtrace.
> -    stack->PopStackFrames(2);
> +    // Attempt to pop off the ASAN functions from the backtrace.
> +    uptr cnt;
> +    for (cnt = 0; cnt < stack->size; cnt++)
> +      if (stack->frame[cnt] == bp) {
> +       stack->PopStackFrames(cnt);
> +       break;
> +      }
>  #else
>      if (!asan_inited) return;
>      if (AsanThread *t = asanThreadRegistry().GetCurrent())
> diff -urpN -X /home/bergner/cvs/dontdiff gcc-fsf-mainline-kcc/libsanitizer/sanitizer_common/sanitizer_stacktrace.cc gcc-fsf-mainline-asan/libsanitizer/sanitizer_common/sanitizer_stacktrace.cc
> --- gcc-fsf-mainline-kcc/libsanitizer/sanitizer_common/sanitizer_stacktrace.cc  2012-11-20 11:42:08.834439704 -0600
> +++ gcc-fsf-mainline-asan/libsanitizer/sanitizer_common/sanitizer_stacktrace.cc 2012-11-20 12:59:29.896106625 -0600
> @@ -144,7 +144,8 @@ void StackTrace::PopStackFrames(uptr cou
>    CHECK(size > count);
>    size -= count;
>    for (uptr i = 0; i < size; i++) {
> -    trace[i] = trace[i + count];
> +    trace[i] = trace[i+count];
> +    frame[i] = frame[i+count];
>    }
>  }
>
> diff -urpN -X /home/bergner/cvs/dontdiff gcc-fsf-mainline-kcc/libsanitizer/sanitizer_common/sanitizer_stacktrace.h gcc-fsf-mainline-asan/libsanitizer/sanitizer_common/sanitizer_stacktrace.h
> --- gcc-fsf-mainline-kcc/libsanitizer/sanitizer_common/sanitizer_stacktrace.h   2012-11-20 11:42:08.821389243 -0600
> +++ gcc-fsf-mainline-asan/libsanitizer/sanitizer_common/sanitizer_stacktrace.h  2012-11-20 11:12:44.551390980 -0600
> @@ -23,6 +23,7 @@ struct StackTrace {
>    uptr size;
>    uptr max_size;
>    uptr trace[kStackTraceMax];
> +  uptr frame[kStackTraceMax];  // For use by _Unwind_Backtrace architectures.

This is bad. The objects of this type are already too big, we should
not make them 2x larger.
Hopefully Evgeniy can handle this tomorrow.

--kcc


>    static void PrintStack(const uptr *addr, uptr size,
>                           bool symbolize, const char *strip_file_prefix,
>                           SymbolizeCallback symbolize_callback);
>
>