(based on a conversation I had with @labath yesterday in
https://github.com/llvm/llvm-project/pull/106442)
Most APIs that currently vend a Status would be better served by
returning llvm::Expected<> instead. If possibles APIs should be
refactored to avoid Status. The only legitimate long-term uses of Status
are objects that need to store an error for a long time (which should be
questioned as a design decision, too).
This patch makes the transition to llvm::Error easier by making the
places that cannot switch to llvm::Error explicit: They are marked with
a call to Status::clone(). Every other API can and should be refactored
to use llvm::Expected. In the end Status should only be used in very few
places.
Whenever an unchecked Error is dropped by Status it logs this to the
verbose API channel.
Implementation notes:
This patch introduces two new kinds of error_category as well as new
llvm::Error types. Here is the mapping of lldb::ErrorType to
llvm::Errors:
```
(eErrorTypeInvalid)
eErrorTypeGeneric llvm::StringError
eErrorTypePOSIX llvm::ECError
eErrorTypeMachKernel MachKernelError
eErrorTypeExpression llvm::ErrorList<ExpressionError>
eErrorTypeWin32 Win32Error
```
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.
Compilers and language runtimes often use helper functions that are
fundamentally uninteresting when debugging anything but the
compiler/runtime itself. This patch introduces a user-extensible
mechanism that allows for these frames to be hidden from backtraces and
automatically skipped over when navigating the stack with `up` and
`down`.
This does not affect the numbering of frames, so `f <N>` will still
provide access to the hidden frames. The `bt` output will also print a
hint that frames have been hidden.
My primary motivation for this feature is to hide thunks in the Swift
programming language, but I'm including an example recognizer for
`std::function::operator()` that I wished for myself many times while
debugging LLDB.
rdar://126629381
Example output. (Yes, my proof-of-concept recognizer could hide even
more frames if we had a method that returned the function name without
the return type or I used something that isn't based off regex, but it's
really only meant as an example).
before:
```
(lldb) thread backtrace --filtered=false
* thread #1, queue = 'com.apple.main-thread', stop reason = breakpoint 1.1
* frame #0: 0x0000000100001f04 a.out`foo(x=1, y=1) at main.cpp:4:10
frame #1: 0x0000000100003a00 a.out`decltype(std::declval<int (*&)(int, int)>()(std::declval<int>(), std::declval<int>())) std::__1::__invoke[abi:se200000]<int (*&)(int, int), int, int>(__f=0x000000016fdff280, __args=0x000000016fdff224, __args=0x000000016fdff220) at invoke.h:149:25
frame #2: 0x000000010000399c a.out`int std::__1::__invoke_void_return_wrapper<int, false>::__call[abi:se200000]<int (*&)(int, int), int, int>(__args=0x000000016fdff280, __args=0x000000016fdff224, __args=0x000000016fdff220) at invoke.h:216:12
frame #3: 0x0000000100003968 a.out`std::__1::__function::__alloc_func<int (*)(int, int), std::__1::allocator<int (*)(int, int)>, int (int, int)>::operator()[abi:se200000](this=0x000000016fdff280, __arg=0x000000016fdff224, __arg=0x000000016fdff220) at function.h:171:12
frame #4: 0x00000001000026bc a.out`std::__1::__function::__func<int (*)(int, int), std::__1::allocator<int (*)(int, int)>, int (int, int)>::operator()(this=0x000000016fdff278, __arg=0x000000016fdff224, __arg=0x000000016fdff220) at function.h:313:10
frame #5: 0x0000000100003c38 a.out`std::__1::__function::__value_func<int (int, int)>::operator()[abi:se200000](this=0x000000016fdff278, __args=0x000000016fdff224, __args=0x000000016fdff220) const at function.h:430:12
frame #6: 0x0000000100002038 a.out`std::__1::function<int (int, int)>::operator()(this= Function = foo(int, int) , __arg=1, __arg=1) const at function.h:989:10
frame #7: 0x0000000100001f64 a.out`main(argc=1, argv=0x000000016fdff4f8) at main.cpp:9:10
frame #8: 0x0000000183cdf154 dyld`start + 2476
(lldb)
```
after
```
(lldb) bt
* thread #1, queue = 'com.apple.main-thread', stop reason = breakpoint 1.1
* frame #0: 0x0000000100001f04 a.out`foo(x=1, y=1) at main.cpp:4:10
frame #1: 0x0000000100003a00 a.out`decltype(std::declval<int (*&)(int, int)>()(std::declval<int>(), std::declval<int>())) std::__1::__invoke[abi:se200000]<int (*&)(int, int), int, int>(__f=0x000000016fdff280, __args=0x000000016fdff224, __args=0x000000016fdff220) at invoke.h:149:25
frame #2: 0x000000010000399c a.out`int std::__1::__invoke_void_return_wrapper<int, false>::__call[abi:se200000]<int (*&)(int, int), int, int>(__args=0x000000016fdff280, __args=0x000000016fdff224, __args=0x000000016fdff220) at invoke.h:216:12
frame #6: 0x0000000100002038 a.out`std::__1::function<int (int, int)>::operator()(this= Function = foo(int, int) , __arg=1, __arg=1) const at function.h:989:10
frame #7: 0x0000000100001f64 a.out`main(argc=1, argv=0x000000016fdff4f8) at main.cpp:9:10
frame #8: 0x0000000183cdf154 dyld`start + 2476
Note: Some frames were hidden by frame recognizers
```
This patch tries to fix an issue with the windows debug builds where the
PDB file for python scripted interfaces cannot be opened since its path
length exceed the windows `MAX_PATH` limit:
https://github.com/llvm/llvm-project/pull/101672#issuecomment-2289481324
This patch addresses the issue by building all the interfaces as a
single library plugin that initiliazes each component as part of its
`Initialize` method, instead of building each interface as its own
library plugin.
This keeps the build artifact path length smaller while respecting the
naming convention and without making any exception in the build system.
Fixes#104895.
Signed-off-by: Med Ismail Bennani <ismail@bennani.ma>
```
lldb-python.h:16:30: warning: ‘g_fcxx_modules_workaround’ defined but not used [-Wunused-variable]
16 | static llvm::Expected<bool> *g_fcxx_modules_workaround;
|
```
Workaround originally added in 36cb29cbbe1b22dcd298ad65e1fabe899b7d7249.
This reverts commit 9effefbae8d96006a4dd29bb9ab8532fd408559d.
With the include order in ScriptedProcessPythonInterface.cpp fixed
(though I cannot explain exactly why it works) and removes the /H flag
intended for debugging this issue.
I think it is something to do with Process.h pulling in PosixApi.h
somewhere along the line, and including Process.h after lldb-python.h
means that NO_PID_T is defined to prevent a redefinition of pid_t.
This patch relands 2402b3213c2f to investigate the ambigious typedef
issue happening on the windows bots:
https://lab.llvm.org/buildbot/#/builders/141/builds/1175/
However this patch adds the `/H` compiler flag when building
the ScriptedProcessPythonInterface library to be able to investigate the
include order issue.
This patch will be revert after 1 failing run on the windows bot.
Signed-off-by: Med Ismail Bennani <ismail@bennani.ma>
This patch relands 2402b3213c2f to investigate the ambigious typedef
issue happening on the windows bots:
https://lab.llvm.org/buildbot/#/builders/141/builds/1175/
However this patch adds the `-H` compiler flag when building
the ScriptedProcessPythonInterface library to be able to investigate the
include order issue.
This patch will be revert after 1 failing run on the windows bot.
Signed-off-by: Med Ismail Bennani <ismail@bennani.ma>
This patch relands 2402b3213c2f to investigate the ambigious typedef
issue happening on the windows bots:
https://lab.llvm.org/buildbot/#/builders/141/builds/1175/
However this patch adds the `-H` & `-MM` compiler flags when building
the ScriptedProcessPythonInterface library to be able to investigate the
include order issue.
This patch will be revert after 1 failing run on the windows bot.
Signed-off-by: Med Ismail Bennani <ismail@bennani.ma>
This patch is a follow-up to bccff3baeff8 which adds the
`ScriptedProcess` extension to the `scripting template list` command as
well as its description.
Signed-off-by: Med Ismail Bennani <ismail@bennani.ma>
This patch is a follow-up to bccff3baeff8 which adds the
`OperatingSystem` extension to the `scripting template list` command as
well as its description.
Signed-off-by: Med Ismail Bennani <ismail@bennani.ma>
This patch is a follow-up to bccff3baeff8 which adds the
`ScriptedPlatform` extension to the `scripting template list` command as
well as its description.
Signed-off-by: Med Ismail Bennani <ismail@bennani.ma>
This patch introduces a new `template` multiword sub-command to the
`scripting` top-level command. As the name suggests, this sub-command
operates on scripting templates, and currently has the ability to
automatically discover the various scripting extensions that lldb
supports.
This was previously reviewed in #97273.
Signed-off-by: Med Ismail Bennani <ismail@bennani.ma>
This patch tries to fix the following build failure on windows:
https://lab.llvm.org/buildbot/#/builders/141/builds/1083
This started happening following 2914a4b88837, and it seems to be caused
by some special `#include` ordering for the lldb-python header on Windows.
Signed-off-by: Med Ismail Bennani <ismail@bennani.ma>
This patch tries to fix the following build failure on windows:
https://lab.llvm.org/buildbot/#/builders/141/builds/1083
This started happening following 2914a4b88837, and it seems to be caused
by some special `#include` ordering for the lldb-python header on Windows.
Signed-off-by: Med Ismail Bennani <ismail@bennani.ma>
The condition checking for missing class name, interpreter dictionary,
and script object incorrectly used logical AND (&&), which could never
be true to enter the 'if' block.
This commit uses separate if conditions for each class name, interpreter
dictionary, and script object.
Cought by cppcheck -
lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedPythonInterface.h:89:11:
warning: Identical inner 'if' condition is always true.
[identicalInnerCondition]
lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedPythonInterface.h:91:16:
warning: Identical inner 'if' condition is always true.
[identicalInnerCondition]
Fix#89195
---------
Co-authored-by: Shivam Gupta <shivma98.tkg@gmail.com>
This patch tries to fix the following build failure on windows:
https://lab.llvm.org/buildbot/#/builders/141/builds/1083
This started happening following 2914a4b88837, and it seems to be caused
by some special `#include` ordering for the lldb-python header on Windows.
Signed-off-by: Med Ismail Bennani <ismail@bennani.ma>
This patch introduces a new `template` multiword sub-command to the
`scripting` top-level command. As the name suggests, this sub-command
operates on scripting templates, and currently has the ability to
automatically discover the various scripting extensions that lldb
supports.
This was previously reviewed in #97273.
Signed-off-by: Med Ismail Bennani <ismail@bennani.ma>
This patch introduces a new `template` multiword sub-command to the
`scripting` top-level command. As the name suggests, this sub-command
operates on scripting templates, and currently has the ability to
automatically discover the various scripting extensions that lldb
supports.
Signed-off-by: Med Ismail Bennani <ismail@bennani.ma>
Among other things, returning an empty string as the repeat command
disables auto-repeat, which can be useful for state-changing commands.
There's one remaining refinement to this setup, which is that for parsed
script commands, it should be possible to change an option value, or add
a new option value that wasn't originally specified, then ask lldb "make
this back into a command string". That would make doing fancy things
with repeat commands easier.
That capability isn't present in the lldb_private side either, however.
So that's for a next iteration.
I haven't added this to the docs on adding commands yet. I wanted to
make sure this was an acceptable approach before I spend the time to do
that.
This patch changes `ScriptedThreadPlan::GetStopDescription` behavior by
discarding its return value since it is optional in the first place (the
user doesn't need to provide a return value in their implementation).
This patch also addresses the test failures in TestStepScripted
following 9a9ec22 and re-enables the tests that were XFAIL'd previously.
The issue here was that the `Stream*` that's passed to
`ThreadPlanPython::GetDescription` wasn't being passed by reference to
the python method so it was never updated to reflect how the python
method interacted with it.
This patch solves this issue by making a temporary `StreamSP` that will
be passed to the python method by reference, after what we will copy its
content to the caller `Stream` pointer argument.
---------
Signed-off-by: Med Ismail Bennani <ismail@bennani.ma>
This patch makes ScriptedThreadPlan conforming to the ScriptedInterface
& ScriptedPythonInterface facilities by introducing 2
ScriptedThreadPlanInterface & ScriptedThreadPlanPythonInterface classes.
This allows us to get rid of every ScriptedThreadPlan-specific SWIG
method and re-use the same affordances as other scripting offordances,
like Scripted{Process,Thread,Platform} & OperatingSystem.
To do so, this adds new transformer methods for `ThreadPlan`, `Stream` &
`Event`, to allow the bijection between C++ objects and their python
counterparts.
This just re-lands #70392 after fixing test failures.
Signed-off-by: Med Ismail Bennani <ismail@bennani.ma>
For the significant amount of call sites that want to create an
incontrovertible error, such a wrapper function creates a significant
readability improvement and lowers the cost of entry to add error
handling in more places.
This patch makes ScriptedThreadPlan conforming to the ScriptedInterface
& ScriptedPythonInterface facilities by introducing 2
ScriptedThreadPlanInterface & ScriptedThreadPlanPythonInterface classes.
This allows us to get rid of every ScriptedThreadPlan-specific SWIG
method and re-use the same affordances as other scripting offordances,
like Scripted{Process,Thread,Platform} & OperatingSystem.
To do so, this adds new transformer methods for `ThreadPlan`, `Stream` &
`Event`, to allow the bijection between C++ objects and their python
counterparts.
This just re-lands #70392 after fixing test failures.
Signed-off-by: Med Ismail Bennani <ismail@bennani.ma>
Addresses issue #87243.
The current code incorrectly checks the validity of ```obj``` twice when
it should be checking the new ```str_obj``` pointer.
Signed-off-by: Troy-Butler <squintik@outlook.com>
Co-authored-by: Troy-Butler <squintik@outlook.com>
There was a think-o in a previous commit that made us only able to
define 1 line commands when using command script add interactively.
There was also no test for this feature, so I fixed the think-o and
added a test.
The Python documentation [1] says that `PyImport_AppendInittab` should
be called before `Py_Initialize()`. Starting with Python 3.12, this is
enforced with a fatal error:
Fatal Python error: PyImport_AppendInittab: PyImport_AppendInittab()
may not be called after Py_Initialize()
This commit ensures we only modify the table of built-in modules if
Python hasn't been initialized. For Python embedded in LLDB, that means
this happen exactly once, before the first call to `Py_Initialize`,
which becomes a NO-OP after. However, when lldb is imported in an
existing Python interpreter, Python will have already been initialized,
but by definition, the lldb module will already have been loaded, so
it's safe to skip adding it (again).
This fixes#70453.
[1] https://docs.python.org/3.12/c-api/import.html#c.PyImport_AppendInittab
This allows you to specify options and arguments and their definitions
and then have lldb handle the completions, help, etc. in the same way
that lldb does for its parsed commands internally.
This feature has some design considerations as well as the code, so I've
also set up an RFC, but I did this one first and will put the RFC
address in here once I've pushed it...
Note, the lldb "ParsedCommand interface" doesn't actually do all the
work that it should. For instance, saying the type of an option that has
a completer doesn't automatically hook up the completer, and ditto for
argument values. We also do almost no work to verify that the arguments
match their definition, or do auto-completion for them. This patch
allows you to make a command that's bug-for-bug compatible with built-in
ones, but I didn't want to stall it on getting the auto-command checking
to work all the way correctly.
As an overall design note, my primary goal here was to make an interface
that worked well in the script language. For that I needed, for
instance, to have a property-based way to get all the option values that
were specified. It was much more convenient to do that by making a
fairly bare-bones C interface to define the options and arguments of a
command, and set their values, and then wrap that in a Python class
(installed along with the other bits of the lldb python module) which
you can then derive from to make your new command. This approach will
also make it easier to experiment.
See the file test_commands.py in the test case for examples of how this
works.
Temporarily revert to unblock the CI bots, this is breaking the -DLLVM_ENABLE_MODULES=On
modules style build. I've notified Ismail.
This reverts commit 888501bc631c4f6d373b4081ff6c504a1ce4a682.
This patch makes ScriptedThreadPlan conforming to the ScriptedInterface
& ScriptedPythonInterface facilities by introducing 2
ScriptedThreadPlanInterface & ScriptedThreadPlanPythonInterface classes.
This allows us to get rid of every ScriptedThreadPlan-specific SWIG
method and re-use the same affordances as other scripting offordances,
like Scripted{Process,Thread,Platform} & OperatingSystem.
To do so, this adds new transformer methods for `ThreadPlan`, `Stream` &
`Event`, to allow the bijection between C++ objects and their python
counterparts.
Signed-off-by: Med Ismail Bennani <ismail@bennani.ma>
This patch enforces that every scripted object implements all the
necessary abstract methods.
Every scripted affordance language interface can implement a list of
abstract methods name that checked when the object is instanciated.
Since some scripting affordances implementations can be derived from
template base classes, we can't check the object dictionary since it
will contain the definition of the base class, so instead, this checks
the scripting class dictionary.
Previously, for the various python interfaces, we used
`ABC.abstractmethod` decorators but this is too language specific and
doesn't work for scripting affordances that are not derived from
template base classes (i.e OperatingSystem, ScriptedThreadPlan, ...), so
this patch provides generic/language-agnostic checks for every scripted
affordance.
Signed-off-by: Med Ismail Bennani <ismail@bennani.ma>
This patch enforces that every scripted object implements all the
necessary abstract methods.
Every scripted affordance language interface can implement a list of
abstract methods name that checked when the object is instanciated.
Since some scripting affordances implementations can be derived from
template base classes, we can't check the object dictionary since it
will contain the definition of the base class, so instead, this checks
the scripting class dictionary.
Previously, for the various python interfaces, we used
`ABC.abstractmethod` decorators but this is too language specific and
doesn't work for scripting affordances that are not derived from
template base classes (i.e OperatingSystem, ScriptedThreadPlan, ...), so
this patch provides generic/language-agnostic checks for every scripted
affordance.
Signed-off-by: Med Ismail Bennani <ismail@bennani.ma>
This patch makes the various Scripted Interface base class abstract by
making the `CreatePluginObject` method pure virtual.
This means that we cannot construct a Scripted Interface base class
instance, so this patch also updates the various
`ScriptedInterpreter::CreateScripted*Interface` methods to return a
`nullptr` instead.`
This patch also removes the `ScriptedPlatformInterface` member from the
`ScriptInterpreter` class since it the interpreter can be owned by the
`ScriptedPlatform` instance itself, like we do for `ScriptedProcess`
objects.
Signed-off-by: Med Ismail Bennani <ismail@bennani.ma>