The problem is that the when the "attach" command is initiated, the
ExecutionContext for the command has a process - it's the exited one
from the previour run. But the `attach wait` creates a new process for
the attach, and then errors out instead of interrupting when it finds
that its process and the one in the command's ExecutionContext don't
match.
This change checks that if we're returning a target from
GetExecutionContext, we fill the context with it's current process, not
some historical one.
Some functions in Process were using LLDB_INVALID_ADDRESS instead of
LLDB_INVALID_TOKEN.
The only visible effect of this appears to be that "process unload
<tab>" would complete to 0 even after the image was unloaded. Since the
command is checking for LLDB_INVALID_TOKEN.
Everything else worked somehow. I've added a check to the existing load
unload tests anyway.
The tab completion cannot be checked as is, but when I make them more
strict in a later patch it will be tested.
D157648 broke the function because it put the blocking wait into a
critical section. This meant that, if m_iohandler_sync was not updated
before entering the function, no amount of waiting would help.
Fix that by restriciting the scope of the critical section to the
iohandler check.
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
Thread sanitizer reports a data race in Process.cpp in the usage of
m_process_input_reader. Fix this data race by introducing a mutex
guarding the access to this variable.
Differential Revision: https://reviews.llvm.org/D157648
ThreadSanitizer reports the following issue:
```
Write of size 8 at 0x00010a70abb0 by thread T3 (mutexes: write M0):
#0 lldb_private::ThreadList::Update(lldb_private::ThreadList&) ThreadList.cpp:741 (liblldb.18.0.0git.dylib:arm64+0x5dedf4) (BuildId: 9bced2aafa373580ae9d750d9cf79a8f32000000200000000100000000000e00)
#1 lldb_private::Process::UpdateThreadListIfNeeded() Process.cpp:1212 (liblldb.18.0.0git.dylib:arm64+0x53bbec) (BuildId: 9bced2aafa373580ae9d750d9cf79a8f32000000200000000100000000000e00)
Previous read of size 8 at 0x00010a70abb0 by main thread (mutexes: write M1):
#0 lldb_private::ThreadList::GetMutex() const ThreadList.cpp:785 (liblldb.18.0.0git.dylib:arm64+0x5df138) (BuildId: 9bced2aafa373580ae9d750d9cf79a8f32000000200000000100000000000e00)
#1 lldb_private::ThreadList::DidResume() ThreadList.cpp:656 (liblldb.18.0.0git.dylib:arm64+0x5de5c0) (BuildId: 9bced2aafa373580ae9d750d9cf79a8f32000000200000000100000000000e00)
#2 lldb_private::Process::PrivateResume() Process.cpp:3130 (liblldb.18.0.0git.dylib:arm64+0x53cd7c) (BuildId: 9bced2aafa373580ae9d750d9cf79a8f32000000200000000100000000000e00)
```
Fix this by only using the mutex in ThreadList and removing the one in
process entirely.
Differential Revision: https://reviews.llvm.org/D158034
Before the addition of the process "Shadow Listener" you could only have one
Listener observing the Process Broadcaster. That was necessary because fetching the
Process event is what switches the public process state, and for the execution
control logic to be manageable you needed to keep other listeners from causing
this to happen before the main process control engine was ready.
Ismail added the notion of a "ShadowListener" - which allowed you ONE
extra process listener. This patch inverts that setup by designating the
first listener as primary - and giving it priority in fetching events.
Differential Revision: https://reviews.llvm.org/D157556
My primary motivation here is actually to change something in
UnixSignals, but this change is a necesary precondition.
I've also updated the documentation and rewritten the log statements to
use `formatv` instead of `printf` (printf-style formatting and
llvm::StringRef don't mix well).
Differential Revision: https://reviews.llvm.org/D157662
StreamFile subclasses Stream (from lldbUtility) and is backed by a File
(from lldbHost). It does not depend on anything from lldbCore or any of its
sibling libraries, so I think it makes sense for this to live in
lldbHost instead.
Differential Revision: https://reviews.llvm.org/D157460
Fix incorrect uses of formatv specifiers in LLDB_LOG. Unlike Python,
arguments must be numbered. All the affected log statements take
llvm:Errors so use the LLDB_LOG_ERROR macro instead.
Differential revision: https://reviews.llvm.org/D154532
This patch should fix some data races when a python script (i.e. a
Scripted Process) has a nested call to another python script (i.e. a
OperatingSystem Plugin), which can cause concurrent writes to the python
lock count.
This patch also fixes a data race happening when resetting the operating
system unique pointer.
To address these issues, both accesses is guarded by a mutex.
rdar://109413039
Differential Revision: https://reviews.llvm.org/D154271
Signed-off-by: Med Ismail Bennani <ismail@bennani.ma>
I removed ConstString from OptionValueProperties in 643ba926c1, but
there are a few call sites that still create a ConstString as an
argument. I did not catch these initially because ConstString has an
implicit conversion method to StringRef.
Differential Revision: https://reviews.llvm.org/D153673
The use of ConstString in StructuredDataPlugin is unneccessary as fast
comparisons are not neeeded for StructuredDataPlugins.
Differential Revision: https://reviews.llvm.org/D153482
To aid in integration testing/debugging. Verifying that the address
mask/addressable bits values from different sources are correctly
registered by lldb.
We need to clear non-addressable bits from addresses across
the lldb sources. Currently these need to use an ABI method
to clear those bits from addresses, which you do by taking a
Process, getting the current ABI, then calling the method.
Simplify this by providing methods in Process which call into
the ABI methods themselves.
Differential Revision: https://reviews.llvm.org/D152863
Instead of having a map from ConstString to StructuredDataPluginSP, we
can use an llvm::StringMap. The keys themselves don't need to be
ConstStrings, so an llvm::StringMap feels most natural.
Differential Revision: https://reviews.llvm.org/D151960
This patch changes the way we load a crash report into a scripted
process by creating a empty target.
To do so, it parses the architecture information from the report (for
both the legacy and json format) and uses that to create a target that
doesn't have any executable, like what we do when attaching to a process.
For the legacy format, we mostly rely on the `Code Type` line, since the
architure is an optional field on the `Binary Images` sections.
However for the json format, we first try to get the architecture while
parsing the image dictionary if we couldn't find it, we try to infer it
using the "flavor" key when parsing the frame's registers.
If the architecture is still not set after parsing the report, we raise
an exception.
rdar://107850263
Differential Revision: https://reviews.llvm.org/D151849
Differential
Signed-off-by: Med Ismail Bennani <ismail@bennani.ma>
On AArch64, it is possible to have a program that accesses both low
(0x000...) and high (0xfff...) memory, and with pointer authentication,
you can have different numbers of bits used for pointer authentication
depending on whether the address is in high or low memory.
This adds a new target.process.highmem-virtual-addressable-bits
setting which the AArch64 Mac ABI plugin will use, when set, to
always set those unaddressable high bits for high memory addresses,
and will use the existing target.process.virtual-addressable-bits
setting for low memory addresses.
This patch does not change the existing behavior when only
target.process.virtual-addressable-bits is set. In that case, the
value will apply to all addresses.
Not yet done is recognizing metadata in a live process connection
(gdb-remote qHostInfo) or a Mach-O corefile LC_NOTE to set the
correct number of addressing bits for both memory ranges. That
will be a future change.
Differential Revision: https://reviews.llvm.org/D151292
rdar://109746900
This patch fixes the log commands by replacing the LLDB_LOG macro by the
LLDB_LOGF macro. This is necessary in order to format argument with printf.
Signed-off-by: Med Ismail Bennani <ismail@bennani.ma>
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
The majority of call sites are nullptr as the execution context.
Refactor OptionValueProperties to make the argument optional and
simplify all the callers.
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.
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.
This patch improves process state change logging messages to include to
process plugin name.
It also replaces the `LLDB_LOGF` macro by `LLDB_LOG` macro that adds the
class and method name in the log message using the compiler instead of
having to change the string litteral for every method.
This is very useful when investigating interactions between different
types of process plugins. That comes very handy when investigating bugs
related to interactive scripted process debugging.
Differential Revision: https://reviews.llvm.org/D148399
Signed-off-by: Med Ismail Bennani <medismail.bennani@gmail.com>
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
This patch should address an issue that caused the process public run
lock to not be updated during a process launch/attach when the process
stops.
That caused the public run lock to report its state as running while the
process state is stopped. This prevents the users to interact with the
process (through the command line or via the SBAPI) since it's
considered still running.
To address that, this patch refactors the name of the internal hijack
listeners to a specific pattern `lldb.internal.<action>.hijack` that
are used to ensure that we've attached to or launched a process successfully.
Then, when updating the process public state, after updating the state
value, if the process is not hijacked externally, meaning if the process
doens't have a hijack listener that matches the internal hijack
listeners pattern, we can update the public run lock accordingly.
rdar://108283017
Differential Revision: https://reviews.llvm.org/D148400
Signed-off-by: Med Ismail Bennani <medismail.bennani@gmail.com>
These probably do not need to be in the ConstString StringPool as they
don't really need any of the advantages that ConstStrings offer.
Lifetime for these things is always static and we never need to perform
comparisons for setting descriptions.
Differential Revision: https://reviews.llvm.org/D148679
This patch should address an issue that caused the process public run
lock to not be updated during a process launch/attach when the process
stops.
That caused the public run lock to report its state as running while the
process state is stopped. This prevents the users to interact with the
process (through the command line or via the SBAPI) since it's
considered still running.
To address that, this patch refactors the name of the internal hijack
listeners to a specific pattern `lldb.internal.<action>.hijack` that
are used to ensure that we've attached to or launched a process successfully.
Then, when updating the process public state, after updating the state
value, if the process is not hijacked externally, meaning if the process
doens't have a hijack listener that matches the internal hijack
listeners pattern, we can update the public run lock accordingly.
Differential Revision: https://reviews.llvm.org/D148400
Signed-off-by: Med Ismail Bennani <medismail.bennani@gmail.com>
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
lldb was originally designed to get the watchpoint exception behavior
from the gdb remote serial protocol stub -- exceptions are either
received before the instruction executes, or after the instruction
has executed. This behavior was reported via two lldb extensions
to gdb RSP, so generic remote stubs like gdbserver or a JTAG stub,
would not tell lldb which behavior was correct, and it would default
to "exceptions are received after the instruction has executed".
Two architectures hard coded their correct "exceptions before
instruction" behavior, to work around this issue.
Most architectures have a fixed behavior of watchpoint exceptions,
and we can center that information in lldb. We can allow a remote
stub to override the default behavior via our packet extensions
if it's needed on a specific target.
This patch also separates the fetching of the number of watchpoints
from whether exceptions are before/after the insn. Currently if
lldb couldn't fetch the number of watchpoints (not really needed), it
also wouldn't get when exceptions are received, and watchpoint
handling would fail. lldb doesn't actually use the number of
watchpoints for anything beyond printing it to the user.
Differential Revision: https://reviews.llvm.org/D143215
rdar://101426626
Summary: For functions that accept an rvalue reference type
parameter, use move to avoid copying the parameter.
These were found when implementing CppCoreGuideline F.18 in
clang-tidy.
Committed on behalf of ccotter (Chris Cotter)
Reviewers: Michael137
Differential Revision: https://reviews.llvm.org/D142824
This patch is preparatory work for Scripted Platform support and does
multiple things:
First, it introduces new options for the `platform select` command and
`SBPlatform::Create` API, to hold a reference to the debugger object,
the name of the python script managing the Scripted Platform and a
structured data dictionary that the user can use to pass arbitrary data.
Then, it updates the various `Create` and `GetOrCreate` methods for
the `Platform` and `PlatformList` classes to pass down the new parameter
to the `Platform::CreateInstance` callbacks.
Finally, it updates every callback to reflect these changes.
Differential Revision: https://reviews.llvm.org/D139249
Signed-off-by: Med Ismail Bennani <medismail.bennani@gmail.com>
After an exec has occured, resources used to manage the state of a
Process are cleaned up. One such resource is the AllocatedMemoryCache
which keeps track of memory allocations made in the process for things
like expression evaluation. After an exec is performed, the allocated
memory regions in the process are gone, so it does not make sense to try
to deallocate those regions.
rdar://103188106
Differential Revision: https://reviews.llvm.org/D140249
This patch mechanically replaces None with std::nullopt where the
compiler would warn if None were deprecated. The intent is to reduce
the amount of manual work required in migrating from Optional to
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
When a process gets restarted TypeSystem objects associated with it
may get deleted, and any CompilerType objects holding on to a
reference to that type system are a use-after-free in waiting. Because
of the SBAPI, we don't have tight control over where CompilerTypes go
and when they are used. This is particularly a problem in the Swift
plugin, where the scratch TypeSystem can be restarted while the
process is still running. The Swift plugin has a lock to prevent
abuse, but where there's a lock there can be bugs.
This patch changes CompilerType to store a std::weak_ptr<TypeSystem>.
Most of the std::weak_ptr<TypeSystem>* uglyness is hidden by
introducing a wrapper class CompilerType::WrappedTypeSystem that has a
dyn_cast_or_null() method. The only sites that need to know about the
weak pointer implementation detail are the ones that deal with
creating TypeSystems.
rdar://101505232
Differential Revision: https://reviews.llvm.org/D136650
We generally prohibit this at a higher level - for instance requiring
the process to be stopped for "expr". But when we trigger an expression
for internal purposes (e.g. to fetch types from the ObjC runtime) we weren't
checking the process state. Now we explicitly check this at the very start
of the job so we don't get into bad states.
Differential Revision: https://reviews.llvm.org/D137684
This ensures it is run regardless of the method we use to initiate the
session (previous version did not handle connects), and it is the same
place that is used for resetting watchpoints.
Differential Revision: https://reviews.llvm.org/D134882
