llvm / llvm-project

The LLVM Project is a collection of modular and reusable compiler and toolchain technologies.
http://llvm.org
Other
28.49k stars 11.78k forks source link

[lldb] Possible design issue when using Python >= 3.12 #70453

Closed tuliom closed 8 months ago

tuliom commented 11 months ago

While trying to update lldb to work with Python >= 3.12, I found the following issue:

Starting with Python 3.12, there is code now preventing the usage of PyImport_AppendInittab after Py_Initialize(). However, according to this source code comment:

We cannot control what external code may have done before getting to this point in LLDB,
including potentially having already initialized Python...

This is causing the following failures:

Failed Tests (19):
  lldb-unit :: ScriptInterpreter/Python/./ScriptInterpreterPythonTests/0/20
  lldb-unit :: ScriptInterpreter/Python/./ScriptInterpreterPythonTests/1/20
  lldb-unit :: ScriptInterpreter/Python/./ScriptInterpreterPythonTests/10/20
  lldb-unit :: ScriptInterpreter/Python/./ScriptInterpreterPythonTests/11/20
  lldb-unit :: ScriptInterpreter/Python/./ScriptInterpreterPythonTests/12/20
  lldb-unit :: ScriptInterpreter/Python/./ScriptInterpreterPythonTests/13/20
  lldb-unit :: ScriptInterpreter/Python/./ScriptInterpreterPythonTests/14/20
  lldb-unit :: ScriptInterpreter/Python/./ScriptInterpreterPythonTests/15/20
  lldb-unit :: ScriptInterpreter/Python/./ScriptInterpreterPythonTests/16/20
  lldb-unit :: ScriptInterpreter/Python/./ScriptInterpreterPythonTests/17/20
  lldb-unit :: ScriptInterpreter/Python/./ScriptInterpreterPythonTests/18/20
  lldb-unit :: ScriptInterpreter/Python/./ScriptInterpreterPythonTests/2/20
  lldb-unit :: ScriptInterpreter/Python/./ScriptInterpreterPythonTests/3/20
  lldb-unit :: ScriptInterpreter/Python/./ScriptInterpreterPythonTests/4/20
  lldb-unit :: ScriptInterpreter/Python/./ScriptInterpreterPythonTests/5/20
  lldb-unit :: ScriptInterpreter/Python/./ScriptInterpreterPythonTests/6/20
  lldb-unit :: ScriptInterpreter/Python/./ScriptInterpreterPythonTests/7/20
  lldb-unit :: ScriptInterpreter/Python/./ScriptInterpreterPythonTests/8/20
  lldb-unit :: ScriptInterpreter/Python/./ScriptInterpreterPythonTests/9/20

With the following output:

Fatal Python error: PyImport_AppendInittab: PyImport_AppendInittab() may not be called after Py_Initialize()
Python runtime state: initialized

Current thread 0x00007f60b876c300 (most recent call first):
  <no Python frame>
 #0 0x00007f60b059bceb llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) (/build/llvm-build.f39/lib/libLLVM-
18git.so+0x79bceb)
 #1 0x00007f60b05992ab SignalHandler(int) Signals.cpp:0:0
 #2 0x00007f60afa089a0 __restore_rt (/lib64/libc.so.6+0x3e9a0)
 #3 0x00007f60afa5a834 __pthread_kill_implementation (/lib64/libc.so.6+0x90834)
 #4 0x00007f60afa088ee gsignal (/lib64/libc.so.6+0x3e8ee)
 #5 0x00007f60af9f08ff abort (/lib64/libc.so.6+0x268ff)
 #6 0x00007f60b435acaa /usr/src/debug/python3.12-3.12.0-1.fc39.x86_64/Python/pylifecycle.c:2712:0
 #7 0x00007f60b435acaa /usr/src/debug/python3.12-3.12.0-1.fc39.x86_64/Python/pylifecycle.c:2820:0
 #8 0x00007f60b435b020 /usr/src/debug/python3.12-3.12.0-1.fc39.x86_64/Modules/faulthandler.c:1035:0
 #9 0x00007f60b42b2a6c /usr/include/bits/string_fortified.h:59:10
