Fix stepping past unwritable kernel helper on nios2-linux-gnu.
authorSandra Loosemore <sandra@codesourcery.com>
Thu, 28 Mar 2019 16:29:22 +0000 (09:29 -0700)
committerSandra Loosemore <sandra@codesourcery.com>
Thu, 28 Mar 2019 16:29:22 +0000 (09:29 -0700)
This patch fixes a problem on nios2-linux-gnu with stepping past the
kernel helper __kuser_cmpxchg, which was exposed by the testcase
gdb.threads/watchpoint-fork.exp.  The kernel maps this function into
user space on an unwritable page.  In this testcase, the cmpxchg
helper is invoked indirectly from the setbuf call in the test program.
Since this target lacks hardware breakpoint/watchpoint support, GDB
tries to single-step through the program by setting software
breakpoints, and was just giving an error when it reached the function
on the unwritable page.

The solution here is to always step over the call instead of stepping
into it; cmpxchg is supposed to be an atomic operation so this
behavior seems reasonable.  The hook in nios2_get_next_pc is somewhat
generic, but at present cmpxchg is the only helper provided by the
Linux kernel that is invoked by an ordinary function call.  (Signal
return trampolines also go through the unwritable page but not by a
function call.)

Fixing this issue also revealed that the testcase needs a much larger
timeout factor when software single-stepping is used.  That has also
been fixed in this patch.

gdb/ChangeLog

2019-03-28  Sandra Loosemore  <sandra@codesourcery.com>

        * nios2-tdep.h (struct gdbarch_tdep): Add is_kernel_helper.
        * nios2-tdep.c (nios2_get_next_pc): Skip over kernel helpers.
        * nios2-linux-tdep.c (nios2_linux_is_kernel_helper): New.
        (nios2_linux_init_abi): Install it.

gdb/testsuite/ChangeLog

2019-03-28  Sandra Loosemore  <sandra@codesourcery.com>

        * gdb.threads/watchpoint-fork.exp (test): Use large timeout
        factor when no hardware watchpoint support.

gdb/ChangeLog
gdb/nios2-linux-tdep.c
gdb/nios2-tdep.c
gdb/nios2-tdep.h
gdb/testsuite/ChangeLog
gdb/testsuite/gdb.threads/watchpoint-fork.exp

index 0bd86ee26dd1309669a7d688ee57a5b4c0b7cd5b..0b3ba3c785b05ff914841a5f8ade8a7ba9fd6097 100644 (file)
@@ -1,3 +1,10 @@
+2019-03-28  Sandra Loosemore  <sandra@codesourcery.com>
+
+       * nios2-tdep.h (struct gdbarch_tdep): Add is_kernel_helper.
+       * nios2-tdep.c (nios2_get_next_pc): Skip over kernel helpers.
+       * nios2-linux-tdep.c (nios2_linux_is_kernel_helper): New.
+       (nios2_linux_init_abi): Install it.
+
 2019-03-28  Alan Hayward  <alan.hayward@arm.com>
 
        * aarch64-tdep.c (aarch64_vnv_type): Use vector types.
index e4482d73f013a078caabeae8325d9f3c16fffdc9..2d38e603c068a98d503849aeed96268da132e6be 100644 (file)
@@ -200,6 +200,17 @@ nios2_linux_syscall_next_pc (struct frame_info *frame,
   return pc + op->size;
 }
 
+/* Return true if PC is a kernel helper, a function mapped by the kernel
+   into user space on an unwritable page.  Currently the only such function
+   is __kuser_cmpxchg at 0x1004.  See arch/nios2/kernel/entry.S in the Linux
+   kernel sources and sysdeps/unix/sysv/linux/nios2/atomic-machine.h in
+   GLIBC.  */
+static bool
+nios2_linux_is_kernel_helper (CORE_ADDR pc)
+{
+  return pc == 0x1004;
+}
+
 /* Hook function for gdbarch_register_osabi.  */
 
 static void
@@ -230,6 +241,7 @@ nios2_linux_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
                                  &nios2_r1_linux_rt_sigreturn_tramp_frame);
 
   tdep->syscall_next_pc = nios2_linux_syscall_next_pc;
+  tdep->is_kernel_helper = nios2_linux_is_kernel_helper;
 
   /* Index of target address word in glibc jmp_buf.  */
   tdep->jb_pc = 10;
index ee45db98af634a9d705947c2545cbed428adb7df..08664541133dc9aebac00147ebc75195c43f5f24 100644 (file)
@@ -2169,13 +2169,32 @@ nios2_get_next_pc (struct regcache *regcache, CORE_ADDR pc)
        }
     }
 
-  else if (nios2_match_jmpi (insn, op, mach, &uimm)
-          || nios2_match_calli (insn, op, mach, &uimm))
+  else if (nios2_match_jmpi (insn, op, mach, &uimm))
     pc = (pc & 0xf0000000) | uimm;
+  else if (nios2_match_calli (insn, op, mach, &uimm))
+    {
+      CORE_ADDR callto = (pc & 0xf0000000) | uimm;
+      if (tdep->is_kernel_helper != NULL
+         && tdep->is_kernel_helper (callto))
+       /* Step over call to kernel helper, which we cannot debug
+          from user space.  */
+       pc += op->size;
+      else
+       pc = callto;
+    }
 
