[Bug gdb/27338] New: Stepping over a breakpoint on an instruction that makes the thread exit is not correctly handled

[Bug gdb/27338] New: Stepping over a breakpoint on an instruction that makes the thread exit is not correctly handled

[simark at simark dot ca]

https://sourceware.org/bugzilla/show_bug.cgi?id=27338

            Bug ID: 27338
           Summary: Stepping over a breakpoint on an instruction that
                    makes the thread exit is not correctly handled
           Product: gdb
           Version: HEAD
            Status: NEW
          Severity: normal
          Priority: P2
         Component: gdb
          Assignee: unassigned at sourceware dot org
          Reporter: simark at simark dot ca
  Target Milestone: ---

By default, targets don't report thread exit events to infrun, because infrun
generally doesn't need to know about that.  However, if the exiting thread was
doing a step-over (in-line or displaced), the step-over is never completed.  In
the case of an in-line step-over, the step over info is never cleared, meaning
infurn still thinks the thread is doing its in-line step, and that prevents
anything else from running.  In the case of a displaced step, the displaced
step buffer stays marked as used and is never freed for another thread to use,
which leads to a similar deadlock.

I'll attach a test case that reproduces the issue.

A possible solution is for targets to voluntarily report thread exit events
when the exiting thread was being stepped.  Infrun would then have a chance to
do the appropriate cleanup, and that wouldn't require enabling thread events
for the whole target.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug gdb/27338] Stepping over a breakpoint on an instruction that makes the thread exit is not correctly handled

[simark at simark dot ca]

https://sourceware.org/bugzilla/show_bug.cgi?id=27338

--- Comment #1 from Simon Marchi <simark at simark dot ca> ---
Created attachment 13199
  --> https://sourceware.org/bugzilla/attachment.cgi?id=13199&action=edit
Test case

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug gdb/27338] Stepping over a breakpoint on an instruction that makes the thread exit is not correctly handled

[simark at simark dot ca]

https://sourceware.org/bugzilla/show_bug.cgi?id=27338

--- Comment #2 from Simon Marchi <simark at simark dot ca> ---
A corner case I can think of is if:

- A thread (1) is doing a displaced step over an instruction that will cause it
to exit
- Meanwhile, another thread (2) hits a breakpoint that causes a stop, and
causes stop_all_threads to be called
- The thread exit event for thread 1 is reported to stop_all_threads

We need to handle that correctly.  We already call displaced_step_finish if the
event is TARGET_WAITKIND_STOPPED, we should probably call also in the
TARGET_WAITKIND_THREAD_EXITED case.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug gdb/27338] Stepping over a breakpoint on an instruction that makes the thread exit is not correctly handled

[pedro at palves dot net]

https://sourceware.org/bugzilla/show_bug.cgi?id=27338

Pedro Alves <pedro at palves dot net> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |pedro at palves dot net
           Assignee|unassigned at sourceware dot org   |pedro at palves dot net
             Status|NEW                         |ASSIGNED

--- Comment #3 from Pedro Alves <pedro at palves dot net> ---
I've posted a series that fixes it, here:
 [PATCH 00/25] Step over thread clone and thread exit
 https://sourceware.org/pipermail/gdb-patches/2022-June/190181.html

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug gdb/27338] Stepping over a breakpoint on an instruction that makes the thread exit is not correctly handled

[cvs-commit at gcc dot gnu.org]

https://sourceware.org/bugzilla/show_bug.cgi?id=27338

--- Comment #4 from cvs-commit at gcc dot gnu.org <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Pedro Alves <palves@sourceware.org>:

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

commit 58c010877e77ad9dd4e14272f9500d40b75e7429
Author: Pedro Alves <pedro@palves.net>
Date:   Tue Jun 22 15:42:51 2021 +0100

    displaced step: pass down target_waitstatus instead of gdb_signal

    This commit tweaks displaced_step_finish & friends to pass down a
    target_waitstatus instead of a gdb_signal.  This is needed because a
    patch later in the step-over-{thread-exit,clone] series will want to
    make displaced_step_buffers::finish handle
    TARGET_WAITKIND_THREAD_EXITED.  It also helps with the
    TARGET_WAITKIND_THREAD_CLONED patch later in that same series.

    It's also a bit more logical this way, as we don't have to pass down
    signals when the thread didn't actually stop for a signal.  So we can
    also think of it as a clean up.

    Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=27338
    Change-Id: I4c5d338647b028071bc498c4e47063795a2db4c0
    Approved-By: Andrew Burgess <aburgess@redhat.com>

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug gdb/27338] Stepping over a breakpoint on an instruction that makes the thread exit is not correctly handled

