[binutils-gdb] gdb/arm: Fix backtrace for pthread_cond_timedwait

[binutils-gdb] gdb/arm: Fix backtrace for pthread_cond_timedwait

[Jan Kratochvil]

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=3026cdbdde0e1937f811b52ba18fe3fbb1419ef9

commit 3026cdbdde0e1937f811b52ba18fe3fbb1419ef9
Author: Jan Kratochvil <jan.kratochvil@redhat.com>
Date:   Sat Apr 1 15:42:57 2023 +0200

    gdb/arm: Fix backtrace for pthread_cond_timedwait
    
    GDB expected PC should point right after the SVC instruction when the
    syscall is active. But some active syscalls keep PC pointing to the SVC
    instruction itself.
    
    This leads to a broken backtrace like:
     Backtrace stopped: previous frame identical to this frame (corrupt stack?)
     #0  0xb6f8681c in pthread_cond_timedwait@@GLIBC_2.4 () from /lib/arm-linux-gnueabihf/libpthread.so.0
     #1  0xb6e21f80 in ?? ()
    
    The reason is that .ARM.exidx unwinder gives up if PC does not point
    right after the SVC (syscall) instruction. I did not investigate why but
    some syscalls will point PC to the SVC instruction itself. This happens
    for the "futex" syscall used by pthread_cond_timedwait.
    
    That normally does not matter as ARM prologue unwinder gets called
    instead of the .ARM.exidx one. Unfortunately some glibc calls have more
    complicated prologue where the GDB unwinder fails to properly determine
    the return address (that is in fact an orthogonal GDB bug). I expect it
    is due to the "vpush" there in this case but I did not investigate it more:
    
    Dump of assembler code for function pthread_cond_timedwait@@GLIBC_2.4:
       0xb6f8757c <+0>:     push    {r4, r5, r6, r7, r8, r9, r10, r11, lr}
       0xb6f87580 <+4>:     mov     r10, r2
       0xb6f87584 <+8>:     vpush   {d8}
    
    Regression tested on armv7l kernel 5.15.32-v7l+ (Raspbian 11).
    
    Approved-By: Luis Machado <luis.machado@arm.com>

Diff:
---
 gdb/arm-tdep.c                                     |  42 +++++---
 .../gdb.arch/arm-pthread_cond_timedwait-bt.c       |  67 ++++++++++++
 .../gdb.arch/arm-pthread_cond_timedwait-bt.exp     | 119 +++++++++++++++++++++
 3 files changed, 211 insertions(+), 17 deletions(-)

diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c
index 803596d0fe6..492a71fa987 100644
--- a/gdb/arm-tdep.c
+++ b/gdb/arm-tdep.c
@@ -3115,26 +3115,34 @@ arm_exidx_unwind_sniffer (const struct frame_unwind *self,
 	  && get_frame_type (get_next_frame (this_frame)) == NORMAL_FRAME)
 	exc_valid = 1;
 
-      /* We also assume exception information is valid if we're currently
-	 blocked in a system call.  The system library is supposed to
-	 ensure this, so that e.g. pthread cancellation works.  */
-      if (arm_frame_is_thumb (this_frame))
+      /* Some syscalls keep PC pointing to the SVC instruction itself.  */
+      for (int shift = 0; shift <= 1 && !exc_valid; ++shift)
 	{
-	  ULONGEST insn;
+	  /* We also assume exception information is valid if we're currently
+	     blocked in a system call.  The system library is supposed to
+	     ensure this, so that e.g. pthread cancellation works.  */
+	  if (arm_frame_is_thumb (this_frame))
+	    {
+	      ULONGEST insn;
 
-	  if (safe_read_memory_unsigned_integer (get_frame_pc (this_frame) - 2,
-						 2, byte_order_for_code, &insn)
-	      && (insn & 0xff00) == 0xdf00 /* svc */)
-	    exc_valid = 1;
-	}
-      else
-	{
-	  ULONGEST insn;
+	      if (safe_read_memory_unsigned_integer ((get_frame_pc (this_frame)
+						      - (shift ? 2 : 0)),
+						     2, byte_order_for_code,
+						     &insn)
+		  && (insn & 0xff00) == 0xdf00 /* svc */)
+		exc_valid = 1;
+	    }
+	  else
+	    {
+	      ULONGEST insn;
 
-	  if (safe_read_memory_unsigned_integer (get_frame_pc (this_frame) - 4,
-						 4, byte_order_for_code, &insn)
-	      && (insn & 0x0f000000) == 0x0f000000 /* svc */)
-	    exc_valid = 1;
+	      if (safe_read_memory_unsigned_integer ((get_frame_pc (this_frame)
+						      - (shift ? 4 : 0)),
+						     4, byte_order_for_code,
+						     &insn)
+		  && (insn & 0x0f000000) == 0x0f000000 /* svc */)
+		exc_valid = 1;
+	    }
 	}
 	
       /* Bail out if we don't know that exception information is valid.  */
