The class was taking ownership of the SBCommandPluginInterface pointer
it was passed in, by wrapping it in a shared pointer. This causes a
double free in the unit test when the object is destroyed and the same
pointer gets freed once when the SBCommandPluginInterface goes away and
then again when the shared pointer hits a zero refcount.
Xcode uses a pseudoterminal for the debugger console.
- The upside of this apporach is that it means that it can rely on
LLDB's IOHandlers for multiline and script input.
- The downside of this approach is that the command output is printed to
the PTY and you don't get a SBCommandReturnObject. Adrian added support
for inline diagnostics (#110901) and we'd like to access those from the
IDE.
This patch adds support for registering a callback in the command
interpreter that gives access to the `(SB)CommandReturnObject` right
before it will be printed. The callback implementation can choose
whether it likes to handle printing the result or defer to lldb. If the
callback indicated it handled the result, the command interpreter will
skip printing the result.
We considered a few other alternatives to solve this problem:
- The most obvious one is using `HandleCommand`, which returns a
`SBCommandReturnObject`. The problem with this approach is the multiline
input mentioned above. We would need a way to tell the IDE that it
should expect multiline input, which isn't known until LLDB starts
handling the command.
- To address the multiline issue,we considered exposing (some of the)
IOHandler machinery through the SB API. To solve this particular issue,
that would require reimplementing a ton of logic that already exists
today in the CommandInterpeter. Furthermore that seems like overkill
compared to the proposed solution.
rdar://141254310
# Motivation
Currently, the user can already get the "transcript" (for "what is the
transcript", see `CommandInterpreter::SaveTranscript`). However, the
only way to obtain the transcript data as a user is to first destroy the
debugger, then read the save directory. Note that destroy-callbacks
cannot be used, because 1\ transcript data is private to the command
interpreter (see `CommandInterpreter.h`), and 2\ the writing of the
transcript is *after* the invocation of destory-callbacks (see
`Debugger::Destroy`).
So basically, there is no way to obtain the transcript:
* during the lifetime of a debugger (including the destroy-callbacks,
which often performs logging tasks, where the transcript can be useful)
* without relying on external storage
In theory, there are other ways for user to obtain transcript data
during a debugger's life cycle:
* Use Python API and intercept commands and results.
* Use CLI and record console input/output.
However, such ways rely on the client's setup and are not supported
natively by LLDB.
# Proposal
Add a new Python API `SBCommandInterpreter::GetTranscript()`.
Goals:
* It can be called at any time during the debugger's life cycle,
including in destroy-callbacks.
* It returns data in-memory.
Structured data:
* To make data processing easier, the return type is `SBStructuredData`.
See comments in code for how the data is organized.
* In the future, `SaveTranscript` can be updated to write different
formats using such data (e.g. JSON). This is probably accompanied by a
new setting (e.g. `interpreter.save-session-format`).
# Alternatives
The return type can also be `std::vector<std::pair<std::string,
SBCommandReturnObject>>`. This will make implementation easier, without
having to translate it to `SBStructuredData`. On the other hand,
`SBStructuredData` can convert to JSON easily, so it's more convenient
for user to process.
# Privacy
Both user commands and output/error in the transcript can contain
privacy data. However, as mentioned, the transcript is already available
to the user. The addition of the new API doesn't increase the level of
risk. In fact, it _lowers_ the risk of privacy data being leaked later
on, by avoiding writing such data to external storage.
Once the user (or their code) gets the transcript, it will be their
responsibility to make sure that any required privacy policies are
guaranteed.
# Tests
```
bin/llvm-lit -sv ../external/llvm-project/lldb/test/API/python_api/interpreter/TestCommandInterpreterAPI.py
```
```
bin/llvm-lit -sv ../external/llvm-project/lldb/test/API/commands/session/save/TestSessionSave.py
```
---------
Co-authored-by: Roy Shi <royshi@meta.com>
Co-authored-by: Med Ismail Bennani <ismail@bennani.ma>
These are hardcoded strings that are already present in the data section
of the binary, no need to immediately place them in the ConstString
StringPools. Lots of code still calls `GetBroadcasterClass` and places
the return value into a ConstString. Changing that would be a good
follow-up.
Additionally, calls to these functions are still wrapped in ConstStrings
at the SBAPI layer. This is because we must guarantee the lifetime of
all strings handed out publicly.
Adding command interpreter statistics into "statistics dump" command so
that we can track the command usage frequency for telemetry purpose.
This is useful to answer questions like what is the most frequently used
lldb commands across all our users.
---------
Co-authored-by: jeffreytan81 <jeffreytan@fb.com>
[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
The default constructor for SBCommandInterpreter was (unintentionally)
made protected in 27b6a4e63afe. The goal of the patch was to make the
constructor taking an lldb_private type protected, but due to the
presence of a default argument, this ctor also served as the default
constructor. The latter should remain public.
This commit reinstates the original behavior by removing the default
argument and having an explicit, public default constructor.
None of these need to be in the ConstString StringPool. For the most
part they are constant strings and do not require fast comparisons.
I did change IOHandlerDelegateMultiline slightly -- specifically, the
`m_end_line` member always has a `\n` at the end of it now. This was so
that `IOHandlerGetControlSequence` can always return a StringRef. This
did require a slight change to `IOHandlerIsInputComplete` where we must
drop the newline before comparing it against the input parameter.
Differential Revision: https://reviews.llvm.org/D151597
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
Many SB classes have public constructors or methods involving types that
are private. Some are more obvious (e.g. containing lldb_private in the
name) than others (lldb::FooSP is usually std::shared_pointer<lldb_private::Foo>).
This commit explicitly does not address FileSP, so I'm leaving that one
alone for now.
Some of these were for other SB classes to use and should have been made
protected/private with a friend class entry added. Some of these were
public for some of the swig python helpers to use. I put all of those
functions into a class and made them static methods. The relevant SB
classes mark that class as a friend so they can access those
private/protected members.
I've also removed an outdated SBStructuredData test (can you guess which
constructor it was using?) and updated the other relevant tests.
Differential Revision: https://reviews.llvm.org/D150157
The latter only checks built-in commands. I also added some docs to
make the distinction clear and a test.
Differential Revision: https://reviews.llvm.org/D144929
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
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
This patch adds introduces a new kind of an lldbinit file. Unlike the
lldbinit in the home directory (useful for customizing lldb to the needs
of a particular user), or the cwd lldbinit file (useful for
project-specific settings), this file can be used to customize an entire
lldb installation to a particular environment.
The feature is enabled at build time, by setting the
LLDB_GLOBAL_INIT_DIRECTORY variable to a path to a directory which
should contain an "lldbinit" file. Lldb will then load the file at
startup, if it exists, and if automatic init loading has not been
disabled. Relative paths will be resolved (at runtime) relative to the
location of the lldb library (liblldb or LLDB.framework).
The system-wide lldbinit file will be loaded first, before any
$HOME/.lldbinit and $CWD/.lldbinit files are processed, so that those
can override any system-wide settings.
More information can be found on the RFC thread at
<https://discourse.llvm.org/t/rfc-system-wide-lldbinit/59933>.
Differential Revision: https://reviews.llvm.org/D119831
This patch adds the ability for the user to check if the command
interpreter's IOHandler is interactive.
Differential Revision: https://reviews.llvm.org/D119499
Signed-off-by: Med Ismail Bennani <medismail.bennani@gmail.com>
This way if you have a long stack, you can issue "thread backtrace --count 10"
and then subsequent <Return>-s will page you through the stack.
This took a little more effort than just adding the repeat command, since
the GetRepeatCommand API was returning a "const char *". That meant the command
had to keep the repeat string alive, which is inconvenient. The original
API returned either a nullptr, or a const char *, so I changed the private API to
return an llvm::Optional<std::string>. Most of the patch is propagating that change.
Also, there was a little thinko in fetching the repeat command. We don't
fetch repeat commands for commands that aren't being added to history, which
is in general reasonable. And we don't add repeat commands to the history -
also reasonable. But we do want the repeat command to be able to generate
the NEXT repeat command. So I adjusted the logic in HandleCommand to work
that way.
Differential Revision: https://reviews.llvm.org/D119046
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
The point is to allow users with a related set of script based commands
to organize their commands in a hierarchy in the command set, rather than
having to have only top-level commands.
Differential Revision: https://reviews.llvm.org/D110298
Since https://reviews.llvm.org/D103701 AppendError<...>
sets this for you.
This change includes all of the non-command uses.
Some uses remain where it's either tricky to reason about
the logic, or they aren't paired with AppendError calls.
Reviewed By: teemperor
Differential Revision: https://reviews.llvm.org/D104379
This reverts commit a01b26fb51c710a3a8ef88cc83b0701461f5b9ab, because it
breaks the "finish" command in some way -- the command does not
terminate after it steps out, but continues running the target. The
exact blast radius is not clear, but it at least affects the usage of
the "finish" command in TestGuiBasicDebug.py. The error is *not*
gui-related, as the same issue can be reproduced by running the same
steps outside of the gui.
There is some kind of a race going on, as the test fails only 20% of the
time on the buildbot.
Currently, the interpreter's context is not updated until a command is executed.
This has resulted in the behavior of SB-interface functions and some commands
depends on previous user actions. The interpreter's context can stay uninitialized,
point to a currently selected target, or point to one of previously selected targets.
This patch removes any usages of CommandInterpreter::UpdateExecutionContext.
CommandInterpreter::HandleCommand* functions still may override context temporarily,
but now they always restore it before exiting. CommandInterpreter saves overriden
contexts to the stack, that makes nesting commands possible.
Added test reproduces one of the issues. Without this fix, the last assertion fails
because interpreter's execution context is empty until running "target list", so,
the value of the global property was updated instead of process's local instance.
Differential Revision: https://reviews.llvm.org/D92164
This patch adds the infrastructure to have language specific REPL init
files. It's the foundation work to a following patch that will introduce
Swift REPL init file.
When lldb is launched with the `--repl` option, it will look for a REPL
init file in the home directory and source it. This overrides the
default `~/.lldbinit`, which content might make the REPL behave
unexpectedly. If the REPL init file doesn't exists, lldb will fall back
to the default init file.
rdar://65836048
Differential Revision: https://reviews.llvm.org/D86242
Signed-off-by: Med Ismail Bennani <medismail.bennani@gmail.com>
Currently, `SBCommandInterpreterRunOptions` is defined in
`SBCommandInterpreter.h`. Given that the options are always passed by
reference, a forward declaration is sufficient.
That's not the case for `SBCommandInterpreterRunResults`, which we need
for a new overload for `RunCommandInterpreter` and that returns this new
class by value. We can't include `SBCommandInterpreter.h` because
`SBCommandInterpreter::GetDebugger()` returns SBDebugger by value and
therefore needs a full definition.
This patch moves the definition of `SBCommandInterpreterRunOptions` into
a new header. In a later patch, `SBCommandInterpreterRunResults` will
be defined in there as well, solving the aforementioned problem.
Differential revision: https://reviews.llvm.org/D79115
This implements Greg's suggestion from D78825 to include "auto handle
events" and "spawn thread" in CommandInterpreterRunOptions. This change
is in preparation for adding a new overload for RunCommandInterpreter
that takes only SBCommandInterpreterRunOptions and returns
SBCommandInterpreterRunResults.
Differential revision: https://reviews.llvm.org/D79108
Summary:
This adds support for commands created through the API to support autorepeat.
This covers the case of single word and multiword commands.
Comprehensive tests are included as well.
Reviewers: labath, clayborg
Subscribers: lldb-commits
Tags: #lldb
Differential Revision: https://reviews.llvm.org/D77444
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
I was writing an SB API client and it was crashing on:
bool DoExecute(SBDebugger dbg, char **command, SBCommandReturnObject &result) {
result = subcommand(dbg, "help");
That is because SBCommandReturnObject &result gets initialized inside LLDB by:
bool DoExecute(Args &command, CommandReturnObject &result) override {
// std::unique_ptr gets initialized here from `&result`!!!
SBCommandReturnObject sb_return(&result);
DoExecute(...);
sb_return.Release();
Differential revision: https://reviews.llvm.org/D67589
llvm-svn: 373775
Summary:
We still have some leftovers of the old completion API in the internals of
LLDB that haven't been replaced by the new CompletionRequest. These leftovers
are:
* The return values (int/size_t) in all completion functions.
* Our result array that starts indexing at 1.
* `WordComplete` mode.
I didn't replace them back then because it's tricky to figure out what exactly they
are used for and the completion code is relatively untested. I finally got around
to writing more tests for the API and understanding the semantics, so I think it's
a good time to get rid of them.
A few words why those things should be removed/replaced:
* The return values are really cryptic, partly redundant and rarely documented.
They are also completely ignored by Xcode, so whatever information they contain will end up
breaking Xcode's completion mechanism. They are also partly impossible to even implement
as we assign negative values special meaning and our completion API sometimes returns size_t.
Completion functions are supposed to return -2 to rewrite the current line. We seem to use this
in some untested code path to expand the history repeat character to the full command, but
I haven't figured out why that doesn't work at the moment.
Completion functions return -1 to 'insert the completion character', but that isn't implemented
(even though we seem to activate this feature in LLDB sometimes).
All positive values have to match the number of results. This is obviously just redundant information
as the user can just look at the result list to get that information (which is what Xcode does).
* The result array that starts indexing at 1 is obviously unexpected. The first element of the array is
reserved for the common prefix of all completions (e.g. "foobar" and "footar" -> "foo"). The idea is
that we calculate this to make the life of the API caller easier, but obviously forcing people to have
1-based indices is not helpful (or even worse, forces them to manually copy the results to make it
0-based like Xcode has to do).
* The `WordComplete` mode indicates that LLDB should enter a space behind the completion. The
idea is that we let the top-level API know that we just provided a full completion. Interestingly we
`WordComplete` is just a single bool that somehow represents all N completions. And we always
provide full completions in LLDB, so in theory it should always be true.
The only use it currently serves is providing redundant information about whether we have a single
definitive completion or not (which we already know from the number of results we get).
This patch essentially removes `WordComplete` mode and makes the result array indexed from 0.
It also removes all return values from all internal completion functions. The only non-redundant information
they contain is about rewriting the current line (which is broken), so that functionality was moved
to the CompletionRequest API. So you can now do `addCompletion("blub", "description", CompletionMode::RewriteLine)`
to do the same.
For the SB API we emulate the old behaviour by making the array indexed from 1 again with the common
prefix at index 0. I didn't keep the special negative return codes as we either never sent them before (e.g. -2) or we
didn't even implement them in the Editline handler (e.g. -1).
I tried to keep this patch minimal and I'm aware we can probably now even further simplify a bunch of related code,
but I would prefer doing this in follow-up NFC commits
Reviewers: JDevlieghere
Reviewed By: JDevlieghere
Subscribers: arphaman, abidh, lldb-commits
Tags: #lldb
Differential Revision: https://reviews.llvm.org/D66536
llvm-svn: 369624
This patch moves the remaining completion functions from the
old completion API (that used several variables) to just
passing a single CompletionRequest.
This is for the most part a simple change as we just replace
the old arguments with a single CompletionRequest argument.
There are a few places where I had to create new CompletionRequests
in the called functions as CompletionRequests itself are immutable
and don't expose their internal match list anymore. This means that
if a function wanted to change the CompletionRequest or directly
access the result list, we need to work around this by creating
a new CompletionRequest and a temporary match/description list.
Preparation work for rdar://53769355
llvm-svn: 369000
Completion requests have two fields that are essentially unimplemented:
`m_match_start_point` and `m_max_return_elements`. This would've been
okay, if it wasn't for the fact that this caused a bunch of useless
parameters to be passed around. Occasionally there would be a comment or
assert saying that they are not supported. This patch removes them.
llvm-svn: 367385
I was looking at the current implementation of SourceInitFile and there
were a few things that made this function hard to read:
* The code to find the ~/.lldbinit file is duplicated across the cwd
and non-cwd branch.
* The ./.lldbinit is once computed by resolving .lldbinit and once by
resolving ./.lldbinit.
* It wasn't clear to me what happened when you're sourcing the
.lldbinit file in the current working directory. Apparently we do
nothing when we property to control that is set to warn (makes sense)
and we don't care when the property is set to true (debatable).
* There were at least two branches where the status of the
CommandReturnObject were not set.
This patch attempts to simplify that code.
Differential revision: https://reviews.llvm.org/D61994
llvm-svn: 361080
For some reason I had convinced myself that functions returning by
pointer or reference do not require recording their result. However,
after further considering I don't see how that could work, at least not
with the current implementation. Interestingly enough, the reproducer
instrumentation already (mostly) accounts for this, though the
lldb-instr tool did not.
This patch adds the missing macros and updates the lldb-instr tool.
Differential revision: https://reviews.llvm.org/D60178
llvm-svn: 357639
Move SBRegistry method registrations from SBReproducer.cpp into files
declaring the individual APIs, in order to reduce the memory consumption
during build and improve maintainability. The current humongous
SBRegistry constructor exhausts all memory on a NetBSD system with 4G
RAM + 4G swap, therefore making it impossible to build LLDB.
Differential Revision: https://reviews.llvm.org/D59427
llvm-svn: 356481
Summary:
Our python version of the SB API has (the python equivalent of)
operator bool, but the C++ version doesn't.
This is because our python operators are added by modify-python-lldb.py,
which performs postprocessing on the swig-generated interface files.
In this patch, I add the "operator bool" to all SB classes which have an
IsValid method (which is the same logic used by modify-python-lldb.py).
This way, we make the two interfaces more constent, and it allows us to
rely on swig's automatic syntesis of python __nonzero__ methods instead
of doing manual fixups.
Reviewers: zturner, jingham, clayborg, jfb, serge-sans-paille
Subscribers: jdoerfert, lldb-commits
Differential Revision: https://reviews.llvm.org/D58792
llvm-svn: 355824
The current record macros already log the function being called. This
patch extends the macros to also log their input arguments and removes
explicit logging from the SB API.
This might degrade the amount of information in some cases (because of
smarter casts or efforts to log return values). However I think this is
outweighed by the increased coverage and consistency. Furthermore, using
the reproducer infrastructure, diagnosing bugs in the API layer should
become much easier compared to relying on log messages.
Differential revision: https://reviews.llvm.org/D59101
llvm-svn: 355649
This patch adds the SBReproducer macros needed to capture and reply the
corresponding calls. This patch was generated by running the lldb-instr
tool on the API source files.
Differential revision: https://reviews.llvm.org/D57475
llvm-svn: 355459
Unlike std::make_unique, which is only available since C++14,
std::make_shared is available since C++11. Not only is std::make_shared
a lot more readable compared to ::reset(new), it also performs a single
heap allocation for the object and control block.
Differential revision: https://reviews.llvm.org/D57990
llvm-svn: 353764