[cvs-commit at gcc dot gnu.org]

https://sourceware.org/bugzilla/show_bug.cgi?id=27338

--- Comment #5 from cvs-commit at gcc dot gnu.org <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Pedro Alves <palves@sourceware.org>:

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

commit d8d96409c83f84f5c15d2e846a7c0a59ceed187c
Author: Pedro Alves <pedro@palves.net>
Date:   Thu Mar 17 19:25:03 2022 +0000

    Introduce GDB_THREAD_OPTION_EXIT thread option, fix step-over-thread-exit

    When stepping over a breakpoint with displaced stepping, GDB needs to
    be informed if the stepped thread exits, otherwise the displaced
    stepping buffer that was allocated to that thread leaks, and this can
    result in deadlock, with other threads waiting for their turn to
    displaced step, but their turn never comes.

    Similarly, when stepping over a breakpoint in line, GDB also needs to
    be informed if the stepped thread exits, so that is can clear the step
    over state and re-resume threads.

    This commit makes it possible for GDB to ask the target to report
    thread exit events for a given thread, using the new "thread options"
    mechanism introduced by a previous patch.

    This only adds the core bits.  Following patches in the series will
    teach the Linux backends (native & gdbserver) to handle the
    GDB_THREAD_OPTION_EXIT option, and then a later patch will make use of
    these thread exit events to clean up displaced stepping and inline
    stepping state properly.

    Reviewed-By: Andrew Burgess <aburgess@redhat.com>
    Change-Id: I96b719fdf7fee94709e98bb3a90751d8134f3a38
    Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=27338

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug gdb/27338] Stepping over a breakpoint on an instruction that makes the thread exit is not correctly handled

[cvs-commit at gcc dot gnu.org]

https://sourceware.org/bugzilla/show_bug.cgi?id=27338

--- Comment #6 from cvs-commit at gcc dot gnu.org <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Pedro Alves <palves@sourceware.org>:

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

commit 4898949800206894305b954e10a9a8c36e2d08c7
Author: Pedro Alves <pedro@palves.net>
Date:   Thu Mar 17 19:25:03 2022 +0000

    Implement GDB_THREAD_OPTION_EXIT support for Linux GDBserver

    This implements support for the new GDB_THREAD_OPTION_EXIT thread
    option for Linux GDBserver.

    Reviewed-By: Andrew Burgess <aburgess@redhat.com>
    Change-Id: I96b719fdf7fee94709e98bb3a90751d8134f3a38
    Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=27338

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug gdb/27338] Stepping over a breakpoint on an instruction that makes the thread exit is not correctly handled

[cvs-commit at gcc dot gnu.org]

https://sourceware.org/bugzilla/show_bug.cgi?id=27338

--- Comment #7 from cvs-commit at gcc dot gnu.org <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Pedro Alves <palves@sourceware.org>:

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

commit a51e14efe29d64a966161837e991193efb2e2457
Author: Pedro Alves <pedro@palves.net>
Date:   Mon Mar 21 19:09:13 2022 +0000

    Implement GDB_THREAD_OPTION_EXIT support for native Linux

    This implements support for the new GDB_THREAD_OPTION_EXIT thread
    option for native Linux.

    Reviewed-By: Andrew Burgess <aburgess@redhat.com>
    Change-Id: Ia69fc0b9b96f9af7de7cefc1ddb1fba9bbb0bb90
    Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=27338

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug gdb/27338] Stepping over a breakpoint on an instruction that makes the thread exit is not correctly handled

[cvs-commit at gcc dot gnu.org]

https://sourceware.org/bugzilla/show_bug.cgi?id=27338

--- Comment #8 from cvs-commit at gcc dot gnu.org <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Pedro Alves <palves@sourceware.org>:

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

commit 21d4830415e38880bd6babf6a4a3f46e682d3b9c
Author: Pedro Alves <pedro@palves.net>
Date:   Fri Feb 5 16:42:32 2021 -0500

    gdb: clear step over information on thread exit (PR gdb/27338)

    GDB doesn't handle correctly the case where a thread steps over a
    breakpoint (using either in-line or displaced stepping), and the
    executed instruction causes the thread to exit.

    Using the test program included later in the series, this is what it
    looks like with displaced-stepping, on x86-64 Linux, where we have two
    displaced-step buffers:

      $ ./gdb -q -nx --data-directory=data-directory
