labath added a comment.
I think it'd be better to commit this in two patches after all. The first one
could include the server bits (and the multiprocess+ server thingy) -- I
believe this is good to go already. The second patch/review could deal with the
thread/process id business on the client (and finally enable multiprocess+
client-side).
================
Comment at: lldb/include/lldb/Utility/StringExtractorGDBRemote.h:197
+ static constexpr lldb::pid_t AllProcesses = UINT64_MAX;
+ static constexpr lldb::tid_t AllThreads = UINT64_MAX;
+
----------------
These also need a definition in the .cpp file (debug builds fail without that).
================
Comment at:
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp:741
if (response.GetChar() == 'C') {
- m_curr_pid = response.GetHexMaxU32(false, LLDB_INVALID_PROCESS_ID);
+ m_curr_pid = response.GetHexMaxU64(false, LLDB_INVALID_PROCESS_ID);
if (m_curr_pid != LLDB_INVALID_PROCESS_ID) {
----------------
mgorny wrote:
> I've noticed a few places where we take U32 instead of U64, so fixed that as
> well.
Maybe just commit that separately (no review needed).
================
Comment at:
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp:2795
std::vector<lldb::tid_t> &thread_ids, bool &sequence_mutex_unavailable) {
+ // lldb::pid_t pid = GetCurrentProcessID();
+ lldb::pid_t pid = 0;
----------------
mgorny wrote:
> @labath, any clue if we could make this work some other way? Calling
> `GetCurrentProcessID()` causes a crash for some tests here:
>
> ```
> Fatal Python error: Segmentation fault
>
> Thread 0x00007f918d203640 (most recent call first):
> File
> "/home/mgorny/git/llvm-project/lldb/test/API/functionalities/gdb_remote_client/gdbclientutils.py",
> line 470 in _handlePacket
> File
> "/home/mgorny/git/llvm-project/lldb/test/API/functionalities/gdb_remote_client/gdbclientutils.py",
> line 380 in _receive
> File
> "/home/mgorny/git/llvm-project/lldb/test/API/functionalities/gdb_remote_client/gdbclientutils.py",
> line 364 in _run
> File "/usr/lib/python3.10/threading.py", line 910 in run
> File "/usr/lib/python3.10/threading.py", line 972 in _bootstrap_inner
> File "/usr/lib/python3.10/threading.py", line 930 in _bootstrap
>
> Current thread 0x00007f9196327740 (most recent call first):
> File
> "/home/mgorny/git/llvm-project/build.gentoo/lib/python3.10/site-packages/lldb/__init__.py",
> line 10250 in ConnectRemote
> File
> "/home/mgorny/git/llvm-project/lldb/test/API/functionalities/gdb_remote_client/gdbclientutils.py",
> line 526 in connect
> File
> "/home/mgorny/git/llvm-project/lldb/test/API/functionalities/gdb_remote_client/TestRecognizeBreakpoint.py",
> line 103 in test
> File
> "/home/mgorny/git/llvm-project/lldb/third_party/Python/module/unittest2/unittest2/case.py",
> line 413 in runMethod
> File
> "/home/mgorny/git/llvm-project/lldb/third_party/Python/module/unittest2/unittest2/case.py",
> line 383 in run
> File
> "/home/mgorny/git/llvm-project/lldb/third_party/Python/module/unittest2/unittest2/case.py",
> line 458 in __call__
> File
> "/home/mgorny/git/llvm-project/lldb/third_party/Python/module/unittest2/unittest2/suite.py",
> line 117 in _wrapped_run
> File
> "/home/mgorny/git/llvm-project/lldb/third_party/Python/module/unittest2/unittest2/suite.py",
> line 115 in _wrapped_run
> File
> "/home/mgorny/git/llvm-project/lldb/third_party/Python/module/unittest2/unittest2/suite.py",
> line 85 in run
> File
> "/home/mgorny/git/llvm-project/lldb/third_party/Python/module/unittest2/unittest2/suite.py",
> line 66 in __call__
> File
> "/home/mgorny/git/llvm-project/lldb/third_party/Python/module/unittest2/unittest2/runner.py",
> line 165 in run
> File
> "/home/mgorny/git/llvm-project/lldb/packages/Python/lldbsuite/test/dotest.py",
> line 997 in run_suite
> File "/home/mgorny/git/llvm-project/lldb/test/API/dotest.py", line 7 in
> <module>
>
> Extension modules: lldb._lldb (total: 1)
> ```
This backtrace is useless, as it only shows python threads (and the crash
happens in a native one), but if you run the test in a debugger, the problem
becomes obvious -- mutual recursion between `GetCurrentThreadIDs` and
`GetCurrentProcessID`. To fix that, we need to break it.
I'd probably to in by creating a new function, which implements the core of
q[fs]ThreadInfo functionality (for instance, it could return something like
(`vector<pair<pid, tid>>` with INVALID_PID for unknown), and then call it from
both of these functions to do their thing.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D98482/new/
https://reviews.llvm.org/D98482
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
