Commit Graph

355 Commits

Author SHA1 Message Date
Dmitri Gribenko
547917aebd Revert "[lldb] Extend frame recognizers to hide frames from backtraces (#104523)"
This reverts commit f01f80ce6c.

This commit introduces an msan violation. See the discussion on https://github.com/llvm/llvm-project/pull/104523.
2024-08-22 13:24:57 +02:00
Adrian Prantl
f01f80ce6c [lldb] Extend frame recognizers to hide frames from backtraces (#104523)
Compilers and language runtimes often use helper functions that are
fundamentally uninteresting when debugging anything but the
compiler/runtime itself. This patch introduces a user-extensible
mechanism that allows for these frames to be hidden from backtraces and
automatically skipped over when navigating the stack with `up` and
`down`.

This does not affect the numbering of frames, so `f <N>` will still
provide access to the hidden frames. The `bt` output will also print a
hint that frames have been hidden.

My primary motivation for this feature is to hide thunks in the Swift
programming language, but I'm including an example recognizer for
`std::function::operator()` that I wished for myself many times while
debugging LLDB.

rdar://126629381


Example output. (Yes, my proof-of-concept recognizer could hide even
more frames if we had a method that returned the function name without
the return type or I used something that isn't based off regex, but it's
really only meant as an example).

before:
```
(lldb) thread backtrace --filtered=false
* thread #1, queue = 'com.apple.main-thread', stop reason = breakpoint 1.1
  * frame #0: 0x0000000100001f04 a.out`foo(x=1, y=1) at main.cpp:4:10
    frame #1: 0x0000000100003a00 a.out`decltype(std::declval<int (*&)(int, int)>()(std::declval<int>(), std::declval<int>())) std::__1::__invoke[abi:se200000]<int (*&)(int, int), int, int>(__f=0x000000016fdff280, __args=0x000000016fdff224, __args=0x000000016fdff220) at invoke.h:149:25
    frame #2: 0x000000010000399c a.out`int std::__1::__invoke_void_return_wrapper<int, false>::__call[abi:se200000]<int (*&)(int, int), int, int>(__args=0x000000016fdff280, __args=0x000000016fdff224, __args=0x000000016fdff220) at invoke.h:216:12
    frame #3: 0x0000000100003968 a.out`std::__1::__function::__alloc_func<int (*)(int, int), std::__1::allocator<int (*)(int, int)>, int (int, int)>::operator()[abi:se200000](this=0x000000016fdff280, __arg=0x000000016fdff224, __arg=0x000000016fdff220) at function.h:171:12
    frame #4: 0x00000001000026bc a.out`std::__1::__function::__func<int (*)(int, int), std::__1::allocator<int (*)(int, int)>, int (int, int)>::operator()(this=0x000000016fdff278, __arg=0x000000016fdff224, __arg=0x000000016fdff220) at function.h:313:10
    frame #5: 0x0000000100003c38 a.out`std::__1::__function::__value_func<int (int, int)>::operator()[abi:se200000](this=0x000000016fdff278, __args=0x000000016fdff224, __args=0x000000016fdff220) const at function.h:430:12
    frame #6: 0x0000000100002038 a.out`std::__1::function<int (int, int)>::operator()(this= Function = foo(int, int) , __arg=1, __arg=1) const at function.h:989:10
    frame #7: 0x0000000100001f64 a.out`main(argc=1, argv=0x000000016fdff4f8) at main.cpp:9:10
    frame #8: 0x0000000183cdf154 dyld`start + 2476
(lldb) 
```

after

```
(lldb) bt
* thread #1, queue = 'com.apple.main-thread', stop reason = breakpoint 1.1
  * frame #0: 0x0000000100001f04 a.out`foo(x=1, y=1) at main.cpp:4:10
    frame #1: 0x0000000100003a00 a.out`decltype(std::declval<int (*&)(int, int)>()(std::declval<int>(), std::declval<int>())) std::__1::__invoke[abi:se200000]<int (*&)(int, int), int, int>(__f=0x000000016fdff280, __args=0x000000016fdff224, __args=0x000000016fdff220) at invoke.h:149:25
    frame #2: 0x000000010000399c a.out`int std::__1::__invoke_void_return_wrapper<int, false>::__call[abi:se200000]<int (*&)(int, int), int, int>(__args=0x000000016fdff280, __args=0x000000016fdff224, __args=0x000000016fdff220) at invoke.h:216:12
    frame #6: 0x0000000100002038 a.out`std::__1::function<int (int, int)>::operator()(this= Function = foo(int, int) , __arg=1, __arg=1) const at function.h:989:10
    frame #7: 0x0000000100001f64 a.out`main(argc=1, argv=0x000000016fdff4f8) at main.cpp:9:10
    frame #8: 0x0000000183cdf154 dyld`start + 2476
Note: Some frames were hidden by frame recognizers
```
2024-08-20 16:01:22 -07:00
jeffreytan81
f838fa820f New ThreadPlanSingleThreadTimeout to resolve potential deadlock in single thread stepping (#90930)
This PR introduces a new `ThreadPlanSingleThreadTimeout` that will be
used to address potential deadlock during single-thread stepping.

While debugging a target with a non-trivial number of threads (around
5000 threads in one example target), we noticed that a simple step over
can take as long as 10 seconds. Enabling single-thread stepping mode
significantly reduces the stepping time to around 3 seconds. However,
this can introduce deadlock if we try to step over a method that depends
on other threads to release a lock.

To address this issue, we introduce a new
`ThreadPlanSingleThreadTimeout` that can be controlled by the
`target.process.thread.single-thread-plan-timeout` setting during
single-thread stepping mode. The concept involves counting the elapsed
time since the last internal stop to detect overall stepping progress.
Once a timeout occurs, we assume the target is not making progress due
to a potential deadlock, as mentioned above. We then send a new async
interrupt, resume all threads, and `ThreadPlanSingleThreadTimeout`
completes its task.

To support this design, the major changes made in this PR are:
1. `ThreadPlanSingleThreadTimeout` is popped during every internal stop
and reset (re-pushed) to the top of the stack (as a leaf node) during
resume. This is achieved by always returning `true` from
`ThreadPlanSingleThreadTimeout::DoPlanExplainsStop()` and
`ThreadPlanSingleThreadTimeout::MischiefManaged()`.
2. A new thread-specific async interrupt stop is introduced, which can
be detected/consumed by `ThreadPlanSingleThreadTimeout`.
3. The clearing of branch breakpoints in the range thread plan has been
moved from `DoPlanExplainsStop()` to `ShouldStop()`, as it is not
guaranteed that it will be called.

The detailed design is discussed in the RFC below:

[https://discourse.llvm.org/t/improve-single-thread-stepping/74599](https://discourse.llvm.org/t/improve-single-thread-stepping/74599)

---------

Co-authored-by: jeffreytan81 <jeffreytan@fb.com>
2024-08-05 17:26:39 -07:00
Med Ismail Bennani
039cfe812c [lldb/Target] Rename ThreadPlanPython into ScriptedThreadPlan (#101931)
Following 9a9ec228cd, since the ThreadPlanPython class started making
use of the Scripted Interface instead of calling directly into the
python methods, this class can work with other scripting languages (as
long as someone add the interfact for that language ;p).

So it doesn't make sense anymore for it to keep this name and also we
should avoid having language specific related classes outside the plugin
directory.

This patch renames the internal class from `ThreadPlanPython` to
`ScriptedThreadPlan` as its advertised externally, and also updates the
various log messages.

This should hopefully make the codebase more coherent.

Signed-off-by: Med Ismail Bennani <ismail@bennani.ma>
2024-08-05 10:43:42 -07:00
Jason Molenda
52c08d7ffd Revert "[lldb] Change lldb's breakpoint handling behavior (#96260)"
This reverts commit 05f0e86cc8.

The debuginfo dexter tests are failing, probably because the way
stepping over breakpoints has changed with my patches.  And there
are two API tests fails on the ubuntu-arm (32-bit) bot. I'll need
to investigate both of these, neither has an obvious failure reason.
2024-07-19 18:43:53 -07:00
Jason Molenda
05f0e86cc8 [lldb] Change lldb's breakpoint handling behavior (#96260)
lldb today has two rules: When a thread stops at a BreakpointSite, we
set the thread's StopReason to be "breakpoint hit" (regardless if we've
actually hit the breakpoint, or if we've merely stopped *at* the
breakpoint instruction/point and haven't tripped it yet). And second,
when resuming a process, any thread sitting at a BreakpointSite is
silently stepped over the BreakpointSite -- because we've already
flagged the breakpoint hit when we stopped there originally.

In this patch, I change lldb to only set a thread's stop reason to
breakpoint-hit when we've actually executed the instruction/triggered
the breakpoint. When we resume, we only silently step past a
BreakpointSite that we've registered as hit. We preserve this state
across inferior function calls that the user may do while stopped, etc.

Also, when a user adds a new breakpoint at $pc while stopped, or changes
$pc to be the address of a BreakpointSite, we will silently step past
that breakpoint when the process resumes. This is purely a UX call, I
don't think there's any person who wants to set a breakpoint at $pc and
then hit it immediately on resuming.

One non-intuitive UX from this change, but I'm convinced it is
necessary: If you're stopped at a BreakpointSite that has not yet
executed, you `stepi`, you will hit the breakpoint and the pc will not
yet advance. This thread has not completed its stepi, and the thread
plan is still on the stack. If you then `continue` the thread, lldb will
now stop and say, "instruction step completed", one instruction past the
BreakpointSite. You can continue a second time to resume execution. I
discussed this with Jim, and trying to paper over this behavior will
lead to more complicated scenarios behaving non-intuitively. And mostly
it's the testsuite that was trying to instruction step past a breakpoint
and getting thrown off -- and I changed those tests to expect the new
behavior.

The bugs driving this change are all from lldb dropping the real stop
reason for a thread and setting it to breakpoint-hit when that was not
the case. Jim hit one where we have an aarch64 watchpoint that triggers
one instruction before a BreakpointSite. On this arch we are notified of
the watchpoint hit after the instruction has been unrolled -- we disable
the watchpoint, instruction step, re-enable the watchpoint and collect
the new value. But now we're on a BreakpointSite so the watchpoint-hit
stop reason is lost.

Another was reported by ZequanWu in
https://discourse.llvm.org/t/lldb-unable-to-break-at-start/78282 we
attach to/launch a process with the pc at a BreakpointSite and
misbehave. Caroline Tice mentioned it is also a problem they've had with
putting a breakpoint on _dl_debug_state.

The change to each Process plugin that does execution control is that

1. If we've stopped at a BreakpointSite that has not been executed yet,
we will call Thread::SetThreadStoppedAtUnexecutedBP(pc) to record
that.  When the thread resumes, if the pc is still at the same site, we
will continue, hit the breakpoint, and stop again.

2. When we've actually hit a breakpoint (enabled for this thread or not),
the Process plugin should call Thread::SetThreadHitBreakpointSite().
When we go to resume the thread, we will push a step-over-breakpoint
ThreadPlan before resuming.

The biggest set of changes is to StopInfoMachException where we
translate a Mach Exception into a stop reason. The Mach exception codes
differ in a few places depending on the target (unambiguously), and I
didn't want to duplicate the new code for each target so I've tested
what mach exceptions we get for each action on each target, and
reorganized StopInfoMachException::CreateStopReasonWithMachException to
document these possible values, and handle them without specializing
based on the target arch.

rdar://123942164
2024-07-19 17:26:13 -07:00
Alex Langford
5779483527 [lldb][nfc] Move broadcaster class strings away from ConstString (#89690)
These are hardcoded strings that are already present in the data section
of the binary, no need to immediately place them in the ConstString
StringPools. Lots of code still calls `GetBroadcasterClass` and places
the return value into a ConstString. Changing that would be a good
follow-up.

Additionally, calls to these functions are still wrapped in ConstStrings
at the SBAPI layer. This is because we must guarantee the lifetime of
all strings handed out publicly.
2024-04-24 12:13:18 -07:00
Jonas Devlieghere
556fe5f290 [lldb] Reland: Store SupportFile in FileEntry (NFC) (#85892)
This is another step towards supporting DWARF5 checksums and inline
source code in LLDB. This is a reland of #85468 but without the
functional change of storing the support file from the line table (yet).
2024-03-21 08:40:08 -07:00
Jonas Devlieghere
a289f66efd Revert "[lldb] Store SupportFile in FileEntry (NFC)" (#85885)
Reverts llvm/llvm-project#85468 because @slackito reports this broke
stepping in one of their tests [1] and this patch was meant to be NFC.

[1]
d5a277d309 (commitcomment-139991120)
2024-03-19 17:48:46 -07:00
Jonas Devlieghere
d5a277d309 [lldb] Store SupportFile in FileEntry (NFC) (#85468)
This is another step towards supporting DWARF5 checksums and inline
source code in LLDB.
2024-03-15 15:03:54 -07:00
Jason Molenda
aab48c99c2 [lldb] Detect a Darwin kernel issue and work around it (#81573)
On arm64 machines, when there is a hardware breakpoint or watchpoint
set, and lldb has instruction-stepped a thread, and then done a
Process::Resume, we will sometimes receive an extra "instruction step
completed" mach exception and the pc has not advanced. From a user's
perspective, they hit Continue and lldb stops again at the same spot.
From the testsuite's perspective, this has been a constant source of
testsuite failures for any test using hardware watchpoints and
breakpoints, the arm64 CI bots seem especially good at hitting this
issue.

Jim and I have been slowly looking at this for a few months now, and
finally I decided to try to detect this situation in lldb and silently
resume the process again when it happens.

We were already detecting this "got an insn-step finished mach exception
but this thread was not instruction stepping" combination in
StopInfoMachException where we take the mach exception and create a
StopInfo object for it. We had a lot of logging we used to understand
the failure as it was hit on the bots in assert builds.

This patch adds a new case to `Thread::GetPrivateStopInfo()` to call the
StopInfo's (new) `IsContinueInterrupted()` method. In
StopInfoMachException, where we previously had logging for assert
builds, I now note it in an ivar, and when
`Thread::GetPrivateStopInfo()` asks if this has happened, we check all
of the combination of events that this comes up: We have a hardware
breakpoint or watchpoint, we were not instruction stepping this thread
but got an insn-step mach exception, the pc is the same as the previous
stop's pc. And in that case, `Thread::GetPrivateStopInfo()` returns no
StopInfo -- indicating that this thread would like to resume execution.

The `Thread` object has two StackFrameLists, `m_curr_frames_sp` and
`m_prev_frames_sp`. When a thread resumes execution, we move
`m_curr_frames_sp` in to `m_prev_frames_sp` and when it stops executing,
w euse `m_prev_frames_sp` to seed the new `m_curr_frames_sp` if most of
the stack is the same as before.

In this same location, I now save the Thread's RegisterContext::GetPC
into an ivar, `m_prev_framezero_pc`. StopInfoMachException needs this
information to check all of the conditions I outlined above for
`IsContinueInterrupted`.

This has passed exhaustive testing and we do not have any testsuite
failures for hardware watchpoints and breakpoints due to this kernel bug
with the patch in place. In focusing on these tests for thousands of
runs, I have found two other uncommon race conditions for the
TestConcurrent* tests on arm64. TestConcurrentManyBreakpoints.py (which
uses no hardware watchpoint/breakpoints) will sometimes only have 99
breakpoints when it expects 100, and any of the concurrent tests using
the shared harness (I've seen it in
TestConcurrentWatchBreakDelay.py,
TestConcurrentTwoBreakpointsOneSignal.py,
TestConcurrentSignalDelayWatch.py) can fail when the test harness checks
that there is only one thread still running at the end, and it finds two
-- one of them under pthread_exit / pthread_terminate. Both of these
failures happen on github main without my changes, and with my changes -
they are unrelated race conditions in these tests, and I'm sure I'll be
looking into them at some point if they hit the CI bots with frequency.
On my computer, these are in the 0.3-0.5% of the time class. But the CI
bots do have different timing.
2024-02-14 13:06:20 -08:00
Alex Langford
0cea54a382 [lldb][NFCI] Remove EventData* param from BroadcastEvent (#78773)
BroadcastEvent currently takes its EventData* param and shoves it into
an Event object, which takes ownership of the pointer and places it into
a shared_ptr to manage the lifetime.

Instead of relying on `new` and passing raw pointers around, I think it
would make more sense to create the shared_ptr up front.
2024-01-22 10:46:20 -08:00
David Spickett
a8af51dfa5 [lldb][test] Skip global module cache on Arm/AArch64 Linux (again)
This reverts commit 01c4ecb7ae,
d14d52158b and
a756dc4724.

This removes the logging and workaround I added earlier,
and puts back the skip for Arm/AArch64 Linux.

I've not seen it fail on AArch64 since, but let's not create
more noise if it does.

I've written up the issue as https://github.com/llvm/llvm-project/issues/76057.
It's something to do with trying to destroy a process while
a thread is doing a single sep. So my workaround wouldn't have
worked in any case. It needs a more involved fix.
2023-12-20 13:56:40 +00:00
David Spickett
d14d52158b [lldb][test] Add extra logging for module cache test
And remove the workaround I was trying, as this logging may prove what
the actual issue is.

Which I think is that the thread plan map in Process is cleared before
the threads are destroyed. So Thread::ShouldStop could be getting
the current plan, then the plan map is cleared, then Thread::ShouldStop
is deciding based on that plan to pop a plan from the now empty stack.
2023-12-20 09:41:33 +00:00
David Spickett
1bce61e6b0 [lldb] Remove PopPlan asserts and skip test on Arm/AArch64 Linux
This reverts commit 481bb62e50 and
71b4d7498f, along with the logging
and assert I had added to the test previously.

Now that I've caught it failing on Arm:
https://lab.llvm.org/buildbot/#/builders/17/builds/46598

Now I have enough to investigate, skip the test on the effected
platforms while I do that.
2023-12-14 19:17:49 +00:00
David Spickett
71b4d7498f [lldb] Fixup PopPlan assert
Fixes 481bb62e50.
2023-12-14 16:55:07 +00:00
David Spickett
481bb62e50 [lldb] Assert immediately prior to calling PopPlan
This is part of ongoing attempts to catch the test from
2684281d20 failing on Arm and AArch64.

I did get logs for the failure but only on Arm, where the backtrace is
truncated. So, let's do the assert that PopPlan was going to do,
before we call it.

Then I should know exactly which PopPlan is asserting.

Technically I should take a mutex here, but technically I shouldn't
be debugging via buildbot, so I'm going to take the risk temporarily.
2023-12-14 16:51:12 +00:00
David Spickett
d0f5039e5d Reland "Add a test for evicting unreachable modules from the global module cache (#74894)"
This reverts commit 35dacf2f51.

And relands the original change with two additions so I can debug the failure on Arm/AArch64:
* Enable lldb step logging in the tests.
* Assert that the current plan is not the base plan at the spot I believe is calling PopPlan.

These will be removed and replaced with a proper fix once I see some failures on the bots,
I couldn't reproduce it locally.

(also, no sign of it on the x86_64 bot)
2023-12-14 10:54:03 +00:00
Walter Erquinigo
1654d7dc38 [lldb-dap] Add an option to provide a format for threads (#72196)
When this option gets enabled, descriptions of threads will be generated
using the format provided in the launch configuration instead of
generating it manually in the dap code. This allows lldb-dap to show an
output similar to the one in the CLI.
This is very similar to https://github.com/llvm/llvm-project/pull/71843
2023-11-14 13:23:55 -05:00
jeffreytan81
e8ea47602b [lldb] Implement thread local storage for linux (#67470)
This patch implements the thread local storage support for linux
(https://github.com/llvm/llvm-project/issues/28766).

TLS feature is originally only implemented for Mac. With my previous
patch to enable `fs_base` register for Linux
(https://reviews.llvm.org/D155256), now it is feasible to implement this
feature for Linux.

The major changes are:
* Track the main module's link address during launch
* Fetch thread pointer from `fs_base` register
* Create register alias for thread pointer
* Read pthread metadata from target memory instead of process so that it
works for coredump

With the patch the failing test is passing now. Note: I am only enabling
this test for Mac and Linux because I do not have machine to test for
FreeBSD/NetBSD.

---------

Co-authored-by: jeffreytan81 <jeffreytan@fb.com>
2023-09-27 09:14:40 -07:00
Alex Langford
14d95b26ae [lldb][NFCI] Remove unneeded ConstString conversions
ConstString can be implicitly converted into a llvm::StringRef. This is
very useful in many places, but it also hides places where we are
creating a ConstString only to use it as a StringRef for the entire
lifespan of the ConstString object.

I locally removed the implicit conversion and found some of the places we
were doing this.

Differential Revision: https://reviews.llvm.org/D159237
2023-08-31 11:27:59 -07:00
Alex Langford
0871f22edc [lldb][NFCI] Use size_t in OptionValueProperties
In many places we're using uint32_t where we should be using size_t.
We should be consistent.

Differential Revision: https://reviews.llvm.org/D151949
2023-06-05 12:57:43 -07:00
Med Ismail Bennani
1370a1cb5b [lldb] Add support for negative integer to {SB,}StructuredData
This patch refactors the `StructuredData::Integer` class to make it
templated, makes it private and adds 2 public specialization for both
`int64_t` & `uint64_t` with a public type aliases, respectively
`SignedInteger` & `UnsignedInteger`.

It adds new getter for signed and unsigned interger values to the
`StructuredData::Object` base class and changes the implementation of
`StructuredData::Array::GetItemAtIndexAsInteger` and
`StructuredData::Dictionary::GetValueForKeyAsInteger` to support signed
and unsigned integers.

This patch also adds 2 new `Get{Signed,Unsigned}IntegerValue` to the
`SBStructuredData` class and marks `GetIntegerValue` as deprecated.

Finally, this patch audits all the caller of `StructuredData::Integer`
or `StructuredData::GetIntegerValue` to use the proper type as well the
various tests that uses `SBStructuredData.GetIntegerValue`.

rdar://105575764

Differential Revision: https://reviews.llvm.org/D150485

Signed-off-by: Med Ismail Bennani <ismail@bennani.ma>
2023-05-22 16:14:00 -07:00
Jonas Devlieghere
300dce986f [lldb] Migrate to GetPropertyAtIndexAs for FileSpecList (NFC)
Use the templated GetPropertyAtIndexAs helper for FileSpecList.
2023-05-04 22:24:23 -07:00
Jonas Devlieghere
6f8b33f6df [lldb] Use templates to simplify {Get,Set}PropertyAtIndex (NFC)
Use templates to simplify {Get,Set}PropertyAtIndex. It has always
bothered me how cumbersome those calls are when adding new properties.
After this patch, SetPropertyAtIndex infers the type from its arguments
and GetPropertyAtIndex required a single template argument for the
return value. As an added benefit, this enables us to remove a bunch of
wrappers from UserSettingsController and OptionValueProperties.

Differential revision: https://reviews.llvm.org/D149774
2023-05-04 16:42:46 -07:00
Jonas Devlieghere
611bd6c6ae [lldb] Make exe_ctx an optional argument in OptionValueProperties (NFC)
The majority of call sites are nullptr as the execution context.
Refactor OptionValueProperties to make the argument optional and
simplify all the callers.
2023-05-02 10:36:11 -07:00
Jonas Devlieghere
ddd9358bca [lldb] Remove unused will_modify argument (NFC)
Various OptionValue related classes are passing around will_modify but
the value is never used. This patch simplifies the interfaces by
removing the redundant argument.
2023-05-02 00:20:34 -07:00
Jonas Devlieghere
9c48aa68f4 [lldb] Refactor OptionValueProperties to return a std::optional (NFC)
Similar to fdbe7c7faa, refactor OptionValueProperties to return a
std::optional instead of taking a fail value. This allows the caller to
handle situations where there's no value, instead of being unable to
distinguish between the absence of a value and the value happening the
match the fail value. When a fail value is required,
std::optional::value_or() provides the same functionality.
2023-05-01 21:46:32 -07:00
Jonas Devlieghere
b12b35ad4b [lldb] Add debugger.external-editor setting
Add a new setting (debugger.external-editor) to specify an external
editor. The setting takes precedence over the existing
LLDB_EXTERNAL_EDITOR environment variable.

Differential revision: https://reviews.llvm.org/D149565
2023-05-01 14:11:11 -07:00
Jonas Devlieghere
13dbc16b4d [lldb] Refactor host::OpenFileInExternalEditor
This patch refactors the macOS implementation of
OpenFileInExternalEditor. It fixes an AppleEvent memory leak, the
caching of LLDB_EXTERNAL_EDITOR and speculatively fixes a crash when
CFURL is NULL (rdar://108633464). The new code also improves error
handling, readability and documents calls to the CoreFoundation Launch
Services APIs.

A bunch of the Launch Services APIs have been deprecated
(LSFindApplicationForInfo, LSOpenURLsWithRole). The preferred API is
LSOpenCFURLRef but it doesn't specifying the "location" Apple Event
which is used to highlight the current line and switching over would
regress the existing behavior.

rdar://108633464

Differential revision: https://reviews.llvm.org/D149482
2023-04-28 18:12:41 -07:00
Jim Ingham
076341d108 Make sure SelectMostRelevantFrame happens only when returning to the user.
This is a user facing action, it is meant to focus the user's attention on
something other than the 0th frame when you stop somewhere where that's
helpful. For instance, stopping in pthread_kill after an assert will select
the assert frame.

This is not something you want to have happen internally in lldb, both
because internally you really don't want the selected frame changing out
from under you, and because the recognizers can do arbitrary work, and that
can cause deadlocks or other unexpected behavior.

However, it's not something that the current code does
explicitly after a stop has been delivered, it's expected to happen implicitly
as part of stopping. I changing this to call SMRF explicitly after a user
stop, but that got pretty ugly quickly.

So I added a bool to control whether to run this and audited all the current
uses to determine whether we're returning to the user or not.

Differential Revision: https://reviews.llvm.org/D148863
2023-04-21 14:21:25 -07:00
Alex Langford
6ebf1bc66b [lldb] Change return type of EventData::GetFlavor
There's no reason these strings need to be in the ConstString
StringPool, they're already string literals with static lifetime.

I plan on addressing other similar functions in follow up commits.

Differential Revision: https://reviews.llvm.org/D147833
2023-04-11 10:49:17 -07:00
Jim Ingham
730c8e160c [lldb] Move "SelectMostRelevantFrame" from Thread::WillStop
SelectMostRelevantFrame triggers the StackFrameRecognizer construction,
which can run arbitrary Python code, call expressions etc. WillStop gets
called on every private stop while the recognizers are a user-facing
feature, so first off doing this work on every stop is inefficient. But
more importantly, you can get in to situations where the recognizer
causes an expression to get run, then when we fetch the stop event at
the end of the expression evaluation, we call WillStop again on the
expression handling thread, which will do the same StackFrameRecognizer
work again. If anyone is locking along that path, you will end up with a
deadlock between the two threads.

The example that brought this to my attention was the
objc_exception_throw recognizer which can cause the objc runtime
introspection functions to get run, and those take a lock in
AppleObjCRuntimeV2::DynamicClassInfoExtractor::UpdateISAToDescriptorMap
along this path, so the second thread servicing the expression deadlocks
against the first thread waiting for the expression to complete.

It makes more sense to have the frame recognizers run on demand, either
when someone asks for the variables for the frame, or when someone does
GetSelectedFrame. The former already worked that way, the only reason
this was being done in WillStop was because the StackFrameRecognizers
can change the SelectedFrame, so you needed to run them before the
anyone requested the SelectedFrame.

This patch moves SelectMostRelevantFrame to StackFrameList, and runs it
when GetSelectedFrame is called for the first time on a given stop. If
you call SetSelectedFrame before GetSelectedFrame, then you should NOT
run the recognizer & change the frame out from under you. This patch
also makes that work. There were already tests for this behavior, and
for the feature that caused the hang, but the hang is racy, and it
doesn't trigger all the time, so I don't have a way to test that
explicitly.

One more detail: it's actually pretty easy to end up calling
GetSelectedFrame, for instance if you ask for the best ExecutionContext
from an ExecutionContextRef it will fill the StackFrame with the result
of GetSelectedFrame and that would still have the same problems if this
happens on the Private State Thread. So this patch also short-circuits
SelectMostRelevantFrame if run on the that thread. I can't think of any
reason the computations that go on on the Private State Thread would
actually want the SelectedFrame - that's a user-facing concept, so
avoiding that complication is the best way to go.

rdar://107643231

Differential revision: https://reviews.llvm.org/D147753
2023-04-07 12:21:00 -07:00
Jim Ingham
9b655c2627 Revert "Fix a problem with "watchpoint triggers before" watchpoint handling."
This reverts commit 8d024a79ea.

I accidentally included some "in progress" work that wasn't supposed to
go with this commit.
2023-03-20 16:05:57 -07:00
Jim Ingham
8d024a79ea Fix a problem with "watchpoint triggers before" watchpoint handling.
We need to step the watchpoint instruction in these cases, but the
when we queued the ThreadPlanStepOverWatchpoint to do this, we didn't
make it a Controlling plan.  So if you are stepping, this plan returns as
though it were a utility plan, and the stepping plan keeps going.

This only partially fixes the problem on Darwin; there's another bug
with reporting a watchpoint when we're instruction single stepping over
an instruction that triggers a watchpoint.  The kernel reports the
"single step completed" but not the watchpoint hit.  So this commit
also refactors the test into a part that works (at least on Darwin) and
a part that still fails.

We may have to adjust the test result expectations for other systems after
this fix.

Differential Revision: https://reviews.llvm.org/D146337
2023-03-20 15:17:15 -07:00
Kazu Hirata
2fe8327406 [lldb] Use std::optional instead of llvm::Optional (NFC)
This patch replaces (llvm::|)Optional< with std::optional<.  I'll post
a separate patch to clean up the "using" declarations, #include
"llvm/ADT/Optional.h", etc.

This is part of an effort to migrate from llvm::Optional to
std::optional:

https://discourse.llvm.org/t/deprecating-llvm-optional-x-hasvalue-getvalue-getvalueor/63716
2023-01-07 14:18:35 -08:00
Kazu Hirata
f190ce625a [lldb] Add #include <optional> (NFC)
This patch adds #include <optional> to those files containing
llvm::Optional<...> or Optional<...>.

I'll post a separate patch to actually replace llvm::Optional with
std::optional.

This is part of an effort to migrate from llvm::Optional to
std::optional:

https://discourse.llvm.org/t/deprecating-llvm-optional-x-hasvalue-getvalue-getvalueor/63716
2023-01-07 13:43:00 -08:00
Fangrui Song
4215a84559 [lldb] Remove unneeded .get() NFC 2023-01-04 22:05:40 -08:00
Muhammad Omair Javaid
58e9cc13e2 Revert "[lldb] Remove redundant .c_str() and .get() calls"
This reverts commit fbaf48be0f.

This has broken all LLDB buildbots:
https://lab.llvm.org/buildbot/#/builders/68/builds/44990
https://lab.llvm.org/buildbot/#/builders/96/builds/33160
2022-12-19 13:52:10 +05:00
Fangrui Song
fbaf48be0f [lldb] Remove redundant .c_str() and .get() calls
Removing .c_str() has a semantics difference, but the use scenarios
likely do not matter as we don't have NUL in the strings.
2022-12-18 01:15:25 +00:00
Fangrui Song
f43886e7ba [lldb] llvm::Optional::value() && => operator*/operator->
std::optional::value() has undesired exception checking semantics and is
unavailable in older Xcode (see _LIBCPP_AVAILABILITY_BAD_OPTIONAL_ACCESS). The
call sites block std::optional migration.
2022-12-17 20:37:13 +00:00
Jason Molenda
ee11ef6dc0 Launch state discoverable in Darwin, use for SafeToCallFunctions
The dynamic linker on Darwin, dyld, can provide status of
the process state for a few significant points early on,
most importantly, when libSystem has been initialized and it
is safe to call functions behind the scenes.  Pipe this
information up from debugserver to DynamicLoaderMacOS, for
the DynamicLoader::IsFullyInitialized() method, then have
Thread::SafeToCallFunctions use this information.  Finally,
for the two utility functions in the AppleObjCRuntimeV2
LanguageRuntime plugin that I was fixing, call this method
before running our utility functions to collect the list of
objc classes registered in the runtime.

User expressions will still be allowed to run any time -
we assume the user knows what they are doing - but these
two additional utility functions that they are unaware of
will be limited by this state.

Differential Revision: https://reviews.llvm.org/D139054
rdar://102436092
can probably make function calls.
2022-12-13 11:42:56 -08:00
Jim Ingham
0948f1cf81 Reapply the commits to enable accurate hit-count detection for watchpoints.
This commit combines the initial commit (7c240de609af), a fix for x86_64 Linux
(3a0581501e76) and a fix for thinko in a last minute rewrite that I really
should have run the testsuite on.

Also, make sure that all the "I need to step over watchpoint" plans execute
before we call a public stop.  Otherwise, e.g. if you have N watchpoints and
a Signal, the signal stop info will get us to stop with the watchpoints in a
half-done state.

Differential Revision: https://reviews.llvm.org/D130674
2022-08-05 11:01:27 -07:00
Kazu Hirata
5cff5142a8 Use value instead of getValue (NFC) 2022-07-15 20:03:13 -07:00
Kazu Hirata
3b7c3a654c Revert "Don't use Optional::hasValue (NFC)"
This reverts commit aa8feeefd3.
2022-06-25 11:56:50 -07:00
Kazu Hirata
aa8feeefd3 Don't use Optional::hasValue (NFC) 2022-06-25 11:55:57 -07:00
Luboš Luňák
76bc772920 [lldb][gui] make 'step out' step out of the selected frame
Differential Revision: https://reviews.llvm.org/D123001
2022-04-05 08:29:13 +02:00
Dave Lee
47f652d695 [lldb] Call ThreadPlan::DiscardPlan from Thread::DiscardPlan
Correct `Thread::DiscardPlan` to call `DiscardPlan` instead of `PopPlan`.

Differential Revision: https://reviews.llvm.org/D96655
2022-03-10 13:58:08 -08:00
Dave Lee
e3dfa30501 [lldb] Remove extra space in step logging message
When seeing the extra space in the log, it wasn't clear if there was a missing
printf argument. Removing the extra space removes the potential confusion.
2022-03-10 12:02:58 -08:00
Pavel Labath
c34698a811 [lldb] Rename Logging.h to LLDBLog.h and clean up includes
Most of our code was including Log.h even though that is not where the
"lldb" log channel is defined (Log.h defines the generic logging
infrastructure). This worked because Log.h included Logging.h, even
though it should.

After the recent refactor, it became impossible the two files include
each other in this direction (the opposite inclusion is needed), so this
patch removes the workaround that was put in place and cleans up all
files to include the right thing. It also renames the file to LLDBLog to
better reflect its purpose.
2022-02-03 14:47:01 +01:00