#10 0x00007f60b42b2a6c PyImport_AppendInittab /usr/src/debug/python3.12-3.12.0-1.fc39.x86_64/Python/import.c:1
502:5
#11 0x0000000000480faf lldb_private::ScriptInterpreterPythonImpl::Initialize() (/build/llvm-build.f39/tools/ll
db/unittests/ScriptInterpreter/Python/./ScriptInterpreterPythonTests+0x480faf)
#12 0x000000000044cded PythonTestSuite::SetUp() (/build/llvm-build.f39/tools/lldb/unittests/ScriptInterpreter/
Python/./ScriptInterpreterPythonTests+0x44cded)
#13 0x0000000000441789 PythonDataObjectsTest::SetUp() NameMatches.cpp:0:0
#14 0x0000000000465687 testing::Test::Run() (.part.0) gtest-all.cc:0:0
#15 0x000000000046a58a testing::TestInfo::Run() (/build/llvm-build.f39/tools/lldb/unittests/ScriptInterpreter/
Python/./ScriptInterpreterPythonTests+0x46a58a)
#16 0x000000000046abcc testing::TestSuite::Run() (.part.0) gtest-all.cc:0:0
#17 0x0000000000471d10 testing::internal::UnitTestImpl::RunAllTests() (/build/llvm-build.f39/tools/lldb/unitte
sts/ScriptInterpreter/Python/./ScriptInterpreterPythonTests+0x471d10)
#18 0x000000000046503e testing::UnitTest::Run() (/build/llvm-build.f39/tools/lldb/unittests/ScriptInterpreter/
Python/./ScriptInterpreterPythonTests+0x46503e)
#19 0x000000000042ab26 main (/build/llvm-build.f39/tools/lldb/unittests/ScriptInterpreter/Python/./ScriptInter
preterPythonTests+0x42ab26)
#20 0x00007f60af9f214a __libc_start_call_main (/lib64/libc.so.6+0x2814a)
#21 0x00007f60af9f220b __libc_start_main@GLIBC_2.2.5 (/lib64/libc.so.6+0x2820b)
#22 0x000000000042b1a5 _start (/build/llvm-build.f39/tools/lldb/unittests/ScriptInterpreter/Python/./ScriptInt
erpreterPythonTests+0x42b1a5)

One can reduce the list of failures to 2 by guaranteeing the critical code in InitializePythonRAII() is executed only once. But it still doesn't solve all the cases.

llvmbot commented 11 months ago

@llvm/issue-subscribers-lldb

Author: Tulio Magno Quites Machado Filho (tuliom)

