labath added a comment. In D100191#2704748 <https://reviews.llvm.org/D100191#2704748>, @mgorny wrote:
> In D100191#2704681 <https://reviews.llvm.org/D100191#2704681>, @labath wrote: > >> In D100191#2704492 <https://reviews.llvm.org/D100191#2704492>, @mgorny wrote: >> >>> In D100191#2704403 <https://reviews.llvm.org/D100191#2704403>, @labath >>> wrote: >>> >>>> Let's identify the set of patches needed to make this testable via the >>>> lldb-server suite (this one, D100153 <https://reviews.llvm.org/D100153>, >>>> D100208 <https://reviews.llvm.org/D100208> (or equivalent for some other >>>> os), and what else?) and test that? >>> >>> In its current form, D100208 <https://reviews.llvm.org/D100208> relies at >>> least on D100196 <https://reviews.llvm.org/D100196> as well. I suppose we >>> might get away without other patches for now. My logic is that as long as >>> client doesn't indicate fork support, the regular LLDB behavior won't >>> change. We can mock-enable `fork-events` in the server tests to get things >>> rolling, unless I'm missing something. >> >> Sounds great. >> >>> I think we could avoid merging D100196 <https://reviews.llvm.org/D100196> >>> if I split D100208 <https://reviews.llvm.org/D100208> into two parts, the >>> earlier part just calling `NewSubprocess()` without actually reporting >>> stop. This won't be really functional (or used in real sessions) but should >>> suffice for testing. >> >> I'm not sure how would that work -- you'd still have to provide a way to >> notify the client (test) about the new process and its pid, so that the test >> can assert something meaningful. Let's just keep D100196 >> <https://reviews.llvm.org/D100196>, but leave out the client specific parts >> -- just keep it about adding the new enums and relevant server code (trivial >> case switches are fine). > > Do you mean the `DidFoo()` methods, or the whole `StopInfo` stuff along with > the logic in `ProcessGDBRemote`? All of it. > If the latter, should I add some asserts for the unhandled stop reasons or > just ignore them for now? Nah, just ignore it completely lldb-server will not be sending those as long as the client does not advertise support. It's possible lldb will misbehave if a broken/evil server sends reasons like these, but that's not a problem for this patch. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D100191/new/ https://reviews.llvm.org/D100191 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
