294 Commits

Author SHA1 Message Date
Jacob Lalonde
6a7f572ef9
[LLDB] Fix Memory64 BaseRVA, move all non-stack memory to Mem64. (#146777)
### Context

Over a year ago, I landed support for 64b Memory ranges in Minidump
(#95312). In this patch we added the Memory64 list stream, which is
effectively a Linked List on disk. The layout is a sixteen byte header
and then however many Memory descriptors.

### The Bug
This is a classic off-by one error, where I added 8 bytes instead of 16
for the header. This caused the first region to start 8 bytes before the
correct RVA, thus shifting all memory reads by 8 bytes. We are correctly
writing all the regions to disk correctly, with no physical corruption
but the RVA is defined wrong, meaning we were incorrectly reading memory


![image](https://github.com/user-attachments/assets/049ef55d-856c-4f3c-9376-aeaa3fe8c0e1)


### Why wasn't this caught?

One problem we've had is forcing Minidump to actually use the 64b mode,
it would be a massive waste of resources to have a test that actually
wrote >4.2gb of IO to validate the 64b regions, and so almost all
validation has been manual. As a weakness of manual testing, this issue
is psuedo non-deterministic, as what regions end up in 64b or 32b is
handled greedily and iterated in the order it's laid out in
/proc/pid/maps. We often validated 64b was written correctly by
hexdumping the Minidump itself, which was not corrupted (other than the
BaseRVA)


![image](https://github.com/user-attachments/assets/b599e3be-2d59-47e2-8a2d-75f182bb0b1d)

### Why is this showing up now?

During internal usage, we had a bug report that the Minidump wasn't
displaying values. I was unable to repro the issue, but during my
investigation I saw the variables were in the 64b regions which resulted
in me identifying the bug.

### How do we prevent future regressions?

To prevent regressions, and honestly to save my sanity for figuring out
where 8 bytes magically came from, I've added a new API to
SBSaveCoreOptions.

```SBSaveCoreOptions::GetMemoryRegionsToSave()```
The ability to get the memory regions that we intend to include in the Coredump. I added this so we can compare what we intended to include versus what was actually included. Traditionally we've always had issues comparing regions because Minidump includes `/proc/pid/maps` and it can be difficult to know what memoryregion read failure was a genuine error or just a page that wasn't meant to be included. 

We are also leveraging this API to choose the memory regions to be generated, as well as for testing what regions should be bytewise 1:1.

After much debate with @clayborg, I've moved all non-stack memory to the Memory64 List. This list doesn't incur us any meaningful overhead and Greg originally suggested doing this in the original 64b PR. This also means we're exercising the 64b path every single time we save a Minidump, preventing regressions on this feature from slipping through testing in the future.

Snippet produced by [minidump.py](https://github.com/clayborg/scripts) 
```
MINIDUMP_MEMORY_LIST:
NumberOfMemoryRanges = 0x00000002
MemoryRanges[0] = [0x00007f61085ff9f0 - 0x00007f6108601000) @ 0x0003f655
MemoryRanges[1] = [0x00007ffe47e50910 - 0x00007ffe47e52000) @ 0x00040c65

MINIDUMP_MEMORY64_LIST:
NumberOfMemoryRanges = 0x000000000000002e
BaseRva              = 0x0000000000042669
MemoryRanges[0]      = [0x00005584162d8000 - 0x00005584162d9000)
MemoryRanges[1]      = [0x00005584162d9000 - 0x00005584162db000)
MemoryRanges[2]      = [0x00005584162db000 - 0x00005584162dd000)
MemoryRanges[3]      = [0x00005584162dd000 - 0x00005584162ff000)
MemoryRanges[4]      = [0x00007f6100000000 - 0x00007f6100021000)
MemoryRanges[5]      = [0x00007f6108800000 - 0x00007f6108828000)
MemoryRanges[6]      = [0x00007f6108828000 - 0x00007f610899d000)
MemoryRanges[7]      = [0x00007f610899d000 - 0x00007f61089f9000)
MemoryRanges[8]      = [0x00007f61089f9000 - 0x00007f6108a08000)
MemoryRanges[9]      = [0x00007f6108bf5000 - 0x00007f6108bf7000)
```

### Misc
As a part of this fix I had to look at LLDB logs a lot, you'll notice I added `0x` to many of the PRIx64 `LLDB_LOGF`. This is so the user (or I) can directly copy paste the address in the logs instead of adding the hex prefix themselves.

Added some SBSaveCore tests for the new GetMemoryAPI, and Docstrings.

CC: @DavidSpickett, @da-viper @labath because we've been working together on save-core plugins, review it optional and I didn't tag you but figured you'd want to know
2025-07-18 13:05:15 -07:00
Jonas Devlieghere
d193a586c0
[lldb] Change breakpoint interfaces for error handling (#146972)
This RP changes some Breakpoint-related interfaces to return errors. On
its own these improvements are small, but they encourage better error
handling going forward. There are a bunch of other candidates, but these
were the functions that I touched while working on #146602.
2025-07-09 13:19:02 -07:00
Ebuka Ezike
8d83d04637
[lldb] add plugin names to process save-core error output. (#143126)
continuation of
[#142684](https://github.com/llvm/llvm-project/pull/142684) to show
plugin names.

From issue [#14258](https://github.com/llvm/llvm-project/issues/142581)
2025-06-23 18:02:58 +01:00
Ebuka Ezike
7214a3d3da
[lldb] Do not accept invalid process save-core plugins (#142684)
Fixes #142581
2025-06-04 11:33:35 +01:00
David Spickett
7d01b85c2a Reland "[lldb] Do not bump memory modificator ID when "internal" debugger memory is updated (#129092)"
This reverts commit daa4061d61216456baa83ab404e096200e327fb4.

Original PR https://github.com/llvm/llvm-project/pull/129092.

I have restricted the test to X86 Windows because it turns out the only
reason that `expr x.get()` would change m_memory_id is that on x86 we
have to write the return address to the stack in ABIWindows_X86_64::PrepareTrivialCall:
```
  // Save return address onto the stack
  if (!process_sp->WritePointerToMemory(sp, return_addr, error))
    return false;
```

This is not required on AArch64 so m_memory_id was not changed:
```
(lldb) expr x.get()
(int) $0 = 0
(lldb) process status -d
Process 15316 stopped
* thread #1, stop reason = Exception 0x80000003 encountered at address 0x7ff764a31034
    frame #0: 0x00007ff764a31038 TestProcessModificationIdOnExpr.cpp.tmp`main at TestProcessModificationIdOnExpr.cpp:35
   32     __builtin_debugtrap();
   33     __builtin_debugtrap();
   34     return 0;
-> 35   }
   36
   37   // CHECK-LABEL: process status -d
   38   // CHECK: m_stop_id: 2
ProcessModID:
  m_stop_id: 3
  m_last_natural_stop_id: 0
  m_resume_id: 0
  m_memory_id: 0
```

Really we should find a better way to force a memory write here, but
I can't think of one right now.
2025-05-02 13:00:12 +00:00
David Spickett
daa4061d61 Revert "[lldb] Do not bump memory modificator ID when "internal" debugger memory is updated (#129092)"
And a follow up warning fix.

This reverts commit 6aa963f780d63d4c8fa80de97dd79c932bc35f4e
and 2bff80f25d51e24d3c552e033a2863dd36ef648b.

This is failing on Windows on Arm: https://lab.llvm.org/buildbot/#/builders/141/builds/8375

Seems to produce the line the test wants but not in the right place.
Reverting while I investigate.
2025-05-02 09:14:16 +00:00
Mikhail Zakharov
6aa963f780
[lldb] Do not bump memory modificator ID when "internal" debugger memory is updated (#129092)
This change adds a setting `target.process.track-memory-cache-changes`.
Disabling this setting prevents invalidating and updating values in
`ValueObject::UpdateValueIfNeeded` when only "internal" debugger memory
is updated. Writing to "internal" debugger memory happens when, for
instance, expressions are evaluated by visualizers (pretty printers).
One of the examples when cache invalidation has a particularly heavy
impact is visualizations of some collections: in some collections
getting collection size is an expensive operation (it requires traversal
of the collection).
At the same time evaluating user expression with side effects (visible
to target, not only to debugger) will still bump memory ID because:

- If expression is evaluated via interpreter: it will cause write to
"non-internal" memory
- If expression is JIT-compiled: then to call the function LLDB will
write to "non-internal" stack memory

The downside of disabled `target.process.track-memory-cache-changes`
setting is that convenience variables won't reevaluate synthetic
children automatically.

---------

Co-authored-by: Mikhail Zakharov <mikhail.zakharov@jetbrains.com>
2025-05-01 11:10:41 -07:00
Robert O'Callahan
239718055d
[lldb] Implement CLI support for reverse-continue (#132783)
This introduces the options "-F/--forward" and "-R/--reverse" to
`process continue`.

These only work if you're running with a gdbserver backend that supports
reverse execution, such as rr. For testing we rely on the fake
reverse-execution functionality in `lldbreverse.py`.
2025-04-23 16:16:30 -07:00
Jonas Devlieghere
ae28a375d0
[lldb] Document <run-args> behavior in process launch help (#128215) 2025-02-22 11:11:40 -06:00
walter erquinigo
e952728f88 [LLDB] Retry Add a target.launch-working-dir setting
This retries the PR 113521 skipping a test in a remote environment.
2024-11-05 13:29:51 -05:00
Walter Erquinigo
5d39e0c7e1
Revert "[LLDB] Add a target.launch-working-dir setting" (#114973)
Reverts llvm/llvm-project#113521 due to build bot failures mentioned in
the original PR.
2024-11-05 07:12:20 -05:00
Walter Erquinigo
6620cd2523
[LLDB] Add a target.launch-working-dir setting (#113521)
Internally we use bazel in a way in which it can drop you in a LLDB
session with the target launched in a particular cwd, which is needed
for things to work. We've been making this automation work via `process
launch -w`. However, if later the user wants to restart the process with
`r`, then they end up using a different cwd for relaunching the process.
As a way to fix this, I'm adding a target-level setting that allows
configuring a default cwd used for launching the process without needing
the user to specify it manually.
2024-11-05 06:33:25 -05:00
Ryan Mansfield
4494c54326
[lldb] Fix typos in various help messages. (#109851) 2024-09-24 16:48:27 -07:00
Jacob Lalonde
96b7c64b8a
[LLDB] Reapply SBSaveCore Add Memory List (#107937)
Recently in #107731 this change was revereted due to excess memory size
in `TestSkinnyCore`. This was due to a bug where a range's end was being
passed as size. Creating massive memory ranges.

Additionally, and requiring additional review, I added more unit tests
and more verbose logic to the merging of save core memory regions.

@jasonmolenda as an FYI.
2024-09-11 10:33:19 -07:00
Jonas Devlieghere
bb343468ff
Revert "[LLDB] Reappply SBSaveCore AddMemoryList" (#107731)
Reverts llvm/llvm-project#107159 as this is still causing
`TestSkinnyCorefile.py` to time out.


https://ci.swift.org/view/all/job/llvm.org/view/LLDB/job/as-lldb-cmake/11099/

https://ci.swift.org/view/all/job/llvm.org/view/LLDB/job/lldb-cmake/5544/
2024-09-07 17:10:20 -07:00
Jacob Lalonde
d4d4e77918
[LLDB] Reappply SBSaveCore AddMemoryList (#107159)
Reapplies #106293, testing identified issue in the merging code. I used
this opportunity to strip CoreFileMemoryRanges to it's own file and then
add unit tests on it's behavior.
2024-09-06 09:04:33 -07:00
Jacob Lalonde
b959532484
Revert "[LLDB][SBSaveCore] Add selectable memory regions to SBSaveCor… (#106293)
Reverts #105442. Due to `TestSkinnyCoreFailing` and root causing of the
failure will likely take longer than EOD.
2024-08-27 14:23:00 -07:00
Adrian Prantl
0642cd768b
[lldb] Turn lldb_private::Status into a value type. (#106163)
This patch removes all of the Set.* methods from Status.

This cleanup is part of a series of patches that make it harder use the
anti-pattern of keeping a long-lives Status object around and updating
it while dropping any errors it contains on the floor.

This patch is largely NFC, the more interesting next steps this enables
is to:
1. remove Status.Clear()
2. assert that Status::operator=() never overwrites an error
3. remove Status::operator=()

Note that step (2) will bring 90% of the benefits for users, and step
(3) will dramatically clean up the error handling code in various
places. In the end my goal is to convert all APIs that are of the form

`    ResultTy DoFoo(Status& error)
`
to

`    llvm::Expected<ResultTy> DoFoo()
`
How to read this patch?

The interesting changes are in Status.h and Status.cpp, all other
changes are mostly

` perl -pi -e 's/\.SetErrorString/ = Status::FromErrorString/g' $(git
grep -l SetErrorString lldb/source)
`
plus the occasional manual cleanup.
2024-08-27 10:59:31 -07:00
Jacob Lalonde
d517b22411
[LLDB][SBSaveCore] Add selectable memory regions to SBSaveCore (#105442)
This patch adds the option to specify specific memory ranges to be
included in a given core file. The current implementation lets user
specified ranges either be in addition to a certain save style, or
independent of them via the newly added custom enum.

To achieve being inclusive of save style, I've moved from a std::vector
of ranges to a RangeDataVector, and to join overlapping ranges to
prevent duplication of memory ranges in the core file.

As a non function bonus, when SBSavecore was initially created, the
header was included in the lldb-private interfaces, and I've fixed that
and moved it the forward declare as an oversight. CC @bulbazord in case
we need to include that into swift.
2024-08-27 07:33:12 -07:00
Jonas Devlieghere
c6ce324fa7
[lldb] Eliminate more Targer* in favor of Target& in CommandObjects (NFC)
The majority of the replaced Target pointers were already used
unconditionally and the few that were shouldn't even be NULL.
2024-08-02 09:53:34 -07:00
Jonas Devlieghere
8398ad9cb2
[lldb] Unify the way we get the Target in CommandObject (#101208)
Currently, CommandObjects are obtaining a target in a variety of ways.
Often the command incorrectly operates on the selected target. As an
example, when a breakpoint command is running, the current target is
passed into the command but the target that hit the breakpoint is not
the selected target. In other places we use the CommandObject's
execution context, which is frozen during the execution of the command,
and comes with its own limitations. Finally, we often want to fall back
to the dummy target if no real target is available.

Instead of having to guess how to get the target, this patch introduces
one helper function in CommandObject to get the most relevant target. In
order of priority, that's the target from the command object's execution
context, from the interpreter's execution context, the selected target
or the dummy target.

rdar://110846511
2024-07-31 09:57:10 -07:00
Jason Molenda
56535a090d
[lldb] Don't crash when attaching to pid and no binaries found (#100287)
There is a narrow window during process launch on macOS where lldb can
attach and no binaries will be seen as loaded in the process (none
reported by libdyld SPI). A year ago I made changes to set the
new-binary-loaded breakpoint correctly despite this. But we've seen a
crash when this combination is seen, where
CommandObjectProcessAttach::DoExecute assumed there was at least one
binary registered in the Target. Fix that.

Also fix two FileSpec API uses from when we didn't have a GetPath()
method that returned a std::string, and was copying the filepaths into
fixed length buffers. All of this code was from ~14 years ago when we
didn't have that API.

rdar://131631627
2024-07-23 17:50:40 -07:00
Jacob Lalonde
4120570dc4
[LLDB][SaveCore] Add SBSaveCoreOptions Object, and SBProcess::SaveCore() overload (#98403)
This PR adds `SBSaveCoreOptions`, which is a container class for options
when LLDB is taking coredumps. For this first iteration this container
just keeps parity with the extant API of `file, style, plugin`. In the
future this options object can be extended to allow users to take a
subset of their core dumps.
2024-07-18 17:10:15 -07:00
Vladislav Dzhidzhoev
139df36d89
[LLDB] Make 'process load' take remote os path delimiter into account (#98690)
Currently, if we execute 'process load' with remote debugging, it uses
the host's path delimiter to look up files on a target machine. If we
run remote debugging of Linux target on Windows and execute "process
load C:\foo\a.so", lldb-server tries to load \foo\a.so instead of
/foo/a.so on the remote.

It affects several API tests.

This commit fixes that error. Also, it contains minor fixes for
TestLoadUnload.py for testing on Windows host and Linux target.
2024-07-16 15:21:06 +02:00
Jason Molenda
aeaa11aeac
[lldb] Address mask sbprocess apis and new mask invalid const (#83663)
[lldb] Add SBProcess methods for get/set/use address masks (#83095)

I'm reviving a patch from phabracator, https://reviews.llvm.org/D155905
which was approved but I wasn't thrilled with all the API I was adding
to SBProcess for all of the address mask types / memory regions. In this
update, I added enums to control type address mask type (code, data,
any) and address space specifiers (low, high, all) with defaulted
arguments for the most common case.  I originally landed this via
https://github.com/llvm/llvm-project/pull/83095 but it failed on CIs
outside of arm64 Darwin so I had to debug it on more environments
and update the patch.

This patch is also fixing a bug in the "addressable bits to address
mask" calculation I added in AddressableBits::SetProcessMasks. If lldb
were told that 64 bits are valid for addressing, this method would
overflow the calculation and set an invalid mask. Added tests to check
this specific bug while I was adding these APIs.

This patch changes the value of "no mask set" from 0 to
LLDB_INVALID_ADDRESS_MASK, which is UINT64_MAX. A mask of all 1's
means "no bits are used for addressing" which is an impossible mask,
whereas a mask of 0 means "all bits are used for addressing" which
is possible.

I added a base class implementation of ABI::FixCodeAddress and
ABI::FixDataAddress that will apply the Process mask values if they
are set to a value other than LLDB_INVALID_ADDRESS_MASK.

I updated all the callers/users of the Mask methods which were
handling a value of 0 to mean invalid mask to use
LLDB_INVALID_ADDRESS_MASK.

I added code to the all AArch64 ABI Fix* methods to apply the
Highmem masks if they have been set.  These will not be set on a
Linux environment, but in TestAddressMasks.py I test the highmem
masks feature for any AArch64 target, so all AArch64 ABI  plugins 
must handle it.

rdar://123530562
2024-03-06 10:06:56 -08:00
jimingham
2d704f4bf2
Start to clean up the process of defining command arguments. (#83097)
Partly, there's just a lot of unnecessary boiler plate. It's also
possible to define combinations of arguments that make no sense (e.g.
eArgRepeatPlus followed by eArgRepeatPlain...) but these are never
checked since we just push_back directly into the argument definitions.

This commit is step 1 of this cleanup - do the obvious stuff. In it, all
the simple homogenous argument lists and the breakpoint/watchpoint
ID/Range types, are set with common functions. This is an NFC change, it
just centralizes boiler plate. There's no checking yet because you can't
get a single argument wrong.

The end goal is that all argument definition goes through functions and
m_arguments is hidden so that you can't define inconsistent argument
sets.
2024-02-27 10:34:01 -08:00
jimingham
563ef30601
Add the RegisterCompleter to eArgTypeRegisterName in g_argument_table (#82428)
This is a follow-on to:

https://github.com/llvm/llvm-project/pull/82085

The completer for register names was missing from the argument table. I
somehow missed that the only register completer test was x86_64, so that
test broke.

I added the completer in to the right slot in the argument table, and
added a small completions test that just uses the alias register names.
If we end up having a platform that doesn't define register names, we'll
have to skip this test there, but it should add a sniff test for
register completion that will run most everywhere.
2024-02-20 14:18:03 -08:00
Shubham Sandeep Rastogi
9ed8b272c3 Revert "Centralize the handling of completion for simple argument lists. (#82085)"
This reverts commit 21631494b068d9364b8dc8f18e59adee9131a0a5.

Reverted because of greendragon failure:

******************** TEST 'lldb-api :: functionalities/completion/TestCompletion.py' FAILED ********************
Script:
2024-02-19 18:19:59 -08:00
jimingham
21631494b0
Centralize the handling of completion for simple argument lists. (#82085)
Most commands were adding argument completion handling by themselves,
resulting in a lot of unnecessary boilerplate. In many cases, this could
be done generically given the argument definition and the entries in the
g_argument_table.

I'm going to address this in a couple passes. In this first pass, I
added handling of commands that have only one argument list, with one
argument type, either single or repeated, and changed all the commands
that are of this sort (and don't have other bits of business in their
completers.)

I also added some missing connections between arg types and completions
to the table, and added a RemoteFilename and RemotePath to use in places
where we were using the Remote completers. Those arguments used to say
they were "files" but they were in fact remote files.

I also added a module arg type to use where we were using the module
completer. In that case, we should call the argument module.
2024-02-19 16:43:08 -08:00
Alex Langford
307cd88354
[lldb][NFCI] Remove CommandObjectProcessHandle::VerifyCommandOptionValue (#79901)
I was refactoring something else but ran into this function. It was
somewhat confusing to read through and understand, but it boils down to
two steps:
- First we try `OptionArgParser::ToBoolean`. If that works, then we're
good to go.
- Second, we try `llvm::to_integer` to see if it's an integer. If it
parses to 0 or 1, we're good.
- Failing either of the steps above means we cannot parse it into a
bool.

Instead of having an integer out param and a bool return value, the
interface is better served with an optional<bool> -- Either it parses
into true or false, or you get back nothing (nullopt).
2024-02-14 10:20:58 -08:00
Alex Langford
5f22d3356c
[lldb][NFCI] Change BreakpointIDList::FindBreakpointID to BreakpointIDList::Contains (#79517)
`FindBreakpointID` take a BreakpointID and a pointer to a size_t (so you
can get position information). It returns a bool to indicate whether the
id was found in the list or not.

There are 2 callers of this currently and neither one actually uses the
position information, so I removed it. After that, I renamed it to
Contains to more accurately reflect the intent. Additionally, I changed
the argument type from a reference to a value (because BreakpointID is
just a wrapper around 2 integers, copies are cheap).
2024-01-26 10:19:03 -08:00
Jason Molenda
c73a3f16f8 [lldb] [mostly NFC] Large WP foundation: WatchpointResources (#68845)
This patch is rearranging code a bit to add WatchpointResources to
Process. A WatchpointResource is meant to represent a hardware
watchpoint register in the inferior process. It has an address, a size,
a type, and a list of Watchpoints that are using this
WatchpointResource.

This current patch doesn't add any of the features of
WatchpointResources that make them interesting -- a user asking to watch
a 24 byte object could watch this with three 8 byte WatchpointResources.
Or a Watchpoint on 1 byte at 0x1002 and a second watchpoint on 1 byte at
0x1003, these must both be served by a single WatchpointResource on that
doubleword at 0x1000 on a 64-bit target, if two hardware watchpoint
registers were used to track these separately, one of them may not be
hit. Or if you have one Watchpoint on a variable with a condition set,
and another Watchpoint on that same variable with a command defined or
different condition, or ignorecount, both of those Watchpoints need to
evaluate their criteria/commands when their WatchpointResource has been
hit.

There's a bit of code movement to rearrange things in the direction I'll
need for implementing this feature, so I want to start with reviewing &
landing this mostly NFC patch and we can focus on the algorithmic
choices about how WatchpointResources are shared and handled as they're
triggeed, separately.

This patch also stops printing "Watchpoint <n> hit: old value: <x>, new
vlaue: <y>" for Read watchpoints. I could make an argument for print
"Watchpoint <n> hit: current value <x>" but the current output doesn't
make any sense, and the user can print the value if they are
particularly interested. Read watchpoints are used primarily to
understand what code is reading a variable.

This patch adds more fallbacks for how to print the objects being
watched if we have types, instead of assuming they are all integral
values, so a struct will print its elements. As large watchpoints are
added, we'll be doing a lot more of those.

To track the WatchpointSP in the WatchpointResources, I changed the
internal API which took a WatchpointSP and devolved it to a Watchpoint*,
which meant touching several different Process files. I removed the
watchpoint code in ProcessKDP which only reported that watchpoints
aren't supported, the base class does that already.

I haven't yet changed how we receive a watchpoint to identify the
WatchpointResource responsible for the trigger, and identify all
Watchpoints that are using this Resource to evaluate their conditions
etc. This is the same work that a BreakpointSite needs to do when it has
been tiggered, where multiple Breakpoints may be at the same address.

There is not yet any printing of the Resources that a Watchpoint is
implemented in terms of ("watchpoint list", or
SBWatchpoint::GetDescription).

"watchpoint set var" and "watchpoint set expression" take a size
argument which was previously 1, 2, 4, or 8 (an enum). I've changed this
to an unsigned int. Most hardware implementations can only watch 1, 2,
4, 8 byte ranges, but with Resources we'll allow a user to ask for
different sized watchpoints and set them in hardware-expressble terms
soon.

I've annotated areas where I know there is work still needed with
LWP_TODO that I'll be working on once this is landed.

I've tested this on aarch64 macOS, aarch64 Linux, and Intel macOS.

https://discourse.llvm.org/t/rfc-large-watchpoint-support-in-lldb/72116
(cherry picked from commit fc6b72523f3d73b921690a713e97a433c96066c6)
2023-11-30 14:59:10 -08:00
David Spickett
b0af8a1ede Revert "[lldb] [mostly NFC] Large WP foundation: WatchpointResources (#68845)"
...and follow ups.

As it has caused test failures on Linux Arm and AArch64:
https://lab.llvm.org/buildbot/#/builders/96/builds/49126
https://lab.llvm.org/buildbot/#/builders/17/builds/45824

```
  lldb-shell :: Subprocess/clone-follow-child-wp.test
  lldb-shell :: Subprocess/fork-follow-child-wp.test
  lldb-shell :: Subprocess/vfork-follow-child-wp.test
```

This reverts commit a6c62bf1a4717accc852463b664cd1012237d334,
commit a0a1ff3ab40e347589b4e27d8fd350c600526735 and commit
fc6b72523f3d73b921690a713e97a433c96066c6.
2023-11-28 09:39:37 +00:00
Jason Molenda
fc6b72523f
[lldb] [mostly NFC] Large WP foundation: WatchpointResources (#68845)
This patch is rearranging code a bit to add WatchpointResources to
Process. A WatchpointResource is meant to represent a hardware
watchpoint register in the inferior process. It has an address, a size,
a type, and a list of Watchpoints that are using this
WatchpointResource.

This current patch doesn't add any of the features of
WatchpointResources that make them interesting -- a user asking to watch
a 24 byte object could watch this with three 8 byte WatchpointResources.
Or a Watchpoint on 1 byte at 0x1002 and a second watchpoint on 1 byte at
0x1003, these must both be served by a single WatchpointResource on that
doubleword at 0x1000 on a 64-bit target, if two hardware watchpoint
registers were used to track these separately, one of them may not be
hit. Or if you have one Watchpoint on a variable with a condition set,
and another Watchpoint on that same variable with a command defined or
different condition, or ignorecount, both of those Watchpoints need to
evaluate their criteria/commands when their WatchpointResource has been
hit.

There's a bit of code movement to rearrange things in the direction I'll
need for implementing this feature, so I want to start with reviewing &
landing this mostly NFC patch and we can focus on the algorithmic
choices about how WatchpointResources are shared and handled as they're
triggeed, separately.

This patch also stops printing "Watchpoint <n> hit: old value: <x>, new
vlaue: <y>" for Read watchpoints. I could make an argument for print
"Watchpoint <n> hit: current value <x>" but the current output doesn't
make any sense, and the user can print the value if they are
particularly interested. Read watchpoints are used primarily to
understand what code is reading a variable.

This patch adds more fallbacks for how to print the objects being
watched if we have types, instead of assuming they are all integral
values, so a struct will print its elements. As large watchpoints are
added, we'll be doing a lot more of those.

To track the WatchpointSP in the WatchpointResources, I changed the
internal API which took a WatchpointSP and devolved it to a Watchpoint*,
which meant touching several different Process files. I removed the
watchpoint code in ProcessKDP which only reported that watchpoints
aren't supported, the base class does that already.

I haven't yet changed how we receive a watchpoint to identify the
WatchpointResource responsible for the trigger, and identify all
Watchpoints that are using this Resource to evaluate their conditions
etc. This is the same work that a BreakpointSite needs to do when it has
been tiggered, where multiple Breakpoints may be at the same address.

There is not yet any printing of the Resources that a Watchpoint is
implemented in terms of ("watchpoint list", or
SBWatchpoint::GetDescription).

"watchpoint set var" and "watchpoint set expression" take a size
argument which was previously 1, 2, 4, or 8 (an enum). I've changed this
to an unsigned int. Most hardware implementations can only watch 1, 2,
4, 8 byte ranges, but with Resources we'll allow a user to ask for
different sized watchpoints and set them in hardware-expressble terms
soon.

I've annotated areas where I know there is work still needed with
LWP_TODO that I'll be working on once this is landed.

I've tested this on aarch64 macOS, aarch64 Linux, and Intel macOS.

https://discourse.llvm.org/t/rfc-large-watchpoint-support-in-lldb/72116
2023-11-27 13:28:59 -08:00
José Lira Junior
bd8f1068ca
[lldb] correct inconsistent order of messages on process launch (#73173)
Fixes [#68035](https://github.com/llvm/llvm-project/issues/68035), where
an inconsistency in the order of "Process launched" and "Process
stopped" messages occurs during `process launch`.

The fix involves adjusting the message output sequence in
`CommandObjectProcessLaunch::DoExecute` within
`source/Commands/CommandObjectProcess.cpp`. This ensures "Process
launched" consistently precedes "Process stopped" when executing
commands with the '-o' flag, i.e., non-interactive mode.

Upon implementing this change, two tests failed:
`lldb/test/Shell/Breakpoint/jit-loader_jitlink_elf.test` and
`lldb/test/Shell/Breakpoint/jit-loader_rtdyld_elf.test`. These failures
were expected as they relied on the previous, now-corrected message
order. Updating these tests to align with the new message sequence is
part of this PR's scope.
2023-11-24 15:48:16 +00:00
Pete Lawrence
92d8a28cc6
[lldb] Part 2 of 2 - Refactor CommandObject::DoExecute(...) return void (not bool) (#69991)
[lldb] Part 2 of 2 - Refactor `CommandObject::DoExecute(...)` to return
`void` instead of ~~`bool`~~

Justifications:
- The code doesn't ultimately apply the `true`/`false` return values.
- The methods already pass around a `CommandReturnObject`, typically
with a `result` parameter.
- Each command return object already contains:
	- A more precise status
	- The error code(s) that apply to that status

Part 1 refactors the `CommandObject::Execute(...)` method.
- See
[https://github.com/llvm/llvm-project/pull/69989](https://github.com/llvm/llvm-project/pull/69989)

rdar://117378957
2023-10-30 13:21:00 -07:00
Alex Langford
58fe7b751d [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
2023-08-21 12:44:17 -07:00
Med Ismail Bennani
ec05cf50da [lldb] Improve corefile saving ergonomics
This patch improves the way the user can save the process state into a
corefile by adding completion handler that would provide tab completion
for the corefile path and also resolves the corefile path to expand
relative path.

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

Signed-off-by: Med Ismail Bennani <ismail@bennani.ma>
2023-06-13 13:44:51 -07:00
Med Ismail Bennani
6a9c3e6115 [lldb/Commands] Add support to auto-completion for user commands
This patch should allow the user to set specific auto-completion type
for their custom commands.

To do so, we had to hoist the `CompletionType` enum so the user can
access it and add a new completion type flag to the CommandScriptAdd
Command Object.

So now, the user can specify which completion type will be used with
their custom command, when they register it.

This also makes the `crashlog` custom commands use disk-file completion
type, to browse through the user file system and load the report.

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

Signed-off-by: Med Ismail Bennani <ismail@bennani.ma>
2023-06-06 10:58:34 -07:00
Med Ismail Bennani
601583e5a3 [lldb/Utility] Fix layering violation caused by ScriptedMetadata
This patch moves `ScriptedMetadata.h` from the `Interpreter` directory to
the `Utility` sub-directory since `ProcessInfo.h` depends on it.

It also gets rid of the unused `OptionGroupPythonClassWithDict`
constructor for `ScriptedMetadata` which would address the layering
violation.

Signed-off-by: Med Ismail Bennani <medismail.bennani@gmail.com>
2023-03-06 13:14:15 -08:00
Med Ismail Bennani
b9d4c94a60 [lldb/Plugins] Add Attach capabilities to ScriptedProcess
This patch adds process attach capabilities to the ScriptedProcess
plugin. This doesn't really expects a PID or process name, since the
process state is already script, however, this allows to create a
scripted process without requiring to have an executuble in the target.

In order to do so, this patch also turns the scripted process related
getters and setters from the `ProcessLaunchInfo` and
`ProcessAttachInfo` classes to a `ScriptedMetadata` instance and moves
it in the `ProcessInfo` class, so it can be accessed interchangeably.

This also adds the necessary SWIG wrappers to convert the internal
`Process{Attach,Launch}InfoSP` into a `SB{Attach,Launch}Info` to pass it
as argument the scripted process python implementation and convert it
back to the internal representation.

rdar://104577406

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

Signed-off-by: Med Ismail Bennani <medismail.bennani@gmail.com>
2023-03-03 19:33:02 -08:00
Med Ismail Bennani
3014a1c5a1 [lldb] Add scripted process launch/attach option to {,platform }process commands
This patch does several things:

First, it refactors the `CommandObject{,Platform}ProcessObject` command
option class into a separate `CommandOptionsProcessAttach` option group.

This will make sure both the `platform process attach` and `process attach`
command options will always stay in sync without having with duplicate
them each time. But more importantly, making this class an `OptionGroup`
allows us to combine with a `OptionGroupPythonClassWithDict` to add
support for the scripted process managing class name and user-provided
dictionary options.

This patch also improves feature parity between `ProcessLaunchInfo` and
`ProcessAttachInfo` with regard to ScriptedProcesses, by exposing the
various getters and setters necessary to use them through the SBAPI.

This is foundation work for adding support to "attach" to a process from
the scripted platform.

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

Signed-off-by: Med Ismail Bennani <medismail.bennani@gmail.com>
2023-03-03 19:33:02 -08:00
serge-sans-paille
984b800a03
Move from llvm::makeArrayRef to ArrayRef deduction guides - last part
This is a follow-up to https://reviews.llvm.org/D140896, split into
several parts as it touches a lot of files.

Differential Revision: https://reviews.llvm.org/D141298
2023-01-10 11:47:43 +01: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
Med Ismail Bennani
e861d053dd [lldb/Utility] Add GetDescription(Stream&) to StructureData::*
This patch improves the StructuredData classes to provide a
GetDescription(lldb_private::Stream&) affordance.

This is very convenient compared to the Dump method because this try to
pretty print the structure instead of just serializing it into a JSON.

This patch also updates some parts of lldb (i.e. extended crash info) to
use this new affordance instead of StructuredData::Dump.

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

Signed-off-by: Med Ismail Bennani <medismail.bennani@gmail.com>
2022-11-03 14:44:53 -07:00
Jonas Devlieghere
7ced9fff95
[lldb] Refactor command option enum values (NFC)
Refactor the command option enum values and the command argument table
to connect the two. This has two benefits:

 - We guarantee that two options that use the same argument type have
   the same accepted values.
 - We can print the enum values and their description in the help
   output. (D129707)

Differential revision: https://reviews.llvm.org/D129703
2022-07-14 21:18:07 -07:00
Walter Erquinigo
b532dd545f [trace] Add an option to save a compact trace bundle
A trace bundle contains many trace files, and, in the case of intel pt, the
largest files are often the context switch traces because they are not
compressed by default. As a way to improve this, I'm adding a --compact option
to the `trace save` command that filters out unwanted processes from the
context switch traces. Eventually we can do the same for intel pt traces as
well.

Differential Revision: https://reviews.llvm.org/D129239
2022-07-13 11:43:28 -07:00
Jim Ingham
c1b07d6177 Have CommandObjectParsed check for "commands that take no arguments".
This is currently being done in an ad hoc way, and so for some
commands it isn't being checked.  We have the info to make this check,
since commands are supposed to add their arguments to the m_arguments
field of the CommandObject.  This change uses that info to check whether
the command received arguments in error.

A handful of commands weren't defining their argument types, I also had
to fix them.  And a bunch of commands were checking for arguments by
hand, so I removed those checks in favor of the CommandObject one.  That
also meant I had to change some tests that were checking for the ad hoc
error outputs.

Differential Revision: https://reviews.llvm.org/D128453
2022-06-27 15:14:41 -07:00
Jim Ingham
bae10a6bbb Fix a bug with "process continue -b" when no breakpoints are
passed.  I was passing the empty list of breakponts to the
VerifyBreakpointList routine, but that treats empty as "choose
the default breakpoint" which we don't want here.
2022-06-22 12:18:07 -07:00