build/binutils-gdb/gdb/testsuite/outputs/gdb.threads/step-over-thread-exit/step-over-thread-exit
-ex "b my_exit_syscall" -ex r
      Reading symbols from
build/binutils-gdb/gdb/testsuite/outputs/gdb.threads/step-over-thread-exit/step-over-thread-exit...
      Breakpoint 1 at 0x123c: file
src/binutils-gdb/gdb/testsuite/lib/my-syscalls.S, line 68.
      Starting program:
build/binutils-gdb/gdb/testsuite/outputs/gdb.threads/step-over-thread-exit/step-over-thread-exit
      [Thread debugging using libthread_db enabled]
      Using host libthread_db library "/usr/lib/../lib/libthread_db.so.1".
      [New Thread 0x7ffff7c5f640 (LWP 2915510)]
      [Switching to Thread 0x7ffff7c5f640 (LWP 2915510)]

      Thread 2 "step-over-threa" hit Breakpoint 1, my_exit_syscall () at
src/binutils-gdb/gdb/testsuite/lib/my-syscalls.S:68
      68              syscall
      (gdb) c
      Continuing.
      [New Thread 0x7ffff7c5f640 (LWP 2915524)]
      [Thread 0x7ffff7c5f640 (LWP 2915510) exited]
      [Switching to Thread 0x7ffff7c5f640 (LWP 2915524)]

      Thread 3 "step-over-threa" hit Breakpoint 1, my_exit_syscall () at
src/binutils-gdb/gdb/testsuite/lib/my-syscalls.S:68
      68              syscall
      (gdb) c
      Continuing.
      [New Thread 0x7ffff7c5f640 (LWP 2915616)]
      [Thread 0x7ffff7c5f640 (LWP 2915524) exited]
      [Switching to Thread 0x7ffff7c5f640 (LWP 2915616)]

      Thread 4 "step-over-threa" hit Breakpoint 1, my_exit_syscall () at
src/binutils-gdb/gdb/testsuite/lib/my-syscalls.S:68
      68              syscall
      (gdb) c
      Continuing.
      ... hangs ...

    The first two times we do "continue", we displaced-step the syscall
    instruction that causes the thread to exit.  When the thread exits,
    the main thread, waiting on pthread_join, is unblocked.  It spawns a
    new thread, which hits the breakpoint on the syscall again.  However,
    infrun was never notified that the displaced-stepping threads are done
    using the displaced-step buffer, so now both buffers are marked as
    used.  So when we do the third continue, there are no buffers
    available to displaced-step the syscall, so the thread waits forever
    for its turn.

    When trying the same but with in-line step over (displaced-stepping
    disabled):

      $ ./gdb -q -nx --data-directory=data-directory \
     
build/binutils-gdb/gdb/testsuite/outputs/gdb.threads/step-over-thread-exit/step-over-thread-exit
\
        -ex "b my_exit_syscall" -ex "set displaced-stepping off" -ex r
      Reading symbols from
build/binutils-gdb/gdb/testsuite/outputs/gdb.threads/step-over-thread-exit/step-over-thread-exit...
      Breakpoint 1 at 0x123c: file
src/binutils-gdb/gdb/testsuite/lib/my-syscalls.S, line 68.
      Starting program:
build/binutils-gdb/gdb/testsuite/outputs/gdb.threads/step-over-thread-exit/step-over-thread-exit
      [Thread debugging using libthread_db enabled]
      Using host libthread_db library "/usr/lib/../lib/libthread_db.so.1".
      [New Thread 0x7ffff7c5f640 (LWP 2928290)]
      [Switching to Thread 0x7ffff7c5f640 (LWP 2928290)]

      Thread 2 "step-over-threa" hit Breakpoint 1, my_exit_syscall () at
src/binutils-gdb/gdb/testsuite/lib/my-syscalls.S:68
      68              syscall
      (gdb) c
      Continuing.
      [Thread 0x7ffff7c5f640 (LWP 2928290) exited]
      No unwaited-for children left.
      (gdb) i th
        Id   Target Id                                             Frame
        1    Thread 0x7ffff7c60740 (LWP 2928285) "step-over-threa"
