Revert "[lldb] Add 'modify' type watchpoints, make it default (#66308)"

This reverts commit 933ad5c897ee366759a54869b35b2d7285a92137.

This caused 1 test failure and an unexpected pass on AArch64 Li

Revert "[lldb] Add 'modify' type watchpoints, make it default (#66308)"

This reverts commit 933ad5c897ee366759a54869b35b2d7285a92137.

This caused 1 test failure and an unexpected pass on AArch64 Linux:
https://lab.llvm.org/buildbot/#/builders/96/builds/45765

Wasn't reported because the bot was already red at the time.

show more ...

[lldb] Add 'modify' type watchpoints, make it default (#66308)

Watchpoints in lldb can be either 'read', 'write', or 'read/write'. This
is exposing the actual behavior of hardware watchpoints. gdb h

[lldb] Add 'modify' type watchpoints, make it default (#66308)

Watchpoints in lldb can be either 'read', 'write', or 'read/write'. This
is exposing the actual behavior of hardware watchpoints. gdb has a
different behavior: a "write" type watchpoint only stops when the
watched memory region *changes*.

A user is using a watchpoint for one of three reasons:

1. Want to find what is changing/corrupting this memory.
2. Want to find what is writing to this memory.
3. Want to find what is reading from this memory.

I believe (1) is the most common use case for watchpoints, and it
currently can't be done in lldb -- the user needs to continue every time
the same value is written to the watched-memory manually. I think gdb's
behavior is the correct one. There are some use cases where a developer
wants to find every function that writes/reads to/from a memory region,
regardless of value, I want to still allow that functionality.

This is also a bit of groundwork for my large watchpoint support
proposal
https://discourse.llvm.org/t/rfc-large-watchpoint-support-in-lldb/72116
where I will be adding support for AArch64 MASK watchpoints which watch
power-of-2 memory regions. A user might ask to watch 24 bytes, and a
MASK watchpoint stub can do this with a 32-byte MASK watchpoint if it is
properly aligned. And we need to ignore writes to the final 8 bytes of
that watched region, and not show those hits to the user.

This patch adds a new 'modify' watchpoint type and it is the default.

Re-landing this patch after addressing testsuite failures found in CI on
Linux, Intel machines, and windows.

rdar://108234227

show more ...

[lldb][AArch64] Implement resizing of SME's ZA register

The size of ZA depends on the streaming vector length regardless
of the active mode. So in addition to vg (which reports the active
mode) we m

[lldb][AArch64] Implement resizing of SME's ZA register

The size of ZA depends on the streaming vector length regardless
of the active mode. So in addition to vg (which reports the active
mode) we must send the client svg.

Otherwise the mechanics are the same as for non-streaming SVE.
Use the svg value to update the defined size of ZA, accounting
for the fact that ZA is not a single vector but a suqare matrix.

So if svg is 8, a single streaming vector would be 8*8 = 64 bytes.
ZA is that squared, so 64*64 = 4096 bytes.

Testing is included in a later patch.

Reviewed By: omjavaid

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

show more ...

Revert "[lldb] Add 'modify' type watchpoints, make it default (#66308)"

TestStepOverWatchpoint.py and TestUnalignedWatchpoint.py are failing
on the ubuntu and debian bots
https://lab.llvm.org/buildb

Revert "[lldb] Add 'modify' type watchpoints, make it default (#66308)"

TestStepOverWatchpoint.py and TestUnalignedWatchpoint.py are failing
on the ubuntu and debian bots
https://lab.llvm.org/buildbot/#/builders/68/builds/60204
https://lab.llvm.org/buildbot/#/builders/96/builds/45623

and the newly added test TestModifyWatchpoint.py does not
work on windows bot
https://lab.llvm.org/buildbot/#/builders/219/builds/5708

I will debug tomorrow morning and reland.

This reverts commit 3692267ca8f9c51cb55e4387283762d921fe2ae2.

show more ...

[lldb] Add 'modify' type watchpoints, make it default (#66308)

Watchpoints in lldb can be either 'read', 'write', or 'read/write'. This
is exposing the actual behavior of hardware watchpoints. gdb

[lldb] Add 'modify' type watchpoints, make it default (#66308)

Watchpoints in lldb can be either 'read', 'write', or 'read/write'. This
is exposing the actual behavior of hardware watchpoints. gdb has a
different behavior: a "write" type watchpoint only stops when the
watched memory region *changes*.

A user is using a watchpoint for one of three reasons:

1. Want to find what is changing/corrupting this memory.
2. Want to find what is writing to this memory.
3. Want to find what is reading from this memory.

I believe (1) is the most common use case for watchpoints, and it
currently can't be done in lldb -- the user needs to continue every time
the same value is written to the watched-memory manually. I think gdb's
behavior is the correct one. There are some use cases where a developer
wants to find every function that writes/reads to/from a memory region,
regardless of value, I want to still allow that functionality.

This is also a bit of groundwork for my large watchpoint support
proposal
https://discourse.llvm.org/t/rfc-large-watchpoint-support-in-lldb/72116
where I will be adding support for AArch64 MASK watchpoints which watch
power-of-2 memory regions. A user might ask to watch 24 bytes, and a
MASK watchpoint stub can do this with a 32-byte MASK watchpoint if it is
properly aligned. And we need to ignore writes to the final 8 bytes of
that watched region, and not show those hits to the user.

This patch adds a new 'modify' watchpoint type and it is the default.

rdar://108234227

show more ...

[lldb][NFCI] Remove use of ConstString in StructuredData

The remaining use of ConstString in StructuredData is the Dictionary
class. Internally it's backed by a `std::map<ConstString, ObjectSP>`.
I

[lldb][NFCI] Remove use of ConstString in StructuredData

The remaining use of ConstString in StructuredData is the Dictionary
class. Internally it's backed by a `std::map<ConstString, ObjectSP>`.
I propose that we replace it with a `llvm::StringMap<ObjectSP>`.

Many StructuredData::Dictionary objects are ephemeral and only exist for
a short amount of time. Many of these Dictionaries are only produced
once and are never used again. That leaves us with a lot of string data
in the ConstString StringPool that is sitting there never to be used
again. Even if the same string is used many times for keys of different
Dictionary objects, that is something we can measure and adjust for
instead of assuming that every key may be reused at some point in the
future.

Quick comparisons of key data is likely not a concern with Dictionary,
but the use of `llvm::StringMap` means that lookups should be fast with
its hashing strategy.

Switching to a llvm::StringMap meant that the iteration order may be
different. To account for this when serializing/dumping the dictionary,
I added some code to sort the output by key before emitting anything.

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

show more ...

[lldb] Fix duplicate word typos; NFC

Those fixes were taken from https://reviews.llvm.org/D137338

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

[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

show more ...

[lldb] Change UnixSignals::GetSignalAsCString to GetSignalAsStringRef

This is in preparation to remove the uses of ConstString from
UnixSignals.

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

[lldb] Change UnixSignals::GetSignalAsCString to GetSignalAsStringRef

This is in preparation to remove the uses of ConstString from
UnixSignals.

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

show more ...

[lldb][NFCI] Rewrite error-handling code in ProcessGDBRemote::MonitorDebugserverProcess

I'm rewriting this for 2 reasons:
1) Although a 1024 char buffer is probably enough space, the error
string

[lldb][NFCI] Rewrite error-handling code in ProcessGDBRemote::MonitorDebugserverProcess

I'm rewriting this for 2 reasons:
1) Although a 1024 char buffer is probably enough space, the error
string may grow beyond that and we could lose some information. Using
StringStream (as we do in the rest of ProcessGDBRemote) seems like a
sensible solution.
2) I am planning on changing `UnixSignals::GetSignalAsCString`,
rewriting things with `llvm::formatv` will make that process easier.

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

show more ...

hi/low addr space bits can be sent in stop-rely packet

Add support for the `low_mem_addressing_bits` and
`high_mem_addressing_bits` keys in the stop reply packet,
in addition to the existing `addres

hi/low addr space bits can be sent in stop-rely packet

Add support for the `low_mem_addressing_bits` and
`high_mem_addressing_bits` keys in the stop reply packet,
in addition to the existing `addressing_bits`. Same
behavior as in the qHostInfo packet.

Clean up AddressableBits so we don't need to check if
any values have been set in the object before using it
to potentially update the Process address masks.

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

show more ...

Update qHostInfo/LC_NOTE so multiple address bits can be specified

On AArch64 systems, we may have different page table setups for
low memory and high memory, and therefore a different number of
bit

Update qHostInfo/LC_NOTE so multiple address bits can be specified

On AArch64 systems, we may have different page table setups for
low memory and high memory, and therefore a different number of
bits used for addressing depending on which half of memory the
address is in.

This patch extends the qHostInfo and LC_NOTE "addrable bits" so
that it can specify the number of addressing bits in high memory
and in low memory separately. It builds on the patch I added in
https://reviews.llvm.org/D151292 where Process tracks the separate
address masks, and there is a user setting to set them manually.

Differential Revision: https://reviews.llvm.org/D157667
rdar://113225907

show more ...

[lldb] Sink StreamFile into lldbHost

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 libra

[lldb] Sink StreamFile into lldbHost

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

show more ...

Flag for LoadBinaryWithUUIDAndAddress, to create memory image or not

DynamicLoader::LoadBinaryWithUUIDAndAddress can create a Module based
on the binary image in memory, which in some cases contains

Flag for LoadBinaryWithUUIDAndAddress, to create memory image or not

DynamicLoader::LoadBinaryWithUUIDAndAddress can create a Module based
on the binary image in memory, which in some cases contains symbol
names and can be genuinely useful. If we don't have a filename, it
creates a name in the form `memory-image-0x...` with the header address.

In practice, this is most useful with Darwin userland corefiles
where the binary was stored in the corefile in whole, and we can't
find a binary with the matching UUID. Using the binary out of
the corefile memory in this case works well.

But in other cases, akin to firmware debugging, we merely end up
with an oddly named binary image and no symbols.

Add a flag to control whether we will create these memory images
and add them to the Target or not; only set it to true when working
with a userland Mach-O image with the "all image infos" LC_NOTE for
a userland corefile.

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

show more ...

Add a generic Process method to dump plugin history.

I need to call this to figure out why the assert in
StopInfoMachException::CreateStopReasonWithMachException is triggering, but
it isn't appropri

Add a generic Process method to dump plugin history.

I need to call this to figure out why the assert in
StopInfoMachException::CreateStopReasonWithMachException is triggering, but
it isn't appropriate to directly access the GDBRemoteCommunication there. And
dumping whatever history the process plugin has collected during the run isn't
gdb-remote specific...

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

show more ...

[lldb] Fix incorrect uses of LLDB_LOG_ERROR

Fix incorrect uses of LLDB_LOG_ERROR. The macro doesn't automatically
inject the error in the log message: it merely passes the error as the
first argumen

[lldb] Fix incorrect uses of LLDB_LOG_ERROR

Fix incorrect uses of LLDB_LOG_ERROR. The macro doesn't automatically
inject the error in the log message: it merely passes the error as the
first argument to formatv and therefore must be referenced with {0}.

Thanks to Nicholas Allegra for collecting a list of places where the
macro was misused.

rdar://111581655

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

show more ...

[lldb][NFCI] Remove ConstString from Process::ConfigureStructuredData

This is a follow-up to b4827a3c0a7ef121ca376713e115b04eff0f5194.

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

Fix a few bugs with Mach-O corefile loading, plus perf

In ProcessMachCore::LoadBinariesViaMetadata(), if we did
load some binaries via metadata in the core file, don't
then search for a userland dyl

Fix a few bugs with Mach-O corefile loading, plus perf

In ProcessMachCore::LoadBinariesViaMetadata(), if we did
load some binaries via metadata in the core file, don't
then search for a userland dyld in the corefile / kernel
and throw away that binary list. Also fix a little bug
with correctly recognizing corefiles using a `main bin spec`
LC_NOTE that explicitly declare that this is a userland
corefile.

LocateSymbolFileMacOSX.cpp's Symbols::DownloadObjectAndSymbolFile
clarify the comments on how the force_lookup and how the
dbgshell_command local both have the same effect.

In PlatformDarwinKernel::LoadPlatformBinaryAndSetup, don't
log a message unless we actually found a kernel fileset.

Reorganize ObjectFileMachO::LoadCoreFileImages so that it delegates
binary searching to DynamicLoader::LoadBinaryWithUUIDAndAddress and
doesn't duplicate those searches. For searches that fail, we would
perform them multiple times in both methods. When we have the
mach-o segment vmaddrs for a binary, don't let LoadBinaryWithUUIDAndAddress
load the binary first at its mach-o header address in the Target;
we'll load the segments at the correct addresses individually later
in this method.

DynamicLoaderDarwin::ImageInfo::PutToLog fix a LLDB_LOG logging
formatter.

In DynamicLoader::LoadBinaryWithUUIDAndAddress, instead of using
Target::GetOrCreateModule as a way to find a binary already registered
in lldb's global module cache (and implicitly add it to the Target
image list), use ModuleList::GetSharedModule() which only searches
the global module cache, don't add it to the Target. We may not
want to add an unstripped binary to the Target.

Add a call to Symbols::DownloadObjectAndSymbolFile() even if
"force_symbol_search" isn't set -- this will turn into a
DebugSymbols call / Spotlight search on a macOS system, which
we want.

Only set the Module's LoadAddress if the caller asked us to do that.

Differential Revision: https://reviews.llvm.org/D150928
rdar://109186357

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] Eliminate {Get,Set}PropertyAtIndexAsFileSpec (NFC)

This patch is a continuation of 6f8b33f6dfd0 and eliminates the
{Get,Set}PropertyAtIndexAsFileSpec functions.

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

[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

show more ...

Recognize `addressing_bits` kv in stop reply packet

If a remote stub provides the addressing_bits kv pair in
the stop reply packet, update the Process address masks with
that value as it possibly ch

Recognize `addressing_bits` kv in stop reply packet

If a remote stub provides the addressing_bits kv pair in
the stop reply packet, update the Process address masks with
that value as it possibly changes during the process runtime.
This is an unusual situation, most likely a JTAG remote stub
and some very early startup code that is setting up the page
tables. Nearly all debug sessions will have a single address
mask that cannot change during the lifetime of a Process.

Differential Revision: https://reviews.llvm.org/D149803
rdar://61900565

show more ...

[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

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

show more ...

[lldb] Refactor OptionValueProperties to return a std::optional (NFC)

Similar to fdbe7c7faa54, refactor OptionValueProperties to return a
std::optional instead of taking a fail value. This allows th

[lldb] Refactor OptionValueProperties to return a std::optional (NFC)

Similar to fdbe7c7faa54, 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.

show more ...

[LLDB] Discard register flags where the size doesn't match the register

In the particular case I was looking at I autogenerated a 128 bit set
of flags that is only 64 bit. This doesn't crash lldb bu

[LLDB] Discard register flags where the size doesn't match the register

In the particular case I was looking at I autogenerated a 128 bit set
of flags that is only 64 bit. This doesn't crash lldb but it was certainly
not expected.

I suspect that we would have crashed if the top 64 bits weren't
marked as unused (or at least invoked some very undefined behaviour).

When this happens, log the details and ignore the flags. Like this:
```
Size of register flags TTBR0_EL1_flags (16 bytes) for register TTBR0_EL1 does not match the register size (8 bytes). Ignoring this set of flags.
```

Turns out a few of the tests relied on this bug so I have updated
them and added a specific test for this case.

Reviewed By: jasonmolenda

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

show more ...