While trying to update lldb to work with Python >= 3.12, I found the following issue: Starting with Python 3.12, there is code now preventing the usage of `PyImport_AppendInittab` after `Py_Initialize()`. However, according to [this source code comment](https://github.com/llvm/llvm-project/blob/7a1e8783586ecc90ee15f12c7b76799313bb32e8/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp#L88-L90): ``` We cannot control what external code may have done before getting to this point in LLDB, including potentially having already initialized Python... ``` This is causing the following failures: ``` Failed Tests (19): lldb-unit :: ScriptInterpreter/Python/./ScriptInterpreterPythonTests/0/20 lldb-unit :: ScriptInterpreter/Python/./ScriptInterpreterPythonTests/1/20 lldb-unit :: ScriptInterpreter/Python/./ScriptInterpreterPythonTests/10/20 lldb-unit :: ScriptInterpreter/Python/./ScriptInterpreterPythonTests/11/20 lldb-unit :: ScriptInterpreter/Python/./ScriptInterpreterPythonTests/12/20 lldb-unit :: ScriptInterpreter/Python/./ScriptInterpreterPythonTests/13/20 lldb-unit :: ScriptInterpreter/Python/./ScriptInterpreterPythonTests/14/20 lldb-unit :: ScriptInterpreter/Python/./ScriptInterpreterPythonTests/15/20 lldb-unit :: ScriptInterpreter/Python/./ScriptInterpreterPythonTests/16/20 lldb-unit :: ScriptInterpreter/Python/./ScriptInterpreterPythonTests/17/20 lldb-unit :: ScriptInterpreter/Python/./ScriptInterpreterPythonTests/18/20 lldb-unit :: ScriptInterpreter/Python/./ScriptInterpreterPythonTests/2/20 lldb-unit :: ScriptInterpreter/Python/./ScriptInterpreterPythonTests/3/20 lldb-unit :: ScriptInterpreter/Python/./ScriptInterpreterPythonTests/4/20 lldb-unit :: ScriptInterpreter/Python/./ScriptInterpreterPythonTests/5/20 lldb-unit :: ScriptInterpreter/Python/./ScriptInterpreterPythonTests/6/20 lldb-unit :: ScriptInterpreter/Python/./ScriptInterpreterPythonTests/7/20 lldb-unit :: ScriptInterpreter/Python/./ScriptInterpreterPythonTests/8/20 lldb-unit :: ScriptInterpreter/Python/./ScriptInterpreterPythonTests/9/20 ``` With the following output: ``` Fatal Python error: PyImport_AppendInittab: PyImport_AppendInittab() may not be called after Py_Initialize() Python runtime state: initialized Current thread 0x00007f60b876c300 (most recent call first): <no Python frame> #0 0x00007f60b059bceb llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) (/build/llvm-build.f39/lib/libLLVM- 18git.so+0x79bceb) #1 0x00007f60b05992ab SignalHandler(int) Signals.cpp:0:0 #2 0x00007f60afa089a0 __restore_rt (/lib64/libc.so.6+0x3e9a0) #3 0x00007f60afa5a834 __pthread_kill_implementation (/lib64/libc.so.6+0x90834) #4 0x00007f60afa088ee gsignal (/lib64/libc.so.6+0x3e8ee) #5 0x00007f60af9f08ff abort (/lib64/libc.so.6+0x268ff) #6 0x00007f60b435acaa /usr/src/debug/python3.12-3.12.0-1.fc39.x86_64/Python/pylifecycle.c:2712:0 #7 0x00007f60b435acaa /usr/src/debug/python3.12-3.12.0-1.fc39.x86_64/Python/pylifecycle.c:2820:0 #8 0x00007f60b435b020 /usr/src/debug/python3.12-3.12.0-1.fc39.x86_64/Modules/faulthandler.c:1035:0 #9 0x00007f60b42b2a6c /usr/include/bits/string_fortified.h:59:10 #10 0x00007f60b42b2a6c PyImport_AppendInittab /usr/src/debug/python3.12-3.12.0-1.fc39.x86_64/Python/import.c:1 502:5 #11 0x0000000000480faf lldb_private::ScriptInterpreterPythonImpl::Initialize() (/build/llvm-build.f39/tools/ll db/unittests/ScriptInterpreter/Python/./ScriptInterpreterPythonTests+0x480faf) #12 0x000000000044cded PythonTestSuite::SetUp() (/build/llvm-build.f39/tools/lldb/unittests/ScriptInterpreter/ Python/./ScriptInterpreterPythonTests+0x44cded) #13 0x0000000000441789 PythonDataObjectsTest::SetUp() NameMatches.cpp:0:0 #14 0x0000000000465687 testing::Test::Run() (.part.0) gtest-all.cc:0:0 #15 0x000000000046a58a testing::TestInfo::Run() (/build/llvm-build.f39/tools/lldb/unittests/ScriptInterpreter/ Python/./ScriptInterpreterPythonTests+0x46a58a) #16 0x000000000046abcc testing::TestSuite::Run() (.part.0) gtest-all.cc:0:0 #17 0x0000000000471d10 testing::internal::UnitTestImpl::RunAllTests() (/build/llvm-build.f39/tools/lldb/unitte sts/ScriptInterpreter/Python/./ScriptInterpreterPythonTests+0x471d10) #18 0x000000000046503e testing::UnitTest::Run() (/build/llvm-build.f39/tools/lldb/unittests/ScriptInterpreter/ Python/./ScriptInterpreterPythonTests+0x46503e) #19 0x000000000042ab26 main (/build/llvm-build.f39/tools/lldb/unittests/ScriptInterpreter/Python/./ScriptInter preterPythonTests+0x42ab26) #20 0x00007f60af9f214a __libc_start_call_main (/lib64/libc.so.6+0x2814a) #21 0x00007f60af9f220b __libc_start_main@GLIBC_2.2.5 (/lib64/libc.so.6+0x2820b) #22 0x000000000042b1a5 _start (/build/llvm-build.f39/tools/lldb/unittests/ScriptInterpreter/Python/./ScriptInt erpreterPythonTests+0x42b1a5) ``` One can reduce the list of failures to 2 by guaranteeing the critical code in `InitializePythonRAII()` is executed only once. But it still doesn't solve all the cases.
bulbazord commented 11 months ago

@tuliom How far did you get here? I know you were able to do some work with LLDB's Python usage, is the failure list still at those 19?

I assume we (the LLDB maintainers) will need to contend with this at some point in the near future, so thank you for opening this issue.

JDevlieghere commented 11 months ago

FWIW I also just hit this with Python 3.12.

tuliom commented 11 months ago

@tuliom How far did you get here? I know you were able to do some work with LLDB's Python usage, is the failure list still at those 19?

@bulbazord I didn't start working on this issue. The 19 failures are remaining failures after my recent changes. I'm afraid that a larger re-design of the plugins mechanism is required in order to solve this issue.

bulbazord commented 11 months ago

@tuliom How far did you get here? I know you were able to do some work with LLDB's Python usage, is the failure list still at those 19?

@bulbazord I didn't start working on this issue. The 19 failures are remaining failures after my recent changes. I'm afraid that a larger re-design of the plugins mechanism is required in order to solve this issue.

Gotcha. Redesigning the python plugins is no small task but it will have to be done eventually. Thanks for bringing this to our attention.

torokati44 commented 10 months ago

On Fedora 39, with the python3-lldb package installed (Python 3.12.0, LLVM 17.0.4), all one has to do to get this crash is:

$ python3
>>> import lldb

This leaves the Scripting Bridge API essentially unusable.

Of course, OP might be familiar with this, being the packager of said Fedora package... :)

