This is another step towards supporting DWARF5 checksums and inline
source code in LLDB. This is a reland of #85468 but without the
functional change of storing the support file from the line table (yet).
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)
...and follow ups.
As it has caused test failures on Linux Arm and AArch64:
https://lab.llvm.org/buildbot/#/builders/96/builds/49126https://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.
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
When this option gets enabled, descriptions of threads will be generated
using the format provided in the launch configuration instead of
generating it manually in the dap code. This allows lldb-dap to show an
output similar to the one in the CLI.
This is very similar to https://github.com/llvm/llvm-project/pull/71843
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
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>
LLDB should guarantee that the strings returned by SBAPI methods
live forever. I went through every method that returns a string and made
sure that it was added to the ConstString StringPool before returning if
it wasn't obvious that it was already doing so.
I've also updated the docs to document this behavior.
Differential Revision: https://reviews.llvm.org/D150804
Re-lands 04aa943be8ed5c03092e2a90112ac638360ec253 with modifications
to fix tests.
I originally reverted this because it caused a test to fail on Linux.
The problem was that I inverted a condition on accident.
There are many situations where we'll iterate over a SymbolContextList
with the pattern:
```
SymbolContextList sc_list;
// Fill in sc_list here
for (auto i = 0; i < sc_list.GetSize(); i++) {
SymbolContext sc;
sc_list.GetSymbolAtContext(i, sc);
// Do work with sc
}
```
Adding an iterator to iterate over the instances directly means we don't
have to do bounds checking or create a copy of every element of the
SymbolContextList.
Differential Revision: https://reviews.llvm.org/D149900
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 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
Add Thread::GetSiginfo() and SBThread::GetSiginfo() methods to retrieve
the siginfo value from server.
Differential Revision: https://reviews.llvm.org/D118055
Add Thread::GetSiginfo() and SBThread::GetSiginfo() methods to retrieve
the siginfo value from server.
Differential Revision: https://reviews.llvm.org/D118055
Remove the last remaining references to the reproducers from the
instrumentation. This patch renames the relevant files and macros.
Differential revision: https://reviews.llvm.org/D117712
[NFC] As part of using inclusive language within the llvm project, this patch
renames master plan to controlling plan in lldb.
Reviewed By: jingham
Differential Revision: https://reviews.llvm.org/D113019
This patch refactors a good part of the code base turning the usual
FileSpec, Line, Column, CheckInlines, ExactMatch arguments into a
SourceLocationSpec object.
This change is required for a following patch that will add handling of the
column line information when doing symbol resolution.
Differential Revision: https://reviews.llvm.org/D100965
Signed-off-by: Med Ismail Bennani <medismail.bennani@gmail.com>
Introduce three new stop reasons for fork, vfork and vforkdone events.
This includes server support for serializing fork/vfork events into
gdb-remote protocol. The stop infos for the two base events take a pair
of PID and TID for the newly forked process.
Differential Revision: https://reviews.llvm.org/D100196
This implements the interactive trace start and stop methods.
This diff ended up being much larger than I anticipated because, by doing it, I found that I had implemented in the beginning many things in a non optimal way. In any case, the code is much better now.
There's a lot of boilerplate code due to the gdb-remote protocol, but the main changes are:
- New tracing packets: jLLDBTraceStop, jLLDBTraceStart, jLLDBTraceGetBinaryData. The gdb-remote packet definitions are quite comprehensive.
- Implementation of the "process trace start|stop" and "thread trace start|stop" commands.
- Implementaiton of an API in Trace.h to interact with live traces.
- Created an IntelPTDecoder for live threads, that use the debugger's stop id as checkpoint for its internal cache.
- Added a functionality to stop the process in case "process tracing" is enabled and a new thread can't traced.
- Added tests
I have some ideas to unify the code paths for post mortem and live threads, but I'll do that in another diff.
Differential Revision: https://reviews.llvm.org/D91679
Several SB API functions return strings using (char*, size_t) output
arguments. During capture, we serialize an empty string for the char*
because the memory can be uninitialized.
During active replay, we have custom replay redirects that ensure that
we don't override the buffer from which we're reading, but rather write
to a buffer on the heap with the given length. This is sufficient for
the active reproducer use case, where we only care about the side
effects of the API calls, not the values actually returned.
This approach does not not work for passive replay because here we
ignore all the incoming arguments, and re-execute the current function
with the arguments deserialized from the reproducer. This means that
these function will update the deserialized copy of the arguments,
rather than whatever was passed in by the SWIG wrapper.
To solve this problem, this patch extends the reproducer instrumentation
to handle this special case for passive replay. We nog ignore the
replayer in the registry and the incoming char pointer, and instead
reinvoke the current method on the deserialized class, and populate the
output argument.
Differential revision: https://reviews.llvm.org/D77759
Summary:
This gets rid of some nesting and of the raw char* variable that caused
the memory management bug we hit recently.
This commit also removes the fallback code which should trigger when
the StopInfo provides no stop description. All currently implemented
StopInfos have a `GetDescription()` method that shouldn't return an
empty description.
Reviewers: JDevlieghere, labath, mib
Subscribers: lldb-commits
Tags: #lldb
Differential Revision: https://reviews.llvm.org/D74157
Refactore GetStopReasonExtendedBacktraces so that the reproducer macro
is passed an instrumented copy constructor rather than the constructor
taking a ThreadCollectionSP, which is not instrumented.
When a thread stops, this checks depending on the platform if the top frame is
an abort stack frame. If so, it looks for an assert stack frame in the upper
frames and set it as the most relavant frame when found.
To do so, the StackFrameRecognizer class holds a "Most Relevant Frame" and a
"cooked" stop reason description. When the thread is about to stop, it checks
if the current frame is recognized, and if so, it fetches the recognized frame's
attributes and applies them.
rdar://58528686
Differential Revision: https://reviews.llvm.org/D73303
Signed-off-by: Med Ismail Bennani <medismail.bennani@gmail.com>
For the methods taking a char* and a length that have a custom replayer,
ignore the incoming string in the instrumentation macro. This prevents
potentially reading garbage and blowing up the SB API log.
Some SB API methods returns strings through a char* and a length. This
is a problem for the deserializer, which considers a single type at a
time, and therefore cannot know how many bytes to allocate for the
character buffer.
We can solve this problem by implementing a custom replayer, which
ignores the passed-in char* and allocates a buffer of the correct size
itself, before invoking the original API method or function.
This patch adds three new macros to register a custom replayer for
methods that take a char* and a size_t. It supports arbitrary return
values (some functions return a bool while others return a size_t).
When trying to get the stop reason description using the SB API, the
buffer fetched was not null-terminated causing failures on the sanitized bot.
This patch should address those failures.
Signed-off-by: Med Ismail Bennani <medismail.bennani@gmail.com>
When a thread stops, this checks depending on the platform if the top frame is
an abort stack frame. If so, it looks for an assert stack frame in the upper
frames and set it as the most relavant frame when found.
To do so, the StackFrameRecognizer class holds a "Most Relevant Frame" and a
"cooked" stop reason description. When the thread is about to stop, it checks
if the current frame is recognized, and if so, it fetches the recognized frame's
attributes and applies them.
rdar://58528686
Differential Revision: https://reviews.llvm.org/D73303
Signed-off-by: Med Ismail Bennani <medismail.bennani@gmail.com>
GetStopDescription writes to a const char* with a given length. However,
the reproducer instrumentation serialized the char pointer and length
separately.
To serialize the string, we naively look for the first null byte to
determine its length. This can lead to the method overwriting the input
buffer when the assumed string length is smaller than the actual number
of bytes written by GetStopDescription.
The real solution is to have a custom serializer that takes both
arguments into account. However, given that these are output parameters,
they don't affect replay. If the string is passed as input later, it's
is recorded as such. Therefore I've replaced the instrumentation macro
with LLDB_RECORD_DUMMY which skips the serialization.
When a thread stops, this checks depending on the platform if the top frame is
an abort stack frame. If so, it looks for an assert stack frame in the upper
frames and set it as the most relavant frame when found.
To do so, the StackFrameRecognizer class holds a "Most Relevant Frame" and a
"cooked" stop reason description. When the thread is about to stop, it checks
if the current frame is recognized, and if so, it fetches the recognized frame's
attributes and applies them.
rdar://58528686
Differential Revision: https://reviews.llvm.org/D73303
Signed-off-by: Med Ismail Bennani <medismail.bennani@gmail.com>
Summary:
A *.cpp file header in LLDB (and in LLDB) should like this:
```
//===-- TestUtilities.cpp -------------------------------------------------===//
```
However in LLDB most of our source files have arbitrary changes to this format and
these changes are spreading through LLDB as folks usually just use the existing
source files as templates for their new files (most notably the unnecessary
editor language indicator `-*- C++ -*-` is spreading and in every review
someone is pointing out that this is wrong, resulting in people pointing out that this
is done in the same way in other files).
This patch removes most of these inconsistencies including the editor language indicators,
all the different missing/additional '-' characters, files that center the file name, missing
trailing `===//` (mostly caused by clang-format breaking the line).
Reviewers: aprantl, espindola, jfb, shafik, JDevlieghere
Reviewed By: JDevlieghere
Subscribers: dexonsmith, wuzish, emaste, sdardis, nemanjai, kbarton, MaskRay, atanasyan, arphaman, jfb, abidh, jsji, JDevlieghere, usaxena95, lldb-commits
Tags: #lldb
Differential Revision: https://reviews.llvm.org/D73258
Summary:
I found the above named method hard to read because it had
a) many nested blocks,
b) one return statement at the end with some logic involved,
c) a duplicated while-loop with just small differences in it.
I decided to refactor this function by employing an early exit strategy.
In order to capture the logic in the return statement and to not have it
repeated more than once I chose to implement a very small lamda function
that captures all the variables it needs.
I also replaced the two while-loops with just one.
This is a non-functional change (NFC).
Reviewers: jdoerfert, teemperor
Reviewed By: teemperor
Subscribers: labath, teemperor, lldb-commits
Tags: #lldb
Differential Revision: https://reviews.llvm.org/D70774
Summary:
I found the above named method hard to read because it had
a) many nested blocks and
b) one return statement at the end with some logic involved.
I decided to refactor this function by employing an early exit strategy.
In order to capture the logic in the return statement and to not have it
repeated more than once I chose to implement a very small lamda function
that captures all the variables it needs.
This is a non-functional change (NFC).
Reviewers: jdoerfert
Subscribers: lldb-commits
Tags: #lldb
Differential Revision: https://reviews.llvm.org/D70774
This will allow us to write reusable scripted ThreadPlans, since
you can use key/value pairs with known keys in the plan to parametrize
its behavior.
Differential Revision: https://reviews.llvm.org/D68366
llvm-svn: 373675
Summary:
NFC = [[ https://llvm.org/docs/Lexicon.html#nfc | Non functional change ]]
This commit is the result of modernizing the LLDB codebase by using
`nullptr` instread of `0` or `NULL`. See
https://clang.llvm.org/extra/clang-tidy/checks/modernize-use-nullptr.html
for more information.
This is the command I ran and I to fix and format the code base:
```
run-clang-tidy.py \
-header-filter='.*' \
-checks='-*,modernize-use-nullptr' \
-fix ~/dev/llvm-project/lldb/.* \
-format \
-style LLVM \
-p ~/llvm-builds/debug-ninja-gcc
```
NOTE: There were also changes to `llvm/utils/unittest` but I did not
include them because I felt that maybe this library shall be updated in
isolation somehow.
NOTE: I know this is a rather large commit but it is a nobrainer in most
parts.
Reviewers: martong, espindola, shafik, #lldb, JDevlieghere
Reviewed By: JDevlieghere
Subscribers: arsenm, jvesely, nhaehnle, hiraditya, JDevlieghere, teemperor, rnkovacs, emaste, kubamracek, nemanjai, ki.stfu, javed.absar, arichardson, kbarton, jrtc27, MaskRay, atanasyan, dexonsmith, arphaman, jfb, jsji, jdoerfert, lldb-commits, llvm-commits
Tags: #lldb, #llvm
Differential Revision: https://reviews.llvm.org/D61847
llvm-svn: 361484
A lot of comments in LLDB are surrounded by an ASCII line to delimit the
begging and end of the comment.
Its use is not really consistent across the code base, sometimes the
lines are longer, sometimes they are shorter and sometimes they are
omitted. Furthermore, it looks kind of weird with the 80 column limit,
where the comment actually extends past the line, but not by much.
Furthermore, when /// is used for Doxygen comments, it looks
particularly odd. And when // is used, it incorrectly gives the
impression that it's actually a Doxygen comment.
I assume these lines were added to improve distinguishing between
comments and code. However, given that todays editors and IDEs do a
great job at highlighting comments, I think it's worth to drop this for
the sake of consistency. The alternative is fixing all the
inconsistencies, which would create a lot more churn.
Differential revision: https://reviews.llvm.org/D60508
llvm-svn: 358135
Summary:
Some of these were present in files which should never be read by swig
(and we also had one in the interface file, which is only read by swig).
They are probably leftovers from the time when we were running swig over
lldb headers directly.
While writing this patch, I noticed that some of the #ifdefs were
guarding public functions that were operating on lldb_private data
types. While it wasn't strictly necessary for this patch, I made these
private, as nobody should really be accessing them. This can potentially
break existing code if it happened to use these methods, though it will
only break at build time -- if someone builds against an old header, he
should still be able to link to a new lldb library, since the functions
are still there.
We could keep these public for backward compatbility, but I would argue
that if anyone was actually using these functions for anything, his code
is already broken.
Reviewers: JDevlieghere, clayborg, jingham
Subscribers: lldb-commits
Differential Revision: https://reviews.llvm.org/D60400
llvm-svn: 357984
