This patch allows the crashlog script to surface its errors to lldb by
using the provided SBCommandReturnObject argument.
rdar://95048193
Differential Revision: https://reviews.llvm.org/D129614
Signed-off-by: Med Ismail Bennani <medismail.bennani@gmail.com>
Resubmission of https://reviews.llvm.org/D130309 with the 2 patches that fixed the linux buildbot, and new windows fixes.
The FileSpec APIs allow users to modify instance variables directly by getting a non const reference to the directory and filename instance variables. This makes it impossible to control all of the times the FileSpec object is modified so we can clear cached member variables like m_resolved and with an upcoming patch caching if the file is relative or absolute. This patch modifies the APIs of FileSpec so no one can modify the directory or filename instance variables directly by adding set accessors and by removing the get accessors that are non const.
Many clients were using FileSpec::GetCString(...) which returned a unique C string from a ConstString'ified version of the result of GetPath() which returned a std::string. This caused many locations to use this convenient function incorrectly and could cause many strings to be added to the constant string pool that didn't need to. Most clients were converted to using FileSpec::GetPath().c_str() when possible. Other clients were modified to use the newly renamed version of this function which returns an actualy ConstString:
ConstString FileSpec::GetPathAsConstString(bool denormalize = true) const;
This avoids the issue where people were getting an already uniqued "const char *" that came from a ConstString only to put the "const char *" back into a "ConstString" object. By returning the ConstString instead of a "const char *" clients can be more efficient with the result.
The patch:
- Removes the non const GetDirectory() and GetFilename() get accessors
- Adds set accessors to replace the above functions: SetDirectory() and SetFilename().
- Adds ClearDirectory() and ClearFilename() to replace usage of the FileSpec::GetDirectory().Clear()/FileSpec::GetFilename().Clear() call sites
- Fixed all incorrect usage of FileSpec::GetCString() to use FileSpec::GetPath().c_str() where appropriate, and updated other call sites that wanted a ConstString to use the newly returned ConstString appropriately and efficiently.
Differential Revision: https://reviews.llvm.org/D130549
The FileSpect APIs allow users to modify instance variables directly by getting a non const reference to the directory and filename instance variables. This makes it impossibly to control all of the times the FileSpec object is modified so we can clear the cache. This patch modifies the APIs of FileSpec so no one can modify the directory or filename directly by adding set accessors and by removing the get accessors that are non const.
Many clients were using FileSpec::GetCString(...) which returned a unique C string from a ConstString'ified version of the result of GetPath() which returned a std::string. This caused many locations to use this convenient function incorrectly and could cause many strings to be added to the constant string pool that didn't need to. Most clients were converted to using FileSpec::GetPath().c_str() when possible. Other clients were modified to use the newly renamed version of this function which returns an actualy ConstString:
ConstString FileSpec::GetPathAsConstString(bool denormalize = true) const;
This avoids the issue where people were getting an already uniqued "const char *" that came from a ConstString only to put the "const char *" back into a "ConstString" object. By returning the ConstString instead of a "const char *" clients can be more efficient with the result.
The patch:
- Removes the non const GetDirectory() and GetFilename() get accessors
- Adds set accessors to replace the above functions: SetDirectory() and SetFilename().
- Adds ClearDirectory() and ClearFilename() to replace usage of the FileSpec::GetDirectory().Clear()/FileSpec::GetFilename().Clear() call sites
- Fixed all incorrect usage of FileSpec::GetCString() to use FileSpec::GetPath().c_str() where appropriate, and updated other call sites that wanted a ConstString to use the newly returned ConstString appropriately and efficiently.
Differential Revision: https://reviews.llvm.org/D130309
We were just emitting "invalid module" w/o saying which module. That's
not particularly helpful.
Differential Revision: https://reviews.llvm.org/D129338
We dropped downstream support for Python 2 in the previous release. Now
that we have branched for the next release the window where this kind of
change could introduce conflicts is closing too. Start by getting rid of
Python 2 support in the Script Interpreter plugin.
Differential revision: https://reviews.llvm.org/D124429
This patch removes the ability to instantiate the LLDB FileSystem class
with a FileCollector. It keeps the ability to collect files, but uses
the FileCollectorFileSystem to do that transparently.
Because the two are intertwined, this patch also removes the
finalization logic which copied the files over out of process.
Most of our code was including Log.h even though that is not where the
"lldb" log channel is defined (Log.h defines the generic logging
infrastructure). This worked because Log.h included Logging.h, even
though it should.
After the recent refactor, it became impossible the two files include
each other in this direction (the opposite inclusion is needed), so this
patch removes the workaround that was put in place and cleans up all
files to include the right thing. It also renames the file to LLDBLog to
better reflect its purpose.
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
We got a few crash reports that showed LLDB initializing Python on two
separate threads. Make sure Python is initialized exactly once.
rdar://87287005
Differential revision: https://reviews.llvm.org/D117601
Return our PythonObject wrappers instead of raw PyObjects (obfuscated as
void *). This ensures that ownership (reference counts) of python
objects is automatically tracked.
Differential Revision: https://reviews.llvm.org/D117462
The GIL must be held when calling any Python C API functions. In multithreaded applications that use callbacks this requirement can easily be violated by accident. A general tool to ensure GIL health is not available, but patching Python Py_INCREF to add an assert provides a basic health check:
```
+int PyGILState_Check(void); /* Include/internal/pystate.h */
+
#define Py_INCREF(op) ( \
+ assert(PyGILState_Check()), \
_Py_INC_REFTOTAL _Py_REF_DEBUG_COMMA \
((PyObject *)(op))->ob_refcnt++)
#define Py_DECREF(op) \
do { \
+ assert(PyGILState_Check()); \
PyObject *_py_decref_tmp = (PyObject *)(op); \
if (_Py_DEC_REFTOTAL _Py_REF_DEBUG_COMMA \
--(_py_decref_tmp)->ob_refcnt != 0) \
```
Adding this assertion causes around 50 test failures in LLDB. Adjusting the scope of things guarded by `py_lock` fixes them.
More background: https://docs.python.org/3/glossary.html#term-global-interpreter-lock
Patch by Ralf Grosse-Kunstleve
Differential Revision: https://reviews.llvm.org/D114722
When LLDB receives a SIGINT while running the embedded Python REPL it
currently just crashes in ScriptInterpreterPythonImpl::Interrupt with an
error such as the one below:
Fatal Python error: PyThreadState_Get: the function must be called
with the GIL held, but the GIL is released (the current Python thread
state is NULL)
The faulty code that causes this error is this part of
ScriptInterpreterPythonImpl::Interrupt:
PyThreadState *state = PyThreadState_GET();
if (!state)
state = GetThreadState();
if (state) {
long tid = state->thread_id;
PyThreadState_Swap(state);
int num_threads = PyThreadState_SetAsyncExc(tid, PyExc_KeyboardInterrupt);
The obvious fix I tried is to just acquire the GIL before this code is
running which fixes the crash but the KeyboardInterrupt we want to raise
immediately is actually just queued and would only be raised once the
next line of input has been parsed (which e.g. won't interrupt Python
code that is currently waiting on a timer or IO from what I can see).
Also none of the functions we call here is marked as safe to be called
from a signal handler from what I can see, so we might still end up
crashing here with some bad timing.
Python 3.2 introduced PyErr_SetInterrupt to solve this and the function
takes care of all the details and avoids doing anything that isn't safe
to do inside a signal handler. The only thing we need to do is to
manually setup our own fake SIGINT handler that behaves the same way as
the standalone Python REPL signal handler (which raises a
KeyboardInterrupt).
From what I understand the old code used to work with Python 2 so I kept
the old code around until we officially drop support for Python 2.
There is a small gap here with Python 3.0->3.1 where we might still be
crashing, but those versions have reached their EOL more than a decade
ago so I think we don't need to bother about them.
Differential revision: https://reviews.llvm.org/D104886
This starts to fix the other half of the lifetime problems in this code
-- dangling references. SB objects created on the stack will go away
when the function returns, which is a problem if the python code they
were meant for stashes a reference to them somewhere. Most of the time
this goes by unnoticed, as the code rarely has a reason to store these,
but in case it does, we shouldn't respond by crashing.
This patch fixes the management for a couple of SB objects (Debugger,
Frame, Thread). The SB objects are now created on the heap, and
their ownership is immediately passed on to SWIG, which will ensure they
are destroyed when the last python reference goes away. I will handle
the other objects in separate patches.
I include one test which demonstrates the lifetime issue for SBDebugger.
Strictly speaking, one should create a test case for each of these
objects and each of the contexts they are being used. That would require
figuring out how to persist (and later access) each of these objects.
Some of those may involve a lot of hoop-jumping (we can run python code
from within a frame-format string). I don't think that is
necessary/worth it since the new wrapper functions make it very hard to
get this wrong.
Differential Revision: https://reviews.llvm.org/D115925
StructuredDataImpl ownership semantics is unclear at best. Various
structures were holding a non-owning pointer to it, with a comment that
the object is owned somewhere else. From what I was able to gather that
"somewhere else" was the SBStructuredData object, but I am not sure that
all created object eventually made its way there. (It wouldn't matter
even if they did, as we are leaking most of our SBStructuredData
objects.)
Since StructuredDataImpl is just a collection of two (shared) pointers,
there's really no point in elaborate lifetime management, so this patch
replaces all StructuredDataImpl pointers with actual objects or
unique_ptrs to it. This makes it much easier to resolve SBStructuredData
leaks in a follow-up patch.
Differential Revision: https://reviews.llvm.org/D114791
The LLDBSWIGPython functions had (at least) two problems:
- There wasn't a single source of truth (a header file) for the
prototypes of these functions. This meant that subtle differences
in copies of function declarations could go by undetected. And
not-so-subtle differences would result in strange runtime failures.
- All of the declarations had to have an extern "C" interface, because
the function definitions were being placed inside and extert "C" block
generated by swig.
This patch fixes both problems by moving the function definitions to the
%header block of the swig files. This block is not surrounded by extern
"C", and seems more appropriate anyway, as swig docs say it is meant for
"user-defined support code" (whereas the previous %wrapper code was for
automatically-generated wrappers).
It also puts the declarations into the SWIGPythonBridge header file
(which seems to have been created for this purpose), and ensures it is
included by all code wishing to define or use these functions. This
means that any differences in the declaration become a compiler error
instead of a runtime failure.
Differential Revision: https://reviews.llvm.org/D114369
Using an lldb_private object in the bindings involves three steps
- wrapping the object in it's lldb::SB variant
- using swig to convert/wrap that to a PyObject
- wrapping *that* in a lldb_private::python::PythonObject
Our SBTypeToSWIGWrapper was only handling the middle part. This doesn't
just result in increased boilerplate in the callers, but is also a
functionality problem, as it's very hard to get the lifetime of of all
of these objects right. Most of the callers are creating the SB object
(step 1) on the stack, which means that we end up with dangling python
objects after the function terminates. Most of the time this isn't a
problem, because the python code does not need to persist the objects.
However, there are legitimate cases where they can do it (and even if
the use case is not completely legitimate, crashing is not the best
response to that).
For this reason, some of our code creates the SB object on the heap, but
it has another problem -- it never gets cleaned up.
This patch begins to add a new function (ToSWIGWrapper), which does all
of the three steps, while properly taking care of ownership. In the
first step, I have converted most of the leaky code (except for
SBStructuredData, which needs a bit more work).
Differential Revision: https://reviews.llvm.org/D114259
Apparently "{sys.prefix}/bin/python3" isn't where you find the
python interpreter on windows, so the test I wrote for
-print-script-interpreter-info is failing.
We can't rely on sys.executable at runtime, because that will point
to lldb.exe not python.exe.
We can't just record sys.executable from build time, because python
could have been moved to a different location.
But it should be OK to apply relative path from sys.prefix to sys.executable
from build-time to the sys.prefix at runtime.
Reviewed By: JDevlieghere
Differential Revision: https://reviews.llvm.org/D113650
When LLDB receives a SIGINT while running the embedded Python REPL it currently
just crashes in `ScriptInterpreterPythonImpl::Interrupt` with an error such as
the one below:
```
Fatal Python error: PyThreadState_Get: the function must be called with the GIL
held, but the GIL is released (the current Python thread state is NULL)
```
The faulty code that causes this error is this part of `ScriptInterpreterPythonImpl::Interrupt`:
```
PyThreadState *state = PyThreadState_GET();
if (!state)
state = GetThreadState();
if (state) {
long tid = state->thread_id;
PyThreadState_Swap(state);
int num_threads = PyThreadState_SetAsyncExc(tid, PyExc_KeyboardInterrupt);
```
The obvious fix I tried is to just acquire the GIL before this code is running
which fixes the crash but the `KeyboardInterrupt` we want to raise immediately
is actually just queued and would only be raised once the next line of input has
been parsed (which e.g. won't interrupt Python code that is currently waiting on
a timer or IO from what I can see). Also none of the functions we call here is
marked as safe to be called from a signal handler from what I can see, so we
might still end up crashing here with some bad timing.
Python 3.2 introduced `PyErr_SetInterrupt` to solve this and the function takes
care of all the details and avoids doing anything that isn't safe to do inside a
signal handler. The only thing we need to do is to manually setup our own fake
SIGINT handler that behaves the same way as the standalone Python REPL signal
handler (which raises a KeyboardInterrupt).
From what I understand the old code used to work with Python 2 so I kept the old
code around until we officially drop support for Python 2.
There is a small gap here with Python 3.0->3.1 where we might still be crashing,
but those versions have reached their EOL more than a decade ago so I think we
don't need to bother about them.
Reviewed By: JDevlieghere
Differential Revision: https://reviews.llvm.org/D104886
It is surprisingly difficult to write a simple python script that
can reliably `import lldb` without failing, or crashing. I'm
currently resorting to convolutions like this:
def find_lldb(may_reexec=False):
if prefix := os.environ.get('LLDB_PYTHON_PREFIX'):
if os.path.realpath(prefix) != os.path.realpath(sys.prefix):
raise Exception("cannot import lldb.\n"
f" sys.prefix should be: {prefix}\n"
f" but it is: {sys.prefix}")
else:
line1, line2 = subprocess.run(
['lldb', '-x', '-b', '-o', 'script print(sys.prefix)'],
encoding='utf8', stdout=subprocess.PIPE,
check=True).stdout.strip().splitlines()
assert line1.strip() == '(lldb) script print(sys.prefix)'
prefix = line2.strip()
os.environ['LLDB_PYTHON_PREFIX'] = prefix
if sys.prefix != prefix:
if not may_reexec:
raise Exception(
"cannot import lldb.\n" +
f" This python, at {sys.prefix}\n"
f" does not math LLDB's python at {prefix}")
os.environ['LLDB_PYTHON_PREFIX'] = prefix
python_exe = os.path.join(prefix, 'bin', 'python3')
os.execl(python_exe, python_exe, *sys.argv)
lldb_path = subprocess.run(['lldb', '-P'],
check=True, stdout=subprocess.PIPE,
encoding='utf8').stdout.strip()
sys.path = [lldb_path] + sys.path
This patch aims to replace all that with:
#!/usr/bin/env lldb-python
import lldb
...
... by adding the following features:
* new command line option: --print-script-interpreter-info. This
prints language-specific information about the script interpreter
in JSON format.
* new tool (unix only): lldb-python which finds python and exec's it.
Reviewed By: JDevlieghere
Differential Revision: https://reviews.llvm.org/D112973
There is no reason why this function should be returning a ConstString.
While modifying these files, I also fixed several instances where
GetPluginName and GetPluginNameStatic were returning different strings.
I am not changing the return type of GetPluginNameStatic in this patch, as that
would necessitate additional changes, and this patch is big enough as it is.
Differential Revision: https://reviews.llvm.org/D111877
Refactor TerminalState to make the code simpler. Move 'struct termios'
to a PImpl-style subclass. Add an RAII interface to automatically store
and restore the state.
Differential revision: https://reviews.llvm.org/D110721
In all these years, we haven't found a use for this function (it has
zero callers). Lets just remove the boilerplate.
Differential Revision: https://reviews.llvm.org/D109600
Add the ability to silence command script import. The motivation for
this change is being able to add command script import -s
lldb.macosx.crashlog to your ~/.lldbinit without it printing the
following message at the beginning of every debug session.
"malloc_info", "ptr_refs", "cstr_refs", "find_variable", and
"objc_refs" commands have been installed, use the "--help" options on
these commands for detailed help.
In addition to forwarding the silent option to LoadScriptingModule, this
also changes ScriptInterpreterPythonImpl::ExecuteOneLineWithReturn and
ScriptInterpreterPythonImpl::ExecuteMultipleLines to honor the enable IO
option in ExecuteScriptOptions, which until now was ignored.
Note that IO is only enabled (or disabled) at the start of a session,
and for this particular use case, that's done when taking the Python
lock in LoadScriptingModule, which means that the changes to these two
functions are not strictly necessary, but (IMO) desirable nonetheless.
Differential revision: https://reviews.llvm.org/D105327
This is an NFC cleanup.
Many of the API's that returned BreakpointOptions always returned valid ones.
Internally the BreakpointLocations usually have null BreakpointOptions, since they
use their owner's options until an option is set specifically on the location.
So the original code used pointers & unique_ptr everywhere for consistency.
But that made the code hard to reason about from the outside.
This patch changes the code so that everywhere an API is guaranteed to
return a non-null BreakpointOption, it returns it as a reference to make
that clear.
It also changes the Breakpoint to hold a BreakpointOption
member where it previously had a UP. Since we were always filling the UP
in the Breakpoint constructor, having the UP wasn't helping anything.
Differential Revision: https://reviews.llvm.org/D104162
This converts a default constructor's member initializers into C++11
default member initializers. This patch was automatically generated with
clang-tidy and the modernize-use-default-member-init check.
$ run-clang-tidy.py -header-filter='lldb' -checks='-*,modernize-use-default-member-init' -fix
This is a mass-refactoring patch and this commit will be added to
.git-blame-ignore-revs.
Differential revision: https://reviews.llvm.org/D103483
The C headers are deprecated so as requested in D102845, this is replacing them
all with their (not deprecated) C++ equivalent.
Reviewed By: shafik
Differential Revision: https://reviews.llvm.org/D103084
This patch adds a ScriptedProcess interface to the ScriptInterpreter and
more specifically, to the ScriptInterpreterPython.
This interface will be used in the C++ `ScriptProcess` Process Plugin to
call the script methods.
At the moment, not all methods are implemented, they will upstreamed in
upcoming patches.
This patch also adds helper methods to the ScriptInterpreter to
convert `SBAPI` Types (SBData & SBError) to `lldb_private` types
(DataExtractor & Status).
rdar://65508855
Differential Revision: https://reviews.llvm.org/D95711
Signed-off-by: Med Ismail Bennani <medismail.bennani@gmail.com>
This function would fail in debug builds, as the two usages of the
LLDB_PYTHON_RELATIVE_LIBDIR macro would expand to two distinct strings.
The path iterator macros don't support that.
Use a temporary variable to ensure everything points to a single string.
This patch adds a ScriptedProcess interface to the ScriptInterpreter and
more specifically, to the ScriptInterpreterPython.
This interface will be used in the C++ `ScriptProcess` Process Plugin to
call the script methods.
At the moment, not all methods are implemented, they will upstreamed in
upcoming patches.
This patch also adds helper methods to the ScriptInterpreter to
convert `SBAPI` Types (SBData & SBError) to `lldb_private` types
(DataExtractor & Status).
rdar://65508855
Differential Revision: https://reviews.llvm.org/D95711
Signed-off-by: Med Ismail Bennani <medismail.bennani@gmail.com>
Our code for locating the shared library directory works via dladdr (or
the windows equivalent) to locate the path of an address known to reside
in liblldb. This works great for C++ programs, but there's a catch.
When (lib)lldb is used from python (like in our test suite), this dladdr
call will return a path to the _lldb.so (or such) file in the python
directory. To compensate for this, we have code which attempts to
resolve this symlink, to ensure we get the canonical location. However,
here's the second catch.
On windows, this file is not a symlink (but a copy), so this logic
fails. Since most of our other paths are derived from the liblldb
location, all of these paths will be wrong, when running the test suite.
One effect of this was the failure to find lldb-server in D96202.
To fix this issue, I add some windows-specific code to locate the
liblldb directory. Since it cannot rely on symlinks, it works by
manually walking the directory tree -- essentially doing the opposite of
what we do when computing the python directory.
To avoid python leaking back into the host code, I implement this with
the help of a callback which can be passed to HostInfo::Initialize in
order to assist with the directory location. The callback lives inside
the python plugin.
I also strenghten the existing path test to ensure the returned path is
the right one.
Differential Revision: https://reviews.llvm.org/D96779
LLDB does not like to import Python files with dashes or dots in their
name. While the former are technically allowed, it is discouraged. Dots
are allowed for subpackages but not in module names. This patch improves
the user experience by printing a useful error.
Before this patch:
error: module importing failed: SyntaxError('invalid syntax',
('<string>', 1, 11, 'import foo-bar\n'))
After this patch:
error: module importing failed: Python discourages dashes in module
names: foo-bar
rdar://74263511
[1] https://www.python.org/dev/peps/pep-0008/#package-and-module-names
Differential revision: https://reviews.llvm.org/D96833
This patch introduces a LLDB_SCOPED_TIMER macro to hide the needlessly
repetitive creation of scoped timers in LLDB. It's similar to the
LLDB_LOG(F) macro.
Differential revision: https://reviews.llvm.org/D93663
The reproducers currently use a static variable to track the API
boundary. This is obviously incorrect when the SB API is used
concurrently. While I do not plan to support that use-case (right now),
I do want to avoid us crashing. As a first step, correctly track API
boundaries across multiple threads.
Before this patch SB API calls made by the embedded script interpreter
would be considered "behind the API boundary" and correctly ignored.
After this patch, we need to tell the reproducers to ignore the
scripting thread as a "private thread".
Differential revision: https://reviews.llvm.org/D92811
The code that gets the ScriptInterpreter was not considering the
case that it receives a Lua interpreter.
Differential Revision: https://reviews.llvm.org/D92249
Make it possible to use a relative path in command script import to the
location of the file being sourced. This allows the user to put Python
scripts next to LLDB command files and importing them without having to
specify an absolute path.
To enable this behavior pass `-c` to `command script import`. The
argument can only be used when sourcing the command from a file.
rdar://68310384
Differential revision: https://reviews.llvm.org/D89334
Currently we only strip the Python extension when the file exists on
disk because we assumed that if it didn't exist it was a module.
However, with the change from D89334 this is no longer the case as we
want to be able to import a relative path to a .py as a module. Since we
always import a scripting module as a "python module" we should always
strip the extension if present.
Differential revision: https://reviews.llvm.org/D89352
This reverts commit f775fe5964.
I fixed a return type error in the original patch that was causing a test failure.
Also added a REQUIRES: python to the shell test so we'll skip this for
people who build lldb w/o Python.
Also added another test for the error printing.
Dave Lee