This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
[PATCH] 65479 - sanitizer stack trace missing frames past #0 on powerpc64
- From: Martin Sebor <msebor at redhat dot com>
- To: Gcc Patch List <gcc-patches at gcc dot gnu dot org>
- Date: Sun, 19 Apr 2015 19:48:03 -0600
- Subject: [PATCH] 65479 - sanitizer stack trace missing frames past #0 on powerpc64
- Authentication-results: sourceware.org; auth=none
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 <msebor@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 <msebor@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 <msebor@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 <doko@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 <msebor@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.gribov@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) {