0x00007ffff7f7c9b7 in __pthread_clockjoin_ex () from /usr/lib/libpthread.so.0

      The current thread <Thread ID 2> has terminated.  See `help thread'.
      (gdb) thread 1
      [Switching to thread 1 (Thread 0x7ffff7c60740 (LWP 2928285))]
      #0  0x00007ffff7f7c9b7 in __pthread_clockjoin_ex () from
/usr/lib/libpthread.so.0
      (gdb) c
      Continuing.
      ^C^C
      ... hangs ...

    The "continue" causes an in-line step to occur, meaning the main
    thread is stopped while we step the syscall.  The stepped thread exits
    when executing the syscall, the linux-nat target notices there are no
    more resumed threads to be waited for, so returns
    TARGET_WAITKIND_NO_RESUMED, which causes the prompt to return.  But
    infrun never clears the in-line step over info.  So if we try
    continuing the main thread, GDB doesn't resume it, because it thinks
    there's an in-line step in progress that we need to wait for to
    finish, and we are stuck there.

    To fix this, infrun needs to be informed when a thread doing a
    displaced or in-line step over exits.  We can do that with the new
    target_set_thread_options mechanism which is optimal for only enabling
    exit events of the thread that needs it; or, if that is not supported,
    by using target_thread_events, which enables thread exit events for
    all threads.  This is done by this commit.

    This patch then modifies handle_inferior_event in infrun.c to clean up
    any step-over the exiting thread might have been doing at the time of
    the exit.  The cases to consider are:

     - the exiting thread was doing an in-line step-over with an all-stop
       target
     - the exiting thread was doing an in-line step-over with a non-stop
       target
     - the exiting thread was doing a displaced step-over with a non-stop
       target

    The displaced-stepping buffer implementation in displaced-stepping.c
    is modified to account for the fact that it's possible that we
    "finish" a displaced step after a thread exit event.  The buffer that
    the exiting thread was using is marked as available again and the
    original instructions under the scratch pad are restored.  However, it
    skips applying the fixup, which wouldn't make sense since the thread
    does not exist anymore.

    Another case that needs handling is if a displaced-stepping thread
    exits, and the event is reported while we are in stop_all_threads.  We
    should call displaced_step_finish in the handle_one function, in that
    case.  It was already called in other code paths, just not the "thread
    exited" path.

    This commit doesn't make infrun ask the target to report the
    TARGET_WAITKIND_THREAD_EXITED events yet, that'll be done later in the
    series.

    Note that "stop_print_frame = false;" line is moved to normal_stop,
    because TARGET_WAITKIND_THREAD_EXITED can also end up with the event
    transmorphed into TARGET_WAITKIND_NO_RESUMED.  Moving it to
    normal_stop keeps it centralized.

    Co-authored-by: Simon Marchi <simon.marchi@efficios.com>
    Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=27338
    Reviewed-By: Andrew Burgess <aburgess@redhat.com>
    Change-Id: I745c6955d7ef90beb83bcf0ff1d1ac8b9b6285a5

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug gdb/27338] Stepping over a breakpoint on an instruction that makes the thread exit is not correctly handled

[cvs-commit at gcc dot gnu.org]

https://sourceware.org/bugzilla/show_bug.cgi?id=27338

--- Comment #9 from cvs-commit at gcc dot gnu.org <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Pedro Alves <palves@sourceware.org>:

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

commit 7c6cb899c759fb00dc1a95a2ff904220fca1a9dc
Author: Simon Marchi <simon.marchi@efficios.com>
Date:   Fri Feb 5 16:42:32 2021 -0500

    Testcases for stepping over thread exit syscall (PR gdb/27338)

    Add new gdb.threads/step-over-thread-exit.exp and
    gdb.threads/step-over-thread-exit-while-stop-all-threads.exp
    testcases, exercising stepping over thread exit syscall.  These make
    use of lib/my-syscalls.S to define the exit syscall.

    Co-authored-by: Pedro Alves <pedro@palves.net>
    Reviewed-By: Andrew Burgess <aburgess@redhat.com>
    Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=27338
    Change-Id: Ie8b2c5747db99b7023463a897a8390d9e814a9c9

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug gdb/27338] Stepping over a breakpoint on an instruction that makes the thread exit is not correctly handled

[pedro at palves dot net]

https://sourceware.org/bugzilla/show_bug.cgi?id=27338

Pedro Alves <pedro at palves dot net> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |FIXED
   Target Milestone|---                         |15.1
             Status|ASSIGNED                    |RESOLVED

--- Comment #10 from Pedro Alves <pedro at palves dot net> ---
Fixed in master.

-- 
You are receiving this mail because:
You are on the CC list for the bug.