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

2021-03-17 Thread Michał Górny via Phabricator via lldb-commits
mgorny updated this revision to Diff 331327. mgorny marked 2 inline comments as done. mgorny added a comment. - switched thread-id parsing to use `view.consumeInteger()` and replaced `m_index` manipulations with a single update at the very end - added additional test for parsing multiple thread-i

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

2021-03-17 Thread Michał Górny via Phabricator via lldb-commits
mgorny added a comment. In D98482#2631817 , @labath wrote: > In D98482#2626237 , @mgorny wrote: > >> Create a new `GDBRemoteError` class to pass gdb-remote `$E` codes through >> cleanly. > > The error codes we use

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

2021-03-17 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. In D98482#2626237 , @mgorny wrote: > Create a new `GDBRemoteError` class to pass gdb-remote `$E` codes through > cleanly. The error codes we use right now are completely meaningless, so don't bother preserving them. I don't think

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

2021-03-17 Thread Michał Górny via Phabricator via lldb-commits
mgorny updated this revision to Diff 331220. mgorny edited the summary of this revision. mgorny added a comment. I've just discovered that `LLDB_INVALID_*_ID` isn't -1/UINT_MAX as I thought, so I've had to introduce additional constants. That said, I'm not convinced about the current 0/-1 API. M

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

2021-03-15 Thread Michał Górny via Phabricator via lldb-commits
mgorny updated this revision to Diff 330674. mgorny added a comment. clang-format. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D98482/new/ https://reviews.llvm.org/D98482 Files: lldb/include/lldb/Utility/GDBRemoteError.h lldb/include/lldb/Utility/StringExtractorGDBRemote.h lldb/

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

2021-03-15 Thread Michał Górny via Phabricator via lldb-commits
mgorny updated this revision to Diff 330671. mgorny edited the summary of this revision. mgorny added a comment. Updated `GetPidTid()` to use `llvm::Optional>` return type with a default PID approach as suggested by @labath. Updated `ReadTid()` to use `llvm::Expected`. Create a new `GDBRemoteErr

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

2021-03-15 Thread Michał Górny via Phabricator via lldb-commits
mgorny added a comment. Or scratch that. What you propose is cleaner and requires less changes to tests ;-). CHANGES SINCE LAST ACTION https://reviews.llvm.org/D98482/new/ https://reviews.llvm.org/D98482 ___ lldb-commits mailing list lldb-commits@

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

2021-03-15 Thread Michał Górny via Phabricator via lldb-commits
mgorny added a comment. In D98482#2624973 , @labath wrote: > I'm still bugged by the GetPidTid interface -- the pair of optionals is very > complicated. What if we had this function take a `default_pid` argument > (which would be equal to the currently s

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

2021-03-14 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. I'm still bugged by the GetPidTid interface -- the pair of optionals is very complicated. What if we had this function take a `default_pid` argument (which would be equal to the currently selected process), and it returned that as the process id, in case the packet does

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

2021-03-13 Thread Michał Górny via Phabricator via lldb-commits
mgorny updated this revision to Diff 330441. mgorny added a comment. Added support for 0 and -1 values in `ReadTid()` helper, controlled via `allow_any` and `allow_all` parameters. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D98482/new/ https://reviews.llvm.org/D98482 Files: lldb/i

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

2021-03-13 Thread Michał Górny via Phabricator via lldb-commits
mgorny updated this revision to Diff 330435. mgorny added a comment. Add a `GDBRemoteCommunicationServerLLGS::ReadTid()` helper that does appropriate PID verification and constructs error packets. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D98482/new/ https://reviews.llvm.org/D98482

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

2021-03-13 Thread Michał Górny via Phabricator via lldb-commits
mgorny added a comment. In D98482#2623005 , @labath wrote: > - I wouldn't add `multiprocess` to the qSupported packets just yet. Since > this is accepting the extended syntax regardless of whether the other side > claimed to support it (there's no hard i

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

2021-03-13 Thread Michał Górny via Phabricator via lldb-commits
mgorny updated this revision to Diff 330432. mgorny marked an inline comment as done. mgorny added a comment. Updated utility function to use `StringRef` and reformatted. Further changes incoming. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D98482/new/ https://reviews.llvm.org/D98482

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

2021-03-12 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. Some initial remarks: - the parsing function is clearly gdb-related, so it should go into StringExtractorGDBRemote.h, and not its base class - the universalness of the function makes its interface pretty complicated, and bloats the call sites. Its probably good for the l

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

2021-03-12 Thread Michał Górny via Phabricator via lldb-commits
mgorny updated this revision to Diff 330225. mgorny edited the summary of this revision. mgorny added a comment. Updated to include `multiprocess` in `qSupported`, and proposed how to handle it server-side on the example of `H` packet. Basically, accept the correct PID of current process, reject

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

2021-03-12 Thread Michał Górny via Phabricator via lldb-commits
mgorny created this revision. mgorny added reviewers: labath, emaste, krytarowski. mgorny requested review of this revision. This is a work-in-progress to provide minimal support for multiprocess GDB protocol extension. For a start, a new function to parse thread-ids accounting for the possible