EDIT: One workaround is to install the python3.11 package, and start the interpreter like this: PYTHONPATH=$(lldb -P) python3.11 [source].

ferdnyc commented 8 months ago

@tuliom

While trying to update lldb to work with Python >= 3.12, I found the following issue: Starting with Python 3.12, there is code now preventing the usage of PyImport_AppendInittab after Py_Initialize().

Which sounds like a whole big mess, except for this one tantalizing note in the Python C API docs on initialization

If Python is initialized multiple times, PyImport_AppendInittab() or PyImport_ExtendInittab() must be called before each Python initialization.

While lldb can't control what other code has done before running python, including possibly initializing it, perhaps it's possible to reinitialize it, and therefore have access to PyImport_AppendInittab() again.

Presumably that would only be possible when embedding Python into LLDB using the C API, where the initialization can be adjusted ahead-of-time. Whereas the extension module might be better off avoiding any attempts to meddle with the Python initialization.

After all, what if the environment is an already-initialized, standalone Python interpreter that's only now loading the extension? Like in @torokati44's example of typing import lldb at the REPL prompt. Not only is Python readline already loaded, there, but it's active and managing interactive state for the user. Swapping it out in that context sounds like a Bad Time™.

cc: @bulbazord

JDevlieghere commented 8 months ago

This came up again today and I decided to give this another go. I've convinced myself that it's safe (and correct) to guard the call to PyImport_AppendInittab behind a check that Python hasn't been initialized yet. I've explained my reasoning in #82095.

