The attached patch resolves the failures in a number of address sanitizer tests on powerpc64*-*-*-* discussed in bug 65479 (the failures in c-c++-common/asan/swapcontext-test-1.c reported in pr65643 remain unresolved).
The patch has been tested on powerpc64*-*-*-* and x86_64 with no regressions. Is this okay for trunk? For 5.1? Martin
diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog index b4052ef..18eede3 100644 --- a/gcc/testsuite/ChangeLog +++ b/gcc/testsuite/ChangeLog @@ -1,3 +1,12 @@ +2015-04-19 Martin Sebor <mse...@redhat.com> + + PR sanitizer/65479 + * gcc/testsuite/c-c++-common/asan/misalign-1.c [powerpc*-*-*-*]: + Use -fno-omit-frame-pointer. Adjust line numbers and expect exact + matches. + * gcc/testsuite/c-c++-common/asan/misalign-2.c: Ditto. + * gcc/testsuite/c-c++-common/asan/null-deref-1.c: Ditto. + 2015-04-18 Martin Sebor <mse...@redhat.com> * gfortran.dg/pr32627.f03 (strptr): Change size to match the number diff --git a/gcc/testsuite/c-c++-common/asan/misalign-1.c b/gcc/testsuite/c-c++-common/asan/misalign-1.c index f1cca16..833b82a 100644 --- a/gcc/testsuite/c-c++-common/asan/misalign-1.c +++ b/gcc/testsuite/c-c++-common/asan/misalign-1.c @@ -1,5 +1,6 @@ /* { dg-do run { target { ilp32 || lp64 } } } */ /* { dg-options "-O2" } */ +/* { dg-additional-options "-fasynchronous-unwind-tables" { target powerpc*-*-*-* } } */ /* { dg-additional-options "-fno-omit-frame-pointer" { target *-*-darwin* } } */ /* { dg-shouldfail "asan" } */ @@ -39,5 +40,5 @@ main () /* { dg-output "ERROR: AddressSanitizer:\[^\n\r]*on address\[^\n\r]*" } */ /* { dg-output "0x\[0-9a-f\]+ at pc 0x\[0-9a-f\]+ bp 0x\[0-9a-f\]+ sp 0x\[0-9a-f\]+\[^\n\r]*(\n|\r\n|\r)" } */ /* { dg-output "\[^\n\r]*READ of size 4 at 0x\[0-9a-f\]+ thread T0\[^\n\r]*(\n|\r\n|\r)" } */ -/* { dg-output " #0 0x\[0-9a-f\]+ +(in _*foo(\[^\n\r]*misalign-1.c:1\[01]|\[^\n\r]*:0)|\[(\])\[^\n\r]*(\n|\r\n|\r)" } */ -/* { dg-output " #1 0x\[0-9a-f\]+ +(in _*main (\[^\n\r]*misalign-1.c:3\[45]|\[^\n\r]*:0)|\[(\]).*(\n|\r\n|\r)" } */ +/* { dg-output " #0 0x\[0-9a-f\]+ +(in _*foo(\[^\n\r]*misalign-1.c:12|\[^\n\r]*:0)|\[(\])\[^\n\r]*(\n|\r\n|\r)" } */ +/* { dg-output " #1 0x\[0-9a-f\]+ +(in _*main (\[^\n\r]*misalign-1.c:36|\[^\n\r]*:0)|\[(\]).*(\n|\r\n|\r)" } */ diff --git a/gcc/testsuite/c-c++-common/asan/misalign-2.c b/gcc/testsuite/c-c++-common/asan/misalign-2.c index 9f400b4..923d26b 100644 --- a/gcc/testsuite/c-c++-common/asan/misalign-2.c +++ b/gcc/testsuite/c-c++-common/asan/misalign-2.c @@ -1,5 +1,6 @@ /* { dg-do run { target { ilp32 || lp64 } } } */ /* { dg-options "-O2" } */ +/* { dg-additional-options "-fasynchronous-unwind-tables" { target powerpc*-*-*-* } } */ /* { dg-additional-options "-fno-omit-frame-pointer" { target *-*-darwin* } } */ /* { dg-shouldfail "asan" } */ @@ -39,5 +40,5 @@ main () /* { dg-output "ERROR: AddressSanitizer:\[^\n\r]*on address\[^\n\r]*" } */ /* { dg-output "0x\[0-9a-f\]+ at pc 0x\[0-9a-f\]+ bp 0x\[0-9a-f\]+ sp 0x\[0-9a-f\]+\[^\n\r]*(\n|\r\n|\r)" } */ /* { dg-output "\[^\n\r]*READ of size 4 at 0x\[0-9a-f\]+ thread T0\[^\n\r]*(\n|\r\n|\r)" } */ -/* { dg-output " #0 0x\[0-9a-f\]+ +(in _*baz(\[^\n\r]*misalign-2.c:2\[23]|\[^\n\r]*:0)|\[(\])\[^\n\r]*(\n|\r\n|\r)" } */ -/* { dg-output " #1 0x\[0-9a-f\]+ +(in _*main (\[^\n\r]*misalign-2.c:3\[45]|\[^\n\r]*:0)|\[(\]).*(\n|\r\n|\r)" } */ +/* { dg-output " #0 0x\[0-9a-f\]+ +(in _*baz(\[^\n\r]*misalign-2.c:24|\[^\n\r]*:0)|\[(\])\[^\n\r]*(\n|\r\n|\r)" } */ +/* { dg-output " #1 0x\[0-9a-f\]+ +(in _*main (\[^\n\r]*misalign-2.c:36|\[^\n\r]*:0)|\[(\]).*(\n|\r\n|\r)" } */ diff --git a/gcc/testsuite/c-c++-common/asan/null-deref-1.c b/gcc/testsuite/c-c++-common/asan/null-deref-1.c index 45d35ac..b9bc3e5 100644 --- a/gcc/testsuite/c-c++-common/asan/null-deref-1.c +++ b/gcc/testsuite/c-c++-common/asan/null-deref-1.c @@ -1,5 +1,6 @@ /* { dg-do run } */ /* { dg-options "-fno-omit-frame-pointer -fno-shrink-wrap" } */ +/* { dg-additional-options "-fasynchronous-unwind-tables" { target { powerpc*-*-*-*} } } */ /* { dg-additional-options "-mno-omit-leaf-frame-pointer" { target { i?86-*-* x86_64-*-* } } } */ /* { dg-shouldfail "asan" } */ @@ -18,5 +19,5 @@ int main() /* { dg-output "ERROR: AddressSanitizer:? SEGV on unknown address\[^\n\r]*" } */ /* { dg-output "0x\[0-9a-f\]+ \[^\n\r]*pc 0x\[0-9a-f\]+\[^\n\r]*(\n|\r\n|\r)" } */ -/* { dg-output "\[^\n\r]* #0 0x\[0-9a-f\]+ +(in \[^\n\r]*NullDeref\[^\n\r]* (\[^\n\r]*null-deref-1.c:10|\[^\n\r]*:0)|\[(\])\[^\n\r]*(\n|\r\n|\r)" } */ -/* { dg-output " #1 0x\[0-9a-f\]+ +(in _*main (\[^\n\r]*null-deref-1.c:15|\[^\n\r]*:0)|\[(\])\[^\n\r]*(\n|\r\n|\r)" } */ +/* { dg-output "\[^\n\r]* #0 0x\[0-9a-f\]+ +(in \[^\n\r]*NullDeref\[^\n\r]* (\[^\n\r]*null-deref-1.c:11|\[^\n\r]*:0)|\[(\])\[^\n\r]*(\n|\r\n|\r)" } */ +/* { dg-output " #1 0x\[0-9a-f\]+ +(in _*main (\[^\n\r]*null-deref-1.c:16|\[^\n\r]*:0)|\[(\])\[^\n\r]*(\n|\r\n|\r)" } */ diff --git a/libbacktrace/ChangeLog b/libbacktrace/ChangeLog index e385d8f..9348321 100644 --- a/libbacktrace/ChangeLog +++ b/libbacktrace/ChangeLog @@ -1,3 +1,11 @@ +2015-04-19 Martin Sebor <mse...@redhat.com> + + PR sanitizer/65479 + * libbacktrace/dwarf.c (struct line): Add idx data member. + (line_compare): Use struct line idx member. + (add_line): Set ln->idx. + (read_line_info): Clear ln->idx. + 2015-01-24 Matthias Klose <d...@ubuntu.com> * configure.ac: Move AM_ENABLE_MULTILIB before AC_PROG_CC. diff --git a/libbacktrace/dwarf.c b/libbacktrace/dwarf.c index 919b568..e32c468 100644 --- a/libbacktrace/dwarf.c +++ b/libbacktrace/dwarf.c @@ -211,6 +211,10 @@ struct line const char *filename; /* Line number. */ int lineno; + /* Index of the object in the original array read from the DWARF + section, before it has been sorted. The index makes it possible + to use Quicksort and maintain stability. */ + int idx; }; /* A growable vector of line number information. This is used while @@ -940,9 +944,10 @@ unit_addrs_search (const void *vkey, const void *ventry) return 0; } -/* Sort the line vector by PC. We want a stable sort here. We know - that the pointers are into the same array, so it is safe to compare - them directly. */ +/* Sort the line vector by PC. We want a stable sort here to maintain + the order of lines for the same PC values. Since the sequence is + being sorted in place, their addresses cannot be relied on to + maintain stability. That is the purpose of the index member. */ static int line_compare (const void *v1, const void *v2) @@ -954,9 +959,9 @@ line_compare (const void *v1, const void *v2) return -1; else if (ln1->pc > ln2->pc) return 1; - else if (ln1 < ln2) + else if (ln1->idx < ln2->idx) return -1; - else if (ln1 > ln2) + else if (ln1->idx > ln2->idx) return 1; else return 0; @@ -1551,6 +1556,7 @@ add_line (struct backtrace_state *state, struct dwarf_data *ddata, ln->filename = filename; ln->lineno = lineno; + ln->idx = vec->count; ++vec->count; @@ -2011,6 +2017,7 @@ read_line_info (struct backtrace_state *state, struct dwarf_data *ddata, ln->pc = (uintptr_t) -1; ln->filename = NULL; ln->lineno = 0; + ln->idx = 0; if (!backtrace_vector_release (state, &vec.vec, error_callback, data)) goto fail; diff --git a/libsanitizer/ChangeLog b/libsanitizer/ChangeLog index 6f44dcf..7b82378 100644 --- a/libsanitizer/ChangeLog +++ b/libsanitizer/ChangeLog @@ -1,3 +1,15 @@ +2015-04-19 Martin Sebor <mse...@redhat.com> + + PR sanitizer/65479 + * libsanitizer/sanitizer_common/sanitizer_stacktrace.h + (StackTrace::signaled, StackTrace::min_insn_bytes): New data members. + (StackTrace::StackTrace): Initialize signaled. + * libsanitizer/sanitizer_common/sanitizer_stacktrace.cc + (StackTrace::GetPreviousInstructionPc): Rewrite. + * libsanitizer/sanitizer_common/sanitizer_stacktrace_libcdep.cc + (StackTrace::Print): Use min_insn_bytes to adjust PC value. + (BufferedStackTrace::Unwind): Set signaled. + 2015-04-13 Yury Gribov <y.gri...@samsung.com> PR sanitizer/64839 diff --git a/libsanitizer/sanitizer_common/sanitizer_stacktrace.cc b/libsanitizer/sanitizer_common/sanitizer_stacktrace.cc index 9b99b5b..9b61b85 100644 --- a/libsanitizer/sanitizer_common/sanitizer_stacktrace.cc +++ b/libsanitizer/sanitizer_common/sanitizer_stacktrace.cc @@ -15,19 +15,33 @@ namespace __sanitizer { -uptr StackTrace::GetPreviousInstructionPc(uptr pc) { -#if defined(__arm__) - // Cancel Thumb bit. - pc = pc & (~1); -#endif -#if defined(__powerpc__) || defined(__powerpc64__) - // PCs are always 4 byte aligned. - return pc - 4; -#elif defined(__sparc__) || defined(__mips__) - return pc - 8; +const unsigned StackTrace::min_insn_bytes = +#if defined __ia64__ + // Intel Itanium has 5 byte instructions. + 5 +#elif defined AVR + 2 +#elif defined __i386__ || defined __x86_64__ || defined mc68000 + // Intel x86 and Motorola 68000 have variable instruction sets. + 1 +#elif defined __s390__ || defined __sh__ + // System 390 has 2 or 4 byte instructions. + // Hitachi SuperH is a 32-bit CPU with 16-bit instructions. + 2 #else - return pc - 1; + // Most instruction sets define instructions whose size in bytes + // matches that of the int type. I.e., 32-bit and 64-bit RISC + // CPUs typically have 32-bit instructions, while 16-bit RISC CPUs + // have 16 bit instructions. + sizeof (int) #endif + ; + +uptr StackTrace::GetPreviousInstructionPc(uptr pc) { + // Return a value that represents the address of either the first + // byte or some byte past the first byte of the instruction before + // the argument. + return pc - min_insn_bytes; } uptr StackTrace::GetCurrentPc() { diff --git a/libsanitizer/sanitizer_common/sanitizer_stacktrace.h b/libsanitizer/sanitizer_common/sanitizer_stacktrace.h index 31495cf..a01f676 100644 --- a/libsanitizer/sanitizer_common/sanitizer_stacktrace.h +++ b/libsanitizer/sanitizer_common/sanitizer_stacktrace.h @@ -39,9 +39,18 @@ static const uptr kStackTraceMax = 256; struct StackTrace { const uptr *trace; uptr size; - - StackTrace() : trace(nullptr), size(0) {} - StackTrace(const uptr *trace, uptr size) : trace(trace), size(size) {} + // Greater than zero of the stack trace was generated in response + // to a signal, -1 otherwise. + int signaled; + + // Minimum instruction size in bytes. Typically 4 on RISC processors + // and 1 on CISC. Used to advance the PC to the previous instruction + // in a stack trace and back. + static const unsigned min_insn_bytes; + + StackTrace() : trace(nullptr), size(0), signaled(-1) {} + StackTrace(const uptr *trace, uptr size, int signaled = -1) + : trace(trace), size(size), signaled(signaled) {} // Prints a symbolized stacktrace, followed by an empty line. void Print() const; diff --git a/libsanitizer/sanitizer_common/sanitizer_stacktrace_libcdep.cc b/libsanitizer/sanitizer_common/sanitizer_stacktrace_libcdep.cc index 2d55b73..7282d4b 100644 --- a/libsanitizer/sanitizer_common/sanitizer_stacktrace_libcdep.cc +++ b/libsanitizer/sanitizer_common/sanitizer_stacktrace_libcdep.cc @@ -29,9 +29,16 @@ void StackTrace::Print() const { InternalScopedString frame_desc(GetPageSizeCached() * 2); uptr frame_num = 0; for (uptr i = 0; i < size && trace[i]; i++) { - // PCs in stack traces are actually the return addresses, that is, - // addresses of the next instructions after the call. - uptr pc = GetPreviousInstructionPc(trace[i]); + // Except when the stack trace was generated in response to a signal + // and the frame is #0, decrementing the PC in the stack trace by + // the size of the smallest instruction helps find the expected + // corresponding line number in the debug info. This is because + // with the exception of frame #0 the PC saved on the stack is + // the return address, that is, the address of the next instructions + // after a branch and link or call instruction. + const uptr pc = (0 == i) && signaled ? + trace[i] : trace[i] - StackTrace::min_insn_bytes; + uptr addr_frames_num = Symbolizer::GetOrInit()->SymbolizePC( pc, addr_frames.data(), kMaxAddrFrames); if (addr_frames_num == 0) { @@ -40,6 +47,10 @@ void StackTrace::Print() const { } for (uptr j = 0; j < addr_frames_num; j++) { AddressInfo &info = addr_frames[j]; + if (i || !signaled) { + // Restore the PC to match the stack trace (see also PR65749). + info.address += StackTrace::min_insn_bytes; + } frame_desc.clear(); RenderFrame(&frame_desc, common_flags()->stack_trace_format, frame_num++, info, common_flags()->strip_path_prefix); @@ -54,6 +65,9 @@ void StackTrace::Print() const { void BufferedStackTrace::Unwind(uptr max_depth, uptr pc, uptr bp, void *context, uptr stack_top, uptr stack_bottom, bool request_fast_unwind) { + // Record that the stack trace was generated as a result of a signal. + signaled = 0 != context; + top_frame_bp = (max_depth > 0) ? bp : 0; // Avoid doing any work for small max_depth. if (max_depth == 0) {