[all-commits] [llvm/llvm-project] 87fadb: [lldb] Correctly annotate threads at a bp site as ...

[Jason Molenda via All-commits]

  Branch: refs/heads/main
  Home:   https://github.com/llvm/llvm-project
  Commit: 87fadb3929163752f650a1fc08d5fb13ee4c1a3f
      https://github.com/llvm/llvm-project/commit/87fadb3929163752f650a1fc08d5fb13ee4c1a3f
  Author: Jason Molenda <jmolenda at apple.com>
  Date:   2024-02-23 (Fri, 23 Feb 2024)

  Changed paths:
    M lldb/include/lldb/Target/Thread.h
    M lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp

  Log Message:
  -----------
  [lldb] Correctly annotate threads at a bp site as hitting it (#82709)

This is next in my series of "fix the racey tests that fail on
greendragon" addressing the failure of TestConcurrentManyBreakpoints.py
where we set a breakpoint in a function that 100 threads execute, and we
check that we hit the breakpoint 100 times. But sometimes it is only hit
99 times, and the test fails.

When we hit a software breakpoint, the pc value for the thread is the
address of the breakpoint instruction - as if it had not been hit yet.
And because a user might ADD a breakpoint for the current pc from the
commandline, when we go to resume execution, any thread that is sitting
at a breakpoint site will be silently advanced past the breakpoint
instruction (disable bp, instruction step that thread, re-enable bp)
before resuming -- whether that thread has hit its breakpoint or not.

What this test is exposing is that there is another corner case, a
thread that is sitting at a breakpoint site but has not yet executed the
breakpoint instruction. The thread will have no stop reason, no mach
exception, so it will not be recorded as having hit the breakpoint
(because it hasn't yet). But when we resume execution, because it is
sitting at a breakpoint site, we advance past it and miss the breakpoint
hit.

In 2016 Abhishek Aggarwal handled a similar issue with a patch in
`ProcessGDBRemote::SetThreadStopInfo()`, adding a breakpoint StopInfo
for a thread sitting at a breakpoint site that has no stop reason.
debugserver's `jThreadsInfo` would not correctly execute Abhishek's code
though because it would respond with `"reason":"none"` for a thread with
no stop reason, and `SetThreadStopInfo()` expected an empty reason here.
The first part of my patch is to clear the `reason` if it is `"none"` so
we flow through the code correctly.

On Darwin, though, our stop reply packet (Txx...) includes the
`threads`, `thread-pcs`, and `jstopinfo` keys, which give us the tids
for all current threads, the pc values for those threads, and
`jstopinfo` has a JSON dictionary with the mach exceptions for all
threads that have a mach exception. In
`ProcessGDBRemote::CalculateThreadStopInfo()` we set the StopInfo for
each thread for a private stop and if we have `jstopinfo` it is the
source of all the StopInfos. I have to add the same logic here, to give
the thread a breakpoint StopInfo even though it hasn't executed the
breakpoint yet. In this case we are very early in thread construction
and I only have the information in the Txx stop reply packet -- tids,
pcs, and jstopinfo, so I can't use the normal general mechanisms of
going through the RegisterContext to get the pc, it's a bit different.

If I hack debugserver to not issue `jstopinfo`,
`CalculateThreadStopInfo` will fall back to sending `qThreadStopInfo`
for each thread and going through
`ProcessGDBRemote::SetThreadStopInfo()` to set the stop infos (and with
the `reason:none` fix, use Abhishek's code).

rdar://110549165



To unsubscribe from these emails, change your notification settings at https://github.com/llvm/llvm-project/settings/notifications