The two remaining unit test failures are a different issue with Python 3.12 that I haven't figured out yet. The reason I think that is because the tests don't actually use anything from the lldb module, and the issue also reproduces when skipping the call to ScriptInterpreterPythonImpl::Initialize and doing it by hand (#82096).

JDevlieghere commented 8 months ago

82098 fixes the two remaining unit tests.

torokati44 commented 7 months ago

Until this is released, and gets included in the majority of the mainstream distros... I had the brilliant idea of somehow using lldb and its script command as if it was a python3 interpreter, with its own binding module already loaded. Can this work? Is this silly?

Of course, one could write a small C++ program that uses CommandObjectScript::DoExecute, or ScriptInterpreterPython, or just InitializePythonRAII, to get the same thing. The problem with this is that I don't know whether this can be achieved through the SB API alone - and if not, that will be dependent on the installed LLVM version, which we'd like to avoid...

rjra100 commented 5 months ago

It's possible I've just got lost in a maze of twisty commits, but it looks like this fix didn't make it into LLDB 18 and also missed the cut for the LLDB 19 branch (or at least, it wasn't at the point tagged llvmorg-19-init). Please could it be merged across before LLDB 19 goes live? I've got some testing of visualisers that uses the scripting bridge to manage the tests, and I'd rather not be stuck running the tests on 17 any longer than necessary. Of course, if there's a workaround, that'd be great. But the only suggestion I've seen above doesn't work (ImportError: cannot import name '_lldb' from partially initialized module 'lldb' (most likely due to a circular import)

jendrikw commented 4 months ago

This also breaks python's own help function:

python3 -c "help('modules')"

Results in

Fatal Python error: PyImport_AppendInittab: PyImport_AppendInittab() may not be called after Py_Initialize()
Python runtime state: initialized

Current thread 0x00007cbe700e3740 (most recent call first):
  File "/usr/lib/python3.12/site-packages/lldb/__init__.py", line 4327 in Initialize
  File "/usr/lib/python3.12/site-packages/lldb/__init__.py", line 15710 in <module>
  File "<frozen importlib._bootstrap>", line 488 in _call_with_frames_removed
  File "<frozen importlib._bootstrap_external>", line 995 in exec_module
  File "<frozen importlib._bootstrap>", line 935 in _load_unlocked
  File "<frozen importlib._bootstrap>", line 1331 in _find_and_load_unlocked
  File "<frozen importlib._bootstrap>", line 1360 in _find_and_load
  File "/usr/lib/python3.12/pkgutil.py", line 78 in walk_packages
  File "/usr/lib/python3.12/pydoc.py", line 2328 in run
  File "/usr/lib/python3.12/pydoc.py", line 2299 in listmodules
  File "/usr/lib/python3.12/pydoc.py", line 2145 in help
  File "/usr/lib/python3.12/pydoc.py", line 2096 in __call__
  File "<frozen _sitebuiltins>", line 103 in __call__
  File "<string>", line 1 in <module>

Extension modules: pyalpm, yaml._yaml, google.protobuf.pyext._message, google3.net.proto2.python.internal.cpp._message, PyQt6.QtCore, PyQt6.QtGui, PyQt6.QtNetwork, PyQt6.QtQml, PyQt6.QtQuick, PyQt6.QtWebChannel, PyQt6.QtWebEngineCore, PyQt6.QtWidgets, PyQt6.QtPrintSupport, PyQt6.QtWebEngineWidgets, gi._gi, cairo._cairo, gi._gi_cairo, markupsafe._speedups, lxml._elementpath, lxml.etree, _dbus_bindings, _blueman, breezy._chunks_to_lines_pyx, breezy._known_graph_pyx, breezy.bzr._simple_set_pyx, breezy.bzr._static_tuple_c, msgpack._cmsgpack, _cffi_backend, evdev._input, evdev._ecodes, evdev._uinput, fastbencode._bencode_pyx, PyQt5.QtCore, PyQt5.QtGui, PyQt5.QtWidgets, PyQt5.QtNetwork, PyQt5.QtSvg, gpg._gpgme, mercurial.cext.parsers, mercurial.zstd, mercurial.thirdparty.sha1dc, mercurial.cext.osutil, mercurial.cext.base85, mercurial.cext.bdiff, mercurial.cext.mpatch, libsvn._client, libsvn._core, libsvn._delta, libsvn._wc, libsvn._ra, hgext.fsmonitor.pywatchman.bser, multidict._multidict, libmount.pylibmount, lldb._lldb (total: 54)
zsh: IOT instruction (core dumped)  python3 -c "help('modules')"
ferdnyc commented 4 months ago

@jendrikw

That pydoc has to import a module to extract its documentation is a known issue with it, and there are a lot of modules that break it. Try running a python3 -m pydoc -k <term> search some time (doesn't matter what the term is), and watch how much noise is spewed while it walks the various packages. Ifyou're lucky lldb is the only one that actually crashes it, I've seen plenty others.

But as far as the current LLDB Python extension goes, until this fix is released it doesn't work anyway, so it's pretty useless to have installed. The simplest solution is probably to just remove /usr/lib/python3.12/site-packages/lldb entirely. (Whether that involves using pip, or your distro package manager, or what have you...)

On Fedora it's sudo dnf remove python3-lldb. That'll also force removal of lldb-devel, which may or may not be a problem. YMMV, of course.

Archdsenna commented 4 months ago

@JDevlieghere Thanks for your submission. I still encounter this problem on lldb 18. It seems that the fix has not been merged into the mainline. Will it be merged in the upcoming version?

cc: @bulbazord