[Lldb-commits] [PATCH] D98482: [lldb] Support for multiprocess extension

[Pavel Labath via Phabricator via lldb-commits]

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