Revert "[lldb] Change lldb's breakpoint handling behavior (#96260)"

This reverts commit 05f0e86cc895181b3d2210458c78938f83353002.

The debuginfo dexter tests are failing, probably because the way
st

Revert "[lldb] Change lldb's breakpoint handling behavior (#96260)"

This reverts commit 05f0e86cc895181b3d2210458c78938f83353002.

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.

show more ...

[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

[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

show more ...

[lldb] Change interface of StructuredData::Array::GetItemAtIndexAsDictionary (#71961)

Similar to my previous patch (#71613) where I changed
`GetItemAtIndexAsString`, this patch makes the same chang

[lldb] Change interface of StructuredData::Array::GetItemAtIndexAsDictionary (#71961)

Similar to my previous patch (#71613) where I changed
`GetItemAtIndexAsString`, this patch makes the same change to
`GetItemAtIndexAsDictionary`.

`GetItemAtIndexAsDictionary` now returns a std::optional that is either
`std::nullopt` or is a valid pointer. Therefore, if the optional is
populated, we consider the pointer to always be valid (i.e. no need to
check pointer validity).

show more ...

[lldb] Fix assertions caused by un-checked errors in ScriptedProcess

This patch should fix some assertion that started getting hit after f22d82c.

That commit changed the scripted object plugin crea

[lldb] Fix assertions caused by un-checked errors in ScriptedProcess

This patch should fix some assertion that started getting hit after f22d82c.

That commit changed the scripted object plugin creation to use
`llvm::Expected<T>` as a return type to enforce error handling, however
I forgot to handle the error which caused the assert.

The interesting part about this, is that since that assert was triggered
in the ScriptedProcess constructor (where the `llvm::Error` wasn't
handled), that impacted every test that launched any kind of process,
since the process plugin manager would eventually also iterate over the
`ScriptedProcess::Create` factory method.

This patch should fix the assertions by handling the errors.

Signed-off-by: Med Ismail Bennani <ismail@bennani.ma>

show more ...

[lldb/Interpreter] Make ScriptedInterface Object creation more generic (#68052)

This patch changes the way plugin objects used with Scripted Interfaces
are created.

Instead of implementing a dif

[lldb/Interpreter] Make ScriptedInterface Object creation more generic (#68052)

This patch changes the way plugin objects used with Scripted Interfaces
are created.

Instead of implementing a different SWIG method to create the object for
every scripted interface, this patch makes the creation more generic by
re-using some of the ScriptedPythonInterface templated Dispatch code.

This patch also improves error handling of the object creation by
returning an `llvm::Expected`.

Signed-off-by: Med Ismail Bennani <ismail@bennani.ma>

Signed-off-by: Med Ismail Bennani <ismail@bennani.ma>

show more ...

[lldb] Fix failure in TestStackCoreScriptedProcess on x86_64

This patch should address the failure of TestStackCoreScriptedProcess
that is happening specifically on x86_64.

It turns out that in 137

[lldb] Fix failure in TestStackCoreScriptedProcess on x86_64

This patch should address the failure of TestStackCoreScriptedProcess
that is happening specifically on x86_64.

It turns out that in 1370a1cb5b97, I changed the way we extract integers
from a `StructuredData::Dictionary` and in order to get a stop info from
the scripted process, we call a method that returns a `SBStructuredData`
containing the stop reason data.

TestStackCoreScriptedProcess` was failing specifically on x86_64 because
the stop info dictionary contains the signal number, that the `Scripted
Thread` was trying to extract as a signed integer where it was actually
parsed as an unsigned integer. That caused `GetValueForKeyAsInteger` to
return the default value parameter, `LLDB_INVALID_SIGNAL_NUMBER`.

This patch address the issue by extracting the signal number with the
appropriate type and re-enables the test.

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

Signed-off-by: Med Ismail Bennani <ismail@bennani.ma>

show more ...

[lldb] Introduce DynamicRegisterInfo::CreateFromDict

I want to add some error handling to DynamicRegisterInfo because there
are many operations that can fail and many of these operations do not
give

[lldb] Introduce DynamicRegisterInfo::CreateFromDict

I want to add some error handling to DynamicRegisterInfo because there
are many operations that can fail and many of these operations do not
give meaningful information back to the caller.

To begin that process, I want to add a static method that is responsible
for creating a DynamicRegisterInfo from a StructuredData::Dictionary
(and ArchSpec). This is meant to replace the equivalent constructor
because constructors are ill-equipped to perform error handling.

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

show more ...

Revert "[lldb] Fix failure in TestStackCoreScriptedProcess on x86_64"

This reverts commit 4177b490358432a457935ba5d6d076ae60de588f, since I
landed it by mistake.

[lldb] Fix failure in TestStackCoreScriptedProcess on x86_64

This patch should address the failure of TestStackCoreScriptedProcess
that is happening specifically on x86_64.

It turns out that in 137

[lldb] Fix failure in TestStackCoreScriptedProcess on x86_64

This patch should address the failure of TestStackCoreScriptedProcess
that is happening specifically on x86_64.

It turns out that in 1370a1cb5b97, I changed the way we extract integers
from a `StructuredData::Dictionary` and in order to get a stop info from
the scripted process, we call a method that returns a `SBStructuredData`
containing the stop reason data.

TestStackCoreScriptedProcess` was failing specifically on x86_64 because
the stop info dictionary contains the signal number, that the `Scripted
Thread` was trying to extract as a signed integer where it was actually
parsed as an unsigned integer. That caused `GetValueForKeyAsInteger` to
return the default value parameter, `LLDB_INVALID_SIGNAL_NUMBER`.

This patch address the issue by extracting the signal number with the
appropriate type and re-enables the test.

Signed-off-by: Med Ismail Bennani <ismail@bennani.ma>

show more ...

[lldb] Add "Trace" stop reason in Scripted Thread

This patch adds support to eStopReasonTrace to Scripted Threads.

This is necessary when using a Scrited Process with a Scripted Thread
Plan to repo

[lldb] Add "Trace" stop reason in Scripted Thread

This patch adds support to eStopReasonTrace to Scripted Threads.

This is necessary when using a Scrited Process with a Scripted Thread
Plan to report a special thread stop reason to the thread plan.

rdar://109425542

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

Signed-off-by: Med Ismail Bennani <ismail@bennani.ma>

show more ...

[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 bo

[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>

show more ...

[lldb/Plugins] Fix method dispatch bug when using multiple scripted processes

This patch should address a bug when a user have multiple scripted
processes in the same debugging session.

In order fo

[lldb/Plugins] Fix method dispatch bug when using multiple scripted processes

This patch should address a bug when a user have multiple scripted
processes in the same debugging session.

In order for the scripted process plugin to be able to call into the
scripted object instance methods to fetch the necessary data to
reconstruct its state, the scripted process plugin calls into a
scripted process interface, that has a reference to the created script
object instance.

However, prior to this patch, we only had a single instance of the
scripted process interface, living the script interpreter. So every time
a new scripted process plugin was created, it would overwrite the script
object instance that was held by the single scripted process interface
in the script interpreter.

That would cause all the method calls made to the scripted process
interface to be dispatched by the last instanciated script object
instance, which is wrong.

In order to prevent that, this patch moves the scripted process
interface reference to be help by the scripted process plugin itself.

rdar://104882562

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

Signed-off-by: Med Ismail Bennani <medismail.bennani@gmail.com>

show more ...

[lldb/Interpreter] Make ScriptedProcessInfo more generic

This patch moves the ScriptedProcessInfo class out of the
ScriptedProcess and hoist it as a standalone interpreter class, so it can be
reused

[lldb/Interpreter] Make ScriptedProcessInfo more generic

This patch moves the ScriptedProcessInfo class out of the
ScriptedProcess and hoist it as a standalone interpreter class, so it can be
reused with the Scripted Platform.

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

Signed-off-by: Med Ismail Bennani <medismail.bennani@gmail.com>

show more ...

[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/

[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

show more ...

[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::Optiona

[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

show more ...

[lldb/crashlog] Add support for Application Specific Backtraces & Information

For an exception crashlog, the thread backtraces aren't usually very helpful
and instead, developpers look at the "Appli

[lldb/crashlog] Add support for Application Specific Backtraces & Information

For an exception crashlog, the thread backtraces aren't usually very helpful
and instead, developpers look at the "Application Specific Backtrace" that
was generated by `objc_exception_throw`.

LLDB could already parse and symbolicate these Application Specific Backtraces
for regular textual-based crashlog, so this patch adds support to parse them
in JSON crashlogs, and materialize them a HistoryThread extending the
crashed ScriptedThread.

This patch also includes the Application Specific Information messages
as part of the process extended crash information log. To do so, the
ScriptedProcess Python interface has a new GetMetadata method that
returns an arbitrary dictionary with data related to the process.

rdar://93207586

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

Signed-off-by: Med Ismail Bennani <medismail.bennani@gmail.com>

show more ...

[lldb/Plugins] Cleanup error handling in Scripted{Process,Thread} (NFC)

Signed-off-by: Med Ismail Bennani <medismail.bennani@gmail.com>

[lldb] Fixed a number of typos

I went over the output of the following mess of a command:

(ulimit -m 2000000; ulimit -v 2000000; git ls-files -z | parallel
--xargs -0 cat | aspell list --mode=n

[lldb] Fixed a number of typos

I went over the output of the following mess of a command:

(ulimit -m 2000000; ulimit -v 2000000; git ls-files -z | parallel
--xargs -0 cat | aspell list --mode=none --ignore-case | grep -E
'^[A-Za-z][a-z]*$' | sort | uniq -c | sort -n | grep -vE '.{25}' |
aspell pipe -W3 | grep : | cut -d' ' -f2 | less)

and proceeded to spend a few days looking at it to find probable typos
and fixed a few hundred of them in all of the llvm project (note, the
ones I found are not anywhere near all of them, but it seems like a
good start).

Differential revision: https://reviews.llvm.org/D131122

show more ...

[lldb/crashlog] Improve exception reporting for interactive mode

This patch improve exception reporting when loading a crash report in a
scripted process. Now, we parse the `exception` dictionary fr

[lldb/crashlog] Improve exception reporting for interactive mode

This patch improve exception reporting when loading a crash report in a
scripted process. Now, we parse the `exception` dictionary from the
crash report use it the create a higher fidelity `MachException` stop info.

This patch also updates the test to reflect that change.

rdar://97096486

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

Signed-off-by: Med Ismail Bennani <medismail.bennani@gmail.com>

show more ...

[lldb/Plugin] Use static ScriptedInterface::ErrorWithMessage function (NFC)

This patch replaces the calls to ErrorWithMessage using the GetInterface
message by a call to the static method directly.

[lldb/Plugin] Use static ScriptedInterface::ErrorWithMessage function (NFC)

This patch replaces the calls to ErrorWithMessage using the GetInterface
message by a call to the static method directly.

Signed-off-by: Med Ismail Bennani <medismail.bennani@gmail.com>

show more ...

[lldb/Plugin] Add artificial stackframe loading in ScriptedThread

This patch adds the ability for ScriptedThread to load artificial stack
frames. To do so, the interpreter instance can create a list

[lldb/Plugin] Add artificial stackframe loading in ScriptedThread

This patch adds the ability for ScriptedThread to load artificial stack
frames. To do so, the interpreter instance can create a list that will
contain the frame index and its pc address.

Then, when the Scripted Process plugin stops, it will refresh its
Scripted Threads state by invalidating their register context and load
to list from the interpreter object and reconstruct each frame.

This patch also removes all of the default implementation for
`get_stackframes` from the derived ScriptedThread classes, and add the
interface code for the Scripted Thread Interface.

rdar://88721095

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

Signed-off-by: Med Ismail Bennani <medismail.bennani@gmail.com>

show more ...

[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
inf

[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.

show more ...

[lldb] Convert "LLDB" log channel to the new API

[lldb/Plugins] Fix ScriptedThread IndexID reporting

When listing all the Scripted Threads of a ScriptedProcess, we can see that all
have the thread index set to 1. This is caused by the lldb_private

[lldb/Plugins] Fix ScriptedThread IndexID reporting

When listing all the Scripted Threads of a ScriptedProcess, we can see that all
have the thread index set to 1. This is caused by the lldb_private::Thread
constructor, which sets the m_index_id member using the provided thread id `tid`.

Because the call to the super constructor is done before instantiating
the `ScriptedThreadInterface`, lldb can't fetch the thread id from the
script instance, so it uses `LLDB_INVALID_THREAD_ID` instead.

To mitigate this, this patch takes advantage of the `ScriptedThread::Create`
fallible constructor idiom to defer calling the `ScriptedThread` constructor
(and the `Thread` super constructor with it), until we can fetch a valid
thread id `tid` from the `ScriptedThreadInterface`.

rdar://87432065

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

Signed-off-by: Med Ismail Bennani <medismail.bennani@gmail.com>

show more ...

[lldb/Plugins] Enrich ScriptedThreads Stop Reasons with Exceptions

This patch adds Exceptions to the list of supported stop reasons for
Scripted Threads.

The main motivation for this is that breakp

[lldb/Plugins] Enrich ScriptedThreads Stop Reasons with Exceptions

This patch adds Exceptions to the list of supported stop reasons for
Scripted Threads.

The main motivation for this is that breakpoints are triggered as a
special exception class on ARM platforms, so adding it as a stop reason
allows the ScriptedProcess to selected the ScriptedThread that stopped at
a breakpoint (or crashed :p).

rdar://87430376

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

Signed-off-by: Med Ismail Bennani <medismail.bennani@gmail.com>

show more ...