Re: [patch, nios2] fix stepping past unwritable kernel helper

[Kevin Buettner <kevinb at redhat dot com>]

On Wed, 27 Mar 2019 22:47:44 -0600
Sandra Loosemore <sandra@codesourcery.com> wrote:

>     Fix stepping past unwritable kernel helper on nios2-linux-gnu.
>     
>     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-27  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-27  Sandra Loosemore  <sandra@codesourcery.com>
>     
>     	* gdb.threads/watchpoint-fork.exp (test): Use large timeout
>     	factor when no hardware watchpoint support.

Thanks for the explanation, above.

The one thing that concerned me when I first saw it was the use of the
constant 0x1004 as the address for __kuser_cmpxchg.  But after checking
the glibc sources at the path indicated in the comment, I saw that it's
hardcoded using the same constant there as well.

So... this patch is okay.

Kevin