diff --git a/gdb/testsuite/gdb.arch/arm-pthread_cond_timedwait-bt.c b/gdb/testsuite/gdb.arch/arm-pthread_cond_timedwait-bt.c
new file mode 100644
index 00000000000..4ee46238d56
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/arm-pthread_cond_timedwait-bt.c
@@ -0,0 +1,67 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2023 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#include <pthread.h>
+#include <sys/time.h>
+#include <unistd.h>
+#include <assert.h>
+
+static pthread_mutex_t mutex = PTHREAD_MUTEX_INITIALIZER;
+static pthread_cond_t cond = PTHREAD_COND_INITIALIZER;
+
+static void *fun(void *arg)
+{
+  struct timeval now;
+  struct timespec until;
+  int err;
+
+  err = gettimeofday(&now, NULL);
+  assert(!err);
+
+  until.tv_sec = now.tv_sec + 60;
+  until.tv_nsec = now.tv_usec * 1000UL;
+
+  pthread_cond_timedwait(&cond, &mutex, &until);
+  assert(0);
+  err = pthread_mutex_unlock(&mutex);
+  assert(!err);
+
+  return arg;
+}
+
+void breakhere()
+{
+}
+
+int main()
+{
+  pthread_t thread;
+  void *ret;
+  int err;
+
+  err = pthread_mutex_lock(&mutex);
+  assert(!err);
+  err = pthread_create(&thread, NULL, fun, NULL);
+  assert(!err);
+  err = pthread_mutex_lock(&mutex);
+  assert(!err);
+  breakhere();
+  err = pthread_join(thread, &ret);
+  assert(0);
+
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.arch/arm-pthread_cond_timedwait-bt.exp b/gdb/testsuite/gdb.arch/arm-pthread_cond_timedwait-bt.exp
new file mode 100644
index 00000000000..0cf7f273b39
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/arm-pthread_cond_timedwait-bt.exp
@@ -0,0 +1,119 @@
+# Copyright 2023 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# GDB expected PC should point right after the SVC instruction when the
+# syscall is active. But some active syscalls keep PC pointing to the SVC
+# instruction itself.
+
+standard_testfile
+
+if { [prepare_for_testing "failed to prepare" ${testfile} ${srcfile} \
+			  {debug pthreads}] } {
+    return
+}
+
+# See if we have target board readnow.exp or similar.
+if { [lsearch -exact $GDBFLAGS -readnow] != -1 \
+         || [lsearch -exact $GDBFLAGS --readnow] != -1 } {
+    untested "--readnever not allowed in combination with --readnow"
+    return -1
+}
+
+save_vars { GDBFLAGS } {
+    append GDBFLAGS " --readnever"
+    if { [clean_restart ${binfile}] == -1 } {
+       return -1
+    }
+}
+
+if { ![runto_main] } {
+    return
+}
+
+gdb_test "advance breakhere" " in breakhere .*"
+
+gdb_test "thread 2" "Switching to thread 2 .*" "thread 2 for svc check"
+
+# GDB expected PC should point right after the SVC instruction when the syscall is active.
+# But some active syscalls keep PC pointing to the SVC instruction itself.
+set test "pc points to svc"
+gdb_test_multiple {x/i $pc} $test {
+    -re ":\tsvc\t(0x00000000|0)\r\n$gdb_prompt $" {
+	pass $test
+    }
+    -re "\r\n$gdb_prompt $" {
+	untested $test
+	return
+    }
+}
+
+gdb_test "thread 1" "Switching to thread 1 .*"
+gdb_test_no_output "set debug frame 1"
+
+# PASS:
+#  [frame] frame_unwind_try_unwinder: trying unwinder "arm exidx"
+#  [frame] frame_unwind_register_value: enter
+#...
+#  [frame] frame_unwind_register_value: exit
+#  [frame] frame_unwind_try_unwinder: yes
+#...
+#  [frame] get_prev_frame_always_1:   -> {level=0,type=NORMAL_FRAME,unwinder="arm exidx",pc=0xb6f8681c,id=<not computed>,func=<unknown>} // cached
+
+# FAIL:
+#  [frame] frame_unwind_try_unwinder: trying unwinder "arm exidx"
+#  [frame] frame_unwind_register_value: enter
+#...
+#  [frame] frame_unwind_register_value: exit
+#  [frame] frame_unwind_try_unwinder: no
+#  [frame] frame_unwind_try_unwinder: trying unwinder "arm epilogue"
+#  [frame] frame_unwind_register_value: enter
+#...
+#  [frame] frame_unwind_register_value: exit
+#  [frame] frame_unwind_try_unwinder: no
+#  [frame] frame_unwind_try_unwinder: trying unwinder "arm prologue"
+#  [frame] frame_unwind_try_unwinder: yes
+#...
+#  [frame] get_prev_frame_always_1:   -> {level=0,type=NORMAL_FRAME,unwinder="arm prologue",pc=0xb6f8681c,id=<not computed>,func=<unknown>} // cached
+
+set test "unwinder is arm exidx"
+# Switch the threads to reset frame cache.
+gdb_test_multiple {thread 2} $test {
+    -re "\{level=0,type=NORMAL_FRAME,unwinder=\"arm exidx\",pc=.*\r\n$gdb_prompt $" {
+	pass $test
+    }
+    -re "\{level=0,type=NORMAL_FRAME,unwinder=\"arm prologue\",pc=.*\r\n$gdb_prompt $" {
+	fail $test
+    }
+    -re "\r\n$gdb_prompt $" {
+	untested $test
+	return
+    }
+}
+
+gdb_test "thread 2" "Switching to thread 2 .*" "thread 2 for debug frame check"
+
+gdb_test_no_output "set debug frame 0"
+
+# PASS:
+# #0  0xb6f8681c in pthread_cond_timedwait@@GLIBC_2.4 () from /lib/arm-linux-gnueabihf/libpthread.so.0
+# #1  0x00010648 in fun (arg=0x0) at .../gdb/testsuite/gdb.arch/arm-pthread_cond_timedwait-bt.c:38
+# ...
+
+# FAIL:
+# #0  0xb6f8681c in pthread_cond_timedwait@@GLIBC_2.4 () from /lib/arm-linux-gnueabihf/libpthread.so.0
+# #1  0xb6e21f80 in ?? ()
+# Backtrace stopped: previous frame identical to this frame (corrupt stack?)
+
+gdb_test "bt" { in fun \(\).*} "unwind of pthread_cond_timedwait"