-  else if (nios2_match_jmpr (insn, op, mach, &ra)
-          || nios2_match_callr (insn, op, mach, &ra))
+  else if (nios2_match_jmpr (insn, op, mach, &ra))
     pc = regcache_raw_get_unsigned (regcache, ra);
+  else if (nios2_match_callr (insn, op, mach, &ra))
+    {
+      CORE_ADDR callto = regcache_raw_get_unsigned (regcache, ra);
+      if (tdep->is_kernel_helper != NULL
+         && tdep->is_kernel_helper (callto))
+       /* Step over call to kernel helper.  */
+       pc += op->size;
+      else
+       pc = callto;
+    }
 
   else if (nios2_match_ldwm (insn, op, mach, &uimm, &ra, &imm, &wb, &id, &ret)
           && ret)
index a2f0163ac515b2ede7fa8b38f1605c40c891b6bd..275dcd74aeb53acbf8b00f64f481156df8fe3c58 100644 (file)
@@ -74,6 +74,9 @@ struct gdbarch_tdep
   CORE_ADDR (*syscall_next_pc) (struct frame_info *frame,
                                const struct nios2_opcode *op);
 
+  /* Returns true if PC points to a kernel helper function.  */
+  bool (*is_kernel_helper) (CORE_ADDR pc);
+
   /* Offset to PC value in jump buffer.
      If this is negative, longjmp support will be disabled.  */
   int jb_pc;
index ba97fc392570e4889bb2f2ec93680b8ee3d510f9..9f0065b016ab2da03a9bc7a384c52e5e0b009b32 100644 (file)
@@ -1,3 +1,8 @@
+2019-03-28  Sandra Loosemore  <sandra@codesourcery.com>
+
+       * gdb.threads/watchpoint-fork.exp (test): Use large timeout
+       factor when no hardware watchpoint support.
+
 2019-03-28  Alan Hayward  <alan.hayward@arm.com>
            Pedro Alves  <palves@redhat.com>
 
index 878f784b7e444520bb50805811bf53943fa12735..49a6167ed15088cd8281be993faef3f3aaf83018 100644 (file)
@@ -39,6 +39,12 @@ proc test {type symbol} {
            if [target_info exists gdb,no_hardware_watchpoints] {
                # The software watchpoint functionality is in GDB an unrelated test.
                gdb_test_no_output "set can-use-hw-watchpoints 0"
+               # Software watchpoints can be quite slow on remote targets
+               # on this test because it ends up single-stepping through
+               # code to initialize dynamic libraries, etc.
+               set factor 20
+           } else {
+               set factor 1
            }
 
            gdb_test "show detach-on-fork" "Whether gdb will detach the child of a fork is on\\."
@@ -63,19 +69,21 @@ proc test {type symbol} {
 
            gdb_breakpoint "mark_exit"
 
-           gdb_test "continue" \
-               "reakpoint \[0-9\]+, marker.*" "hardware breakpoints work"
-           gdb_test "continue" \
-               "atchpoint \[0-9\]+: var.*Old value = 0.*New value = 1.*forkoff *\\(1\\).*" "watchpoints work"
-           gdb_test "continue" \
-               "reakpoint \[0-9\]+, marker.*" "breakpoint after the first fork"
-           gdb_test "continue" \
-               "atchpoint \[0-9\]+: var.*Old value = 1.*New value = 2.*forkoff *\\(2\\).*" "watchpoint after the first fork"
-           gdb_test "continue" \
-               "reakpoint \[0-9\]+, marker.*" "breakpoint after the second fork"
-           gdb_test "continue" \
-               "atchpoint \[0-9\]+: var.*Old value = 2.*New value = 3.*mark_exit \\(\\);" "watchpoint after the second fork"
-           gdb_test "continue" "Continuing\\..*\r\n(Thread .* hit )?Breakpoint \[0-9\]+, mark_exit .*" "finish"
+           with_timeout_factor $factor {
+               gdb_test "continue" \
+                   "reakpoint \[0-9\]+, marker.*" "hardware breakpoints work"
+               gdb_test "continue" \
+                   "atchpoint \[0-9\]+: var.*Old value = 0.*New value = 1.*forkoff *\\(1\\).*" "watchpoints work"
+               gdb_test "continue" \
+                   "reakpoint \[0-9\]+, marker.*" "breakpoint after the first fork"
+               gdb_test "continue" \
+                   "atchpoint \[0-9\]+: var.*Old value = 1.*New value = 2.*forkoff *\\(2\\).*" "watchpoint after the first fork"
+               gdb_test "continue" \
+                   "reakpoint \[0-9\]+, marker.*" "breakpoint after the second fork"
+               gdb_test "continue" \
+                   "atchpoint \[0-9\]+: var.*Old value = 2.*New value = 3.*mark_exit \\(\\);" "watchpoint after the second fork"
+               gdb_test "continue" "Continuing\\..*\r\n(Thread .* hit )?Breakpoint \[0-9\]+, mark_exit .*" "finish"
+           }
        }
 
        # threads